From dcd231eb41573c12640b5d7f72c8c85abc9034d9 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 19:24:28 +0100 Subject: [PATCH 1/9] netcore: teach TdsParserStateObject to clear the buffer after a password has been written to it --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) 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 86409c3b3b..4604f641c0 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 @@ -215,8 +215,10 @@ private uint GetSniPacket(PacketHandle packet, ref uint dataSize) return SniPacketGetData(packet, _inBuff, ref dataSize); } - private void SetBufferSecureStrings() + private bool TrySetBufferSecureStrings() { + bool mustClearBuffer = false; + if (_securePasswords != null) { for (int i = 0; i < _securePasswords.Length; i++) @@ -240,6 +242,8 @@ private void SetBufferSecureStrings() } TdsParserStaticMethods.ObfuscatePassword(data); data.CopyTo(_outBuff, _securePasswordOffsetsInBuffer[i]); + + mustClearBuffer = true; } finally { @@ -248,6 +252,8 @@ private void SetBufferSecureStrings() } } } + + return mustClearBuffer; } public void ReadAsyncCallback(PacketHandle packet, uint error) => @@ -738,9 +744,13 @@ private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. PacketHandle packet = GetResetWritePacket(_outBytesUsed); + bool mustClearBuffer = TrySetBufferSecureStrings(); - SetBufferSecureStrings(); SetPacketData(packet, _outBuff, _outBytesUsed); + if (mustClearBuffer) + { + _outBuff.AsSpan(0, _outBytesUsed).Clear(); + } Debug.Assert(Parser.Connection._parserLock.ThreadMayHaveLock(), "Thread is writing without taking the connection lock"); Task task = SNIWritePacket(packet, out _, canAccumulate, callerHasConnectionLock: true); From 6eee2fc9363e8ba5d5a6fb45a913cde0decad327 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 19:47:46 +0100 Subject: [PATCH 2/9] Port SetPacketData --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs | 6 ++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) 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 4604f641c0..9fa4620388 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 @@ -738,8 +738,6 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa internal abstract PacketHandle CreateAndSetAttentionPacket(); - internal abstract void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed); - private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. 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 9ffd372742..442198510a 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 @@ -344,6 +344,12 @@ internal override void ClearAllWritePackets() } } + internal override void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed) + { + Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); + SniNativeWrapper.SniPacketSetData(packet.NativePacket, buffer, bytesUsed); + } + internal override uint SniGetConnectionId(ref Guid clientConnectionId) => SniNativeWrapper.SniGetConnectionId(Handle, ref clientConnectionId); 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 343eaeca5a..fcea679f2f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -536,6 +536,8 @@ internal abstract void CreatePhysicalSNIHandle( protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); + internal abstract void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed); + protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); internal abstract void Dispose(); From 484fde866e191e53a9e8a866e8a8753e1a28b939 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 19:54:05 +0100 Subject: [PATCH 3/9] Port references to SetPacketData, cleanup reference to SniPacketSetData overload --- .../SqlClient/TdsParserStateObject.netfx.cs | 52 ++++++++- .../Interop/Windows/Sni/SniNativeWrapper.cs | 109 ------------------ 2 files changed, 49 insertions(+), 112 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 276eabce8c..5101defd50 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -243,6 +243,47 @@ private uint GetSniPacket(PacketHandle packet, ref uint dataSize) return SniPacketGetData(packet, _inBuff, ref dataSize); } + private bool TrySetBufferSecureStrings() + { + bool mustClearBuffer = false; + + if (_securePasswords != null) + { + for (int i = 0; i < _securePasswords.Length; i++) + { + if (_securePasswords[i] != null) + { + IntPtr str = IntPtr.Zero; + try + { + str = Marshal.SecureStringToBSTR(_securePasswords[i]); + byte[] data = new byte[_securePasswords[i].Length * 2]; + Marshal.Copy(str, data, 0, _securePasswords[i].Length * 2); + if (!BitConverter.IsLittleEndian) + { + Span span = data.AsSpan(); + for (int ii = 0; ii < _securePasswords[i].Length * 2; ii += 2) + { + short value = BinaryPrimitives.ReadInt16LittleEndian(span.Slice(ii)); + BinaryPrimitives.WriteInt16BigEndian(span.Slice(ii), value); + } + } + TdsParserStaticMethods.ObfuscatePassword(data); + data.CopyTo(_outBuff, _securePasswordOffsetsInBuffer[i]); + + mustClearBuffer = true; + } + finally + { + Marshal.ZeroFreeBSTR(str); + } + } + } + } + + return mustClearBuffer; + } + public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) { // Key never used. @@ -721,7 +762,7 @@ internal PacketHandle CreateAndSetAttentionPacket() { SNIPacket attnPacket = new SNIPacket(Handle); _sniAsyncAttnPacket = attnPacket; - SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN, null, null); + SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); return PacketHandle.FromNativePacket(attnPacket); } @@ -729,8 +770,13 @@ private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. PacketHandle packet = GetResetWritePacket(_outBytesUsed); - SNIPacket nativePacket = packet.NativePacket; - SniNativeWrapper.SniPacketSetData(nativePacket, _outBuff, _outBytesUsed, _securePasswords, _securePasswordOffsetsInBuffer); + bool mustClearBuffer = TrySetBufferSecureStrings(); + + SetPacketData(packet, _outBuff, _outBytesUsed); + if (mustClearBuffer) + { + _outBuff.AsSpan(0, _outBytesUsed).Clear(); + } Debug.Assert(Parser.Connection._parserLock.ThreadMayHaveLock(), "Thread is writing without taking the connection lock"); Task task = SNIWritePacket(packet, out _, canAccumulate, callerHasConnectionLock: true); diff --git a/src/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniNativeWrapper.cs b/src/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniNativeWrapper.cs index 4f90fe3518..6dc01dc31e 100644 --- a/src/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniNativeWrapper.cs +++ b/src/Microsoft.Data.SqlClient/src/Interop/Windows/Sni/SniNativeWrapper.cs @@ -305,115 +305,6 @@ internal static unsafe void SniPacketSetData(SNIPacket packet, byte[] data, int } } - #if NETFRAMEWORK - // Notes on SecureString: Writing out security sensitive information to managed buffer - // should be avoided as these can be moved around by GC. There are two set of - // information which falls into this category: passwords and new changed password which - // are passed in as SecureString by a user. Writing out clear passwords information is - // delayed until this layer to ensure that the information is written out to buffer - // which is pinned in this method already. This also ensures that processing a clear - // password is done right before it is written out to SNI_Packet where gets encrypted - // properly. TdsParserStaticMethods.EncryptPassword operation is also done here to - // minimize the time the clear password is held in memory. Any time loose encryption - // algorithms are changed it should be done in both in this method and - // TdsParserStaticMethods.EncryptPassword. - // Up to current release, it is also guaranteed that both password and new change - // password will fit into a single login packet whose size is fixed to 4096 So, no - // splitting logic is needed. - internal static void SniPacketSetData( - SNIPacket packet, - byte[] data, - int length, - SecureString[] passwords, // pointer to the passwords which need to be written out to SNI Packet - int[] passwordOffsets) // Offset into data buffer where the password to be written out to - { - Debug.Assert(passwords is null || (passwordOffsets is not null && passwords.Length == passwordOffsets.Length), "The number of passwords does not match the number of password offsets"); - - bool mustRelease = false; - bool mustClearBuffer = false; - IntPtr clearPassword = IntPtr.Zero; - - try - { - unsafe - { - if (passwords != null) - { - // Process SecureString - for (int i = 0; i < passwords.Length; ++i) - { - // SecureString is used - if (passwords[i] != null) - { - try - { - // ============================================================ - // Get the clear text of secure string without converting it - // to string type - // ============================================================ - clearPassword = Marshal.SecureStringToCoTaskMemUnicode(passwords[i]); - - // ============================================================ - // Loosely encrypt the clear text - The encryption algorithm - // should exactly match the TdsParserStaticMethods.EncryptPassword - // ============================================================ - char* pwChar = (char*)clearPassword.ToPointer(); - byte* pByte = (byte*)clearPassword.ToPointer(); - - int passwordsLength = passwords[i].Length; - for (int j = 0; j < passwordsLength; ++j) - { - int s = *pwChar; - byte bLo = (byte)(s & 0xff); - byte bHi = (byte)((s >> 8) & 0xff); - *(pByte++) = (byte)((((bLo & 0x0f) << 4) | (bLo >> 4)) ^ 0xa5); - *(pByte++) = (byte)((((bHi & 0x0f) << 4) | (bHi >> 4)) ^ 0xa5); - ++pwChar; - } - - // ============================================================ - // Write out the loosely encrypted passwords to data buffer - // ============================================================ - mustClearBuffer = true; - Marshal.Copy(clearPassword, data, passwordOffsets[i], passwordsLength * 2); - } - finally - { - // Make sure that we clear the security sensitive information - if (clearPassword != IntPtr.Zero) - { - Marshal.ZeroFreeCoTaskMemUnicode(clearPassword); - } - } - } - } - } - - packet.DangerousAddRef(ref mustRelease); - Debug.Assert(mustRelease, "AddRef Failed!"); - - SniPacketSetData(packet, data, length); - } - } - finally - { - if (mustRelease) - { - packet.DangerousRelease(); - } - - // Make sure that we clear the security sensitive information - if (mustClearBuffer) - { - for (int i = 0; i < data.Length; ++i) - { - data[i] = 0; - } - } - } - } - #endif - internal static void SniPacketReset(SNIHandle pConn, IoType ioType, SNIPacket pPacket, ConsumerNumber consNum) => s_nativeMethods.SniPacketReset(pConn, ioType, pPacket, consNum); From dfd030c10dc1c2ea3260d236cea33a6d5ceb0193 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 20:08:29 +0100 Subject: [PATCH 4/9] Port CreateAndSetAttentionPacket --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Data/SqlClient/TdsParserStateObjectNative.cs | 5 ++--- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 8 -------- .../Data/SqlClient/TdsParserStateObjectNative.cs | 8 ++++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 5 files changed, 12 insertions(+), 13 deletions(-) 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 9fa4620388..5c6c72b22c 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 @@ -736,8 +736,6 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa } } - internal abstract PacketHandle CreateAndSetAttentionPacket(); - private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. 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 bf7c4aebb7..a7f31da617 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 @@ -308,10 +308,9 @@ internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint internal override PacketHandle CreateAndSetAttentionPacket() { - SNIHandle handle = Handle; - SNIPacket attnPacket = new SNIPacket(handle); + SNIPacket attnPacket = new SNIPacket(Handle); _sniAsyncAttnPacket = attnPacket; - SetPacketData(PacketHandle.FromNativePacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN); + SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); return PacketHandle.FromNativePacket(attnPacket); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 5101defd50..dfafd0fc04 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -758,14 +758,6 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa } } - internal PacketHandle CreateAndSetAttentionPacket() - { - SNIPacket attnPacket = new SNIPacket(Handle); - _sniAsyncAttnPacket = attnPacket; - SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); - return PacketHandle.FromNativePacket(attnPacket); - } - private Task WriteSni(bool canAccumulate) { // Prepare packet, and write to packet. 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 442198510a..e34fe11031 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 @@ -284,6 +284,14 @@ internal override PacketHandle ReadSyncOverAsync(int timeoutRemaining, out uint return PacketHandle.FromNativePointer(readPacketPtr); } + internal override PacketHandle CreateAndSetAttentionPacket() + { + SNIPacket attnPacket = new SNIPacket(Handle); + _sniAsyncAttnPacket = attnPacket; + SniNativeWrapper.SniPacketSetData(attnPacket, SQL.AttentionHeader, TdsEnums.HEADER_LEN); + return PacketHandle.FromNativePacket(attnPacket); + } + internal override uint WritePacket(PacketHandle packet, bool sync) { Debug.Assert(packet.Type == PacketHandle.NativePacketType, "unexpected packet type when requiring NativePacket"); 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 fcea679f2f..53d716fcf0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -534,6 +534,8 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract PacketHandle GetResetWritePacket(int dataSize); + internal abstract PacketHandle CreateAndSetAttentionPacket(); + protected abstract uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize); internal abstract void SetPacketData(PacketHandle packet, byte[] buffer, int bytesUsed); From eae83d2e61483d55cc6ca0fc5f0c0abf1d1158bb Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 20:14:51 +0100 Subject: [PATCH 5/9] Port CheckConnection --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 8 +------- .../Data/SqlClient/TdsParserStateObjectNative.cs | 6 ++++++ .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 9 insertions(+), 9 deletions(-) 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 5c6c72b22c..de1db77097 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 @@ -59,8 +59,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); - internal abstract uint CheckConnection(); - internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index dfafd0fc04..cbc9a25936 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -83,8 +83,6 @@ internal SNIHandle Handle // General methods // ///////////////////// - internal uint CheckConnection() => SniNativeWrapper.SniCheckConnection(Handle); - internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); @@ -121,11 +119,7 @@ internal bool ValidateSNIConnection() try { Interlocked.Increment(ref _readingCount); - SNIHandle handle = Handle; - if (handle != null) - { - error = SniNativeWrapper.SniCheckConnection(handle); - } + error = CheckConnection(); } finally { 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 e34fe11031..bc36dc8e34 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 @@ -269,6 +269,12 @@ internal override void ReleasePacket(PacketHandle syncReadPacket) SniNativeWrapper.SniPacketRelease(syncReadPacket.NativePointer); } + internal override uint CheckConnection() + { + SNIHandle handle = Handle; + return handle == null ? TdsEnums.SNI_SUCCESS : SniNativeWrapper.SniCheckConnection(handle); + } + internal override PacketHandle ReadAsync(SessionHandle handle, out uint error) { IntPtr readPacketPtr = IntPtr.Zero; 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 53d716fcf0..984aa9fbdc 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -532,6 +532,8 @@ internal abstract void CreatePhysicalSNIHandle( string hostNameInCertificate = "", string serverCertificateFilename = ""); + internal abstract uint CheckConnection(); + internal abstract PacketHandle GetResetWritePacket(int dataSize); internal abstract PacketHandle CreateAndSetAttentionPacket(); From 0ca01b896ada31508e5139aa2683a0a7f3fe50bb Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:30:28 +0100 Subject: [PATCH 6/9] Port EnableSSL --- .../SqlClient/TdsParserStateObject.netcore.cs | 2 -- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 13 +------------ .../Data/SqlClient/TdsParserStateObjectNative.cs | 15 +++++++++++++++ .../Data/SqlClient/TdsParserStateObject.cs | 2 ++ 4 files changed, 18 insertions(+), 14 deletions(-) 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 de1db77097..83157347bd 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 @@ -57,8 +57,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo // General methods // ///////////////////// - internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); - internal int DecrementPendingCallbacks(bool release) { int remaining = Interlocked.Decrement(ref _pendingCallbacks); 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 d65b0db83e..e9601df19b 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 @@ -945,19 +945,8 @@ private void EnableSsl(uint info, SqlConnectionEncryptOption encrypt, bool integ info |= TdsEnums.SNI_SSL_IGNORE_CHANNEL_BINDINGS; } - // Add SSL (Encryption) SNI provider. - AuthProviderInfo authInfo = new AuthProviderInfo(); - authInfo.flags = info; - authInfo.tlsFirst = encrypt == SqlConnectionEncryptOption.Strict; - authInfo.certId = null; - authInfo.certHash = false; - authInfo.clientCertificateCallbackContext = IntPtr.Zero; - authInfo.clientCertificateCallback = null; - authInfo.serverCertFileName = string.IsNullOrEmpty(serverCertificateFilename) ? null : serverCertificateFilename; - Debug.Assert((_encryptionOption & EncryptionOptions.CLIENT_CERT) == 0, "Client certificate authentication support has been removed"); - - error = SniNativeWrapper.SniAddProvider(_physicalStateObj.Handle, Provider.SSL_PROV, authInfo); + error = _physicalStateObj.EnableSsl(ref info, encrypt == SqlConnectionEncryptOption.Strict, serverCertificateFilename); if (error != TdsEnums.SNI_SUCCESS) { 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 bc36dc8e34..81aaa63168 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 @@ -410,6 +410,21 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb return error; } + internal override uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename) + { + AuthProviderInfo authInfo = new AuthProviderInfo(); + authInfo.flags = info; + authInfo.tlsFirst = tlsFirst; + authInfo.certId = null; + authInfo.certHash = false; + authInfo.clientCertificateCallbackContext = IntPtr.Zero; + authInfo.clientCertificateCallback = null; + authInfo.serverCertFileName = string.IsNullOrEmpty(serverCertificateFilename) ? null : serverCertificateFilename; + + // Add SSL (Encryption) SNI provider. + return SniNativeWrapper.SniAddProvider(Handle, Provider.SSL_PROV, ref authInfo); + } + 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 984aa9fbdc..ced9a158f4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -532,6 +532,8 @@ internal abstract void CreatePhysicalSNIHandle( string hostNameInCertificate = "", string serverCertificateFilename = ""); + internal abstract uint EnableSsl(ref uint info, bool tlsFirst, string serverCertificateFilename); + internal abstract uint CheckConnection(); internal abstract PacketHandle GetResetWritePacket(int dataSize); From d7d4ed4d176076a33131861ab6ea049e9478a958 Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:39:18 +0100 Subject: [PATCH 7/9] Port Handle property and remaining variables --- .../SqlClient/TdsParserStateObject.netfx.cs | 19 +------------------ .../SqlClient/TdsParserStateObjectNative.cs | 6 ++++++ 2 files changed, 7 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index cbc9a25936..1fa560b054 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -20,12 +20,6 @@ namespace Microsoft.Data.SqlClient { internal partial class TdsParserStateObject { - protected SNIHandle _sessionHandle = null; // the SNI handle we're to work on - - // SNI variables // multiple resultsets in one batch. - protected SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS - internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn - // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; internal int _tracePasswordLength = 0; @@ -68,17 +62,6 @@ protected TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCo _lastSuccessfulIOTimer = parser._physicalStateObj._lastSuccessfulIOTimer; } - //////////////// - // Properties // - //////////////// - internal SNIHandle Handle - { - get - { - return _sessionHandle; - } - } - ///////////////////// // General methods // ///////////////////// @@ -92,7 +75,7 @@ internal int DecrementPendingCallbacks(bool release) // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it - Debug.Assert((remaining == -1 && _sessionHandle == null) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); + Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); return remaining; } 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 81aaa63168..6fc1664d3f 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 @@ -19,6 +19,10 @@ namespace Microsoft.Data.SqlClient { internal class TdsParserStateObjectNative : TdsParserStateObject { + private SNIHandle _sessionHandle = null; // the SNI handle we're to work on + + private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS + internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used private GCHandle _gcHandle; // keeps this object alive until we're closed. @@ -37,6 +41,8 @@ internal TdsParserStateObjectNative(TdsParser parser) #region Properties + internal SNIHandle Handle => _sessionHandle; + internal override uint Status => _sessionHandle != null ? _sessionHandle.Status : TdsEnums.SNI_UNINITIALIZED; internal override SessionHandle SessionHandle => SessionHandle.FromNativeHandle(_sessionHandle); From 4b8c0932b5510efeb85a1d3253158aa07419ac8a Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:42:59 +0100 Subject: [PATCH 8/9] Remove remnants of CERs and tidy usings --- .../Data/SqlClient/TdsParserStateObject.netcore.cs | 9 +-------- .../Data/SqlClient/TdsParserStateObjectNative.cs | 1 - .../Data/SqlClient/TdsParserStateObject.netfx.cs | 4 ---- .../Data/SqlClient/TdsParserStateObjectNative.cs | 3 --- 4 files changed, 1 insertion(+), 16 deletions(-) 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 83157347bd..9111a19eec 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 @@ -11,7 +11,6 @@ using System.Threading.Tasks; using Microsoft.Data.Common; using Microsoft.Data.ProviderBase; -using Microsoft.Data.SqlClient.ManagedSni; namespace Microsoft.Data.SqlClient { @@ -563,13 +562,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu } // Async operation completion may be delayed (success pending). - try - { - } - finally - { - sniError = WritePacket(packet, sync); - } + sniError = WritePacket(packet, sync); if (sniError == TdsEnums.SNI_SUCCESS_IO_PENDING) { 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 a7f31da617..3b81cea250 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 @@ -8,7 +8,6 @@ using System.Net; using System.Runtime.InteropServices; using System.Security.Authentication; -using System.Text; using System.Threading.Tasks; using Interop.Windows.Sni; using Microsoft.Data.Common; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 1fa560b054..194beaca5f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -4,15 +4,11 @@ using System; using System.Buffers.Binary; -using System.Collections.Generic; using System.Diagnostics; -using System.Runtime.CompilerServices; -using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Security; using System.Threading; using System.Threading.Tasks; -using Interop.Windows.Sni; using Microsoft.Data.Common; using Microsoft.Data.ProviderBase; 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 6fc1664d3f..5b91d85973 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 @@ -6,8 +6,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Net; -using System.Runtime.CompilerServices; -using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; using System.Security.Authentication; using System.Threading.Tasks; @@ -251,7 +249,6 @@ internal override void Dispose() DisposePacketCache(); } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] protected override void FreeGcHandle(int remaining, bool release) { if ((0 == remaining || release) && _gcHandle.IsAllocated) From 849acf4e62a0095751e2f740ec0e81a2f395577c Mon Sep 17 00:00:00 2001 From: Edward Neal <55035479+edwardneal@users.noreply.github.com> Date: Fri, 5 Sep 2025 21:57:04 +0100 Subject: [PATCH 9/9] Final cleanup - reference to SessionHandle, already-merged CER --- .../SqlClient/TdsParserStateObjectNative.cs | 32 +++++++------------ .../SqlClient/TdsParserStateObjectNative.cs | 32 +++++++------------ .../SSPI/NativeSspiContextProvider.cs | 6 ++-- 3 files changed, 26 insertions(+), 44 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 3b81cea250..a553b43dde 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 @@ -397,28 +397,20 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb PacketHandle temp = default; uint error = TdsEnums.SNI_SUCCESS; -#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); + 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/TdsParserStateObjectNative.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs index 5b91d85973..cb85e2b9b1 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 @@ -385,28 +385,20 @@ internal override uint PostReadAsyncForMars(TdsParserStateObject physicalStateOb PacketHandle temp = default; uint error = TdsEnums.SNI_SUCCESS; -#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); + 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/src/Microsoft/Data/SqlClient/SSPI/NativeSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NativeSspiContextProvider.cs index 1cc4af3e9c..7f57fccd54 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NativeSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NativeSspiContextProvider.cs @@ -51,12 +51,10 @@ private void LoadSSPILibrary() protected override bool GenerateContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) { -#if NETFRAMEWORK - SNIHandle handle = _physicalStateObj.Handle; -#else +#if NET Debug.Assert(_physicalStateObj.SessionHandle.Type == SessionHandle.NativeHandleType); - SNIHandle handle = _physicalStateObj.SessionHandle.NativeHandle; #endif + SNIHandle handle = _physicalStateObj.SessionHandle.NativeHandle; // This must start as the length of the input, but will be updated by the call to SNISecGenClientContext to the written length var sendLength = s_maxSSPILength;