From 7ef0fc0518195d2591dc93840c88f3faf788a4ef Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 28 Oct 2021 13:53:30 -0700 Subject: [PATCH 1/5] diable sending NT Authority if specific trust was specified --- .../src/System/Net/Security/SecureChannel.cs | 14 +++++++++++--- .../System/Net/Security/SslSessionsCache.cs | 18 +++++++++++++----- .../Net/Security/SslStreamPal.Windows.cs | 18 ++++++++++++++---- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 10dd32c0316519..7184ce8f434b7a 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -557,7 +557,7 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint) // SECURITY: selectedCert ref if not null is a safe object that does not depend on possible **user** inherited X509Certificate type. // byte[]? guessedThumbPrint = selectedCert?.GetCertHash(); - SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy); + SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, false); // We can probably do some optimization here. If the selectedCert is returned by the delegate // we can always go ahead and use the certificate to create our credential @@ -695,11 +695,13 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint) _sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert); } + Debug.Assert(_sslAuthenticationOptions.CertificateContext != null); // // Note selectedCert is a safe ref possibly cloned from the user passed Cert object // byte[] guessedThumbPrint = selectedCert.GetCertHash(); - SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy); + bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust != null && _sslAuthenticationOptions.CertificateContext.Trust._sendTrustInHandshake; + SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustedList); if (cachedCredentialHandle != null) { @@ -763,6 +765,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte byte[]? result = Array.Empty(); SecurityStatusPal status = default; bool cachedCreds = false; + bool sendTrustList = false; byte[]? thumbPrint = null; // @@ -779,6 +782,11 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte cachedCreds = _sslAuthenticationOptions.IsServer ? AcquireServerCredentials(ref thumbPrint) : AcquireClientCredentials(ref thumbPrint); + + if (cachedCreds && _sslAuthenticationOptions.IsServer) + { + sendTrustList = _sslAuthenticationOptions.CertificateContext != null && _sslAuthenticationOptions.CertificateContext.Trust != null && _sslAuthenticationOptions.CertificateContext.Trust._sendTrustInHandshake; + } } if (_sslAuthenticationOptions.IsServer) @@ -820,7 +828,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte // if (!cachedCreds && _securityContext != null && !_securityContext.IsInvalid && _credentialsHandle != null && !_credentialsHandle.IsInvalid) { - SslSessionsCache.CacheCredential(_credentialsHandle, thumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy); + SslSessionsCache.CacheCredential(_credentialsHandle, thumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustList); } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs index 13e4f772f6b3b9..fd6ae46b23f56d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs @@ -25,18 +25,20 @@ internal static class SslSessionsCache private readonly int _allowedProtocols; private readonly EncryptionPolicy _encryptionPolicy; private readonly bool _isServerMode; + private readonly bool _sendTrustList; // // SECURITY: X509Certificate.GetCertHash() is virtual hence before going here, // the caller of this ctor has to ensure that a user cert object was inspected and // optionally cloned. // - internal SslCredKey(byte[]? thumbPrint, int allowedProtocols, bool isServerMode, EncryptionPolicy encryptionPolicy) + internal SslCredKey(byte[]? thumbPrint, int allowedProtocols, bool isServerMode, EncryptionPolicy encryptionPolicy, bool sendTrustList) { _thumbPrint = thumbPrint ?? Array.Empty(); _allowedProtocols = allowedProtocols; _encryptionPolicy = encryptionPolicy; _isServerMode = isServerMode; + _sendTrustList = sendTrustList; } public override int GetHashCode() @@ -65,6 +67,7 @@ public override int GetHashCode() hashCode ^= _allowedProtocols; hashCode ^= (int)_encryptionPolicy; hashCode ^= _isServerMode ? 0x10000 : 0x20000; + hashCode ^= _sendTrustList ? 0x40000 : 0x80000; return hashCode; } @@ -96,6 +99,11 @@ public bool Equals(SslCredKey other) return false; } + if (_sendTrustList != other._sendTrustList) + { + return false; + } + for (int i = 0; i < thumbPrint.Length; ++i) { if (thumbPrint[i] != otherThumbPrint[i]) @@ -114,7 +122,7 @@ public bool Equals(SslCredKey other) // ATTN: The returned handle can be invalid, the callers of InitializeSecurityContext and AcceptSecurityContext // must be prepared to execute a back-out code if the call fails. // - internal static SafeFreeCredentials? TryCachedCredential(byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy) + internal static SafeFreeCredentials? TryCachedCredential(byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList) { if (s_cachedCreds.IsEmpty) { @@ -122,7 +130,7 @@ public bool Equals(SslCredKey other) return null; } - var key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy); + var key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList); //SafeCredentialReference? cached; SafeFreeCredentials? credentials = GetCachedCredential(key); @@ -147,7 +155,7 @@ public bool Equals(SslCredKey other) // // ATTN: The thumbPrint must be from inspected and possibly cloned user Cert object or we get a security hole in SslCredKey ctor. // - internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy) + internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList) { Debug.Assert(creds != null, "creds == null"); @@ -157,7 +165,7 @@ internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPri return; } - SslCredKey key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy); + SslCredKey key = new SslCredKey(thumbPrint, (int)sslProtocols, isServer, encryptionPolicy, sendTrustList); SafeFreeCredentials? credentials = GetCachedCredential(key); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 29ab3bed0ee8c6..98752ff4448141 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -124,8 +124,8 @@ public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateC { // New crypto API supports TLS1.3 but it does not allow to force NULL encryption. SafeFreeCredentials cred = !UseNewCryptoApi || policy == EncryptionPolicy.NoEncryption ? - AcquireCredentialsHandleSchannelCred(certificateContext?.Certificate, protocols, policy, isServer) : - AcquireCredentialsHandleSchCredentials(certificateContext?.Certificate, protocols, policy, isServer); + AcquireCredentialsHandleSchannelCred(certificateContext, protocols, policy, isServer) : + AcquireCredentialsHandleSchCredentials(certificateContext, protocols, policy, isServer); if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake) { AttachCertificateStore(cred, certificateContext.Trust._store!); @@ -157,8 +157,9 @@ private static unsafe void AttachCertificateStore(SafeFreeCredentials cred, X509 // This is legacy crypto API used on .NET Framework and older Windows versions. // It only supports TLS up to 1.2 - public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(X509Certificate2? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { + X509Certificate2? certificate = certificateContext?.Certificate; int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); Interop.SspiCli.SCHANNEL_CRED.Flags flags; Interop.SspiCli.CredentialUse direction; @@ -183,6 +184,10 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(X5 { direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; flags = Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_SEND_AUX_RECORD; + if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake) + { + flags |= Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_NO_SYSTEM_MAPPER; + } } if (NetEventSource.Log.IsEnabled()) NetEventSource.Info($"flags=({flags}), ProtocolFlags=({protocolFlags}), EncryptionPolicy={policy}"); @@ -203,8 +208,9 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(X5 } // This function uses new crypto API to support TLS 1.3 and beyond. - public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(X509Certificate2? certificate, SslProtocols protocols, EncryptionPolicy policy, bool isServer) + public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { + X509Certificate2? certificate = certificateContext?.Certificate; int protocolFlags = GetProtocolFlagsFromSslProtocols(protocols, isServer); Interop.SspiCli.SCH_CREDENTIALS.Flags flags; Interop.SspiCli.CredentialUse direction; @@ -212,6 +218,10 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials( { direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; + if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake) + { + flags |= Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_SYSTEM_MAPPER; + } } else { From 7ba2af57ba2268af2d32e29229837fe0df65ae6f Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 28 Oct 2021 16:18:50 -0700 Subject: [PATCH 2/5] feedback from review --- .../src/System/Net/Security/SecureChannel.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 7184ce8f434b7a..9da063f082e7ad 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -557,7 +557,7 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint) // SECURITY: selectedCert ref if not null is a safe object that does not depend on possible **user** inherited X509Certificate type. // byte[]? guessedThumbPrint = selectedCert?.GetCertHash(); - SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, false); + SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy); // We can probably do some optimization here. If the selectedCert is returned by the delegate // we can always go ahead and use the certificate to create our credential From ef12cfeb165866fb2bdc657153659c2b02bd1f19 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 28 Oct 2021 17:48:17 -0700 Subject: [PATCH 3/5] add missing file --- .../src/System/Net/Security/SslSessionsCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs index fd6ae46b23f56d..35eb4ec534b83d 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslSessionsCache.cs @@ -122,7 +122,7 @@ public bool Equals(SslCredKey other) // ATTN: The returned handle can be invalid, the callers of InitializeSecurityContext and AcceptSecurityContext // must be prepared to execute a back-out code if the call fails. // - internal static SafeFreeCredentials? TryCachedCredential(byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList) + internal static SafeFreeCredentials? TryCachedCredential(byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList = false) { if (s_cachedCreds.IsEmpty) { @@ -155,7 +155,7 @@ public bool Equals(SslCredKey other) // // ATTN: The thumbPrint must be from inspected and possibly cloned user Cert object or we get a security hole in SslCredKey ctor. // - internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList) + internal static void CacheCredential(SafeFreeCredentials creds, byte[]? thumbPrint, SslProtocols sslProtocols, bool isServer, EncryptionPolicy encryptionPolicy, bool sendTrustList = false) { Debug.Assert(creds != null, "creds == null"); From 963532d6a51fa9e17740233ada5e279901f92322 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Fri, 29 Oct 2021 14:54:21 -0700 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: Cory Nelson --- .../src/System/Net/Security/SecureChannel.cs | 4 ++-- .../src/System/Net/Security/SslStreamPal.Windows.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 9da063f082e7ad..914c00dc4c3080 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -700,7 +700,7 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint) // Note selectedCert is a safe ref possibly cloned from the user passed Cert object // byte[] guessedThumbPrint = selectedCert.GetCertHash(); - bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust != null && _sslAuthenticationOptions.CertificateContext.Trust._sendTrustInHandshake; + bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust?._sendTrustInHandshake == true; SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustedList); if (cachedCredentialHandle != null) @@ -785,7 +785,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte if (cachedCreds && _sslAuthenticationOptions.IsServer) { - sendTrustList = _sslAuthenticationOptions.CertificateContext != null && _sslAuthenticationOptions.CertificateContext.Trust != null && _sslAuthenticationOptions.CertificateContext.Trust._sendTrustInHandshake; + sendTrustList = _sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true; } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index 98752ff4448141..1824e9b0233123 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -184,7 +184,7 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchannelCred(Ss { direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; flags = Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_SEND_AUX_RECORD; - if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake) + if (certificateContext?.Trust?._sendTrustInHandshake == true) { flags |= Interop.SspiCli.SCHANNEL_CRED.Flags.SCH_CRED_NO_SYSTEM_MAPPER; } @@ -218,7 +218,7 @@ public static unsafe SafeFreeCredentials AcquireCredentialsHandleSchCredentials( { direction = Interop.SspiCli.CredentialUse.SECPKG_CRED_INBOUND; flags = Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_SEND_AUX_RECORD; - if (certificateContext != null && certificateContext.Trust != null && certificateContext.Trust._sendTrustInHandshake) + if (certificateContext?.Trust?._sendTrustInHandshake == true) { flags |= Interop.SspiCli.SCH_CREDENTIALS.Flags.SCH_CRED_NO_SYSTEM_MAPPER; } From bd266ff9bacdff152f6e4ff7155aaf1e966b5915 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 1 Nov 2021 14:45:36 -0700 Subject: [PATCH 5/5] one more style update --- .../src/System/Net/Security/SecureChannel.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs index 914c00dc4c3080..adb6680fb999bf 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SecureChannel.cs @@ -700,7 +700,7 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint) // Note selectedCert is a safe ref possibly cloned from the user passed Cert object // byte[] guessedThumbPrint = selectedCert.GetCertHash(); - bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust?._sendTrustInHandshake == true; + bool sendTrustedList = _sslAuthenticationOptions.CertificateContext!.Trust?._sendTrustInHandshake ?? false; SafeFreeCredentials? cachedCredentialHandle = SslSessionsCache.TryCachedCredential(guessedThumbPrint, _sslAuthenticationOptions.EnabledSslProtocols, _sslAuthenticationOptions.IsServer, _sslAuthenticationOptions.EncryptionPolicy, sendTrustedList); if (cachedCredentialHandle != null) @@ -785,7 +785,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte if (cachedCreds && _sslAuthenticationOptions.IsServer) { - sendTrustList = _sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake == true; + sendTrustList = _sslAuthenticationOptions.CertificateContext?.Trust?._sendTrustInHandshake ?? false; } }