From e8b6a5f73cc826a9e6b182e9296dadd862aaca24 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Thu, 16 Nov 2023 10:36:44 -0800 Subject: [PATCH 01/17] Push unit test to continue development in VM. --- .../Microsoft/Data/SqlClient/SNI/SNIProxy.cs | 9 ++++++-- .../ManualTests/DataCommon/DataTestUtility.cs | 21 +++++++++++++++++++ .../SQL/InstanceNameTest/InstanceNameTest.cs | 16 ++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs index 339e6be972..93b353f9c6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs @@ -230,7 +230,7 @@ private static byte[][] GetSqlServerSPNs(DataSource dataSource, string serverSPN } else if (!string.IsNullOrWhiteSpace(dataSource.InstanceName)) { - postfix = dataSource.InstanceName; + postfix = (dataSource._connectionProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName); } SqlClientEventSource.Log.TryTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerName {0}, InstanceName {1}, Port {2}, postfix {3}", dataSource?.ServerName, dataSource?.InstanceName, dataSource?.Port, postfix); @@ -317,7 +317,7 @@ private static SNITCPHandle CreateTcpHandle( { try { - port = isAdminConnection ? + details.ResolvedPort = port = isAdminConnection ? SSRP.GetDacPortByInstanceName(hostName, details.InstanceName, timeout, parallel, ipPreference) : SSRP.GetPortByInstanceName(hostName, details.InstanceName, timeout, parallel, ipPreference); } @@ -436,6 +436,11 @@ internal enum Protocol { TCP, NP, None, Admin }; /// internal int Port { get; private set; } = -1; + /// + /// The port resolved by SSRP when InstanceName is specified + /// + internal int ResolvedPort { get; set; } = -1; + /// /// Provides the inferred Instance Name from Server Data Source /// diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 07f866f98b..754243fe54 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1073,6 +1073,27 @@ public static string GetMachineFQDN(string hostname) return fqdn.ToString(); } + public static bool IsManagedSNI() + { + return UseManagedSNIOnWindows; + } + + public static bool IsNotLocalhost() + { + // get the tcp connection string + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + + // remove tcp from connection string + string dataSourceStr = builder.DataSource.Replace("tcp:", ""); + // create a string array + string[] serverNamePartsByBackSlash = dataSourceStr.Split('\\'); + // first element of array is the hostname + string hostname = serverNamePartsByBackSlash[0]; + + // check if hostname = localhost + return hostname.Equals("localhost", StringComparison.OrdinalIgnoreCase); + } + private static bool RunningAsUWPApp() { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 311ce5fce5..07397c6124 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -83,6 +83,22 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } + [PlatformSpecific(TestPlatforms.AnyUnix)] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsManagedSNI), nameof(DataTestUtility.IsNotLocalhost), nameof(DataTestUtility.IsKerberosTest), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse))] + public static void PortNumberInSPNTest() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + + Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); + + if (IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName)) + { + builder.DataSource = hostname + "\\" + instanceName; + using SqlConnection connection = new(builder.ConnectionString); + connection.Open(); + } + } + private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03; From ce2481db5991e4dbe5e52413710abdfa95432862 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Fri, 17 Nov 2023 21:49:57 +0000 Subject: [PATCH 02/17] Added unit test for SPN in managed SNI and tested in VMs in sqldrv.ad domain. --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 6 ++++++ .../SQL/InstanceNameTest/InstanceNameTest.cs | 10 ++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 754243fe54..90f0c585ab 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1078,6 +1078,12 @@ public static bool IsManagedSNI() return UseManagedSNIOnWindows; } + public static bool IsIntegratedSecurity() + { + SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + return builder.IntegratedSecurity; + } + public static bool IsNotLocalhost() { // get the tcp connection string diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 07397c6124..88ac2651c1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -83,7 +83,8 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } - [PlatformSpecific(TestPlatforms.AnyUnix)] + // Note: This Unit test was tested in a VM within the sqldrv.ad domain. i.e. from server sqldrv-win22 and + // is connecting to a Sql Server using Kerberos at sqldrv-sql22 server in the same domain. [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsManagedSNI), nameof(DataTestUtility.IsNotLocalhost), nameof(DataTestUtility.IsKerberosTest), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse))] public static void PortNumberInSPNTest() { @@ -93,9 +94,14 @@ public static void PortNumberInSPNTest() if (IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName)) { - builder.DataSource = hostname + "\\" + instanceName; using SqlConnection connection = new(builder.ConnectionString); connection.Open(); + using SqlCommand command = new("SELECT auth_scheme, local_tcp_port from sys.dm_exec_connections where session_id = @@spid", connection); + using SqlDataReader reader = command.ExecuteReader(); + Assert.True(reader.Read(), "Expected to receive one row data"); + Assert.Equal("KERBEROS", reader.GetString(0)); + int Port = reader.GetInt32(1); + Assert.True( Port > 0); } } From 60f061ffd309f53aa7873cae70f813334db39779 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Sat, 18 Nov 2023 01:34:12 +0000 Subject: [PATCH 03/17] Save to my branch all changes --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 88ac2651c1..8d8bfe6dee 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -5,6 +5,7 @@ using System; using System.Net; using System.Net.Sockets; +using System.Reflection; using System.Text; using System.Threading.Tasks; using Xunit; @@ -102,9 +103,21 @@ public static void PortNumberInSPNTest() Assert.Equal("KERBEROS", reader.GetString(0)); int Port = reader.GetInt32(1); Assert.True( Port > 0); + Port = GetSPNPort(builder.DataSource); } } + private static int GetSPNPort(string datasource) + { + Assembly systemData = Assembly.GetAssembly(typeof(SqlConnection)); + Type SniProxy = systemData.GetType("Microsoft.Data.SqlClient.SNI.SNIProxy"); + ConstructorInfo sniProxyConstructor = SniProxy.GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, new Type[] { }, null); + + Object sniProxy = sniProxyConstructor.Invoke(new object[] { }); + + return 0; + } + private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03; From e77bfb6241b0c51353013875c76ecf5cb16b64cc Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Thu, 23 Nov 2023 03:26:36 +0000 Subject: [PATCH 04/17] Implemented Unit Test using Reflections. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 100 ++++++++++++++++-- 1 file changed, 94 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 8d8bfe6dee..3ea18aca81 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -3,11 +3,13 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections; using System.Net; using System.Net.Sockets; using System.Reflection; using System.Text; using System.Threading.Tasks; +using Azure.Core; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests @@ -102,20 +104,106 @@ public static void PortNumberInSPNTest() Assert.True(reader.Read(), "Expected to receive one row data"); Assert.Equal("KERBEROS", reader.GetString(0)); int Port = reader.GetInt32(1); - Assert.True( Port > 0); - Port = GetSPNPort(builder.DataSource); + + int port = -1; + string spnInfo = GetSPNInfo(builder.DataSource, out port); + + // sample output to validate = MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" + Assert.Contains($"MSSQLSvc/{hostname}", spnInfo); + // the local_tcp_port Port is the same as the inferred port from instance name + Assert.Equal(Port, port); } } - private static int GetSPNPort(string datasource) + private static string GetSPNInfo(string datasource, out int out_port) { Assembly systemData = Assembly.GetAssembly(typeof(SqlConnection)); + + // Get all required types using reflection Type SniProxy = systemData.GetType("Microsoft.Data.SqlClient.SNI.SNIProxy"); - ConstructorInfo sniProxyConstructor = SniProxy.GetConstructor(BindingFlags.NonPublic | BindingFlags.Instance, null, new Type[] { }, null); + Type SSRP = systemData.GetType("Microsoft.Data.SqlClient.SNI.SSRP"); + Type DataSource = systemData.GetType("Microsoft.Data.SqlClient.SNI.DataSource"); + Type TimeoutTimer = systemData.GetType("Microsoft.Data.ProviderBase.TimeoutTimer"); + + // Used in Datasource constructor param type array + Type[] types = new Type[1]; + types[0] = typeof(string); + + // Used in GetSqlServerSPNs function param types array + Type[] types2 = new Type[2]; + types2[0] = DataSource; + types2[1] = typeof(string); + + // GetPortByInstanceName parameters array + Type[] types3 = new Type[5]; + types3[0] = typeof(string); + types3[1] = typeof(string); + types3[2] = TimeoutTimer; + types3[3] = typeof(bool); + types3[4] = typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference); + + // TimeoutTimer.StartSecondsTimeout params + Type[] types4 = new Type[1]; + types4[0] = typeof(int); + + // Get all types constructors + ConstructorInfo sniProxyCtor = SniProxy.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo SSRPCtor = SSRP.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo datasSourceCtor = DataSource.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types , null); + ConstructorInfo timeoutTimerCtor = TimeoutTimer.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + + // Instantiate SNIProxy + var sniProxy = sniProxyCtor.Invoke(new object[] { }); + + // Instatntiate datasource + var details = datasSourceCtor.Invoke(new object[] { datasource }); + + // Instantiate SSRP + var ssrp = SSRPCtor.Invoke(new object[] { }); + + // Instantiate TimeoutTimer + var timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); + + // Get TimeoutTimer.StartSecondsTimeout Method + MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types4, null); + // Create a timeoutTimer that expires in 100,000 milliseconds + timeoutTimer = startSecondsTimeout.Invoke(details, new object[] { 100000 }); + + // Parse the datasource to separate the server name and instance name + MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types, null); + var dataSrcInfo = ParseServerName.Invoke(details, new object[] { datasource }); + + // Get the GetPortByInstanceName method of SSRP + MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types3, null); + + // Get the server name + PropertyInfo serverInfo = dataSrcInfo.GetType().GetProperty("ServerName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + var serverName = serverInfo.GetValue(dataSrcInfo, null).ToString(); + + // Get the instance name + PropertyInfo instanceNameInfo = dataSrcInfo.GetType().GetProperty("InstanceName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + var instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); + + // Get the port number using the GetPortByInstanceName method of SSRP + var port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 } ); + + // Set the resolved port property of datasource + PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + resolvedPortInfo.SetValue(dataSrcInfo, (int)port, null); + + // Prepare the GetSqlServerSPNs method + string serverSPN = ""; + MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types2, null); + + // Finally call GetSqlServerSPNs + dynamic result = getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); + + // MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" + var spnInfo = System.Text.Encoding.Unicode.GetString(result[0]); - Object sniProxy = sniProxyConstructor.Invoke(new object[] { }); + out_port = (int)port; - return 0; + return spnInfo; } private static bool IsBrowserAlive(string browserHostname) From ed15e3704fd94c35c2240282bb8cd788a39b89e8 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Thu, 23 Nov 2023 03:33:53 +0000 Subject: [PATCH 05/17] Change TimeourTimer from 100000 to just 30 seconds. --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 3ea18aca81..d2882224a6 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -166,8 +166,8 @@ private static string GetSPNInfo(string datasource, out int out_port) // Get TimeoutTimer.StartSecondsTimeout Method MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types4, null); - // Create a timeoutTimer that expires in 100,000 milliseconds - timeoutTimer = startSecondsTimeout.Invoke(details, new object[] { 100000 }); + // Create a timeoutTimer that expires in 30 seconds + timeoutTimer = startSecondsTimeout.Invoke(details, new object[] { 30 }); // Parse the datasource to separate the server name and instance name MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types, null); From d671acdef65f4f83db7b638e672bfa6657c3d47c Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 22 Nov 2023 20:08:27 -0800 Subject: [PATCH 06/17] Removed and sort usings. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index d2882224a6..b5909528aa 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -3,13 +3,11 @@ // See the LICENSE file in the project root for more information. using System; -using System.Collections; using System.Net; using System.Net.Sockets; using System.Reflection; using System.Text; using System.Threading.Tasks; -using Azure.Core; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests From d62c9e11c6758f27e57507e19970aafa5707a809 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 22 Nov 2023 20:19:35 -0800 Subject: [PATCH 07/17] Fix backwards DataTestUtility.IsNotLocalhost logic. --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 90f0c585ab..5a01bf8c1e 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1097,7 +1097,7 @@ public static bool IsNotLocalhost() string hostname = serverNamePartsByBackSlash[0]; // check if hostname = localhost - return hostname.Equals("localhost", StringComparison.OrdinalIgnoreCase); + return !hostname.Equals("localhost", StringComparison.OrdinalIgnoreCase); } private static bool RunningAsUWPApp() From 8de601435f3529702617302356098bde241d5bc4 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Thu, 23 Nov 2023 09:50:55 -0800 Subject: [PATCH 08/17] Removed DataTestUtility.IsManagedSNI as there is already DataTestUtility.IsUsingManagedSNI. --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 5 ----- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 5a01bf8c1e..f857b5ad83 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1073,11 +1073,6 @@ public static string GetMachineFQDN(string hostname) return fqdn.ToString(); } - public static bool IsManagedSNI() - { - return UseManagedSNIOnWindows; - } - public static bool IsIntegratedSecurity() { SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index b5909528aa..cf0afd0fe5 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,7 +86,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a VM within the sqldrv.ad domain. i.e. from server sqldrv-win22 and // is connecting to a Sql Server using Kerberos at sqldrv-sql22 server in the same domain. - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsManagedSNI), nameof(DataTestUtility.IsNotLocalhost), nameof(DataTestUtility.IsKerberosTest), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI), nameof(DataTestUtility.IsNotLocalhost), nameof(DataTestUtility.IsKerberosTest), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse))] public static void PortNumberInSPNTest() { SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); From 2d3ae85ac50a499d29a6f3c8c92812a84a337543 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Thu, 23 Nov 2023 16:53:48 -0800 Subject: [PATCH 09/17] Add Type suffix to reflected types. Fixed checks for localhost. Use object initializer for array of types declarations. --- .../ManualTests/DataCommon/DataTestUtility.cs | 6 ++- .../SQL/InstanceNameTest/InstanceNameTest.cs | 39 +++++++------------ 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index f857b5ad83..91516ab1e4 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1091,8 +1091,10 @@ public static bool IsNotLocalhost() // first element of array is the hostname string hostname = serverNamePartsByBackSlash[0]; - // check if hostname = localhost - return !hostname.Equals("localhost", StringComparison.OrdinalIgnoreCase); + // check if hostname = localhost or . or 127.0.0.1 or ::1 + if (hostname == "localhost" || hostname == "." || hostname == "127.0.0.1" || hostname == "::1") return false; + + return true; } private static bool RunningAsUWPApp() diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index cf0afd0fe5..881ab130ba 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -115,46 +115,37 @@ public static void PortNumberInSPNTest() private static string GetSPNInfo(string datasource, out int out_port) { - Assembly systemData = Assembly.GetAssembly(typeof(SqlConnection)); + Assembly sqlConnectionAssembly = Assembly.GetAssembly(typeof(SqlConnection)); // Get all required types using reflection - Type SniProxy = systemData.GetType("Microsoft.Data.SqlClient.SNI.SNIProxy"); - Type SSRP = systemData.GetType("Microsoft.Data.SqlClient.SNI.SSRP"); - Type DataSource = systemData.GetType("Microsoft.Data.SqlClient.SNI.DataSource"); - Type TimeoutTimer = systemData.GetType("Microsoft.Data.ProviderBase.TimeoutTimer"); + Type sniProxyType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.SNIProxy"); + Type ssrpType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.SSRP"); + Type dataSourceType = sqlConnectionAssembly.GetType("Microsoft.Data.SqlClient.SNI.DataSource"); + Type timeoutTimerType = sqlConnectionAssembly.GetType("Microsoft.Data.ProviderBase.TimeoutTimer"); // Used in Datasource constructor param type array - Type[] types = new Type[1]; - types[0] = typeof(string); + Type[] types = new Type[1] { typeof(string) }; // Used in GetSqlServerSPNs function param types array - Type[] types2 = new Type[2]; - types2[0] = DataSource; - types2[1] = typeof(string); + Type[] types2 = new Type[2] { dataSourceType, typeof(string) }; // GetPortByInstanceName parameters array - Type[] types3 = new Type[5]; - types3[0] = typeof(string); - types3[1] = typeof(string); - types3[2] = TimeoutTimer; - types3[3] = typeof(bool); - types3[4] = typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference); + Type[] types3 = new Type[5] { typeof(string), typeof(string), timeoutTimerType, typeof(bool), typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference) }; // TimeoutTimer.StartSecondsTimeout params - Type[] types4 = new Type[1]; - types4[0] = typeof(int); + Type[] types4 = new Type[1] { typeof(int) }; // Get all types constructors - ConstructorInfo sniProxyCtor = SniProxy.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo SSRPCtor = SSRP.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo datasSourceCtor = DataSource.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types , null); - ConstructorInfo timeoutTimerCtor = TimeoutTimer.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo sniProxyCtor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo SSRPCtor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); + ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types , null); + ConstructorInfo timeoutTimerCtor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); // Instantiate SNIProxy var sniProxy = sniProxyCtor.Invoke(new object[] { }); - // Instatntiate datasource - var details = datasSourceCtor.Invoke(new object[] { datasource }); + // Instantiate datasource + var details = dataSourceCtor.Invoke(new object[] { datasource }); // Instantiate SSRP var ssrp = SSRPCtor.Invoke(new object[] { }); From 590749702117fe7e2bd583815743068f2d2ae0e0 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 24 Nov 2023 08:37:20 -0800 Subject: [PATCH 10/17] Use meaningful variable names per Davoud. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 881ab130ba..f251ecfeac 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -124,21 +124,21 @@ private static string GetSPNInfo(string datasource, out int out_port) Type timeoutTimerType = sqlConnectionAssembly.GetType("Microsoft.Data.ProviderBase.TimeoutTimer"); // Used in Datasource constructor param type array - Type[] types = new Type[1] { typeof(string) }; + Type[] dataSourceConstructorTypesArray = new Type[] { typeof(string) }; // Used in GetSqlServerSPNs function param types array - Type[] types2 = new Type[2] { dataSourceType, typeof(string) }; + Type[] getSqlServerSPNsTypesArray = new Type[] { dataSourceType, typeof(string) }; // GetPortByInstanceName parameters array - Type[] types3 = new Type[5] { typeof(string), typeof(string), timeoutTimerType, typeof(bool), typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference) }; + Type[] getPortByInstanceNameTypesArray = new Type[] { typeof(string), typeof(string), timeoutTimerType, typeof(bool), typeof(Microsoft.Data.SqlClient.SqlConnectionIPAddressPreference) }; // TimeoutTimer.StartSecondsTimeout params - Type[] types4 = new Type[1] { typeof(int) }; + Type[] startSecondsTimeoutTypesArray = new Type[] { typeof(int) }; // Get all types constructors ConstructorInfo sniProxyCtor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); ConstructorInfo SSRPCtor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types , null); + ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray , null); ConstructorInfo timeoutTimerCtor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); // Instantiate SNIProxy @@ -154,24 +154,24 @@ private static string GetSPNInfo(string datasource, out int out_port) var timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); // Get TimeoutTimer.StartSecondsTimeout Method - MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types4, null); + MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); // Create a timeoutTimer that expires in 30 seconds timeoutTimer = startSecondsTimeout.Invoke(details, new object[] { 30 }); // Parse the datasource to separate the server name and instance name - MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types, null); + MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); var dataSrcInfo = ParseServerName.Invoke(details, new object[] { datasource }); // Get the GetPortByInstanceName method of SSRP - MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types3, null); + MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); // Get the server name PropertyInfo serverInfo = dataSrcInfo.GetType().GetProperty("ServerName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - var serverName = serverInfo.GetValue(dataSrcInfo, null).ToString(); + string serverName = serverInfo.GetValue(dataSrcInfo, null).ToString(); // Get the instance name PropertyInfo instanceNameInfo = dataSrcInfo.GetType().GetProperty("InstanceName", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); - var instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); + string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); // Get the port number using the GetPortByInstanceName method of SSRP var port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 } ); @@ -182,13 +182,13 @@ private static string GetSPNInfo(string datasource, out int out_port) // Prepare the GetSqlServerSPNs method string serverSPN = ""; - MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, types2, null); + MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); // Finally call GetSqlServerSPNs dynamic result = getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); - // MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" - var spnInfo = System.Text.Encoding.Unicode.GetString(result[0]); + // Example result: MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" + string spnInfo = Encoding.Unicode.GetString(result[0]); out_port = (int)port; From 589d5bd15d0c67147b27700bff9fa17d78c79a90 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Fri, 24 Nov 2023 11:12:39 -0800 Subject: [PATCH 11/17] Use object type instead of var type to make code easier to understand per Javad. --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index f251ecfeac..39920a7a61 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -142,16 +142,16 @@ private static string GetSPNInfo(string datasource, out int out_port) ConstructorInfo timeoutTimerCtor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); // Instantiate SNIProxy - var sniProxy = sniProxyCtor.Invoke(new object[] { }); + object sniProxy = sniProxyCtor.Invoke(new object[] { }); // Instantiate datasource - var details = dataSourceCtor.Invoke(new object[] { datasource }); + object details = dataSourceCtor.Invoke(new object[] { datasource }); // Instantiate SSRP - var ssrp = SSRPCtor.Invoke(new object[] { }); + object ssrp = SSRPCtor.Invoke(new object[] { }); // Instantiate TimeoutTimer - var timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); + object timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); // Get TimeoutTimer.StartSecondsTimeout Method MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); @@ -160,7 +160,7 @@ private static string GetSPNInfo(string datasource, out int out_port) // Parse the datasource to separate the server name and instance name MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - var dataSrcInfo = ParseServerName.Invoke(details, new object[] { datasource }); + object dataSrcInfo = ParseServerName.Invoke(details, new object[] { datasource }); // Get the GetPortByInstanceName method of SSRP MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); @@ -174,7 +174,7 @@ private static string GetSPNInfo(string datasource, out int out_port) string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); // Get the port number using the GetPortByInstanceName method of SSRP - var port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 } ); + object port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 } ); // Set the resolved port property of datasource PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); @@ -185,6 +185,7 @@ private static string GetSPNInfo(string datasource, out int out_port) MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); // Finally call GetSqlServerSPNs + // Use dynamic type for indexing to work at design time dynamic result = getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); // Example result: MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" From a7bd4edd60e869064f456b521e5d9b98c5399130 Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Tue, 28 Nov 2023 16:47:29 -0800 Subject: [PATCH 12/17] Implement suggestions from PR Review. --- .../ManualTests/DataCommon/DataTestUtility.cs | 26 ++++++++----------- .../SQL/InstanceNameTest/InstanceNameTest.cs | 18 +++++++++---- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 91516ab1e4..31c8a47792 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1073,28 +1073,24 @@ public static string GetMachineFQDN(string hostname) return fqdn.ToString(); } - public static bool IsIntegratedSecurity() - { - SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); - return builder.IntegratedSecurity; - } - public static bool IsNotLocalhost() { // get the tcp connection string SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); - // remove tcp from connection string - string dataSourceStr = builder.DataSource.Replace("tcp:", ""); - // create a string array - string[] serverNamePartsByBackSlash = dataSourceStr.Split('\\'); - // first element of array is the hostname - string hostname = serverNamePartsByBackSlash[0]; + string hostname = ""; - // check if hostname = localhost or . or 127.0.0.1 or ::1 - if (hostname == "localhost" || hostname == "." || hostname == "127.0.0.1" || hostname == "::1") return false; + // parse the datasource + ParseDataSource(builder.DataSource, out hostname, out _, out _); - return true; + // hostname must not be localhost, ., 127.0.0.1 nor ::1 + return !(new string[] { "localhost", ".", "127.0.0.1", "::1" }).Contains(hostname.ToLowerInvariant()); + + } + + public static bool IsKerberosManagedSNI() + { + return (AreConnStringsSetup() && IsUsingManagedSNI() && IsNotLocalhost() && IsKerberosTest && IsNotAzureServer() && IsNotAzureSynapse()); } private static bool RunningAsUWPApp() diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 39920a7a61..b1e0b85266 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -84,12 +84,20 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } - // Note: This Unit test was tested in a VM within the sqldrv.ad domain. i.e. from server sqldrv-win22 and - // is connecting to a Sql Server using Kerberos at sqldrv-sql22 server in the same domain. - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsUsingManagedSNI), nameof(DataTestUtility.IsNotLocalhost), nameof(DataTestUtility.IsKerberosTest), nameof(DataTestUtility.IsNotAzureServer), nameof(DataTestUtility.IsNotAzureSynapse))] + // Note: This Unit test was tested in a domain-joined VM connecting to a remote + // SQL Server using Kerberos in the same domain. + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosManagedSNI))] public static void PortNumberInSPNTest() { - SqlConnectionStringBuilder builder = new(DataTestUtility.TCPConnectionString); + string connStr = DataTestUtility.TCPConnectionString; + // If config.json.SupportsIntegratedSecurity = true, replace all keys defined below with Integrated Security=true + if (DataTestUtility.IsIntegratedSecuritySetup()) + { + string[] removeKeys = { "Authentication", "User ID", "Password", "UID", "PWD", "Trusted_Connection" }; + connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.TCPConnectionString, removeKeys) + $"Integrated Security=true"; + } + + SqlConnectionStringBuilder builder = new(connStr); Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); @@ -188,7 +196,7 @@ private static string GetSPNInfo(string datasource, out int out_port) // Use dynamic type for indexing to work at design time dynamic result = getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); - // Example result: MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" + // Example result: MSSQLSvc/machine.domain.tld:port" string spnInfo = Encoding.Unicode.GetString(result[0]); out_port = (int)port; From 94457f9d62b16baac1dfba185af7c9564557079a Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Tue, 28 Nov 2023 16:57:05 -0800 Subject: [PATCH 13/17] Removed all instance of top secret domain name. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index b1e0b85266..b97f7946bc 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -114,7 +114,7 @@ public static void PortNumberInSPNTest() int port = -1; string spnInfo = GetSPNInfo(builder.DataSource, out port); - // sample output to validate = MSSQLSvc/sqldrv-sql22.sqldrv.ad:1433" + // sample output to validate = MSSQLSvc/machine.domain.tld:port" Assert.Contains($"MSSQLSvc/{hostname}", spnInfo); // the local_tcp_port Port is the same as the inferred port from instance name Assert.Equal(Port, port); From 6c6f6dca41780a1812c5333918682adbb3a9e37c Mon Sep 17 00:00:00 2001 From: v-arellegue Date: Wed, 29 Nov 2023 17:04:18 -0800 Subject: [PATCH 14/17] Moved IsManagedKerberos function from DataTestUtility class to InstanceNameTest class. --- .../tests/ManualTests/DataCommon/DataTestUtility.cs | 5 ----- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 7 ++++++- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 31c8a47792..e167a264a2 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -1088,11 +1088,6 @@ public static bool IsNotLocalhost() } - public static bool IsKerberosManagedSNI() - { - return (AreConnStringsSetup() && IsUsingManagedSNI() && IsNotLocalhost() && IsKerberosTest && IsNotAzureServer() && IsNotAzureSynapse()); - } - private static bool RunningAsUWPApp() { if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index b97f7946bc..171b9b59c5 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,7 +86,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a domain-joined VM connecting to a remote // SQL Server using Kerberos in the same domain. - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsKerberosManagedSNI))] + [ConditionalFact(nameof(IsKerberosManagedSNI))] public static void PortNumberInSPNTest() { string connStr = DataTestUtility.TCPConnectionString; @@ -204,6 +204,11 @@ private static string GetSPNInfo(string datasource, out int out_port) return spnInfo; } + private static bool IsKerberosManagedSNI() + { + return (DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsUsingManagedSNI() && DataTestUtility.IsNotLocalhost() && DataTestUtility.IsKerberosTest && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); + } + private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03; From 030d8f7f5e6e420b03eec9c587908e68b7d58335 Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Wed, 6 Dec 2023 21:45:26 +0000 Subject: [PATCH 15/17] Applied suggestions by Davoud "Even though this PR suggests an update on ManagedSNI, we expect the same behavior on native SNI.". --- .../ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 171b9b59c5..f3bfc95f35 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,7 +86,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a domain-joined VM connecting to a remote // SQL Server using Kerberos in the same domain. - [ConditionalFact(nameof(IsKerberosManagedSNI))] + [ConditionalFact(nameof(IsKerberos))] public static void PortNumberInSPNTest() { string connStr = DataTestUtility.TCPConnectionString; @@ -204,9 +204,9 @@ private static string GetSPNInfo(string datasource, out int out_port) return spnInfo; } - private static bool IsKerberosManagedSNI() + private static bool IsKerberos() { - return (DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsUsingManagedSNI() && DataTestUtility.IsNotLocalhost() && DataTestUtility.IsKerberosTest && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); + return (DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotLocalhost() && DataTestUtility.IsKerberosTest && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); } private static bool IsBrowserAlive(string browserHostname) From 4a92bc7f73851ee168f282cd8b184c73d642f93a Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Thu, 7 Dec 2023 18:40:31 +0000 Subject: [PATCH 16/17] Disable unit test for Linux SPN Port# issue using ActiveIssue 27824. --- .../Microsoft/Data/SqlClient/SNI/SNIProxy.cs | 2 +- .../SQL/InstanceNameTest/InstanceNameTest.cs | 46 +++++++++++-------- 2 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs index 93b353f9c6..3df369a2f6 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs @@ -230,7 +230,7 @@ private static byte[][] GetSqlServerSPNs(DataSource dataSource, string serverSPN } else if (!string.IsNullOrWhiteSpace(dataSource.InstanceName)) { - postfix = (dataSource._connectionProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName); + postfix = dataSource._connectionProtocol == DataSource.Protocol.TCP ? dataSource.ResolvedPort.ToString() : dataSource.InstanceName; } SqlClientEventSource.Log.TryTraceEvent("SNIProxy.GetSqlServerSPN | Info | ServerName {0}, InstanceName {1}, Port {2}, postfix {3}", dataSource?.ServerName, dataSource?.InstanceName, dataSource?.Port, postfix); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index f3bfc95f35..1ddf8fce71 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,6 +86,8 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a domain-joined VM connecting to a remote // SQL Server using Kerberos in the same domain. + // Disable this test for now as Davoud said there is an issue. + [ActiveIssue("27824")] // Per Davoud, "With specifying instance name and port number, this method call always returns false!" [ConditionalFact(nameof(IsKerberos))] public static void PortNumberInSPNTest() { @@ -99,9 +101,12 @@ public static void PortNumberInSPNTest() SqlConnectionStringBuilder builder = new(connStr); - Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); + Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName), "Data source to be parsed must contain a host name and instance name"); - if (IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName)) + bool condition = IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName); + Assert.True(condition, "Browser service is not running or instance name is invalid"); + + if (condition) { using SqlConnection connection = new(builder.ConnectionString); connection.Open(); @@ -109,15 +114,15 @@ public static void PortNumberInSPNTest() using SqlDataReader reader = command.ExecuteReader(); Assert.True(reader.Read(), "Expected to receive one row data"); Assert.Equal("KERBEROS", reader.GetString(0)); - int Port = reader.GetInt32(1); + int localTcpPort = reader.GetInt32(1); - int port = -1; - string spnInfo = GetSPNInfo(builder.DataSource, out port); + int spnPort = -1; + string spnInfo = GetSPNInfo(builder.DataSource, out spnPort); - // sample output to validate = MSSQLSvc/machine.domain.tld:port" + // sample output to validate = MSSQLSvc/machine.domain.tld:spnPort" Assert.Contains($"MSSQLSvc/{hostname}", spnInfo); - // the local_tcp_port Port is the same as the inferred port from instance name - Assert.Equal(Port, port); + // the local_tcp_port should be the same as the inferred SPN port from instance name + Assert.Equal(localTcpPort, spnPort); } } @@ -146,17 +151,17 @@ private static string GetSPNInfo(string datasource, out int out_port) // Get all types constructors ConstructorInfo sniProxyCtor = sniProxyType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); ConstructorInfo SSRPCtor = ssrpType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); - ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray , null); + ConstructorInfo dataSourceCtor = dataSourceType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); ConstructorInfo timeoutTimerCtor = timeoutTimerType.GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, Type.EmptyTypes, null); // Instantiate SNIProxy - object sniProxy = sniProxyCtor.Invoke(new object[] { }); + object sniProxy = sniProxyCtor.Invoke(new object[] { }); // Instantiate datasource - object details = dataSourceCtor.Invoke(new object[] { datasource }); + object dataSourceObj = dataSourceCtor.Invoke(new object[] { datasource }); // Instantiate SSRP - object ssrp = SSRPCtor.Invoke(new object[] { }); + object ssrp = SSRPCtor.Invoke(new object[] { }); // Instantiate TimeoutTimer object timeoutTimer = timeoutTimerCtor.Invoke(new object[] { }); @@ -164,11 +169,11 @@ private static string GetSPNInfo(string datasource, out int out_port) // Get TimeoutTimer.StartSecondsTimeout Method MethodInfo startSecondsTimeout = timeoutTimer.GetType().GetMethod("StartSecondsTimeout", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, startSecondsTimeoutTypesArray, null); // Create a timeoutTimer that expires in 30 seconds - timeoutTimer = startSecondsTimeout.Invoke(details, new object[] { 30 }); + timeoutTimer = startSecondsTimeout.Invoke(dataSourceObj, new object[] { 30 }); // Parse the datasource to separate the server name and instance name - MethodInfo ParseServerName = details.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); - object dataSrcInfo = ParseServerName.Invoke(details, new object[] { datasource }); + MethodInfo ParseServerName = dataSourceObj.GetType().GetMethod("ParseServerName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, dataSourceConstructorTypesArray, null); + object dataSrcInfo = ParseServerName.Invoke(dataSourceObj, new object[] { datasource }); // Get the GetPortByInstanceName method of SSRP MethodInfo getPortByInstanceName = ssrp.GetType().GetMethod("GetPortByInstanceName", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getPortByInstanceNameTypesArray, null); @@ -182,7 +187,7 @@ private static string GetSPNInfo(string datasource, out int out_port) string instanceName = instanceNameInfo.GetValue(dataSrcInfo, null).ToString(); // Get the port number using the GetPortByInstanceName method of SSRP - object port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 } ); + object port = getPortByInstanceName.Invoke(ssrp, parameters: new object[] { serverName, instanceName, timeoutTimer, false, 0 }); // Set the resolved port property of datasource PropertyInfo resolvedPortInfo = dataSrcInfo.GetType().GetProperty("ResolvedPort", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); @@ -193,8 +198,7 @@ private static string GetSPNInfo(string datasource, out int out_port) MethodInfo getSqlServerSPNs = sniProxy.GetType().GetMethod("GetSqlServerSPNs", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic, null, CallingConventions.Any, getSqlServerSPNsTypesArray, null); // Finally call GetSqlServerSPNs - // Use dynamic type for indexing to work at design time - dynamic result = getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); + byte[][] result = (byte[][])getSqlServerSPNs.Invoke(sniProxy, new object[] { dataSrcInfo, serverSPN }); // Example result: MSSQLSvc/machine.domain.tld:port" string spnInfo = Encoding.Unicode.GetString(result[0]); @@ -206,7 +210,11 @@ private static string GetSPNInfo(string datasource, out int out_port) private static bool IsKerberos() { - return (DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotLocalhost() && DataTestUtility.IsKerberosTest && DataTestUtility.IsNotAzureServer() && DataTestUtility.IsNotAzureSynapse()); + return (DataTestUtility.AreConnStringsSetup() + && DataTestUtility.IsNotLocalhost() + && DataTestUtility.IsKerberosTest + && DataTestUtility.IsNotAzureServer() + && DataTestUtility.IsNotAzureSynapse()); } private static bool IsBrowserAlive(string browserHostname) From 2c8e139383dd0ae6d9cd2216f2bd85b04332a67c Mon Sep 17 00:00:00 2001 From: Aris Rellegue Date: Thu, 7 Dec 2023 20:07:10 +0000 Subject: [PATCH 17/17] Changed comment for ActiveIssue 27824. --- .../tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs index 1ddf8fce71..087b44d964 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -86,8 +86,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove // Note: This Unit test was tested in a domain-joined VM connecting to a remote // SQL Server using Kerberos in the same domain. - // Disable this test for now as Davoud said there is an issue. - [ActiveIssue("27824")] // Per Davoud, "With specifying instance name and port number, this method call always returns false!" + [ActiveIssue("27824")] // When specifying instance name and port number, this method call always returns false [ConditionalFact(nameof(IsKerberos))] public static void PortNumberInSPNTest() {