Fix | Don't validate server certificate when using Authentication opt…

…ion but not encrypting (#2224)
dotnet · Nov 30, 2023 · 4203237 · 4203237
1 parent 1543998
commit 4203237
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 9 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
@@ -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("<sc.TdsParser.ConsumePreLoginHandshake|ERR> {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]);
                         }
 

diff --git 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
@@ -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();
+        }
     }
 }