From aaa816113c2646f2f5c1c3cb35a527b4af2a050e Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 06:32:07 +0100 Subject: [PATCH 1/7] Remove uncalled LoadSSPILibrary method This was lingering from earlier work to change the SSPI abstractions --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs index c993df824c..25b3ae6228 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs @@ -13,11 +13,6 @@ internal void PostReadAsyncForMars() // No-Op } - private void LoadSSPILibrary() - { - // No - Op - } - private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion) { // No - Op From a66fe7e3ede6d58ae70209afc56b9120ea23a3b9 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 06:44:09 +0100 Subject: [PATCH 2/7] netcore: move PostReadAsyncForMars to TdsParserStateObject The managed SNI doesn't use this, so will always return SNI_SUCCESS_IO_PENDING. --- .../Data/SqlClient/TdsParser.Unix.cs | 5 --- .../Data/SqlClient/TdsParser.Windows.cs | 37 ------------------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 8 +++- .../SqlClient/TdsParserStateObjectManaged.cs | 2 + .../SqlClient/TdsParserStateObjectNative.cs | 29 +++++++++++++++ .../SqlClient/TdsParserStateObjectNative.cs | 29 +++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 + 7 files changed, 69 insertions(+), 43 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs index 25b3ae6228..781ca5a620 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs @@ -8,11 +8,6 @@ namespace Microsoft.Data.SqlClient { sealed internal partial class TdsParser { - internal void PostReadAsyncForMars() - { - // No-Op - } - private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion) { // No - Op diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs index 2c924b3b79..c4e657720a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs @@ -9,43 +9,6 @@ namespace Microsoft.Data.SqlClient { internal sealed partial class TdsParser { - internal void PostReadAsyncForMars() - { - if (TdsParserStateObjectFactory.UseManagedSNI) - return; - - // HACK HACK HACK - for Async only - // Have to post read to initialize MARS - will get callback on this when connection goes - // down or is closed. - - PacketHandle temp = default; - uint error = TdsEnums.SNI_SUCCESS; - - _pMarsPhysicalConObj.IncrementPendingCallbacks(); - SessionHandle handle = _pMarsPhysicalConObj.SessionHandle; - // we do not need to consider partial packets when making this read because we - // expect this read to pend. a partial packet should not exist at setup of the - // parser - Debug.Assert(_physicalStateObj.PartialPacket==null); - temp = _pMarsPhysicalConObj.ReadAsync(handle, out error); - - Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - - if (temp.NativePointer != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - _pMarsPhysicalConObj.ReleasePacket(temp); - } - - Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); - if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) - { - Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); - _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); - ThrowExceptionAndWarning(_physicalStateObj); - } - } - private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion) { // in the case where an async connection is made, encryption is used and Windows Authentication is used, diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index b5febf7add..c81276435f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -646,7 +646,13 @@ internal void EnableMars() ThrowExceptionAndWarning(_physicalStateObj); } - PostReadAsyncForMars(); + error = _pMarsPhysicalConObj.PostReadAsyncForMars(_physicalStateObj); + if (error != TdsEnums.SNI_SUCCESS_IO_PENDING) + { + Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); + _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); + ThrowExceptionAndWarning(_physicalStateObj); + } _physicalStateObj = CreateSession(); // Create and open default MARS stateObj and connection. } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs index 16be2e763c..a09f3c0e72 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -365,6 +365,8 @@ internal override uint EnableMars(ref uint info) return TdsEnums.SNI_ERROR; } + internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateObject) => TdsEnums.SNI_SUCCESS_IO_PENDING; + internal override uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename) { SniHandle sessionHandle = GetSessionSNIHandleHandleOrThrow(); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 895c46c7bf..b8eb06440f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -385,6 +385,35 @@ internal override uint DisableSsl() internal override uint EnableMars(ref uint info) => SniNativeWrapper.SniAddProvider(Handle, Provider.SMUX_PROV, ref info); + internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateObject) + { + // HACK HACK HACK - for Async only + // Have to post read to initialize MARS - will get callback on this when connection goes + // down or is closed. + + PacketHandle temp = default; + uint error = TdsEnums.SNI_SUCCESS; + + IncrementPendingCallbacks(); + SessionHandle handle = SessionHandle; + // we do not need to consider partial packets when making this read because we + // expect this read to pend. a partial packet should not exist at setup of the + // parser + Debug.Assert(physicalStateObject.PartialPacket == null); + temp = ReadAsync(handle, out error); + + Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + + if (temp.NativePointer != IntPtr.Zero) + { + // Be sure to release packet, otherwise it will be leaked by native. + ReleasePacket(temp); + } + + Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); + return error; + } + internal override uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename) { AuthProviderInfo authInfo = new AuthProviderInfo(); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 807474d98b..3c76533a83 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -172,6 +172,35 @@ internal override uint DisableSsl() internal override uint EnableMars(ref uint info) => SniNativeWrapper.SniAddProvider(Handle, Provider.SMUX_PROV, ref info); + internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateObject) + { + // HACK HACK HACK - for Async only + // Have to post read to initialize MARS - will get callback on this when connection goes + // down or is closed. + + PacketHandle temp = default; + uint error = TdsEnums.SNI_SUCCESS; + + IncrementPendingCallbacks(); + SessionHandle handle = SessionHandle; + // we do not need to consider partial packets when making this read because we + // expect this read to pend. a partial packet should not exist at setup of the + // parser + Debug.Assert(physicalStateObject.PartialPacket == null); + temp = ReadAsync(handle, out error); + + Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + + if (temp.NativePointer != IntPtr.Zero) + { + // Be sure to release packet, otherwise it will be leaked by native. + ReleasePacket(temp); + } + + Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); + return error; + } + internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) => SniNativeWrapper.SniSetInfo(Handle, QueryType.SNI_QUERY_CONN_BUFSIZE, ref unsignedPacketSize); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 781f360991..4835498726 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -539,6 +539,8 @@ internal long TimeoutTime internal abstract uint EnableMars(ref uint info); + internal abstract uint PostReadAsyncForMars(TdsParserStateObject physicalStateObject); + internal abstract uint SetConnectionBufferSize(ref uint unsignedPacketSize); internal abstract void DisposePacketCache(); From 494e74a6a950dcc50a2dbc4fc9287b0659c50657 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 06:52:49 +0100 Subject: [PATCH 3/7] Merge TdsParser handling of PostReadAsyncForMars --- .../SqlClient/TdsParserStateObjectNative.cs | 32 ++++++++++++------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 23 +------------ .../SqlClient/TdsParserStateObjectNative.cs | 32 ++++++++++++------- 3 files changed, 41 insertions(+), 46 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index b8eb06440f..111d741e35 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -394,20 +394,28 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb PacketHandle temp = default; uint error = TdsEnums.SNI_SUCCESS; - IncrementPendingCallbacks(); - SessionHandle handle = SessionHandle; - // we do not need to consider partial packets when making this read because we - // expect this read to pend. a partial packet should not exist at setup of the - // parser - Debug.Assert(physicalStateObject.PartialPacket == null); - temp = ReadAsync(handle, out error); +#if NETFRAMEWORK + RuntimeHelpers.PrepareConstrainedRegions(); +#endif + try + { } + finally + { + IncrementPendingCallbacks(); + SessionHandle handle = SessionHandle; + // we do not need to consider partial packets when making this read because we + // expect this read to pend. a partial packet should not exist at setup of the + // parser + Debug.Assert(physicalStateObject.PartialPacket == null); + temp = ReadAsync(handle, out error); - Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - if (temp.NativePointer != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - ReleasePacket(temp); + if (temp.NativePointer != IntPtr.Zero) + { + // Be sure to release packet, otherwise it will be leaked by native. + ReleasePacket(temp); + } } Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 57c8de6960..e236714fc8 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -697,28 +697,7 @@ internal void EnableMars() ThrowExceptionAndWarning(_physicalStateObj); } - // HACK HACK HACK - for Async only - // Have to post read to intialize MARS - will get callback on this when connection goes - // down or is closed. - - IntPtr temp = IntPtr.Zero; - - RuntimeHelpers.PrepareConstrainedRegions(); - try - { } - finally - { - _pMarsPhysicalConObj.IncrementPendingCallbacks(); - - error = SniNativeWrapper.SniReadAsync(_pMarsPhysicalConObj.Handle, ref temp); - - if (temp != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - SniNativeWrapper.SniPacketRelease(temp); - } - } - Debug.Assert(IntPtr.Zero == temp, "unexpected syncReadPacket without corresponding SNIPacketRelease"); + error = _pMarsPhysicalConObj.PostReadAsyncForMars(_physicalStateObj); if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) { Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 3c76533a83..f5d2a04447 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -181,20 +181,28 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb PacketHandle temp = default; uint error = TdsEnums.SNI_SUCCESS; - IncrementPendingCallbacks(); - SessionHandle handle = SessionHandle; - // we do not need to consider partial packets when making this read because we - // expect this read to pend. a partial packet should not exist at setup of the - // parser - Debug.Assert(physicalStateObject.PartialPacket == null); - temp = ReadAsync(handle, out error); +#if NETFRAMEWORK + RuntimeHelpers.PrepareConstrainedRegions(); +#endif + try + { } + finally + { + IncrementPendingCallbacks(); + SessionHandle handle = SessionHandle; + // we do not need to consider partial packets when making this read because we + // expect this read to pend. a partial packet should not exist at setup of the + // parser + Debug.Assert(physicalStateObject.PartialPacket == null); + temp = ReadAsync(handle, out error); - Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); + Debug.Assert(temp.Type == PacketHandle.NativePointerType, "unexpected packet type when requiring NativePointer"); - if (temp.NativePointer != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - ReleasePacket(temp); + if (temp.NativePointer != IntPtr.Zero) + { + // Be sure to release packet, otherwise it will be leaked by native. + ReleasePacket(temp); + } } Debug.Assert(IntPtr.Zero == temp.NativePointer, "unexpected syncReadPacket without corresponding SNIPacketRelease"); From 1e677d0b83422f695920ba4b8137011482312f7d Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 07:19:58 +0100 Subject: [PATCH 4/7] netcore: remove WaitForSSLHandShakeToComplete The handling of this method is unusual. Only Windows actually waits for the SSL handshake. --- .../Microsoft/Data/SqlClient/TdsParser.Unix.cs | 4 ---- .../Microsoft/Data/SqlClient/TdsParser.Windows.cs | 13 ------------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 15 ++++++++++++++- .../Data/SqlClient/TdsParserStateObjectManaged.cs | 2 +- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs index 781ca5a620..82b94494f1 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs @@ -8,9 +8,5 @@ namespace Microsoft.Data.SqlClient { sealed internal partial class TdsParser { - private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion) - { - // No - Op - } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs index c4e657720a..fe0c535139 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs @@ -9,18 +9,5 @@ namespace Microsoft.Data.SqlClient { internal sealed partial class TdsParser { - private void WaitForSSLHandShakeToComplete(ref uint error, ref int protocolVersion) - { - // in the case where an async connection is made, encryption is used and Windows Authentication is used, - // wait for SSL handshake to complete, so that the SSL context is fully negotiated before we try to use its - // Channel Bindings as part of the Windows Authentication context build (SSL handshake must complete - // before calling SNISecGenClientContext). - error = _physicalStateObj.WaitForSSLHandShakeToComplete(out protocolVersion); - if (error != TdsEnums.SNI_SUCCESS) - { - _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); - ThrowExceptionAndWarning(_physicalStateObj); - } - } } // tdsparser }//namespace diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index c81276435f..474be0572a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -909,7 +909,20 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ } int protocolVersion = 0; - WaitForSSLHandShakeToComplete(ref error, ref protocolVersion); + + // in the case where an async connection is made, encryption is used and Windows Authentication is used, + // wait for SSL handshake to complete, so that the SSL context is fully negotiated before we try to use its + // Channel Bindings as part of the Windows Authentication context build (SSL handshake must complete + // before calling SNISecGenClientContext). + if (OperatingSystem.IsWindows()) + { + error = _physicalStateObj.WaitForSSLHandShakeToComplete(out protocolVersion); + if (error != TdsEnums.SNI_SUCCESS) + { + _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); + ThrowExceptionAndWarning(_physicalStateObj); + } + } SslProtocols protocol = (SslProtocols)protocolVersion; string warningMessage = protocol.GetProtocolWarning(); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs index a09f3c0e72..307ae5ad02 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -391,7 +391,7 @@ internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion) { protocolVersion = GetSessionSNIHandleHandleOrThrow().ProtocolVersion; - return 0; + return TdsEnums.SNI_SUCCESS; } internal override SniErrorDetails GetErrorDetails() From 6c84a214d96011095e2fbbcb733ccef9a846aaac Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 07:39:08 +0100 Subject: [PATCH 5/7] Remove now-unused files Also mop up the unused SNIErrorDetails structure --- .../src/Microsoft/Data/SqlClient/TdsParser.Unix.cs | 12 ------------ .../Microsoft/Data/SqlClient/TdsParser.Windows.cs | 13 ------------- .../Microsoft/Data/SqlClient/TdsParser.netcore.cs | 11 ----------- 3 files changed, 36 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs deleted file mode 100644 index 82b94494f1..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Unix.cs +++ /dev/null @@ -1,12 +0,0 @@ -// 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 Microsoft.Data.SqlClient.ManagedSni; - -namespace Microsoft.Data.SqlClient -{ - sealed internal partial class TdsParser - { - } -} diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs deleted file mode 100644 index fe0c535139..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.Windows.cs +++ /dev/null @@ -1,13 +0,0 @@ -// 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.Diagnostics; - -namespace Microsoft.Data.SqlClient -{ - internal sealed partial class TdsParser - { - } // tdsparser -}//namespace diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.netcore.cs index 71ef5e9381..95dd0d9731 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.netcore.cs @@ -10,17 +10,6 @@ namespace Microsoft.Data.SqlClient { internal sealed partial class TdsParser { - internal struct SNIErrorDetails - { - public string errorMessage; - public uint nativeError; - public uint sniErrorNumber; - public int provider; - public uint lineNumber; - public string function; - public Exception exception; - } - internal static void FillGuidBytes(Guid guid, Span buffer) => guid.TryWriteBytes(buffer); internal static void FillDoubleBytes(double value, Span buffer) => BinaryPrimitives.TryWriteInt64LittleEndian(buffer, BitConverter.DoubleToInt64Bits(value)); From 0bb49cfe318a2effba5f83140c1b89ad3813c8e3 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 23:24:34 +0100 Subject: [PATCH 6/7] Merge and move WaitForSSLHandShakeToComplete This exposes a behavioural difference in TdsParser between Unix and Windows: downlevel TLS warnings only appear on Windows. --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 7 +++--- .../SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../SqlClient/TdsParserStateObjectManaged.cs | 4 ++-- .../SqlClient/TdsParserStateObjectNative.cs | 16 +++++++------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 22 ++++++++++++------- .../SqlClient/TdsParserStateObjectNative.cs | 5 +++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 ++ 7 files changed, 35 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 474be0572a..ed4d6dd651 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -11,9 +11,8 @@ using System.Diagnostics; using System.Globalization; using System.IO; -#if NET using System.Security.Authentication; -#else +#if NETFRAMEWORK using System.Runtime.CompilerServices; #endif using System.Text; @@ -908,13 +907,15 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ ThrowExceptionAndWarning(_physicalStateObj); } - int protocolVersion = 0; + uint protocolVersion = 0; // in the case where an async connection is made, encryption is used and Windows Authentication is used, // wait for SSL handshake to complete, so that the SSL context is fully negotiated before we try to use its // Channel Bindings as part of the Windows Authentication context build (SSL handshake must complete // before calling SNISecGenClientContext). +#if NET if (OperatingSystem.IsWindows()) +#endif { error = _physicalStateObj.WaitForSSLHandShakeToComplete(out protocolVersion); if (error != TdsEnums.SNI_SUCCESS) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index bb3b07b0e8..4cd52593e8 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -92,8 +92,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); - internal abstract uint WaitForSSLHandShakeToComplete(out int protocolVersion); - internal abstract void Dispose(); internal abstract uint CheckConnection(); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs index 307ae5ad02..82e95e127b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs @@ -388,9 +388,9 @@ internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) return TdsEnums.SNI_SUCCESS; } - internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion) + internal override uint WaitForSSLHandShakeToComplete(out uint protocolVersion) { - protocolVersion = GetSessionSNIHandleHandleOrThrow().ProtocolVersion; + protocolVersion = (uint)GetSessionSNIHandleHandleOrThrow().ProtocolVersion; return TdsEnums.SNI_SUCCESS; } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 111d741e35..9d0dfa385b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -436,7 +436,7 @@ internal override uint EnableSsl(ref uint info, bool tlsFirst, string serverCert internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) => SniNativeWrapper.SniSetInfo(Handle, QueryType.SNI_QUERY_CONN_BUFSIZE, ref unsignedPacketSize); - internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion) + internal override uint WaitForSSLHandShakeToComplete(out uint protocolVersion) { uint returnValue = SniNativeWrapper.SniWaitForSslHandshakeToComplete(Handle, GetTimeoutRemaining(), out uint nativeProtocolVersion); var nativeProtocol = (NativeProtocols)nativeProtocolVersion; @@ -444,35 +444,35 @@ internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion) #pragma warning disable CA5398 // Avoid hardcoded SslProtocols values if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_2_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_2_SERVER)) { - protocolVersion = (int)SslProtocols.Tls12; + protocolVersion = (uint)SslProtocols.Tls12; } else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_3_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_3_SERVER)) { /* The SslProtocols.Tls13 is supported by netcoreapp3.1 and later */ - protocolVersion = (int)SslProtocols.Tls13; + protocolVersion = (uint)SslProtocols.Tls13; } else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_1_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_1_SERVER)) { - protocolVersion = (int)SslProtocols.Tls11; + protocolVersion = (uint)SslProtocols.Tls11; } else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_0_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_TLS1_0_SERVER)) { - protocolVersion = (int)SslProtocols.Tls; + protocolVersion = (uint)SslProtocols.Tls; } else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_SERVER)) { // SSL 2.0 and 3.0 are only referenced to log a warning, not explicitly used for connections #pragma warning disable CS0618, CA5397 - protocolVersion = (int)SslProtocols.Ssl3; + protocolVersion = (uint)SslProtocols.Ssl3; } else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL2_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL2_SERVER)) { - protocolVersion = (int)SslProtocols.Ssl2; + protocolVersion = (uint)SslProtocols.Ssl2; #pragma warning restore CS0618, CA5397 } else //if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_NONE)) { - protocolVersion = (int)SslProtocols.None; + protocolVersion = (uint)SslProtocols.None; } #pragma warning restore CA5398 // Avoid hardcoded SslProtocols values return returnValue; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index e236714fc8..089f5d0ef0 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -11,9 +11,8 @@ using System.Diagnostics; using System.Globalization; using System.IO; -#if NET using System.Security.Authentication; -#else +#if NETFRAMEWORK using System.Runtime.CompilerServices; #endif using System.Text; @@ -975,19 +974,26 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ ThrowExceptionAndWarning(_physicalStateObj); } + uint protocolVersion = 0; + // in the case where an async connection is made, encryption is used and Windows Authentication is used, // wait for SSL handshake to complete, so that the SSL context is fully negotiated before we try to use its // Channel Bindings as part of the Windows Authentication context build (SSL handshake must complete // before calling SNISecGenClientContext). - error = SniNativeWrapper.SniWaitForSslHandshakeToComplete(_physicalStateObj.Handle, _physicalStateObj.GetTimeoutRemaining(), out uint protocolVersion); - - if (error != TdsEnums.SNI_SUCCESS) +#if NET + if (OperatingSystem.IsWindows()) +#endif { - _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); - ThrowExceptionAndWarning(_physicalStateObj); + error = _physicalStateObj.WaitForSSLHandShakeToComplete(out protocolVersion); + if (error != TdsEnums.SNI_SUCCESS) + { + _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); + ThrowExceptionAndWarning(_physicalStateObj); + } } - string warningMessage = ((System.Security.Authentication.SslProtocols)protocolVersion).GetProtocolWarning(); + SslProtocols protocol = (SslProtocols)protocolVersion; + string warningMessage = protocol.GetProtocolWarning(); if (!string.IsNullOrEmpty(warningMessage)) { if (!encrypt && LocalAppContextSwitches.SuppressInsecureTlsWarning) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index f5d2a04447..9fe5864696 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs @@ -212,6 +212,11 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize) => SniNativeWrapper.SniSetInfo(Handle, QueryType.SNI_QUERY_CONN_BUFSIZE, ref unsignedPacketSize); + internal override uint WaitForSSLHandShakeToComplete(out uint protocolVersion) + { + return SniNativeWrapper.SniWaitForSslHandshakeToComplete(Handle, GetTimeoutRemaining(), out protocolVersion); + } + internal override SniErrorDetails GetErrorDetails() { SniNativeWrapper.SniGetLastError(out SniError sniError); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 4835498726..4d7684327e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -533,6 +533,8 @@ internal long TimeoutTime internal abstract uint SniGetConnectionId(ref Guid clientConnectionId); + internal abstract uint WaitForSSLHandShakeToComplete(out uint protocolVersion); + internal abstract uint DisableSsl(); internal abstract SspiContextProvider CreateSspiContextProvider(); From 86f4c91a2afe769d532a1f7a8f3f69f08b5d9f22 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Wed, 9 Jul 2025 23:27:59 +0100 Subject: [PATCH 7/7] Coding style cleanup --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index ed4d6dd651..ca1609cd96 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -648,7 +648,7 @@ internal void EnableMars() error = _pMarsPhysicalConObj.PostReadAsyncForMars(_physicalStateObj); if (error != TdsEnums.SNI_SUCCESS_IO_PENDING) { - Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); + Debug.Assert(error != TdsEnums.SNI_SUCCESS, "Unexpected successful read async on physical connection before enabling MARS!"); _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); ThrowExceptionAndWarning(_physicalStateObj); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 089f5d0ef0..b9524402b5 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -697,9 +697,9 @@ internal void EnableMars() } error = _pMarsPhysicalConObj.PostReadAsyncForMars(_physicalStateObj); - if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) + if (error != TdsEnums.SNI_SUCCESS_IO_PENDING) { - Debug.Assert(TdsEnums.SNI_SUCCESS != error, "Unexpected successful read async on physical connection before enabling MARS!"); + Debug.Assert(error != TdsEnums.SNI_SUCCESS, "Unexpected successful read async on physical connection before enabling MARS!"); _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); ThrowExceptionAndWarning(_physicalStateObj); }