Skip to content

Commit

Permalink
Addressing review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@JRahnama
JRahnama committed Aug 21, 2023
1 parent 610a2c2 commit 8c411aa
Show file tree
Hide file tree
Showing 8 changed files with 623 additions and 21 deletions.
122 changes: 104 additions & 18 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics;
using System.Net;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using Microsoft.Data.Common;

namespace Microsoft.Data.SqlClient.SNI
{
Expand Down Expand Up @@ -146,31 +149,72 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C
return true;
}

if ((policyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != 0)
// If we get to this point then there is a ssl policy flag.
StringBuilder messageBuilder = new();
if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, SslPolicyError {1}, SSL Policy certificate chain has errors.", args0: targetServerName, args1: policyErrors);

// get the chain status from the certificate
X509Certificate2 cert2 = cert 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, "targetServerName {0}, SslPolicyError {1}, SSL Policy certificate chain has errors. ChainStatus {2}", args0: targetServerName, args1: policyErrors, args2: chainStatusInformation);
messageBuilder.AppendFormat(Strings.SQL_RemoteCertificateChainErrors, chainStatusInformation);
messageBuilder.AppendLine();
}

if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, SSL Policy invalidated certificate.", args0: targetServerName);
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNotAvailable);
}

if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNameMismatch))
{
#if NET7_0_OR_GREATER
X509Certificate2 cert2 = cert as X509Certificate2;
if (!cert2.MatchesHostname(targetServerName))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, Target Server name or HNIC does not match the Subject/SAN in Certificate.", args0: targetServerName);
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
#else
// To Do: include certificate SAN (Subject Alternative Name) check.
string certServerName = cert.Subject.Substring(cert.Subject.IndexOf('=') + 1);

// Verify that target server name matches subject in the certificate
if (targetServerName.Length > certServerName.Length)
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, Target Server name is of greater length than Subject in Certificate.", args0: targetServerName);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
else if (targetServerName.Length == certServerName.Length)
{
// Both strings have the same length, so targetServerName must be a FQDN
if (!targetServerName.Equals(certServerName, StringComparison.OrdinalIgnoreCase))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, Target Server name does not match Subject in Certificate.", args0: targetServerName);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
}
else
{
if (string.Compare(targetServerName, 0, certServerName, 0, targetServerName.Length, StringComparison.OrdinalIgnoreCase) != 0)
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, Target Server name does not match Subject in Certificate.", args0: targetServerName);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}

// Server name matches cert name for its whole length, so ensure that the
Expand All @@ -180,21 +224,22 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C
if (certServerName[targetServerName.Length] != '.')
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, Target Server name does not match Subject in Certificate.", args0: targetServerName);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
}
#endif
}
else

if (messageBuilder.Length > 0)
{
// Fail all other SslPolicy cases besides RemoteCertificateNameMismatch
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "targetServerName {0}, SslPolicyError {1}, SSL Policy invalidated certificate.", args0: targetServerName, args1: policyErrors);
return false;
throw ADP.SSLCertificateAuthenticationException(messageBuilder.ToString());
}
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.INFO, "targetServerName {0}, Client certificate validated successfully.", args0: targetServerName);

SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.INFO, " Remote certificate with subject: {0}, validated successfully.", args0: cert.Subject);
return true;
}
}

/// <summary>
/// We validate the provided certificate provided by the client with the one from the server to see if it matches.
/// Certificate validation and chain trust validations are done by SSLStream class [System.Net.Security.SecureChannel.VerifyRemoteCertificate method]
Expand All @@ -214,26 +259,67 @@ internal static bool ValidateSslServerCertificate(X509Certificate clientCert, X5
return true;
}

if ((policyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != 0)
StringBuilder messageBuilder = new();
if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNotAvailable))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "serverCert {0}, SSL Server certificate not validated as PolicyErrors set to RemoteCertificateNotAvailable.", args0: clientCert.Subject);
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNotAvailable);
}

if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors))
{
// get the chain status from the server 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, "certificate subject from server is {0}, and does not match with the certificate provided client.", args0: cert2.SubjectName.Name);
messageBuilder.AppendFormat(Strings.SQL_RemoteCertificateChainErrors, chainStatusInformation);
messageBuilder.AppendLine();
}

if (policyErrors.HasFlag(SslPolicyErrors.RemoteCertificateNameMismatch))
{
#if NET7_0_OR_GREATER
X509Certificate2 s_cert = serverCert as X509Certificate2;
X509Certificate2 c_cert = clientCert as X509Certificate2;

if (!s_cert.MatchesHostname(c_cert.SubjectName.Name))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "certificate from server does not match with the certificate provided client.", args0: s_cert.Subject);
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
#else
// Verify that subject name matches
if (serverCert.Subject != clientCert.Subject)
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "certificate subject from server is {0}, and does not match with the certificate provided client.", args0: serverCert.Subject);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}

if (!serverCert.Equals(clientCert))
{
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "certificate from server does not match with the certificate provided client.", args0: serverCert.Subject);
return false;
messageBuilder.AppendLine(Strings.SQL_RemoteCertificateNameMismatch);
}
#endif
}
else

if (messageBuilder.Length > 0)
{
// Fail all other SslPolicy cases besides RemoteCertificateNameMismatch
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.ERR, "certificate subject: {0}, SslPolicyError {1}, SSL Policy invalidated certificate.", args0: clientCert.Subject, args1: policyErrors);
return false;
throw ADP.SSLCertificateAuthenticationException(messageBuilder.ToString());
}

SqlClientEventSource.Log.TrySNITraceEvent(nameof(SNICommon), EventType.INFO, "certificate subject {0}, Client certificate validated successfully.", args0: clientCert.Subject);
return true;
}
Expand Down
12 changes: 10 additions & 2 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using IsolationLevel = System.Data.IsolationLevel;
using Microsoft.Identity.Client;
using Microsoft.SqlServer.Server;
using System.Security.Authentication;

#if NETFRAMEWORK
using Microsoft.Win32;
Expand Down Expand Up @@ -326,9 +327,16 @@ internal static ArgumentOutOfRangeException ArgumentOutOfRange(string message, s
TraceExceptionAsReturnValue(e);
return e;
}
#endregion

#region Helper Functions
internal static AuthenticationException SSLCertificateAuthenticationException(string message)
{
AuthenticationException e = new(message);
TraceExceptionAsReturnValue(e);
return e;
}
#endregion

#region Helper Functions
internal static ArgumentOutOfRangeException NotSupportedEnumerationValue(Type type, string value, string method)
=> ArgumentOutOfRange(StringsHelper.GetString(Strings.ADP_NotSupportedEnumerationValue, type.Name, value, method), type.Name);

Expand Down
27 changes: 27 additions & 0 deletions src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/Microsoft.Data.SqlClient/src/Resources/Strings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4731,4 +4731,13 @@
<data name="SqlParameter_SourceColumnNullMapping" xml:space="preserve">
<value>When used by DataAdapter.Update, the parameter value is changed from DBNull.Value into (Int32)1 or (Int32)0 if non-null.</value>
</data>
<data name="SQL_RemoteCertificateChainErrors" xml:space="preserve">
<value>Certificate failed chain validation. Error(s): '{0}'.</value>
</data>
<data name="SQL_RemoteCertificateNameMismatch" xml:space="preserve">
<value>Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.</value>
</data>
<data name="SQL_RemoteCertificateNotAvailable" xml:space="preserve">
<value>Certificate not available while validating the certificate.</value>
</data>
</root>

0 comments on commit 8c411aa

Please sign in to comment.