From e4180895bf7f4d4083cda7b6fe206dc4828cf7a6 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Mon, 24 Aug 2026 09:26:49 -0700 Subject: [PATCH] Fix | Perform ServerCertificate pin validation to verify with server provided cert (#4445) * Server Certificate validation changes * Address comments * Address review feedback: reorder pin check after policy checks - Perform ServerCertificate pin validation after chain/name policy error handling, making the additive semantics explicit in control flow. - Extract ValidateCertificatePin/LoadValidationCertificate helpers so the loaded pin certificate lives in a narrow 'using' scope (no try/finally). - Guard against a null server certificate in the pin path. - Clarify method docs on the policyErrors relationship and reword the no-pin fast-path trace message. - Replace ambiguous 'platform trust' wording in docs with chain-and-name validation, and note that pinning does not itself confer trust. - Add TempCertFile disposable test helper and a null-server-cert regression test for the policyErrors == None case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Revert to minimal design: pin always consulted, fail closed on load error Restores the original semantic where an exact ServerCertificate match satisfies certificate validation. The only behavior changes remain: - The policyErrors == None fast path now also requires that no ServerCertificate was supplied, so a configured certificate is always compared against the one presented by the server. - A ServerCertificate that cannot be loaded/parsed now fails the connection instead of being silently ignored. Also keeps the non-design fixes: the loaded certificate is disposed via a narrow using scope, and a null server certificate throws AuthenticationException instead of NullReferenceException. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Address review: dedicated string, RemoteCertificateNotAvailable guard, safe Dispose - Add SQL_ServerCertificateNotAvailable so the failure names the option in play rather than reusing the generic policy-error text. - A matching ServerCertificate no longer satisfies validation when RemoteCertificateNotAvailable is reported. The flag and a null serverCert are expected to agree, but both are now checked so the contradictory combination cannot be accepted via comparison. Documented the relationship on the method. - Guard TempCertFile.Dispose so a cleanup failure cannot mask a real assertion failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../SqlConnection.xml | 2 +- .../SqlConnectionStringBuilder.xml | 7 + .../SqlClient/ManagedSni/SniCommon.netcore.cs | 151 ++++++---- .../src/Resources/Strings.Designer.cs | 18 ++ .../src/Resources/Strings.resx | 6 + ...iCommonValidateSslServerCertificateTest.cs | 284 ++++++++++++++++++ 6 files changed, 411 insertions(+), 57 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniCommonValidateSslServerCertificateTest.cs diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml index 11602bd078..79e7322815 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml @@ -989,7 +989,7 @@ The following table lists the valid names for keyword values within the
If the value of this key is "", then **Initial Catalog** must be present, and its value must not be "".

The server name can be 128 characters or less.

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.

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

-or-

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.

(Only available in v5.0+)| |Host Name In Certificate

-or-

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.

(Only available in v5.0+)| -|Server Certificate

-or-

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.

(Only available in v5.1+)| +|Server Certificate

-or-

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.

When specified, this comparison is always performed, including when the certificate already passes the usual chain-and-name validation. If the presented certificate does not match, or the file cannot be loaded or parsed, the TLS handshake fails; a configured `ServerCertificate` is never silently ignored.

Certificate validation itself can be disabled by `TrustServerCertificate=true` (except with `Encrypt=strict`, where validation is always performed). When validation is disabled, `ServerCertificate` is not consulted.

(Only available in v5.1+)| |Initial Catalog

-or-

Database|N/A|The name of the database.

The database name can be 128 characters or less.| |Integrated Security

-or-

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.

Recognized values are `true`, `false`, `yes`, `no`, and `sspi` (strongly recommended), which is equivalent to `true`.

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.

is a more secure way to specify credentials for a connection that uses SQL Server Authentication (`Integrated Security=false`).| |IP Address Preference

-or-

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:

`IPAddressPreference=IPv4First`

`IPAddressPreference=IPv6First`

`IPAddressPreference=UsePlatformDefault`| diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml index cd105cbf17..998aef7cf0 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml @@ -1307,6 +1307,13 @@ Database = AdventureWorks [!NOTE] +> When `ServerCertificate` is specified, the certificate presented by the server is always compared against the certificate loaded from this path, including when the presented certificate already passes chain-and-name validation. An exact match satisfies certificate validation. +> +> If the presented certificate does not match, or the file cannot be loaded or parsed, the TLS handshake fails; a configured `ServerCertificate` is never silently ignored. +> +> Certificate validation itself can be disabled by `TrustServerCertificate=true` (except with `Encrypt=strict`, where validation is always performed). When validation is disabled, `ServerCertificate` is not consulted. + ]]> diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs index 9653c94d34..06a0cce855 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniCommon.netcore.cs @@ -47,6 +47,20 @@ internal class SniCommon /// 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. /// + /// + /// When is supplied, the presented server + /// certificate is always compared against it, even when is + /// . An exact match satisfies certificate validation. A + /// configured certificate that cannot be loaded or parsed, or that does not match, fails + /// the connection; it is never silently ignored. + /// + /// The one condition a match does not satisfy is a missing server certificate. SslStream + /// reports that as and passes a + /// null ; the two are expected to agree, but both are checked + /// so that neither an absent certificate nor the contradictory combination can be accepted + /// on the basis of a comparison. + /// + /// /// Connection ID/GUID for tracing /// Server that client is expecting to connect to /// Optional hostname to use for server certificate validation @@ -58,14 +72,14 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ { using (TrySNIEventScope.Create(nameof(SniCommon))) { - if (policyErrors == SslPolicyErrors.None) + // When a pin is configured, require it to match even if the platform trusts the cert. + if (string.IsNullOrEmpty(validationCertFileName) && policyErrors == SslPolicyErrors.None) { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, targetServerName {1}, SSL Server certificate not validated as PolicyErrors set to None.", args0: connectionId, args1: targetServerName); + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, targetServerName {1}, SSL Server certificate has no policy errors and no ServerCertificate was supplied; skipping certificate comparison.", args0: connectionId, args1: targetServerName); return true; } string serverNameToValidate; - X509Certificate validationCertificate = null; if (!string.IsNullOrEmpty(hostNameInCertificate)) { serverNameToValidate = hostNameInCertificate; @@ -77,23 +91,19 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ if (!string.IsNullOrEmpty(validationCertFileName)) { - try + // There is no trustworthy certificate to compare against when the server + // presented none. SslStream reports that as RemoteCertificateNotAvailable, and + // serverCert is null in that case; both are checked because a matching + // ServerCertificate must never satisfy validation when the presented + // certificate is absent or cannot be relied upon. + if (serverCert is null || policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable)) { -#if NET9_0_OR_GREATER - validationCertificate = X509CertificateLoader.LoadCertificateFromFile(validationCertFileName); -#else - validationCertificate = new X509Certificate(validationCertFileName); -#endif - } - 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}, Exception occurred loading specified ServerCertificate: {1}, treating it as if ServerCertificate has not been specified.", args0: connectionId, args1: e.Message); + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, ServerCertificate was specified but the server presented no certificate. Certificate validation failed.", args0: connectionId); + throw ADP.SSLCertificateAuthenticationException(Strings.SQL_ServerCertificateNotAvailable); } - } - if (validationCertificate != null) - { + using X509Certificate validationCertificate = LoadValidationCertificate(connectionId, validationCertFileName); + if (serverCert.GetRawCertData().AsSpan().SequenceEqual(validationCertificate.GetRawCertData().AsSpan())) { SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, ServerCertificate matches the certificate provided by the server. Certificate validation passed.", args0: connectionId); @@ -101,65 +111,94 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ } else { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, ServerCertificate doesn't match the certificate provided by the server. Certificate validation failed.", args0: connectionId); + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, ServerCertificate doesn't match the certificate provided by the server. Certificate validation failed.", args0: connectionId); throw ADP.SSLCertificateAuthenticationException(Strings.SQL_RemoteCertificateDoesNotMatchServerCertificate); } } - else + + // If we get to this point then there is a ssl policy flag. + StringBuilder messageBuilder = new(); + if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable)) { - // If we get to this point then there is a ssl policy flag. - StringBuilder messageBuilder = new(); - if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable)) - { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {1}, SSL Server certificate not validated as PolicyErrors set to RemoteCertificateNotAvailable.", args0: connectionId, args1: targetServerName); - messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNotAvailable); - } + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {1}, SSL Server certificate not validated as PolicyErrors set to RemoteCertificateNotAvailable.", args0: connectionId, args1: targetServerName); + messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNotAvailable); + } - if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors)) - { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {0}, SslPolicyError {1}, SSL Policy certificate chain has errors.", args0: connectionId, args1: targetServerName, args2: policyErrors); - - // get the chain status from the certificate - X509Certificate2 cert2 = serverCert as X509Certificate2; - X509Chain chain = new(); - chain.ChainPolicy.RevocationMode = X509RevocationMode.Offline; - StringBuilder chainStatusInformation = new(); - bool chainIsValid = chain.Build(cert2); - Debug.Assert(!chainIsValid, "RemoteCertificateChainError flag is detected, but certificate chain is valid."); - if (!chainIsValid) - { - foreach (X509ChainStatus chainStatus in chain.ChainStatus) - { - chainStatusInformation.Append($"{chainStatus.StatusInformation}, [Status: {chainStatus.Status}]"); - chainStatusInformation.AppendLine(); - } - } - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {1}, SslPolicyError {2}, SSL Policy certificate chain has errors. ChainStatus {3}", args0: connectionId, args1: targetServerName, args2: policyErrors, args3: chainStatusInformation); - messageBuilder.AppendFormat(Strings.SQL_RemoteCertificateChainErrors, chainStatusInformation); - messageBuilder.AppendLine(); - } + if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors)) + { + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {0}, SslPolicyError {1}, SSL Policy certificate chain has errors.", args0: connectionId, args1: targetServerName, args2: policyErrors); - if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNameMismatch)) + // get the chain status from the certificate + X509Certificate2 cert2 = serverCert as X509Certificate2; + X509Chain chain = new(); + chain.ChainPolicy.RevocationMode = X509RevocationMode.Offline; + StringBuilder chainStatusInformation = new(); + bool chainIsValid = chain.Build(cert2); + Debug.Assert(!chainIsValid, "RemoteCertificateChainError flag is detected, but certificate chain is valid."); + if (!chainIsValid) { - X509Certificate2 cert2 = serverCert as X509Certificate2; - if (!cert2.MatchesHostname(serverNameToValidate)) + foreach (X509ChainStatus chainStatus in chain.ChainStatus) { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, serverNameToValidate {1}, Target Server name or HNIC does not match the Subject/SAN in Certificate.", args0: connectionId, args1: serverNameToValidate); - messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch); + chainStatusInformation.Append($"{chainStatus.StatusInformation}, [Status: {chainStatus.Status}]"); + chainStatusInformation.AppendLine(); } } + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, targetServerName {1}, SslPolicyError {2}, SSL Policy certificate chain has errors. ChainStatus {3}", args0: connectionId, args1: targetServerName, args2: policyErrors, args3: chainStatusInformation); + messageBuilder.AppendFormat(Strings.SQL_RemoteCertificateChainErrors, chainStatusInformation); + messageBuilder.AppendLine(); + } - if (messageBuilder.Length > 0) + if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNameMismatch)) + { + X509Certificate2 cert2 = serverCert as X509Certificate2; + if (!cert2.MatchesHostname(serverNameToValidate)) { - throw ADP.SSLCertificateAuthenticationException(messageBuilder.ToString()); + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, serverNameToValidate {1}, Target Server name or HNIC does not match the Subject/SAN in Certificate.", args0: connectionId, args1: serverNameToValidate); + messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch); } } + if (messageBuilder.Length > 0) + { + throw ADP.SSLCertificateAuthenticationException(messageBuilder.ToString()); + } + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, certificate with subject: {1}, validated successfully.", args0: connectionId, args1: serverCert.Subject); return true; } } + /// + /// Loads the certificate specified by the ServerCertificate option, failing closed + /// when it cannot be loaded or parsed. + /// + /// Connection ID/GUID for tracing + /// Path to the X.509 certificate file + /// + /// Thrown when the file cannot be loaded or parsed. + /// + private static X509Certificate LoadValidationCertificate(Guid connectionId, string validationCertFileName) + { + try + { +#if NET9_0_OR_GREATER + return X509CertificateLoader.LoadCertificateFromFile(validationCertFileName); +#else + return new X509Certificate(validationCertFileName); +#endif + } + catch (Exception e) + { + // Fail closed: the caller explicitly asked us to compare against a specific + // certificate, so if we cannot load / parse it we must not fall back to validating + // by host name alone. The exception details are traced separately; the user-facing + // message identifies the configured file path so operators can locate the + // misconfiguration. + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.ERR, "Connection Id {0}, Exception occurred loading specified ServerCertificate '{1}': {2}. Failing certificate validation.", args0: connectionId, args1: validationCertFileName, args2: e.Message); + throw ADP.SSLCertificateAuthenticationException(StringsHelper.GetString(Strings.SQL_ServerCertificateFileLoadFailed, validationCertFileName)); + } + } + internal static IPAddress[] GetDnsIpAddresses(string serverName, TimeoutTimer timeout) { IPAddress[] ipAddresses = GetDnsIpAddresses(serverName); diff --git a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs index 9f460e7c04..469208c411 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs @@ -10271,6 +10271,24 @@ internal static string SQL_ScaleValueOutOfRange { } } + /// + /// Looks up a localized string similar to The certificate file specified by the 'ServerCertificate' option could not be loaded or parsed: '{0}'.. + /// + internal static string SQL_ServerCertificateFileLoadFailed { + get { + return ResourceManager.GetString("SQL_ServerCertificateFileLoadFailed", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The 'ServerCertificate' option was specified, but the server did not present a certificate that could be compared against it.. + /// + internal static string SQL_ServerCertificateNotAvailable { + get { + return ResourceManager.GetString("SQL_ServerCertificateNotAvailable", resourceCulture); + } + } + /// /// Looks up a localized string similar to Cannot set the Credential property if 'Authentication=Active Directory Device Code Flow' has been specified in the connection string.. /// diff --git a/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx index 71cc24cc56..5cfce2a3ff 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx @@ -4731,6 +4731,12 @@ The certificate provided by the server does not match the certificate provided by the ServerCertificate option. + + The certificate file specified by the 'ServerCertificate' option could not be loaded or parsed: '{0}'. + + + The 'ServerCertificate' option was specified, but the server did not present a certificate that could be compared against it. + Invalid attempt to get JsonDocument on column '{0}'. JsonDocument is only supported for columns of type json. diff --git a/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniCommonValidateSslServerCertificateTest.cs b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniCommonValidateSslServerCertificateTest.cs new file mode 100644 index 0000000000..5f76a33602 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniCommonValidateSslServerCertificateTest.cs @@ -0,0 +1,284 @@ +// 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. + +#if NET + +using System; +using System.IO; +using System.Net.Security; +using System.Security.Authentication; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.Data.SqlClient.ManagedSni; +using Xunit; + +namespace Microsoft.Data.SqlClient.UnitTests.ManagedSni +{ + /// + /// Tests for to ensure + /// that when a caller supplies a ServerCertificate, the driver always compares it + /// against the certificate presented by the server — including when the platform reported + /// no policy errors — and fails closed when the configured file cannot be loaded. + /// + /// The tests exercise the shared helper directly. + /// + public class SniCommonValidateSslServerCertificateTest + { + /// + /// When a ServerCertificate pin is supplied and the presented server certificate + /// does not match the pin, validation must fail — even if the platform reported + /// . This is the security invariant the pin + /// exists to enforce. + /// + [Fact] + public void ValidateSslServerCertificate_MismatchedPin_PolicyNone_Throws() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + using X509Certificate2 pinCert = CreateSelfSignedCertificate("other.contoso.com"); + using TempCertFile pinFile = new(pinCert); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.None)); + } + + /// + /// When a ServerCertificate pin is supplied and the presented server certificate + /// matches the pin exactly, validation succeeds (assuming no policy errors). + /// + [Fact] + public void ValidateSslServerCertificate_MatchingPin_PolicyNone_ReturnsTrue() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + using TempCertFile pinFile = new(serverCert); + + bool result = SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.None); + + Assert.True(result); + } + + /// + /// An exact ServerCertificate match satisfies validation, which is the purpose of the + /// option: the caller has told us precisely which certificate to accept. This preserves + /// the long-standing behavior for servers whose certificate would otherwise fail chain + /// validation (for example a self-signed or private-CA certificate). + /// + [Fact] + public void ValidateSslServerCertificate_MatchingPin_ChainErrors_ReturnsTrue() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + using TempCertFile pinFile = new(serverCert); + + bool result = SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.RemoteCertificateChainErrors); + + Assert.True(result); + } + + /// + /// A ServerCertificate mismatch fails even when the platform reported a policy error that + /// would otherwise be evaluated, because the caller's explicit choice of certificate takes + /// precedence. + /// + [Fact] + public void ValidateSslServerCertificate_MismatchedPin_ChainErrors_Throws() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + using X509Certificate2 pinCert = CreateSelfSignedCertificate("other.contoso.com"); + using TempCertFile pinFile = new(pinCert); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.RemoteCertificateChainErrors)); + } + + /// + /// When a ServerCertificate pin is supplied but the server did not present a + /// certificate at all (, + /// serverCert == null), the helper must fail with an + /// — not a + /// from attempting to dereference the missing server certificate during comparison. + /// + [Fact] + public void ValidateSslServerCertificate_Pin_NullServerCert_NotAvailable_Throws() + { + using X509Certificate2 pinCert = CreateSelfSignedCertificate("server.contoso.com"); + using TempCertFile pinFile = new(pinCert); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: null, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.RemoteCertificateNotAvailable)); + } + + /// + /// A matching ServerCertificate must not satisfy validation when the platform reported + /// , even if a non-null + /// certificate was somehow also handed to the callback. The two are expected to agree, + /// but the contradictory combination must fail closed rather than let a comparison stand + /// in for a certificate the platform said was unavailable. + /// + [Fact] + public void ValidateSslServerCertificate_MatchingPin_NotAvailable_Throws() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + using TempCertFile pinFile = new(serverCert); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.RemoteCertificateNotAvailable)); + } + + /// + /// Defense in depth: a null server certificate is normally reported through + /// , but should the two ever + /// disagree, the comparison must still fail with an + /// rather than a . + /// + [Fact] + public void ValidateSslServerCertificate_Pin_NullServerCert_PolicyNone_Throws() + { + using X509Certificate2 pinCert = CreateSelfSignedCertificate("server.contoso.com"); + using TempCertFile pinFile = new(pinCert); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: null, + validationCertFileName: pinFile.Path, + policyErrors: SslPolicyErrors.None)); + } + + /// + /// When no ServerCertificate pin is supplied, the historical short-circuit is + /// preserved: means the platform already + /// validated the certificate, so the helper returns true. This guards against + /// unintended breakage of the pin-less path. + /// + [Fact] + public void ValidateSslServerCertificate_NoPin_PolicyNone_ReturnsTrue() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + + bool result = SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: null, + policyErrors: SslPolicyErrors.None); + + Assert.True(result); + } + + /// + /// When a ServerCertificate pin is supplied but the file cannot be loaded + /// (missing file, corrupt bytes, wrong format, etc.), validation must fail + /// closed rather than silently ignoring the option and falling back to host name + /// validation. The caller explicitly asked us to compare against a specific + /// certificate, so we cannot accept the connection on any weaker basis. + /// + [Fact] + public void ValidateSslServerCertificate_UnreadablePinFile_PolicyNone_Throws() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + + string missingPath = Path.Combine( + Path.GetTempPath(), + "SqlClient_MissingPin_" + Guid.NewGuid().ToString("N") + ".cer"); + + Assert.False(File.Exists(missingPath)); + + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: missingPath, + policyErrors: SslPolicyErrors.None)); + } + + private static X509Certificate2 CreateSelfSignedCertificate(string subjectCommonName) + { + using RSA rsa = RSA.Create(2048); + CertificateRequest request = new( + $"CN={subjectCommonName}", + rsa, + HashAlgorithmName.SHA256, + RSASignaturePadding.Pkcs1); + + DateTimeOffset notBefore = DateTimeOffset.UtcNow.AddMinutes(-5); + DateTimeOffset notAfter = notBefore.AddHours(1); + return request.CreateSelfSigned(notBefore, notAfter); + } + + /// + /// Writes a certificate to a temporary file for use as a ServerCertificate pin and + /// deletes it on disposal. + /// + private sealed class TempCertFile : IDisposable + { + public string Path { get; } + + public TempCertFile(X509Certificate2 cert) + { + Path = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "SqlClient_Pin_" + Guid.NewGuid().ToString("N") + ".cer"); + File.WriteAllBytes(Path, cert.Export(X509ContentType.Cert)); + } + + public void Dispose() + { + try + { + File.Delete(Path); + } + catch (IOException) + { + // Best effort: a cleanup failure must not mask the assertion failure that + // caused the test to unwind through this Dispose. + } + catch (UnauthorizedAccessException) + { + } + } + } + } +} + +#endif