Address comments and renamed serverCert to serverCertificateFilename

Author: lcheunglci

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

@@ -542,7 +542,7 @@ End Module
 |Failover Partner|N/A|The name of the failover partner server where database mirroring is configured.<br /><br /> If the value of this key is "", then **Initial Catalog** must be present, and its value must not be "".<br /><br /> The server name can be 128 characters or less.<br /><br /> If you specify a failover partner but the failover partner server is not configured for database mirroring and the primary server (specified with the Server keyword) is not available, then the connection will fail.<br /><br /> If you specify a failover partner and the primary server is not configured for database mirroring, the connection to the primary server (specified with the Server keyword) will succeed if the primary server is available.|  
 |Failover Partner SPN<br /><br /> -or-<br /><br /> FailoverPartnerSPN|N/A|The SPN for the failover partner. The default value is an empty string, which causes SqlClient to use the default, driver-generated SPN.<br /><br /> (Only available in v5.0+)|  
 |Host Name In Certificate<br /><br /> -or-<br /><br />HostNameInCertificate|N/A|The host name to use when validating the server certificate. When not specified, the server name from the Data Source is used for certificate validation.<br /><br /> (Only available in v5.0+)|
-|Server Certificate<br /><br /> -or-<br /><br />ServerCertificate|N/A|The path to a certificate file to match against the SQL Server TLS/SSL certificate. The accepted certificate formats are PEM, DER, and CER. If specified, the SQL Server certificate is checked by seeing if the ServerCertificate provided is an exact match.<br /><br /> (Only available in v5.0+)|
+|Server Certificate<br /><br /> -or-<br /><br />ServerCertificate|N/A|The path to a certificate file to match against the SQL Server TLS/SSL certificate. The accepted certificate formats are PEM, DER, and CER. If specified, the SQL Server certificate is checked by verifying if the ServerCertificate provided is an exact match.<br /><br /> (Only available in v5.0+)|
 |Initial Catalog<br /><br /> -or-<br /><br /> Database|N/A|The name of the database.<br /><br /> The database name can be 128 characters or less.|  
 |Integrated Security<br /><br /> -or-<br /><br /> Trusted_Connection|'false'|When `false`, User ID and Password are specified in the connection. When `true`, the current Windows account credentials are used for authentication.<br /><br /> Recognized values are `true`, `false`, `yes`, `no`, and `sspi` (strongly recommended), which is equivalent to `true`.<br /><br /> If User ID and Password are specified and Integrated Security is set to true, the User ID and Password will be ignored and Integrated Security will be used.<br /><br /> <xref:Microsoft.Data.SqlClient.SqlCredential> is a more secure way to specify credentials for a connection that uses SQL Server Authentication (`Integrated Security=false`).|  
 |IP Address Preference<br /><br /> -or-<br /><br /> IPAddressPreference|IPv4First|The IP address family preference when establishing TCP connections. If `Transparent Network IP Resolution` (in .NET Framework) or `Multi Subnet Failover` is set to true, this setting has no effect. Supported values include:<br /><br /> `IPAddressPreference=IPv4First`<br /><br />`IPAddressPreference=IPv6First`<br /><br />`IPAddressPreference=UsePlatformDefault`|  

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

@@ -200,8 +200,8 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C
         /// Certificate validation and chain trust validations are done by SSLStream class [System.Net.Security.SecureChannel.VerifyRemoteCertificate method]
         /// This method is called as a result of callback for SSL Stream Certificate validation.
         /// </summary>
-        /// <param name="clientCert">X.509 server certificate provided by the client</param>
-        /// <param name="serverCert">X.509 certificate provuded by the server</param>
+        /// <param name="clientCert">X.509 certificate provided by the client</param>
+        /// <param name="serverCert">X.509 certificate provided by the server</param>
         /// <param name="policyErrors">Policy errors</param>
         /// <returns>True if certificate is valid</returns>
         internal static bool ValidateSslServerCertificate(X509Certificate clientCert, X509Certificate serverCert, SslPolicyErrors policyErrors)

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

@@ -144,7 +144,7 @@ private static bool IsErrorStatus(SecurityStatusPalErrorCode errorCode)
         /// <param name="pendingDNSInfo">Used for DNS Cache</param>
         /// <param name="tlsFirst">Support TDS8.0</param>
         /// <param name="hostNameInCertificate">Used for the HostName in certificate</param>
-        /// <param name="serverCert">Used for the path to the Server Certificate</param>
+        /// <param name="serverCertificateFilename">Used for the path to the Server Certificate</param>
         /// <returns>SNI handle</returns>
         internal static SNIHandle CreateConnectionHandle(
             string fullServerName,
@@ -162,7 +162,7 @@ internal static SNIHandle CreateConnectionHandle(
             ref SQLDNSInfo pendingDNSInfo,
             bool tlsFirst,
             string hostNameInCertificate,
-            string serverCert)
+            string serverCertificateFilename)
         {
             instanceName = new byte[1];
 
@@ -189,7 +189,7 @@ internal static SNIHandle CreateConnectionHandle(
                 case DataSource.Protocol.None: // default to using tcp if no protocol is provided
                 case DataSource.Protocol.TCP:
                     sniHandle = CreateTcpHandle(details, timerExpire, parallel, ipPreference, cachedFQDN, ref pendingDNSInfo,
-                        tlsFirst, hostNameInCertificate, serverCert);
+                        tlsFirst, hostNameInCertificate, serverCertificateFilename);
                     break;
                 case DataSource.Protocol.NP:
                     sniHandle = CreateNpHandle(details, timerExpire, parallel, tlsFirst);
@@ -288,7 +288,7 @@ private static byte[][] GetSqlServerSPNs(string hostNameOrAddress, string portOr
         /// <param name="pendingDNSInfo">Used for DNS Cache</param>
         /// <param name="tlsFirst">Support TDS8.0</param>
         /// <param name="hostNameInCertificate">Host name in certificate</param>
-        /// <param name="serverCert">Used for the path to the Server Certificate</param>
+        /// <param name="serverCertificateFilename">Used for the path to the Server Certificate</param>
         /// <returns>SNITCPHandle</returns>
         private static SNITCPHandle CreateTcpHandle(
             DataSource details,
@@ -299,7 +299,7 @@ private static SNITCPHandle CreateTcpHandle(
             ref SQLDNSInfo pendingDNSInfo,
             bool tlsFirst,
             string hostNameInCertificate,
-            string serverCert)
+            string serverCertificateFilename)
         {
             // TCP Format:
             // tcp:<host name>\<instance name>
@@ -338,7 +338,7 @@ private static SNITCPHandle CreateTcpHandle(
             }
 
             return new SNITCPHandle(hostName, port, timerExpire, parallel, ipPreference, cachedFQDN, ref pendingDNSInfo,
-                tlsFirst, hostNameInCertificate, serverCert);
+                tlsFirst, hostNameInCertificate, serverCertificateFilename);
         }
 
         /// <summary>

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

@@ -27,7 +27,7 @@ internal sealed class SNITCPHandle : SNIPhysicalHandle
         private readonly Socket _socket;
         private NetworkStream _tcpStream;
         private readonly string _hostNameInCertificate;
-        private readonly string _serverCertificate;
+        private readonly string _serverCertificateFilename;
         private readonly bool _tlsFirst;
 
         private Stream _stream;
@@ -123,7 +123,7 @@ public override int ProtocolVersion
         /// <param name="pendingDNSInfo">Used for DNS Cache</param>
         /// <param name="tlsFirst">Support TDS8.0</param>
         /// <param name="hostNameInCertificate">Host Name in Certificate</param>
-        /// <param name="serverCert">Used for the path to the Server Certificate</param>
+        /// <param name="serverCertifiateFilename">Used for the path to the Server Certificate</param>
         public SNITCPHandle(
             string serverName,
             int port,
@@ -134,7 +134,7 @@ public SNITCPHandle(
             ref SQLDNSInfo pendingDNSInfo,
             bool tlsFirst,
             string hostNameInCertificate,
-            string serverCert)
+            string serverCertifiateFilename)
         {
             using (TrySNIEventScope.Create(nameof(SNITCPHandle)))
             {
@@ -143,7 +143,7 @@ public SNITCPHandle(
                 _targetServer = serverName;
                 _tlsFirst = tlsFirst;
                 _hostNameInCertificate = hostNameInCertificate;
-                _serverCertificate = serverCert;
+                _serverCertificateFilename = serverCertifiateFilename;
                 _sendSync = new object();
 
                 SQLDNSInfo cachedDNSInfo;
@@ -653,11 +653,11 @@ public override void DisableSsl()
         /// Validate server certificate callback
         /// </summary>
         /// <param name="sender">Sender object</param>
-        /// <param name="cert">X.509 certificate</param>
+        /// <param name="serverCertificate">X.509 certificate provided from the server</param>
         /// <param name="chain">X.509 chain</param>
         /// <param name="policyErrors">Policy errors</param>
         /// <returns>True if certificate is valid</returns>
-        private bool ValidateServerCertificate(object sender, X509Certificate cert, X509Chain chain, SslPolicyErrors policyErrors)
+        private bool ValidateServerCertificate(object sender, X509Certificate serverCertificate, X509Chain chain, SslPolicyErrors policyErrors)
         {
             if (!_validateCert)
             {
@@ -675,13 +675,13 @@ private bool ValidateServerCertificate(object sender, X509Certificate cert, X509
                 serverNameToValidate = _targetServer;
             }
 
-            if (!string.IsNullOrEmpty(_serverCertificate))
+            if (!string.IsNullOrEmpty(_serverCertificateFilename))
             {
-                X509Certificate serverCert = null;
+                X509Certificate clientCertificate = null;
                 try
                 {
-                    serverCert = new X509Certificate(_serverCertificate);
-                    return SNICommon.ValidateSslServerCertificate(serverCert, cert, policyErrors);
+                    clientCertificate = new X509Certificate(_serverCertificateFilename);
+                    return SNICommon.ValidateSslServerCertificate(clientCertificate, serverCertificate, policyErrors);
                 }
                 catch (Exception e)
                 {
@@ -691,7 +691,7 @@ private bool ValidateServerCertificate(object sender, X509Certificate cert, X509
             }
 
             SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNITCPHandle), EventType.INFO, "Connection Id {0}, Certificate will be validated for Target Server name", args0: _connectionId);
-            return SNICommon.ValidateSslServerCertificate(serverNameToValidate, cert, policyErrors);
+            return SNICommon.ValidateSslServerCertificate(serverNameToValidate, serverCertificate, policyErrors);
         }
 
         /// <summary>

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

@@ -365,11 +365,12 @@ internal void Connect(
             bool withFailover)
         {
             SqlConnectionEncryptOption encrypt = connectionOptions.Encrypt;
+            bool isTlsFirst = (encrypt == SqlConnectionEncryptOption.Strict);
             bool trustServerCert = connectionOptions.TrustServerCertificate;
             bool integratedSecurity = connectionOptions.IntegratedSecurity;
             SqlAuthenticationMethod authType = connectionOptions.Authentication;
             string hostNameInCertificate = connectionOptions.HostNameInCertificate;
-            string serverCert = connectionOptions.ServerCertificate;
+            string serverCertificateFilename = connectionOptions.ServerCertificate;
 
             if (_state != TdsParserState.Closed)
             {
@@ -415,8 +416,8 @@ internal void Connect(
                 SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | SSPI or Active Directory Authentication Library loaded for SQL Server based integrated authentication");
             }
 
-            // if Strict encryption is chosen trust server certificate should always be false.
-            if (encrypt == SqlConnectionEncryptOption.Strict)
+            // if Strict encryption (i.e. isTlsFirst) is chosen trust server certificate should always be false.
+            if (isTlsFirst)
             {
                 trustServerCert = false;
             }
@@ -440,10 +441,23 @@ internal void Connect(
             _connHandler.pendingSQLDNSObject = null;
 
             // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
-            _physicalStateObj.CreatePhysicalSNIHandle(serverInfo.ExtendedServerName, ignoreSniOpenTimeout, timerExpire, out instanceName, ref _sniSpnBuffer,
-                false, true, fParallel, _connHandler.ConnectionOptions.IPAddressPreference, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject, serverInfo.ServerSPN,
-                integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated, encrypt == SqlConnectionEncryptOption.Strict,
-                hostNameInCertificate, serverCert);
+            _physicalStateObj.CreatePhysicalSNIHandle(
+                serverInfo.ExtendedServerName,
+                ignoreSniOpenTimeout,
+                timerExpire,
+                out instanceName,
+                ref _sniSpnBuffer,
+                false,
+                true,
+                fParallel,
+                _connHandler.ConnectionOptions.IPAddressPreference,
+                FQDNforDNSCache,
+                ref _connHandler.pendingSQLDNSObject,
+                serverInfo.ServerSPN,
+                integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated,
+                isTlsFirst,
+                hostNameInCertificate,
+                serverCertificateFilename);
 
             if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
             {
@@ -502,15 +516,21 @@ internal void Connect(
             }
 
             SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Sending prelogin handshake");
-            SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCert);
+            SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCertificateFilename);
 
             _connHandler.TimeoutErrorInternal.EndPhase(SqlConnectionTimeoutErrorPhase.SendPreLoginHandshake);
             _connHandler.TimeoutErrorInternal.SetAndBeginPhase(SqlConnectionTimeoutErrorPhase.ConsumePreLoginHandshake);
 
             _physicalStateObj.SniContext = SniContext.Snix_PreLogin;
             SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Consuming prelogin handshake");
-            PreLoginHandshakeStatus status = ConsumePreLoginHandshake(encrypt, trustServerCert, integratedSecurity, out marsCapable,
-                out _connHandler._fedAuthRequired, encrypt == SqlConnectionEncryptOption.Strict, serverCert);
+            PreLoginHandshakeStatus status = ConsumePreLoginHandshake(
+                encrypt,
+                trustServerCert,
+                integratedSecurity,
+                out marsCapable,
+                out _connHandler._fedAuthRequired,
+                isTlsFirst,
+                serverCertificateFilename);
 
             if (status == PreLoginHandshakeStatus.InstanceFailure)
             {
@@ -520,8 +540,22 @@ internal void Connect(
                 // On Instance failure re-connect and flush SNI named instance cache.
                 _physicalStateObj.SniContext = SniContext.Snix_Connect;
 
-                _physicalStateObj.CreatePhysicalSNIHandle(serverInfo.ExtendedServerName, ignoreSniOpenTimeout, timerExpire, out instanceName, ref _sniSpnBuffer, true, true, fParallel,
-                                                _connHandler.ConnectionOptions.IPAddressPreference, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject, serverInfo.ServerSPN, integratedSecurity);
+                _physicalStateObj.CreatePhysicalSNIHandle(
+                    serverInfo.ExtendedServerName,
+                    ignoreSniOpenTimeout,
+                    timerExpire, out instanceName,
+                    ref _sniSpnBuffer,
+                    true,
+                    true,
+                    fParallel,
+                    _connHandler.ConnectionOptions.IPAddressPreference,
+                    FQDNforDNSCache,
+                    ref _connHandler.pendingSQLDNSObject,
+                    serverInfo.ServerSPN,
+                    integratedSecurity,
+                    isTlsFirst,
+                    hostNameInCertificate,
+                    serverCertificateFilename);
 
                 if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
                 {
@@ -541,9 +575,15 @@ internal void Connect(
                     _physicalStateObj.AssignPendingDNSInfo(serverInfo.UserProtocol, FQDNforDNSCache, ref _connHandler.pendingSQLDNSObject);
                 }
 
-                SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCert);
-                status = ConsumePreLoginHandshake(encrypt, trustServerCert, integratedSecurity, out marsCapable, out _connHandler._fedAuthRequired,
-                    encrypt == SqlConnectionEncryptOption.Strict, serverCert);
+                SendPreLoginHandshake(instanceName, encrypt, integratedSecurity, serverCertificateFilename);
+                status = ConsumePreLoginHandshake(
+                    encrypt,
+                    trustServerCert,
+                    integratedSecurity,
+                    out marsCapable,
+                    out _connHandler._fedAuthRequired,
+                    isTlsFirst,
+                    serverCertificateFilename);
 
                 // Don't need to check for 7.0 failure, since we've already consumed
                 // one pre-login packet and know we are connecting to 2000.
@@ -663,14 +703,14 @@ private void SendPreLoginHandshake(
             byte[] instanceName,
             SqlConnectionEncryptOption encrypt,
             bool integratedSecurity,
-            string serverCert)
+            string serverCertificateFilename)
         {
             if (encrypt == SqlConnectionEncryptOption.Strict)
             {
                 //Always validate the certificate when in strict encryption mode
                 uint info = TdsEnums.SNI_SSL_VALIDATE_CERTIFICATE | TdsEnums.SNI_SSL_USE_SCHANNEL_CACHE | TdsEnums.SNI_SSL_SEND_ALPN_EXTENSION;
 
-                EnableSsl(info, encrypt, integratedSecurity, serverCert);
+                EnableSsl(info, encrypt, integratedSecurity, serverCertificateFilename);
 
                 // Since encryption has already been negotiated, we need to set encryption not supported in
                 // prelogin so that we don't try to negotiate encryption again during ConsumePreLoginHandshake.
@@ -831,7 +871,7 @@ private void SendPreLoginHandshake(
             _physicalStateObj.WritePacket(TdsEnums.HARDFLUSH);
         }
 
-        private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integratedSecurity, string serverCert)
+        private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integratedSecurity, string serverCertificateFilename)
         {
             uint error = 0;
 
@@ -843,7 +883,7 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ
                 info |= TdsEnums.SNI_SSL_IGNORE_CHANNEL_BINDINGS;
             }
 
-            error = _physicalStateObj.EnableSsl(ref info, encrypt == SqlConnectionEncryptOption.Strict, serverCert);
+            error = _physicalStateObj.EnableSsl(ref info, encrypt == SqlConnectionEncryptOption.Strict, serverCertificateFilename);
 
             if (error != TdsEnums.SNI_SUCCESS)
             {