From 8c411aa351fd73841d945496a57ade039e6fa50d Mon Sep 17 00:00:00 2001 From: JRahnama Date: Mon, 21 Aug 2023 14:20:49 -0700 Subject: [PATCH 1/4] Addressing review comments --- .../Microsoft/Data/SqlClient/SNI/SNICommon.cs | 122 +++++++-- .../src/Microsoft/Data/Common/AdapterUtil.cs | 12 +- .../src/Resources/Strings.Designer.cs | 27 ++ .../src/Resources/Strings.resx | 9 + .../ManualTests/DataCommon/DataTestUtility.cs | 85 +++++- ....Data.SqlClient.ManualTesting.Tests.csproj | 6 + .../CertificateTest.cs | 258 ++++++++++++++++++ .../GenerateSelfSignedCertificate.ps1 | 125 +++++++++ 8 files changed, 623 insertions(+), 21 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs create mode 100644 src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs index b92746098f..eb47b7c592 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNICommon.cs @@ -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 { @@ -146,15 +149,56 @@ 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) { @@ -162,7 +206,7 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C 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 @@ -170,7 +214,7 @@ internal static bool ValidateSslServerCertificate(string targetServerName, X509C 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 @@ -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; } } - + /// /// 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] @@ -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; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index 08e46ed284..6665a2cde6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -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; @@ -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); diff --git a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs index 067b3dea02..2823830e05 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.Designer.cs @@ -10206,6 +10206,33 @@ internal static string SQL_PrecisionValueOutOfRange { } } + /// + /// Looks up a localized string similar to Certificate failed chain validation. Error(s): '{0}'.. + /// + internal static string SQL_RemoteCertificateChainErrors { + get { + return ResourceManager.GetString("SQL_RemoteCertificateChainErrors", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.. + /// + internal static string SQL_RemoteCertificateNameMismatch { + get { + return ResourceManager.GetString("SQL_RemoteCertificateNameMismatch", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Certificate not available while validating the certificate.. + /// + internal static string SQL_RemoteCertificateNotAvailable { + get { + return ResourceManager.GetString("SQL_RemoteCertificateNotAvailable", resourceCulture); + } + } + /// /// Looks up a localized string similar to Scale value '{0}' is either less than 0 or greater than the maximum allowed scale of 38.. /// diff --git a/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx index e84af2f09d..78bfde6f9a 100644 --- a/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/src/Resources/Strings.resx @@ -4731,4 +4731,13 @@ When used by DataAdapter.Update, the parameter value is changed from DBNull.Value into (Int32)1 or (Int32)0 if non-null. + + Certificate failed chain validation. Error(s): '{0}'. + + + Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate. + + + Certificate not available while validating the certificate. + \ No newline at end of file diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 9cb15d312d..fa390ed8f5 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -20,6 +20,7 @@ using Xunit; using System.Net.NetworkInformation; using System.Text; +using System.Security.Principal; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { @@ -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(); @@ -235,8 +265,44 @@ private static Task AcquireTokenAsync(string authorityURL, string userID return result.AccessToken; }); + public static bool IsManagedSNI => UseManagedSNIOnWindows; 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) + { + + } + return isTDS8Supported; + } + public static bool IsDatabasePresent(string name) { AvailableDatabases = AvailableDatabases ?? new Dictionary(); @@ -257,6 +323,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); + } + } + /// /// Checks if object SYS.SENSITIVITY_CLASSIFICATIONS exists in SQL Server /// @@ -302,6 +379,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); @@ -885,7 +968,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; } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj index c7187391ac..baff074d3b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTesting.Tests.csproj @@ -274,6 +274,7 @@ + @@ -326,4 +327,9 @@ + + + Always + + diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs new file mode 100644 index 0000000000..970c088715 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -0,0 +1,258 @@ +using System; +using System.Collections.Generic; +using System.Data; +using System.Diagnostics; +using System.IO; +using System.Linq; +using System.Net; +using System.Net.Sockets; +using System.Security.Authentication; +using System.Security.Cryptography.X509Certificates; +using System.ServiceProcess; +using System.Text; +using Microsoft.Win32; +using Xunit; + +namespace Microsoft.Data.SqlClient.ManualTesting.Tests +{ + public class CertificateTest : IDisposable + { + #region Private Fields + private const string IPV4 = @"127.0.0.1"; + private const string IPV6 = @"::1"; + private static readonly string s_fullPathToPowershellScript = Path.Combine(Directory.GetCurrentDirectory(), "SQL", "ConnectionTestWithSSLCert", "GenerateSelfSignedCertificate.ps1"); + private const string LocalHost = "localhost"; + private static readonly string s_fQDN = Dns.GetHostEntry(Environment.MachineName).HostName; + private readonly string _thumbprint; + private const string ThumbPrintEnvName = "Thumbprint"; + + public string ForceEncryptionRegistryPath + { + get + { + if (DataTestUtility.IsSQL2022()) + { + return @"SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL16.MSSQLSERVER\MSSQLServer\SuperSocketNetLib"; + } + if (DataTestUtility.IsSQL2019()) + { + return @"SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL15.MSSQLSERVER\MSSQLServer\SuperSocketNetLib"; + } + if (DataTestUtility.IsSQL2016()) + { + return @"SOFTWARE\Microsoft\Microsoft SQL Server\MSSQL14.MSSQLSERVER\MSSQLServer\SuperSocketNetLib"; + } + return string.Empty; + } + } + #endregion + + public CertificateTest() + { + CreateValidCertificate(s_fullPathToPowershellScript); + _thumbprint = Environment.GetEnvironmentVariable(ThumbPrintEnvName, EnvironmentVariableTarget.Machine); + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [PlatformSpecific(TestPlatforms.Windows)] + public void OpenningConnectionWithGoodCertificateTest() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + + // confirm that ForceEncryption is enabled + using SqlConnection notEncryptedConnection = new(builder.ConnectionString); + builder.Encrypt = SqlConnectionEncryptOption.Optional; + notEncryptedConnection.Open(); + Assert.Equal(ConnectionState.Open, notEncryptedConnection.State); + + // Test with Mandatory encryption + builder.Encrypt = SqlConnectionEncryptOption.Mandatory; + using SqlConnection mandatoryConnection = new(builder.ConnectionString); + mandatoryConnection.Open(); + Assert.Equal(ConnectionState.Open, mandatoryConnection.State); + if (DataTestUtility.IsTDS8Supported) + { + // Test with strict encryption + builder.Encrypt = SqlConnectionEncryptOption.Strict; + using SqlConnection strictConnection = new(builder.ConnectionString); + strictConnection.Open(); + Assert.Equal(ConnectionState.Open, strictConnection.State); + } + } + + // Provided hostname in certificate are: + // localhost, FQDN, Loopback IPv4: 127.0.0.1, IPv6: ::1 + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [PlatformSpecific(TestPlatforms.Windows)] + public void OpeningConnectionWitHNICTest() + { + // Mandatory + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) + { + // 127.0.0.1 most of the cases does not cause any Remote certificate validation error depinding on name resolution on the machine + // It mostly returns SslPolicyErrors.None + DataSource = IPV4, + Encrypt = SqlConnectionEncryptOption.Mandatory, + HostNameInCertificate = "localhost" + }; + using SqlConnection connection = new(builder.ConnectionString); + connection.Open(); + Assert.Equal(ConnectionState.Open, connection.State); + + // Ipv6 however causes name mistmatch error + // In net6 Manged SNI does not check for SAN. Therefore Application using Net6 have to use FQDN as HNIC + // According to above no other hostname in certificate than FQDN will work in net6 which is same as SubjectName in case of RemoteCertificateNameMismatch + // Net7.0 the new API added by dotnet runtime will check SANS and then SubjectName + + builder.DataSource = IPV6; + builder.HostNameInCertificate = Dns.GetHostEntry(Environment.MachineName).HostName; + builder.Encrypt = SqlConnectionEncryptOption.Mandatory; + using SqlConnection connection2 = new(builder.ConnectionString); + connection2.Open(); + Assert.Equal(ConnectionState.Open, connection2.State); + + if (DataTestUtility.IsTDS8Supported) + { + // Strict + builder.DataSource = IPV6; + builder.HostNameInCertificate = Dns.GetHostEntry(Environment.MachineName).HostName; + builder.Encrypt = SqlConnectionEncryptOption.Strict; + using SqlConnection connection3 = new(builder.ConnectionString); + connection3.Open(); + Assert.Equal(ConnectionState.Open, connection3.State); + } + } + + [ActiveIssue(26934)] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsManagedSNI), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [PlatformSpecific(TestPlatforms.Windows)] + public void RemoteCertificateNameMismatchErrorTest() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) + { + DataSource = GetLocalIpAddress(), + Encrypt = SqlConnectionEncryptOption.Mandatory, + HostNameInCertificate = "BadHostName" + }; + using SqlConnection connection = new(builder.ConnectionString); + SqlException exception = Assert.Throws(() => connection.Open()); + Assert.StartsWith("A connection was successfully established with the server, but then an error occurred during the pre-login handshake. (provider: TCP Provider, error: 35 - An internal exception was caught)", exception.Message); + Assert.Equal(20, exception.Class); + Assert.IsType(exception.InnerException); + Assert.StartsWith("Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.", exception.InnerException.Message); + Console.WriteLine(exception.Message); + } + + private static void CreateValidCertificate(string script) + { + if (File.Exists(script)) + { + StringBuilder output = new(); + Process proc = new() + { + StartInfo = + { + FileName = "powershell.exe", + RedirectStandardError = true, + RedirectStandardOutput = true, + UseShellExecute = false, + Arguments = script, + CreateNoWindow = false, + Verb = "runas" + } + }; + + proc.EnableRaisingEvents = true; + + // Use async event handlers to avoid deadlocks + proc.OutputDataReceived += new DataReceivedEventHandler((sender, e) => + { + output.AppendLine(e.Data); + }); + + proc.ErrorDataReceived += new DataReceivedEventHandler((sender, e) => + { + output.AppendLine(e.Data); + }); + + proc.Start(); + proc.BeginOutputReadLine(); + proc.BeginErrorReadLine(); + Console.WriteLine(output); + if (!proc.WaitForExit(60000)) + { + proc.Kill(); + // allow async output to process + proc.WaitForExit(2000); + throw new Exception($"Could not generate certificate.Error out put: {output}"); + } + } + else + { + throw new Exception($"Could not find GenerateSelfSignedCertificate.ps1"); + } + } + + private static string GetLocalIpAddress() + { + string hostname = Dns.GetHostEntry(Environment.MachineName).HostName; + IPHostEntry iphostentry = Dns.GetHostEntry(hostname); + List ipaddress = iphostentry.AddressList.Where(x => x.AddressFamily == AddressFamily.InterNetworkV6).ToList(); + foreach (IPAddress ip in ipaddress) + { + Console.WriteLine($"{ip}"); + if (ip != IPAddress.IPv6Loopback) + { + Console.WriteLine(ip); + return ip.ToString(); + } + } + return null; + } + + private void RemoveCertificate() + { + using X509Store certStore = new(StoreName.Root, StoreLocation.LocalMachine); + certStore.Open(OpenFlags.ReadWrite); + X509Certificate2Collection certCollection = certStore.Certificates.Find(X509FindType.FindByThumbprint, _thumbprint, false); + if (certCollection.Count > 0) + { + certStore.Remove(certCollection[0]); + } + certStore.Close(); + } + + private static void RemoveForceEncryptionFromRegistryPath(string registryPath) + { + RegistryKey key = Registry.LocalMachine.OpenSubKey(registryPath, true); + key?.SetValue("ForceEncryption", 0, RegistryValueKind.DWord); + key?.SetValue("Certificate", "", RegistryValueKind.String); + ServiceController sc = new("MSSQLSERVER"); + sc.Stop(); + sc.WaitForStatus(ServiceControllerStatus.Stopped); + sc.Start(); + sc.WaitForStatus(ServiceControllerStatus.Running); + } + + private static void RemoveEnvironmentVariable(string variableName) + { + Environment.SetEnvironmentVariable(variableName, null); + } + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (disposing && !string.IsNullOrEmpty(ForceEncryptionRegistryPath)) + { + RemoveCertificate(); + RemoveForceEncryptionFromRegistryPath(ForceEncryptionRegistryPath); + RemoveEnvironmentVariable(ThumbPrintEnvName); + } + } + } +} diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 new file mode 100644 index 0000000000..3726876731 --- /dev/null +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 @@ -0,0 +1,125 @@ +# 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. +# Script: Invoke-SqlServerCertificateCommand# Author: SqlClient Team +# Date: July 17, 2023 +# Comments: This scripts creates SQL Server SSL Self-Signed Certificate. +# This script is not intended to be used in production environments. + +function Invoke-SqlServerCertificateCommand { + [CmdletBinding()] + param( + [Parameter(Mandatory = $false)] + [string] $certificateFolder = "C:\Certificates", + [string] $certificateName = "SQLClientSelfSignedCertificate.cer", + [string] $myCertStoreLocation = "Cert:\LocalMachine\My", + [string] $rootCertStoreLocation = "Cert:\LocalMachine\Root", + [string] $sqlServerUserAccount = "NT Service\MSSQLSERVER", + [string] $sqlAliasName = "SQLAliasName", + [string] $localhost = "localhost", + [string] $LoopBackIPV4 = "127.0.0.1", + [string] $LoopBackIPV6 = "::1" + ) + try { + Write-Host Certificate folder path is: $certificateFolder + if (Test-Path -Path $certificateFolder) { + Write-Host "Certificate folder already exists" + Remove-Item -Path $certificateFolder -Force -Recurse + } + # Create a local folder to store the certificates + New-Item -Path $certificateFolder -ItemType Directory + + # Get FQDN of the machine + $fqdn = [System.Net.Dns]::GetHostByName(($env:computerName)).HostName + + # Create a self-signed certificate + $params = @{ + Type = "SSLServerAuthentication" + Subject = "CN=$fqdn" + KeyAlgorithm = "RSA" + KeyLength = 2048 + HashAlgorithm = "SHA256" + TextExtension = "2.5.29.37={text}1.3.6.1.5.5.7.3.1", "2.5.29.17={text}DNS=$fqdn&DNS=$localhost&IPAddress=$LoopBackIPV4&DNS=$sqlAliasName&IPAddress=$LoopBackIPV6" + NotAfter = (Get-Date).AddMonths(36) + KeySpec = "KeyExchange" + Provider = "Microsoft RSA SChannel Cryptographic Provider" + CertStoreLocation = $myCertStoreLocation + } + + $certificate = New-SelfSignedCertificate @params + Write-Host "Certificate created successfully" + Write-Host "Certificate Thumbprint: $($certificate.Thumbprint)" + + [System.Environment]::SetEnvironmentVariable('Thumbprint', "$($certificate.Thumbprint)", [System.EnvironmentVariableTarget]::Machine) + + # Providing Read Access to the certificate for SQL Server Service Account + + $privateKey = [System.Security.Cryptography.X509Certificates.RSACertificateExtensions]::GetRSAPrivateKey($certificate) + $containerName = "" + $identity = $sqlServerUserAccount + $fileSystemRights = [System.Security.AccessControl.FileSystemRights]::Read + $type = [System.Security.AccessControl.AccessControlType]::Allow + $fileSystemAccessRuleArgumentList = @($identity, $fileSystemRights, $type) + if ($privateKey -is [System.Security.Cryptography.RSACng]) { + $containerName = $privateKey.Key.UniqueName + } + else { + $containerName = $privateKey.CspKeyContainerInfo.UniqueKeyContainerName + } + $fullPath = "C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys\$containerName" + + if (-Not(Test-Path -Path $fullPath -PathType Leaf)) { + Write-Host "Certificate private key file not found at $fullPath" + throw "Certificate private key file not found at $fullPath" + } + + # Get Current ACL of the private key + $acl = Get-Acl -Path $fullPath + + # Create a new Access Rule for the SQL Server Service Account + $accessRule = New-Object System.Security.AccessControl.FileSystemAccessRule -ArgumentList $fileSystemAccessRuleArgumentList + $acl.AddAccessRule($accessRule) + + # Set the new ACL + Set-Acl -Path $fullPath -AclObject $acl + + # Export the certificate to a file + Export-Certificate -Cert $certificate -FilePath "$certificateFolder\$certificateName" -Type CERT + + # Import the certificate to the Root store + $params = @{ + FilePath = "$certificateFolder\$certificateName" + CertStoreLocation = $rootCertStoreLocation + } + Import-Certificate @params + + # Import certificate to SQL Server + $path = Get-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\*\MSSQLServer\SuperSocketNetLib" + Set-ItemProperty -Path $path.PsPath -Name "Certificate" -Type String -Value "$($certificate.Thumbprint)" + + # Set Force Encryption to Yes + $path = Get-ItemProperty -Path "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\*\MSSQLServer\SuperSocketNetLib" + Set-ItemProperty -Path $path.PsPath -Name "ForceEncryption" -Type DWord -value 1 + + # Restart SQL Server Service + Restart-Service -Name "MSSQLSERVER" -Force + Start-Sleep 10 + Start-Service SQLSERVERAGENT + + # Print out SQL Service status + $service = Get-Service -Name "MSSQLSERVER" + Write-Host "SQL Server Service Status: $($service.Status)" + Write-Host "Self-Signed Certificate created successfully" + } + catch { + $e = $_.Exception + $msg = $e.Message + while ($e.InnerException) { + $e = $e.InnerException + $msg += "`n" + $e.Message + } + Write-Host "Certificate generation was not successfull. $msg" + } +} + +Invoke-SqlServerCertificateCommand From a77baad45024ccec86aea6795f3d5887801621d6 Mon Sep 17 00:00:00 2001 From: JRahnama Date: Mon, 21 Aug 2023 14:27:28 -0700 Subject: [PATCH 2/4] Adding License to test class. --- .../SQL/ConnectionTestWithSSLCert/CertificateTest.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs index 970c088715..e0725190ec 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -1,4 +1,7 @@ -using System; +// 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. +using System; using System.Collections.Generic; using System.Data; using System.Diagnostics; From f77f952e338ef0c3cdc74b06279e742f1c1812a8 Mon Sep 17 00:00:00 2001 From: JRahnama Date: Fri, 1 Sep 2023 15:10:48 -0700 Subject: [PATCH 3/4] Revierw comments --- .../ManualTests/DataCommon/DataTestUtility.cs | 1 - .../CertificateTest.cs | 17 ++++++++++++----- .../GenerateSelfSignedCertificate.ps1 | 3 ++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index fa390ed8f5..ff8c2037d0 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -265,7 +265,6 @@ private static Task AcquireTokenAsync(string authorityURL, string userID return result.AccessToken; }); - public static bool IsManagedSNI => UseManagedSNIOnWindows; public static bool IsKerberosTest => !string.IsNullOrEmpty(KerberosDomainUser) && !string.IsNullOrEmpty(KerberosDomainPassword); public static string GetSqlServerVersion(string connectionString) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs index e0725190ec..58dadbea4c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -52,11 +52,18 @@ public string ForceEncryptionRegistryPath public CertificateTest() { - CreateValidCertificate(s_fullPathToPowershellScript); - _thumbprint = Environment.GetEnvironmentVariable(ThumbPrintEnvName, EnvironmentVariableTarget.Machine); + if (DataTestUtility.IsAdmin) + { + CreateValidCertificate(s_fullPathToPowershellScript); + _thumbprint = Environment.GetEnvironmentVariable(ThumbPrintEnvName, EnvironmentVariableTarget.Machine); + } + else + { + Assert.False(true, "CertificateTest class needs to be run in Admin mode."); + } } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public void OpenningConnectionWithGoodCertificateTest() { @@ -85,7 +92,7 @@ public void OpenningConnectionWithGoodCertificateTest() // Provided hostname in certificate are: // localhost, FQDN, Loopback IPv4: 127.0.0.1, IPv6: ::1 - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public void OpeningConnectionWitHNICTest() { @@ -127,7 +134,7 @@ public void OpeningConnectionWitHNICTest() } [ActiveIssue(26934)] - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsManagedSNI), nameof(DataTestUtility.IsAdmin), nameof(DataTestUtility.IsNotAzureServer))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.UseManagedSNIOnWindows), nameof(DataTestUtility.IsNotAzureServer))] [PlatformSpecific(TestPlatforms.Windows)] public void RemoteCertificateNameMismatchErrorTest() { diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 index 3726876731..96f0c2b355 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/GenerateSelfSignedCertificate.ps1 @@ -1,7 +1,8 @@ # 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. -# Script: Invoke-SqlServerCertificateCommand# Author: SqlClient Team +# Script: Invoke-SqlServerCertificateCommand# +# Author: SqlClient Team # Date: July 17, 2023 # Comments: This scripts creates SQL Server SSL Self-Signed Certificate. # This script is not intended to be used in production environments. From b73097dc890914dd5059e2dc6041fc04254eff86 Mon Sep 17 00:00:00 2001 From: Javad Date: Fri, 1 Sep 2023 17:26:52 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: DavoudEshtehari <61173489+DavoudEshtehari@users.noreply.github.com> --- .../ConnectionTestWithSSLCert/CertificateTest.cs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs index 58dadbea4c..64df0c0677 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -52,15 +52,10 @@ public string ForceEncryptionRegistryPath public CertificateTest() { - if (DataTestUtility.IsAdmin) - { - CreateValidCertificate(s_fullPathToPowershellScript); - _thumbprint = Environment.GetEnvironmentVariable(ThumbPrintEnvName, EnvironmentVariableTarget.Machine); - } - else - { - Assert.False(true, "CertificateTest class needs to be run in Admin mode."); - } + Assert.True(DataTestUtility.IsAdmin, "CertificateTest class needs to be run in Admin mode."); + + CreateValidCertificate(s_fullPathToPowershellScript); + _thumbprint = Environment.GetEnvironmentVariable(ThumbPrintEnvName, EnvironmentVariableTarget.Machine); } [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]