From 8db62eff0f299bb0481d5683390d5efb5e582b5e Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Thu, 7 Dec 2023 13:42:05 +0100 Subject: [PATCH] Port rest of the changes from #79898 --- .../Interop/Windows/SspiCli/Interop.SSPI.cs | 18 +++++++- .../Windows/SspiCli/SecuritySafeHandles.cs | 6 +++ .../Net/CertificateValidationPal.Android.cs | 4 +- .../Net/CertificateValidationPal.OSX.cs | 2 +- .../Net/CertificateValidationPal.Unix.cs | 2 +- .../Net/CertificateValidationPal.Windows.cs | 18 +++++++- .../System/Net/Security/SslStream.Protocol.cs | 45 ++++++++++++------- .../Net/Security/SslStreamPal.Android.cs | 2 +- .../System/Net/Security/SslStreamPal.OSX.cs | 2 +- .../System/Net/Security/SslStreamPal.Unix.cs | 2 +- .../Net/Security/SslStreamPal.Windows.cs | 12 ++++- .../SslStreamMutualAuthenticationTest.cs | 4 -- 12 files changed, 87 insertions(+), 30 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs index c8e7dd3649024..2e668f6c84638 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/Interop.SSPI.cs @@ -67,6 +67,7 @@ internal enum ContextAttribute SECPKG_ATTR_ISSUER_LIST_EX = 0x59, // returns SecPkgContext_IssuerListInfoEx SECPKG_ATTR_CLIENT_CERT_POLICY = 0x60, // sets SecPkgCred_ClientCertCtlPolicy SECPKG_ATTR_CONNECTION_INFO = 0x5A, // returns SecPkgContext_ConnectionInfo + SECPKG_ATTR_SESSION_INFO = 0x5D, // sets SecPkgContext_SessionInfo SECPKG_ATTR_CIPHER_INFO = 0x64, // returns SecPkgContext_CipherInfo SECPKG_ATTR_REMOTE_CERT_CHAIN = 0x67, // returns PCCERT_CONTEXT SECPKG_ATTR_UI_INFO = 0x68, // sets SEcPkgContext_UiInfo @@ -249,7 +250,7 @@ public enum Flags SCH_CRED_IGNORE_REVOCATION_OFFLINE = 0x1000, SCH_CRED_CACHE_ONLY_URL_RETRIEVAL_ON_CREATE = 0x2000, SCH_SEND_ROOT_CERT = 0x40000, - SCH_SEND_AUX_RECORD = 0x00200000, + SCH_SEND_AUX_RECORD = 0x00200000, SCH_USE_STRONG_CRYPTO = 0x00400000, SCH_USE_PRESHAREDKEY_ONLY = 0x800000, SCH_ALLOW_NULL_ENCRYPTION = 0x02000000, @@ -334,6 +335,21 @@ internal unsafe struct SecPkgCred_ClientCertPolicy public char* pwszSslCtlIdentifier; } + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct SecPkgContext_SessionInfo + { + public uint dwFlags; + public uint cbSessionId; + public fixed byte rgbSessionId[32]; + + [Flags] + public enum Flags + { + Zero = 0, + SSL_SESSION_RECONNECT = 0x01, + }; + } + [LibraryImport(Interop.Libraries.SspiCli, SetLastError = true)] internal static partial int EncryptMessage( ref CredHandle contextHandle, diff --git a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs index 12603f7df6ae9..880189a741625 100644 --- a/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs +++ b/src/libraries/Common/src/Interop/Windows/SspiCli/SecuritySafeHandles.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Globalization; using System.Runtime.InteropServices; +using System.Security.Cryptography.X509Certificates; using System.Security.Authentication.ExtendedProtection; using Microsoft.Win32.SafeHandles; @@ -310,10 +311,15 @@ public static unsafe int AcquireCredentialsHandle( internal sealed class SafeFreeCredential_SECURITY : SafeFreeCredentials { +#pragma warning disable 0649 + // This is used only by SslStream but it is included elsewhere + public X509Certificate? LocalCertificate; +#pragma warning restore 0649 public SafeFreeCredential_SECURITY() : base() { } protected override bool ReleaseHandle() { + LocalCertificate?.Dispose(); return Interop.SspiCli.FreeCredentialsHandle(ref _handle) == 0; } } diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs index dc0865382cdf2..96962f9240be9 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Android.cs @@ -18,7 +18,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( string? hostName) { if (remoteCertificate == null) - return SslPolicyErrors.RemoteCertificateNotAvailable; + return SslPolicyErrors.RemoteCertificateNotAvailable; SslPolicyErrors errors = chain.Build(remoteCertificate) ? SslPolicyErrors.None @@ -93,7 +93,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( // This is only called when we selected local client certificate. // Currently this is only when Java crypto asked for it. - internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true; + internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true; // // Used only by client SSL code, never returns null. diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs index 1f3fce1d7d863..3bd0c7142c3fc 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.OSX.cs @@ -104,7 +104,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( // This is only called when we selected local client certificate. // Currently this is only when Apple crypto asked for it. - internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true; + internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true; // // Used only by client SSL code, never returns null. diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs index 7074726a9177d..90b9275e9af36 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Unix.cs @@ -103,7 +103,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( // This is only called when we selected local client certificate. // Currently this is only when OpenSSL needs it because peer asked. - internal static bool IsLocalCertificateUsed(SafeDeleteContext? _) => true; + internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _1, SafeDeleteContext? _2) => true; // // Used only by client SSL code, never returns null. diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs index 926cb646e7916..3d622f6c2dfc7 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs @@ -8,6 +8,7 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Security.Principal; +using static Interop.SspiCli; namespace System.Net { @@ -90,8 +91,23 @@ internal static SslPolicyErrors VerifyCertificateProperties( } // Check that local certificate was used by schannel. - internal static bool IsLocalCertificateUsed(SafeDeleteContext securityContext) + internal static bool IsLocalCertificateUsed(SafeFreeCredentials? _credentialsHandle, SafeDeleteContext securityContext) { + SecPkgContext_SessionInfo info = default; + // fails on Server 2008 and older. We will fall-back to probing LOCAL_CERT_CONTEXT in that case. + if (SSPIWrapper.QueryBlittableContextAttributes( + GlobalSSPI.SSPISecureChannel, + securityContext, + Interop.SspiCli.ContextAttribute.SECPKG_ATTR_SESSION_INFO, + ref info) && + ((SecPkgContext_SessionInfo.Flags)info.dwFlags).HasFlag(SecPkgContext_SessionInfo.Flags.SSL_SESSION_RECONNECT)) + { + // This is TLS Resumed session. Windows can fail to query the local cert bellow. + // Instead, we will determine the usage form used credentials. + SafeFreeCredential_SECURITY creds = (SafeFreeCredential_SECURITY)_credentialsHandle!; + return creds.LocalCertificate != null; + } + SafeFreeCertContext? localContext = null; try { diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs index 653a8092474c1..45f0bbe408f24 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.Protocol.cs @@ -30,8 +30,6 @@ public partial class SslStream private int _trailerSize = 16; private int _maxDataSize = 16354; - private bool _refreshCredentialNeeded = true; - private static readonly Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1"); private static readonly Oid s_clientAuthOid = new Oid("1.3.6.1.5.5.7.3.2", "1.3.6.1.5.5.7.3.2"); @@ -109,11 +107,6 @@ internal bool RemoteCertRequired } } - internal void SetRefreshCredentialNeeded() - { - _refreshCredentialNeeded = true; - } - internal void CloseContext() { if (!_remoteCertificateExposed) @@ -515,7 +508,7 @@ This will not restart a session but helps minimizing the number of handles we cr --*/ - private bool AcquireClientCredentials(ref byte[]? thumbPrint) + private bool AcquireClientCredentials(ref byte[]? thumbPrint, bool newCredentialsRequested = false) { // Acquire possible Client Certificate information and set it on the handle. @@ -580,7 +573,7 @@ private bool AcquireClientCredentials(ref byte[]? thumbPrint) _sslAuthenticationOptions.CertificateContext = SslStreamCertificateContext.Create(selectedCert!); } - _credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions); + _credentialsHandle = AcquireCredentialsHandle(_sslAuthenticationOptions, newCredentialsRequested); thumbPrint = guessedThumbPrint; // Delay until here in case something above threw. } } @@ -691,9 +684,9 @@ private bool AcquireServerCredentials(ref byte[]? thumbPrint) return cachedCred; } - private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + private static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions, bool newCredentialsRequested = false) { - SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions); + SafeFreeCredentials? cred = SslStreamPal.AcquireCredentialsHandle(sslAuthenticationOptions, newCredentialsRequested); if (sslAuthenticationOptions.CertificateContext != null && cred != null) { @@ -753,7 +746,6 @@ internal ProtocolToken NextMessage(ReadOnlySpan incomingBuffer) if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, "NextMessage() returned SecurityStatusPal.CredentialsNeeded"); - SetRefreshCredentialNeeded(); status = GenerateToken(incomingBuffer, ref nextmsg); } @@ -792,6 +784,11 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte bool sendTrustList = false; byte[]? thumbPrint = null; + // We need to try get credentials at the beginning. + // _credentialsHandle may be always null on some platforms but + // _securityContext will be allocated on first call. + bool refreshCredentialNeeded = _securityContext == null; + // // Looping through ASC or ISC with potentially cached credential that could have been // already disposed from a different thread before ISC or ASC dir increment a cred ref count. @@ -801,7 +798,7 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte do { thumbPrint = null; - if (_refreshCredentialNeeded) + if (refreshCredentialNeeded) { cachedCreds = _sslAuthenticationOptions.IsServer ? AcquireServerCredentials(ref thumbPrint) @@ -830,15 +827,31 @@ private SecurityStatusPal GenerateToken(ReadOnlySpan inputBuffer, ref byte _sslAuthenticationOptions, SelectClientCertificate ); + + if (status.ErrorCode == SecurityStatusPalErrorCode.CredentialsNeeded) + { + refreshCredentialNeeded = true; + cachedCreds = AcquireClientCredentials(ref thumbPrint, newCredentialsRequested: true); + + if (NetEventSource.Log.IsEnabled()) + NetEventSource.Info(this, "InitializeSecurityContext() returned 'CredentialsNeeded'."); + + status = SslStreamPal.InitializeSecurityContext( + ref _credentialsHandle!, + ref _securityContext, + _sslAuthenticationOptions.TargetHost, + inputBuffer, + ref result, + _sslAuthenticationOptions, + SelectClientCertificate); + } } } while (cachedCreds && _credentialsHandle == null); } finally { - if (_refreshCredentialNeeded) + if (refreshCredentialNeeded) { - _refreshCredentialNeeded = false; - // // Assuming the ISC or ASC has referenced the credential, // we want to call dispose so to decrement the effective ref count. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs index f01dd68e294b2..184cbd2a81773 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs @@ -55,7 +55,7 @@ public static SecurityStatusPal Renegotiate( throw new PlatformNotSupportedException(); } - public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2) { return null; } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs index 255b30d7f2c2f..d8fc15ca7a546 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.OSX.cs @@ -62,7 +62,7 @@ public static SecurityStatusPal Renegotiate( throw new PlatformNotSupportedException(); } - public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2) { return null; } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs index 1e1a0df55889e..e4188015d4167 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Unix.cs @@ -46,7 +46,7 @@ public static SecurityStatusPal InitializeSecurityContext( return HandshakeInternal(ref context, inputBuffer, ref outputBuffer, sslAuthenticationOptions, clientCertificateSelectionCallback); } - public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + public static SafeFreeCredentials? AcquireCredentialsHandle(SslAuthenticationOptions _1, bool _2) { return null; } 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 1321cc0754ed0..2c69293145800 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 @@ -136,7 +136,7 @@ public static SecurityStatusPal Renegotiate( return status; } - public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions) + public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOptions sslAuthenticationOptions, bool newCredentialsRequested) { try { @@ -156,6 +156,16 @@ public static SafeFreeCredentials AcquireCredentialsHandle(SslAuthenticationOpti AttachCertificateStore(cred, certificateContext.Trust._store!); } + // Windows can fail to get local credentials in case of TLS Resume. + // We will store associated certificate in credentials and use it in case + // of TLS resume. It will be disposed when the credentials are. + if (newCredentialsRequested && sslAuthenticationOptions.CertificateContext != null) + { + SafeFreeCredential_SECURITY handle = (SafeFreeCredential_SECURITY)cred; + // We need to create copy to avoid Disposal issue. + handle.LocalCertificate = new X509Certificate2(sslAuthenticationOptions.CertificateContext.Certificate); + } + return cred; } catch (Win32Exception e) diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs index 4aca0662bc29e..90c0a6cb04901 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamMutualAuthenticationTest.cs @@ -37,7 +37,6 @@ public enum ClientCertSource { ClientCertificate, SelectionCallback, - CertificateContext } public static TheoryData CertSourceData() @@ -184,9 +183,6 @@ public async Task SslStream_NegotiateClientCertificate_IsMutuallyAuthenticatedCo case ClientCertSource.SelectionCallback: clientOptions.LocalCertificateSelectionCallback = ClientCertSelectionCallback; break; - case ClientCertSource.CertificateContext: - clientOptions.ClientCertificateContext = SslStreamCertificateContext.Create(_clientCertificate, new()); - break; } Task t2 = client.AuthenticateAsClientAsync(clientOptions);