From 71982737163b8968774ac6afb49f7748958f8680 Mon Sep 17 00:00:00 2001 From: David Engel Date: Fri, 15 Apr 2022 16:53:33 -0700 Subject: [PATCH 1/9] Add support for aliases on netcore+Windows --- .../src/Microsoft.Data.SqlClient.csproj | 6 +- .../SqlClient/SqlInternalConnectionTds.cs | 35 ++++- .../netfx/src/Microsoft.Data.SqlClient.csproj | 4 +- .../src/Microsoft/Data/Common/AdapterUtil.cs | 62 +++++---- .../Data/SqlClient/SqlConnectionString.cs | 29 +++-- .../Data/SqlClient/TdsParserStaticMethods.cs | 123 ++++++++++++------ 6 files changed, 170 insertions(+), 89 deletions(-) rename src/Microsoft.Data.SqlClient/{netfx => }/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs (66%) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index 70fe79041e..0c32a3ed3a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -172,7 +172,7 @@ Microsoft\Data\SqlClient\RowsCopiedEventHandler.cs - + Microsoft\Data\SqlClient\SqlSequentialTextReader.cs @@ -475,6 +475,9 @@ Microsoft\Data\SqlClient\TdsParameterSetter.cs + + Microsoft\Data\SqlClient\TdsParserStaticMethods.cs + Microsoft\Data\SqlClient\TdsRecordBufferSetter.cs @@ -649,7 +652,6 @@ - diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index dcbc62ef0d..92ef9b6043 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -1853,11 +1853,36 @@ private void ResolveExtendedServerName(ServerInfo serverInfo, bool aliasLookup, string host = serverInfo.UserServerName; string protocol = serverInfo.UserProtocol; - //TODO: fix local host enforcement with datadirectory and failover - if (options.EnforceLocalHost) - { - // verify LocalHost for |DataDirectory| usage - SqlConnectionString.VerifyLocalHostAndFixup(ref host, true, true /*fix-up to "."*/); + if (aliasLookup) + { // We skip this for UserInstances... + // Perform registry lookup to see if host is an alias. It will appropriately set host and protocol, if an Alias. + // Check if it was already resolved, during CR reconnection _currentSessionData values will be copied from + // _reconnectSessonData of the previous connection + if (_currentSessionData != null && !string.IsNullOrEmpty(host)) + { + Tuple hostPortPair; + if (_currentSessionData._resolvedAliases.TryGetValue(host, out hostPortPair)) + { + host = hostPortPair.Item1; + protocol = hostPortPair.Item2; + } + else + { + TdsParserStaticMethods.AliasRegistryLookup(ref host, ref protocol); + _currentSessionData._resolvedAliases.Add(serverInfo.UserServerName, new Tuple(host, protocol)); + } + } + else + { + TdsParserStaticMethods.AliasRegistryLookup(ref host, ref protocol); + } + + //TODO: fix local host enforcement with datadirectory and failover + if (options.EnforceLocalHost) + { + // verify LocalHost for |DataDirectory| usage + SqlConnectionString.VerifyLocalHostAndFixup(ref host, true, true /*fix-up to "."*/); + } } serverInfo.SetDerivedNames(protocol, host); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj index b1e683badc..19696260a5 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -539,6 +539,9 @@ Microsoft\Data\SqlClient\TdsParameterSetter.cs + + Microsoft\Data\SqlClient\TdsParserStaticMethods.cs + Microsoft\Data\SqlClient\TdsRecordBufferSetter.cs @@ -634,7 +637,6 @@ - diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index d7f6ce7cf0..d61e98ee9e 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -1229,8 +1229,39 @@ static internal Exception InvalidMixedUsageOfCredentialAndAccessToken() internal static bool IsEmpty(string str) => string.IsNullOrEmpty(str); internal static readonly IntPtr s_ptrZero = IntPtr.Zero; + + [ResourceExposure(ResourceScope.Machine)] + [ResourceConsumption(ResourceScope.Machine)] + internal static object LocalMachineRegistryValue(string subkey, string queryvalue) + { // MDAC 77697 + if (!System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return null; + } + + (new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028 + try + { + using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false)) + { + return key?.GetValue(queryvalue); + } + } + catch (SecurityException e) + { + // Even though we assert permission - it's possible there are + // ACL's on registry that cause SecurityException to be thrown. + ADP.TraceExceptionWithoutRethrow(e); + return null; + } + finally + { + RegistryPermission.RevertAssert(); + } + } + #if NETFRAMEWORK -#region netfx project only + #region netfx project only internal static Task CreatedTaskWithException(Exception ex) { TaskCompletionSource completion = new(); @@ -1437,31 +1468,6 @@ internal static string GetComputerNameDnsFullyQualified() return value; } - [ResourceExposure(ResourceScope.Machine)] - [ResourceConsumption(ResourceScope.Machine)] - internal static object LocalMachineRegistryValue(string subkey, string queryvalue) - { // MDAC 77697 - (new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028 - try - { - using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false)) - { - return key?.GetValue(queryvalue); - } - } - catch (SecurityException e) - { - // Even though we assert permission - it's possible there are - // ACL's on registry that cause SecurityException to be thrown. - ADP.TraceExceptionWithoutRethrow(e); - return null; - } - finally - { - RegistryPermission.RevertAssert(); - } - } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.MayFail)] internal static IntPtr IntPtrOffset(IntPtr pbase, int offset) { @@ -1473,9 +1479,9 @@ internal static IntPtr IntPtrOffset(IntPtr pbase, int offset) return (IntPtr)checked(pbase.ToInt64() + offset); } -#endregion + #endregion #else -#region netcore project only + #region netcore project only internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) { // Don't capture the current ExecutionContext and its AsyncLocals onto diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs index f1a488c4b6..aac7c04913 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs @@ -1102,20 +1102,6 @@ internal PoolBlockingPeriod ConvertValueToPoolBlockingPeriod() } } -#if NETFRAMEWORK - protected internal override PermissionSet CreatePermissionSet() - { - PermissionSet permissionSet = new(PermissionState.None); - permissionSet.AddPermission(new SqlClientPermission(this)); - return permissionSet; - } - - internal bool ConvertValueToEncrypt() - { - bool defaultEncryptValue = !Parsetable.ContainsKey(KEY.Authentication) ? DEFAULT.Encrypt : true; - return ConvertValueToBoolean(KEY.Encrypt, defaultEncryptValue); - } - static internal Hashtable NetlibMapping() { const int NetLibCount = 8; @@ -1166,6 +1152,21 @@ internal static class NETLIB } private static Hashtable s_netlibMapping; + +#if NETFRAMEWORK + protected internal override PermissionSet CreatePermissionSet() + { + PermissionSet permissionSet = new(PermissionState.None); + permissionSet.AddPermission(new SqlClientPermission(this)); + return permissionSet; + } + + internal bool ConvertValueToEncrypt() + { + bool defaultEncryptValue = !Parsetable.ContainsKey(KEY.Authentication) ? DEFAULT.Encrypt : true; + return ConvertValueToBoolean(KEY.Encrypt, defaultEncryptValue); + } + private readonly bool _connectionReset; private readonly bool _contextConnection; private readonly bool _transparentNetworkIPResolution; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs similarity index 66% rename from src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs rename to src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs index 7be7f61bb4..235339cc5d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs @@ -7,6 +7,7 @@ using System.Globalization; using System.Runtime.Versioning; using System.Security.Permissions; +using System.Runtime.InteropServices; namespace Microsoft.Data.SqlClient { @@ -23,6 +24,11 @@ private TdsParserStaticMethods() { /* prevent utility class from being insantiat [ResourceConsumption(ResourceScope.Machine, ResourceScope.Machine)] static internal void AliasRegistryLookup(ref string host, ref string protocol) { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + if (!ADP.IsEmpty(host)) { const String folder = "SOFTWARE\\Microsoft\\MSSQLServer\\Client\\ConnectTo"; @@ -88,12 +94,19 @@ static internal void AliasRegistryLookup(ref string host, ref string protocol) } } - // Encrypt password to be sent to SQL Server + // Obfuscate password to be sent to SQL Server + // Blurb from the TDS spec at https://msdn.microsoft.com/en-us/library/dd304523.aspx + // "Before submitting a password from the client to the server, for every byte in the password buffer + // starting with the position pointed to by IbPassword, the client SHOULD first swap the four high bits + // with the four low bits and then do a bit-XOR with 0xA5 (10100101). After reading a submitted password, + // for every byte in the password buffer starting with the position pointed to by IbPassword, the server SHOULD + // first do a bit-XOR with 0xA5 (10100101) and then swap the four high bits with the four low bits." + // The password exchange during Login phase happens over a secure channel i.e. SSL/TLS // Note: The same logic is used in SNIPacketSetData (SniManagedWrapper) to encrypt passwords stored in SecureString // If this logic changed, SNIPacketSetData needs to be changed as well internal static byte[] ObfuscatePassword(string password) { - byte[] bEnc = new byte[password.Length << 1]; + byte[] bObfuscated = new byte[password.Length << 1]; int s; byte bLo; byte bHi; @@ -103,62 +116,92 @@ internal static byte[] ObfuscatePassword(string password) s = (int)password[i]; bLo = (byte)(s & 0xff); bHi = (byte)((s >> 8) & 0xff); - bEnc[i << 1] = (byte)((((bLo & 0x0f) << 4) | (bLo >> 4)) ^ 0xa5); - bEnc[(i << 1) + 1] = (byte)((((bHi & 0x0f) << 4) | (bHi >> 4)) ^ 0xa5); + bObfuscated[i << 1] = (byte)((((bLo & 0x0f) << 4) | (bLo >> 4)) ^ 0xa5); + bObfuscated[(i << 1) + 1] = (byte)((((bHi & 0x0f) << 4) | (bHi >> 4)) ^ 0xa5); } - return bEnc; + return bObfuscated; } - [ResourceExposure(ResourceScope.None)] // SxS: we use this method for TDS login only - [ResourceConsumption(ResourceScope.Process, ResourceScope.Process)] - static internal int GetCurrentProcessIdForTdsLoginOnly() + internal static byte[] ObfuscatePassword(byte[] password) { - return Common.SafeNativeMethods.GetCurrentProcessId(); + byte bLo; + byte bHi; + + for (int i = 0; i < password.Length; i++) + { + bLo = (byte)(password[i] & 0x0f); + bHi = (byte)(password[i] & 0xf0); + password[i] = (byte)(((bHi >> 4) | (bLo << 4)) ^ 0xa5); + } + return password; + } + + private const int NoProcessId = -1; + private static int s_currentProcessId = NoProcessId; + internal static int GetCurrentProcessIdForTdsLoginOnly() + { + if (s_currentProcessId == NoProcessId) + { + // Pick up the process Id from the current process instead of randomly generating it. + // This would be helpful while tracing application related issues. + int processId; + using (System.Diagnostics.Process p = System.Diagnostics.Process.GetCurrentProcess()) + { + processId = p.Id; + } + System.Threading.Volatile.Write(ref s_currentProcessId, processId); + } + return s_currentProcessId; } - [SecurityPermission(SecurityAction.Assert, Flags = SecurityPermissionFlag.UnmanagedCode)] - [ResourceExposure(ResourceScope.None)] // SxS: we use this method for TDS login only - [ResourceConsumption(ResourceScope.Process, ResourceScope.Process)] - static internal Int32 GetCurrentThreadIdForTdsLoginOnly() + internal static int GetCurrentThreadIdForTdsLoginOnly() { -#pragma warning disable 618 - return AppDomain.GetCurrentThreadId(); // don't need this to be support fibres; -#pragma warning restore 618 + return Environment.CurrentManagedThreadId; } + private static byte[] s_nicAddress = null; [ResourceExposure(ResourceScope.None)] // SxS: we use MAC address for TDS login only [ResourceConsumption(ResourceScope.Machine, ResourceScope.Machine)] static internal byte[] GetNetworkPhysicalAddressForTdsLoginOnly() { - // NIC address is stored in NetworkAddress key. However, if NetworkAddressLocal key - // has a value that is not zero, then we cannot use the NetworkAddress key and must - // instead generate a random one. I do not fully understand why, this is simply what - // the native providers do. As for generation, I use a random number generator, which - // means that different processes on the same machine will have different NIC address - // values on the server. It is not ideal, but native does not have the same value for - // different processes either. - - const string key = "NetworkAddress"; - const string localKey = "NetworkAddressLocal"; - const string folder = "SOFTWARE\\Description\\Microsoft\\Rpc\\UuidTemporaryData"; - - int result = 0; - byte[] nicAddress = null; - - object temp = ADP.LocalMachineRegistryValue(folder, localKey); - if (temp is int) + if (s_nicAddress != null) { - result = (int)temp; + return s_nicAddress; } - if (result <= 0) + byte[] nicAddress = null; + + if (System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - temp = ADP.LocalMachineRegistryValue(folder, key); - if (temp is byte[]) + // NIC address is stored in NetworkAddress key. However, if NetworkAddressLocal key + // has a value that is not zero, then we cannot use the NetworkAddress key and must + // instead generate a random one. I do not fully understand why, this is simply what + // the native providers do. As for generation, I use a random number generator, which + // means that different processes on the same machine will have different NIC address + // values on the server. It is not ideal, but native does not have the same value for + // different processes either. + + const string key = "NetworkAddress"; + const string localKey = "NetworkAddressLocal"; + const string folder = "SOFTWARE\\Description\\Microsoft\\Rpc\\UuidTemporaryData"; + + int result = 0; + + object temp = ADP.LocalMachineRegistryValue(folder, localKey); + if (temp is int) { - nicAddress = (byte[])temp; + result = (int)temp; + } + + if (result <= 0) + { + temp = ADP.LocalMachineRegistryValue(folder, key); + if (temp is byte[]) + { + nicAddress = (byte[])temp; + } } } @@ -169,7 +212,9 @@ static internal byte[] GetNetworkPhysicalAddressForTdsLoginOnly() random.NextBytes(nicAddress); } - return nicAddress; + System.Threading.Interlocked.CompareExchange(ref s_nicAddress, nicAddress, null); + + return s_nicAddress; } // translates remaining time in stateObj (from user specified timeout) to timeout value for SNI From 8d85c79623c1d0c0016f1a48d6ea304918d0d6b2 Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 18 Apr 2022 13:36:53 -0700 Subject: [PATCH 2/9] Add test --- .../ManualTests/DataCommon/DataTestUtility.cs | 49 +++++++++++++++++++ .../SQL/ConnectivityTests/ConnectivityTest.cs | 27 ++++++++++ 2 files changed, 76 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index b6d2f73532..2e3c214d1d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -13,6 +13,7 @@ using System.Net; using System.Net.Sockets; using System.Security; +using System.Security.Principal; using System.Threading; using System.Threading.Tasks; using Microsoft.Identity.Client; @@ -296,6 +297,11 @@ public static bool AreConnStringsSetup() return !string.IsNullOrEmpty(NPConnectionString) && !string.IsNullOrEmpty(TCPConnectionString); } + public static bool IsTCPConnStringSetup() + { + return !string.IsNullOrEmpty(TCPConnectionString); + } + // Synapse: Always Encrypted is not supported with Azure Synapse. // Ref: https://feedback.azure.com/forums/307516-azure-synapse-analytics/suggestions/17858869-support-always-encrypted-in-sql-data-warehouse public static bool AreConnStringSetupForAE() @@ -828,6 +834,49 @@ public static string RetrieveValueFromConnStr(string connStr, string[] keywords) return res; } + public static bool IsRunningAsAdmin() + { + using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) + { + WindowsPrincipal principal = new WindowsPrincipal(identity); + return principal.IsInRole(WindowsBuiltInRole.Administrator); + } + } + + public static bool ParseDataSource(string dataSource, out string hostname, out int port, out string instanceName) + { + hostname = string.Empty; + port = -1; + instanceName = string.Empty; + + if (dataSource.Contains(",") && dataSource.Contains("\\")) + return false; + + if (dataSource.Contains(":")) + { + dataSource = dataSource.Substring(dataSource.IndexOf(":") + 1); + } + + if (dataSource.Contains(",")) + { + if (!Int32.TryParse(dataSource.Substring(dataSource.LastIndexOf(",") + 1), out port)) + { + return false; + } + dataSource = dataSource.Substring(0, dataSource.IndexOf(",") - 1); + } + + if (dataSource.Contains("\\")) + { + instanceName = dataSource.Substring(dataSource.LastIndexOf("\\") + 1); + dataSource = dataSource.Substring(0, dataSource.LastIndexOf("\\")); + } + + hostname = dataSource; + + return hostname.Length > 0 && hostname.IndexOfAny(new char[] { '\\', ':', ',' }) == -1; + } + public class AKVEventListener : BaseEventListener { public override string Name => AKVEventSourceName; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs index 41a52f48ac..da35ecd5b1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Threading; +using Microsoft.Win32; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests @@ -368,5 +369,31 @@ public static void ConnectionOpenDisableRetry() duration = timer.Elapsed; Assert.True(duration.Seconds > 5, $"Connection Open() with retries took less time than expected. Expect > 5 sec with transient fault handling. Took {duration.Seconds} sec."); // sqlConnection.Open(); } + + [PlatformSpecific(TestPlatforms.Windows)] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsRunningAsAdmin))] + public static void ConnectionAliasTest() + { + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); + if (!DataTestUtility.ParseDataSource(b.DataSource, out string hostname, out int port, out string instanceName) || + !string.IsNullOrEmpty(instanceName)) + { + // Only works with connection strings that parse successfully and don't include an instance name + return; + } + + b.DataSource = "TESTALIAS-" + Guid.NewGuid().ToString().Replace("-", ""); + using RegistryKey key = Registry.LocalMachine.OpenSubKey("SOFTWARE\\Microsoft\\MSSQLServer\\Client\\ConnectTo", true); + key.SetValue(b.DataSource, "DBMSSOCN," + hostname + "," + (port == -1 ? 1433 : port)); + try + { + using SqlConnection sqlConnection = new(b.ConnectionString); + sqlConnection.Open(); + } + finally + { + key.DeleteValue(b.DataSource); + } + } } } From 9b83392279c8805c29f7f24b01208149ebb36aa9 Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 18 Apr 2022 13:42:43 -0700 Subject: [PATCH 3/9] Revert spacing --- .../src/Microsoft/Data/Common/AdapterUtil.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index d61e98ee9e..b12446353d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -1479,9 +1479,9 @@ internal static IntPtr IntPtrOffset(IntPtr pbase, int offset) return (IntPtr)checked(pbase.ToInt64() + offset); } - #endregion +#endregion #else - #region netcore project only +#region netcore project only internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) { // Don't capture the current ExecutionContext and its AsyncLocals onto From fb98b361a44e70bdc5f1a95325f4c5c799143337 Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 18 Apr 2022 17:02:54 -0700 Subject: [PATCH 4/9] Better partition Windows/Unix code --- .../src/Microsoft.Data.SqlClient.csproj | 6 ++++ .../netfx/src/Microsoft.Data.SqlClient.csproj | 3 ++ .../src/Microsoft/Data/Common/AdapterUtil.cs | 32 +------------------ .../ManualTests/DataCommon/DataTestUtility.cs | 9 ------ .../SQL/ConnectivityTests/ConnectivityTest.cs | 12 ++++++- 5 files changed, 21 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index 0c32a3ed3a..83f2b8d5a1 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -655,6 +655,9 @@ + + Microsoft\Data\Common\AdapterUtil.Windows.cs + Microsoft\Data\Sql\SqlDataSourceEnumeratorNativeHelper.cs @@ -667,6 +670,9 @@ + + Microsoft\Data\Common\AdapterUtil.Unix.cs + diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj index 19696260a5..5d4ff87aa6 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj @@ -95,6 +95,9 @@ Microsoft\Data\Common\AdapterUtil.cs + + Microsoft\Data\Common\AdapterUtil.Windows.cs + Microsoft\Data\Common\DbConnectionStringCommon.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index b12446353d..f60b7a2685 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -42,7 +42,7 @@ namespace Microsoft.Data.Common /// This class is used so that there will be compile time checking of error messages. /// The resource Framework.txt will ensure proper string text based on the appropriate locale. /// - internal static class ADP + internal static partial class ADP { // NOTE: Initializing a Task in SQL CLR requires the "UNSAFE" permission set (http://msdn.microsoft.com/en-us/library/ms172338.aspx) // Therefore we are lazily initializing these Tasks to avoid forcing customers to use the "UNSAFE" set when they are actually using no Async features @@ -1230,36 +1230,6 @@ static internal Exception InvalidMixedUsageOfCredentialAndAccessToken() internal static bool IsEmpty(string str) => string.IsNullOrEmpty(str); internal static readonly IntPtr s_ptrZero = IntPtr.Zero; - [ResourceExposure(ResourceScope.Machine)] - [ResourceConsumption(ResourceScope.Machine)] - internal static object LocalMachineRegistryValue(string subkey, string queryvalue) - { // MDAC 77697 - if (!System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - return null; - } - - (new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028 - try - { - using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false)) - { - return key?.GetValue(queryvalue); - } - } - catch (SecurityException e) - { - // Even though we assert permission - it's possible there are - // ACL's on registry that cause SecurityException to be thrown. - ADP.TraceExceptionWithoutRethrow(e); - return null; - } - finally - { - RegistryPermission.RevertAssert(); - } - } - #if NETFRAMEWORK #region netfx project only internal static Task CreatedTaskWithException(Exception ex) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 2e3c214d1d..85c5fe5023 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -834,15 +834,6 @@ public static string RetrieveValueFromConnStr(string connStr, string[] keywords) return res; } - public static bool IsRunningAsAdmin() - { - using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) - { - WindowsPrincipal principal = new WindowsPrincipal(identity); - return principal.IsInRole(WindowsBuiltInRole.Administrator); - } - } - public static bool ParseDataSource(string dataSource, out string hostname, out int port, out string instanceName) { hostname = string.Empty; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs index da35ecd5b1..98d98f62cf 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Security.Principal; using System.Threading; using Microsoft.Win32; using Xunit; @@ -371,9 +372,18 @@ public static void ConnectionOpenDisableRetry() } [PlatformSpecific(TestPlatforms.Windows)] - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup), nameof(DataTestUtility.IsRunningAsAdmin))] + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup))] public static void ConnectionAliasTest() { + using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) + { + WindowsPrincipal principal = new(identity); + if (!principal.IsInRole(WindowsBuiltInRole.Administrator)) + { + return; + } + } + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); if (!DataTestUtility.ParseDataSource(b.DataSource, out string hostname, out int port, out string instanceName) || !string.IsNullOrEmpty(instanceName)) From a4634005e29b33a07813ccbfa8e430cca43c3c49 Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 18 Apr 2022 17:10:24 -0700 Subject: [PATCH 5/9] Add missing files --- .../Microsoft/Data/Common/AdapterUtil.Unix.cs | 24 +++++++++ .../Data/Common/AdapterUtil.Windows.cs | 49 +++++++++++++++++++ .../src/Microsoft/Data/Common/AdapterUtil.cs | 3 +- .../Data/SqlClient/TdsParserStaticMethods.cs | 5 -- .../ManualTests/DataCommon/DataTestUtility.cs | 3 +- 5 files changed, 75 insertions(+), 9 deletions(-) create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Unix.cs create mode 100644 src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Windows.cs diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Unix.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Unix.cs new file mode 100644 index 0000000000..8b84feecfc --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Unix.cs @@ -0,0 +1,24 @@ +// 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. + +namespace Microsoft.Data.Common +{ + /// + /// The class ADP defines the exceptions that are specific to the Adapters. + /// The class contains functions that take the proper informational variables and then construct + /// the appropriate exception with an error string obtained from the resource framework. + /// The exception is then returned to the caller, so that the caller may then throw from its + /// location so that the catcher of the exception will have the appropriate call stack. + /// This class is used so that there will be compile time checking of error messages. + /// The resource Framework.txt will ensure proper string text based on the appropriate locale. + /// + internal static partial class ADP + { + internal static object LocalMachineRegistryValue(string subkey, string queryvalue) + { + // No registry in non-Windows environments + return null; + } + } +} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Windows.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Windows.cs new file mode 100644 index 0000000000..c9d0f8d91a --- /dev/null +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.Windows.cs @@ -0,0 +1,49 @@ +// 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. + +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using System.Security; +using System.Security.Permissions; +using Microsoft.Win32; + +namespace Microsoft.Data.Common +{ + /// + /// The class ADP defines the exceptions that are specific to the Adapters. + /// The class contains functions that take the proper informational variables and then construct + /// the appropriate exception with an error string obtained from the resource framework. + /// The exception is then returned to the caller, so that the caller may then throw from its + /// location so that the catcher of the exception will have the appropriate call stack. + /// This class is used so that there will be compile time checking of error messages. + /// The resource Framework.txt will ensure proper string text based on the appropriate locale. + /// + internal static partial class ADP + { + [ResourceExposure(ResourceScope.Machine)] + [ResourceConsumption(ResourceScope.Machine)] + internal static object LocalMachineRegistryValue(string subkey, string queryvalue) + { // MDAC 77697 + (new RegistryPermission(RegistryPermissionAccess.Read, "HKEY_LOCAL_MACHINE\\" + subkey)).Assert(); // MDAC 62028 + try + { + using (RegistryKey key = Registry.LocalMachine.OpenSubKey(subkey, false)) + { + return key?.GetValue(queryvalue); + } + } + catch (SecurityException e) + { + // Even though we assert permission - it's possible there are + // ACL's on registry that cause SecurityException to be thrown. + ADP.TraceExceptionWithoutRethrow(e); + return null; + } + finally + { + RegistryPermission.RevertAssert(); + } + } + } +} diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index f60b7a2685..236bbe3902 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -1229,9 +1229,8 @@ static internal Exception InvalidMixedUsageOfCredentialAndAccessToken() internal static bool IsEmpty(string str) => string.IsNullOrEmpty(str); internal static readonly IntPtr s_ptrZero = IntPtr.Zero; - #if NETFRAMEWORK - #region netfx project only +#region netfx project only internal static Task CreatedTaskWithException(Exception ex) { TaskCompletionSource completion = new(); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs index 235339cc5d..414c35bbdb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs @@ -24,11 +24,6 @@ private TdsParserStaticMethods() { /* prevent utility class from being insantiat [ResourceConsumption(ResourceScope.Machine, ResourceScope.Machine)] static internal void AliasRegistryLookup(ref string host, ref string protocol) { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - return; - } - if (!ADP.IsEmpty(host)) { const String folder = "SOFTWARE\\Microsoft\\MSSQLServer\\Client\\ConnectTo"; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 85c5fe5023..9e5b10cdd7 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -13,11 +13,10 @@ using System.Net; using System.Net.Sockets; using System.Security; -using System.Security.Principal; using System.Threading; using System.Threading.Tasks; -using Microsoft.Identity.Client; using Microsoft.Data.SqlClient.TestUtilities; +using Microsoft.Identity.Client; using Xunit; namespace Microsoft.Data.SqlClient.ManualTesting.Tests From 990809f63f7606f695c120ca4a3f37934f90ee5a Mon Sep 17 00:00:00 2001 From: David Engel Date: Mon, 18 Apr 2022 17:35:21 -0700 Subject: [PATCH 6/9] Fix attempt --- .../ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs index 98d98f62cf..de4d766fa4 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Diagnostics; +using System.Runtime.InteropServices; using System.Security.Principal; using System.Threading; using Microsoft.Win32; @@ -375,6 +376,11 @@ public static void ConnectionOpenDisableRetry() [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup))] public static void ConnectionAliasTest() { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + return; + } + using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) { WindowsPrincipal principal = new(identity); From 297514460f6403eb1426c30c8637f782fcfdcf5b Mon Sep 17 00:00:00 2001 From: David Engel Date: Tue, 19 Apr 2022 12:54:01 -0700 Subject: [PATCH 7/9] Re-work new test --- .../SQL/ConnectivityTests/ConnectivityTest.cs | 47 +++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs index de4d766fa4..32115b96d1 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/ConnectivityTest.cs @@ -372,13 +372,13 @@ public static void ConnectionOpenDisableRetry() Assert.True(duration.Seconds > 5, $"Connection Open() with retries took less time than expected. Expect > 5 sec with transient fault handling. Took {duration.Seconds} sec."); // sqlConnection.Open(); } - [PlatformSpecific(TestPlatforms.Windows)] - [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsTCPConnStringSetup))] - public static void ConnectionAliasTest() + private const string ConnectToPath = "SOFTWARE\\Microsoft\\MSSQLServer\\Client\\ConnectTo"; + private static bool CanCreateAliases() { - if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || + !DataTestUtility.IsTCPConnStringSetup()) { - return; + return false; } using (WindowsIdentity identity = WindowsIdentity.GetCurrent()) @@ -386,20 +386,51 @@ public static void ConnectionAliasTest() WindowsPrincipal principal = new(identity); if (!principal.IsInRole(WindowsBuiltInRole.Administrator)) { - return; + return false; } } + using RegistryKey key = Registry.LocalMachine.OpenSubKey(ConnectToPath, true); + if (key == null) + { + // Registry not writable + return false; + } + + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); + if (!DataTestUtility.ParseDataSource(b.DataSource, out string hostname, out int port, out string instanceName) || + !string.IsNullOrEmpty(instanceName)) + { + return false; + } + + return true; + } + + [PlatformSpecific(TestPlatforms.Windows)] + [ConditionalFact(nameof(CanCreateAliases))] + public static void ConnectionAliasTest() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + throw new Exception("Alias test only valid on Windows"); + } + + if (!CanCreateAliases()) + { + throw new Exception("Unable to create aliases in this environment. Windows + Admin + non-instance data source required."); + } + SqlConnectionStringBuilder b = new(DataTestUtility.TCPConnectionString); if (!DataTestUtility.ParseDataSource(b.DataSource, out string hostname, out int port, out string instanceName) || !string.IsNullOrEmpty(instanceName)) { // Only works with connection strings that parse successfully and don't include an instance name - return; + throw new Exception("Unable to create aliases in this configuration. Parsable data source without instance required."); } b.DataSource = "TESTALIAS-" + Guid.NewGuid().ToString().Replace("-", ""); - using RegistryKey key = Registry.LocalMachine.OpenSubKey("SOFTWARE\\Microsoft\\MSSQLServer\\Client\\ConnectTo", true); + using RegistryKey key = Registry.LocalMachine.OpenSubKey(ConnectToPath, true); key.SetValue(b.DataSource, "DBMSSOCN," + hostname + "," + (port == -1 ? 1433 : port)); try { From 369439721ce7f9d816127c90591f04003fc0dd3f Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 19 May 2022 14:26:47 -0700 Subject: [PATCH 8/9] Applying review suggestions --- .../src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs index 414c35bbdb..93aac3fdea 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStaticMethods.cs @@ -3,11 +3,10 @@ // See the LICENSE file in the project root for more information. using System; -using Microsoft.Data.Common; using System.Globalization; -using System.Runtime.Versioning; -using System.Security.Permissions; using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using Microsoft.Data.Common; namespace Microsoft.Data.SqlClient { @@ -168,7 +167,7 @@ static internal byte[] GetNetworkPhysicalAddressForTdsLoginOnly() byte[] nicAddress = null; - if (System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { // NIC address is stored in NetworkAddress key. However, if NetworkAddressLocal key // has a value that is not zero, then we cannot use the NetworkAddress key and must From 55aff924f4403009de70a5d6548448807ee9e88e Mon Sep 17 00:00:00 2001 From: David Engel Date: Thu, 19 May 2022 14:31:54 -0700 Subject: [PATCH 9/9] Deduplicate code --- .../SQL/InstanceNameTest/InstanceNameTest.cs | 36 +------------------ 1 file changed, 1 insertion(+), 35 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 ceba949d55..db20a1eb9b 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/InstanceNameTest/InstanceNameTest.cs @@ -48,7 +48,7 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove builder.IPAddressPreference = ipPreference; - Assert.True(ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); + Assert.True(DataTestUtility.ParseDataSource(builder.DataSource, out string hostname, out _, out string instanceName)); if (IsBrowserAlive(hostname) && IsValidInstance(hostname, instanceName)) { @@ -82,40 +82,6 @@ public static void ConnectManagedWithInstanceNameTest(bool useMultiSubnetFailove } } - private static bool ParseDataSource(string dataSource, out string hostname, out int port, out string instanceName) - { - hostname = string.Empty; - port = -1; - instanceName = string.Empty; - - if (dataSource.Contains(",") && dataSource.Contains("\\")) - return false; - - if (dataSource.Contains(":")) - { - dataSource = dataSource.Substring(dataSource.IndexOf(":") + 1); - } - - if (dataSource.Contains(",")) - { - if (!int.TryParse(dataSource.Substring(dataSource.LastIndexOf(",") + 1), out port)) - { - return false; - } - dataSource = dataSource.Substring(0, dataSource.IndexOf(",") - 1); - } - - if (dataSource.Contains("\\")) - { - instanceName = dataSource.Substring(dataSource.LastIndexOf("\\") + 1); - dataSource = dataSource.Substring(0, dataSource.LastIndexOf("\\")); - } - - hostname = dataSource; - - return hostname.Length > 0 && hostname.IndexOfAny(new char[] { '\\', ':', ',' }) == -1; - } - private static bool IsBrowserAlive(string browserHostname) { const byte ClntUcastEx = 0x03;