From b375f919eca2a5881d1c25b0f211192df5d6fba6 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Wed, 14 May 2025 13:21:40 -0700 Subject: [PATCH 01/20] Reset negotiateAuth if SNI doesn't work This change also adds some book keeping to ensure we're only using the spn that has previously generated a context once one has been created. --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- .../SSPI/NegotiateSspiContextProvider.cs | 6 ++ .../SqlClient/SSPI/SspiContextProvider.cs | 58 +++++++++++++------ .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 +- 5 files changed, 53 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 d66984d8b7..a2516ba590 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 @@ -459,7 +459,7 @@ internal void Connect(ServerInfo serverInfo, hostNameInCertificate, serverCertificateFilename); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { @@ -559,7 +559,7 @@ internal void Connect(ServerInfo serverInfo, hostNameInCertificate, serverCertificateFilename); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { 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 130094dc53..035b705c2b 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 @@ -518,7 +518,7 @@ internal void Connect(ServerInfo serverInfo, FQDNforDNSCache, hostNameInCertificate); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { @@ -612,7 +612,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ResolvedServerName, hostNameInCertificate); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 5dc52010b3..9d75d399c0 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -2,6 +2,7 @@ using System; using System.Buffers; +using System.Diagnostics; using System.Net.Security; #nullable enable @@ -17,6 +18,9 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials; _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); + + Debug.Assert(_negotiateAuth.TargetName == authParams.Resource, "SSPI resource does not match TargetName"); + var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation @@ -29,6 +33,8 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo return true; } + // Reset _negotiateAuth to be generated again for next SPN. + _negotiateAuth = null; return false; } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index ff83422f10..59b482c413 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -1,6 +1,7 @@ using System; using System.Buffers; using System.Diagnostics; +using System.Linq; #nullable enable @@ -10,14 +11,34 @@ internal abstract class SspiContextProvider { private TdsParser _parser = null!; private ServerInfo _serverInfo = null!; + + // This is used to store either a single or multiple SspiAuthenticationParameters. Since we initially have potential + // multiple SPNs, we'll start with that. However, once we've succeeded creating an SSPI context, we'll consider that + // to be the correct SPN going forward + private object? _authParams; + private protected TdsParserStateObject _physicalStateObj = null!; - internal void Initialize(ServerInfo serverInfo, TdsParserStateObject physicalStateObj, TdsParser parser) + internal void Initialize( + ServerInfo serverInfo, + TdsParserStateObject physicalStateObj, + TdsParser parser, +#if NETFRAMEWORK + string serverSpn +#else + string[] serverSpns +#endif + ) { _parser = parser; _physicalStateObj = physicalStateObj; _serverInfo = serverInfo; +#if NETFRAMEWORK + _authParams = CreateAuthParams(serverSpn); +#else + _authParams = serverSpns.Select(CreateAuthParams).ToArray(); +#endif Initialize(); } @@ -27,26 +48,25 @@ private protected virtual void Initialize() protected abstract bool GenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams); - internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter, string serverSpn) + internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) { using var _ = TrySNIEventScope.Create(nameof(SspiContextProvider)); - if (!RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, serverSpn)) + if (_authParams is SspiAuthenticationParameters authParam) { - // If we've hit here, the SSPI context provider implementation failed to generate the SSPI context. - SSPIError(SQLMessage.SSPIGenerateError(), TdsEnums.GEN_CLIENT_CONTEXT); + RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, authParam); + return; } - } - - internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter, ReadOnlySpan serverSpns) - { - using var _ = TrySNIEventScope.Create(nameof(SspiContextProvider)); - - foreach (var serverSpn in serverSpns) + else if (_authParams is SspiAuthenticationParameters[] authParams) { - if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, serverSpn)) + foreach (var p in authParams) { - return; + if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, p)) + { + // Reset the _authParams to only have a single one going forward to always call the context with that one + _authParams = p; + return; + } } } @@ -54,19 +74,23 @@ internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outg SSPIError(SQLMessage.SSPIGenerateError(), TdsEnums.GEN_CLIENT_CONTEXT); } - private bool RunGenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, string serverSpn) + private SspiAuthenticationParameters CreateAuthParams(string serverSpn) { var options = _parser.Connection.ConnectionOptions; - var authParams = new SspiAuthenticationParameters(options.DataSource, serverSpn) + + return new SspiAuthenticationParameters(options.DataSource, serverSpn) { DatabaseName = options.InitialCatalog, UserId = options.UserID, Password = options.Password, }; + } + private bool RunGenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) + { try { - SqlClientEventSource.Log.TryTraceEvent("{0}.{1} | Info | SPN={1}", GetType().FullName, nameof(GenerateSspiClientContext), serverSpn); + SqlClientEventSource.Log.TryTraceEvent("{0}.{1} | Info | SPN={1}", GetType().FullName, nameof(GenerateSspiClientContext), authParams.Resource); return GenerateSspiClientContext(incomingBlob, outgoingBlobWriter, authParams); } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index ad8226c7fe..cb6a4bcb3b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -35,7 +35,7 @@ internal void ProcessSSPI(int receivedLength) try { // make call for SSPI data - _authenticationProvider!.SSPIData(receivedBuff.AsSpan(0, receivedLength), writer, _serverSpn); + _authenticationProvider!.SSPIData(receivedBuff.AsSpan(0, receivedLength), writer); // DO NOT SEND LENGTH - TDS DOC INCORRECT! JUST SEND SSPI DATA! _physicalStateObj.WriteByteSpan(writer.WrittenSpan); @@ -175,7 +175,7 @@ internal void TdsLogin( // byte[] buffer and 0 for the int length. Debug.Assert(SniContext.Snix_Login == _physicalStateObj.SniContext, $"Unexpected SniContext. Expecting Snix_Login, actual value is '{_physicalStateObj.SniContext}'"); _physicalStateObj.SniContext = SniContext.Snix_LoginSspi; - _authenticationProvider.SSPIData(ReadOnlySpan.Empty, sspiWriter, _serverSpn); + _authenticationProvider.SSPIData(ReadOnlySpan.Empty, sspiWriter); _physicalStateObj.SniContext = SniContext.Snix_Login; From fbd62e096471b47a07f38749e990b0f880734c3c Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 13:06:47 -0700 Subject: [PATCH 02/20] initialization only after success --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 30 +++++++++---------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 6 ++-- 2 files changed, 16 insertions(+), 20 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 a2516ba590..39b36e82a8 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 @@ -459,8 +459,6 @@ internal void Connect(ServerInfo serverInfo, hostNameInCertificate, serverCertificateFilename); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); - if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -559,8 +557,6 @@ internal void Connect(ServerInfo serverInfo, hostNameInCertificate, serverCertificateFilename); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); - if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -599,6 +595,8 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); + if (_fMARS && marsCapable) { // if user explicitly disables mars or mars not supported, don't create the session pool @@ -744,7 +742,7 @@ private void SendPreLoginHandshake( // UNDONE - need to do some length verification to ensure packet does not // get too big!!! Not beyond it's max length! - + for (int option = (int)PreLoginOptions.VERSION; option < (int)PreLoginOptions.NUMOPT; option++) { int optionDataSize = 0; @@ -935,7 +933,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake( string serverCertificateFilename) { // Assign default values - marsCapable = _fMARS; + marsCapable = _fMARS; fedAuthRequired = false; Debug.Assert(_physicalStateObj._syncOverAsync, "Should not attempt pends in a synchronous call"); TdsOperationStatus result = _physicalStateObj.TryReadNetworkPacket(); @@ -2181,7 +2179,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle dataStream.BrowseModeInfoConsumed = true; } else - { + { // no dataStream result = stateObj.TrySkipBytes(tokenLength); if (result != TdsOperationStatus.Done) @@ -2195,7 +2193,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle case TdsEnums.SQLDONE: case TdsEnums.SQLDONEPROC: case TdsEnums.SQLDONEINPROC: - { + { // RunBehavior can be modified - see SQL BU DT 269516 & 290090 result = TryProcessDone(cmdHandler, dataStream, ref runBehavior, stateObj); if (result != TdsOperationStatus.Done) @@ -4122,7 +4120,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, { return result; } - + byte len; result = stateObj.TryReadByte(out len); if (result != TdsOperationStatus.Done) @@ -4321,7 +4319,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length, { return result; } - + if (rec.collation.IsUTF8) { // UTF8 collation rec.encoding = Encoding.UTF8; @@ -4776,13 +4774,13 @@ internal TdsOperationStatus TryProcessAltMetaData(int cColumns, TdsParserStateOb { // internal meta data class _SqlMetaData col = altMetaDataSet[i]; - + result = stateObj.TryReadByte(out _); if (result != TdsOperationStatus.Done) { return result; } - + result = stateObj.TryReadUInt16(out _); if (result != TdsOperationStatus.Done) { @@ -5466,7 +5464,7 @@ private TdsOperationStatus TryProcessColInfo(_SqlMetaDataSet columns, SqlDataRea for (int i = 0; i < columns.Length; i++) { _SqlMetaData col = columns[i]; - + TdsOperationStatus result = stateObj.TryReadByte(out _); if (result != TdsOperationStatus.Done) { @@ -7386,7 +7384,7 @@ private byte[] SerializeSqlMoney(SqlMoney value, int length, TdsParserStateObjec private void WriteSqlMoney(SqlMoney value, int length, TdsParserStateObject stateObj) { - // UNDONE: can I use SqlMoney.ToInt64()? + // UNDONE: can I use SqlMoney.ToInt64()? int[] bits = decimal.GetBits(value.Value); // this decimal should be scaled by 10000 (regardless of what the incoming decimal was scaled by) @@ -9906,7 +9904,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet WriteUDTMetaData(value, names[0], names[1], names[2], stateObj); - // UNDONE - re-org to use code below to write value! + // UNDONE - re-org to use code below to write value! if (!isNull) { WriteUnsignedLong((ulong)udtVal.Length, stateObj); // PLP length @@ -12340,7 +12338,7 @@ private Task WriteUnterminatedValue(object value, MetaType type, byte scale, int case TdsEnums.SQLNVARCHAR: case TdsEnums.SQLNTEXT: case TdsEnums.SQLXMLTYPE: - case TdsEnums.SQLJSON: + case TdsEnums.SQLJSON: { Debug.Assert(!isDataFeed || (value is TextDataFeed || value is XmlDataFeed), "Value must be a TextReader or XmlReader"); Debug.Assert(isDataFeed || (value is string || value is byte[]), "Value is a byte array or string"); 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 035b705c2b..1c68e51f8d 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 @@ -518,8 +518,6 @@ internal void Connect(ServerInfo serverInfo, FQDNforDNSCache, hostNameInCertificate); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); - if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -612,8 +610,6 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ResolvedServerName, hostNameInCertificate); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); - if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -648,6 +644,8 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); + if (_fMARS && marsCapable) { // if user explicitly disables mars or mars not supported, don't create the session pool From 65a6ddff6575cca6b1606a499bb18b9433e01963 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 13:13:54 -0700 Subject: [PATCH 03/20] move serverSpn to be local --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 15 +++++++-------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 17 ++++++++--------- 2 files changed, 15 insertions(+), 17 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 39b36e82a8..5eef3505b5 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 @@ -121,8 +121,6 @@ internal sealed partial class TdsParser private bool _is2022 = false; - private string[] _serverSpn = null; - // SqlStatistics private SqlStatistics _statistics = null; @@ -386,6 +384,8 @@ internal void Connect(ServerInfo serverInfo, uint sniStatus = TdsParserStateObjectFactory.Singleton.SNIStatus; + string[] serverSpn = null; + if (sniStatus != TdsEnums.SNI_SUCCESS) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -395,7 +395,7 @@ internal void Connect(ServerInfo serverInfo, } else { - _serverSpn = null; + serverSpn = null; SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); } @@ -407,7 +407,7 @@ internal void Connect(ServerInfo serverInfo, SqlClientEventSource.Log.TryTraceEvent(" Encryption will be disabled as target server is a SQL Local DB instance."); } - _serverSpn = null; + serverSpn = null; _authenticationProvider = null; // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server @@ -446,7 +446,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref _serverSpn, + ref serverSpn, false, true, fParallel, @@ -544,7 +544,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref _serverSpn, + ref serverSpn, true, true, fParallel, @@ -595,7 +595,7 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpn); if (_fMARS && marsCapable) { @@ -13585,7 +13585,6 @@ internal string TraceString() _connHandler == null ? "(null)" : _connHandler.ObjectID.ToString((IFormatProvider)null), _fMARS ? bool.TrueString : bool.FalseString, _sessionPool == null ? "(null)" : _sessionPool.TraceString(), - _serverSpn == null ? "(null)" : _serverSpn.Length.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.ErrorCount.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.WarningCount.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.PreAttentionErrorCount.ToString((IFormatProvider)null), 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 1c68e51f8d..0ebf29d669 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 @@ -121,8 +121,6 @@ internal sealed partial class TdsParser private bool _is2022 = false; - private string _serverSpn = null; - // SqlStatistics private SqlStatistics _statistics = null; @@ -396,6 +394,8 @@ internal void Connect(ServerInfo serverInfo, Debug.Fail("SNI returned status != success, but no error thrown?"); } + string serverSpn = null; + //Create LocalDB instance if necessary if (connHandler.ConnectionOptions.LocalDBInstance != null) { @@ -415,13 +415,13 @@ internal void Connect(ServerInfo serverInfo, if (!string.IsNullOrEmpty(serverInfo.ServerSPN)) { - _serverSpn = serverInfo.ServerSPN; + serverSpn = serverInfo.ServerSPN; SqlClientEventSource.Log.TryTraceEvent(" Server SPN `{0}` from the connection string is used.", serverInfo.ServerSPN); } else { // Empty signifies to interop layer that SPN needs to be generated - _serverSpn = string.Empty; + serverSpn = string.Empty; } SqlClientEventSource.Log.TryTraceEvent(" SSPI or Active Directory Authentication Library for SQL Server based integrated authentication"); @@ -429,7 +429,7 @@ internal void Connect(ServerInfo serverInfo, else { _authenticationProvider = null; - _serverSpn = null; + serverSpn = null; switch (authType) { @@ -508,7 +508,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref _serverSpn, + ref serverSpn, false, true, fParallel, @@ -600,7 +600,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref _serverSpn, + ref serverSpn, true, true, fParallel, @@ -644,7 +644,7 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, _serverSpn); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpn); if (_fMARS && marsCapable) { @@ -13698,7 +13698,6 @@ internal string TraceString() _connHandler == null ? "(null)" : _connHandler.ObjectID.ToString((IFormatProvider)null), _fMARS ? bool.TrueString : bool.FalseString, _sessionPool == null ? "(null)" : _sessionPool.TraceString(), - _serverSpn == null ? "(null)" : _serverSpn.Length.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.ErrorCount.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.WarningCount.ToString((IFormatProvider)null), _physicalStateObj != null ? "(null)" : _physicalStateObj.PreAttentionErrorCount.ToString((IFormatProvider)null), From 18cf7bd9404e6ac7ac43046c7d6ccba1ad359f8d Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 13:20:29 -0700 Subject: [PATCH 04/20] update messaging --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 17 ++++++++--------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 17 ++++++++--------- 2 files changed, 16 insertions(+), 18 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 5eef3505b5..a1388f7253 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 @@ -13554,15 +13554,14 @@ private TdsOperationStatus TryProcessUDTMetaData(SqlMetaDataPriv metaData, TdsPa + " _connHandler = {14}\n\t" + " _fMARS = {15}\n\t" + " _sessionPool = {16}\n\t" - + " _sniSpnBuffer = {17}\n\t" - + " _errors = {18}\n\t" - + " _warnings = {19}\n\t" - + " _attentionErrors = {20}\n\t" - + " _attentionWarnings = {21}\n\t" - + " _statistics = {22}\n\t" - + " _statisticsIsInTransaction = {23}\n\t" - + " _fPreserveTransaction = {24}" - + " _fParallel = {25}" + + " _errors = {17}\n\t" + + " _warnings = {18}\n\t" + + " _attentionErrors = {19}\n\t" + + " _attentionWarnings = {20}\n\t" + + " _statistics = {21}\n\t" + + " _statisticsIsInTransaction = {22}\n\t" + + " _fPreserveTransaction = {23}" + + " _fParallel = {24}" ; internal string TraceString() { 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 0ebf29d669..e81ba17b1c 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 @@ -13667,15 +13667,14 @@ internal ulong PlpBytesTotalLength(TdsParserStateObject stateObj) + " _connHandler = {14}\n\t" + " _fMARS = {15}\n\t" + " _sessionPool = {16}\n\t" - + " _sniSpnBuffer = {17}\n\t" - + " _errors = {18}\n\t" - + " _warnings = {19}\n\t" - + " _attentionErrors = {20}\n\t" - + " _attentionWarnings = {21}\n\t" - + " _statistics = {22}\n\t" - + " _statisticsIsInTransaction = {23}\n\t" - + " _fPreserveTransaction = {24}" - + " _fParallel = {25}" + + " _errors = {17}\n\t" + + " _warnings = {18}\n\t" + + " _attentionErrors = {19}\n\t" + + " _attentionWarnings = {20}\n\t" + + " _statistics = {21}\n\t" + + " _statisticsIsInTransaction = {22}\n\t" + + " _fPreserveTransaction = {23}" + + " _fParallel = {24}" ; internal string TraceString() { From b448eb0ad3106125e83a43ebf50d0d9dfce36626 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 14:05:58 -0700 Subject: [PATCH 05/20] reduce unnecessary assignments --- .../netcore/src/Microsoft/Data/SqlClient/TdsParser.cs | 6 ++---- .../netfx/src/Microsoft/Data/SqlClient/TdsParser.cs | 1 - 2 files changed, 2 insertions(+), 5 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 a1388f7253..3c308fb883 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 @@ -384,8 +384,6 @@ internal void Connect(ServerInfo serverInfo, uint sniStatus = TdsParserStateObjectFactory.Singleton.SNIStatus; - string[] serverSpn = null; - if (sniStatus != TdsEnums.SNI_SUCCESS) { _physicalStateObj.AddError(ProcessSNIError(_physicalStateObj)); @@ -395,7 +393,6 @@ internal void Connect(ServerInfo serverInfo, } else { - serverSpn = null; SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID, authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString()); } @@ -407,7 +404,6 @@ internal void Connect(ServerInfo serverInfo, SqlClientEventSource.Log.TryTraceEvent(" Encryption will be disabled as target server is a SQL Local DB instance."); } - serverSpn = null; _authenticationProvider = null; // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server @@ -441,6 +437,8 @@ internal void Connect(ServerInfo serverInfo, _connHandler.pendingSQLDNSObject = null; + string[] serverSpn = null; + // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server _physicalStateObj.CreatePhysicalSNIHandle( serverInfo.ExtendedServerName, 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 e81ba17b1c..9c4bd4eafe 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 @@ -429,7 +429,6 @@ internal void Connect(ServerInfo serverInfo, else { _authenticationProvider = null; - serverSpn = null; switch (authType) { From 43dff4d2c8b091ccbcbf926c09bc88403d728ef8 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 17:21:46 -0700 Subject: [PATCH 06/20] make sure to raise error if sspi not created --- .../SSPI/NegotiateSspiContextProvider.cs | 2 +- .../Data/SqlClient/SSPI/SspiContextProvider.cs | 15 +++++++-------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 4 ++-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 9d75d399c0..3490b7c38a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -11,7 +11,7 @@ namespace Microsoft.Data.SqlClient { internal sealed class NegotiateSspiContextProvider : SspiContextProvider { - private NegotiateAuthentication? _negotiateAuth = null; + private NegotiateAuthentication? _negotiateAuth; protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index 59b482c413..586935d21c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -15,7 +15,7 @@ internal abstract class SspiContextProvider // This is used to store either a single or multiple SspiAuthenticationParameters. Since we initially have potential // multiple SPNs, we'll start with that. However, once we've succeeded creating an SSPI context, we'll consider that // to be the correct SPN going forward - private object? _authParams; + private object? _authParamValue; private protected TdsParserStateObject _physicalStateObj = null!; @@ -35,9 +35,9 @@ string[] serverSpns _serverInfo = serverInfo; #if NETFRAMEWORK - _authParams = CreateAuthParams(serverSpn); + _authParamValue = CreateAuthParams(serverSpn); #else - _authParams = serverSpns.Select(CreateAuthParams).ToArray(); + _authParamValue = serverSpns.Select(CreateAuthParams).ToArray(); #endif Initialize(); } @@ -48,23 +48,22 @@ private protected virtual void Initialize() protected abstract bool GenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams); - internal void SSPIData(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) + internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) { using var _ = TrySNIEventScope.Create(nameof(SspiContextProvider)); - if (_authParams is SspiAuthenticationParameters authParam) + if (_authParamValue is SspiAuthenticationParameters authParam && RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, authParam)) { - RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, authParam); return; } - else if (_authParams is SspiAuthenticationParameters[] authParams) + else if (_authParamValue is SspiAuthenticationParameters[] authParams) { foreach (var p in authParams) { if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, p)) { // Reset the _authParams to only have a single one going forward to always call the context with that one - _authParams = p; + _authParamValue = p; return; } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index cb6a4bcb3b..675a1483d4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -35,7 +35,7 @@ internal void ProcessSSPI(int receivedLength) try { // make call for SSPI data - _authenticationProvider!.SSPIData(receivedBuff.AsSpan(0, receivedLength), writer); + _authenticationProvider!.WriteSSPIContext(receivedBuff.AsSpan(0, receivedLength), writer); // DO NOT SEND LENGTH - TDS DOC INCORRECT! JUST SEND SSPI DATA! _physicalStateObj.WriteByteSpan(writer.WrittenSpan); @@ -175,7 +175,7 @@ internal void TdsLogin( // byte[] buffer and 0 for the int length. Debug.Assert(SniContext.Snix_Login == _physicalStateObj.SniContext, $"Unexpected SniContext. Expecting Snix_Login, actual value is '{_physicalStateObj.SniContext}'"); _physicalStateObj.SniContext = SniContext.Snix_LoginSspi; - _authenticationProvider.SSPIData(ReadOnlySpan.Empty, sspiWriter); + _authenticationProvider.WriteSSPIContext(ReadOnlySpan.Empty, sspiWriter); _physicalStateObj.SniContext = SniContext.Snix_Login; From 7e974cfe5b7454822970291a87f72702eed2cc73 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 15 May 2025 17:24:15 -0700 Subject: [PATCH 07/20] spelling --- .../src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index 586935d21c..21583d4ca3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -12,7 +12,7 @@ internal abstract class SspiContextProvider private TdsParser _parser = null!; private ServerInfo _serverInfo = null!; - // This is used to store either a single or multiple SspiAuthenticationParameters. Since we initially have potential + // This is used to store either a single or multiple SspiAuthenticationParameters. Since we initially have potentially // multiple SPNs, we'll start with that. However, once we've succeeded creating an SSPI context, we'll consider that // to be the correct SPN going forward private object? _authParamValue; From f6feaf990e266271dfccb531c3ce9de861f98147 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 16 May 2025 08:09:25 -0700 Subject: [PATCH 08/20] per feedback --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 8 ++-- .../SSPI/NegotiateSspiContextProvider.cs | 2 +- .../SqlClient/SSPI/SspiContextProvider.cs | 45 ++++++++++++------- 3 files changed, 35 insertions(+), 20 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 3c308fb883..b3e7d2044b 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 @@ -437,14 +437,14 @@ internal void Connect(ServerInfo serverInfo, _connHandler.pendingSQLDNSObject = null; - string[] serverSpn = null; + string[] serverSpns = null; // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server _physicalStateObj.CreatePhysicalSNIHandle( serverInfo.ExtendedServerName, timeout, out instanceName, - ref serverSpn, + ref serverSpns, false, true, fParallel, @@ -542,7 +542,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref serverSpn, + ref serverSpns, true, true, fParallel, @@ -593,7 +593,7 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpn); + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpns); if (_fMARS && marsCapable) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 3490b7c38a..5a52bbc20f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -19,7 +19,7 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); - Debug.Assert(_negotiateAuth.TargetName == authParams.Resource, "SSPI resource does not match TargetName"); + Debug.Assert(_negotiateAuth.TargetName == authParams.Resource, "SSPI resource does not match TargetName. SspiContextProvider should ensure that once a target is established it will only call with that."); var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out statusCode)!; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index 21583d4ca3..cf009d4654 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -1,5 +1,6 @@ using System; using System.Buffers; +using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -12,10 +13,8 @@ internal abstract class SspiContextProvider private TdsParser _parser = null!; private ServerInfo _serverInfo = null!; - // This is used to store either a single or multiple SspiAuthenticationParameters. Since we initially have potentially - // multiple SPNs, we'll start with that. However, once we've succeeded creating an SSPI context, we'll consider that - // to be the correct SPN going forward - private object? _authParamValue; + private List? _authParams; + private SspiAuthenticationParameters? _authParam; private protected TdsParserStateObject _physicalStateObj = null!; @@ -35,9 +34,9 @@ string[] serverSpns _serverInfo = serverInfo; #if NETFRAMEWORK - _authParamValue = CreateAuthParams(serverSpn); + _authParam = CreateAuthParams(serverSpn); #else - _authParamValue = serverSpns.Select(CreateAuthParams).ToArray(); + _authParams = [.. serverSpns.Select(CreateAuthParams)]; #endif Initialize(); } @@ -52,25 +51,41 @@ internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter + /// If we only have a single auth param, we know it's the correct one to use. + /// + private bool TryRunSingle(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) + { + return _authParam is { } && RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, _authParam); + } + + /// + /// If we have multiple, we need to loop through them, and then identify the correct one for future use. + /// + private bool TryRunMultiple(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) + { + if (_authParams is { }) { - foreach (var p in authParams) + foreach (var authParam in _authParams) { - if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, p)) + if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, authParam)) { // Reset the _authParams to only have a single one going forward to always call the context with that one - _authParamValue = p; - return; + _authParam = authParam; + _authParams = null; + return true; } } } - // If we've hit here, the SSPI context provider implementation failed to generate the SSPI context. - SSPIError(SQLMessage.SSPIGenerateError(), TdsEnums.GEN_CLIENT_CONTEXT); + return false; } private SspiAuthenticationParameters CreateAuthParams(string serverSpn) From a2713c93488f0a45b890e2c17c1c167e2e1fbd74 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 16 May 2025 09:32:01 -0700 Subject: [PATCH 09/20] fix logic --- .../src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index cf009d4654..d6a07a40c8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -51,7 +51,7 @@ internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter Date: Fri, 16 May 2025 12:45:47 -0700 Subject: [PATCH 10/20] only use second --- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 5 ++- .../SqlClient/SSPI/SspiContextProvider.cs | 44 +------------------ 2 files changed, 6 insertions(+), 43 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 b3e7d2044b..5078eb6f5f 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 @@ -593,7 +593,10 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpns); + // We need to initialize the authentication provider with the server SPN + // This array will either be a single entry with the SPN or two entries with the second + // one being including a default port. + _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpns[^1]); if (_fMARS && marsCapable) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index d6a07a40c8..ba1d6700a7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -1,8 +1,6 @@ using System; using System.Buffers; -using System.Collections.Generic; using System.Diagnostics; -using System.Linq; #nullable enable @@ -13,7 +11,6 @@ internal abstract class SspiContextProvider private TdsParser _parser = null!; private ServerInfo _serverInfo = null!; - private List? _authParams; private SspiAuthenticationParameters? _authParam; private protected TdsParserStateObject _physicalStateObj = null!; @@ -22,22 +19,15 @@ internal void Initialize( ServerInfo serverInfo, TdsParserStateObject physicalStateObj, TdsParser parser, -#if NETFRAMEWORK string serverSpn -#else - string[] serverSpns -#endif ) { _parser = parser; _physicalStateObj = physicalStateObj; _serverInfo = serverInfo; -#if NETFRAMEWORK _authParam = CreateAuthParams(serverSpn); -#else - _authParams = [.. serverSpns.Select(CreateAuthParams)]; -#endif + Initialize(); } @@ -51,43 +41,13 @@ internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter - /// If we only have a single auth param, we know it's the correct one to use. - /// - private bool TryRunSingle(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) - { - return _authParam is { } && RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, _authParam); - } - - /// - /// If we have multiple, we need to loop through them, and then identify the correct one for future use. - /// - private bool TryRunMultiple(ReadOnlySpan receivedBuff, IBufferWriter outgoingBlobWriter) - { - if (_authParams is { }) - { - foreach (var authParam in _authParams) - { - if (RunGenerateSspiClientContext(receivedBuff, outgoingBlobWriter, authParam)) - { - // Reset the _authParams to only have a single one going forward to always call the context with that one - _authParam = authParam; - _authParams = null; - return true; - } - } - } - - return false; - } - private SspiAuthenticationParameters CreateAuthParams(string serverSpn) { var options = _parser.Connection.ConnectionOptions; From 806b059d6d549e3290dbe6e7e6e1676d46e102cf Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 16 May 2025 14:45:32 -0700 Subject: [PATCH 11/20] only return the required resolved spn --- .../Microsoft/Data/SqlClient/SNI/SNIProxy.cs | 17 +++++++++-------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 14 ++++++-------- .../SqlClient/TdsParserStateObject.netcore.cs | 4 ++-- .../SqlClient/TdsParserStateObjectManaged.cs | 4 ++-- .../SqlClient/TdsParserStateObjectNative.cs | 6 +++--- 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs index 265f80246c..3a785ac378 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs @@ -34,7 +34,7 @@ internal class SNIProxy /// Full server name from connection string /// Timer expiration /// Instance name - /// SPNs + /// SPN /// pre-defined SPN /// Flush packet cache /// Asynchronous connection @@ -51,7 +51,7 @@ internal static SNIHandle CreateConnectionHandle( string fullServerName, TimeoutTimer timeout, out byte[] instanceName, - ref string[] spns, + out string resolvedSpn, string serverSPN, bool flushCache, bool async, @@ -65,6 +65,7 @@ internal static SNIHandle CreateConnectionHandle( string serverCertificateFilename) { instanceName = new byte[1]; + resolvedSpn = default; bool errorWithLocalDBProcessing; string localDBDataSource = GetLocalDBDataSource(fullServerName, out errorWithLocalDBProcessing); @@ -103,7 +104,7 @@ internal static SNIHandle CreateConnectionHandle( { try { - spns = GetSqlServerSPNs(details, serverSPN); + resolvedSpn = GetSqlServerSPNs(details, serverSPN); } catch (Exception e) { @@ -115,12 +116,12 @@ internal static SNIHandle CreateConnectionHandle( return sniHandle; } - private static string[] GetSqlServerSPNs(DataSource dataSource, string serverSPN) + private static string GetSqlServerSPNs(DataSource dataSource, string serverSPN) { Debug.Assert(!string.IsNullOrWhiteSpace(dataSource.ServerName)); if (!string.IsNullOrWhiteSpace(serverSPN)) { - return new[] { serverSPN }; + return serverSPN; } string hostName = dataSource.ServerName; @@ -138,7 +139,7 @@ private static string[] GetSqlServerSPNs(DataSource dataSource, string serverSPN return GetSqlServerSPNs(hostName, postfix, dataSource.ResolvedProtocol); } - private static string[] GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) + private static string GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) { Debug.Assert(!string.IsNullOrWhiteSpace(hostNameOrAddress)); IPHostEntry hostEntry = null; @@ -169,12 +170,12 @@ private static string[] GetSqlServerSPNs(string hostNameOrAddress, string portOr string serverSpnWithDefaultPort = serverSpn + $":{DefaultSqlServerPort}"; // Set both SPNs with and without Port as Port is optional for default instance SqlClientEventSource.Log.TryAdvancedTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerSPNs {0} and {1}", serverSpn, serverSpnWithDefaultPort); - return new[] { serverSpn, serverSpnWithDefaultPort }; + return serverSpnWithDefaultPort; } // else Named Pipes do not need to valid port SqlClientEventSource.Log.TryAdvancedTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerSPN {0}", serverSpn); - return new[] { serverSpn }; + return serverSpn; } /// 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 5078eb6f5f..4d5c6bcd95 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 @@ -437,14 +437,12 @@ internal void Connect(ServerInfo serverInfo, _connHandler.pendingSQLDNSObject = null; - string[] serverSpns = null; - // AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server _physicalStateObj.CreatePhysicalSNIHandle( serverInfo.ExtendedServerName, timeout, out instanceName, - ref serverSpns, + out var serverSpn, false, true, fParallel, @@ -542,7 +540,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - ref serverSpns, + out serverSpn, true, true, fParallel, @@ -593,10 +591,10 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - // We need to initialize the authentication provider with the server SPN - // This array will either be a single entry with the SPN or two entries with the second - // one being including a default port. - _authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this, serverSpns[^1]); + if (_authenticationProvider is { } && serverSpn is { }) + { + _authenticationProvider.Initialize(serverInfo, _physicalStateObj, this, serverSpn); + } if (_fMARS && marsCapable) { 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 86e077e9f9..9307b4fad5 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 @@ -62,7 +62,7 @@ internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCon AddError(parser.ProcessSNIError(this)); ThrowExceptionAndWarning(); } - + // we post a callback that represents the call to dispose; once the // object is disposed, the next callback will cause the GC Handle to // be released. @@ -181,7 +181,7 @@ internal abstract void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - ref string[] spns, + out string resolvedSpn, bool flushCache, bool async, bool fParallel, 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 e6dddc79f9..5e5fafbd5f 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 @@ -81,7 +81,7 @@ internal override void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - ref string[] spns, + out string resolvedSpn, bool flushCache, bool async, bool parallel, @@ -94,7 +94,7 @@ internal override void CreatePhysicalSNIHandle( string hostNameInCertificate, string serverCertificateFilename) { - SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, timeout, out instanceName, ref spns, serverSPN, + SNIHandle? sessionHandle = SNIProxy.CreateConnectionHandle(serverName, timeout, out instanceName, out resolvedSpn, serverSPN, flushCache, async, parallel, isIntegratedSecurity, iPAddressPreference, cachedFQDN, ref pendingDNSInfo, tlsFirst, hostNameInCertificate, serverCertificateFilename); 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 b8d1b6cccb..087c6cbc92 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 @@ -144,7 +144,7 @@ internal override void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - ref string[] spns, + out string resolvedSpn, bool flushCache, bool async, bool fParallel, @@ -178,7 +178,7 @@ internal override void CreatePhysicalSNIHandle( _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName, flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate); - spns = new[] { serverSPN.TrimEnd() }; + resolvedSpn = serverSPN.TrimEnd(); } protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) @@ -423,7 +423,7 @@ internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion) } 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 + // 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; } From 9f2de24f6cb68e0b9a1246d77080cddec9dffefd Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 16 May 2025 14:48:54 -0700 Subject: [PATCH 12/20] remove extra --- .../Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 5a52bbc20f..93280353f1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -19,8 +19,6 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); - Debug.Assert(_negotiateAuth.TargetName == authParams.Resource, "SSPI resource does not match TargetName. SspiContextProvider should ensure that once a target is established it will only call with that."); - var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out statusCode)!; // Log session id, status code and the actual SPN used in the negotiation @@ -33,8 +31,6 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo return true; } - // Reset _negotiateAuth to be generated again for next SPN. - _negotiateAuth = null; return false; } } From c31ef9759216011a1a57e6729e574d0ea9988200 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 16 May 2025 14:49:14 -0700 Subject: [PATCH 13/20] remove trace --- .../Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 93280353f1..cdee407549 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -31,6 +31,9 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo return true; } + _negotiateAuth.Dispose(); + _negotiateAuth = null; + return false; } } From 45f7dee2660f5f6a23e6ff92bde0fbd9746f4080 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 19 May 2025 08:55:57 -0700 Subject: [PATCH 14/20] fix reflection test --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 9f6673332c..910b1370d8 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -212,10 +212,8 @@ private static string GetSPNInfo(string dataSource, string inInstanceName) string serverSPN = ""; MethodInfo getSqlServerSPNs = sniProxyObj.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); - string[] result = (string[])getSqlServerSPNs.Invoke(sniProxyObj, new object[] { dataSrcInfo, serverSPN }); - - string spnInfo = result[0]; - + string spnInfo = (string)getSqlServerSPNs.Invoke(sniProxyObj, new object[] { dataSrcInfo, serverSPN }); + return spnInfo; } From e11075764e80616eed2a6068bea24343a8402b1b Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Mon, 19 May 2025 09:20:10 -0700 Subject: [PATCH 15/20] inline --- .../SqlClient/SSPI/SspiContextProvider.cs | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index ba1d6700a7..5fcd19b3c5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -26,7 +26,14 @@ string serverSpn _physicalStateObj = physicalStateObj; _serverInfo = serverInfo; - _authParam = CreateAuthParams(serverSpn); + var options = parser.Connection.ConnectionOptions; + + _authParam = new SspiAuthenticationParameters(options.DataSource, serverSpn) + { + DatabaseName = options.InitialCatalog, + UserId = options.UserID, + Password = options.Password, + }; Initialize(); } @@ -48,18 +55,6 @@ internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) { try From 151ee2cbd49baf0234bd113edc7ccd34783ee452 Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Wed, 21 May 2025 08:46:47 -0700 Subject: [PATCH 16/20] log and clean up --- .../Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs | 3 --- .../src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index cdee407549..93280353f1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -31,9 +31,6 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo return true; } - _negotiateAuth.Dispose(); - _negotiateAuth = null; - return false; } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index 5fcd19b3c5..ff9361e1d5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -28,6 +28,8 @@ string serverSpn var options = parser.Connection.ConnectionOptions; + SqlClientEventSource.Log.StateDumpEvent(" Initializing provider {0} with SPN={1}", GetType().FullName, serverSpn); + _authParam = new SspiAuthenticationParameters(options.DataSource, serverSpn) { DatabaseName = options.InitialCatalog, From ce3a5c389ca8b7b4c222d595104d0e4e1b460fec Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Thu, 22 May 2025 14:45:56 -0700 Subject: [PATCH 17/20] per discussion --- .../src/Microsoft.Data.SqlClient.csproj | 1 + .../Data/SqlClient/SNI/ResolvedServerSpn.cs | 43 +++++++++++++++ .../Microsoft/Data/SqlClient/SNI/SNIProxy.cs | 15 ++--- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 8 +-- .../SqlClient/TdsParserStateObject.netcore.cs | 3 +- .../SqlClient/TdsParserStateObjectManaged.cs | 2 +- .../SqlClient/TdsParserStateObjectNative.cs | 5 +- .../SSPI/NegotiateSspiContextProvider.cs | 25 ++++++++- .../SqlClient/SSPI/SspiContextProvider.cs | 55 +++++++++++++++---- .../Data/SqlClient/SqlClientEventSource.cs | 6 ++ 10 files changed, 133 insertions(+), 30 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ResolvedServerSpn.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index eb556a79ec..dddedf29fd 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -730,6 +730,7 @@ + diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ResolvedServerSpn.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ResolvedServerSpn.cs new file mode 100644 index 0000000000..bbf161b66e --- /dev/null +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ResolvedServerSpn.cs @@ -0,0 +1,43 @@ +// 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. + +#nullable enable + +namespace Microsoft.Data.SqlClient.SNI +{ + /// + /// This is used to hold the ServerSpn for a given connection. Most connection types have a single format, although TCP connections may allow + /// with and without a port. Depending on how the SPN is registered on the server, either one may be the correct name. + /// + /// + /// + /// + /// + /// SQL Server SPN format follows these patterns: + /// + /// + /// Default instance, no port (primary): + /// MSSQLSvc/fully-qualified-domain-name + /// + /// + /// Default instance, default port (secondary): + /// MSSQLSvc/fully-qualified-domain-name:1433 + /// + /// + /// Named instance or custom port: + /// MSSQLSvc/fully-qualified-domain-name:port_or_instance_name + /// + /// + /// For TCP connections to named instances, the port number is used in SPN. + /// For Named Pipe connections to named instances, the instance name is used in SPN. + /// When hostname resolution fails, the user-provided hostname is used instead of FQDN. + /// For default instances with TCP protocol, both forms (with and without port) may be returned. + /// + internal readonly struct ResolvedServerSpn(string primary, string? secondary = null) + { + public string Primary => primary; + + public string? Secondary => secondary; + } +} diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs index 3a785ac378..59491bf8b4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs @@ -3,12 +3,9 @@ // See the LICENSE file in the project root for more information. using System; -using System.Buffers; -using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Net; -using System.Net.Security; using System.Net.Sockets; using System.Text; using Microsoft.Data.ProviderBase; @@ -51,7 +48,7 @@ internal static SNIHandle CreateConnectionHandle( string fullServerName, TimeoutTimer timeout, out byte[] instanceName, - out string resolvedSpn, + out ResolvedServerSpn resolvedSpn, string serverSPN, bool flushCache, bool async, @@ -116,12 +113,12 @@ internal static SNIHandle CreateConnectionHandle( return sniHandle; } - private static string GetSqlServerSPNs(DataSource dataSource, string serverSPN) + private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string serverSPN) { Debug.Assert(!string.IsNullOrWhiteSpace(dataSource.ServerName)); if (!string.IsNullOrWhiteSpace(serverSPN)) { - return serverSPN; + return new(serverSPN); } string hostName = dataSource.ServerName; @@ -139,7 +136,7 @@ private static string GetSqlServerSPNs(DataSource dataSource, string serverSPN) return GetSqlServerSPNs(hostName, postfix, dataSource.ResolvedProtocol); } - private static string GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) + private static ResolvedServerSpn GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol) { Debug.Assert(!string.IsNullOrWhiteSpace(hostNameOrAddress)); IPHostEntry hostEntry = null; @@ -170,12 +167,12 @@ private static string GetSqlServerSPNs(string hostNameOrAddress, string portOrIn string serverSpnWithDefaultPort = serverSpn + $":{DefaultSqlServerPort}"; // Set both SPNs with and without Port as Port is optional for default instance SqlClientEventSource.Log.TryAdvancedTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerSPNs {0} and {1}", serverSpn, serverSpnWithDefaultPort); - return serverSpnWithDefaultPort; + return new(serverSpn, serverSpnWithDefaultPort); } // else Named Pipes do not need to valid port SqlClientEventSource.Log.TryAdvancedTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerSPN {0}", serverSpn); - return serverSpn; + return new(serverSpn); } /// 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 4d5c6bcd95..7b947e4ce5 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 @@ -442,7 +442,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - out var serverSpn, + out var resolvedServerSpn, false, true, fParallel, @@ -540,7 +540,7 @@ internal void Connect(ServerInfo serverInfo, serverInfo.ExtendedServerName, timeout, out instanceName, - out serverSpn, + out resolvedServerSpn, true, true, fParallel, @@ -591,9 +591,9 @@ internal void Connect(ServerInfo serverInfo, } SqlClientEventSource.Log.TryTraceEvent(" Prelogin handshake successful"); - if (_authenticationProvider is { } && serverSpn is { }) + if (_authenticationProvider is { }) { - _authenticationProvider.Initialize(serverInfo, _physicalStateObj, this, serverSpn); + _authenticationProvider.Initialize(serverInfo, _physicalStateObj, this, resolvedServerSpn.Primary, resolvedServerSpn.Secondary); } if (_fMARS && marsCapable) 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 5d2525b78d..d0f6225831 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,6 +11,7 @@ using System.Threading.Tasks; using Microsoft.Data.Common; using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.SNI; namespace Microsoft.Data.SqlClient { @@ -71,7 +72,7 @@ internal abstract void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - out string resolvedSpn, + out ResolvedServerSpn resolvedSpn, bool flushCache, bool async, bool fParallel, 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 5e5fafbd5f..706096165d 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 @@ -81,7 +81,7 @@ internal override void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - out string resolvedSpn, + out ResolvedServerSpn resolvedSpn, bool flushCache, bool async, bool parallel, 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 087c6cbc92..7bbd000160 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 @@ -13,6 +13,7 @@ using Interop.Windows.Sni; using Microsoft.Data.Common; using Microsoft.Data.ProviderBase; +using Microsoft.Data.SqlClient.SNI; namespace Microsoft.Data.SqlClient { @@ -144,7 +145,7 @@ internal override void CreatePhysicalSNIHandle( string serverName, TimeoutTimer timeout, out byte[] instanceName, - out string resolvedSpn, + out ResolvedServerSpn resolvedSpn, bool flushCache, bool async, bool fParallel, @@ -178,7 +179,7 @@ internal override void CreatePhysicalSNIHandle( _sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName, flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate); - resolvedSpn = serverSPN.TrimEnd(); + resolvedSpn = new(serverSPN.TrimEnd()); } protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 93280353f1..a448721e68 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -9,7 +9,7 @@ namespace Microsoft.Data.SqlClient { - internal sealed class NegotiateSspiContextProvider : SspiContextProvider + internal sealed class NegotiateSspiContextProvider : SspiContextProvider, IDisposable { private NegotiateAuthentication? _negotiateAuth; @@ -17,7 +17,7 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo { NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials; - _negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); + _negotiateAuth = GetNegotiateAuthenticationForParams(authParams); var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out statusCode)!; @@ -33,6 +33,27 @@ protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlo return false; } + + public void Dispose() + { + _negotiateAuth?.Dispose(); + } + + private NegotiateAuthentication GetNegotiateAuthenticationForParams(SspiAuthenticationParameters authParams) + { + if (_negotiateAuth is { }) + { + if (string.Equals(_negotiateAuth.TargetName, authParams.Resource, StringComparison.Ordinal)) + { + return _negotiateAuth; + } + + // Dispose of it since we're not going to use it now + _negotiateAuth?.Dispose(); + } + + return _negotiateAuth = new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); + } } } #endif diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs index ff9361e1d5..e246d58d38 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiContextProvider.cs @@ -11,15 +11,22 @@ internal abstract class SspiContextProvider private TdsParser _parser = null!; private ServerInfo _serverInfo = null!; - private SspiAuthenticationParameters? _authParam; + private SspiAuthenticationParameters? _primaryAuthParams; + private SspiAuthenticationParameters? _secondaryAuthParams; private protected TdsParserStateObject _physicalStateObj = null!; +#if NET + /// + /// for details as to what and means and why there are two. + /// +#endif internal void Initialize( ServerInfo serverInfo, TdsParserStateObject physicalStateObj, TdsParser parser, - string serverSpn + string primaryServerSpn, + string? secondaryServerSpn = null ) { _parser = parser; @@ -28,16 +35,23 @@ string serverSpn var options = parser.Connection.ConnectionOptions; - SqlClientEventSource.Log.StateDumpEvent(" Initializing provider {0} with SPN={1}", GetType().FullName, serverSpn); + SqlClientEventSource.Log.StateDumpEvent(" Initializing provider {0} with SPN={1} and alternate={2}", GetType().FullName, primaryServerSpn, secondaryServerSpn); - _authParam = new SspiAuthenticationParameters(options.DataSource, serverSpn) + _primaryAuthParams = CreateAuthParams(options, primaryServerSpn); + + if (secondaryServerSpn is { }) { - DatabaseName = options.InitialCatalog, - UserId = options.UserID, - Password = options.Password, - }; + _secondaryAuthParams = CreateAuthParams(options, secondaryServerSpn); + } Initialize(); + + static SspiAuthenticationParameters CreateAuthParams(SqlConnectionString connString, string serverSpn) => new(connString.DataSource, serverSpn) + { + DatabaseName = connString.InitialCatalog, + UserId = connString.UserID, + Password = connString.Password, + }; } private protected virtual void Initialize() @@ -50,11 +64,30 @@ internal void WriteSSPIContext(ReadOnlySpan receivedBuff, IBufferWriter incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs index 90a69b5670..dade5e4fce 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs @@ -893,6 +893,12 @@ internal void StateDumpEvent(string message, T0 args0, T1 args1) { StateDump(string.Format(message, args0?.ToString() ?? NullStr, args1?.ToString() ?? NullStr)); } + + [NonEvent] + internal void StateDumpEvent(string message, T0 args0, T1 args1, T2 args2) + { + StateDump(string.Format(message, args0?.ToString() ?? NullStr, args1?.ToString() ?? NullStr, args2?.ToString())); + } #endregion #region SNI Trace From 0142ecb2dd34bfd1aedc5d1ea43702b5613f214a Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 23 May 2025 09:51:47 -0700 Subject: [PATCH 18/20] per feedback --- .../src/Microsoft/Data/SqlClient/SqlClientEventSource.cs | 2 +- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs index dade5e4fce..835ceee88f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs @@ -897,7 +897,7 @@ internal void StateDumpEvent(string message, T0 args0, T1 args1) [NonEvent] internal void StateDumpEvent(string message, T0 args0, T1 args1, T2 args2) { - StateDump(string.Format(message, args0?.ToString() ?? NullStr, args1?.ToString() ?? NullStr, args2?.ToString())); + StateDump(string.Format(message, args0?.ToString() ?? NullStr, args1?.ToString() ?? NullStr, args2?.ToString() ?? NullStr)); } #endregion diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 910b1370d8..ee1905e914 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -212,8 +212,10 @@ private static string GetSPNInfo(string dataSource, string inInstanceName) string serverSPN = ""; MethodInfo getSqlServerSPNs = sniProxyObj.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); - string spnInfo = (string)getSqlServerSPNs.Invoke(sniProxyObj, new object[] { dataSrcInfo, serverSPN }); - + object resolvedSpns = getSqlServerSPNs.Invoke(sniProxyObj, new object[] { dataSrcInfo, serverSPN }); + + string spnInfo = (string)resolvedSpns.GetType().GetProperty("Primary", BindingFlags.Instance | BindingFlags.Public).GetValue(resolvedSpns); + return spnInfo; } From 6a834565d4b8cb2045874a5a95e1529d31d8346c Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 23 May 2025 09:52:14 -0700 Subject: [PATCH 19/20] remove unnecessary ? --- .../Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index a448721e68..2a90f8b2b3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -49,7 +49,7 @@ private NegotiateAuthentication GetNegotiateAuthenticationForParams(SspiAuthenti } // Dispose of it since we're not going to use it now - _negotiateAuth?.Dispose(); + _negotiateAuth.Dispose(); } return _negotiateAuth = new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource }); From 3ca3de6fa49ace06aa2163026ae788e996f974dd Mon Sep 17 00:00:00 2001 From: Taylor Southwick Date: Fri, 23 May 2025 15:48:10 -0700 Subject: [PATCH 20/20] copy locally --- .../Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs index 2a90f8b2b3..a74651cf2d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs @@ -15,15 +15,12 @@ internal sealed class NegotiateSspiContextProvider : SspiContextProvider, IDispo protected override bool GenerateSspiClientContext(ReadOnlySpan incomingBlob, IBufferWriter outgoingBlobWriter, SspiAuthenticationParameters authParams) { - NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials; - - _negotiateAuth = GetNegotiateAuthenticationForParams(authParams); - - var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out statusCode)!; + var negotiateAuth = GetNegotiateAuthenticationForParams(authParams); + var sendBuff = negotiateAuth.GetOutgoingBlob(incomingBlob, out var statusCode)!; // Log session id, status code and the actual SPN used in the negotiation SqlClientEventSource.Log.TryTraceEvent("{0}.{1} | Info | Session Id {2}, StatusCode={3}, SPN={4}", nameof(NegotiateSspiContextProvider), - nameof(GenerateSspiClientContext), _physicalStateObj.SessionId, statusCode, _negotiateAuth.TargetName); + nameof(GenerateSspiClientContext), _physicalStateObj.SessionId, statusCode, negotiateAuth.TargetName); if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded) {