From a614b66f078ac535deb73aa5ded813b2a0ba89aa Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 16 Nov 2023 16:59:36 -0800 Subject: [PATCH 1/2] Fix | Don't validate server certificate when using Authentication option but not encrypting --- .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 7cce889b20..9da85106e2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -1485,12 +1485,10 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( } // Validate Certificate if Trust Server Certificate=false and Encryption forced (EncryptionOptions.ON) from Server. - bool shouldValidateServerCert = (_encryptionOption == EncryptionOptions.ON && !trustServerCert) || - ((authType != SqlAuthenticationMethod.NotSpecified || (_connHandler._accessTokenInBytes != null || - _connHandler._accessTokenCallback != null)) - && !trustServerCert); - - UInt32 info = (shouldValidateServerCert ? TdsEnums.SNI_SSL_VALIDATE_CERTIFICATE : 0) + bool shouldValidateServerCert = (_encryptionOption == EncryptionOptions.ON && !trustServerCert) || + ((_connHandler._accessTokenInBytes != null || _connHandler._accessTokenCallback != null) + && !trustServerCert); + uint info = (shouldValidateServerCert ? TdsEnums.SNI_SSL_VALIDATE_CERTIFICATE : 0) | (is2005OrLater && (_encryptionOption & EncryptionOptions.CLIENT_CERT) == 0 ? TdsEnums.SNI_SSL_USE_SCHANNEL_CACHE : 0); EnableSsl(info, encrypt, integratedSecurity, serverCertificateFilename, serverCallback, clientCallback); @@ -1542,8 +1540,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( if (payload[payloadOffset] != 0x00 && payload[payloadOffset] != 0x01) { SqlClientEventSource.Log.TryTraceEvent(" {0}, " + - "Server sent an unexpected value for FedAuthRequired PreLogin Option. Value was {1}.", ObjectID, (int)payload[payloadOffset]); - + "Server sent an unexpected value for FedAuthRequired PreLogin Option. Value was {1}.", ObjectID, (int)payload[payloadOffset]); throw SQL.ParsingErrorValue(ParsingErrorState.FedAuthRequiredPreLoginResponseInvalidValue, (int)payload[payloadOffset]); } From 33928a4c45c0352a7115f20c9f7ee9274b69fb5b Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 27 Nov 2023 12:27:54 -0800 Subject: [PATCH 2/2] Add new test --- .../ConnectivityTests/AADConnectionTest.cs | 2 +- .../SQL/ConnectivityTests/ConnectivityTest.cs | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs index da1b59132c..5d7cae1f1e 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs @@ -636,7 +636,7 @@ public static void ActiveDirectoryDefaultMustPass() } [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsIntegratedSecuritySetup), nameof(DataTestUtility.AreConnStringsSetup))] - public static void ADInteractiveUsingSSPI() + public static void ADIntegratedUsingSSPI() { // test Passes with correct connection string. string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection", "Integrated Security" }; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs index cd373757a4..60260fe79f 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs @@ -441,5 +441,38 @@ public static void DacConnectionTest() using SqlConnection sqlConnection = new(b.ConnectionString); sqlConnection.Open(); } + + private static bool UsernamePasswordNonEncryptedConnectionSetup() + { + if (!DataTestUtility.IsTCPConnStringSetup()) + { + return false; + } + + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); + return !string.IsNullOrEmpty(b.UserID) && + !string.IsNullOrEmpty(b.Password) && + b.Encrypt == SqlConnectionEncryptOption.Optional && + (b.Authentication == SqlAuthenticationMethod.NotSpecified || b.Authentication == SqlAuthenticationMethod.SqlPassword); + } + + [ConditionalFact(nameof(UsernamePasswordNonEncryptedConnectionSetup))] + public static void SqlPasswordConnectionTest() + { + if (!UsernamePasswordNonEncryptedConnectionSetup()) + { + throw new Exception("Sql credentials and non-Encrypted connection required."); + } + + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); + b.Authentication = SqlAuthenticationMethod.SqlPassword; + + // This ensures we are not validating the server certificate when we shouldn't be + // This test may fail if Encrypt = false but the test server requires encryption + b.TrustServerCertificate = false; + + using SqlConnection sqlConnection = new(b.ConnectionString); + sqlConnection.Open(); + } } }