Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update | Update SSL certificate error messages #2060

Merged
merged 5 commits into from Sep 2, 2023
Merged

Update | Update SSL certificate error messages #2060

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8c411aa
Addressing review comments
JRahnama Aug 21, 2023
a77baad
Adding License to test class.
JRahnama Aug 21, 2023
f77f952
Revierw comments
JRahnama Sep 1, 2023
079dfb0
Merge branch 'main' into update-namemismatch-error
JRahnama Sep 1, 2023
b73097d
Apply suggestions from code review
JRahnama Sep 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
119 changes: 102 additions & 17 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs
View file
Open in desktop
Expand Up @@ -7,6 +7,8 @@
using System.Net;
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;
using System.Text;
using Microsoft.Data.Common;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.ProviderBase;
Expand Down Expand Up @@ -150,31 +152,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 @@ -184,17 +227,18 @@ 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)
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
{
// 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;
}
}
Expand All @@ -218,26 +262,67 @@ internal static bool ValidateSslServerCertificate(X509Certificate clientCert, X5
return true;
}

if ((policyErrors & SslPolicyErrors.RemoteCertificateNameMismatch) != 0)
StringBuilder messageBuilder = new();
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
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))
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
{
#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))
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
{
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)
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
{
// 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
Open in desktop
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
Open in desktop

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
Open in desktop
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>
84 changes: 83 additions & 1 deletion src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@
using Xunit;
using System.Net.NetworkInformation;
using System.Text;
using System.Security.Principal;

namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
Expand Down Expand Up @@ -85,6 +86,35 @@ public static class DataTestUtility
public static readonly string KerberosDomainUser = null;
internal static readonly string KerberosDomainPassword = null;

// SQL server Version
private static string s_sQLServerVersion = string.Empty;
private static bool s_isTDS8Supported;

public static string SQLServerVersion
{
get
{
if (!string.IsNullOrEmpty(TCPConnectionString))
{
s_sQLServerVersion ??= GetSqlServerVersion(TCPConnectionString);
}
return s_sQLServerVersion;
}
}

// Is TDS8 supported
public static bool IsTDS8Supported
{
get
{
if (!string.IsNullOrEmpty(TCPConnectionString))
{
s_isTDS8Supported = GetSQLServerStatusOnTDS8(TCPConnectionString);
}
return s_isTDS8Supported;
}
}

static DataTestUtility()
{
Config c = Config.Load();
Expand Down Expand Up @@ -237,6 +267,41 @@ private static string GenerateAccessToken(string authorityURL, string aADAuthUse

public static bool IsKerberosTest => !string.IsNullOrEmpty(KerberosDomainUser) && !string.IsNullOrEmpty(KerberosDomainPassword);

public static string GetSqlServerVersion(string connectionString)
{
string version = string.Empty;
using SqlConnection conn = new(connectionString);
conn.Open();
SqlCommand command = conn.CreateCommand();
command.CommandText = "SELECT SERVERProperty('ProductMajorVersion')";
SqlDataReader reader = command.ExecuteReader();
if (reader.Read())
{
version = reader.GetString(0);
}
return version;
}

public static bool GetSQLServerStatusOnTDS8(string connectionString)
{
bool isTDS8Supported = false;
SqlConnectionStringBuilder builder = new(connectionString)
{
[nameof(SqlConnectionStringBuilder.Encrypt)] = SqlConnectionEncryptOption.Strict
};
try
{
SqlConnection conn = new(builder.ConnectionString);
conn.Open();
isTDS8Supported = true;
}
catch (SqlException)
{

}
JRahnama marked this conversation as resolved.
Show resolved Hide resolved
return isTDS8Supported;
}

public static bool IsDatabasePresent(string name)
{
AvailableDatabases = AvailableDatabases ?? new Dictionary<string, bool>();
Expand All @@ -257,6 +322,17 @@ public static bool IsDatabasePresent(string name)
return present;
}

public static bool IsAdmin
{
get
{
#if NET6_0_OR_GREATER
System.Diagnostics.Debug.Assert(OperatingSystem.IsWindows());
#endif
return new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(WindowsBuiltInRole.Administrator);
}
}

/// <summary>
/// Checks if object SYS.SENSITIVITY_CLASSIFICATIONS exists in SQL Server
/// </summary>
Expand Down Expand Up @@ -302,6 +378,12 @@ public static bool AreConnStringsSetup()
return !string.IsNullOrEmpty(NPConnectionString) && !string.IsNullOrEmpty(TCPConnectionString);
}

public static bool IsSQL2022() => string.Equals("16", SQLServerVersion.Trim());

public static bool IsSQL2019() => string.Equals("15", SQLServerVersion.Trim());

public static bool IsSQL2016() => string.Equals("14", s_sQLServerVersion.Trim());

public static bool IsSQLAliasSetup()
{
return !string.IsNullOrEmpty(AliasName);
Expand Down Expand Up @@ -885,7 +967,7 @@ public static bool ParseDataSource(string dataSource, out string hostname, out i

if (dataSource.Contains(","))
{
if (!Int32.TryParse(dataSource.Substring(dataSource.LastIndexOf(",",StringComparison.Ordinal) + 1), out port))
if (!Int32.TryParse(dataSource.Substring(dataSource.LastIndexOf(",", StringComparison.Ordinal) + 1), out port))
{
return false;
}
Expand Down