From 5b5dc357b1820c0885c36eb3a77acfdbb93c2b8a Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 12:04:12 -0700 Subject: [PATCH 01/13] tls resume --- .../Interop.OpenSsl.cs | 195 +++++++++++++----- .../Interop.SetProtocolOptions.cs | 4 + .../Interop.Ssl.cs | 29 +++ .../Interop.SslCtxOptions.cs | 6 +- .../Net/Security/Unix/SafeDeleteSslContext.cs | 7 +- .../Security/Unix/SafeFreeSslCredentials.cs | 9 +- .../apibridge.c | 6 + .../apibridge.h | 1 + .../entrypoints.c | 6 + .../opensslshim.h | 13 +- .../pal_ssl.c | 115 ++++++++++- .../pal_ssl.h | 24 ++- .../SslStreamCertificateContext.Linux.cs | 4 + .../System/Net/Security/SslStreamPal.Unix.cs | 2 +- 14 files changed, 344 insertions(+), 77 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 73a9d32bb45c84..8e20a4a5676183 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -8,7 +8,6 @@ using System.IO; using System.Net; using System.Net.Security; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Security.Authentication; using System.Security.Authentication.ExtendedProtection; @@ -45,14 +44,19 @@ internal static partial class OpenSsl return bindingHandle; } - internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX509Handle? certHandle, SafeEvpPKeyHandle? certKeyHandle, EncryptionPolicy policy, SslAuthenticationOptions sslAuthenticationOptions) + // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting + internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) { - SafeSslHandle? context = null; + SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; + SafeX509Handle? certHandle = credential.CertHandle; + SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; + // Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest // mutually supported version and can thus handle any of the set protocols, and we then use // SetProtocolOptions to ensure we only allow the ones requested. - using (SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method)) + SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method); + try { if (innerContext.IsInvalid) { @@ -61,14 +65,14 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 if (!Interop.Ssl.Tls13Supported) { - if (protocols != SslProtocols.None && + if (sslAuthenticationOptions.EnabledSslProtocols != SslProtocols.None && CipherSuitesPolicyPal.WantsTls13(protocols)) { protocols = protocols & (~SslProtocols.Tls13); } } else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, policy)) + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) { if (protocols == SslProtocols.None) { @@ -81,17 +85,17 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 { // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, policy)); + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } } - if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, policy)) + if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) { if (!CipherSuitesPolicyPal.WantsTls13(protocols)) { // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, policy)); + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } protocols = SslProtocols.Tls13; @@ -99,13 +103,15 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 // Configure allowed protocols. It's ok to use DangerousGetHandle here without AddRef/Release as we just // create the handle, it's rooted by the using, no one else has a reference to it, etc. - Ssl.SetProtocolOptions(innerContext.DangerousGetHandle(), protocols); + Ssl.SetProtocolOptions(innerContext, protocols); - // Sets policy and security level - if (!Ssl.SetEncryptionPolicy(innerContext, policy)) + if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) { - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, policy)); + // Sets policy and security level + if (!Ssl.SetEncryptionPolicy(innerContext, sslAuthenticationOptions.EncryptionPolicy)) + { + throw new SslException( SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } } // The logic in SafeSslHandle.Disconnect is simple because we are doing a quiet @@ -117,26 +123,11 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html Ssl.SslCtxSetQuietShutdown(innerContext); - byte[]? cipherList = - CipherSuitesPolicyPal.GetOpenSslCipherList(sslAuthenticationOptions.CipherSuitesPolicy, protocols, policy); - - Debug.Assert(cipherList == null || (cipherList.Length >= 1 && cipherList[cipherList.Length - 1] == 0)); - - byte[]? cipherSuites = - CipherSuitesPolicyPal.GetOpenSslCipherSuites(sslAuthenticationOptions.CipherSuitesPolicy, protocols, policy); - - Debug.Assert(cipherSuites == null || (cipherSuites.Length >= 1 && cipherSuites[cipherSuites.Length - 1] == 0)); - - unsafe + if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { - fixed (byte* cipherListStr = cipherList) - fixed (byte* cipherSuitesStr = cipherSuites) + unsafe { - if (!Ssl.SetCiphers(innerContext, cipherListStr, cipherSuitesStr)) - { - Crypto.ErrClearError(); - throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, policy)); - } + Interop.Ssl.SslCtxSetAlpnSelectCb(innerContext, &AlpnServerSelectCallback, IntPtr.Zero); } } @@ -149,45 +140,74 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 SetSslCertificate(innerContext, certHandle!, certKeyHandle!); } - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) - { - unsafe - { - Ssl.SslCtxSetVerify(innerContext, &VerifyClientCertificate); - } - } + } + catch + { + innerContext.Dispose(); + throw; + } + return innerContext; + } + + // This essentially wraps SSL* SSL_new() + internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) + { + SafeSslHandle? context = null; + SafeSslContextHandle? sslCtx = null; + SafeSslContextHandle? innerContext = null; + SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; + bool cacheSslContext = sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption; + + if (sslAuthenticationOptions.CertificateContext != null && cacheSslContext) + { + sslAuthenticationOptions.CertificateContext.contexts.TryGetValue((int)sslAuthenticationOptions.EnabledSslProtocols, out sslCtx); + } + + if (sslCtx == null) + { + // We did not get SslContext from cache + sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions); + } + + try + { GCHandle alpnHandle = default; try { + context = SafeSslHandle.Create(sslCtx, sslAuthenticationOptions.IsServer); + Debug.Assert(context != null, "Expected non-null return value from SafeSslHandle.Create"); + if (context.IsInvalid) + { + context.Dispose(); + throw CreateSslException(SR.net_allocate_ssl_context_failed); + } + + if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) + { + // Sets policy and security level + if (!Ssl.SetEncryptionPolicy(sslCtx, sslAuthenticationOptions.EncryptionPolicy)) + { + throw new SslException( SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { if (sslAuthenticationOptions.IsServer) { alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); - - unsafe - { - Interop.Ssl.SslCtxSetAlpnSelectCb(innerContext, &AlpnServerSelectCallback, GCHandle.ToIntPtr(alpnHandle)); - } + Interop.Ssl.SslSetData(context, GCHandle.ToIntPtr(alpnHandle)); } else { - if (Interop.Ssl.SslCtxSetAlpnProtos(innerContext, sslAuthenticationOptions.ApplicationProtocols) != 0) + if (Interop.Ssl.SslSetAlpnProtos(context, sslAuthenticationOptions.ApplicationProtocols) != 0) { throw CreateSslException(SR.net_alpn_config_failed); } } } - context = SafeSslHandle.Create(innerContext, sslAuthenticationOptions.IsServer); - Debug.Assert(context != null, "Expected non-null return value from SafeSslHandle.Create"); - if (context.IsInvalid) - { - context.Dispose(); - throw CreateSslException(SR.net_allocate_ssl_context_failed); - } - if (!sslAuthenticationOptions.IsServer) { // The IdnMapping converts unicode input into the IDNA punycode sequence. @@ -200,6 +220,29 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 } } + byte[]? cipherList = + CipherSuitesPolicyPal.GetOpenSslCipherList(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); + + Debug.Assert(cipherList == null || (cipherList.Length >= 1 && cipherList[cipherList.Length - 1] == 0)); + + byte[]? cipherSuites = + CipherSuitesPolicyPal.GetOpenSslCipherSuites(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); + + Debug.Assert(cipherSuites == null || (cipherSuites.Length >= 1 && cipherSuites[cipherSuites.Length - 1] == 0)); + + unsafe + { + fixed (byte* cipherListStr = cipherList) + fixed (byte* cipherSuitesStr = cipherSuites) + { + if (!Ssl.SslSetCiphers(context, cipherListStr, cipherSuitesStr)) + { + Crypto.ErrClearError(); + throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + } + if (sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) { if (!Ssl.AddExtraChainCertificates(context, sslAuthenticationOptions.CertificateContext!.IntermediateCertificates)) @@ -209,6 +252,22 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 } context.AlpnHandle = alpnHandle; + + + if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) + { + unsafe + { + Ssl.SslSetVerifyPeer(context); + } + } + + // Sets policy and security level +// if (!Ssl.SetEncryptionPolicy(context, sslAuthenticationOptions.EncryptionPolicy)) +// { +// throw new SslException( +// SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); +// } } catch { @@ -220,6 +279,22 @@ internal static SafeSslHandle AllocateSslContext(SslProtocols protocols, SafeX50 throw; } } + finally + { + if (innerContext != null && cacheSslContext) + { + // We allocated new context + if (sslAuthenticationOptions.CertificateContext?.contexts != null && + sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext)) + { + //Console.WriteLine("Added {0} to CTX cache", sslCtx.DangerousGetHandle()); + } + else + { + innerContext.Dispose(); + } + } + } return context; } @@ -441,8 +516,20 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte { *outp = null; *outlen = 0; + IntPtr sslData = Ssl.SslGetData(ssl); + + Console.WriteLine("AlpnServerSelectCallback called for {0} and {1}", ssl, sslData); + if (sslData == IntPtr.Zero) + { + // We did not set ALPN list. + *outlen = 0; + *outp = (byte*)arg; + return Ssl.SSL_TLSEXT_ERR_OK; + } + + //return Ssl.SSL_TLSEXT_ERR_OK; - GCHandle protocolHandle = GCHandle.FromIntPtr(arg); + GCHandle protocolHandle = GCHandle.FromIntPtr(sslData); if (!(protocolHandle.Target is List protocolList)) { return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs index d75ab5c5cc55c5..01c7ab1985e7b0 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs @@ -4,6 +4,7 @@ using System; using System.Runtime.InteropServices; using System.Security.Authentication; +using Microsoft.Win32.SafeHandles; internal static partial class Interop { @@ -11,5 +12,8 @@ internal static partial class Ssl { [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetProtocolOptions")] internal static extern void SetProtocolOptions(IntPtr ctx, SslProtocols protocols); + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetProtocolOptions")] + internal static extern void SetProtocolOptions(SafeSslContextHandle ctx, SslProtocols protocols); } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 29154b77da6325..5b63b073803ddf 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Net.Security; using System.Runtime.InteropServices; @@ -44,6 +45,9 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetAcceptState")] internal static extern void SslSetAcceptState(SafeSslHandle ssl); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetAlpnProtos")] + internal static extern int SslSetAlpnProtos(SafeSslHandle ssl, IntPtr protos, int len); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetVersion")] internal static extern IntPtr SslGetVersion(SafeSslHandle ssl); @@ -133,6 +137,31 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetOpenSslCipherSuiteName")] private static extern IntPtr GetOpenSslCipherSuiteName(SafeSslHandle ssl, int cipherSuite, out int isTls12OrLower); + //[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetEncryptionPolicy")] + //internal static extern bool SetEncryptionPolicy(SafeSslHandle ssl, EncryptionPolicy policy); + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetCiphers")] + internal static extern unsafe bool SslSetCiphers(SafeSslHandle ssl, byte* cipherList, byte* cipherSuites); + + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyPeer")] + internal static extern void SslSetVerifyPeer(SafeSslHandle ssl); + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetData")] + internal static extern IntPtr SslGetData(IntPtr ssl); + + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetData")] + internal static extern int SslSetData(SafeSslHandle ssl, IntPtr data); + + internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, List protocols) + { + byte[] buffer = ConvertAlpnProtocolListToByteArray(protocols); + fixed (byte* b = buffer) + { + return SslSetAlpnProtos(ssl, (IntPtr)b, buffer.Length); + } + } + internal static string? GetOpenSslCipherSuiteName(SafeSslHandle ssl, TlsCipherSuite cipherSuite, out bool isTls12OrLower) { string? ret = Marshal.PtrToStringAnsi(GetOpenSslCipherSuiteName(ssl, (int)cipherSuite, out int isTls12OrLowerInt)); diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs index 978f846939c2f9..0e74b39838e37e 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs @@ -26,10 +26,10 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetVerify")] internal static extern unsafe void SslCtxSetVerify(SafeSslContextHandle ctx, delegate* unmanaged callback); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetCiphers")] - internal static extern unsafe bool SetCiphers(SafeSslContextHandle ctx, byte* cipherList, byte* cipherSuites); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCiphers")] + internal static extern unsafe bool SslCtxSetCiphers(SafeSslContextHandle ctx, byte* cipherList, byte* cipherSuites); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetEncryptionPolicy")] + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetEncryptionPolicy")] internal static extern bool SetEncryptionPolicy(SafeSslContextHandle ctx, EncryptionPolicy policy); } } diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs index 3f550d36d7f439..89ac9b6d8dc11c 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeDeleteSslContext.cs @@ -34,12 +34,7 @@ public SafeDeleteSslContext(SafeFreeSslCredentials credential, SslAuthentication try { - _sslContext = Interop.OpenSsl.AllocateSslContext( - credential.Protocols, - credential.CertHandle, - credential.CertKeyHandle, - credential.Policy, - sslAuthenticationOptions); + _sslContext = Interop.OpenSsl.AllocateSslHandle(credential, sslAuthenticationOptions); } catch (Exception ex) { diff --git a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs index ff378ab425139b..ae6bab3968d49e 100644 --- a/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs +++ b/src/libraries/Common/src/System/Net/Security/Unix/SafeFreeSslCredentials.cs @@ -21,6 +21,7 @@ internal sealed class SafeFreeSslCredentials : SafeFreeCredentials private SslProtocols _protocols = SslProtocols.None; private EncryptionPolicy _policy; private bool _isInvalid; + private SslStreamCertificateContext? _context; internal SafeX509Handle? CertHandle { @@ -42,14 +43,15 @@ internal EncryptionPolicy Policy get { return _policy; } } - public SafeFreeSslCredentials(X509Certificate? certificate, SslProtocols protocols, EncryptionPolicy policy) + public SafeFreeSslCredentials(SslStreamCertificateContext? context, SslProtocols protocols, EncryptionPolicy policy, bool isServer) : base(IntPtr.Zero, true) { + Debug.Assert( - certificate == null || certificate is X509Certificate2, + context == null || context.Certificate is X509Certificate2, "Only X509Certificate2 certificates are supported at this time"); - X509Certificate2? cert = (X509Certificate2?)certificate; + X509Certificate2? cert = context?.Certificate; if (cert != null) { @@ -87,6 +89,7 @@ public SafeFreeSslCredentials(X509Certificate? certificate, SslProtocols protoco _protocols = protocols; _policy = policy; + _context = context; } public override bool IsInvalid diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c index 15df621f7a7dfa..0a3705321eec4c 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c @@ -803,6 +803,12 @@ void local_SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level) (void)level; } +void local_SSL_set_security_level(SSL* ssl, int32_t level) +{ + (void)ssl; + (void)level; +} + int local_BIO_up_ref(BIO *bio) { if (!bio) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h index 3998f1abbf3a4a..2079d81fd96816 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.h @@ -36,6 +36,7 @@ int32_t local_SSL_CTX_config(SSL_CTX* ctx, const char* name); unsigned long local_SSL_CTX_set_options(SSL_CTX* ctx, unsigned long options); unsigned long local_SSL_set_options(SSL* ssl, unsigned long options); void local_SSL_CTX_set_security_level(SSL_CTX* ctx, int32_t level); +void local_SSL_set_security_level(SSL* ssl, int32_t level); int local_SSL_session_reused(SSL* ssl); int32_t local_X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername); const ASN1_TIME* local_X509_CRL_get0_nextUpdate(const X509_CRL* crl); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index bf68f23be3633a..13946dbb207165 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -287,6 +287,8 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslRenegotiate) DllImportEntry(CryptoNative_IsSslRenegotiatePending) DllImportEntry(CryptoNative_IsSslStateOK) + DllImportEntry(CryptoNative_SslCtxSetCiphers) + DllImportEntry(CryptoNative_SslCtxSetEncryptionPolicy) DllImportEntry(CryptoNative_SetCiphers) DllImportEntry(CryptoNative_SetEncryptionPolicy) DllImportEntry(CryptoNative_SetProtocolOptions) @@ -305,6 +307,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslDoHandshake) DllImportEntry(CryptoNative_SslGetClientCAList) DllImportEntry(CryptoNative_SslGetCurrentCipherId) + DllImportEntry(CryptoNative_SslGetData) DllImportEntry(CryptoNative_SslGetError) DllImportEntry(CryptoNative_SslGetFinished) DllImportEntry(CryptoNative_SslGetPeerCertChain) @@ -314,10 +317,13 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslRead) DllImportEntry(CryptoNative_SslSessionReused) DllImportEntry(CryptoNative_SslSetAcceptState) + DllImportEntry(CryptoNative_SslSetAlpnProtos) DllImportEntry(CryptoNative_SslSetBio) DllImportEntry(CryptoNative_SslSetConnectState) + DllImportEntry(CryptoNative_SslSetData) DllImportEntry(CryptoNative_SslSetQuietShutdown) DllImportEntry(CryptoNative_SslSetTlsExtHostName) + DllImportEntry(CryptoNative_SslSetVerifyPeer) DllImportEntry(CryptoNative_SslShutdown) DllImportEntry(CryptoNative_SslV2_3Method) DllImportEntry(CryptoNative_SslWrite) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 69c95b532ade08..599f03294e4d7c 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -128,7 +128,6 @@ const SSL_CIPHER* SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr); #include "osslcompat_111.h" #endif - #if !HAVE_OPENSSL_ALPN #undef HAVE_OPENSSL_ALPN #define HAVE_OPENSSL_ALPN 1 @@ -458,6 +457,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); LIGHTUP_FUNCTION(SSL_CIPHER_get_name) \ LIGHTUP_FUNCTION(SSL_CIPHER_get_version) \ REQUIRED_FUNCTION(SSL_ctrl) \ + REQUIRED_FUNCTION(SSL_set_alpn_protos) \ REQUIRED_FUNCTION(SSL_set_quiet_shutdown) \ REQUIRED_FUNCTION(SSL_CTX_check_private_key) \ FALLBACK_FUNCTION(SSL_CTX_config) \ @@ -473,6 +473,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ + FALLBACK_FUNCTION(SSL_set_security_level) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ @@ -482,6 +483,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_get_client_CA_list) \ REQUIRED_FUNCTION(SSL_get_current_cipher) \ REQUIRED_FUNCTION(SSL_get_error) \ + REQUIRED_FUNCTION(SSL_get_ex_data) \ REQUIRED_FUNCTION(SSL_get_finished) \ REQUIRED_FUNCTION(SSL_get_peer_cert_chain) \ REQUIRED_FUNCTION(SSL_get_peer_finished) \ @@ -499,7 +501,10 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ + REQUIRED_FUNCTION(SSL_set_cipher_list) \ + LIGHTUP_FUNCTION(SSL_set_ciphersuites) \ REQUIRED_FUNCTION(SSL_set_connect_state) \ + REQUIRED_FUNCTION(SSL_set_ex_data) \ FALLBACK_FUNCTION(SSL_set_options) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ @@ -908,6 +913,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CIPHER_get_name SSL_CIPHER_get_name_ptr #define SSL_CIPHER_get_version SSL_CIPHER_get_version_ptr #define SSL_ctrl SSL_ctrl_ptr +#define SSL_set_alpn_protos SSL_set_alpn_protos_ptr #define SSL_set_quiet_shutdown SSL_set_quiet_shutdown_ptr #define SSL_CTX_check_private_key SSL_CTX_check_private_key_ptr #define SSL_CTX_config SSL_CTX_config_ptr @@ -922,6 +928,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CTX_set_options SSL_CTX_set_options_ptr #define SSL_CTX_set_quiet_shutdown SSL_CTX_set_quiet_shutdown_ptr #define SSL_CTX_set_security_level SSL_CTX_set_security_level_ptr +#define SSL_set_security_level SSL_set_security_level_ptr #define SSL_CTX_set_verify SSL_CTX_set_verify_ptr #define SSL_CTX_use_certificate SSL_CTX_use_certificate_ptr #define SSL_CTX_use_PrivateKey SSL_CTX_use_PrivateKey_ptr @@ -931,6 +938,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_get_client_CA_list SSL_get_client_CA_list_ptr #define SSL_get_current_cipher SSL_get_current_cipher_ptr #define SSL_get_error SSL_get_error_ptr +#define SSL_get_ex_data SSL_get_ex_data_ptr #define SSL_get_finished SSL_get_finished_ptr #define SSL_get_peer_cert_chain SSL_get_peer_cert_chain_ptr #define SSL_get_peer_finished SSL_get_peer_finished_ptr @@ -951,7 +959,10 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_session_reused SSL_session_reused_ptr #define SSL_set_accept_state SSL_set_accept_state_ptr #define SSL_set_bio SSL_set_bio_ptr +#define SSL_set_cipher_list SSL_set_cipher_list_ptr +#define SSL_set_ciphersuites SSL_set_ciphersuites_ptr #define SSL_set_connect_state SSL_set_connect_state_ptr +#define SSL_set_ex_data SSL_set_ex_data_ptr #define SSL_set_options SSL_set_options_ptr #define SSL_set_verify SSL_set_verify_ptr #define SSL_shutdown SSL_shutdown_ptr diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index 77ac385a799059..9e29e46a32b053 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -39,6 +39,7 @@ static void EnsureLibSsl10Initialized() #endif static int32_t g_config_specified_ciphersuites = 0; +static char* emptyAlpn = ""; static void DetectCiphersuiteConfiguration() { @@ -210,14 +211,14 @@ static long TrySetECDHNamedCurve(SSL_CTX* ctx) } } - return result; + return result; #else (void)ctx; return 1; #endif } -static void ResetProtocolRestrictions(SSL_CTX* ctx) +static void ResetCtxProtocolRestrictions(SSL_CTX* ctx) { #ifndef SSL_CTRL_SET_MIN_PROTO_VERSION #define SSL_CTRL_SET_MIN_PROTO_VERSION 123 @@ -230,6 +231,21 @@ static void ResetProtocolRestrictions(SSL_CTX* ctx) SSL_CTX_ctrl(ctx, SSL_CTRL_SET_MAX_PROTO_VERSION, 0, NULL); } +/* +static void ResetProtocolRestrictions(SSL* ssl) +{ +#ifndef SSL_CTRL_SET_MIN_PROTO_VERSION +#define SSL_CTRL_SET_MIN_PROTO_VERSION 123 +#endif +#ifndef SSL_CTRL_SET_MAX_PROTO_VERSION +#define SSL_CTRL_SET_MAX_PROTO_VERSION 124 +#endif + + SSL_ctrl(ssl, SSL_CTRL_SET_MIN_PROTO_VERSION, 0, NULL); + SSL_ctrl(ssl, SSL_CTRL_SET_MAX_PROTO_VERSION, 0, NULL); +} +*/ + void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) { // Ensure that ECDHE is available @@ -278,7 +294,7 @@ void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) // We manually set protocols - we need to reset OpenSSL restrictions // to a maximum possible range - ResetProtocolRestrictions(ctx); + ResetCtxProtocolRestrictions(ctx); // OpenSSL 1.0 calls this long, OpenSSL 1.1 calls it unsigned long. #pragma clang diagnostic push @@ -469,7 +485,18 @@ void CryptoNative_SslCtxSetVerify(SSL_CTX* ctx, SslCtxSetVerifyCallback callback SSL_CTX_set_verify(ctx, mode, callback); } -int32_t CryptoNative_SetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) +void CryptoNative_SslSetVerifyPeer(SSL* ssl) +{ + SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); +} + +//void +//CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxSetCertVerifyCallbackCallback callback, void* arg) +//{ +// SSL_CTX_set_cert_verify_callback(ctx, callback, arg); +//} + +int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) { switch (policy) { @@ -477,7 +504,24 @@ int32_t CryptoNative_SetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) case NoEncryption: // No minimum security policy, same as OpenSSL 1.0 SSL_CTX_set_security_level(ctx, 0); - ResetProtocolRestrictions(ctx); + ResetCtxProtocolRestrictions(ctx); + return true; + case RequireEncryption: + return true; + } + + return false; +} + +int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy) +{ + switch (policy) + { + case AllowNoEncryption: + case NoEncryption: + // No minimum security policy, same as OpenSSL 1.0 + SSL_set_security_level(ssl, 0); + //ResetCtxProtocolRestrictions(ctx); return true; case RequireEncryption: return true; @@ -486,7 +530,7 @@ int32_t CryptoNative_SetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) return false; } -int32_t CryptoNative_SetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites) +int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites) { int32_t ret = true; @@ -513,6 +557,33 @@ int32_t CryptoNative_SetCiphers(SSL_CTX* ctx, const char* cipherList, const char return ret; } +int32_t CryptoNative_SetCiphers(SSL* ssl, const char* cipherList, const char* cipherSuites) +{ + int32_t ret = true; + + // for < TLS 1.3 + if (cipherList != NULL) + { + ret &= SSL_set_cipher_list(ssl, cipherList); + if (!ret) + { + return ret; + } + } + + // for TLS 1.3 +#if HAVE_OPENSSL_SET_CIPHERSUITES + if (CryptoNative_Tls13Supported() && cipherSuites != NULL) + { + ret &= SSL_set_ciphersuites(ssl, cipherSuites); + } +#else + (void)cipherSuites; +#endif + + return ret; +} + const char* CryptoNative_GetOpenSslCipherSuiteName(SSL* ssl, int32_t cipherSuite, int32_t* isTls12OrLower) { #if HAVE_OPENSSL_SET_CIPHERSUITES @@ -603,7 +674,8 @@ void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, #if HAVE_OPENSSL_ALPN if (API_EXISTS(SSL_CTX_set_alpn_select_cb)) { - SSL_CTX_set_alpn_select_cb(ctx, cb, arg); + (void)arg; + SSL_CTX_set_alpn_select_cb(ctx, cb, emptyAlpn); } #else (void)ctx; @@ -612,6 +684,17 @@ void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, #endif } +int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr) +{ + return SSL_set_ex_data(ssl, 0, ptr); +} + +void* CryptoNative_SslGetData(SSL* ssl) +{ +// void* data = SSL_get_ex_data(ssl, 0, ptr); + return SSL_get_ex_data(ssl, 0); +} + int32_t CryptoNative_SslCtxSetAlpnProtos(SSL_CTX* ctx, const uint8_t* protos, uint32_t protos_len) { #if HAVE_OPENSSL_ALPN @@ -630,6 +713,24 @@ int32_t CryptoNative_SslCtxSetAlpnProtos(SSL_CTX* ctx, const uint8_t* protos, ui } } +int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len) +{ +#if HAVE_OPENSSL_ALPN + if (API_EXISTS(SSL_CTX_set_alpn_protos)) + { + return SSL_set_alpn_protos(ssl, protos, protos_len); + } + else +#else + (void)ctx; + (void)protos; + (void)protos_len; +#endif + { + return 0; + } +} + void CryptoNative_SslGet0AlpnSelected(SSL* ssl, const uint8_t** protocol, uint32_t* len) { #if HAVE_OPENSSL_ALPN diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h index 79c6cbe22f955b..729696ce03fcc8 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h @@ -318,14 +318,33 @@ Shims the SSL_CTX_set_verify method. PALEXPORT void CryptoNative_SslCtxSetVerify(SSL_CTX* ctx, SslCtxSetVerifyCallback callback); /* +Shims the SSL_set_verify method. +*/ +PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl); + + +PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr); +PALEXPORT void* CryptoNative_SslGetData(SSL* ssl); + + +/* +Shims the SSL_CTX_set_cert_verify_callback method. +*/ +//PALEXPORT void +//CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxSetCertVerifyCallbackCallback callback, void* arg); + +/* + Sets the specified encryption policy on the SSL_CTX. */ -PALEXPORT int32_t CryptoNative_SetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy); +PALEXPORT int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy); +PALEXPORT int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy); /* Sets ciphers (< TLS 1.3) and cipher suites (TLS 1.3) on the SSL_CTX */ -PALEXPORT int32_t CryptoNative_SetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites); +PALEXPORT int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites); +PALEXPORT int32_t CryptoNative_SetCiphers(SSL* ssl, const char* cipherList, const char* cipherSuites); /* Determines if TLS 1.3 is supported by this OpenSSL implementation @@ -366,6 +385,7 @@ Shims the ssl_ctx_set_alpn_protos method. Returns 0 on success, non-zero on failure. */ PALEXPORT int32_t CryptoNative_SslCtxSetAlpnProtos(SSL_CTX* ctx, const uint8_t* protos, uint32_t protos_len); +PALEXPORT int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len); /* Shims the ssl_get0_alpn_selected method. diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs index facb437f02b42f..63672ebbcab6f0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Win32.SafeHandles; +using System.Collections.Concurrent; using System.Security.Cryptography.X509Certificates; namespace System.Net.Security @@ -8,12 +10,14 @@ namespace System.Net.Security public partial class SslStreamCertificateContext { private const bool TrimRootCertificate = true; + internal readonly ConcurrentDictionary contexts; private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] intermediates, SslCertificateTrust? trust) { Certificate = target; IntermediateCertificates = intermediates; Trust = trust; + contexts = new ConcurrentDictionary(); } internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, 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 c3b2e7e291e89f..78b6cf47405c3e 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 @@ -39,7 +39,7 @@ public static SecurityStatusPal InitializeSecurityContext(ref SafeFreeCredential public static SafeFreeCredentials AcquireCredentialsHandle(SslStreamCertificateContext? certificateContext, SslProtocols protocols, EncryptionPolicy policy, bool isServer) { - return new SafeFreeSslCredentials(certificateContext?.Certificate, protocols, policy); + return new SafeFreeSslCredentials(certificateContext, protocols, policy, isServer); } public static SecurityStatusPal EncryptMessage(SafeDeleteSslContext securityContext, ReadOnlyMemory input, int headerSize, int trailerSize, ref byte[] output, out int resultSize) From 6bf265c18c2c80eb378733efa0c83e4e76000e71 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 12:41:58 -0700 Subject: [PATCH 02/13] fix alpn --- .../System.Security.Cryptography.Native/Interop.OpenSsl.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 8e20a4a5676183..cd99b95331617d 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -123,7 +123,7 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html Ssl.SslCtxSetQuietShutdown(innerContext); - if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) + if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { unsafe { @@ -157,7 +157,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? sslCtx = null; SafeSslContextHandle? innerContext = null; SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; - bool cacheSslContext = sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption; + bool cacheSslContext = sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && + (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); if (sslAuthenticationOptions.CertificateContext != null && cacheSslContext) { From b6b40d753a11177e66e13c5fd3c8b22478f024ce Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 16:50:32 -0700 Subject: [PATCH 03/13] fix build with old openssl --- .../Interop.OpenSsl.cs | 16 ++-------------- .../Interop.Ssl.cs | 3 --- .../entrypoints.c | 1 - .../pal_ssl.c | 17 ----------------- .../pal_ssl.h | 8 -------- 5 files changed, 2 insertions(+), 43 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index cd99b95331617d..2f5a8ccf16cc49 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -262,13 +262,6 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia Ssl.SslSetVerifyPeer(context); } } - - // Sets policy and security level -// if (!Ssl.SetEncryptionPolicy(context, sslAuthenticationOptions.EncryptionPolicy)) -// { -// throw new SslException( -// SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); -// } } catch { @@ -285,12 +278,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (innerContext != null && cacheSslContext) { // We allocated new context - if (sslAuthenticationOptions.CertificateContext?.contexts != null && - sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext)) - { - //Console.WriteLine("Added {0} to CTX cache", sslCtx.DangerousGetHandle()); - } - else + if (sslAuthenticationOptions.CertificateContext?.contexts == null || + !sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext)) { innerContext.Dispose(); } @@ -519,7 +508,6 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte *outlen = 0; IntPtr sslData = Ssl.SslGetData(ssl); - Console.WriteLine("AlpnServerSelectCallback called for {0} and {1}", ssl, sslData); if (sslData == IntPtr.Zero) { // We did not set ALPN list. diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 5b63b073803ddf..6674f8fb36d945 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -137,9 +137,6 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetOpenSslCipherSuiteName")] private static extern IntPtr GetOpenSslCipherSuiteName(SafeSslHandle ssl, int cipherSuite, out int isTls12OrLower); - //[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetEncryptionPolicy")] - //internal static extern bool SetEncryptionPolicy(SafeSslHandle ssl, EncryptionPolicy policy); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetCiphers")] internal static extern unsafe bool SslSetCiphers(SafeSslHandle ssl, byte* cipherList, byte* cipherSuites); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 13946dbb207165..a35a273609c404 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -290,7 +290,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxSetCiphers) DllImportEntry(CryptoNative_SslCtxSetEncryptionPolicy) DllImportEntry(CryptoNative_SetCiphers) - DllImportEntry(CryptoNative_SetEncryptionPolicy) DllImportEntry(CryptoNative_SetProtocolOptions) DllImportEntry(CryptoNative_SslAddExtraChainCert) DllImportEntry(CryptoNative_SslCreate) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index 9e29e46a32b053..eef0d89a9ccc23 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -513,23 +513,6 @@ int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy po return false; } -int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy) -{ - switch (policy) - { - case AllowNoEncryption: - case NoEncryption: - // No minimum security policy, same as OpenSSL 1.0 - SSL_set_security_level(ssl, 0); - //ResetCtxProtocolRestrictions(ctx); - return true; - case RequireEncryption: - return true; - } - - return false; -} - int32_t CryptoNative_SslCtxSetCiphers(SSL_CTX* ctx, const char* cipherList, const char* cipherSuites) { int32_t ret = true; diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h index 729696ce03fcc8..2c1981a5236faf 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h @@ -326,19 +326,11 @@ PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl); PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr); PALEXPORT void* CryptoNative_SslGetData(SSL* ssl); - -/* -Shims the SSL_CTX_set_cert_verify_callback method. -*/ -//PALEXPORT void -//CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxSetCertVerifyCallbackCallback callback, void* arg); - /* Sets the specified encryption policy on the SSL_CTX. */ PALEXPORT int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy); -PALEXPORT int32_t CryptoNative_SetEncryptionPolicy(SSL* ssl, EncryptionPolicy policy); /* Sets ciphers (< TLS 1.3) and cipher suites (TLS 1.3) on the SSL_CTX From 9c99a2581ae4deb462e442c17b555103341e6beb Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 17:17:58 -0700 Subject: [PATCH 04/13] remove handling of empty ALPN --- .../Interop.OpenSsl.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 2f5a8ccf16cc49..ff98fea0f4f124 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -508,16 +508,6 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte *outlen = 0; IntPtr sslData = Ssl.SslGetData(ssl); - if (sslData == IntPtr.Zero) - { - // We did not set ALPN list. - *outlen = 0; - *outp = (byte*)arg; - return Ssl.SSL_TLSEXT_ERR_OK; - } - - //return Ssl.SSL_TLSEXT_ERR_OK; - GCHandle protocolHandle = GCHandle.FromIntPtr(sslData); if (!(protocolHandle.Target is List protocolList)) { From 30e82716b7960b48465bcf374b098d366c6d77ed Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 19:41:55 -0700 Subject: [PATCH 05/13] more OpenSSL1.0 fixes --- .../Unix/System.Security.Cryptography.Native/opensslshim.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h index 599f03294e4d7c..28c1bbfe3023f9 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/opensslshim.h @@ -115,6 +115,7 @@ int EC_POINT_set_affine_coordinates_GF2m( #undef HAVE_OPENSSL_SET_CIPHERSUITES #define HAVE_OPENSSL_SET_CIPHERSUITES 1 int SSL_CTX_set_ciphersuites(SSL_CTX *ctx, const char *str); +int SSL_set_ciphersuites(SSL *s, const char *str); const SSL_CIPHER* SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr); #endif @@ -473,7 +474,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ - FALLBACK_FUNCTION(SSL_set_security_level) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ @@ -928,7 +928,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CTX_set_options SSL_CTX_set_options_ptr #define SSL_CTX_set_quiet_shutdown SSL_CTX_set_quiet_shutdown_ptr #define SSL_CTX_set_security_level SSL_CTX_set_security_level_ptr -#define SSL_set_security_level SSL_set_security_level_ptr #define SSL_CTX_set_verify SSL_CTX_set_verify_ptr #define SSL_CTX_use_certificate SSL_CTX_use_certificate_ptr #define SSL_CTX_use_PrivateKey SSL_CTX_use_PrivateKey_ptr From 8cb81fb0abe2313ab0d5176f724866ee3a62d314 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 9 Aug 2021 20:13:47 -0700 Subject: [PATCH 06/13] fix android --- .../src/System.Net.Security.csproj | 2 +- .../SslStreamCertificateContext.Android.cs | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index 60c21f69a2cfde..aaf56dd14bed4f 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -367,7 +367,7 @@ - + diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs new file mode 100644 index 00000000000000..facb437f02b42f --- /dev/null +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Android.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Security.Cryptography.X509Certificates; + +namespace System.Net.Security +{ + public partial class SslStreamCertificateContext + { + private const bool TrimRootCertificate = true; + + private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] intermediates, SslCertificateTrust? trust) + { + Certificate = target; + IntermediateCertificates = intermediates; + Trust = trust; + } + + internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null); + } +} From 179edc8b6f52ac6d1c11b326d8f5a110c3a2a90d Mon Sep 17 00:00:00 2001 From: wfurt Date: Sun, 15 Aug 2021 21:24:08 -0700 Subject: [PATCH 07/13] feedback from review --- .../Interop.OpenSsl.cs | 126 +++++++++++------- .../Interop.Ssl.cs | 3 + .../SslStreamCertificateContext.Linux.cs | 5 +- 3 files changed, 86 insertions(+), 48 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index ff98fea0f4f124..d70fc8724dbce9 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -18,6 +18,8 @@ internal static partial class Interop { internal static partial class OpenSsl { + private const string DisableTlsResumeCtxSwitch = "System.Net.Security.DisableTlsResume"; + private const string DisableTlsResumeEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_DISABLETLSRESUME"; private static readonly IdnMapping s_idnMapping = new IdnMapping(); #region internal methods @@ -44,10 +46,38 @@ internal static partial class OpenSsl return bindingHandle; } + private static volatile int s_disableTlsResume = -1; + + private static bool DisableTlsResume + { + get + { + int disableTlsResume = s_disableTlsResume; + if (disableTlsResume != -1) + { + return disableTlsResume != 0; + } + + // First check for the AppContext switch, giving it priority over the environment variable. + if (AppContext.TryGetSwitch(DisableTlsResumeCtxSwitch, out bool value)) + { + s_disableTlsResume = value ? 1 : 0; + } + else + { + // AppContext switch wasn't used. Check the environment variable. + s_disableTlsResume = + Environment.GetEnvironmentVariable(DisableTlsResumeEnvironmentVariable) is string envVar && + (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase)) ? 1 : 0; + } + + return s_disableTlsResume != 0; + } + } + // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting - internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) + internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols) { - SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; SafeX509Handle? certHandle = credential.CertHandle; SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; @@ -63,44 +93,6 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c throw CreateSslException(SR.net_allocate_ssl_context_failed); } - if (!Interop.Ssl.Tls13Supported) - { - if (sslAuthenticationOptions.EnabledSslProtocols != SslProtocols.None && - CipherSuitesPolicyPal.WantsTls13(protocols)) - { - protocols = protocols & (~SslProtocols.Tls13); - } - } - else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (protocols == SslProtocols.None) - { - // we are using default settings but cipher suites policy says that TLS 1.3 - // is not compatible with our settings (i.e. we requested no encryption or disabled - // all TLS 1.3 cipher suites) - protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; - } - else - { - // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - } - - if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (!CipherSuitesPolicyPal.WantsTls13(protocols)) - { - // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - - protocols = SslProtocols.Tls13; - } - // Configure allowed protocols. It's ok to use DangerousGetHandle here without AddRef/Release as we just // create the handle, it's rooted by the using, no one else has a reference to it, etc. Ssl.SetProtocolOptions(innerContext, protocols); @@ -157,18 +149,57 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? sslCtx = null; SafeSslContextHandle? innerContext = null; SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; - bool cacheSslContext = sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && - (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); + bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && + (!sslAuthenticationOptions.IsServer || + (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); + + if (!Interop.Ssl.Tls13Supported) + { + if (protocols != SslProtocols.None && + CipherSuitesPolicyPal.WantsTls13(protocols)) + { + protocols = protocols & (~SslProtocols.Tls13); + } + } + else if (CipherSuitesPolicyPal.WantsTls13(protocols) && + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (protocols == SslProtocols.None) + { + // we are using default settings but cipher suites policy says that TLS 1.3 + // is not compatible with our settings (i.e. we requested no encryption or disabled + // all TLS 1.3 cipher suites) + protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; + } + else + { + // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + + if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (!CipherSuitesPolicyPal.WantsTls13(protocols)) + { + // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + + protocols = SslProtocols.Tls13; + } if (sslAuthenticationOptions.CertificateContext != null && cacheSslContext) { - sslAuthenticationOptions.CertificateContext.contexts.TryGetValue((int)sslAuthenticationOptions.EnabledSslProtocols, out sslCtx); + sslAuthenticationOptions.CertificateContext.SslContexts.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtx); } if (sslCtx == null) { // We did not get SslContext from cache - sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions); + sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions, protocols); } try @@ -278,8 +309,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (innerContext != null && cacheSslContext) { // We allocated new context - if (sslAuthenticationOptions.CertificateContext?.contexts == null || - !sslAuthenticationOptions.CertificateContext.contexts.TryAdd((int)sslAuthenticationOptions.EnabledSslProtocols, innerContext)) + if (sslAuthenticationOptions.CertificateContext?.SslContexts == null || + !sslAuthenticationOptions.CertificateContext.SslContexts.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, innerContext)) { innerContext.Dispose(); } @@ -508,6 +539,9 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte *outlen = 0; IntPtr sslData = Ssl.SslGetData(ssl); + // reset application data to avoid dangling pointer. + Ssl.SslSetData(ssl, IntPtr.Zero); + GCHandle protocolHandle = GCHandle.FromIntPtr(sslData); if (!(protocolHandle.Target is List protocolList)) { diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 6674f8fb36d945..5d19c09930bee5 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -150,6 +150,9 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetData")] internal static extern int SslSetData(SafeSslHandle ssl, IntPtr data); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetData")] + internal static extern int SslSetData(IntPtr ssl, IntPtr data); + internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, List protocols) { byte[] buffer = ConvertAlpnProtocolListToByteArray(protocols); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs index 63672ebbcab6f0..0b40ccc367e842 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs @@ -3,6 +3,7 @@ using Microsoft.Win32.SafeHandles; using System.Collections.Concurrent; +using System.Security.Authentication; using System.Security.Cryptography.X509Certificates; namespace System.Net.Security @@ -10,14 +11,14 @@ namespace System.Net.Security public partial class SslStreamCertificateContext { private const bool TrimRootCertificate = true; - internal readonly ConcurrentDictionary contexts; + internal readonly ConcurrentDictionary SslContexts; private SslStreamCertificateContext(X509Certificate2 target, X509Certificate2[] intermediates, SslCertificateTrust? trust) { Certificate = target; IntermediateCertificates = intermediates; Trust = trust; - contexts = new ConcurrentDictionary(); + SslContexts = new ConcurrentDictionary(); } internal static SslStreamCertificateContext Create(X509Certificate2 target) => Create(target, null); From 55a17700b24bbf492d7331eb02672717d9e35283 Mon Sep 17 00:00:00 2001 From: wfurt Date: Sun, 15 Aug 2021 21:29:18 -0700 Subject: [PATCH 08/13] dispose innerContext if we don't cache --- .../System.Security.Cryptography.Native/Interop.OpenSsl.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index d70fc8724dbce9..72bb8e93b6fbaa 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -306,10 +306,10 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia } finally { - if (innerContext != null && cacheSslContext) + if (innerContext != null) { - // We allocated new context - if (sslAuthenticationOptions.CertificateContext?.SslContexts == null || + // We allocated new context and we want to cache + if (!cacheSslContext || sslAuthenticationOptions.CertificateContext?.SslContexts == null || !sslAuthenticationOptions.CertificateContext.SslContexts.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, innerContext)) { innerContext.Dispose(); From 9aed0c8681778898460c43fa7a6825351b12d956 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 16 Aug 2021 14:06:58 -0700 Subject: [PATCH 09/13] disable TLS resume with CipherSuitePolicy --- .../Interop.OpenSsl.cs | 97 +++++++++++++------ .../Interop.SetProtocolOptions.cs | 8 +- .../entrypoints.c | 2 +- .../pal_ssl.c | 27 +----- .../pal_ssl.h | 2 +- 5 files changed, 76 insertions(+), 60 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 72bb8e93b6fbaa..71a765ab98a9df 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -76,11 +76,11 @@ private static bool DisableTlsResume } // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting - internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols) + internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) { SafeX509Handle? certHandle = credential.CertHandle; SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; - + SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; // Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest // mutually supported version and can thus handle any of the set protocols, and we then use @@ -93,9 +93,47 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c throw CreateSslException(SR.net_allocate_ssl_context_failed); } + if (!Interop.Ssl.Tls13Supported) + { + if (protocols != SslProtocols.None && + CipherSuitesPolicyPal.WantsTls13(protocols)) + { + protocols = protocols & (~SslProtocols.Tls13); + } + } + else if (CipherSuitesPolicyPal.WantsTls13(protocols) && + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (protocols == SslProtocols.None) + { + // we are using default settings but cipher suites policy says that TLS 1.3 + // is not compatible with our settings (i.e. we requested no encryption or disabled + // all TLS 1.3 cipher suites) + protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; + } + else + { + // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + + if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (!CipherSuitesPolicyPal.WantsTls13(protocols)) + { + // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + + protocols = SslProtocols.Tls13; + } + // Configure allowed protocols. It's ok to use DangerousGetHandle here without AddRef/Release as we just // create the handle, it's rooted by the using, no one else has a reference to it, etc. - Ssl.SetProtocolOptions(innerContext, protocols); + Ssl.SslCtxSetProtocolOptions(innerContext, protocols); if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) { @@ -106,6 +144,29 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c } } + byte[]? cipherList = + CipherSuitesPolicyPal.GetOpenSslCipherList(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); + + Debug.Assert(cipherList == null || (cipherList.Length >= 1 && cipherList[cipherList.Length - 1] == 0)); + + byte[]? cipherSuites = + CipherSuitesPolicyPal.GetOpenSslCipherSuites(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); + + Debug.Assert(cipherSuites == null || (cipherSuites.Length >= 1 && cipherSuites[cipherSuites.Length - 1] == 0)); + + unsafe + { + fixed (byte* cipherListStr = cipherList) + fixed (byte* cipherSuitesStr = cipherSuites) + { + if (!Ssl.SslCtxSetCiphers(innerContext, cipherListStr, cipherSuitesStr)) + { + Crypto.ErrClearError(); + throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + } + // The logic in SafeSslHandle.Disconnect is simple because we are doing a quiet // shutdown (we aren't negotiating for session close to enable later session // restoration). @@ -150,6 +211,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? innerContext = null; SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && + sslAuthenticationOptions.CipherSuitesPolicy == null && (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); @@ -199,7 +261,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (sslCtx == null) { // We did not get SslContext from cache - sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions, protocols); + sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions); } try @@ -230,6 +292,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia { alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); Interop.Ssl.SslSetData(context, GCHandle.ToIntPtr(alpnHandle)); + context.AlpnHandle = alpnHandle; } else { @@ -252,29 +315,6 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia } } - byte[]? cipherList = - CipherSuitesPolicyPal.GetOpenSslCipherList(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); - - Debug.Assert(cipherList == null || (cipherList.Length >= 1 && cipherList[cipherList.Length - 1] == 0)); - - byte[]? cipherSuites = - CipherSuitesPolicyPal.GetOpenSslCipherSuites(sslAuthenticationOptions.CipherSuitesPolicy, protocols, sslAuthenticationOptions.EncryptionPolicy); - - Debug.Assert(cipherSuites == null || (cipherSuites.Length >= 1 && cipherSuites[cipherSuites.Length - 1] == 0)); - - unsafe - { - fixed (byte* cipherListStr = cipherList) - fixed (byte* cipherSuitesStr = cipherSuites) - { - if (!Ssl.SslSetCiphers(context, cipherListStr, cipherSuitesStr)) - { - Crypto.ErrClearError(); - throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - } - } - if (sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) { if (!Ssl.AddExtraChainCertificates(context, sslAuthenticationOptions.CertificateContext!.IntermediateCertificates)) @@ -283,9 +323,6 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia } } - context.AlpnHandle = alpnHandle; - - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) { unsafe diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs index 01c7ab1985e7b0..a8439617d1d689 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SetProtocolOptions.cs @@ -10,10 +10,10 @@ internal static partial class Interop { internal static partial class Ssl { - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetProtocolOptions")] - internal static extern void SetProtocolOptions(IntPtr ctx, SslProtocols protocols); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetProtocolOptions")] + internal static extern void SslCtxSetProtocolOptions(IntPtr ctx, SslProtocols protocols); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetProtocolOptions")] - internal static extern void SetProtocolOptions(SafeSslContextHandle ctx, SslProtocols protocols); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetProtocolOptions")] + internal static extern void SslCtxSetProtocolOptions(SafeSslContextHandle ctx, SslProtocols protocols); } } diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index a35a273609c404..51514fef0dd5eb 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -290,7 +290,6 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxSetCiphers) DllImportEntry(CryptoNative_SslCtxSetEncryptionPolicy) DllImportEntry(CryptoNative_SetCiphers) - DllImportEntry(CryptoNative_SetProtocolOptions) DllImportEntry(CryptoNative_SslAddExtraChainCert) DllImportEntry(CryptoNative_SslCreate) DllImportEntry(CryptoNative_SslCtxCheckPrivateKey) @@ -298,6 +297,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxDestroy) DllImportEntry(CryptoNative_SslCtxSetAlpnProtos) DllImportEntry(CryptoNative_SslCtxSetAlpnSelectCb) + DllImportEntry(CryptoNative_SslCtxSetProtocolOptions) DllImportEntry(CryptoNative_SslCtxSetQuietShutdown) DllImportEntry(CryptoNative_SslCtxSetVerify) DllImportEntry(CryptoNative_SslCtxUseCertificate) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index eef0d89a9ccc23..b42c858d0c4527 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -231,22 +231,7 @@ static void ResetCtxProtocolRestrictions(SSL_CTX* ctx) SSL_CTX_ctrl(ctx, SSL_CTRL_SET_MAX_PROTO_VERSION, 0, NULL); } -/* -static void ResetProtocolRestrictions(SSL* ssl) -{ -#ifndef SSL_CTRL_SET_MIN_PROTO_VERSION -#define SSL_CTRL_SET_MIN_PROTO_VERSION 123 -#endif -#ifndef SSL_CTRL_SET_MAX_PROTO_VERSION -#define SSL_CTRL_SET_MAX_PROTO_VERSION 124 -#endif - - SSL_ctrl(ssl, SSL_CTRL_SET_MIN_PROTO_VERSION, 0, NULL); - SSL_ctrl(ssl, SSL_CTRL_SET_MAX_PROTO_VERSION, 0, NULL); -} -*/ - -void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) +void CryptoNative_SslCtxSetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols) { // Ensure that ECDHE is available if (TrySetECDHNamedCurve(ctx) == 0) @@ -490,12 +475,6 @@ void CryptoNative_SslSetVerifyPeer(SSL* ssl) SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } -//void -//CryptoNative_SslCtxSetCertVerifyCallback(SSL_CTX* ctx, SslCtxSetCertVerifyCallbackCallback callback, void* arg) -//{ -// SSL_CTX_set_cert_verify_callback(ctx, callback, arg); -//} - int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) { switch (policy) @@ -821,8 +800,8 @@ int32_t CryptoNative_OpenSslGetProtocolSupport(SslProtocols protocol) if (clientCtx != NULL && serverCtx != NULL && cert != NULL && evp != NULL && bio1 != NULL && bio2 != NULL) { - CryptoNative_SetProtocolOptions(serverCtx, protocol); - CryptoNative_SetProtocolOptions(clientCtx, protocol); + CryptoNative_SslCtxSetProtocolOptions(serverCtx, protocol); + CryptoNative_SslCtxSetProtocolOptions(clientCtx, protocol); SSL_CTX_set_verify(clientCtx, SSL_VERIFY_NONE, NULL); SSL_CTX_set_verify(serverCtx, SSL_VERIFY_NONE, NULL); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h index 2c1981a5236faf..d79a9cc0c13de1 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h @@ -142,7 +142,7 @@ PALEXPORT SSL_CTX* CryptoNative_SslCtxCreate(const SSL_METHOD* method); /* Sets the specified protocols in the SSL_CTX options. */ -PALEXPORT void CryptoNative_SetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols); +PALEXPORT void CryptoNative_SslCtxSetProtocolOptions(SSL_CTX* ctx, SslProtocols protocols); /* Shims the SSL_new method. From 06dc955d3af570d9815d94c0615dac0cf5748d87 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 25 Aug 2021 21:07:53 -0700 Subject: [PATCH 10/13] partial feedback from review --- .../Interop.OpenSsl.cs | 92 ++++++++----------- .../Interop.Ssl.cs | 80 +++++----------- .../Interop.SslCtx.cs | 41 +++------ .../Interop.SslCtxOptions.cs | 3 + .../entrypoints.c | 3 +- .../pal_ssl.c | 29 +----- .../pal_ssl.h | 15 ++- 7 files changed, 96 insertions(+), 167 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 71a765ab98a9df..e5bee9b0a37694 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -85,10 +85,10 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c // Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest // mutually supported version and can thus handle any of the set protocols, and we then use // SetProtocolOptions to ensure we only allow the ones requested. - SafeSslContextHandle innerContext = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method); + SafeSslContextHandle sslCtx = Ssl.SslCtxCreate(Ssl.SslMethods.SSLv23_method); try { - if (innerContext.IsInvalid) + if (sslCtx.IsInvalid) { throw CreateSslException(SR.net_allocate_ssl_context_failed); } @@ -102,7 +102,7 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c } } else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) { if (protocols == SslProtocols.None) { @@ -131,14 +131,12 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c protocols = SslProtocols.Tls13; } - // Configure allowed protocols. It's ok to use DangerousGetHandle here without AddRef/Release as we just - // create the handle, it's rooted by the using, no one else has a reference to it, etc. - Ssl.SslCtxSetProtocolOptions(innerContext, protocols); + Ssl.SslCtxSetProtocolOptions(sslCtx, protocols); if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) { // Sets policy and security level - if (!Ssl.SetEncryptionPolicy(innerContext, sslAuthenticationOptions.EncryptionPolicy)) + if (!Ssl.SetEncryptionPolicy(sslCtx, sslAuthenticationOptions.EncryptionPolicy)) { throw new SslException( SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } @@ -159,7 +157,7 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c fixed (byte* cipherListStr = cipherList) fixed (byte* cipherSuitesStr = cipherSuites) { - if (!Ssl.SslCtxSetCiphers(innerContext, cipherListStr, cipherSuitesStr)) + if (!Ssl.SslCtxSetCiphers(sslCtx, cipherListStr, cipherSuitesStr)) { Crypto.ErrClearError(); throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); @@ -174,13 +172,13 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c // If you find yourself wanting to remove this line to enable bidirectional // close-notify, you'll probably need to rewrite SafeSslHandle.Disconnect(). // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html - Ssl.SslCtxSetQuietShutdown(innerContext); + Ssl.SslCtxSetQuietShutdown(sslCtx); if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { unsafe { - Interop.Ssl.SslCtxSetAlpnSelectCb(innerContext, &AlpnServerSelectCallback, IntPtr.Zero); + Interop.Ssl.SslCtxSetAlpnSelectCb(sslCtx, &AlpnServerSelectCallback, IntPtr.Zero); } } @@ -190,28 +188,35 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c if (hasCertificateAndKey) { - SetSslCertificate(innerContext, certHandle!, certKeyHandle!); + SetSslCertificate(sslCtx, certHandle!, certKeyHandle!); } + if (sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) + { + if (!Ssl.AddExtraChainCertificates(sslCtx, sslAuthenticationOptions.CertificateContext.IntermediateCertificates)) + { + throw CreateSslException(SR.net_ssl_use_cert_failed); + } + } } catch { - innerContext.Dispose(); + sslCtx.Dispose(); throw; } - return innerContext; + return sslCtx; } // This essentially wraps SSL* SSL_new() internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) { - SafeSslHandle? context = null; - SafeSslContextHandle? sslCtx = null; - SafeSslContextHandle? innerContext = null; + SafeSslHandle? sslHandle = null; + SafeSslContextHandle? sslCtxHandle = null; + SafeSslContextHandle? newCtxHandle = null; SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && - sslAuthenticationOptions.CipherSuitesPolicy == null && + sslAuthenticationOptions.CipherSuitesPolicy == null && (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); @@ -220,11 +225,11 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (protocols != SslProtocols.None && CipherSuitesPolicyPal.WantsTls13(protocols)) { - protocols = protocols & (~SslProtocols.Tls13); + protocols &= ~SslProtocols.Tls13; } } else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) { if (protocols == SslProtocols.None) { @@ -255,13 +260,13 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (sslAuthenticationOptions.CertificateContext != null && cacheSslContext) { - sslAuthenticationOptions.CertificateContext.SslContexts.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtx); + sslAuthenticationOptions.CertificateContext.SslContexts.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtxHandle); } - if (sslCtx == null) + if (sslCtxHandle == null) { // We did not get SslContext from cache - sslCtx = innerContext = AllocateSslContext(credential, sslAuthenticationOptions); + sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions); } try @@ -269,34 +274,25 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia GCHandle alpnHandle = default; try { - context = SafeSslHandle.Create(sslCtx, sslAuthenticationOptions.IsServer); - Debug.Assert(context != null, "Expected non-null return value from SafeSslHandle.Create"); - if (context.IsInvalid) + sslHandle = SafeSslHandle.Create(sslCtxHandle, sslAuthenticationOptions.IsServer); + Debug.Assert(sslHandle != null, "Expected non-null return value from SafeSslHandle.Create"); + if (sslHandle.IsInvalid) { - context.Dispose(); + sslHandle.Dispose(); throw CreateSslException(SR.net_allocate_ssl_context_failed); } - if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) - { - // Sets policy and security level - if (!Ssl.SetEncryptionPolicy(sslCtx, sslAuthenticationOptions.EncryptionPolicy)) - { - throw new SslException( SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - } - if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) { if (sslAuthenticationOptions.IsServer) { alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); - Interop.Ssl.SslSetData(context, GCHandle.ToIntPtr(alpnHandle)); - context.AlpnHandle = alpnHandle; + Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(alpnHandle)); + sslHandle.AlpnHandle = alpnHandle; } else { - if (Interop.Ssl.SslSetAlpnProtos(context, sslAuthenticationOptions.ApplicationProtocols) != 0) + if (Interop.Ssl.SslSetAlpnProtos(sslHandle, sslAuthenticationOptions.ApplicationProtocols) != 0) { throw CreateSslException(SR.net_alpn_config_failed); } @@ -309,25 +305,17 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. - if (!Ssl.SslSetTlsExtHostName(context, punyCode)) + if (!Ssl.SslSetTlsExtHostName(sslHandle, punyCode)) { Crypto.ErrClearError(); } } - if (sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.IntermediateCertificates.Length > 0) - { - if (!Ssl.AddExtraChainCertificates(context, sslAuthenticationOptions.CertificateContext!.IntermediateCertificates)) - { - throw CreateSslException(SR.net_ssl_use_cert_failed); - } - } - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) { unsafe { - Ssl.SslSetVerifyPeer(context); + Ssl.SslSetVerifyPeer(sslHandle); } } } @@ -343,18 +331,18 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia } finally { - if (innerContext != null) + if (newCtxHandle != null) { // We allocated new context and we want to cache if (!cacheSslContext || sslAuthenticationOptions.CertificateContext?.SslContexts == null || - !sslAuthenticationOptions.CertificateContext.SslContexts.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, innerContext)) + !sslAuthenticationOptions.CertificateContext.SslContexts.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, newCtxHandle)) { - innerContext.Dispose(); + newCtxHandle.Dispose(); } } } - return context; + return sslHandle; } internal static SecurityStatusPal SslRenegotiate(SafeSslHandle sslContext, out byte[]? outputBuffer) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index 5d19c09930bee5..e949096f505f86 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -124,9 +124,6 @@ internal static partial class Ssl [return: MarshalAs(UnmanagedType.Bool)] internal static extern bool SslSessionReused(SafeSslHandle ssl); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslAddExtraChainCert")] - internal static extern bool SslAddExtraChainCert(SafeSslHandle ssl, SafeX509Handle x509); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetClientCAList")] private static extern SafeSharedX509NameStackHandle SslGetClientCAList_private(SafeSslHandle ssl); @@ -162,6 +159,31 @@ internal static unsafe int SslSetAlpnProtos(SafeSslHandle ssl, List applicationProtocols) + { + int protocolSize = 0; + foreach (SslApplicationProtocol protocol in applicationProtocols) + { + if (protocol.Protocol.Length == 0 || protocol.Protocol.Length > byte.MaxValue) + { + throw new ArgumentException(SR.net_ssl_app_protocols_invalid, nameof(applicationProtocols)); + } + + protocolSize += protocol.Protocol.Length + 1; + } + + byte[] buffer = new byte[protocolSize]; + var offset = 0; + foreach (SslApplicationProtocol protocol in applicationProtocols) + { + buffer[offset++] = (byte)(protocol.Protocol.Length); + protocol.Protocol.Span.CopyTo(buffer.AsSpan(offset)); + offset += protocol.Protocol.Length; + } + + return buffer; + } + internal static string? GetOpenSslCipherSuiteName(SafeSslHandle ssl, TlsCipherSuite cipherSuite, out bool isTls12OrLower) { string? ret = Marshal.PtrToStringAnsi(GetOpenSslCipherSuiteName(ssl, (int)cipherSuite, out int isTls12OrLowerInt)); @@ -188,58 +210,6 @@ internal static SafeSharedX509NameStackHandle SslGetClientCAList(SafeSslHandle s return handle; } - internal static bool AddExtraChainCertificates(SafeSslHandle sslContext, X509Chain chain) - { - Debug.Assert(chain != null, "X509Chain should not be null"); - Debug.Assert(chain.ChainElements.Count > 0, "chain.Build should have already been called"); - - // If the last certificate is a root certificate, don't send it. PartialChain means the last cert wasn't a root. - int stop = chain.ChainElements.Count - 1; - foreach (X509ChainStatus s in chain.ChainStatus) - { - if ((s.Status & X509ChainStatusFlags.PartialChain) != 0) - { - stop++; - break; - } - } - - // Don't include the first item (the cert whose private key we have) - for (int i = 1; i < stop; i++) - { - SafeX509Handle dupCertHandle = Crypto.X509UpRef(chain.ChainElements[i].Certificate!.Handle); - Crypto.CheckValidOpenSslHandle(dupCertHandle); - if (!SslAddExtraChainCert(sslContext, dupCertHandle)) - { - Crypto.ErrClearError(); - dupCertHandle.Dispose(); // we still own the safe handle; clean it up - return false; - } - dupCertHandle.SetHandleAsInvalid(); // ownership has been transferred to sslHandle; do not free via this safe handle - } - - return true; - } - - internal static bool AddExtraChainCertificates(SafeSslHandle sslContext, X509Certificate2[] chain) - { - // send pre-computed list of intermediates. - for (int i = 0; i < chain.Length; i++) - { - SafeX509Handle dupCertHandle = Crypto.X509UpRef(chain[i].Handle); - Crypto.CheckValidOpenSslHandle(dupCertHandle); - if (!SslAddExtraChainCert(sslContext, dupCertHandle)) - { - Crypto.ErrClearError(); - dupCertHandle.Dispose(); // we still own the safe handle; clean it up - return false; - } - dupCertHandle.SetHandleAsInvalid(); // ownership has been transferred to sslHandle; do not free via this safe handle - } - - return true; - } - internal static class SslMethods { internal static readonly IntPtr SSLv23_method = SslV2_3Method(); diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs index d39b6595d39b1e..e8e208e5859aae 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Net.Security; using System.Runtime.InteropServices; +using System.Security.Cryptography.X509Certificates; using System.Text; using Microsoft.Win32.SafeHandles; @@ -18,44 +19,26 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxDestroy")] internal static extern void SslCtxDestroy(IntPtr ctx); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetAlpnProtos")] - internal static extern int SslCtxSetAlpnProtos(SafeSslContextHandle ctx, IntPtr protos, int len); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetAlpnSelectCb")] internal static extern unsafe void SslCtxSetAlpnSelectCb(SafeSslContextHandle ctx, delegate* unmanaged callback, IntPtr arg); - internal static unsafe int SslCtxSetAlpnProtos(SafeSslContextHandle ctx, List protocols) - { - byte[] buffer = ConvertAlpnProtocolListToByteArray(protocols); - fixed (byte* b = buffer) - { - return SslCtxSetAlpnProtos(ctx, (IntPtr)b, buffer.Length); - } - } - - internal static byte[] ConvertAlpnProtocolListToByteArray(List applicationProtocols) + internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, X509Certificate2[] chain) { - int protocolSize = 0; - foreach (SslApplicationProtocol protocol in applicationProtocols) + // send pre-computed list of intermediates. + for (int i = 0; i < chain.Length; i++) { - if (protocol.Protocol.Length == 0 || protocol.Protocol.Length > byte.MaxValue) + SafeX509Handle dupCertHandle = Crypto.X509UpRef(chain[i].Handle); + Crypto.CheckValidOpenSslHandle(dupCertHandle); + if (!SslCtxAddExtraChainCert(ctx, dupCertHandle)) { - throw new ArgumentException(SR.net_ssl_app_protocols_invalid, nameof(applicationProtocols)); + Crypto.ErrClearError(); + dupCertHandle.Dispose(); // we still own the safe handle; clean it up + return false; } - - protocolSize += protocol.Protocol.Length + 1; + dupCertHandle.SetHandleAsInvalid(); // ownership has been transferred to sslHandle; do not free via this safe handle } - byte[] buffer = new byte[protocolSize]; - var offset = 0; - foreach (SslApplicationProtocol protocol in applicationProtocols) - { - buffer[offset++] = (byte)(protocol.Protocol.Length); - protocol.Protocol.Span.CopyTo(buffer.AsSpan(offset)); - offset += protocol.Protocol.Length; - } - - return buffer; + return true; } } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs index 0e74b39838e37e..60c81072325c60 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtxOptions.cs @@ -11,6 +11,9 @@ internal static partial class Interop { internal static partial class Ssl { + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxAddExtraChainCert")] + internal static extern bool SslCtxAddExtraChainCert(SafeSslContextHandle ctx, SafeX509Handle x509); + [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxUseCertificate")] internal static extern int SslCtxUseCertificate(SafeSslContextHandle ctx, SafeX509Handle certPtr); diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c index 51514fef0dd5eb..102ccd91e8b44b 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/entrypoints.c @@ -287,15 +287,14 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslRenegotiate) DllImportEntry(CryptoNative_IsSslRenegotiatePending) DllImportEntry(CryptoNative_IsSslStateOK) + DllImportEntry(CryptoNative_SslCtxAddExtraChainCert) DllImportEntry(CryptoNative_SslCtxSetCiphers) DllImportEntry(CryptoNative_SslCtxSetEncryptionPolicy) DllImportEntry(CryptoNative_SetCiphers) - DllImportEntry(CryptoNative_SslAddExtraChainCert) DllImportEntry(CryptoNative_SslCreate) DllImportEntry(CryptoNative_SslCtxCheckPrivateKey) DllImportEntry(CryptoNative_SslCtxCreate) DllImportEntry(CryptoNative_SslCtxDestroy) - DllImportEntry(CryptoNative_SslCtxSetAlpnProtos) DllImportEntry(CryptoNative_SslCtxSetAlpnSelectCb) DllImportEntry(CryptoNative_SslCtxSetProtocolOptions) DllImportEntry(CryptoNative_SslCtxSetQuietShutdown) diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c index b42c858d0c4527..2b4eb271b022a2 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.c @@ -39,7 +39,7 @@ static void EnsureLibSsl10Initialized() #endif static int32_t g_config_specified_ciphersuites = 0; -static char* emptyAlpn = ""; +static char* g_emptyAlpn = ""; static void DetectCiphersuiteConfiguration() { @@ -615,15 +615,14 @@ int32_t CryptoNative_Tls13Supported() #endif } -int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509) +int32_t CryptoNative_SslCtxAddExtraChainCert(SSL_CTX* ctx, X509* x509) { - if (!x509 || !ssl) + if (!x509 || !ctx) { return 0; } - SSL_CTX* ssl_ctx = SSL_get_SSL_CTX(ssl); - if (SSL_CTX_add_extra_chain_cert(ssl_ctx, x509) == 1) + if (SSL_CTX_add_extra_chain_cert(ctx, x509) == 1) { return 1; } @@ -637,7 +636,7 @@ void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, if (API_EXISTS(SSL_CTX_set_alpn_select_cb)) { (void)arg; - SSL_CTX_set_alpn_select_cb(ctx, cb, emptyAlpn); + SSL_CTX_set_alpn_select_cb(ctx, cb, g_emptyAlpn); } #else (void)ctx; @@ -657,24 +656,6 @@ void* CryptoNative_SslGetData(SSL* ssl) return SSL_get_ex_data(ssl, 0); } -int32_t CryptoNative_SslCtxSetAlpnProtos(SSL_CTX* ctx, const uint8_t* protos, uint32_t protos_len) -{ -#if HAVE_OPENSSL_ALPN - if (API_EXISTS(SSL_CTX_set_alpn_protos)) - { - return SSL_CTX_set_alpn_protos(ctx, protos, protos_len); - } - else -#else - (void)ctx; - (void)protos; - (void)protos_len; -#endif - { - return 0; - } -} - int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len) { #if HAVE_OPENSSL_ALPN diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h index d79a9cc0c13de1..0a18f7a764c17f 100644 --- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_ssl.h @@ -322,8 +322,14 @@ Shims the SSL_set_verify method. */ PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl); - +/* +Shims SSL_set_ex_data to attach application context. +*/ PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr); + +/* +Shims SSL_get_ex_data to retrieve application context. +*/ PALEXPORT void* CryptoNative_SslGetData(SSL* ssl); /* @@ -360,12 +366,12 @@ Shims the SSL_session_reused macro. PALEXPORT int32_t CryptoNative_SslSessionReused(SSL* ssl); /* -adds the given certificate to the extra chain certificates associated with ctx that is associated with the ssl. +adds the given certificate to the extra chain certificates associated with ctx. libssl frees the x509 object. Returns 1 if success and 0 in case of failure */ -PALEXPORT int32_t CryptoNative_SslAddExtraChainCert(SSL* ssl, X509* x509); +PALEXPORT int32_t CryptoNative_SslCtxAddExtraChainCert(SSL_CTX* ctx, X509* x509); /* Shims the ssl_ctx_set_alpn_select_cb method. @@ -373,10 +379,9 @@ Shims the ssl_ctx_set_alpn_select_cb method. PALEXPORT void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void *arg); /* -Shims the ssl_ctx_set_alpn_protos method. +Shims the ssl_set_alpn_protos method. Returns 0 on success, non-zero on failure. */ -PALEXPORT int32_t CryptoNative_SslCtxSetAlpnProtos(SSL_CTX* ctx, const uint8_t* protos, uint32_t protos_len); PALEXPORT int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len); /* From 3b21da79e369fa1dd31bd185c33b03e7b3140247 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 25 Aug 2021 21:46:20 -0700 Subject: [PATCH 11/13] fix build --- src/libraries/System.Net.Http/src/System.Net.Http.csproj | 2 ++ src/libraries/System.Net.Quic/src/System.Net.Quic.csproj | 1 + 2 files changed, 3 insertions(+) diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index f6bcf3faacba20..516b4ddeb54693 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -596,6 +596,8 @@ Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Ssl.cs" /> + + From df51d00e59c24f224369477cee9b07468a42cd9d Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 26 Aug 2021 14:19:02 -0700 Subject: [PATCH 12/13] feedback from review --- .../Interop.OpenSsl.cs | 99 +++++++++---------- 1 file changed, 46 insertions(+), 53 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index e5bee9b0a37694..be839b812fc15c 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -216,6 +216,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? newCtxHandle = null; SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && + sslAuthenticationOptions.CertificateContext != null && + sslAuthenticationOptions.CertificateContext.SslContexts != null && sslAuthenticationOptions.CipherSuitesPolicy == null && (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); @@ -258,88 +260,79 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia protocols = SslProtocols.Tls13; } - if (sslAuthenticationOptions.CertificateContext != null && cacheSslContext) + if (cacheSslContext) { - sslAuthenticationOptions.CertificateContext.SslContexts.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtxHandle); + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtxHandle); } if (sslCtxHandle == null) { // We did not get SslContext from cache sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions); + + if (cacheSslContext && sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, newCtxHandle)) + { + newCtxHandle = null; + } } + GCHandle alpnHandle = default; try { - GCHandle alpnHandle = default; - try + sslHandle = SafeSslHandle.Create(sslCtxHandle, sslAuthenticationOptions.IsServer); + Debug.Assert(sslHandle != null, "Expected non-null return value from SafeSslHandle.Create"); + if (sslHandle.IsInvalid) { - sslHandle = SafeSslHandle.Create(sslCtxHandle, sslAuthenticationOptions.IsServer); - Debug.Assert(sslHandle != null, "Expected non-null return value from SafeSslHandle.Create"); - if (sslHandle.IsInvalid) - { - sslHandle.Dispose(); - throw CreateSslException(SR.net_allocate_ssl_context_failed); - } - - if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) - { - if (sslAuthenticationOptions.IsServer) - { - alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); - Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(alpnHandle)); - sslHandle.AlpnHandle = alpnHandle; - } - else - { - if (Interop.Ssl.SslSetAlpnProtos(sslHandle, sslAuthenticationOptions.ApplicationProtocols) != 0) - { - throw CreateSslException(SR.net_alpn_config_failed); - } - } - } + sslHandle.Dispose(); + throw CreateSslException(SR.net_allocate_ssl_context_failed); + } - if (!sslAuthenticationOptions.IsServer) + if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) + { + if (sslAuthenticationOptions.IsServer) { - // The IdnMapping converts unicode input into the IDNA punycode sequence. - string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); - - // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. - if (!Ssl.SslSetTlsExtHostName(sslHandle, punyCode)) - { - Crypto.ErrClearError(); - } + alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); + Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(alpnHandle)); + sslHandle.AlpnHandle = alpnHandle; } - - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) + else { - unsafe + if (Interop.Ssl.SslSetAlpnProtos(sslHandle, sslAuthenticationOptions.ApplicationProtocols) != 0) { - Ssl.SslSetVerifyPeer(sslHandle); + throw CreateSslException(SR.net_alpn_config_failed); } } } - catch + + if (!sslAuthenticationOptions.IsServer) { - if (alpnHandle.IsAllocated) + // The IdnMapping converts unicode input into the IDNA punycode sequence. + string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); + + // Similar to windows behavior, set SNI on openssl by default for client context, ignore errors. + if (!Ssl.SslSetTlsExtHostName(sslHandle, punyCode)) { - alpnHandle.Free(); + Crypto.ErrClearError(); } + } - throw; + if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.RemoteCertRequired) + { + Ssl.SslSetVerifyPeer(sslHandle); } } - finally + catch { - if (newCtxHandle != null) + if (alpnHandle.IsAllocated) { - // We allocated new context and we want to cache - if (!cacheSslContext || sslAuthenticationOptions.CertificateContext?.SslContexts == null || - !sslAuthenticationOptions.CertificateContext.SslContexts.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, newCtxHandle)) - { - newCtxHandle.Dispose(); - } + alpnHandle.Free(); } + + throw; + } + finally + { + newCtxHandle?.Dispose(); } return sslHandle; From 47f3742e51d9085b1aa33613d9cb4669b57ec7ee Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 26 Aug 2021 14:43:37 -0700 Subject: [PATCH 13/13] refactor protocol selection to helper function --- .../Interop.OpenSsl.cs | 137 +++++++----------- .../Interop.Ssl.cs | 1 - 2 files changed, 53 insertions(+), 85 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index be839b812fc15c..828af2acfac0ee 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -72,15 +72,60 @@ private static bool DisableTlsResume } return s_disableTlsResume != 0; - } - } + } + } + + // This is helper function to adjust requested protocols based on CipherSuitePolicy and system capability. + private static SslProtocols CalculateEffectiveProtocols(SslAuthenticationOptions sslAuthenticationOptions) + { + SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; + + if (!Interop.Ssl.Tls13Supported) + { + if (protocols != SslProtocols.None && + CipherSuitesPolicyPal.WantsTls13(protocols)) + { + protocols = protocols & (~SslProtocols.Tls13); + } + } + else if (CipherSuitesPolicyPal.WantsTls13(protocols) && + CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (protocols == SslProtocols.None) + { + // we are using default settings but cipher suites policy says that TLS 1.3 + // is not compatible with our settings (i.e. we requested no encryption or disabled + // all TLS 1.3 cipher suites) + protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; + } + else + { + // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + } + + if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) + { + if (!CipherSuitesPolicyPal.WantsTls13(protocols)) + { + // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites + throw new SslException( + SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); + } + + protocols = SslProtocols.Tls13; + } + + return protocols; + } // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting - internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions) + internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols) { SafeX509Handle? certHandle = credential.CertHandle; SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; - SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; // Always use SSLv23_method, regardless of protocols. It supports negotiating to the highest // mutually supported version and can thus handle any of the set protocols, and we then use @@ -93,44 +138,6 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c throw CreateSslException(SR.net_allocate_ssl_context_failed); } - if (!Interop.Ssl.Tls13Supported) - { - if (protocols != SslProtocols.None && - CipherSuitesPolicyPal.WantsTls13(protocols)) - { - protocols = protocols & (~SslProtocols.Tls13); - } - } - else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (protocols == SslProtocols.None) - { - // we are using default settings but cipher suites policy says that TLS 1.3 - // is not compatible with our settings (i.e. we requested no encryption or disabled - // all TLS 1.3 cipher suites) - protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; - } - else - { - // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - } - - if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (!CipherSuitesPolicyPal.WantsTls13(protocols)) - { - // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - - protocols = SslProtocols.Tls13; - } - Ssl.SslCtxSetProtocolOptions(sslCtx, protocols); if (sslAuthenticationOptions.EncryptionPolicy != EncryptionPolicy.RequireEncryption) @@ -214,7 +221,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslHandle? sslHandle = null; SafeSslContextHandle? sslCtxHandle = null; SafeSslContextHandle? newCtxHandle = null; - SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols; + SslProtocols protocols = CalculateEffectiveProtocols(sslAuthenticationOptions); bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && sslAuthenticationOptions.CertificateContext != null && sslAuthenticationOptions.CertificateContext.SslContexts != null && @@ -222,55 +229,17 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia (!sslAuthenticationOptions.IsServer || (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)); - if (!Interop.Ssl.Tls13Supported) - { - if (protocols != SslProtocols.None && - CipherSuitesPolicyPal.WantsTls13(protocols)) - { - protocols &= ~SslProtocols.Tls13; - } - } - else if (CipherSuitesPolicyPal.WantsTls13(protocols) && - CipherSuitesPolicyPal.ShouldOptOutOfTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (protocols == SslProtocols.None) - { - // we are using default settings but cipher suites policy says that TLS 1.3 - // is not compatible with our settings (i.e. we requested no encryption or disabled - // all TLS 1.3 cipher suites) - protocols = SslProtocols.Tls | SslProtocols.Tls11 | SslProtocols.Tls12; - } - else - { - // user explicitly asks for TLS 1.3 but their policy is not compatible with TLS 1.3 - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - } - - if (CipherSuitesPolicyPal.ShouldOptOutOfLowerThanTls13(sslAuthenticationOptions.CipherSuitesPolicy, sslAuthenticationOptions.EncryptionPolicy)) - { - if (!CipherSuitesPolicyPal.WantsTls13(protocols)) - { - // We cannot provide neither TLS 1.3 or non TLS 1.3, user disabled all cipher suites - throw new SslException( - SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } - - protocols = SslProtocols.Tls13; - } - if (cacheSslContext) { - sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(sslAuthenticationOptions.EnabledSslProtocols, out sslCtxHandle); + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols, out sslCtxHandle); } if (sslCtxHandle == null) { // We did not get SslContext from cache - sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions); + sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions, protocols); - if (cacheSslContext && sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(sslAuthenticationOptions.EnabledSslProtocols, newCtxHandle)) + if (cacheSslContext && sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(protocols, newCtxHandle)) { newCtxHandle = null; } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs index e949096f505f86..a73de253541283 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs @@ -137,7 +137,6 @@ internal static partial class Ssl [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SetCiphers")] internal static extern unsafe bool SslSetCiphers(SafeSslHandle ssl, byte* cipherList, byte* cipherSuites); - [DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetVerifyPeer")] internal static extern void SslSetVerifyPeer(SafeSslHandle ssl);