Merge main

Author: edwardneal

doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml

@@ -3604,7 +3604,9 @@ Before you call <xref:Microsoft.Data.SqlClient.SqlCommand.Prepare%2A>, specify t
 
 If you call an `Execute` method after calling <xref:Microsoft.Data.SqlClient.SqlCommand.Prepare%2A>, any parameter value that is larger than the value specified by the <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A> property is automatically truncated to the original specified size of the parameter, and no truncation errors are returned.
 
-Output parameters (whether prepared or not) must have a user-specified data type. If you specify a variable length data type, you must also specify the maximum <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A>.
+Output parameters (whether prepared or not) must have a user-specified data type. If you specify a variable length data type except vector, you must also specify the maximum <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A>.
+
+For vector data types, the <xref:Microsoft.Data.SqlClient.SqlParameter.Size%2A> property is ignored. The size of the vector is inferred from the <xref:Microsoft.Data.SqlClient.SqlParameter.Value%2A> of type <xref:Microsoft.Data.SqlTypes.SqlVector%2A>.
 
 Prior to Visual Studio 2010, <xref:Microsoft.Data.SqlClient.SqlCommand.Prepare%2A> threw an exception.  Beginning in Visual Studio 2010, this method does not throw an exception.
 

doc/snippets/Microsoft.Data.SqlTypes/SqlVector.xml

@@ -5,20 +5,11 @@
       <summary>Represents a vector value in SQL Server.</summary>
     </SqlVector>
     <ctor1>
-      <param name="length"></param>
-      <summary>
-        Constructs a null vector of the given length. SQL Server requires vector arguments to specify their length even when null.
-      </summary>
-      <exception cref="T:System.ArgumentOutOfRangeException">
-        Vector length must be non-negative.
-      </exception>
-    </ctor1>
-    <ctor2>
       <param name="memory"></param>
       <summary>
         Constructs a vector with the given values.
       </summary>
-    </ctor2>
+    </ctor1>
     <IsNull>
       <inheritdoc/>
     </IsNull>
@@ -37,13 +28,17 @@
         Returns the number of elements in the vector.
       </summary>
     </Length>
-    <Size>
-      <summary>
-        Returns the number of bytes required to represent this vector when communicating with SQL Server.
-      </summary>
-    </Size>
     <Memory>
       <summary>Returns the vector values as a memory region. No copies are made.</summary>
     </Memory>
+    <CreateNull>
+      <param name="length"></param>
+      <summary>
+        Constructs a null vector of the given length. SQL Server requires vector arguments to specify their length even when null.
+      </summary>
+      <exception cref="T:System.ArgumentOutOfRangeException">
+        Vector length must be non-negative.
+      </exception>
+    </CreateNull>
   </members>
 </docs>

src/Directory.Packages.props

@@ -10,6 +10,7 @@
         <PackageVersion Include="System.Memory" Version="4.5.5" />
         <PackageVersion Include="System.Data.Common" Version="4.3.0" />
         <PackageVersion Include="System.Text.Encodings.Web" Version="8.0.0" />
+        <PackageVersion Include="System.ValueTuple" Version="4.6.1" />
     </ItemGroup>
     <!-- NetFx and NetCore project dependencies -->
     <ItemGroup>

src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/AzureSqlKeyCryptographer.cs

@@ -7,12 +7,12 @@
 using Azure.Security.KeyVault.Keys.Cryptography;
 using System;
 using System.Collections.Concurrent;
-using System.Threading.Tasks;
+using System.Threading;
 using static Azure.Security.KeyVault.Keys.Cryptography.SignatureAlgorithm;
 
 namespace Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider
 {
-    internal class AzureSqlKeyCryptographer
+    internal sealed class AzureSqlKeyCryptographer : IDisposable
     {
         /// <summary>
         /// TokenCredential to be used with the KeyClient
@@ -25,16 +25,14 @@ internal class AzureSqlKeyCryptographer
         private readonly ConcurrentDictionary<Uri, KeyClient> _keyClientDictionary = new();
 
         /// <summary>
-        /// Holds references to the fetch key tasks and maps them to their corresponding Azure Key Vault Key Identifier (URI).
-        /// These tasks will be used for returning the key in the event that the fetch task has not finished depositing the 
-        /// key into the key dictionary.
+        /// Holds references to the Azure Key Vault keys and maps them to their corresponding Azure Key Vault Key Identifier (URI).
         /// </summary>
-        private readonly ConcurrentDictionary<string, Task<Azure.Response<KeyVaultKey>>> _keyFetchTaskDictionary = new();
+        private readonly ConcurrentDictionary<string, KeyVaultKey> _keyDictionary = new();
 
         /// <summary>
-        /// Holds references to the Azure Key Vault keys and maps them to their corresponding Azure Key Vault Key Identifier (URI).
+        /// SemaphoreSlim to ensure thread safety when accessing the key dictionary or making network calls to Azure Key Vault to fetch keys.
         /// </summary>
-        private readonly ConcurrentDictionary<string, KeyVaultKey> _keyDictionary = new();
+        private SemaphoreSlim _keyDictionarySemaphore = new(1, 1);
 
         /// <summary>
         /// Holds references to the Azure Key Vault CryptographyClient objects and maps them to their corresponding Azure Key Vault Key Identifier (URI).
@@ -50,20 +48,44 @@ internal AzureSqlKeyCryptographer(TokenCredential tokenCredential)
             TokenCredential = tokenCredential;
         }
 
+        /// <summary>
+        /// Disposes the SemaphoreSlim used for thread safety.
+        /// </summary>
+        public void Dispose()
+        {
+            _keyDictionarySemaphore.Dispose();
+        }
+
         /// <summary>
         /// Adds the key, specified by the Key Identifier URI, to the cache.
+        /// Validates the key type and fetches the key from Azure Key Vault if it is not already cached.
         /// </summary>
         /// <param name="keyIdentifierUri"></param>
         internal void AddKey(string keyIdentifierUri)
         {
-            if (TheKeyHasNotBeenCached(keyIdentifierUri))
+            // Allow only one thread to proceed to ensure thread safety
+            // as we will need to fetch key information from Azure Key Vault if the key is not found in cache.
+            _keyDictionarySemaphore.Wait();
+
+            try
             {
-                ParseAKVPath(keyIdentifierUri, out Uri vaultUri, out string keyName, out string keyVersion);
-                CreateKeyClient(vaultUri);
-                FetchKey(vaultUri, keyName, keyVersion, keyIdentifierUri);
-            }
+                if (!_keyDictionary.ContainsKey(keyIdentifierUri))
+                {
+                    ParseAKVPath(keyIdentifierUri, out Uri vaultUri, out string keyName, out string keyVersion);
+
+                    // Fetch the KeyClient for the Key vault URI.
+                    KeyClient keyClient = GetOrCreateKeyClient(vaultUri);
+
+                    // Fetch the key from Azure Key Vault.
+                    KeyVaultKey key = FetchKeyFromKeyVault(keyClient, keyName, keyVersion);
 
-            bool TheKeyHasNotBeenCached(string k) => !_keyDictionary.ContainsKey(k) && !_keyFetchTaskDictionary.ContainsKey(k);
+                    _keyDictionary.AddOrUpdate(keyIdentifierUri, key, (k, v) => key);
+                }
+            }
+            finally
+            {
+                _keyDictionarySemaphore.Release();
+            }
         }
 
         /// <summary>
@@ -75,18 +97,12 @@ internal KeyVaultKey GetKey(string keyIdentifierUri)
         {
             if (_keyDictionary.TryGetValue(keyIdentifierUri, out KeyVaultKey key))
             {
-                AKVEventSource.Log.TryTraceEvent("Fetched master key from cache");
+                AKVEventSource.Log.TryTraceEvent("Fetched key name={0} from cache", key.Name);
                 return key;
             }
 
-            if (_keyFetchTaskDictionary.TryGetValue(keyIdentifierUri, out Task<Azure.Response<KeyVaultKey>> task))
-            {
-                AKVEventSource.Log.TryTraceEvent("New Master key fetched.");
-                return Task.Run(() => task).GetAwaiter().GetResult();
-            }
-
             // Not a public exception - not likely to occur.
-            AKVEventSource.Log.TryTraceEvent("Master key not found.");
+            AKVEventSource.Log.TryTraceEvent("Key not found; URI={0}", keyIdentifierUri);
             throw ADP.MasterKeyNotFound(keyIdentifierUri);
         }
 
@@ -95,10 +111,7 @@ internal KeyVaultKey GetKey(string keyIdentifierUri)
         /// </summary>
         /// <param name="keyIdentifierUri">The key vault key identifier URI</param>
         /// <returns></returns>
-        internal int GetKeySize(string keyIdentifierUri)
-        {
-            return GetKey(keyIdentifierUri).Key.N.Length;
-        }
+        internal int GetKeySize(string keyIdentifierUri) =>  GetKey(keyIdentifierUri).Key.N.Length;
 
         /// <summary>
         /// Generates signature based on RSA PKCS#v1.5 scheme using a specified Azure Key Vault Key URL. 
@@ -142,49 +155,58 @@ private CryptographyClient GetCryptographyClient(string keyIdentifierUri)
 
             CryptographyClient cryptographyClient = new(GetKey(keyIdentifierUri).Id, TokenCredential);
             _cryptoClientDictionary.TryAdd(keyIdentifierUri, cryptographyClient);
-
             return cryptographyClient;
         }
 
         /// <summary>
-        /// 
+        /// Fetches the column encryption key from the Azure Key Vault.
         /// </summary>
-        /// <param name="vaultUri">The Azure Key Vault URI</param>
+        /// <param name="keyClient">The KeyClient instance</param>
         /// <param name="keyName">The name of the Azure Key Vault key</param>
         /// <param name="keyVersion">The version of the Azure Key Vault key</param>
-        /// <param name="keyResourceUri">The Azure Key Vault key identifier</param>
-        private void FetchKey(Uri vaultUri, string keyName, string keyVersion, string keyResourceUri)
+        private KeyVaultKey FetchKeyFromKeyVault(KeyClient keyClient, string keyName, string keyVersion)
         {
-            Task<Azure.Response<KeyVaultKey>> fetchKeyTask = FetchKeyFromKeyVault(vaultUri, keyName, keyVersion);
-            _keyFetchTaskDictionary.AddOrUpdate(keyResourceUri, fetchKeyTask, (k, v) => fetchKeyTask);
+            AKVEventSource.Log.TryTraceEvent("Fetching key name={0}", keyName);
 
-            fetchKeyTask
-                .ContinueWith(k => ValidateRsaKey(k.GetAwaiter().GetResult()))
-                .ContinueWith(k => _keyDictionary.AddOrUpdate(keyResourceUri, k.GetAwaiter().GetResult(), (key, v) => k.GetAwaiter().GetResult()));
+            Azure.Response<KeyVaultKey> keyResponse = keyClient?.GetKey(keyName, keyVersion);
 
-            Task.Run(() => fetchKeyTask);
+            // Handle the case where the key response is null or contains an error
+            // This can happen if the key does not exist or if there is an issue with the KeyClient.
+            // In such cases, we log the error and throw an exception.
+            if (keyResponse == null || keyResponse.Value == null || keyResponse.GetRawResponse().IsError)
+            {
+                AKVEventSource.Log.TryTraceEvent("Get Key failed to fetch Key from Azure Key Vault for key {0}, version {1}", keyName, keyVersion);
+                if (keyResponse?.GetRawResponse() is Azure.Response response)
+                {
+                    AKVEventSource.Log.TryTraceEvent("Response status {0} : {1}", response.Status, response.ReasonPhrase);
+                }
+                throw ADP.GetKeyFailed(keyName);
+            }
+
+            KeyVaultKey key = keyResponse.Value;
+
+            // Validate that the key is of type RSA
+            key = ValidateRsaKey(key);
+            return key;
         }
 
         /// <summary>
-        /// Looks up the KeyClient object by it's URI and then fetches the key by name.
+        /// Gets or creates a KeyClient for the specified Azure Key Vault URI.
         /// </summary>
-        /// <param name="vaultUri">The Azure Key Vault URI</param>
-        /// <param name="keyName">Then name of the key</param>
-        /// <param name="keyVersion">Then version of the key</param>
+        /// <param name="vaultUri">Key Identifier URL</param>
         /// <returns></returns>
-        private Task<Azure.Response<KeyVaultKey>> FetchKeyFromKeyVault(Uri vaultUri, string keyName, string keyVersion)
+        private KeyClient GetOrCreateKeyClient(Uri vaultUri)
         {
-            _keyClientDictionary.TryGetValue(vaultUri, out KeyClient keyClient);
-            AKVEventSource.Log.TryTraceEvent("Fetching requested master key: {0}", keyName);
-            return keyClient?.GetKeyAsync(keyName, keyVersion);
+            return _keyClientDictionary.GetOrAdd(
+                vaultUri, (_) => new KeyClient(vaultUri, TokenCredential));
         }
 
         /// <summary>
         /// Validates that a key is of type RSA
         /// </summary>
         /// <param name="key"></param>
         /// <returns></returns>
-        private KeyVaultKey ValidateRsaKey(KeyVaultKey key)
+        private static KeyVaultKey ValidateRsaKey(KeyVaultKey key)
         {
             if (key.KeyType != KeyType.Rsa && key.KeyType != KeyType.RsaHsm)
             {
@@ -195,26 +217,14 @@ private KeyVaultKey ValidateRsaKey(KeyVaultKey key)
             return key;
         }
 
-        /// <summary>
-        /// Instantiates and adds a KeyClient to the KeyClient dictionary
-        /// </summary>
-        /// <param name="vaultUri">The Azure Key Vault URI</param>
-        private void CreateKeyClient(Uri vaultUri)
-        {
-            if (!_keyClientDictionary.ContainsKey(vaultUri))
-            {
-                _keyClientDictionary.TryAdd(vaultUri, new KeyClient(vaultUri, TokenCredential));
-            }
-        }
-
         /// <summary>
         /// Validates and parses the Azure Key Vault URI and key name.
         /// </summary>
         /// <param name="masterKeyPath">The Azure Key Vault key identifier</param>
         /// <param name="vaultUri">The Azure Key Vault URI</param>
         /// <param name="masterKeyName">The name of the key</param>
         /// <param name="masterKeyVersion">The version of the key</param>
-        private void ParseAKVPath(string masterKeyPath, out Uri vaultUri, out string masterKeyName, out string masterKeyVersion)
+        private static void ParseAKVPath(string masterKeyPath, out Uri vaultUri, out string masterKeyName, out string masterKeyVersion)
         {
             Uri masterKeyPathUri = new(masterKeyPath);
             vaultUri = new Uri(masterKeyPathUri.GetLeftPart(UriPartial.Authority));

src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Constants.cs

@@ -9,16 +9,24 @@ internal static class Constants
         /// <summary>
         /// Azure Key Vault Domain Name
         /// </summary>
-        internal static readonly string[] AzureKeyVaultPublicDomainNames = new string[] {
-            @"vault.azure.net", // default
-            @"vault.azure.cn", // Azure China
-            @"vault.usgovcloudapi.net", // US Government
-            @"vault.microsoftazure.de", // Azure Germany
-            @"managedhsm.azure.net", // public HSM vault
-            @"managedhsm.azure.cn", // Azure China HSM vault
-            @"managedhsm.usgovcloudapi.net", // US Government HSM vault
-            @"managedhsm.microsoftazure.de" // Azure Germany HSM vault
-        };
+        internal static readonly string[] AzureKeyVaultPublicDomainNames =
+        [
+            // Azure Key Vaults
+            "vault.azure.net",                 // Default
+            "vault.azure.cn",                  // China
+            "vault.usgovcloudapi.net",         // US Government
+            "vault.microsoftazure.de",         // Azure Germany
+            "vault.sovcloud-api.fr",           // France (Bleu)
+            "vault.sovcloud-api.de",           // Germany (Delos)
+
+            // Managed High Security Modules (HSM) Vaults
+            "managedhsm.azure.net",
+            "managedhsm.azure.cn",
+            "managedhsm.usgovcloudapi.net",
+            "managedhsm.microsoftazure.de",
+            "managedhsm.sovcloud-api.fr",
+            "managedhsm.sovcloud-api.de"
+        ];
 
         /// <summary>
         /// Always Encrypted Parameter names for exec handling

src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/LocalCache.cs

@@ -2,8 +2,8 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using Microsoft.Extensions.Caching.Memory;
 using System;
+using Microsoft.Extensions.Caching.Memory;
 using static System.Math;
 
 namespace Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider
@@ -92,6 +92,7 @@ internal TValue GetOrCreate(TKey key, Func<TValue> createItem)
 
         /// <summary>
         /// Determines whether the <see cref="LocalCache{TKey, TValue}">LocalCache</see> contains the specified key.
+        /// Used in unit tests to verify that the cache contains the expected entries.
         /// </summary>
         /// <param name="key"></param>
         /// <returns></returns>