Fix SPN using instance name instead of port when SSRP resolves port without tcp: prefix

When connecting to a named instance without specifying a protocol prefix
(e.g. 'server\instance'), the DataSource.ResolvedProtocol is Protocol.None.
The SPN generation in GetSqlServerSPNs only checked for Protocol.TCP when
deciding to use the SSRP-resolved port, causing Protocol.None and
Protocol.Admin connections to incorrectly use the instance name in the SPN
(MSSQLSvc/host:instance) instead of the resolved port (MSSQLSvc/host:port).

The fix inverts the condition to check for Protocol.NP (Named Pipes), which
is the only protocol that should use instance name in the SPN. All other
protocols (TCP, None, Admin) now correctly use the resolved port.

Fixes #3566

Author: paulmedynski

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniProxy.netcore.cs

@@ -116,7 +116,7 @@ internal static SniHandle CreateConnectionHandle(
             return sniHandle;
         }
 
-        private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string serverSPN)
+        internal static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string serverSPN)
         {
             Debug.Assert(!string.IsNullOrWhiteSpace(dataSource.ServerName));
             if (!string.IsNullOrWhiteSpace(serverSPN))
@@ -132,14 +132,19 @@ private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string
             }
             else if (!string.IsNullOrWhiteSpace(dataSource.InstanceName))
             {
-                postfix = dataSource.ResolvedProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName;
+                // Named Pipes use the instance name in the SPN (MSSQLSvc/host:instance).
+                // All other protocols (TCP, None, Admin) use the port resolved by SSRP
+                // (MSSQLSvc/host:port). Protocol.None is the default when no prefix is
+                // specified in the data source (e.g. "server\instance"), and it is treated
+                // as TCP for connection purposes. See GitHub issue #3566.
+                postfix = dataSource.ResolvedProtocol == DataSource.Protocol.NP ? dataSource.InstanceName : dataSource.ResolvedPort.ToString();
             }
 
             SqlClientEventSource.Log.TryTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerName {0}, InstanceName {1}, Port {2}, postfix {3}", dataSource?.ServerName, dataSource?.InstanceName, dataSource?.Port, postfix);
             return GetSqlServerSPNs(hostName, postfix, dataSource.ResolvedProtocol);
         }
 
-        private static ResolvedServerSpn GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol)
+        internal static ResolvedServerSpn GetSqlServerSPNs(string hostNameOrAddress, string portOrInstanceName, DataSource.Protocol protocol)
         {
             Debug.Assert(!string.IsNullOrWhiteSpace(hostNameOrAddress));
             IPHostEntry hostEntry = null;
@@ -607,8 +612,8 @@ private bool InferNamedPipesInformation()
                 // If the data source starts with "np:servername"
                 if (!_dataSourceAfterTrimmingProtocol.Contains(PipeBeginning))
                 {
-                    // Assuming that user did not change default NamedPipe name, if the datasource is in the format servername\instance, 
-                    // separate servername and instance and prepend instance with MSSQL$ and append default pipe path 
+                    // Assuming that user did not change default NamedPipe name, if the datasource is in the format servername\instance,
+                    // separate servername and instance and prepend instance with MSSQL$ and append default pipe path
                     // https://learn.microsoft.com/en-us/sql/tools/configuration-manager/named-pipes-properties?view=sql-server-ver16
                     if (_dataSourceAfterTrimmingProtocol.Contains(PathSeparator) && ResolvedProtocol == Protocol.NP)
                     {

src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ManagedSni/SniProxyGetSqlServerSPNsTest.cs

@@ -0,0 +1,117 @@
+// 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.
+
+#if NET
+
+using Microsoft.Data.SqlClient.ManagedSni;
+using Xunit;
+
+namespace Microsoft.Data.SqlClient.UnitTests.ManagedSni
+{
+    public class SniProxyGetSqlServerSPNsTest
+    {
+        /// <summary>
+        /// Verifies that when connecting to a named instance without a protocol prefix
+        /// (Protocol.None), the SPN uses the resolved port number from SSRP rather than
+        /// the instance name. This is a regression test for GitHub issue #3566.
+        /// </summary>
+        [Fact]
+        public void GetSqlServerSPNs_ProtocolNone_WithResolvedPort_UsesPortNotInstanceName()
+        {
+            // Arrange: parse "server\instance" which sets Protocol.None and IsSsrpRequired
+            DataSource dataSource = DataSource.ParseServerName(@"server\instance");
+            Assert.NotNull(dataSource);
+            Assert.Equal(DataSource.Protocol.None, dataSource.ResolvedProtocol);
+            Assert.Equal("instance", dataSource.InstanceName);
+            Assert.Equal(-1, dataSource.Port);
+
+            // Simulate SSRP resolution setting the port (as CreateTcpHandle would do)
+            dataSource.ResolvedPort = 12345;
+
+            // Act
+            ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty);
+
+            // Assert: SPN should contain the resolved port, NOT the instance name
+            Assert.Contains(":12345", spn.Primary);
+            Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase);
+        }
+
+        /// <summary>
+        /// Verifies that when connecting with an explicit tcp: prefix (Protocol.TCP),
+        /// the SPN uses the resolved port number. This was the original fix for #2187.
+        /// </summary>
+        [Fact]
+        public void GetSqlServerSPNs_ProtocolTcp_WithResolvedPort_UsesPort()
+        {
+            // Arrange: parse "tcp:server\instance" which sets Protocol.TCP
+            DataSource dataSource = DataSource.ParseServerName(@"tcp:server\instance");
+            Assert.NotNull(dataSource);
+            Assert.Equal(DataSource.Protocol.TCP, dataSource.ResolvedProtocol);
+
+            dataSource.ResolvedPort = 54321;
+
+            // Act
+            ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty);
+
+            // Assert
+            Assert.Contains(":54321", spn.Primary);
+            Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase);
+        }
+
+        /// <summary>
+        /// Verifies that when connecting with Named Pipes protocol, the SPN uses
+        /// the instance name rather than a port number.
+        /// </summary>
+        [Fact]
+        public void GetSqlServerSPNs_ProtocolNp_WithInstanceName_UsesInstanceName()
+        {
+            // Named Pipes data sources go through a different parsing path
+            // (InferNamedPipesInformation) that doesn't populate InstanceName,
+            // so we test the lower-level overload directly.
+            ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs("server", "myinstance", DataSource.Protocol.NP);
+
+            Assert.Contains(":myinstance", spn.Primary);
+            Assert.Null(spn.Secondary);
+        }
+
+        /// <summary>
+        /// Verifies that when a custom ServerSPN is provided in the connection string,
+        /// it is used as-is regardless of protocol or instance name.
+        /// </summary>
+        [Fact]
+        public void GetSqlServerSPNs_CustomSpnProvided_UsesCustomSpn()
+        {
+            DataSource dataSource = DataSource.ParseServerName(@"server\instance");
+            Assert.NotNull(dataSource);
+            dataSource.ResolvedPort = 12345;
+
+            string customSpn = "MSSQLSvc/myserver.domain.com:1433";
+            ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: customSpn);
+
+            Assert.Equal(customSpn, spn.Primary);
+            Assert.Null(spn.Secondary);
+        }
+
+        /// <summary>
+        /// Verifies that when connecting with admin: prefix (DAC), the SPN uses
+        /// the resolved port number (DAC also resolves via SSRP).
+        /// </summary>
+        [Fact]
+        public void GetSqlServerSPNs_ProtocolAdmin_WithResolvedPort_UsesPort()
+        {
+            DataSource dataSource = DataSource.ParseServerName(@"admin:server\instance");
+            Assert.NotNull(dataSource);
+            Assert.Equal(DataSource.Protocol.Admin, dataSource.ResolvedProtocol);
+
+            dataSource.ResolvedPort = 11111;
+
+            ResolvedServerSpn spn = SniProxy.GetSqlServerSPNs(dataSource, serverSPN: string.Empty);
+
+            Assert.Contains(":11111", spn.Primary);
+            Assert.DoesNotContain("instance", spn.Primary, System.StringComparison.OrdinalIgnoreCase);
+        }
+    }
+}
+
+#endif