From b8e4102440a92ca36f2f718c592683c5dd9ac52c Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 13:57:50 +0100 Subject: [PATCH 01/17] Fix RemoteCertificateNameMismatchErrorTest --- .../CertificateTest.cs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 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 d500391227..f5a9a804d5 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -9,6 +9,7 @@ using System.Linq; using System.Net; using System.Net.Sockets; +using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; using System.ServiceProcess; @@ -90,7 +91,7 @@ private static bool IsLocalHost() private static bool AreConnStringsSetup() => DataTestUtility.AreConnStringsSetup(); private static bool IsNotAzureServer() => DataTestUtility.IsNotAzureServer(); - private static bool UseManagedSNIOnWindows() => DataTestUtility.UseManagedSNIOnWindows; + private static bool IsAdminOnWindows() => DataTestUtility.IsAdminOnWindows; [ActiveIssue("31754")] [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] @@ -166,9 +167,7 @@ public void OpeningConnectionWitHNICTest() } } - [ActiveIssue("31754")] - [ConditionalFact(nameof(AreConnStringsSetup), nameof(UseManagedSNIOnWindows), nameof(IsNotAzureServer), nameof(IsLocalHost))] - [PlatformSpecific(TestPlatforms.Windows)] + [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] public void RemoteCertificateNameMismatchErrorTest() { SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) @@ -179,11 +178,20 @@ public void RemoteCertificateNameMismatchErrorTest() }; 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); + + if (DataTestUtility.IsUsingNativeSNI()) + { + Assert.StartsWith("A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - The certificate's CN name does not match the passed value.)", exception.Message); + Assert.IsType(exception.InnerException); + Assert.StartsWith("The certificate's CN name does not match the passed value", exception.InnerException.Message); + } + else + { + 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.IsType(exception.InnerException); + Assert.StartsWith("Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.", exception.InnerException.Message); + } } private static void CreateValidCertificate(string script) From d52b55d5abf6ca9d695597964b6eb65dfe51bf92 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 14:23:38 +0100 Subject: [PATCH 02/17] Improve test --- .../ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs | 1 - 1 file changed, 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 f5a9a804d5..b11b4904e3 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -91,7 +91,6 @@ private static bool IsLocalHost() private static bool AreConnStringsSetup() => DataTestUtility.AreConnStringsSetup(); private static bool IsNotAzureServer() => DataTestUtility.IsNotAzureServer(); - private static bool IsAdminOnWindows() => DataTestUtility.IsAdminOnWindows; [ActiveIssue("31754")] [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] From 14da8d7101cfe40c59e44363674631e4a5bcde41 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 15:27:23 +0100 Subject: [PATCH 03/17] Try with TrustServerCertificate = false --- .../ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs | 1 + 1 file changed, 1 insertion(+) 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 b11b4904e3..ff0ba5dd7c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -173,6 +173,7 @@ public void RemoteCertificateNameMismatchErrorTest() { DataSource = GetLocalIpAddress(), Encrypt = SqlConnectionEncryptOption.Mandatory, + TrustServerCertificate = false, HostNameInCertificate = "BadHostName" }; using SqlConnection connection = new(builder.ConnectionString); From a9d80d988afde73806a982ee200bce3111d1a168 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 16:16:57 +0100 Subject: [PATCH 04/17] Add logging --- .../templates/steps/run-all-tests-step.yml | 2 ++ eng/pipelines/dotnet-sqlclient-ci-core.yml | 6 ++-- ...qlclient-ci-package-reference-pipeline.yml | 31 ------------------- .../CertificateTest.cs | 12 ++++++- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/eng/pipelines/common/templates/steps/run-all-tests-step.yml b/eng/pipelines/common/templates/steps/run-all-tests-step.yml index f0eb2eff66..4f038a18ca 100644 --- a/eng/pipelines/common/templates/steps/run-all-tests-step.yml +++ b/eng/pipelines/common/templates/steps/run-all-tests-step.yml @@ -55,6 +55,7 @@ steps: - ${{if eq(parameters.operatingSystem, 'Windows')}}: - task: MSBuild@1 displayName: 'Run Functional Tests ${{parameters.msbuildArchitecture }}' + enabled: false inputs: solution: build.proj msbuildArchitecture: ${{parameters.msbuildArchitecture }} @@ -84,6 +85,7 @@ steps: - ${{ else }}: # Linux or macOS - task: DotNetCoreCLI@2 displayName: 'Run Functional Tests' + enabled: false inputs: command: custom projects: build.proj diff --git a/eng/pipelines/dotnet-sqlclient-ci-core.yml b/eng/pipelines/dotnet-sqlclient-ci-core.yml index 41a2a25416..66432123db 100644 --- a/eng/pipelines/dotnet-sqlclient-ci-core.yml +++ b/eng/pipelines/dotnet-sqlclient-ci-core.yml @@ -12,12 +12,12 @@ parameters: - name: targetFrameworks displayName: 'Target Frameworks on Windows' type: object - default: [net462, net8.0, net9.0] + default: [net462, net8.0] - name: targetFrameworksLinux displayName: 'Target Frameworks on Non-Windows' type: object - default: [net8.0, net9.0] + default: [net8.0] - name: netcoreVersionTestUtils displayName: 'Netcore Version for Test Utilities' @@ -32,7 +32,7 @@ parameters: - name: testSets displayName: 'Test Sets' type: object - default: [1, 2, 3] + default: [2] - name: useManagedSNI displayName: | diff --git a/eng/pipelines/dotnet-sqlclient-ci-package-reference-pipeline.yml b/eng/pipelines/dotnet-sqlclient-ci-package-reference-pipeline.yml index 4956b15c89..9bd73fb9cc 100644 --- a/eng/pipelines/dotnet-sqlclient-ci-package-reference-pipeline.yml +++ b/eng/pipelines/dotnet-sqlclient-ci-package-reference-pipeline.yml @@ -5,37 +5,6 @@ ################################################################################# name: $(DayOfYear)$(Rev:rr) -trigger: - batch: true - branches: - include: - - main - - internal/main - paths: - include: - - src\Microsoft.Data.SqlClient\netcore\ref - - src\Microsoft.Data.SqlClient\netfx\ref - - src\Microsoft.Data.SqlClient\ref - - eng - - tools - - .config - - Nuget.config - -schedules: -- cron: '0 4 * * Fri' - displayName: Weekly Thursday 9:00 PM (UTC - 7) Build - branches: - include: - - internal/main - always: true - -- cron: '0 0 * * Mon-Fri' - displayName: Daily build 5:00 PM (UTC - 7) Build - branches: - include: - - main - always: true - parameters: # parameters are shown up in ADO UI in a build queue time - name: 'debug' displayName: 'Enable debug output' 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 ff0ba5dd7c..137b281da0 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -16,6 +16,7 @@ using System.Text; using Microsoft.Win32; using Xunit; +using Xunit.Abstractions; namespace Microsoft.Data.SqlClient.ManualTesting.Tests { @@ -39,6 +40,8 @@ public class CertificateTest : IDisposable // SlashInstance is used to override IPV4 and IPV6 defined about so it includes an instance name private static string SlashInstanceName = ""; + private readonly ITestOutputHelper _testOutputHelper; + private static string ForceEncryptionRegistryPath { get @@ -60,8 +63,9 @@ private static string ForceEncryptionRegistryPath } #endregion - public CertificateTest() + public CertificateTest(ITestOutputHelper testOutputHelper) { + _testOutputHelper = testOutputHelper; SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); if (!LocalHost.Equals(hostname, StringComparison.OrdinalIgnoreCase)) @@ -178,6 +182,12 @@ public void RemoteCertificateNameMismatchErrorTest() }; using SqlConnection connection = new(builder.ConnectionString); SqlException exception = Assert.Throws(() => connection.Open()); + + _testOutputHelper.WriteLine("Actual exception:"); + _testOutputHelper.WriteLine(exception.ToString()); + + _testOutputHelper.WriteLine("Actual inner exception:"); + _testOutputHelper.WriteLine(exception.InnerException?.ToString() ?? "None"); Assert.Equal(20, exception.Class); if (DataTestUtility.IsUsingNativeSNI()) From 2858d3ca3f4455817ea81844a10122774778282e Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 17:09:02 +0100 Subject: [PATCH 05/17] See if we can trust the SQL Certificate on Windows --- .../steps/configure-sql-server-win-step.yml | 380 ++++++++++++++++++ 1 file changed, 380 insertions(+) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index f0c27e6152..3c8f1a7a3a 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -248,3 +248,383 @@ steps: sqlcmd -S "(localdb)\.\${{parameters.LocalDbSharedInstanceName }}" -q "SELECT @@VERSION" displayName: 'Enable LocalDB [Win]' condition: ${{parameters.condition }} + +- powershell: | + # Script Source: https://gist.github.com/jborean93/44f92e4dfa613c5a1e7889fa7a7c2563 + # Copyright: (c) 2023, Jordan Borean (@jborean93) + # MIT License (see LICENSE or https://opensource.org/licenses/MIT) + + Function Get-SqlServerTlsCertificate { + <# + .SYNOPSIS + Gets the MS SQL X509 Certificate. + .DESCRIPTION + Gets the X509 Certificate that is being used by a remote MS SQL Server. + This certificate contains information like the Subject, SAN entries, expiry and other useful information for debugging purposes. + .PARAMETER ComputerName + The remote MS SQL Server to extract the certificate from. + .PARAMETER ConnectTimeout + The timeout, in milliseconds, to wait until the connection was successful, defaults to 5000 (5 seconds). + If the timeout is reached, the cmdlet will write an error. + .PARAMETER ConnectionType + The connection type to use for retrieving the certificate, defaults to SQLBrowser. + This can be set to SQLBrowser, NamedPipe, or TCP. + The SQLBrowser option will use the SQL Browser service to find the named pipe or TCP port for the instance requested. + The SQLBrowser needs access to the UDP port 1434 as well as the NamedPipe or TCP Port that is selected to work. + The NamedPipe option will connect to the named pipe for the instance requested. + The NamedPipe will need access to the TCP port 445 to work. + The TCP option will connect to the TCP port requested. + .PARAMETER InstanceName + The MS SQL instance to connect to. + When used with '-ConnectionType SQLBrowser', it will only connect to the instance that matches this name. + When used with '-ConnectionType NamedPipe', it will use this instance name to build the named pipe name. + Set to an empty string to use the first instance found by the SQLBrowser. + .PARAMETER Port + The TCP port to use for the connection, defaults to 1433. + This is only used if '-ConnectionType TCP' is requested. + .PARAMETER StrictEncrypt + Perform strict encryption that was introduced with TDS 8.0 (SQL Server 2022 and newer). + Strict encryption simplifies the connection process but will only work if the server is new enough to support it. + .EXAMPLE + PS> Get-SqlServerTlsCertificate -ComputerName sql01 + Gets the certificate of the first instance found on sql01 using the SQL Browser service to find the TCP port or Named Pipe. + .EXAMPLE + PS> Get-SqlServerTlsCertificate -ComputerName sql01 -Instance MySQLInstance + Gets the certificate for the instance 'sql01\MySQLInstance' using the SQL Browser service to find the TCP port or Named Pipe. + .EXAMPLE + PS> Get-SqlServerTlSCertificate -ComputerName sql01 -Port 65334 -ConnectionType TCP + Gets the certificate for server sql01 using the TCP port 65334 + .EXAMPLE + PS> Get-SqlServerTlsCertificate -ComputerName sql01 -ConnectionType NamedPipe + Gets the certificate for the default instance on sql01 using the Named Pipe connection. + .EXAMPLE + PS> Get-SqlServerTlsCertificate -ComputerName sql01 -InstanceName MySQLInstance -ConnectionType NamedPipe + Gets the certificate for the instance 'sql01\MySQLInstance' using the Named Pipe connection. + .EXAMPLE + PS> $cert = Get-SqlServerTlsCertificate -ComputerName sql01 + PS> $certBytes = $cert.Export("Cert") + PS> $setParams = @{} + PS> if ($PSVersionTable.PSVersion -lt [Version]'6.0') { + ... $setParams.Raw = $true + ... } else { + ... $setParams.AsByteStream = $true + ... } + PS> Set-Content -Path sql01.crt -Value $certBytes @setParams + Gets the certificate for the SQL server sql01 and exports it to a .crt file for use in Windows. + .OUTPUTS + System.Security.Cryptography.X509Certificates.X509Certificate2 + This cmdlet will output the X509Certificate2 object retrieved from the server. + .NOTES + Run with -Verbose to get a better understanding of how this cmdlet connects to the MS SQL server. + A warning will be emitted if the remote certificate is not trusted and it will try to include the reasons why. + #> + [OutputType([System.Security.Cryptography.X509Certificates.X509Certificate2])] + param ( + [Parameter(Mandatory)] + [string] + $ComputerName, + + [Parameter()] + [int] + $ConnectTimeout = 5000, + + [Parameter()] + [ValidateSet("SQLBrowser", "TCP", "NamedPipe")] + [string] + $ConnectionType = "SQLBrowser", + + [Parameter()] + [AllowEmptyString()] + [string] + $InstanceName = "", + + [Parameter()] + [int] + $Port = 1433, + + [Parameter()] + [switch] + $StrictEncrypt + ) + + class TdsTlsStream : System.IO.Stream { + [System.IO.Stream]$InnerStream + [int]$PayloadLength = 0 + + TdsTlsStream([System.IO.Stream]$InnerStream) { + $this.InnerStream = $InnerStream + } + + [bool] get_CanRead() { return $this.InnerStream.CanRead } + [bool] get_CanWrite() { return $this.InnerStream.CanWrite } + [bool] get_CanSeek() { return $this.InnerStream.CanSeek } + [Int64] get_Length() { return $this.InnerStream.Length } + [Int64] get_Position() { return $this.InnerStream.Position } + [void] set_Position([Int64]$Value) { $this.InnerStream.Position = $Value } + [int] get_ReadTimeout() { return $this.InnerStream.ReadTimeout } + [int] get_WriteTimeout() { return $this.InnerStream.WriteTimeout } + + [void] Flush() { $this.InnerStream.Flush() } + [Int64] Seek([Int64]$Offset, [System.IO.SeekOrigin]$Origin) { return $this.InnerStream.Seek($Offset, $Origin) } + [void] SetLength([Int64]$Value) { $this.InnerStream.SetLength($Value) } + + [int] Read([byte[]]$Buffer, [int]$Offset, [int]$Count) { + # We need to strip off the TDS header before setting the Buffer + if ($this.PayloadLength -eq 0) { + $header = [byte[]]::new(8) + $read = 0 + while ($read -lt 8) { + $read += $this.InnerStream.Read($header, 0, 8) + } + + $lengthBeforeHeader = [System.BitConverter]::ToUInt16([byte[]]@($header[3], $header[2]), 0) + $lengthBeforeHeader -= 8 + $this.PayloadLength = $lengthBeforeHeader + } + + if ($Count -gt $this.PayloadLength) { + $Count = $this.PayloadLength + } + $read = $this.InnerStream.Read($Buffer, $Offset, $Count) + $this.PayloadLength -= $read + return $read + } + + [void] Write([byte[]]$Buffer, [int]$Offset, [int]$Count) { + $newPayload = $this.GenerateTdsHeader($Buffer, $Offset, $Count) + $this.InnerStream.Write($newPayload, 0, $newPayload.Length) + } + + [byte[]] GenerateTdsHeader([byte[]]$Payload, [int]$Offset, [int]$Count) { + # The length is big endian encoded so it is inserted in reverse order + $lengthBytes = [System.BitConverter]::GetBytes([uint16]($Count + 8)) + + $newPayload = [byte[]]::new(8 + $Count) + $newPayload[0] = 0x12 # Type - Pre-Login + $newPayload[1] = 0x01 # Status - End of message (EOM) + $newPayload[2] = $lengthBytes[1] + $newPayload[3] = $lengthBytes[0] + $newPayload[4] = 0 # SPID + $newPayload[5] = 0 # SPID + $newPayload[6] = 0 # PacketID + $newPayload[7] = 0 # Window + [System.Array]::Copy($Payload, $Offset, $newPayload, 8, $Count) + + return $newPayload + } + } + + $udpClient = $socket = $targetStream = $sslStream = $null + try { + $pipeName = if ($InstanceName -and $InstanceName -ne 'MSSQLSERVER') { + 'MSSQL${0}\sql\query' -f $InstanceName + } + else { + 'sql\query' + } + + if ($ConnectionType -eq "SQLBrowser") { + # Use the SQLBrowser + # https://learn.microsoft.com/en-us/openspecs/windows_protocols/mc-sqlr/2e1560c9-5097-4023-9f5e-72b9ff1ec3b1 + $udpClient = [System.Net.Sockets.UdpClient]::new($ComputerName, 1434) + $udpClient.Client.SendTimeout = $ConnectTimeout + $udpClient.Client.ReceiveTimeout = $ConnectTimeout + $null = $udpClient.Send([byte[]]@(0x03), 1) # CLNT_UCAST_EX + $resp = $udpClient.Receive([ref]$null) + + $respSize = [System.BitConverter]::ToUInt16($resp, 1) + $rawResponse = [System.Text.Encoding]::UTF8.GetString($resp, 3, $respSize) + Write-Verbose -Message "Recieved SQL Browser response: '$rawResponse'" + $response = $rawResponse -split ';' + + $instanceInfo = [Ordered]@{} + $remoteInstance = @( + for ($i = 0; $i -lt $response.Length; $i += 2) { + if ($response[$i]) { + $instanceInfo[$response[$i]] = $response[$i + 1] + } + elseif ($i -eq $response.Length - 1) { + break + } + else { + $info = [PSCustomObject]$instanceInfo + Write-Verbose -Message "Processed SQL Browser Response:`n$($info | Out-String)" + + $info + $instanceInfo = [Ordered]@{} + $i -= 1 + } + } + ) | Where-Object { -not $InstanceName -or $_.InstanceName -eq $InstanceName } | Select-Object -First 1 + + if ($remoteInstance.np) { + $ConnectionType = 'NamedPipe' + $ComputerName = $remoteInstance.ServerName + $pipeName = $remoteInstance.np -replace "\\\\.*?\\pipe\\(.*)", '$1' + } + elseif ($remoteInstance.tcp) { + $ConnectionType = 'TCP' + $ComputerName = $remoteInstance.ServerName + $Port = $remoteInstance.tcp + } + else { + throw "Failed to receive any SQL Browser responses from $($ComputerName):1434, cannot continue" + } + } + + if ($ConnectionType -eq "TCP") { + Write-Verbose -Message "Connecting to TCP/IP endpoint $($ComputerName):$Port" + + $socket = [System.Net.Sockets.TcpClient]::new() + $connectTask = $socket.ConnectAsync($ComputerName, $Port) + if (-not $connectTask.Wait($ConnectTimeout)) { + throw "Timed out connecting to TCP/IP endpoint $($ComputerName):$Port" + } + + $null = $connectTask.GetAwaiter().GetResult() + $targetStream = $socket.GetStream() + } + else { + Write-Verbose -Message "Connecting to Named Pipe endpoint \\$($ComputerName)\pipe\$pipeName" + $targetStream = [System.IO.Pipes.NamedPipeClientStream]::new( + $ComputerName, + $pipeName, + [System.IO.Pipes.PipeDirection]::InOut) + $targetStream.Connect($ConnectTimeout) + } + + # Before TDS 8.0, TLS was done after the Pre-Login message after it was + # negotiated with the server. It also needs to prepend a header to each TLS + # payload making it more difficult. TDS 8.0 (Encrypt=strict) is a lot + # simpler as the TLS handshake is done before anything. + if ($StrictEncrypt) { + Write-Verbose -Message "Using TDS 8 TLS Handshake" + $streamToWrap = $targetStream + } + else { + Write-Verbose -Message "Using TDS 7.x Pre-Login method for the TLS handshake" + + # This is a pre-calculated TDS Pre-Login payload with the ENCRYPTION + # value of ENCRYPT_REQ (0x03). + # https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/60f56408-0188-4cd5-8b90-25c6f2423868 + $tdsPreLogin = [byte[]]@( + 0x12, 0x01, 0x00, 0x2f, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x1a, 0x00, 0x06, 0x01, 0x00, 0x20, + 0x00, 0x01, 0x02, 0x00, 0x21, 0x00, 0x01, 0x03, + 0x00, 0x22, 0x00, 0x04, 0x04, 0x00, 0x26, 0x00, + 0x01, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 + ) + $targetStream.Write($tdsPreLogin, 0, $tdsPreLogin.Count) + + $headerBytes = [byte[]]::new(8) + $read = 0 + while ($read -ne $headerBytes.Length) { + $read += $targetStream.Read($headerBytes, $read, $headerBytes.Length - $read) + } + + # Integer values are big endian encoded so swap them around. It also + # includes the header length which we've already gotten + $payloadLength = [System.BitConverter]::ToUInt16([byte[]]@($headerBytes[3], $headerBytes[2]), 0) + $payloadLength -= 8 + + $tdsPreLoginResp = [byte[]]::new($payloadLength) + $read = 0 + while ($read -ne $tdsPreLoginResp.Length) { + $read += $targetStream.Read($tdsPreLoginResp, $read, $tdsPreLoginResp.Length - $read) + } + + # The TDS Pre-Login payload starts with a variable amount of headers + # TYPE - BYTE + # OFFSET - USHORT (offset in the payload of the value) + # LENGTH - USHORT + # The headers are terminated with the type 0xFF. We want to extract the + # value for the ENCRYPT type (1) from the payload to see if the server + # supported encryption. + $serverEncrypt = 0 + $offset = 0 + while ($true) { + $plOptionType = $tdsPreLoginResp[$offset] + if ($plOptionType -eq 0xFF) { + break + } + elseif ($plOptionType -ne 1) { + $offset += 5 + continue + } + + $valueOffset = [System.BitConverter]::ToUInt16([byte[]]@($tdsPreLoginResp[$offset + 2], $tdsPreLoginResp[$offset + 1]), 0) + $serverEncrypt = $tdsPreLoginResp[$valueOffset] + break + } + + # Strip off the extra flags, we only care about these specific bits + $serverEncrypt = $serverEncrypt -band 0x0F + + # ENCRYPT_OFF, ENCRYPT_NOT_SUP + if ($serverEncrypt -in @(0, 2)) { + $msg = 'Server reported an encryption level of 0x{0:X2} which indicates it does not support TDS encryption.' -f $serverEncrypt + throw $msg + } + + # Now we know the server supports TLS we need to wrap the raw stream + # with a custom wrapper to ensure each TLS payload sent below is + # preceeded with the TDS header as required. While not implemented + # there is a note that TDS 7.1 or earlier (SQL Server 2000 or earlier) + # should use the table response type (0x04) instead. As this is so old + # I'm not going to implement that. + $streamToWrap = [TdsTlsStream]::new($targetStream) + } + + # Create the SslStream with a disable certificate verification callback. + # This allows it to connect to a self signed or cert with different + # hostname. The callback will also capture more information about the peer + # Allows us to emit warnings if it was going to fail. + $certState = @{} + $sslStream = [System.Net.Security.SslStream]::new($streamToWrap, $false, { + param($Sender, $Certificate, $Chain, $SslPolicyErrors) + + $certState.Chain = $chain + $certState.SslPolicyErrors = $SslPolicyErrors + $true + }) + Write-Verbose -Message "Starting TLS Handshake" + $sslStream.AuthenticateAsClient($ComputerName) + Write-Verbose -Message "TLS result: $($certState.SslPolicyErrors)" + + if ($certState.SslPolicyErrors -ne 'None') { + $msg = @( + "Client does not trust remote certificate: $($certState.SslPolicyErrors)" + $certState.ChainStatus | ForEach-Object { $_.Status; $_.StatusInformation } + ) -join ([System.Environment]::NewLine) + Write-Warning -Message $msg.TrimEnd() + } + + $cert = [System.Security.Cryptography.X509Certificates.X509Certificate2]::new($sslStream.RemoteCertificate) + Write-Verbose -Message "Found cert for $($cert.Subject), Expires: $($cert.NotAfter), SANs: $($cert.DnsNameList -join ", ")" + + $cert + } + catch { + $PSCmdlet.WriteError($_) + } + finally { + if ($udpClient) { $udpClient.Dispose() } + if ($sslStream) { $sslStream.Dispose() } + if ($targetStream) { $targetStream.Dispose() } + if ($socket) { $socket.Dispose() } + } + } + + $cert = Get-SqlServerTlsCertificate -ConnectionType TCP -ComputerName 127.0.0.1 -Port 1433 + + $store = new-object System.Security.Cryptography.X509Certificates.X509Store( + [System.Security.Cryptography.X509Certificates.StoreName]::Root, + "localmachine" + ) + + $store.open("MaxAllowed") + $store.add($cert) + $store.close() + displayName: 'Add SQL Certificate as Trusted [Win]' + condition: ${{parameters.condition }} \ No newline at end of file From f8266afad2785d5096065bb897a52e747a133bda Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 20:39:45 +0100 Subject: [PATCH 06/17] Generate our own Certificate for SQL Server --- .../steps/configure-sql-server-win-step.yml | 414 ++---------------- 1 file changed, 34 insertions(+), 380 deletions(-) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index 3c8f1a7a3a..242fa73007 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -195,6 +195,40 @@ steps: displayName: 'Setup SQL Alias [Win]' condition: ${{parameters.condition }} +- powershell: | + # Create Certificate + $computerDnsName = [System.Net.Dns]::Resolve($null).HostName + $certificate = New-SelfSignedCertificate -DnsName $computerDnsName,localhost -CertStoreLocation cert:\LocalMachine\My -FriendlyName test99 -KeySpec KeyExchange + + # Get path to Private key (used later) + $keyPath = $certificate.PrivateKey.CspKeyContainerInfo.UniqueKeyContainerName + $machineKeyPath = "$env:ProgramData\Microsoft\Crypto\RSA\MachineKeys\$keyPath" + + # Add certificate to trusted roots + $store = new-object System.Security.Cryptography.X509Certificates.X509Store( + [System.Security.Cryptography.X509Certificates.StoreName]::Root, + "localmachine" + ) + + $store.open("MaxAllowed") + $store.add($certificate) + $store.close() + + # Get SQL Server instances and add the Certificate + $instances = Get-ItemProperty 'HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\Instance Names\SQL' + foreach ($instance in $instances){ + $instance | ForEach-Object { + $_.PSObject.Properties | Where-Object { $_.Name -notmatch '^PS.*' } | ForEach-Object { + Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() + + # Grant read access to Private Key for SQL Service Account + icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" + } + } + } + displayName: 'Add SQL Certificate [Win]' + condition: ${{parameters.condition }} + - powershell: | # You need to restart SQL Server for the change to persist # -Force takes care of any dependent services, like SQL Agent. @@ -248,383 +282,3 @@ steps: sqlcmd -S "(localdb)\.\${{parameters.LocalDbSharedInstanceName }}" -q "SELECT @@VERSION" displayName: 'Enable LocalDB [Win]' condition: ${{parameters.condition }} - -- powershell: | - # Script Source: https://gist.github.com/jborean93/44f92e4dfa613c5a1e7889fa7a7c2563 - # Copyright: (c) 2023, Jordan Borean (@jborean93) - # MIT License (see LICENSE or https://opensource.org/licenses/MIT) - - Function Get-SqlServerTlsCertificate { - <# - .SYNOPSIS - Gets the MS SQL X509 Certificate. - .DESCRIPTION - Gets the X509 Certificate that is being used by a remote MS SQL Server. - This certificate contains information like the Subject, SAN entries, expiry and other useful information for debugging purposes. - .PARAMETER ComputerName - The remote MS SQL Server to extract the certificate from. - .PARAMETER ConnectTimeout - The timeout, in milliseconds, to wait until the connection was successful, defaults to 5000 (5 seconds). - If the timeout is reached, the cmdlet will write an error. - .PARAMETER ConnectionType - The connection type to use for retrieving the certificate, defaults to SQLBrowser. - This can be set to SQLBrowser, NamedPipe, or TCP. - The SQLBrowser option will use the SQL Browser service to find the named pipe or TCP port for the instance requested. - The SQLBrowser needs access to the UDP port 1434 as well as the NamedPipe or TCP Port that is selected to work. - The NamedPipe option will connect to the named pipe for the instance requested. - The NamedPipe will need access to the TCP port 445 to work. - The TCP option will connect to the TCP port requested. - .PARAMETER InstanceName - The MS SQL instance to connect to. - When used with '-ConnectionType SQLBrowser', it will only connect to the instance that matches this name. - When used with '-ConnectionType NamedPipe', it will use this instance name to build the named pipe name. - Set to an empty string to use the first instance found by the SQLBrowser. - .PARAMETER Port - The TCP port to use for the connection, defaults to 1433. - This is only used if '-ConnectionType TCP' is requested. - .PARAMETER StrictEncrypt - Perform strict encryption that was introduced with TDS 8.0 (SQL Server 2022 and newer). - Strict encryption simplifies the connection process but will only work if the server is new enough to support it. - .EXAMPLE - PS> Get-SqlServerTlsCertificate -ComputerName sql01 - Gets the certificate of the first instance found on sql01 using the SQL Browser service to find the TCP port or Named Pipe. - .EXAMPLE - PS> Get-SqlServerTlsCertificate -ComputerName sql01 -Instance MySQLInstance - Gets the certificate for the instance 'sql01\MySQLInstance' using the SQL Browser service to find the TCP port or Named Pipe. - .EXAMPLE - PS> Get-SqlServerTlSCertificate -ComputerName sql01 -Port 65334 -ConnectionType TCP - Gets the certificate for server sql01 using the TCP port 65334 - .EXAMPLE - PS> Get-SqlServerTlsCertificate -ComputerName sql01 -ConnectionType NamedPipe - Gets the certificate for the default instance on sql01 using the Named Pipe connection. - .EXAMPLE - PS> Get-SqlServerTlsCertificate -ComputerName sql01 -InstanceName MySQLInstance -ConnectionType NamedPipe - Gets the certificate for the instance 'sql01\MySQLInstance' using the Named Pipe connection. - .EXAMPLE - PS> $cert = Get-SqlServerTlsCertificate -ComputerName sql01 - PS> $certBytes = $cert.Export("Cert") - PS> $setParams = @{} - PS> if ($PSVersionTable.PSVersion -lt [Version]'6.0') { - ... $setParams.Raw = $true - ... } else { - ... $setParams.AsByteStream = $true - ... } - PS> Set-Content -Path sql01.crt -Value $certBytes @setParams - Gets the certificate for the SQL server sql01 and exports it to a .crt file for use in Windows. - .OUTPUTS - System.Security.Cryptography.X509Certificates.X509Certificate2 - This cmdlet will output the X509Certificate2 object retrieved from the server. - .NOTES - Run with -Verbose to get a better understanding of how this cmdlet connects to the MS SQL server. - A warning will be emitted if the remote certificate is not trusted and it will try to include the reasons why. - #> - [OutputType([System.Security.Cryptography.X509Certificates.X509Certificate2])] - param ( - [Parameter(Mandatory)] - [string] - $ComputerName, - - [Parameter()] - [int] - $ConnectTimeout = 5000, - - [Parameter()] - [ValidateSet("SQLBrowser", "TCP", "NamedPipe")] - [string] - $ConnectionType = "SQLBrowser", - - [Parameter()] - [AllowEmptyString()] - [string] - $InstanceName = "", - - [Parameter()] - [int] - $Port = 1433, - - [Parameter()] - [switch] - $StrictEncrypt - ) - - class TdsTlsStream : System.IO.Stream { - [System.IO.Stream]$InnerStream - [int]$PayloadLength = 0 - - TdsTlsStream([System.IO.Stream]$InnerStream) { - $this.InnerStream = $InnerStream - } - - [bool] get_CanRead() { return $this.InnerStream.CanRead } - [bool] get_CanWrite() { return $this.InnerStream.CanWrite } - [bool] get_CanSeek() { return $this.InnerStream.CanSeek } - [Int64] get_Length() { return $this.InnerStream.Length } - [Int64] get_Position() { return $this.InnerStream.Position } - [void] set_Position([Int64]$Value) { $this.InnerStream.Position = $Value } - [int] get_ReadTimeout() { return $this.InnerStream.ReadTimeout } - [int] get_WriteTimeout() { return $this.InnerStream.WriteTimeout } - - [void] Flush() { $this.InnerStream.Flush() } - [Int64] Seek([Int64]$Offset, [System.IO.SeekOrigin]$Origin) { return $this.InnerStream.Seek($Offset, $Origin) } - [void] SetLength([Int64]$Value) { $this.InnerStream.SetLength($Value) } - - [int] Read([byte[]]$Buffer, [int]$Offset, [int]$Count) { - # We need to strip off the TDS header before setting the Buffer - if ($this.PayloadLength -eq 0) { - $header = [byte[]]::new(8) - $read = 0 - while ($read -lt 8) { - $read += $this.InnerStream.Read($header, 0, 8) - } - - $lengthBeforeHeader = [System.BitConverter]::ToUInt16([byte[]]@($header[3], $header[2]), 0) - $lengthBeforeHeader -= 8 - $this.PayloadLength = $lengthBeforeHeader - } - - if ($Count -gt $this.PayloadLength) { - $Count = $this.PayloadLength - } - $read = $this.InnerStream.Read($Buffer, $Offset, $Count) - $this.PayloadLength -= $read - return $read - } - - [void] Write([byte[]]$Buffer, [int]$Offset, [int]$Count) { - $newPayload = $this.GenerateTdsHeader($Buffer, $Offset, $Count) - $this.InnerStream.Write($newPayload, 0, $newPayload.Length) - } - - [byte[]] GenerateTdsHeader([byte[]]$Payload, [int]$Offset, [int]$Count) { - # The length is big endian encoded so it is inserted in reverse order - $lengthBytes = [System.BitConverter]::GetBytes([uint16]($Count + 8)) - - $newPayload = [byte[]]::new(8 + $Count) - $newPayload[0] = 0x12 # Type - Pre-Login - $newPayload[1] = 0x01 # Status - End of message (EOM) - $newPayload[2] = $lengthBytes[1] - $newPayload[3] = $lengthBytes[0] - $newPayload[4] = 0 # SPID - $newPayload[5] = 0 # SPID - $newPayload[6] = 0 # PacketID - $newPayload[7] = 0 # Window - [System.Array]::Copy($Payload, $Offset, $newPayload, 8, $Count) - - return $newPayload - } - } - - $udpClient = $socket = $targetStream = $sslStream = $null - try { - $pipeName = if ($InstanceName -and $InstanceName -ne 'MSSQLSERVER') { - 'MSSQL${0}\sql\query' -f $InstanceName - } - else { - 'sql\query' - } - - if ($ConnectionType -eq "SQLBrowser") { - # Use the SQLBrowser - # https://learn.microsoft.com/en-us/openspecs/windows_protocols/mc-sqlr/2e1560c9-5097-4023-9f5e-72b9ff1ec3b1 - $udpClient = [System.Net.Sockets.UdpClient]::new($ComputerName, 1434) - $udpClient.Client.SendTimeout = $ConnectTimeout - $udpClient.Client.ReceiveTimeout = $ConnectTimeout - $null = $udpClient.Send([byte[]]@(0x03), 1) # CLNT_UCAST_EX - $resp = $udpClient.Receive([ref]$null) - - $respSize = [System.BitConverter]::ToUInt16($resp, 1) - $rawResponse = [System.Text.Encoding]::UTF8.GetString($resp, 3, $respSize) - Write-Verbose -Message "Recieved SQL Browser response: '$rawResponse'" - $response = $rawResponse -split ';' - - $instanceInfo = [Ordered]@{} - $remoteInstance = @( - for ($i = 0; $i -lt $response.Length; $i += 2) { - if ($response[$i]) { - $instanceInfo[$response[$i]] = $response[$i + 1] - } - elseif ($i -eq $response.Length - 1) { - break - } - else { - $info = [PSCustomObject]$instanceInfo - Write-Verbose -Message "Processed SQL Browser Response:`n$($info | Out-String)" - - $info - $instanceInfo = [Ordered]@{} - $i -= 1 - } - } - ) | Where-Object { -not $InstanceName -or $_.InstanceName -eq $InstanceName } | Select-Object -First 1 - - if ($remoteInstance.np) { - $ConnectionType = 'NamedPipe' - $ComputerName = $remoteInstance.ServerName - $pipeName = $remoteInstance.np -replace "\\\\.*?\\pipe\\(.*)", '$1' - } - elseif ($remoteInstance.tcp) { - $ConnectionType = 'TCP' - $ComputerName = $remoteInstance.ServerName - $Port = $remoteInstance.tcp - } - else { - throw "Failed to receive any SQL Browser responses from $($ComputerName):1434, cannot continue" - } - } - - if ($ConnectionType -eq "TCP") { - Write-Verbose -Message "Connecting to TCP/IP endpoint $($ComputerName):$Port" - - $socket = [System.Net.Sockets.TcpClient]::new() - $connectTask = $socket.ConnectAsync($ComputerName, $Port) - if (-not $connectTask.Wait($ConnectTimeout)) { - throw "Timed out connecting to TCP/IP endpoint $($ComputerName):$Port" - } - - $null = $connectTask.GetAwaiter().GetResult() - $targetStream = $socket.GetStream() - } - else { - Write-Verbose -Message "Connecting to Named Pipe endpoint \\$($ComputerName)\pipe\$pipeName" - $targetStream = [System.IO.Pipes.NamedPipeClientStream]::new( - $ComputerName, - $pipeName, - [System.IO.Pipes.PipeDirection]::InOut) - $targetStream.Connect($ConnectTimeout) - } - - # Before TDS 8.0, TLS was done after the Pre-Login message after it was - # negotiated with the server. It also needs to prepend a header to each TLS - # payload making it more difficult. TDS 8.0 (Encrypt=strict) is a lot - # simpler as the TLS handshake is done before anything. - if ($StrictEncrypt) { - Write-Verbose -Message "Using TDS 8 TLS Handshake" - $streamToWrap = $targetStream - } - else { - Write-Verbose -Message "Using TDS 7.x Pre-Login method for the TLS handshake" - - # This is a pre-calculated TDS Pre-Login payload with the ENCRYPTION - # value of ENCRYPT_REQ (0x03). - # https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-tds/60f56408-0188-4cd5-8b90-25c6f2423868 - $tdsPreLogin = [byte[]]@( - 0x12, 0x01, 0x00, 0x2f, 0x00, 0x00, 0x01, 0x00, - 0x00, 0x00, 0x1a, 0x00, 0x06, 0x01, 0x00, 0x20, - 0x00, 0x01, 0x02, 0x00, 0x21, 0x00, 0x01, 0x03, - 0x00, 0x22, 0x00, 0x04, 0x04, 0x00, 0x26, 0x00, - 0x01, 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 - ) - $targetStream.Write($tdsPreLogin, 0, $tdsPreLogin.Count) - - $headerBytes = [byte[]]::new(8) - $read = 0 - while ($read -ne $headerBytes.Length) { - $read += $targetStream.Read($headerBytes, $read, $headerBytes.Length - $read) - } - - # Integer values are big endian encoded so swap them around. It also - # includes the header length which we've already gotten - $payloadLength = [System.BitConverter]::ToUInt16([byte[]]@($headerBytes[3], $headerBytes[2]), 0) - $payloadLength -= 8 - - $tdsPreLoginResp = [byte[]]::new($payloadLength) - $read = 0 - while ($read -ne $tdsPreLoginResp.Length) { - $read += $targetStream.Read($tdsPreLoginResp, $read, $tdsPreLoginResp.Length - $read) - } - - # The TDS Pre-Login payload starts with a variable amount of headers - # TYPE - BYTE - # OFFSET - USHORT (offset in the payload of the value) - # LENGTH - USHORT - # The headers are terminated with the type 0xFF. We want to extract the - # value for the ENCRYPT type (1) from the payload to see if the server - # supported encryption. - $serverEncrypt = 0 - $offset = 0 - while ($true) { - $plOptionType = $tdsPreLoginResp[$offset] - if ($plOptionType -eq 0xFF) { - break - } - elseif ($plOptionType -ne 1) { - $offset += 5 - continue - } - - $valueOffset = [System.BitConverter]::ToUInt16([byte[]]@($tdsPreLoginResp[$offset + 2], $tdsPreLoginResp[$offset + 1]), 0) - $serverEncrypt = $tdsPreLoginResp[$valueOffset] - break - } - - # Strip off the extra flags, we only care about these specific bits - $serverEncrypt = $serverEncrypt -band 0x0F - - # ENCRYPT_OFF, ENCRYPT_NOT_SUP - if ($serverEncrypt -in @(0, 2)) { - $msg = 'Server reported an encryption level of 0x{0:X2} which indicates it does not support TDS encryption.' -f $serverEncrypt - throw $msg - } - - # Now we know the server supports TLS we need to wrap the raw stream - # with a custom wrapper to ensure each TLS payload sent below is - # preceeded with the TDS header as required. While not implemented - # there is a note that TDS 7.1 or earlier (SQL Server 2000 or earlier) - # should use the table response type (0x04) instead. As this is so old - # I'm not going to implement that. - $streamToWrap = [TdsTlsStream]::new($targetStream) - } - - # Create the SslStream with a disable certificate verification callback. - # This allows it to connect to a self signed or cert with different - # hostname. The callback will also capture more information about the peer - # Allows us to emit warnings if it was going to fail. - $certState = @{} - $sslStream = [System.Net.Security.SslStream]::new($streamToWrap, $false, { - param($Sender, $Certificate, $Chain, $SslPolicyErrors) - - $certState.Chain = $chain - $certState.SslPolicyErrors = $SslPolicyErrors - $true - }) - Write-Verbose -Message "Starting TLS Handshake" - $sslStream.AuthenticateAsClient($ComputerName) - Write-Verbose -Message "TLS result: $($certState.SslPolicyErrors)" - - if ($certState.SslPolicyErrors -ne 'None') { - $msg = @( - "Client does not trust remote certificate: $($certState.SslPolicyErrors)" - $certState.ChainStatus | ForEach-Object { $_.Status; $_.StatusInformation } - ) -join ([System.Environment]::NewLine) - Write-Warning -Message $msg.TrimEnd() - } - - $cert = [System.Security.Cryptography.X509Certificates.X509Certificate2]::new($sslStream.RemoteCertificate) - Write-Verbose -Message "Found cert for $($cert.Subject), Expires: $($cert.NotAfter), SANs: $($cert.DnsNameList -join ", ")" - - $cert - } - catch { - $PSCmdlet.WriteError($_) - } - finally { - if ($udpClient) { $udpClient.Dispose() } - if ($sslStream) { $sslStream.Dispose() } - if ($targetStream) { $targetStream.Dispose() } - if ($socket) { $socket.Dispose() } - } - } - - $cert = Get-SqlServerTlsCertificate -ConnectionType TCP -ComputerName 127.0.0.1 -Port 1433 - - $store = new-object System.Security.Cryptography.X509Certificates.X509Store( - [System.Security.Cryptography.X509Certificates.StoreName]::Root, - "localmachine" - ) - - $store.open("MaxAllowed") - $store.add($cert) - $store.close() - displayName: 'Add SQL Certificate as Trusted [Win]' - condition: ${{parameters.condition }} \ No newline at end of file From 9d93256f1b459d14adf805f9d8abd53e4c651ca5 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 20:43:27 +0100 Subject: [PATCH 07/17] Improve Admin check --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 54058a9218..7bf6530c30 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -361,10 +361,8 @@ public static bool IsAdmin { get { -#if !NETFRAMEWORK - System.Diagnostics.Debug.Assert(OperatingSystem.IsWindows()); -#endif - return new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(WindowsBuiltInRole.Administrator); + return !RuntimeInformation.IsOSPlatform(OSPlatform.Windows) + || new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(WindowsBuiltInRole.Administrator); } } From b485ea4ae3979e237647954f62da0f7c64792b39 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 20:53:14 +0100 Subject: [PATCH 08/17] Try with Local Service --- .../common/templates/steps/configure-sql-server-win-step.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index 242fa73007..e875dedb93 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -222,7 +222,8 @@ steps: Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() # Grant read access to Private Key for SQL Service Account - icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" + # icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" + icacls $machineKeyPath /grant "NT AUTHORITY\LOCAL SERVICE:R" } } } From 35ba44cbc38945501b7634c8b1c2465c2aad2f51 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 21:03:23 +0100 Subject: [PATCH 09/17] Log the service account name --- .../steps/configure-sql-server-win-step.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index e875dedb93..3c9463942a 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -227,6 +227,18 @@ steps: } } } + + $serviceName = "${{parameters.instanceName }}" + $InstancePrefix = 'MSSQL$' + + if ( "${{parameters.instanceName }}" -ne "MSSQLSERVER" ) + { + $serviceName = $InstancePrefix+"${{parameters.instanceName }}" + } + + # Get the account running the SQL Server service + $service = Get-CimInstance -ClassName Win32_Service -Filter "Name='$serviceName'" + Write-Output "SQL Server is running under: $($service.StartName)" displayName: 'Add SQL Certificate [Win]' condition: ${{parameters.condition }} From 6d3c59d4b84d7f8f168a5df930eb4ee15be397c0 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Tue, 3 Dec 2024 21:21:46 +0100 Subject: [PATCH 10/17] Use correct service account for permissions --- .../steps/configure-sql-server-win-step.yml | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index 3c9463942a..b0ef65fde4 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -222,23 +222,14 @@ steps: Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() # Grant read access to Private Key for SQL Service Account - # icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" - icacls $machineKeyPath /grant "NT AUTHORITY\LOCAL SERVICE:R" + if ($($_.Name) -eq "MSSQLSERVER") { + icacls $machineKeyPath /grant "NT Service\MSSQLSERVER:R" + } else { + icacls $machineKeyPath /grant "NT Service\MSSQL`$$($_.Name):R" + } } } } - - $serviceName = "${{parameters.instanceName }}" - $InstancePrefix = 'MSSQL$' - - if ( "${{parameters.instanceName }}" -ne "MSSQLSERVER" ) - { - $serviceName = $InstancePrefix+"${{parameters.instanceName }}" - } - - # Get the account running the SQL Server service - $service = Get-CimInstance -ClassName Win32_Service -Filter "Name='$serviceName'" - Write-Output "SQL Server is running under: $($service.StartName)" displayName: 'Add SQL Certificate [Win]' condition: ${{parameters.condition }} From 7097b303e260a2f48ca10d8d20938672b16616ea Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Wed, 4 Dec 2024 07:27:11 +0100 Subject: [PATCH 11/17] Only test on Windows --- .../ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs | 1 + 1 file changed, 1 insertion(+) 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 137b281da0..959ba49c04 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -171,6 +171,7 @@ public void OpeningConnectionWitHNICTest() } [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] + [PlatformSpecific(TestPlatforms.Windows)] public void RemoteCertificateNameMismatchErrorTest() { SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString) From 58349d8efc3afb1676f9c51b7941a5d450399ed7 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Wed, 4 Dec 2024 16:22:03 +0100 Subject: [PATCH 12/17] Logging, Managed SNI on Windows --- .../steps/configure-sql-server-win-step.yml | 1 + .../CertificateTest.cs | 19 +++++-------------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index b0ef65fde4..c0b81ada95 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -219,6 +219,7 @@ steps: foreach ($instance in $instances){ $instance | ForEach-Object { $_.PSObject.Properties | Where-Object { $_.Name -notmatch '^PS.*' } | ForEach-Object { + Write-Output "Configuring instance $($_.Name) (Value: $($_.Value))" Set-ItemProperty "HKLM:\SOFTWARE\Microsoft\Microsoft SQL Server\$($_.Value)\MSSQLServer\SuperSocketNetLib" -Name Certificate -Value $certificate.Thumbprint.ToLower() # Grant read access to Private Key for SQL Service Account 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 959ba49c04..0d1714b1c8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionTestWithSSLCert/CertificateTest.cs @@ -95,6 +95,7 @@ private static bool IsLocalHost() private static bool AreConnStringsSetup() => DataTestUtility.AreConnStringsSetup(); private static bool IsNotAzureServer() => DataTestUtility.IsNotAzureServer(); + private static bool UseManagedSNIOnWindows() => DataTestUtility.UseManagedSNIOnWindows; [ActiveIssue("31754")] [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] @@ -170,7 +171,7 @@ public void OpeningConnectionWitHNICTest() } } - [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost))] + [ConditionalFact(nameof(AreConnStringsSetup), nameof(IsNotAzureServer), nameof(IsLocalHost), nameof(UseManagedSNIOnWindows))] [PlatformSpecific(TestPlatforms.Windows)] public void RemoteCertificateNameMismatchErrorTest() { @@ -190,19 +191,9 @@ public void RemoteCertificateNameMismatchErrorTest() _testOutputHelper.WriteLine("Actual inner exception:"); _testOutputHelper.WriteLine(exception.InnerException?.ToString() ?? "None"); Assert.Equal(20, exception.Class); - - if (DataTestUtility.IsUsingNativeSNI()) - { - Assert.StartsWith("A connection was successfully established with the server, but then an error occurred during the login process. (provider: SSL Provider, error: 0 - The certificate's CN name does not match the passed value.)", exception.Message); - Assert.IsType(exception.InnerException); - Assert.StartsWith("The certificate's CN name does not match the passed value", exception.InnerException.Message); - } - else - { - 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.IsType(exception.InnerException); - Assert.StartsWith("Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.", exception.InnerException.Message); - } + 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.IsType(exception.InnerException); + Assert.StartsWith("Certificate name mismatch. The provided 'DataSource' or 'HostNameInCertificate' does not match the name in the certificate.", exception.InnerException.Message); } private static void CreateValidCertificate(string script) From c8a7876447ddea7d4e0056f60aedba63c96c6fec Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Wed, 4 Dec 2024 17:07:53 +0100 Subject: [PATCH 13/17] Try restarting all instances --- .../steps/configure-sql-server-win-step.yml | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml index c0b81ada95..4abe1c9a5f 100644 --- a/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml +++ b/eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml @@ -237,19 +237,7 @@ steps: - powershell: | # You need to restart SQL Server for the change to persist # -Force takes care of any dependent services, like SQL Agent. - # Note: if the instance is named, replace MSSQLSERVER with MSSQL$ followed by - # the name of the instance (e.g. MSSQL$MYINSTANCE) - - $serviceName = "${{parameters.instanceName }}" - $InstancePrefix = 'MSSQL$' - - if ( "${{parameters.instanceName }}" -ne "MSSQLSERVER" ) - { - $serviceName = $InstancePrefix+"${{parameters.instanceName }}" - } - - Restart-Service -Name "$serviceName" -Force - + Get-Service MSSQL* | Restart-Service -Force displayName: 'Restart SQL Server [Win]' condition: ${{parameters.condition }} From e167445d0986a0d180040bd0ab27e8afac8070f0 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Thu, 5 Dec 2024 06:31:44 +0100 Subject: [PATCH 14/17] Activate SplitPacketTest --- .../ManualTests/DataCommon/DataTestUtility.cs | 8 +- .../SQL/SplitPacketTest/SplitPacketTest.cs | 102 +++++++++++------- 2 files changed, 68 insertions(+), 42 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 7bf6530c30..20961d155b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -452,7 +452,13 @@ public static bool IsAADAuthorityURLSetup() public static bool IsNotAzureServer() { - return !AreConnStringsSetup() || !Utils.IsAzureSqlServer(new SqlConnectionStringBuilder((TCPConnectionString)).DataSource); + return !AreConnStringsSetup() || !Utils.IsAzureSqlServer(new SqlConnectionStringBuilder(TCPConnectionString).DataSource); + } + + public static bool IsLocalHost() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + return ParseDataSource(builder.DataSource, out string hostname, out _, out _) && string.Equals("localhost", hostname, StringComparison.OrdinalIgnoreCase); } // Synapse: Always Encrypted is not supported with Azure Synapse. diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs index 1147e704ca..911980885c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs @@ -11,79 +11,83 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests { - [ActiveIssue("5538")] // Only testable on localhost - public class SplitPacketTest + public class SplitPacketTest : IDisposable { - private int Port = -1; - private int SplitPacketSize = 1; - private string BaseConnString; + private int _port = -1; + private int _splitPacketSize = 1; + private string _baseConnString; + private TcpListener _listener; + private CancellationTokenSource _cts = new CancellationTokenSource(); public SplitPacketTest() { - string actualHost; - int actualPort; - SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); - GetTcpInfoFromDataSource(builder.DataSource, out actualHost, out actualPort); + GetTcpInfoFromDataSource(builder.DataSource, out string actualHost, out int actualPort); - Task.Factory.StartNew(() => { SetupProxy(actualHost, actualPort); }); + Task.Factory.StartNew(() => { SetupProxy(actualHost, actualPort, _cts.Token); }); - for (int i = 0; i < 10 && Port == -1; i++) + for (int i = 0; i < 10 && _port == -1; i++) { Thread.Sleep(500); } - if (Port == -1) + if (_port == -1) throw new InvalidOperationException("Proxy local port not defined!"); - builder.DataSource = "tcp:127.0.0.1," + Port; - BaseConnString = builder.ConnectionString; + builder.DataSource = "tcp:127.0.0.1," + _port; + _baseConnString = builder.ConnectionString; } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void OneByteSplitTest() { - SplitPacketSize = 1; + _splitPacketSize = 1; OpenConnection(); + Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void AlmostFullHeaderTest() { - SplitPacketSize = 7; + _splitPacketSize = 7; OpenConnection(); + Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void FullHeaderTest() { - SplitPacketSize = 8; + _splitPacketSize = 8; OpenConnection(); + Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void HeaderPlusOneTest() { - SplitPacketSize = 9; + _splitPacketSize = 9; OpenConnection(); + Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void MARSSplitTest() { - SplitPacketSize = 1; + _splitPacketSize = 1; OpenMarsConnection("select * from Orders"); + Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] public void MARSReplicateTest() { - SplitPacketSize = 1; + _splitPacketSize = 1; OpenMarsConnection("select REPLICATE('A', 10000)"); + Assert.True(true); } private void OpenMarsConnection(string cmdText) { - using (SqlConnection conn = new SqlConnection((new SqlConnectionStringBuilder(BaseConnString) { MultipleActiveResultSets = true }).ConnectionString)) + using (SqlConnection conn = new SqlConnection((new SqlConnectionStringBuilder(_baseConnString) { MultipleActiveResultSets = true }).ConnectionString)) { conn.Open(); using (SqlCommand cmd1 = new SqlCommand(cmdText, conn)) @@ -102,7 +106,7 @@ private void OpenMarsConnection(string cmdText) private void OpenConnection() { - using (SqlConnection conn = new SqlConnection(BaseConnString)) + using (SqlConnection conn = new SqlConnection(_baseConnString)) { conn.Open(); using (SqlCommand cmd = new SqlCommand("select * from Orders", conn)) @@ -114,23 +118,23 @@ private void OpenConnection() } } - private void SetupProxy(string actualHost, int actualPort) + private void SetupProxy(string actualHost, int actualPort, CancellationToken cancellationToken) { - TcpListener listener = new TcpListener(IPAddress.Loopback, 0); - listener.Start(); - Port = ((IPEndPoint)listener.LocalEndpoint).Port; - var client = listener.AcceptTcpClientAsync().GetAwaiter().GetResult(); + _listener = new TcpListener(IPAddress.Loopback, 0); + _listener.Start(); + _port = ((IPEndPoint)_listener.LocalEndpoint).Port; + var client = _listener.AcceptTcpClientAsync().GetAwaiter().GetResult(); var sqlClient = new TcpClient(); - sqlClient.ConnectAsync(actualHost, actualPort).Wait(); + sqlClient.ConnectAsync(actualHost, actualPort).Wait(cancellationToken); - Task.Factory.StartNew(() => { ForwardToSql(client, sqlClient); }); - Task.Factory.StartNew(() => { ForwardToClient(client, sqlClient); }); + Task.Factory.StartNew(() => { ForwardToSql(client, sqlClient, cancellationToken); }, cancellationToken); + Task.Factory.StartNew(() => { ForwardToClient(client, sqlClient, cancellationToken); }, cancellationToken); } - private void ForwardToSql(TcpClient ourClient, TcpClient sqlClient) + private void ForwardToSql(TcpClient ourClient, TcpClient sqlClient, CancellationToken cancellationToken) { - while (true) + while (!cancellationToken.IsCancellationRequested) { byte[] buffer = new byte[1024]; int bytesRead = ourClient.GetStream().Read(buffer, 0, buffer.Length); @@ -139,11 +143,11 @@ private void ForwardToSql(TcpClient ourClient, TcpClient sqlClient) } } - private void ForwardToClient(TcpClient ourClient, TcpClient sqlClient) + private void ForwardToClient(TcpClient ourClient, TcpClient sqlClient, CancellationToken cancellationToken) { - while (true) + while (!cancellationToken.IsCancellationRequested) { - byte[] buffer = new byte[SplitPacketSize]; + byte[] buffer = new byte[_splitPacketSize]; int bytesRead = sqlClient.GetStream().Read(buffer, 0, buffer.Length); ourClient.GetStream().Write(buffer, 0, bytesRead); @@ -173,5 +177,21 @@ private static void GetTcpInfoFromDataSource(string dataSource, out string hostN throw new InvalidOperationException("TCP Connection String not in correct format!"); } } + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _cts.Cancel(); + _cts.Dispose(); + _listener?.Stop(); + } + } } } From b680fe1dcb7d2ed596bc22d303f656c47b13599b Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Thu, 5 Dec 2024 07:09:00 +0100 Subject: [PATCH 15/17] Close vs Dispose in netcore --- .../tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs index 911980885c..70a23423e0 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs @@ -190,7 +190,11 @@ protected virtual void Dispose(bool disposing) { _cts.Cancel(); _cts.Dispose(); +#if NETFRAMEWORK _listener?.Stop(); +#else + _listener?.Dispose(); +#endif } } } From 514caed16052a6065494cfaa3e4aaac6feae3763 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Thu, 5 Dec 2024 08:02:22 +0100 Subject: [PATCH 16/17] Minor refactoring to use existing functions --- .../SQL/SplitPacketTest/SplitPacketTest.cs | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs index 70a23423e0..bf1a1b36c7 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs @@ -22,9 +22,9 @@ public class SplitPacketTest : IDisposable public SplitPacketTest() { SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString); - GetTcpInfoFromDataSource(builder.DataSource, out string actualHost, out int actualPort); + DataSourceBuilder dataSourceBuilder = new DataSourceBuilder(builder.DataSource); - Task.Factory.StartNew(() => { SetupProxy(actualHost, actualPort, _cts.Token); }); + Task.Factory.StartNew(() => { SetupProxy(dataSourceBuilder.ServerName, dataSourceBuilder.Port ?? 1433, _cts.Token); }); for (int i = 0; i < 10 && _port == -1; i++) { @@ -159,25 +159,6 @@ private void ForwardToClient(TcpClient ourClient, TcpClient sqlClient, Cancellat } } - private static void GetTcpInfoFromDataSource(string dataSource, out string hostName, out int port) - { - string[] dataSourceParts = dataSource.Split(','); - if (dataSourceParts.Length == 1) - { - hostName = dataSourceParts[0].Replace("tcp:", ""); - port = 1433; - } - else if (dataSourceParts.Length == 2) - { - hostName = dataSourceParts[0].Replace("tcp:", ""); - port = int.Parse(dataSourceParts[1]); - } - else - { - throw new InvalidOperationException("TCP Connection String not in correct format!"); - } - } - public void Dispose() { Dispose(true); @@ -190,6 +171,7 @@ protected virtual void Dispose(bool disposing) { _cts.Cancel(); _cts.Dispose(); + _listener?.Server.Dispose(); #if NETFRAMEWORK _listener?.Stop(); #else From 85d6ad54566a0f6bf7a2726c76d3549cb3c18881 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Thu, 5 Dec 2024 09:46:03 +0100 Subject: [PATCH 17/17] Proxy does not work on named instances --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 5 +++++ .../SQL/SplitPacketTest/SplitPacketTest.cs | 12 ++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 20961d155b..84da2d86c4 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -455,6 +455,11 @@ public static bool IsNotAzureServer() return !AreConnStringsSetup() || !Utils.IsAzureSqlServer(new SqlConnectionStringBuilder(TCPConnectionString).DataSource); } + public static bool IsNotNamedInstance() + { + return !AreConnStringsSetup() || !new SqlConnectionStringBuilder(TCPConnectionString).DataSource.Contains(@"\"); + } + public static bool IsLocalHost() { SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs index bf1a1b36c7..4cfd148d72 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SplitPacketTest/SplitPacketTest.cs @@ -37,7 +37,7 @@ public SplitPacketTest() _baseConnString = builder.ConnectionString; } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void OneByteSplitTest() { _splitPacketSize = 1; @@ -45,7 +45,7 @@ public void OneByteSplitTest() Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void AlmostFullHeaderTest() { _splitPacketSize = 7; @@ -53,7 +53,7 @@ public void AlmostFullHeaderTest() Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void FullHeaderTest() { _splitPacketSize = 8; @@ -61,7 +61,7 @@ public void FullHeaderTest() Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void HeaderPlusOneTest() { _splitPacketSize = 9; @@ -69,7 +69,7 @@ public void HeaderPlusOneTest() Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void MARSSplitTest() { _splitPacketSize = 1; @@ -77,7 +77,7 @@ public void MARSSplitTest() Assert.True(true); } - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsLocalHost), nameof(DataTestUtility.IsNotNamedInstance))] public void MARSReplicateTest() { _splitPacketSize = 1;