Skip to content

Commit 42146a3

Browse files
twsouthwicktwsouthwick
authored
Ensure correct SPN when calling SspiContextProvider (#3347)
* 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. * initialization only after success * move serverSpn to be local
1 parent 8435d00 commit 42146a3

File tree

13 files changed

+193
-101
lines changed

13 files changed

+193
-101
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,9 @@
285285
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ManagedSni\ConcurrentQueueSemaphore.netcore.cs">
286286
<Link>Microsoft\Data\SqlClient\ManagedSni\ConcurrentQueueSemaphore.netcore.cs</Link>
287287
</Compile>
288+
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ManagedSni\ResolvedServerSpn.cs">
289+
<Link>Microsoft\Data\SqlClient\ManagedSni\ResolvedServerSpn.cs</Link>
290+
</Compile>
288291
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\ManagedSni\SniAsyncCallback.netcore.cs">
289292
<Link>Microsoft\Data\SqlClient\ManagedSni\SniAsyncCallback.netcore.cs</Link>
290293
</Compile>

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines changed: 27 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,6 @@ internal sealed partial class TdsParser
121121

122122
private bool _is2022 = false;
123123

124-
private string[] _serverSpn = null;
125-
126124
// SqlStatistics
127125
private SqlStatistics _statistics = null;
128126

@@ -395,7 +393,6 @@ internal void Connect(ServerInfo serverInfo,
395393
}
396394
else
397395
{
398-
_serverSpn = null;
399396
SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | Connection Object Id {0}, Authentication Mode: {1}", _connHandler.ObjectID,
400397
authType == SqlAuthenticationMethod.NotSpecified ? SqlAuthenticationMethod.SqlPassword.ToString() : authType.ToString());
401398
}
@@ -407,7 +404,6 @@ internal void Connect(ServerInfo serverInfo,
407404
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Encryption will be disabled as target server is a SQL Local DB instance.");
408405
}
409406

410-
_serverSpn = null;
411407
_authenticationProvider = null;
412408

413409
// AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
@@ -446,7 +442,7 @@ internal void Connect(ServerInfo serverInfo,
446442
serverInfo.ExtendedServerName,
447443
timeout,
448444
out instanceName,
449-
ref _serverSpn,
445+
out var resolvedServerSpn,
450446
false,
451447
true,
452448
fParallel,
@@ -459,8 +455,6 @@ internal void Connect(ServerInfo serverInfo,
459455
hostNameInCertificate,
460456
serverCertificateFilename);
461457

462-
_authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this);
463-
464458
if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
465459
{
466460
_physicalStateObj.AddError(ProcessSNIError(_physicalStateObj));
@@ -546,7 +540,7 @@ internal void Connect(ServerInfo serverInfo,
546540
serverInfo.ExtendedServerName,
547541
timeout,
548542
out instanceName,
549-
ref _serverSpn,
543+
out resolvedServerSpn,
550544
true,
551545
true,
552546
fParallel,
@@ -559,8 +553,6 @@ internal void Connect(ServerInfo serverInfo,
559553
hostNameInCertificate,
560554
serverCertificateFilename);
561555

562-
_authenticationProvider?.Initialize(serverInfo, _physicalStateObj, this);
563-
564556
if (TdsEnums.SNI_SUCCESS != _physicalStateObj.Status)
565557
{
566558
_physicalStateObj.AddError(ProcessSNIError(_physicalStateObj));
@@ -599,6 +591,11 @@ internal void Connect(ServerInfo serverInfo,
599591
}
600592
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Prelogin handshake successful");
601593

594+
if (_authenticationProvider is { })
595+
{
596+
_authenticationProvider.Initialize(serverInfo, _physicalStateObj, this, resolvedServerSpn.Primary, resolvedServerSpn.Secondary);
597+
}
598+
602599
if (_fMARS && marsCapable)
603600
{
604601
// if user explicitly disables mars or mars not supported, don't create the session pool
@@ -744,7 +741,7 @@ private void SendPreLoginHandshake(
744741

745742
// UNDONE - need to do some length verification to ensure packet does not
746743
// get too big!!! Not beyond it's max length!
747-
744+
748745
for (int option = (int)PreLoginOptions.VERSION; option < (int)PreLoginOptions.NUMOPT; option++)
749746
{
750747
int optionDataSize = 0;
@@ -935,7 +932,7 @@ private PreLoginHandshakeStatus ConsumePreLoginHandshake(
935932
string serverCertificateFilename)
936933
{
937934
// Assign default values
938-
marsCapable = _fMARS;
935+
marsCapable = _fMARS;
939936
fedAuthRequired = false;
940937
Debug.Assert(_physicalStateObj._syncOverAsync, "Should not attempt pends in a synchronous call");
941938
TdsOperationStatus result = _physicalStateObj.TryReadNetworkPacket();
@@ -2181,7 +2178,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle
21812178
dataStream.BrowseModeInfoConsumed = true;
21822179
}
21832180
else
2184-
{
2181+
{
21852182
// no dataStream
21862183
result = stateObj.TrySkipBytes(tokenLength);
21872184
if (result != TdsOperationStatus.Done)
@@ -2195,7 +2192,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle
21952192
case TdsEnums.SQLDONE:
21962193
case TdsEnums.SQLDONEPROC:
21972194
case TdsEnums.SQLDONEINPROC:
2198-
{
2195+
{
21992196
// RunBehavior can be modified - see SQL BU DT 269516 & 290090
22002197
result = TryProcessDone(cmdHandler, dataStream, ref runBehavior, stateObj);
22012198
if (result != TdsOperationStatus.Done)
@@ -4122,7 +4119,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length,
41224119
{
41234120
return result;
41244121
}
4125-
4122+
41264123
byte len;
41274124
result = stateObj.TryReadByte(out len);
41284125
if (result != TdsOperationStatus.Done)
@@ -4330,7 +4327,7 @@ internal TdsOperationStatus TryProcessReturnValue(int length,
43304327
{
43314328
return result;
43324329
}
4333-
4330+
43344331
if (rec.collation.IsUTF8)
43354332
{ // UTF8 collation
43364333
rec.encoding = s_utf8EncodingWithoutBom;
@@ -4790,13 +4787,13 @@ internal TdsOperationStatus TryProcessAltMetaData(int cColumns, TdsParserStateOb
47904787
{
47914788
// internal meta data class
47924789
_SqlMetaData col = altMetaDataSet[i];
4793-
4790+
47944791
result = stateObj.TryReadByte(out _);
47954792
if (result != TdsOperationStatus.Done)
47964793
{
47974794
return result;
47984795
}
4799-
4796+
48004797
result = stateObj.TryReadUInt16(out _);
48014798
if (result != TdsOperationStatus.Done)
48024799
{
@@ -5489,7 +5486,7 @@ private TdsOperationStatus TryProcessColInfo(_SqlMetaDataSet columns, SqlDataRea
54895486
for (int i = 0; i < columns.Length; i++)
54905487
{
54915488
_SqlMetaData col = columns[i];
5492-
5489+
54935490
TdsOperationStatus result = stateObj.TryReadByte(out _);
54945491
if (result != TdsOperationStatus.Done)
54955492
{
@@ -7394,7 +7391,7 @@ private byte[] SerializeSqlMoney(SqlMoney value, int length, TdsParserStateObjec
73947391

73957392
private void WriteSqlMoney(SqlMoney value, int length, TdsParserStateObject stateObj)
73967393
{
7397-
// UNDONE: can I use SqlMoney.ToInt64()?
7394+
// UNDONE: can I use SqlMoney.ToInt64()?
73987395
int[] bits = decimal.GetBits(value.Value);
73997396

74007397
// this decimal should be scaled by 10000 (regardless of what the incoming decimal was scaled by)
@@ -10010,7 +10007,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet
1001010007

1001110008
WriteUDTMetaData(value, names[0], names[1], names[2], stateObj);
1001210009

10013-
// UNDONE - re-org to use code below to write value!
10010+
// UNDONE - re-org to use code below to write value!
1001410011
if (!isNull)
1001510012
{
1001610013
WriteUnsignedLong((ulong)udtVal.Length, stateObj); // PLP length
@@ -12484,7 +12481,7 @@ private Task WriteUnterminatedValue(object value, MetaType type, byte scale, int
1248412481
case TdsEnums.SQLNVARCHAR:
1248512482
case TdsEnums.SQLNTEXT:
1248612483
case TdsEnums.SQLXMLTYPE:
12487-
case TdsEnums.SQLJSON:
12484+
case TdsEnums.SQLJSON:
1248812485
{
1248912486
Debug.Assert(!isDataFeed || (value is TextDataFeed || value is XmlDataFeed), "Value must be a TextReader or XmlReader");
1249012487
Debug.Assert(isDataFeed || (value is string || value is byte[]), "Value is a byte array or string");
@@ -13709,15 +13706,14 @@ private TdsOperationStatus TryProcessUDTMetaData(SqlMetaDataPriv metaData, TdsPa
1370913706
+ " _connHandler = {14}\n\t"
1371013707
+ " _fMARS = {15}\n\t"
1371113708
+ " _sessionPool = {16}\n\t"
13712-
+ " _sniSpnBuffer = {17}\n\t"
13713-
+ " _errors = {18}\n\t"
13714-
+ " _warnings = {19}\n\t"
13715-
+ " _attentionErrors = {20}\n\t"
13716-
+ " _attentionWarnings = {21}\n\t"
13717-
+ " _statistics = {22}\n\t"
13718-
+ " _statisticsIsInTransaction = {23}\n\t"
13719-
+ " _fPreserveTransaction = {24}"
13720-
+ " _fParallel = {25}"
13709+
+ " _errors = {17}\n\t"
13710+
+ " _warnings = {18}\n\t"
13711+
+ " _attentionErrors = {19}\n\t"
13712+
+ " _attentionWarnings = {20}\n\t"
13713+
+ " _statistics = {21}\n\t"
13714+
+ " _statisticsIsInTransaction = {22}\n\t"
13715+
+ " _fPreserveTransaction = {23}"
13716+
+ " _fParallel = {24}"
1372113717
;
1372213718
internal string TraceString()
1372313719
{
@@ -13740,7 +13736,6 @@ internal string TraceString()
1374013736
_connHandler == null ? "(null)" : _connHandler.ObjectID.ToString((IFormatProvider)null),
1374113737
_fMARS ? bool.TrueString : bool.FalseString,
1374213738
_sessionPool == null ? "(null)" : _sessionPool.TraceString(),
13743-
_serverSpn == null ? "(null)" : _serverSpn.Length.ToString((IFormatProvider)null),
1374413739
_physicalStateObj != null ? "(null)" : _physicalStateObj.ErrorCount.ToString((IFormatProvider)null),
1374513740
_physicalStateObj != null ? "(null)" : _physicalStateObj.WarningCount.ToString((IFormatProvider)null),
1374613741
_physicalStateObj != null ? "(null)" : _physicalStateObj.PreAttentionErrorCount.ToString((IFormatProvider)null),

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Threading.Tasks;
1212
using Microsoft.Data.Common;
1313
using Microsoft.Data.ProviderBase;
14+
using Microsoft.Data.SqlClient.ManagedSni;
1415

1516
namespace Microsoft.Data.SqlClient
1617
{
@@ -55,7 +56,7 @@ internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalCon
5556
AddError(parser.ProcessSNIError(this));
5657
ThrowExceptionAndWarning();
5758
}
58-
59+
5960
// we post a callback that represents the call to dispose; once the
6061
// object is disposed, the next callback will cause the GC Handle to
6162
// be released.
@@ -71,7 +72,7 @@ internal abstract void CreatePhysicalSNIHandle(
7172
string serverName,
7273
TimeoutTimer timeout,
7374
out byte[] instanceName,
74-
ref string[] spns,
75+
out ResolvedServerSpn resolvedSpn,
7576
bool flushCache,
7677
bool async,
7778
bool fParallel,

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ internal override void CreatePhysicalSNIHandle(
8080
string serverName,
8181
TimeoutTimer timeout,
8282
out byte[] instanceName,
83-
ref string[] spns,
83+
out ResolvedServerSpn resolvedSpn,
8484
bool flushCache,
8585
bool async,
8686
bool parallel,
@@ -93,7 +93,7 @@ internal override void CreatePhysicalSNIHandle(
9393
string hostNameInCertificate,
9494
string serverCertificateFilename)
9595
{
96-
SniHandle? sessionHandle = SniProxy.CreateConnectionHandle(serverName, timeout, out instanceName, ref spns, serverSPN,
96+
SniHandle? sessionHandle = SniProxy.CreateConnectionHandle(serverName, timeout, out instanceName, out resolvedSpn, serverSPN,
9797
flushCache, async, parallel, isIntegratedSecurity, iPAddressPreference, cachedFQDN, ref pendingDNSInfo, tlsFirst,
9898
hostNameInCertificate, serverCertificateFilename);
9999

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ internal override void CreatePhysicalSNIHandle(
155155
string serverName,
156156
TimeoutTimer timeout,
157157
out byte[] instanceName,
158-
ref string[] spns,
158+
out Microsoft.Data.SqlClient.ManagedSni.ResolvedServerSpn resolvedSpn,
159159
bool flushCache,
160160
bool async,
161161
bool fParallel,
@@ -189,7 +189,7 @@ internal override void CreatePhysicalSNIHandle(
189189

190190
_sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName,
191191
flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate);
192-
spns = new[] { serverSPN.TrimEnd() };
192+
resolvedSpn = new(serverSPN.TrimEnd());
193193
}
194194

195195
protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize)
@@ -424,7 +424,7 @@ internal override uint WaitForSSLHandShakeToComplete(out int protocolVersion)
424424
}
425425
else if (nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_CLIENT) || nativeProtocol.HasFlag(NativeProtocols.SP_PROT_SSL3_SERVER))
426426
{
427-
// SSL 2.0 and 3.0 are only referenced to log a warning, not explicitly used for connections
427+
// SSL 2.0 and 3.0 are only referenced to log a warning, not explicitly used for connections
428428
#pragma warning disable CS0618, CA5397
429429
protocolVersion = (int)SslProtocols.Ssl3;
430430
}

0 commit comments

Comments
 (0)