Fix SPN using instance name instead of resolved port for Protocol.None and Protocol.Admin

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,24 @@ private static ResolvedServerSpn GetSqlServerSPNs(DataSource dataSource, string
             }
             else if (!string.IsNullOrWhiteSpace(dataSource.InstanceName))
             {
-                postfix = dataSource.ResolvedProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName;
+                // Per SQL Server Kerberos/SPN guidance, TCP client connections should use
+                // MSSQLSvc/<FQDN>:<port>, while named pipes/shared-memory use
+                // MSSQLSvc/<FQDN>:<instancename> for named instances. For our managed SNI path,
+                // NP uses instance-name postfix and TCP-like protocols (TCP, None, Admin)
+                // use a port postfix (resolved via SSRP for named instances).
+                // If SSRP resolution hasn't populated ResolvedPort yet (value is -1), fall back
+                // to the instance name to avoid producing a malformed SPN like ":-1".
+                // https://learn.microsoft.com/en-us/sql/database-engine/configure-windows/register-a-service-principal-name-for-kerberos-connections?view=sql-server-ver17#named-instance
+                postfix = (dataSource.ResolvedProtocol == DataSource.Protocol.NP || dataSource.ResolvedPort <= 0)
+                    ? 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);
+            SqlClientEventSource.Log.TryTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerName {0}, InstanceName {1}, Port {2}, ResolvedPort {3}, ResolvedProtocol {4}, postfix {5}", dataSource?.ServerName, dataSource?.InstanceName, dataSource?.Port, dataSource?.ResolvedPort, dataSource?.ResolvedProtocol, 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 +617,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/ManualTests/SQL/KerberosTests/KerberosTest.cs

@@ -8,8 +8,25 @@
 
 namespace Microsoft.Data.SqlClient.ManualTesting.Tests
 {
+    /// <summary>
+    /// Integration tests for Kerberos authentication with protocol-specific SPN handling.
+    ///
+    /// These tests verify that the driver generates protocol-specific SPNs for Kerberos authentication
+    /// across different protocol types (TCP, None, Named Pipes, Admin). They complement unit tests
+    /// in SniProxyGetSqlServerSPNsTest by verifying end-to-end authentication behavior in a real
+    /// Kerberos domain environment.
+    ///
+    /// See: https://learn.microsoft.com/en-us/sql/database-engine/configure-windows/register-a-service-principal-name-for-kerberos-connections
+    /// </summary>
     public class KerberosTests
     {
+        /// <summary>
+        /// Baseline Kerberos connectivity test verifying that the Kerberos authentication mechanism works
+        /// with the configured connection strings.
+        ///
+        /// This test runs on Unix platforms with Kerberos credentials and verifies that connections
+        /// authenticate using the KERBEROS auth_scheme (confirmed via sys.dm_exec_connections).
+        /// </summary>
         [PlatformSpecific(TestPlatforms.AnyUnix)]
         [ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosTest))]
         [ClassData(typeof(ConnectionStringsProvider))]
@@ -24,8 +41,207 @@ public void IsKerBerosSetupTestAsync(string connectionStr)
             Assert.True(reader.Read(), "Expected to receive one row data");
             Assert.Equal("KERBEROS", reader.GetString(0));
         }
+
+        /// <summary>
+        /// Tests Kerberos authentication with Protocol.None (default when no protocol prefix specified).
+        ///
+        /// This regression test for GitHub issue #3566 verifies that named instances accessed via
+        /// Protocol.None (e.g. "Data Source=hostname\instancename") use the correct SPN format:
+        /// - If SSRP resolves a port: MSSQLSvc/hostname:port (not instance name)
+        /// - Kerberos should authenticate successfully with the correct SPN
+        ///
+        /// Environment: Requires a named instance running on a domain-joined server with an SSRP-resolvable port.
+        /// </summary>
+        [PlatformSpecific(TestPlatforms.Linux)]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosTest))]
+        public void KerberosTest_ProtocolNone_NamedInstanceWithSsrpResolution()
+        {
+            // Skip if no TCP connection string with a named instance is available
+            string tcpConnStr = DataTestUtility.TCPConnectionString;
+            if (string.IsNullOrEmpty(tcpConnStr) ||
+                !DataTestUtility.ParseDataSource(new SqlConnectionStringBuilder(tcpConnStr).DataSource,
+                    out string hostname, out int port, out string instanceName) ||
+                string.IsNullOrEmpty(instanceName))
+            {
+                return; // Skip test; no named instance available
+            }
+
+            KerberosTicketManagemnt.Init(DataTestUtility.KerberosDomainUser, DataTestUtility.KerberosDomainPassword);
+
+            // Build from the base connection string to preserve environment settings (Encrypt,
+            // TrustServerCertificate, timeouts, etc.), overriding only DataSource and IntegratedSecurity.
+            // SSRP resolution should occur and populate the port in the SPN.
+            SqlConnectionStringBuilder protocolNoneBuilder = new(tcpConnStr)
+            {
+                DataSource = $"{hostname}\\{instanceName}",
+                IntegratedSecurity = true
+            };
+
+            using SqlConnection conn = new(protocolNoneBuilder.ConnectionString);
+            conn.Open(); // Connection should succeed with Kerberos using the SSRP-resolved port in SPN
+
+            // Verify authentication occurred with KERBEROS auth_scheme
+            using SqlCommand command = new("SELECT auth_scheme from sys.dm_exec_connections where session_id = @@spid", conn);
+            using SqlDataReader reader = command.ExecuteReader();
+            Assert.True(reader.Read(), "Expected to receive one row data");
+            Assert.Equal("KERBEROS", reader.GetString(0));
+        }
+
+        /// <summary>
+        /// Tests Kerberos authentication with explicit Protocol.TCP prefix on a named instance.
+        ///
+        /// Verifies that "tcp:hostname\instancename" uses the TCP-like SPN format with port:
+        /// MSSQLSvc/hostname:port (where port is from explicit connection string or SSRP resolution).
+        ///
+        /// Environment: Requires a named instance with an explicitly specified or SSRP-resolvable port.
+        /// </summary>
+        [PlatformSpecific(TestPlatforms.Linux)]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosTest))]
+        public void KerberosTest_ProtocolTcp_NamedInstanceWithExplicitPort()
+        {
+            string tcpConnStr = DataTestUtility.TCPConnectionString;
+            if (string.IsNullOrEmpty(tcpConnStr) ||
+                !DataTestUtility.ParseDataSource(new SqlConnectionStringBuilder(tcpConnStr).DataSource,
+                    out string hostname, out int port, out string instanceName) ||
+                string.IsNullOrEmpty(instanceName))
+            {
+                return; // Skip test; no named instance available
+            }
+
+            KerberosTicketManagemnt.Init(DataTestUtility.KerberosDomainUser, DataTestUtility.KerberosDomainPassword);
+
+            // Build the tcp: data source. Include an explicit port only when the test connection string
+            // already has one; otherwise leave SSRP to resolve the port for the named instance.
+            // Do NOT fall back to port 1433: that disables SSRP and is unlikely to be correct for
+            // named instances, and produces an invalid "host\,1433" when instanceName is empty.
+            string newDataSource = port > 0
+                ? $"tcp:{hostname}\\{instanceName},{port}"
+                : $"tcp:{hostname}\\{instanceName}";
+
+            // Preserve the base connection string settings (Encrypt, TrustServerCertificate, etc.)
+            SqlConnectionStringBuilder builder = new(tcpConnStr)
+            {
+                DataSource = newDataSource,
+                IntegratedSecurity = true
+            };
+
+            using SqlConnection conn = new(builder.ConnectionString);
+            conn.Open();
+
+            using SqlCommand command = new("SELECT auth_scheme from sys.dm_exec_connections where session_id = @@spid", conn);
+            using SqlDataReader reader = command.ExecuteReader();
+            Assert.True(reader.Read(), "Expected to receive one row data");
+            Assert.Equal("KERBEROS", reader.GetString(0));
+        }
+
+        /// <summary>
+        /// Tests Kerberos authentication with a custom ServerSPN override.
+        ///
+        /// Verifies that explicitly setting ServerSPN in the connection string bypasses auto-generation
+        /// and uses the provided SPN for Kerberos authentication. This is critical for environments where:
+        /// - Kerberos SPNs are registered with specific hostnames/ports
+        /// - SQL Server is behind a proxy or alias
+        /// - Multi-instance environments with non-standard naming
+        ///
+        /// Environment: Requires ability to specify a valid SPN for the target instance.
+        /// </summary>
+        [PlatformSpecific(TestPlatforms.Linux)]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosTest))]
+        public void KerberosTest_CustomServerSPN_BypassesAutoGeneration()
+        {
+            string tcpConnStr = DataTestUtility.TCPConnectionString;
+            if (string.IsNullOrEmpty(tcpConnStr) ||
+                !DataTestUtility.ParseDataSource(new SqlConnectionStringBuilder(tcpConnStr).DataSource,
+                    out string hostname, out int port, out string instanceName))
+            {
+                return; // Skip test
+            }
+
+            // For a reliable custom SPN test, we need to know the exact port so we can construct
+            // the TCP-format SPN the server expects: MSSQLSvc/fqdn:port.
+            // Using the instance name here is wrong for TCP environments that register only port-based SPNs.
+            if (port <= 0)
+            {
+                return; // Skip test; cannot construct a valid port-based custom SPN without an explicit port
+            }
+
+            KerberosTicketManagemnt.Init(DataTestUtility.KerberosDomainUser, DataTestUtility.KerberosDomainPassword);
+
+            // Build the TCP-format SPN that matches what the driver would auto-generate.
+            // TCP Kerberos connections use MSSQLSvc/fqdn:port regardless of instance name.
+            string fqdn = DataTestUtility.GetMachineFQDN(hostname);
+            string customSpn = $"MSSQLSvc/{fqdn}:{port}";
+
+            SqlConnectionStringBuilder builder = new(tcpConnStr);
+            builder.IntegratedSecurity = true;
+            builder.ServerSPN = customSpn;
+
+            using SqlConnection conn = new(builder.ConnectionString);
+            conn.Open();
+
+            using SqlCommand command = new("SELECT auth_scheme from sys.dm_exec_connections where session_id = @@spid", conn);
+            using SqlDataReader reader = command.ExecuteReader();
+            Assert.True(reader.Read(), "Expected to receive one row data");
+            Assert.Equal("KERBEROS", reader.GetString(0));
+        }
+
+        /// <summary>
+        /// Tests Kerberos authentication with Protocol.Admin (Dedicated Administrator Connection).
+        ///
+        /// Verifies that DAC connections (prefix "admin:") to named instances use the TCP-like SPN format:
+        /// MSSQLSvc/hostname:port (not instance name). DAC uses SSRP resolution similar to TCP.
+        ///
+        /// Environment: Requires DAC to be enabled on the target SQL Server instance and admin credentials.
+        /// Note: May be skipped if DAC is not enabled or not accessible via Kerberos.
+        /// </summary>
+        [PlatformSpecific(TestPlatforms.Linux)]
+        [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosTest))]
+        public void KerberosTest_ProtocolAdmin_DedicatedAdminConnection()
+        {
+            string tcpConnStr = DataTestUtility.TCPConnectionString;
+            if (string.IsNullOrEmpty(tcpConnStr) ||
+                !DataTestUtility.ParseDataSource(new SqlConnectionStringBuilder(tcpConnStr).DataSource,
+                    out string hostname, out int port, out string instanceName) ||
+                string.IsNullOrEmpty(instanceName))
+            {
+                return; // Skip test; no named instance available
+            }
+
+            KerberosTicketManagemnt.Init(DataTestUtility.KerberosDomainUser, DataTestUtility.KerberosDomainPassword);
+
+            // Build the admin: data source from the base connection string to preserve environment
+            // settings. Do NOT fall back to port 1433 — the DAC port is separate from the regular
+            // SQL Server port and must be discovered via SSRP if not explicitly known.
+            string newDataSource = port > 0
+                ? $"admin:{hostname}\\{instanceName},{port}"
+                : $"admin:{hostname}\\{instanceName}";
+
+            SqlConnectionStringBuilder adminBuilder = new(tcpConnStr)
+            {
+                DataSource = newDataSource,
+                IntegratedSecurity = true
+            };
+
+            // Note: this test requires DAC to be enabled on the target instance
+            // (sp_configure 'remote admin connections', 1). If DAC is not enabled,
+            // the connection will fail with a SqlException and the test will report as failed,
+            // which is the desired behavior — the test environment should be fixed.
+            using SqlConnection conn = new(adminBuilder.ConnectionString);
+            conn.Open();
+
+            using SqlCommand command = new("SELECT auth_scheme from sys.dm_exec_connections where session_id = @@spid", conn);
+            using SqlDataReader reader = command.ExecuteReader();
+            Assert.True(reader.Read(), "Expected to receive one row data");
+            Assert.Equal("KERBEROS", reader.GetString(0));
+        }
     }
 
+    /// <summary>
+    /// Provides connection strings from DataTestUtility for theory-based Kerberos tests.
+    ///
+    /// Each connection string is tested in a separate Kerberos context to ensure protocol-specific
+    /// SPN behavior works across all available SQL Server configurations in the test environment.
+    /// </summary>
     public class ConnectionStringsProvider : IEnumerable<object[]>
     {
         public IEnumerator<object[]> GetEnumerator()