Skip to content

Commit 104a401

Browse files
Avoid unintended SPN generation for non-integrated authentication on native SNI path (#3929)
This PR fixes a connection performance regression introduced by PR #2790, where SPN generation could be triggered for non-integrated authentication modes such as SQL authentication on the native SNI path. This change ensures that SPN generation is only triggered when authentication mode is integrated security (SSPI/Kerberos).
1 parent a0357b2 commit 104a401

2 files changed

Lines changed: 74 additions & 15 deletions

File tree

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObjectNative.windows.cs

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -155,20 +155,8 @@ internal override void CreatePhysicalSNIHandle(
155155
string hostNameInCertificate,
156156
string serverCertificateFilename)
157157
{
158-
if (isIntegratedSecurity)
159-
{
160-
// now allocate proper length of buffer
161-
if (!string.IsNullOrEmpty(serverSPN))
162-
{
163-
// Native SNI requires the Unicode encoding and any other encoding like UTF8 breaks the code.
164-
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Server SPN `{0}` from the connection string is used.", serverSPN);
165-
}
166-
else
167-
{
168-
// Empty signifies to interop layer that SPN needs to be generated
169-
serverSPN = string.Empty;
170-
}
171-
}
158+
// Normalize SPN based on authentication mode
159+
serverSPN = NormalizeServerSpn(serverSPN, isIntegratedSecurity);
172160

173161
ConsumerInfo myInfo = CreateConsumerInfo(async);
174162

@@ -183,7 +171,44 @@ internal override void CreatePhysicalSNIHandle(
183171
transparentNetworkResolutionState, totalTimeout,
184172
#endif
185173
iPAddressPreference, cachedDNSInfo, hostNameInCertificate);
186-
resolvedSpn = new(serverSPN.TrimEnd());
174+
175+
// Only produce resolvedSpn when we actually have one.
176+
if (!string.IsNullOrWhiteSpace(serverSPN))
177+
{
178+
resolvedSpn = new(serverSPN.TrimEnd());
179+
}
180+
else
181+
{
182+
resolvedSpn = default;
183+
}
184+
}
185+
186+
/// <summary>
187+
/// Normalizes the serverSPN based on authentication mode.
188+
/// </summary>
189+
/// <param name="serverSPN">The server SPN value from the connection string.</param>
190+
/// <param name="isIntegratedSecurity">Indicates whether integrated security (SSPI) is being used.</param>
191+
/// <returns>
192+
/// For integrated security: returns <paramref name="serverSPN"/> if provided, otherwise <see cref="string.Empty"/> to trigger SPN generation.
193+
/// For SQL auth: returns <see langword="null"/> if <paramref name="serverSPN"/> is empty (no generation), otherwise returns the provided value.
194+
/// </returns>
195+
internal static string NormalizeServerSpn(string serverSPN, bool isIntegratedSecurity)
196+
{
197+
if (isIntegratedSecurity)
198+
{
199+
if (string.IsNullOrWhiteSpace(serverSPN))
200+
{
201+
// Empty signifies to interop layer that SPN needs to be generated
202+
return string.Empty;
203+
}
204+
205+
// Native SNI requires the Unicode encoding and any other encoding like UTF8 breaks the code.
206+
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Server SPN `{0}` from the connection string is used.", serverSPN);
207+
return serverSPN;
208+
}
209+
210+
// For SQL auth (and other non-SSPI modes), null means "No SPN generation".
211+
return string.IsNullOrWhiteSpace(serverSPN) ? null : serverSPN;
187212
}
188213

189214
protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize)
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
#if NETFRAMEWORK || WINDOWS
6+
7+
using Xunit;
8+
9+
namespace Microsoft.Data.SqlClient.UnitTests
10+
{
11+
public class TdsParserStateObjectNativeTests
12+
{
13+
[Theory]
14+
[InlineData(null, true, "")] // Integrated + null -> empty (generate SPN)
15+
[InlineData("", true, "")] // Integrated + empty -> empty (generate SPN)
16+
[InlineData(" ", true, "")] // Integrated + whitespace -> empty (generate SPN)
17+
[InlineData("MSSQLSvc/host", true, "MSSQLSvc/host")] // Integrated + provided -> use it
18+
[InlineData(null, false, null)] // SQL Auth + null -> null (no generation)
19+
[InlineData("", false, null)] // SQL Auth + empty -> null (no generation)
20+
[InlineData(" ", false, null)] // SQL Auth + whitespace -> null (no generation)
21+
[InlineData("MSSQLSvc/host", false, "MSSQLSvc/host")] // SQL Auth + provided -> use it
22+
[PlatformSpecific(TestPlatforms.Windows)]
23+
public void NormalizeServerSpn_ReturnsExpectedValue(
24+
string? inputSpn,
25+
bool isIntegratedSecurity,
26+
string? expectedSpn)
27+
{
28+
string? result = TdsParserStateObjectNative.NormalizeServerSpn(inputSpn, isIntegratedSecurity);
29+
Assert.Equal(expectedSpn, result);
30+
}
31+
}
32+
}
33+
34+
#endif

0 commit comments

Comments
 (0)