diff --git a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml index 55b60b6120..bdf4839664 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml @@ -1003,7 +1003,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.

This check is **additive** to normal certificate validation, not a replacement for it. When the driver validates the server certificate, the presented certificate must both pass chain-and-name validation **and** match the certificate loaded from this path exactly. If the file cannot be loaded or parsed, the TLS handshake fails; the connection is never silently accepted on the basis of platform trust alone.

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 4014bd1450..fd571980f8 100644 --- a/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml +++ b/doc/snippets/Microsoft.Data.SqlClient/SqlConnectionStringBuilder.xml @@ -1332,6 +1332,16 @@ Database = AdventureWorks [!NOTE] +> `ServerCertificate` is **additive** to normal certificate validation, not a replacement for it. When the driver validates the server certificate, the presented certificate must: +> +> 1. Pass chain-and-name validation, **and** +> 2. Be an exact byte-for-byte match of the certificate loaded from the file at this path. +> +> Both checks are enforced independently. If the file cannot be loaded or parsed, the TLS handshake fails; the connection is never silently accepted on the basis of platform trust alone. +> +> 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 c2d3e0073c..95a4e760a0 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 @@ -44,8 +44,15 @@ internal class SniCommon internal const int LocalDBBadRuntime = 57; /// - /// We either validate that the provided 'validationCert' matches the 'serverCert', or we validate that the server name in the 'serverCert' matches 'targetServerName'. - /// Certificate validation and chain trust validations are done by SSLStream class [System.Net.Security.SecureChannel.VerifyRemoteCertificate method] + /// Validates the server's TLS certificate. When a + /// pin is supplied, the presented server certificate must be a byte-for-byte match of the + /// pinned certificate. The pin is additive: it does not replace chain / name + /// validation. The presented certificate must both match the pin (when supplied) and + /// pass the platform's chain / name checks (i.e. must + /// be ). If the pin file cannot be loaded or parsed, + /// validation fails closed. + /// Certificate chain / name validation is performed by SslStream + /// [System.Net.Security.SecureChannel.VerifyRemoteCertificate]. /// This method is called as a result of callback for SSL Stream Certificate validation. /// /// Connection ID/GUID for tracing @@ -59,7 +66,12 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ { using (SqlClientSNIEventScope.Create(nameof(SniCommon))) { - if (policyErrors == SslPolicyErrors.None) + // When the caller supplies a ServerCertificate pin we always compare the presented + // server certificate against the pin, even if the platform's chain / name validation + // already succeeded (policyErrors == None). The pin is additive: it must match in + // addition to any platform trust decision. Only skip further work when no pin was + // supplied AND the platform reported no policy errors. + 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); return true; @@ -76,39 +88,61 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ serverNameToValidate = targetServerName; } - if (!string.IsNullOrEmpty(validationCertFileName)) + try { - try + if (!string.IsNullOrEmpty(validationCertFileName)) { + try + { #if NET9_0_OR_GREATER - validationCertificate = X509CertificateLoader.LoadCertificateFromFile(validationCertFileName); + validationCertificate = X509CertificateLoader.LoadCertificateFromFile(validationCertFileName); #else - validationCertificate = new X509Certificate(validationCertFileName); + validationCertificate = new X509Certificate(validationCertFileName); #endif + } + catch (Exception e) + { + // Fail closed: the caller explicitly asked us to pin against a specific + // certificate, so if we cannot load / parse that pin we must not accept the + // server certificate on the basis of platform trust alone. The exception + // details are traced separately; the user-facing message identifies the + // configured pin 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)); + } + + // If the server did not present a certificate (e.g. SslPolicyErrors + // .RemoteCertificateNotAvailable), there is nothing to compare the pin + // against. Skip the byte comparison and fall through to the policy-error + // handler below, which will report the missing-certificate condition with + // the appropriate AuthenticationException instead of a NullReferenceException. + if (serverCert != null) + { + // Enforce the pin: the presented server certificate must be a byte-for-byte + // match of the pinned certificate. On mismatch, fail immediately. + if (!serverCert.GetRawCertData().AsSpan().SequenceEqual(validationCertificate.GetRawCertData().AsSpan())) + { + 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); + throw ADP.SSLCertificateAuthenticationException(Strings.SQL_RemoteCertificateDoesNotMatchServerCertificate); + } + + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, ServerCertificate matches the certificate provided by the server. Continuing with chain / name validation.", args0: connectionId); + // Pin matched. Fall through so that chain / name validation is still + // enforced additively; a matching pin must never mask a policy error. + } } - 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); - } - } - if (validationCertificate != null) - { - if (serverCert.GetRawCertData().AsSpan().SequenceEqual(validationCertificate.GetRawCertData().AsSpan())) + // If the platform reported no policy errors, validation succeeds. When a pin + // was supplied we only reach this point after the pin matched, so this covers + // both the pin and no-pin success paths additively. + if (policyErrors == SslPolicyErrors.None) { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, ServerCertificate matches the certificate provided by the server. Certificate validation passed.", args0: connectionId); + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, certificate with subject: {1}, validated successfully.", args0: connectionId, args1: serverCert.Subject); return true; } - 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); - throw ADP.SSLCertificateAuthenticationException(Strings.SQL_RemoteCertificateDoesNotMatchServerCertificate); - } - } - else - { - // If we get to this point then there is a ssl policy flag. + + // At this point at least one platform policy error is set. Build a descriptive + // message and fail — even if a ServerCertificate pin matched above. StringBuilder messageBuilder = new(); if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable)) { @@ -154,10 +188,16 @@ internal static bool ValidateSslServerCertificate(Guid connectionId, string targ { 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; + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniCommon), EventType.INFO, "Connection Id {0}, certificate with subject: {1}, validated successfully.", args0: connectionId, args1: serverCert.Subject); + return true; + } + finally + { + // X509Certificate owns unmanaged handles; dispose the pin certificate we + // loaded above so it is not left to the finalizer. + validationCertificate?.Dispose(); + } } } diff --git a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs index 7064a6c19c..817cac89ac 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs @@ -3867,6 +3867,15 @@ 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 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 e7f438873f..25848865e0 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx @@ -2127,6 +2127,9 @@ 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}'. + 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..1679c6ab02 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniCommonValidateSslServerCertificateTest.cs @@ -0,0 +1,222 @@ +// 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 pin the driver enforces the + /// pin as an *additive* check on top of normal TLS validation, and never accepts a + /// server certificate on the basis of platform trust alone. + /// + /// 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"); + + string pinPath = WriteCertToTempFile(pinCert); + try + { + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinPath, + policyErrors: SslPolicyErrors.None)); + } + finally + { + File.Delete(pinPath); + } + } + + /// + /// 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"); + + string pinPath = WriteCertToTempFile(serverCert); + try + { + bool result = SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinPath, + policyErrors: SslPolicyErrors.None); + + Assert.True(result); + } + finally + { + File.Delete(pinPath); + } + } + + /// + /// A matching ServerCertificate pin must not mask a platform policy error. + /// This regression test guards the "additive" semantics: even when the pin bytes + /// match exactly, if the platform reported + /// (or any other non-None flag), validation must still fail rather than + /// short-circuit on the pin. + /// + [Fact] + public void ValidateSslServerCertificate_MatchingPin_ChainErrors_Throws() + { + using X509Certificate2 serverCert = CreateSelfSignedCertificate("server.contoso.com"); + + string pinPath = WriteCertToTempFile(serverCert); + try + { + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: serverCert, + validationCertFileName: pinPath, + policyErrors: SslPolicyErrors.RemoteCertificateChainErrors)); + } + finally + { + File.Delete(pinPath); + } + } + + /// + /// 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 pin + /// comparison. + /// + [Fact] + public void ValidateSslServerCertificate_Pin_NullServerCert_NotAvailable_Throws() + { + using X509Certificate2 pinCert = CreateSelfSignedCertificate("server.contoso.com"); + + string pinPath = WriteCertToTempFile(pinCert); + try + { + Assert.Throws(() => + SniCommon.ValidateSslServerCertificate( + connectionId: Guid.NewGuid(), + targetServerName: "server.contoso.com", + hostNameInCertificate: null, + serverCert: null, + validationCertFileName: pinPath, + policyErrors: SslPolicyErrors.RemoteCertificateNotAvailable)); + } + finally + { + File.Delete(pinPath); + } + } + + /// + /// 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 falling back to platform trust. The caller + /// explicitly asked us to pin 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); + } + + private static string WriteCertToTempFile(X509Certificate2 cert) + { + string path = Path.Combine( + Path.GetTempPath(), + "SqlClient_Pin_" + Guid.NewGuid().ToString("N") + ".cer"); + File.WriteAllBytes(path, cert.Export(X509ContentType.Cert)); + return path; + } + } +} + +#endif