Fix | Enhance certificate validation

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs

@@ -24,6 +24,8 @@ internal sealed class SNINpHandle : SNIPhysicalHandle
 
         private readonly string _targetServer;
         private readonly object _sendSync;
+        private readonly string _hostNameInCertificate;
+        private readonly string _serverCertificateFilename;
         private readonly bool _tlsFirst;
         private Stream _stream;
         private NamedPipeClientStream _pipeStream;
@@ -38,7 +40,7 @@ internal sealed class SNINpHandle : SNIPhysicalHandle
         private int _bufferSize = TdsEnums.DEFAULT_LOGIN_PACKET_SIZE;
         private readonly Guid _connectionId = Guid.NewGuid();
 
-        public SNINpHandle(string serverName, string pipeName, TimeoutTimer timeout, bool tlsFirst)
+        public SNINpHandle(string serverName, string pipeName, TimeoutTimer timeout, bool tlsFirst, string hostNameInCertificate, string serverCertificateFilename)
         {
             using (TrySNIEventScope.Create(nameof(SNINpHandle)))
             {
@@ -47,6 +49,8 @@ public SNINpHandle(string serverName, string pipeName, TimeoutTimer timeout, boo
                 _sendSync = new object();
                 _targetServer = serverName;
                 _tlsFirst = tlsFirst;
+                _hostNameInCertificate = hostNameInCertificate;
+                _serverCertificateFilename = serverCertificateFilename;
                 try
                 {
                     _pipeStream = new NamedPipeClientStream(
@@ -369,23 +373,23 @@ public override void DisableSsl()
         /// Validate server certificate
         /// </summary>
         /// <param name="sender">Sender object</param>
-        /// <param name="cert">X.509 certificate</param>
+        /// <param name="serverCertificate">X.509 certificate</param>
         /// <param name="chain">X.509 chain</param>
         /// <param name="policyErrors">Policy errors</param>
         /// <returns>true if valid</returns>
-        private bool ValidateServerCertificate(object sender, X509Certificate cert, X509Chain chain, SslPolicyErrors policyErrors)
+        private bool ValidateServerCertificate(object sender, X509Certificate serverCertificate, X509Chain chain, SslPolicyErrors policyErrors)
         {
             using (TrySNIEventScope.Create(nameof(SNINpHandle)))
-            {
+            {			
                 if (!_validateCert)
                 {
                     SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNINpHandle), EventType.INFO, "Connection Id {0}, Certificate validation not requested.", args0: ConnectionId);
                     return true;
                 }
 
                 SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNINpHandle), EventType.INFO, "Connection Id {0}, Proceeding to SSL certificate validation.", args0: ConnectionId);
-                return SNICommon.ValidateSslServerCertificate(_targetServer, cert, policyErrors);
-            }
+                return SNICommon.ValidateSslServerCertificate(_connectionId, _targetServer, _hostNameInCertificate, serverCertificate, _serverCertificateFilename, policyErrors);
+			}	
         }
 
         /// <summary>

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs

@@ -206,7 +206,7 @@ internal static SNIHandle CreateConnectionHandle(
                         tlsFirst, hostNameInCertificate, serverCertificateFilename);
                     break;
                 case DataSource.Protocol.NP:
-                    sniHandle = CreateNpHandle(details, timeout, parallel, tlsFirst);
+                    sniHandle = CreateNpHandle(details, timeout, parallel, tlsFirst, hostNameInCertificate, serverCertificateFilename);
                     break;
                 default:
                     Debug.Fail($"Unexpected connection protocol: {details._connectionProtocol}");
@@ -362,16 +362,18 @@ private static SNITCPHandle CreateTcpHandle(
         /// <param name="timeout">Timer expiration</param>
         /// <param name="parallel">Should MultiSubnetFailover be used. Only returns an error for named pipes.</param>
         /// <param name="tlsFirst"></param>
+        /// <param name="hostNameInCertificate">Host name in certificate</param>
+        /// <param name="serverCertificateFilename">Used for the path to the Server Certificate</param>
         /// <returns>SNINpHandle</returns>
-        private static SNINpHandle CreateNpHandle(DataSource details, TimeoutTimer timeout, bool parallel, bool tlsFirst)
+        private static SNINpHandle CreateNpHandle(DataSource details, TimeoutTimer timeout, bool parallel, bool tlsFirst, string hostNameInCertificate, string serverCertificateFilename)
         {
             if (parallel)
             {
                 // Connecting to a SQL Server instance using the MultiSubnetFailover connection option is only supported when using the TCP protocol
                 SNICommon.ReportSNIError(SNIProviders.NP_PROV, 0, SNICommon.MultiSubnetFailoverWithNonTcpProtocol, Strings.SNI_ERROR_49);
                 return null;
             }
-            return new SNINpHandle(details.PipeHostName, details.PipeName, timeout, tlsFirst);
+            return new SNINpHandle(details.PipeHostName, details.PipeName, timeout, tlsFirst, hostNameInCertificate, serverCertificateFilename);
         }
 
         /// <summary>
@@ -539,8 +541,10 @@ private void PopulateProtocol()
         internal static string GetLocalDBInstance(string dataSource, out bool error)
         {
             string instanceName = null;
+            // ReadOnlySpan is not supported in netstandard 2.0, but installing System.Memory solves the issue
             ReadOnlySpan<char> input = dataSource.AsSpan().TrimStart();
             error = false;
+            // NetStandard 2.0 does not support passing a string to ReadOnlySpan<char>
             int index = input.IndexOf(LocalDbHost.AsSpan().Trim(), StringComparison.InvariantCultureIgnoreCase);
             if (input.StartsWith(LocalDbHost_NP.AsSpan().Trim(), StringComparison.InvariantCultureIgnoreCase))
             {

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs

@@ -644,7 +644,6 @@ public override uint EnableSsl(uint options)
                     }
                     else
                     {
-                        // TODO: Resolve whether to send _serverNameIndication or _targetServer. _serverNameIndication currently results in error. Why?
                         _sslStream.AuthenticateAsClient(_targetServer, null, s_supportedProtocols, false);
                     }
                     if (_sslOverTdsStream is not null)
@@ -698,33 +697,8 @@ private bool ValidateServerCertificate(object sender, X509Certificate serverCert
                 return true;
             }
 
-            string serverNameToValidate;
-            if (!string.IsNullOrEmpty(_hostNameInCertificate))
-            {
-                serverNameToValidate = _hostNameInCertificate;
-            }
-            else
-            {
-                serverNameToValidate = _targetServer;
-            }
-
-            if (!string.IsNullOrEmpty(_serverCertificateFilename))
-            {
-                X509Certificate clientCertificate = null;
-                try
-                {
-                    clientCertificate = new X509Certificate(_serverCertificateFilename);
-                    return SNICommon.ValidateSslServerCertificate(clientCertificate, serverCertificate, policyErrors);
-                }
-                catch (Exception e)
-                {
-                    // if this fails, then fall back to the HostNameInCertificate or TargetServer validation.
-                    SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNITCPHandle), EventType.INFO, "Connection Id {0}, IOException occurred: {1}", args0: _connectionId, args1: e.Message);
-                }
-            }
-
             SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNITCPHandle), EventType.INFO, "Connection Id {0}, Certificate will be validated for Target Server name", args0: _connectionId);
-            return SNICommon.ValidateSslServerCertificate(serverNameToValidate, serverCertificate, policyErrors);
+            return SNICommon.ValidateSslServerCertificate(_connectionId, _targetServer, _hostNameInCertificate, serverCertificate, _serverCertificateFilename, policyErrors);
         }
 
         /// <summary>

src/Microsoft.Data.SqlClient/src/Resources/Strings.resx

@@ -4737,4 +4737,7 @@
   <data name="SQL_RemoteCertificateNotAvailable" xml:space="preserve">
     <value>Certificate not available while validating the certificate.</value>
   </data>
+  <data name="SQL_RemoteCertificateDoesNotMatchServerCertificate" xml:space="preserve">
+    <value>The certificate provided by the server does not match the certificate provided by the ServerCertificate option.</value>
+  </data>
 </root>

src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionBasicTests.cs

@@ -12,6 +12,7 @@
 using System.Security;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.SqlServer.TDS.PreLogin;
 using Microsoft.SqlServer.TDS.Servers;
 using Xunit;
 

src/Microsoft.Data.SqlClient/tests/FunctionalTests/TestTdsServer.cs

@@ -65,6 +65,5 @@ public static TestTdsServer StartTestServer(bool enableFedAuth = false, bool ena
         public void Dispose() => _endpoint?.Stop();
 
         public string ConnectionString { get; private set; }
-
     }
 }

src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/ConnectionTestParameters.cs

@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.SqlServer.TDS.PreLogin;
+
+namespace Microsoft.Data.SqlClient.ManualTesting.Tests.DataCommon
+{
+    public class ConnectionTestParameters
+    {
+        public SqlConnectionEncryptOption Encrypt { get; set; }
+        public bool TrustServerCertificate { get; set; }
+        public string Certificate { get; set; }
+        public string HostNameInCertificate { get; set; }
+        public bool TestResult { get; set; }
+        public TDSPreLoginTokenEncryptionType TdsEncryptionType { get; set; }
+    }
+}