Skip to content

Commit cfaf082

Browse files
Port #3929 to release/6.1 (#3946)
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 901b6ec commit cfaf082

2 files changed

Lines changed: 75 additions & 15 deletions

File tree

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

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -168,28 +168,53 @@ internal override void CreatePhysicalSNIHandle(
168168
string hostNameInCertificate,
169169
string serverCertificateFilename)
170170
{
171-
if (isIntegratedSecurity)
172-
{
173-
// now allocate proper length of buffer
174-
if (!string.IsNullOrEmpty(serverSPN))
175-
{
176-
// Native SNI requires the Unicode encoding and any other encoding like UTF8 breaks the code.
177-
SqlClientEventSource.Log.TryTraceEvent("<{0}.{1}|SEC> Server SPN `{2}` from the connection string is used.", nameof(TdsParserStateObjectNative), nameof(CreatePhysicalSNIHandle), serverSPN);
178-
}
179-
else
180-
{
181-
// This will signal to the interop layer that we need to retrieve the SPN
182-
serverSPN = string.Empty;
183-
}
184-
}
171+
// Normalize SPN based on authentication mode
172+
serverSPN = NormalizeServerSpn(serverSPN, isIntegratedSecurity);
185173

186174
ConsumerInfo myInfo = CreateConsumerInfo(async);
187175
SQLDNSInfo cachedDNSInfo;
188176
bool ret = SQLFallbackDNSCache.Instance.GetDNSInfo(cachedFQDN, out cachedDNSInfo);
189177

190178
_sessionHandle = new SNIHandle(myInfo, serverName, ref serverSPN, timeout.MillisecondsRemainingInt, out instanceName,
191179
flushCache, !async, fParallel, ipPreference, cachedDNSInfo, hostNameInCertificate);
192-
resolvedSpn = new(serverSPN.TrimEnd());
180+
181+
// Only produce resolvedSpn when we actually have one.
182+
if (!string.IsNullOrWhiteSpace(serverSPN))
183+
{
184+
resolvedSpn = new(serverSPN.TrimEnd());
185+
}
186+
else
187+
{
188+
resolvedSpn = default;
189+
}
190+
}
191+
192+
/// <summary>
193+
/// Normalizes the serverSPN based on authentication mode.
194+
/// </summary>
195+
/// <param name="serverSPN">The server SPN value from the connection string.</param>
196+
/// <param name="isIntegratedSecurity">Indicates whether integrated security (SSPI) is being used.</param>
197+
/// <returns>
198+
/// For integrated security: returns <paramref name="serverSPN"/> if provided, otherwise <see cref="string.Empty"/> to trigger SPN generation.
199+
/// For SQL auth: returns <see langword="null"/> if <paramref name="serverSPN"/> is empty (no generation), otherwise returns the provided value.
200+
/// </returns>
201+
internal static string NormalizeServerSpn(string serverSPN, bool isIntegratedSecurity)
202+
{
203+
if (isIntegratedSecurity)
204+
{
205+
if (string.IsNullOrWhiteSpace(serverSPN))
206+
{
207+
// Empty signifies to interop layer that SPN needs to be generated
208+
return string.Empty;
209+
}
210+
211+
// Native SNI requires the Unicode encoding and any other encoding like UTF8 breaks the code.
212+
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.Connect|SEC> Server SPN `{0}` from the connection string is used.", serverSPN);
213+
return serverSPN;
214+
}
215+
216+
// For SQL auth (and other non-SSPI modes), null means "No SPN generation".
217+
return string.IsNullOrWhiteSpace(serverSPN) ? null : serverSPN;
193218
}
194219

195220
protected override uint SniPacketGetData(PacketHandle packet, byte[] _inBuff, ref uint dataSize)
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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 NETCOREAPP && WINDOWS
6+
7+
#nullable enable
8+
9+
using Xunit;
10+
11+
namespace Microsoft.Data.SqlClient.UnitTests
12+
{
13+
public class TdsParserStateObjectNativeTests
14+
{
15+
[Theory]
16+
[InlineData(null, true, "")] // Integrated + null -> empty (generate SPN)
17+
[InlineData("", true, "")] // Integrated + empty -> empty (generate SPN)
18+
[InlineData(" ", true, "")] // Integrated + whitespace -> empty (generate SPN)
19+
[InlineData("MSSQLSvc/host", true, "MSSQLSvc/host")] // Integrated + provided -> use it
20+
[InlineData(null, false, null)] // SQL Auth + null -> null (no generation)
21+
[InlineData("", false, null)] // SQL Auth + empty -> null (no generation)
22+
[InlineData(" ", false, null)] // SQL Auth + whitespace -> null (no generation)
23+
[InlineData("MSSQLSvc/host", false, "MSSQLSvc/host")] // SQL Auth + provided -> use it
24+
public void NormalizeServerSpn_ReturnsExpectedValue(
25+
string? inputSpn,
26+
bool isIntegratedSecurity,
27+
string? expectedSpn)
28+
{
29+
string? result = TdsParserStateObjectNative.NormalizeServerSpn(inputSpn, isIntegratedSecurity);
30+
Assert.Equal(expectedSpn, result);
31+
}
32+
}
33+
}
34+
35+
#endif

0 commit comments

Comments
 (0)