From 5253a34d5bb6e848590a4f078799f0487dc9e5f7 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 4 Jun 2021 12:40:30 -0700 Subject: [PATCH 1/8] give system providers precedence --- .../Microsoft/Data/SqlClient/SqlConnection.cs | 13 +++++----- .../Data/SqlClient/SqlSecurityUtility.cs | 19 ++++++++++++-- .../ManualTests/AlwaysEncrypted/ApiShould.cs | 25 +++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs index 5ce75ba598..71adafb9dd 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -225,6 +225,12 @@ private SqlConnection(SqlConnection connection) CacheConnectionStringProperties(); } + + internal static bool TryGetSystemColumnEncryptionKeyStoreProvider(string keyStoreName, out SqlColumnEncryptionKeyStoreProvider provider) + { + return s_systemColumnEncryptionKeyStoreProviders.TryGetValue(keyStoreName, out provider); + } + /// /// This function walks through both system and custom column encryption key store providers and returns an object if found. /// @@ -240,17 +246,12 @@ internal bool TryGetColumnEncryptionKeyStoreProvider(string providerName, out Sq return _customColumnEncryptionKeyStoreProviders.TryGetValue(providerName, out columnKeyStoreProvider); } - // Search in the sytem provider list. - if (s_systemColumnEncryptionKeyStoreProviders.TryGetValue(providerName, out columnKeyStoreProvider)) - { - return true; - } - lock (s_globalCustomColumnEncryptionKeyProvidersLock) { // If custom provider is not set, then return false if (s_globalCustomColumnEncryptionKeyStoreProviders is null) { + columnKeyStoreProvider = null; return false; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs index 2077590e77..67535743af 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs @@ -9,6 +9,7 @@ using System.Reflection; using System.Security.Cryptography; using System.Text; +using Microsoft.Data.Common; namespace Microsoft.Data.SqlClient { @@ -274,7 +275,7 @@ internal static void DecryptSymmetricKey(SqlTceCipherInfoEntry sqlTceCipherInfoE { try { - sqlClientSymmetricKey = InstanceLevelProvidersAreRegistered(connection, command) ? + sqlClientSymmetricKey = ShouldUseInstanceLevelProviderFlow(keyInfo.keyStoreName, connection, command) ? GetKeyFromLocalProviders(keyInfo, connection, command) : globalCekCache.GetKey(keyInfo, connection, command); encryptionkeyInfoChosen = keyInfo; @@ -367,7 +368,7 @@ internal static void VerifyColumnMasterKeySignature(string keyStoreName, string GetListOfProviderNamesThatWereSearched(connection, command)); } - if (InstanceLevelProvidersAreRegistered(connection, command)) + if (ShouldUseInstanceLevelProviderFlow(keyStoreName,connection, command)) { isValidSignature = provider.VerifyColumnMasterKeyMetadata(keyPath, isEnclaveEnabled, CMKSignature); } @@ -399,6 +400,15 @@ internal static void VerifyColumnMasterKeySignature(string keyStoreName, string } } + // Instance-level providers will be used if at least one is registered on a connection or command and + // the required provider is not a system provider. System providers are pre-registered globally and + // must use the global provider flow + private static bool ShouldUseInstanceLevelProviderFlow(string keyStoreName, SqlConnection connection, SqlCommand command) + { + return InstanceLevelProvidersAreRegistered(connection, command) && + !keyStoreName.StartsWith(ADP.ColumnEncryptionSystemProviderNamePrefix); + } + private static bool InstanceLevelProvidersAreRegistered(SqlConnection connection, SqlCommand command) => connection.HasColumnEncryptionKeyStoreProvidersRegistered || (command is not null && command.HasColumnEncryptionKeyStoreProvidersRegistered); @@ -423,6 +433,11 @@ internal static bool TryGetColumnEncryptionKeyStoreProvider(string keyStoreName, { Debug.Assert(!string.IsNullOrWhiteSpace(keyStoreName), "Provider name is invalid"); + if (SqlConnection.TryGetSystemColumnEncryptionKeyStoreProvider(keyStoreName, out provider)) + { + return true; + } + // command may be null because some callers do not have a command object, eg SqlBulkCopy if (command is not null && command.HasColumnEncryptionKeyStoreProvidersRegistered) { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index 8f18312f94..6eb0c83233 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -2242,6 +2242,31 @@ public void TestCommandCustomKeyStoreProviderDuringAeQuery(string connectionStri } } + // On Windows, "_fixture" will be type SQLSetupStrategyCertStoreProvider + // On non-Windows, "_fixture" will be type SQLSetupStrategyAzureKeyVault + // Test will pass on both but only SQLSetupStrategyCertStoreProvider is a system provider + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE))] + [ClassData(typeof(AEConnectionStringProvider))] + public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string connectionString) + { + if (!SQLSetupStrategyAzureKeyVault.IsAKVProviderRegistered) + { + SqlColumnEncryptionAzureKeyVaultProvider sqlColumnEncryptionAzureKeyVaultProvider = + new(new SqlClientCustomTokenCredential()); + SQLSetupStrategyAzureKeyVault.RegisterGlobalProviders(sqlColumnEncryptionAzureKeyVaultProvider); + } + + using SqlConnection connection = new(connectionString); + connection.Open(); + using SqlCommand command = new( + $"SELECT * FROM [{_fixture.CustomKeyStoreProviderTestTable.Name}] WHERE FirstName = @firstName", + connection, null, SqlCommandColumnEncryptionSetting.Enabled); + command.Parameters.AddWithValue("firstName", "abc"); + connection.RegisterColumnEncryptionKeyStoreProvidersOnConnection(_requiredProvider); + command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(_notRequiredProvider); + command.ExecuteReader(); + } + private void ExecuteQueryThatRequiresCustomKeyStoreProvider(SqlConnection connection) { using (SqlCommand command = CreateCommandThatRequiresCustomKeyStoreProvider(connection)) From d41faebed2ceeef894ddeeae44392a031ded1432 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 4 Jun 2021 12:45:07 -0700 Subject: [PATCH 2/8] Update SqlConnection.xml --- doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml index db0f670ea4..6e2cf6a699 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml @@ -1026,7 +1026,7 @@ GO Registers the column encryption key store providers. This function should only be called once in an app. This does shallow copying of the dictionary so that the app cannot alter the custom provider list once it has been set. - The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. No providers should be registered on the connection or command instances if one of the built-in column master key store providers is needed. + The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. Date: Fri, 4 Jun 2021 12:48:33 -0700 Subject: [PATCH 3/8] netfx changes --- .../src/Microsoft/Data/SqlClient/SqlConnection.cs | 1 - .../src/Microsoft/Data/SqlClient/SqlConnection.cs | 11 +++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs index 71adafb9dd..c9e6aabf49 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -225,7 +225,6 @@ private SqlConnection(SqlConnection connection) CacheConnectionStringProperties(); } - internal static bool TryGetSystemColumnEncryptionKeyStoreProvider(string keyStoreName, out SqlColumnEncryptionKeyStoreProvider provider) { return s_systemColumnEncryptionKeyStoreProviders.TryGetValue(keyStoreName, out provider); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs index ef0d91934e..0a1a6d4d39 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -213,6 +213,11 @@ private static void ValidateCustomProviders(IDictionary /// This function walks through both system and custom column encryption key store providers and returns an object if found. /// @@ -228,12 +233,6 @@ internal bool TryGetColumnEncryptionKeyStoreProvider(string providerName, out Sq return _customColumnEncryptionKeyStoreProviders.TryGetValue(providerName, out columnKeyStoreProvider); } - // Search in the system provider list. - if (s_systemColumnEncryptionKeyStoreProviders.TryGetValue(providerName, out columnKeyStoreProvider)) - { - return true; - } - lock (s_globalCustomColumnEncryptionKeyProvidersLock) { // If custom provider is not set, then return false From 9760930535cf76902b92cf735f5cf34d17bcc6b3 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 4 Jun 2021 12:57:43 -0700 Subject: [PATCH 4/8] update test for akv provider --- .../ManualTests/AlwaysEncrypted/ApiShould.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index f373a199be..dee0221b8b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -2249,12 +2249,13 @@ public void TestCommandCustomKeyStoreProviderDuringAeQuery(string connectionStri [ClassData(typeof(AEConnectionStringProvider))] public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string connectionString) { - if (!SQLSetupStrategyAzureKeyVault.IsAKVProviderRegistered) + Dictionary customKeyStoreProviders = new() { - SqlColumnEncryptionAzureKeyVaultProvider sqlColumnEncryptionAzureKeyVaultProvider = - new(new SqlClientCustomTokenCredential()); - SQLSetupStrategyAzureKeyVault.RegisterGlobalProviders(sqlColumnEncryptionAzureKeyVaultProvider); - } + { + SqlColumnEncryptionAzureKeyVaultProvider.ProviderName, + new SqlColumnEncryptionAzureKeyVaultProvider(new SqlClientCustomTokenCredential()) + } + }; using SqlConnection connection = new(connectionString); connection.Open(); @@ -2262,11 +2263,10 @@ public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string c $"SELECT * FROM [{_fixture.CustomKeyStoreProviderTestTable.Name}] WHERE FirstName = @firstName", connection, null, SqlCommandColumnEncryptionSetting.Enabled); command.Parameters.AddWithValue("firstName", "abc"); - connection.RegisterColumnEncryptionKeyStoreProvidersOnConnection(_requiredProvider); - command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(_notRequiredProvider); + command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(customKeyStoreProviders); command.ExecuteReader(); } - + [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE), nameof(DataTestUtility.EnclaveEnabled))] [ClassData(typeof(AEConnectionStringProvider))] public void TestRetryWhenAEParameterMetadataCacheIsStale(string connectionString) From 74704c07be19ee3e2308ad4a7139a21b8dad726c Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 4 Jun 2021 13:01:19 -0700 Subject: [PATCH 5/8] Update SqlConnection.cs --- .../netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs index 0a1a6d4d39..6e8a581ba3 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -238,6 +238,7 @@ internal bool TryGetColumnEncryptionKeyStoreProvider(string providerName, out Sq // If custom provider is not set, then return false if (s_globalCustomColumnEncryptionKeyStoreProviders is null) { + columnKeyStoreProvider = null; return false; } From 6145ff22d70f6a0ef15dd824e68a42126c032e33 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Fri, 4 Jun 2021 14:02:42 -0700 Subject: [PATCH 6/8] update test to cover connection-level provider --- .../ManualTests/AlwaysEncrypted/ApiShould.cs | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs index dee0221b8b..b92a642ae8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/ApiShould.cs @@ -2257,14 +2257,21 @@ public void TestSystemProvidersHavePrecedenceOverInstanceLevelProviders(string c } }; - using SqlConnection connection = new(connectionString); - connection.Open(); - using SqlCommand command = new( - $"SELECT * FROM [{_fixture.CustomKeyStoreProviderTestTable.Name}] WHERE FirstName = @firstName", - connection, null, SqlCommandColumnEncryptionSetting.Enabled); - command.Parameters.AddWithValue("firstName", "abc"); - command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(customKeyStoreProviders); - command.ExecuteReader(); + using (SqlConnection connection = new(connectionString)) + { + connection.Open(); + using SqlCommand command = CreateCommandThatRequiresSystemKeyStoreProvider(connection); + connection.RegisterColumnEncryptionKeyStoreProvidersOnConnection(customKeyStoreProviders); + command.ExecuteReader(); + } + + using (SqlConnection connection = new(connectionString)) + { + connection.Open(); + using SqlCommand command = CreateCommandThatRequiresSystemKeyStoreProvider(connection); + command.RegisterColumnEncryptionKeyStoreProvidersOnCommand(customKeyStoreProviders); + command.ExecuteReader(); + } } [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringSetupForAE), nameof(DataTestUtility.EnclaveEnabled))] @@ -2343,6 +2350,15 @@ private SqlCommand CreateCommandThatRequiresCustomKeyStoreProvider(SqlConnection return command; } + private SqlCommand CreateCommandThatRequiresSystemKeyStoreProvider(SqlConnection connection) + { + SqlCommand command = new( + $"SELECT * FROM [{_fixture.CustomKeyStoreProviderTestTable.Name}] WHERE FirstName = @firstName", + connection, null, SqlCommandColumnEncryptionSetting.Enabled); + command.Parameters.AddWithValue("firstName", "abc"); + return command; + } + private void AssertExceptionCausedByFailureToDecrypt(Exception ex) { Assert.Contains(_failedToDecryptMessage, ex.Message); From b977fb5d8c448ec723502e4b747d3ccfcefd9ab5 Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Mon, 7 Jun 2021 16:02:07 -0700 Subject: [PATCH 7/8] update comment --- .../netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs | 4 ++-- .../netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs index c9e6aabf49..ed3d7a0ca8 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -231,9 +231,9 @@ internal static bool TryGetSystemColumnEncryptionKeyStoreProvider(string keyStor } /// - /// This function walks through both system and custom column encryption key store providers and returns an object if found. + /// This function walks through both instance-level and global custom column encryption key store providers and returns an object if found. /// - /// Provider Name to be searched in System Provider dictionary and Custom provider dictionary. + /// Provider Name to be searched for. /// If the provider is found, initializes the corresponding SqlColumnEncryptionKeyStoreProvider instance. /// true if the provider is found, else returns false internal bool TryGetColumnEncryptionKeyStoreProvider(string providerName, out SqlColumnEncryptionKeyStoreProvider columnKeyStoreProvider) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs index 6e8a581ba3..11bda368ee 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -219,10 +219,10 @@ internal static bool TryGetSystemColumnEncryptionKeyStoreProvider(string keyStor } /// - /// This function walks through both system and custom column encryption key store providers and returns an object if found. + /// This function walks through both instance-level and global custom column encryption key store providers and returns an object if found. /// - /// Provider Name to be searched in System Provider diction and Custom provider dictionary. - /// If the provider is found, returns the corresponding SqlColumnEncryptionKeyStoreProvider instance. + /// Provider Name to be searched for. + /// If the provider is found, initializes the corresponding SqlColumnEncryptionKeyStoreProvider instance. /// true if the provider is found, else returns false internal bool TryGetColumnEncryptionKeyStoreProvider(string providerName, out SqlColumnEncryptionKeyStoreProvider columnKeyStoreProvider) { From 86d7d7da48600ecaa9b099e84e18328c29c1c6ea Mon Sep 17 00:00:00 2001 From: Johnny Pham Date: Mon, 7 Jun 2021 17:24:03 -0700 Subject: [PATCH 8/8] update docs --- doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml | 4 +++- doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml index 4cd8b3529b..68b10ce395 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml @@ -2798,7 +2798,9 @@ Custom master key store providers can be registered with the driver at three dif Once any key store provider is found at a registration level, the driver will **NOT** fall back to the other registrations to search for a provider. If providers are registered but the proper provider is not found at a level, an exception will be thrown containing only the registered providers in the registration that was checked. -The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. No providers should be registered on the connection or command instances if one of the built-in column master key store providers is needed. +The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. + +This does shallow copying of the dictionary so that the app cannot alter the custom provider list once it has been set. ]]> diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml index 6e2cf6a699..5942a41959 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml @@ -1085,7 +1085,9 @@ Custom master key store providers can be registered with the driver at three dif Once any key store provider is found at a registration level, the driver will **NOT** fall back to the other registrations to search for a provider. If providers are registered but the proper provider is not found at a level, an exception will be thrown containing only the registered providers in the registration that was checked. -The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. No providers should be registered on the connection or command instances if one of the built-in column master key store providers is needed. +The built-in column master key store providers that are available for the Windows Certificate Store, CNG Store and CSP are pre-registered. + +This does shallow copying of the dictionary so that the app cannot alter the custom provider list once it has been set. ]]>