Skip to content

Expose SspiAuthenticationParameters on SspiContextProvider #2454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
May 13, 2025
Merged

Expose SspiAuthenticationParameters on SspiContextProvider #2454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8adfef4
Pass SqlAuthenticationParameters in GenerateSspiClientContext
twsouthwick Feb 1, 2025
bf59162
Merge branch 'main' into auth-params
twsouthwick Mar 13, 2025
72f1c99
Remove comment and whitespace changes
twsouthwick Mar 14, 2025
7f1f800
use net framework logging
twsouthwick Mar 17, 2025
e9c04ad
consolidate builder for sqlauthparameters and use serverSpn as resource
twsouthwick Mar 17, 2025
a0f5c7a
Merge remote-tracking branch 'origin/main' into auth-params
twsouthwick May 1, 2025
7c9b0f3
create separate SspiAuthenticationParameters
twsouthwick May 1, 2025
454da94
rename to resource
twsouthwick May 1, 2025
4e07c89
revert
twsouthwick May 1, 2025
9536950
add resource
twsouthwick May 1, 2025
a813d22
rename to SspiContextProvider
twsouthwick May 1, 2025
ca8fbea
Merge remote-tracking branch 'origin/main' into auth-params
twsouthwick May 13, 2025
f7c90b4
deal with merge conflicts
twsouthwick May 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -656,11 +656,14 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlUtil.cs">
<Link>Microsoft\Data\SqlClient\SqlUtil.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NegotiateSSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NegotiateSSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NegotiateSspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NegotiateSspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SspiAuthenticationParameters.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SspiAuthenticationParameters.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\Utilities\ObjectPool.cs">
<Link>Microsoft\Data\SqlClient\Utilities\ObjectPool.cs</Link>
Expand Down Expand Up @@ -887,8 +890,8 @@
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlColumnEncryptionCspProvider.Windows.cs">
<Link>Microsoft\Data\SqlClient\SqlColumnEncryptionCspProvider.Windows.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NativeSSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NativeSSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NativeSspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NativeSspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SqlColumnEncryptionCertificateStoreProvider.Windows.cs">
<Link>Microsoft\Data\SqlClient\SqlColumnEncryptionCertificateStoreProvider.Windows.cs</Link>
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal sealed partial class TdsParser
private static int _objectTypeCount; // EventSource counter
private readonly SqlClientLogger _logger = new SqlClientLogger();

private SSPIContextProvider _authenticationProvider;
private SspiContextProvider _authenticationProvider;

internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount);
internal int ObjectID => _objectID;
Expand Down Expand Up @@ -413,7 +413,7 @@ internal void Connect(ServerInfo serverInfo,
// AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
if (integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated)
{
_authenticationProvider = _physicalStateObj.CreateSSPIContextProvider();
_authenticationProvider = _physicalStateObj.CreateSspiContextProvider();
SqlClientEventSource.Log.TryTraceEvent("TdsParser.Connect | SEC | SSPI or Active Directory Authentication Library loaded for SQL Server based integrated authentication");
}

Expand Down
4 changes: 2 additions & 2 deletions ...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ private SNIHandle GetSessionSNIHandleHandleOrThrow()
[MethodImpl(MethodImplOptions.NoInlining)] // this forces the exception throwing code not to be inlined for performance
private void ThrowClosedConnection() => throw ADP.ClosedConnectionError();

internal override SSPIContextProvider CreateSSPIContextProvider()
=> new NegotiateSSPIContextProvider();
internal override SspiContextProvider CreateSspiContextProvider()
=> new NegotiateSspiContextProvider();
}
}
2 changes: 1 addition & 1 deletion ...crosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ internal override void DisposePacketCache()
}
}

internal override SSPIContextProvider CreateSSPIContextProvider() => new NativeSSPIContextProvider();
internal override SspiContextProvider CreateSspiContextProvider() => new NativeSspiContextProvider();

internal sealed class WritePacketCache : IDisposable
{
Expand Down
15 changes: 9 additions & 6 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,14 +354,17 @@
<Compile Include="$(CommonSourceRoot)Resources\ResCategoryAttribute.cs">
<Link>Resources\ResCategoryAttribute.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NativeSSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NativeSSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NativeSspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NativeSspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NegotiateSSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NegotiateSSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\NegotiateSspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\NegotiateSspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SSPIContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SSPIContextProvider.cs</Link>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SspiContextProvider.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SspiContextProvider.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\SSPI\SspiAuthenticationParameters.cs">
<Link>Microsoft\Data\SqlClient\SSPI\SspiAuthenticationParameters.cs</Link>
</Compile>
<Compile Include="$(CommonSourceRoot)Microsoft\Data\SqlClient\TdsParser.cs">
<Link>Microsoft\Data\SqlClient\TdsParser.cs</Link>
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ internal sealed partial class TdsParser
private static int _objectTypeCount; // EventSource counter
private readonly SqlClientLogger _logger = new SqlClientLogger();

private SSPIContextProvider _authenticationProvider;
private SspiContextProvider _authenticationProvider;

internal readonly int _objectID = Interlocked.Increment(ref _objectTypeCount);
internal int ObjectID => _objectID;
Expand Down Expand Up @@ -411,7 +411,7 @@ internal void Connect(ServerInfo serverInfo,
// AD Integrated behaves like Windows integrated when connecting to a non-fedAuth server
if (integratedSecurity || authType == SqlAuthenticationMethod.ActiveDirectoryIntegrated)
{
_authenticationProvider = _physicalStateObj.CreateSSPIContextProvider();
_authenticationProvider = _physicalStateObj.CreateSspiContextProvider();

if (!string.IsNullOrEmpty(serverInfo.ServerSPN))
{
Expand Down
2 changes: 1 addition & 1 deletion ...Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ internal override uint EnableMars(ref uint info)
internal override uint SetConnectionBufferSize(ref uint unsignedPacketSize)
=> SniNativeWrapper.SniSetInfo(Handle, QueryType.SNI_QUERY_CONN_BUFSIZE, ref unsignedPacketSize);

internal override SSPIContextProvider CreateSSPIContextProvider() => new NativeSSPIContextProvider();
internal override SspiContextProvider CreateSspiContextProvider() => new NativeSspiContextProvider();
}
}
10 changes: 6 additions & 4 deletions ...lClient/SSPI/NativeSSPIContextProvider.cs → ...lClient/SSPI/NativeSspiContextProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace Microsoft.Data.SqlClient
{
internal sealed class NativeSSPIContextProvider : SSPIContextProvider
internal sealed class NativeSspiContextProvider : SspiContextProvider
{
private static readonly object s_tdsParserLock = new();

Expand Down Expand Up @@ -49,7 +49,7 @@ private void LoadSSPILibrary()
}
}

protected override void GenerateSspiClientContext(ReadOnlySpan<byte> incomingBlob, IBufferWriter<byte> outgoingBlobWriter, ReadOnlySpan<string> serverSpns)
protected override bool GenerateSspiClientContext(ReadOnlySpan<byte> incomingBlob, IBufferWriter<byte> outgoingBlobWriter, SspiAuthenticationParameters authParams)
{
#if NETFRAMEWORK
SNIHandle handle = _physicalStateObj.Handle;
Expand All @@ -62,9 +62,9 @@ protected override void GenerateSspiClientContext(ReadOnlySpan<byte> incomingBlo
var sendLength = s_maxSSPILength;
var outBuff = outgoingBlobWriter.GetSpan((int)sendLength);

if (0 != SniNativeWrapper.SniSecGenClientContext(handle, incomingBlob, outBuff, ref sendLength, serverSpns[0]))
if (0 != SniNativeWrapper.SniSecGenClientContext(handle, incomingBlob, outBuff, ref sendLength, authParams.Resource))
{
throw new InvalidOperationException(SQLMessage.SSPIGenerateError());
return false;
}

if (sendLength > int.MaxValue)
Expand All @@ -73,6 +73,8 @@ protected override void GenerateSspiClientContext(ReadOnlySpan<byte> incomingBlo
}

outgoingBlobWriter.Advance((int)sendLength);

return true;
}
}
}
Expand Down
45 changes: 0 additions & 45 deletions ...icrosoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSSPIContextProvider.cs
View file
Open in desktop

This file was deleted.

36 changes: 36 additions & 0 deletions ...icrosoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/NegotiateSspiContextProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#if NET

using System;
using System.Buffers;
using System.Net.Security;

#nullable enable

namespace Microsoft.Data.SqlClient
{
internal sealed class NegotiateSspiContextProvider : SspiContextProvider
{
private NegotiateAuthentication? _negotiateAuth = null;

protected override bool GenerateSspiClientContext(ReadOnlySpan<byte> incomingBlob, IBufferWriter<byte> outgoingBlobWriter, SspiAuthenticationParameters authParams)
{
NegotiateAuthenticationStatusCode statusCode = NegotiateAuthenticationStatusCode.UnknownCredentials;

_negotiateAuth ??= new(new NegotiateAuthenticationClientOptions { Package = "Negotiate", TargetName = authParams.Resource });
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably missing something, but from my understanding SspiContextProvider attempts to generate sspi context by iterating over each serverSpns (which is passed via authParams.Resource). But because _negotiateAuth is cached here, if it fails for the first spn, it's also going to fail for the others since it's going to be the exact same spn.

Copy link
Contributor

@mdaigle mdaigle May 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, nice catch! The implementation used to reset _negotiateAuth to null if authentication failed, but that's missing now. @twsouthwick can we correct it in a follow up PR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do - good catch. I'll open a separate PR for that.

The looping was added independently of the work I've been doing, so I'm not fully aware of how it's expected to work.

It opens a question as to the design I've moved this to - if we have a list of SPNs, what's the expected outcome? I think things will be just fine if it's the first SPN that completes the SSPI handshake. However if it's the second, on subsequent attempts it will be trying the first again. I can update the SspiContextProvider base to keep track of what was the successfull SPN to only submit that on subsequent calls.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the PR to add that back: #3347

var sendBuff = _negotiateAuth.GetOutgoingBlob(incomingBlob, out 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);

if (statusCode == NegotiateAuthenticationStatusCode.Completed || statusCode == NegotiateAuthenticationStatusCode.ContinueNeeded)
{
outgoingBlobWriter.Write(sendBuff);
return true;
}

return false;
}
}
}
#endif
58 changes: 0 additions & 58 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SSPIContextProvider.cs
View file
Open in desktop

This file was deleted.

23 changes: 23 additions & 0 deletions ...icrosoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SSPI/SspiAuthenticationParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#nullable enable

namespace Microsoft.Data.SqlClient
{
internal sealed class SspiAuthenticationParameters
{
public SspiAuthenticationParameters(string serverName, string resource)
{
ServerName = serverName;
Resource = resource;
}

public string Resource { get; }

public string ServerName { get; }

public string? UserId { get; set; }

public string? DatabaseName { get; set; }

public string? Password { get; set; }
}
}
Loading
Loading