From e52b12fa02e9711d713cb552f65d1ff79f46152a Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 26 Jan 2022 19:57:35 -0800 Subject: [PATCH 01/10] TLS resume on client --- .../Interop.OpenSsl.cs | 133 +++++++++++++++--- .../Interop.Ssl.cs | 18 +++ .../Interop.SslCtx.cs | 116 ++++++++++++++- .../Net/Security/SslAuthenticationOptions.cs | 1 + .../entrypoints.c | 7 + .../opensslshim.h | 20 +++ .../pal_ssl.c | 47 ++++++- .../pal_ssl.h | 47 ++++++- 8 files changed, 362 insertions(+), 27 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 d403daac342ad1..57a018febc7327 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 @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; @@ -21,6 +22,7 @@ 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(); + private static readonly ConcurrentDictionary s_clientSslContexts = new ConcurrentDictionary(); #region internal methods internal static SafeChannelBindingHandle? QueryChannelBinding(SafeSslHandle context, ChannelBindingKind bindingType) @@ -124,7 +126,7 @@ private static SslProtocols CalculateEffectiveProtocols(SslAuthenticationOptions } // This essentially wraps SSL_CTX* aka SSL_CTX_new + setting - internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols, bool enableResume) + internal static unsafe SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials credential, SslAuthenticationOptions sslAuthenticationOptions, SslProtocols protocols, bool enableResume) { SafeX509Handle? certHandle = credential.CertHandle; SafeEvpPKeyHandle? certKeyHandle = credential.CertKeyHandle; @@ -161,16 +163,13 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c Debug.Assert(cipherSuites == null || (cipherSuites.Length >= 1 && cipherSuites[cipherSuites.Length - 1] == 0)); - unsafe + fixed (byte* cipherListStr = cipherList) + fixed (byte* cipherSuitesStr = cipherSuites) { - fixed (byte* cipherListStr = cipherList) - fixed (byte* cipherSuitesStr = cipherSuites) + if (!Ssl.SslCtxSetCiphers(sslCtx, cipherListStr, cipherSuitesStr)) { - if (!Ssl.SslCtxSetCiphers(sslCtx, cipherListStr, cipherSuitesStr)) - { - Crypto.ErrClearError(); - throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); - } + Crypto.ErrClearError(); + throw new PlatformNotSupportedException(SR.Format(SR.net_ssl_encryptionpolicy_notsupported, sslAuthenticationOptions.EncryptionPolicy)); } } @@ -183,14 +182,26 @@ internal static SafeSslContextHandle AllocateSslContext(SafeFreeSslCredentials c // https://www.openssl.org/docs/manmaster/ssl/SSL_shutdown.html Ssl.SslCtxSetQuietShutdown(sslCtx); - Ssl.SslCtxSetCaching(sslCtx, enableResume ? 1 : 0); - - if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) + if (enableResume) { - unsafe + if (sslAuthenticationOptions.IsServer) { - Interop.Ssl.SslCtxSetAlpnSelectCb(sslCtx, &AlpnServerSelectCallback, IntPtr.Zero); + Ssl.SslCtxSetCaching(sslCtx, 1, null, null); } + else + { + Ssl.SslCtxSetCaching(sslCtx, 1, &NewSessionCallback, &RemoveSessionCallback); + sslCtx.EnableSessionCache(); + } + } + else + { + Ssl.SslCtxSetCaching(sslCtx, 0, null, null); + } + + if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) + { + Interop.Ssl.SslCtxSetAlpnSelectCb(sslCtx, &AlpnServerSelectCallback, IntPtr.Zero); } bool hasCertificateAndKey = @@ -266,15 +277,28 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? newCtxHandle = null; SslProtocols protocols = CalculateEffectiveProtocols(sslAuthenticationOptions); bool hasAlpn = sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0; - bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && - sslAuthenticationOptions.IsServer && - sslAuthenticationOptions.CertificateContext != null && - sslAuthenticationOptions.CertificateContext.SslContexts != null && - sslAuthenticationOptions.CipherSuitesPolicy == null; + bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && sslAuthenticationOptions.CipherSuitesPolicy == null && + ((sslAuthenticationOptions.IsServer && + sslAuthenticationOptions.CertificateContext != null && + sslAuthenticationOptions.CertificateContext.SslContexts != null) || + (sslAuthenticationOptions.IsClient && + // since SNI us our key, we don't wont to resume sessions without it. + !string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && + // on client avoid resume with certificates + sslAuthenticationOptions.CertificateContext == null && + sslAuthenticationOptions.CertSelectionDelegate == null)); if (cacheSslContext) { - sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (SslProtocols)(hasAlpn ? 1 : 0), out sslCtxHandle); + if (sslAuthenticationOptions.IsServer) + { + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (SslProtocols)(hasAlpn ? 1 : 0), out sslCtxHandle); + } + else + { + + s_clientSslContexts.TryGetValue(protocols, out sslCtxHandle); + } } if (sslCtxHandle == null) @@ -282,9 +306,15 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia // We did not get SslContext from cache sslCtxHandle = newCtxHandle = AllocateSslContext(credential, sslAuthenticationOptions, protocols, cacheSslContext); - if (cacheSslContext && sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(protocols | (SslProtocols)(hasAlpn ? 1 : 0), newCtxHandle)) + if (cacheSslContext) { - newCtxHandle = null; + bool added = sslAuthenticationOptions.IsServer ? + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(protocols | (SslProtocols)(hasAlpn ? 1 : 0), newCtxHandle) : + s_clientSslContexts.TryAdd(protocols, newCtxHandle); + if (added) + { + newCtxHandle = null; + } } } @@ -316,7 +346,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia } } - if (!sslAuthenticationOptions.IsServer) + if (sslAuthenticationOptions.IsClient) { // The IdnMapping converts unicode input into the IDNA punycode sequence. string punyCode = string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) ? string.Empty : s_idnMapping.GetAscii(sslAuthenticationOptions.TargetHost!); @@ -327,6 +357,11 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia Crypto.ErrClearError(); } + if (cacheSslContext && !string.IsNullOrEmpty(punyCode)) + { + sslCtxHandle.TrySetSession(sslHandle, punyCode); + } + if (sslAuthenticationOptions.CertSelectionDelegate != null && sslAuthenticationOptions.CertificateContext == null) { // We don't have certificate but we have callback. We should wait for remote certificate and @@ -627,6 +662,58 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte return Ssl.SSL_TLSEXT_ERR_ALERT_FATAL; } + [UnmanagedCallersOnly] + // Invoked from OpenSSL when new session is created. + // We attached GCHandle to the SSL so we can find back SafeSslContextHandle holding the cache. + // New session ahs refCount of 1. + // If this function return 0, OpenSSL will drop the refCount and discard the session. + // If we return 1, the ownership is transfered to us and we will need to call SessionFree(). + private static unsafe int NewSessionCallback(IntPtr ssl, IntPtr session) + { + IntPtr ptr = Ssl.SslGetData(ssl); + Debug.Assert(ptr != IntPtr.Zero); + GCHandle gch = (GCHandle)ptr; + + SafeSslContextHandle? ctxHandle = gch.Target as SafeSslContextHandle; + Debug.Assert(ctxHandle != null); + + if (ctxHandle != null && ctxHandle.TryAddSession(Ssl.SslGetServerName(ssl), session)) + { + // offered session was stored in our cache. + return 1; + } + + // OpenSSL will destroy session. + return 0; + } + + [UnmanagedCallersOnly] + private static unsafe void RemoveSessionCallback(IntPtr ctx, IntPtr session) + { + Debug.Assert(ctx != IntPtr.Zero && session != IntPtr.Zero); + + IntPtr ptr = Ssl.SslCtxGetData(ctx); + Debug.Assert(ptr != IntPtr.Zero); + GCHandle gch = (GCHandle)ptr; + if (!gch.IsAllocated) + { + return; + } + + SafeSslContextHandle? ctxHandle = gch.Target as SafeSslContextHandle; + Debug.Assert(ctxHandle != null); + if (ctxHandle == null) + { + return; + } + + string? name = Marshal.PtrToStringAnsi(Ssl.SessionGetHostname(session)); + if (!string.IsNullOrEmpty(name)) + { + ctxHandle.Remove(name, session); + } + } + private static int BioRead(SafeBioHandle bio, byte[] buffer, int count) { Debug.Assert(buffer != 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 c01b829cd189d2..a7122b2315b98c 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 @@ -56,6 +56,12 @@ internal static partial class Ssl [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool SslSetTlsExtHostName(SafeSslHandle ssl, string host); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetServerName")] + internal static unsafe partial IntPtr SslGetServerName(IntPtr ssl); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetSession")] + internal static unsafe partial int SslSetSession(SafeSslHandle ssl, IntPtr session); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGet0AlpnSelected")] internal static partial void SslGetAlpnSelected(SafeSslHandle ssl, out IntPtr protocol, out int len); @@ -162,6 +168,18 @@ internal static partial class Ssl [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Tls13Supported")] private static partial int Tls13SupportedImpl(); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionGetHostname")] + internal static partial IntPtr SessionGetHostname(IntPtr session); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionFree")] + internal static partial void SessionFree(IntPtr session); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetHostname", CharSet = CharSet.Ansi)] + internal static partial int SessionSetHostname(IntPtr session, string name); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetHostname")] + internal static partial int SessionSetHostname(IntPtr session, IntPtr name); + internal static class Capabilities { // needs separate type (separate static cctor) to be sure OpenSSL is initialized. 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 7401389a394f9e..a87099f1e8f470 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 @@ -2,7 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.Net.Security; using System.Runtime.InteropServices; using System.Security.Cryptography.X509Certificates; @@ -19,11 +21,20 @@ internal static partial class Ssl [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxDestroy")] internal static partial void SslCtxDestroy(IntPtr ctx); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxGetData")] + internal static partial IntPtr SslCtxGetData(IntPtr ctx); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetData")] + internal static partial int SslCtxSetData(SafeSslContextHandle ctx, IntPtr data); + + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetData")] + internal static partial int SslCtxSetData(IntPtr ctx, IntPtr data); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetAlpnSelectCb")] internal static unsafe partial void SslCtxSetAlpnSelectCb(SafeSslContextHandle ctx, delegate* unmanaged callback, IntPtr arg); [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCaching")] - internal static unsafe partial void SslCtxSetCaching(SafeSslContextHandle ctx, int mode); + internal static unsafe partial void SslCtxSetCaching(SafeSslContextHandle ctx, int mode, delegate* unmanaged neewSessionCallback, delegate* unmanaged removeSessionCallback); internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, X509Certificate2[] chain) { @@ -50,6 +61,9 @@ namespace Microsoft.Win32.SafeHandles { internal sealed class SafeSslContextHandle : SafeHandle { + private ConcurrentDictionary? _sslSessions; + private GCHandle _gch; + public SafeSslContextHandle() : base(IntPtr.Zero, true) { @@ -69,7 +83,107 @@ protected override bool ReleaseHandle() { Interop.Ssl.SslCtxDestroy(handle); SetHandle(IntPtr.Zero); + if (_gch.IsAllocated) + { + //Interop.Ssl.SslCtxSetData(this, (IntPtr)_gch); + _gch.Free(); + } + + if (_sslSessions != null) + { + lock (_sslSessions) + { + foreach (string name in _sslSessions.Keys) + { + _sslSessions.Remove(name, out IntPtr session); + Interop.Ssl.SessionFree(session); + } + } + } + return true; } + + public void EnableSessionCache() + { + _sslSessions = new ConcurrentDictionary(); + _gch = GCHandle.Alloc(this); + // This is needed so we can find the handle from session remove callback. + Interop.Ssl.SslCtxSetData(this, (IntPtr)_gch); + } + + public bool TryAddSession(IntPtr serverName, IntPtr session) + { + Debug.Assert(_sslSessions != null && session != IntPtr.Zero); + + if (_sslSessions == null || serverName == IntPtr.Zero) + { + return false; + } + + string? name = Marshal.PtrToStringAnsi(serverName); + if (!string.IsNullOrEmpty(name)) + { + Interop.Ssl.SessionSetHostname(session, serverName); + + lock (_sslSessions) + { + IntPtr oldSession = _sslSessions.GetOrAdd(name, session); + if (oldSession != session) + { + _sslSessions.Remove(name, out oldSession); + Interop.Ssl.SessionFree(oldSession); + oldSession = _sslSessions.GetOrAdd(name, session); + Debug.Assert(oldSession == session); + } + } + + return true; + } + + return false; + } + + public void Remove(string name, IntPtr session) + { + if (_sslSessions != null) + { + lock (_sslSessions) + { + if (!_sslSessions.Remove(name, out IntPtr oldSession)) + { + Interop.Ssl.SessionFree(oldSession); + } + } + } + } + + public bool TrySetSession(SafeSslHandle sslHandle, string name) + { + Debug.Assert(_sslSessions != null); + + if (_sslSessions == null || string.IsNullOrEmpty(name)) + { + return false; + } + + // even if we don't have matching session, we can get new one and we need + // way how to link SSL back to `this`. + Interop.Ssl.SslSetData(sslHandle, (IntPtr)_gch); + + lock (_sslSessions) + { + if (_sslSessions.TryGetValue(name, out IntPtr session)) + { + // This will increase reference count on the session as needed. + // We need to hold lock here to prevent session being deleted before the call is done. + Interop.Ssl.SslSetSession(sslHandle, session); + + return true; + } + } + + return false; + } } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs index 4e89f4b9da0571..896da65eb110e0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslAuthenticationOptions.cs @@ -142,6 +142,7 @@ private static SslProtocols FilterOutIncompatibleSslProtocols(SslProtocols proto internal X509CertificateCollection? ClientCertificates { get; set; } internal List? ApplicationProtocols { get; set; } internal bool IsServer { get; set; } + internal bool IsClient => !IsServer; internal SslStreamCertificateContext? CertificateContext { get; set; } internal SslProtocols EnabledSslProtocols { get; set; } internal X509RevocationMode CertificateRevocationCheckMode { get; set; } diff --git a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c index bcca0693f63fbc..de42fb08b60f96 100644 --- a/src/native/libs/System.Security.Cryptography.Native/entrypoints.c +++ b/src/native/libs/System.Security.Cryptography.Native/entrypoints.c @@ -293,7 +293,9 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslCtxCheckPrivateKey) DllImportEntry(CryptoNative_SslCtxCreate) DllImportEntry(CryptoNative_SslCtxDestroy) + DllImportEntry(CryptoNative_SslCtxGetData) DllImportEntry(CryptoNative_SslCtxSetAlpnSelectCb) + DllImportEntry(CryptoNative_SslCtxSetData) DllImportEntry(CryptoNative_SslCtxSetProtocolOptions) DllImportEntry(CryptoNative_SslCtxSetQuietShutdown) DllImportEntry(CryptoNative_SslCtxUseCertificate) @@ -309,8 +311,12 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslGetPeerCertChain) DllImportEntry(CryptoNative_SslGetPeerCertificate) DllImportEntry(CryptoNative_SslGetPeerFinished) + DllImportEntry(CryptoNative_SslGetServerName) DllImportEntry(CryptoNative_SslGetVersion) DllImportEntry(CryptoNative_SslRead) + DllImportEntry(CryptoNative_SslSessionFree) + DllImportEntry(CryptoNative_SslSessionGetHostname) + DllImportEntry(CryptoNative_SslSessionSetHostname) DllImportEntry(CryptoNative_SslSessionReused) DllImportEntry(CryptoNative_SslSetAcceptState) DllImportEntry(CryptoNative_SslSetAlpnProtos) @@ -319,6 +325,7 @@ static const Entry s_cryptoNative[] = DllImportEntry(CryptoNative_SslSetConnectState) DllImportEntry(CryptoNative_SslSetData) DllImportEntry(CryptoNative_SslSetQuietShutdown) + DllImportEntry(CryptoNative_SslSetSession) DllImportEntry(CryptoNative_SslSetTlsExtHostName) DllImportEntry(CryptoNative_SslSetVerifyPeer) DllImportEntry(CryptoNative_SslShutdown) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 0b38944d502e85..e9a559d3119844 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -264,6 +264,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(EC_POINT_mul) \ REQUIRED_FUNCTION(EC_POINT_new) \ REQUIRED_FUNCTION(EC_POINT_set_affine_coordinates_GFp) \ + REQUIRED_FUNCTION(ERR_print_errors_fp) \ REQUIRED_FUNCTION(ERR_clear_error) \ REQUIRED_FUNCTION(ERR_error_string_n) \ REQUIRED_FUNCTION(ERR_get_error) \ @@ -466,13 +467,17 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); FALLBACK_FUNCTION(SSL_CTX_config) \ REQUIRED_FUNCTION(SSL_CTX_ctrl) \ REQUIRED_FUNCTION(SSL_CTX_free) \ + REQUIRED_FUNCTION(SSL_CTX_get_ex_data) \ FALLBACK_FUNCTION(SSL_is_init_finished) \ REQUIRED_FUNCTION(SSL_CTX_new) \ + REQUIRED_FUNCTION(SSL_CTX_sess_set_new_cb) \ + REQUIRED_FUNCTION(SSL_CTX_sess_set_remove_cb) \ LIGHTUP_FUNCTION(SSL_CTX_set_alpn_protos) \ LIGHTUP_FUNCTION(SSL_CTX_set_alpn_select_cb) \ REQUIRED_FUNCTION(SSL_CTX_set_cipher_list) \ LIGHTUP_FUNCTION(SSL_CTX_set_ciphersuites) \ REQUIRED_FUNCTION(SSL_CTX_set_client_cert_cb) \ + REQUIRED_FUNCTION(SSL_CTX_set_ex_data) \ REQUIRED_FUNCTION(SSL_CTX_set_quiet_shutdown) \ FALLBACK_FUNCTION(SSL_CTX_set_options) \ FALLBACK_FUNCTION(SSL_CTX_set_security_level) \ @@ -489,6 +494,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_get_finished) \ REQUIRED_FUNCTION(SSL_get_peer_cert_chain) \ REQUIRED_FUNCTION(SSL_get_peer_finished) \ + REQUIRED_FUNCTION(SSL_get_servername) \ REQUIRED_FUNCTION(SSL_get_SSL_CTX) \ REQUIRED_FUNCTION(SSL_get_version) \ LIGHTUP_FUNCTION(SSL_get0_alpn_selected) \ @@ -500,6 +506,9 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_read) \ REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ + REQUIRED_FUNCTION(SSL_SESSION_free) \ + REQUIRED_FUNCTION(SSL_SESSION_get0_hostname) \ + REQUIRED_FUNCTION(SSL_SESSION_set1_hostname) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ @@ -509,6 +518,7 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_set_connect_state) \ REQUIRED_FUNCTION(SSL_set_ex_data) \ FALLBACK_FUNCTION(SSL_set_options) \ + REQUIRED_FUNCTION(SSL_set_session) \ REQUIRED_FUNCTION(SSL_set_verify) \ REQUIRED_FUNCTION(SSL_shutdown) \ LEGACY_FUNCTION(SSL_state) \ @@ -722,6 +732,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #define EC_POINT_mul EC_POINT_mul_ptr #define EC_POINT_new EC_POINT_new_ptr #define EC_POINT_set_affine_coordinates_GFp EC_POINT_set_affine_coordinates_GFp_ptr +#define ERR_print_errors_fp ERR_print_errors_fp_ptr #define ERR_clear_error ERR_clear_error_ptr #define ERR_error_string_n ERR_error_string_n_ptr #define ERR_get_error ERR_get_error_ptr @@ -926,12 +937,16 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_CTX_config SSL_CTX_config_ptr #define SSL_CTX_ctrl SSL_CTX_ctrl_ptr #define SSL_CTX_free SSL_CTX_free_ptr +#define SSL_CTX_get_ex_data SSL_CTX_get_ex_data_ptr #define SSL_CTX_new SSL_CTX_new_ptr +#define SSL_CTX_sess_set_new_cb SSL_CTX_sess_set_new_cb_ptr +#define SSL_CTX_sess_set_remove_cb SSL_CTX_sess_set_remove_cb_ptr #define SSL_CTX_set_alpn_protos SSL_CTX_set_alpn_protos_ptr #define SSL_CTX_set_alpn_select_cb SSL_CTX_set_alpn_select_cb_ptr #define SSL_CTX_set_cipher_list SSL_CTX_set_cipher_list_ptr #define SSL_CTX_set_ciphersuites SSL_CTX_set_ciphersuites_ptr #define SSL_CTX_set_client_cert_cb SSL_CTX_set_client_cert_cb_ptr +#define SSL_CTX_set_ex_data SSL_CTX_set_ex_data_ptr #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 @@ -948,6 +963,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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 +#define SSL_get_servername SSL_get_servername_ptr #define SSL_get_SSL_CTX SSL_get_SSL_CTX_ptr #define SSL_get_version SSL_get_version_ptr #define SSL_get0_alpn_selected SSL_get0_alpn_selected_ptr @@ -961,6 +977,9 @@ FOR_ALL_OPENSSL_FUNCTIONS #define SSL_read SSL_read_ptr #define SSL_renegotiate SSL_renegotiate_ptr #define SSL_renegotiate_pending SSL_renegotiate_pending_ptr +#define SSL_SESSION_free SSL_SESSION_free_ptr +#define SSL_SESSION_get0_hostname SSL_SESSION_get0_hostname_ptr +#define SSL_SESSION_set1_hostname SSL_SESSION_set1_hostname_ptr #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 @@ -970,6 +989,7 @@ FOR_ALL_OPENSSL_FUNCTIONS #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_session SSL_set_session_ptr #define SSL_set_verify SSL_set_verify_ptr #define SSL_shutdown SSL_shutdown_ptr #define SSL_state SSL_state_ptr diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index b49ca41b1895d2..d1fcaeab76e2d5 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -478,7 +478,7 @@ void CryptoNative_SslSetVerifyPeer(SSL* ssl) SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } -void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode) +void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) { // We never reuse same CTX for both client and server SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_MODE, mode ? SSL_SESS_CACHE_BOTH : SSL_SESS_CACHE_OFF, NULL); @@ -486,6 +486,41 @@ void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode) { SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); } + + if (newSessionCb) + { + SSL_CTX_sess_set_new_cb(ctx, newSessionCb); + } + + if (removeSessionCb) + { + SSL_CTX_sess_set_remove_cb(ctx, removeSessionCb); + } +} + +const char* CryptoNative_SslGetServerName(SSL * ssl) +{ + return SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); +} + +int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION *session) +{ + return SSL_set_session(ssl, session); +} + +void CryptoNative_SslSessionFree(SSL_SESSION *session) +{ + SSL_SESSION_free(session); +} + +const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session) +{ + return SSL_SESSION_get0_hostname(session); +} + +int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname) +{ + return SSL_SESSION_set1_hostname(session, hostname); } int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) @@ -696,6 +731,16 @@ void* CryptoNative_SslGetData(SSL* ssl) return SSL_get_ex_data(ssl, 0); } +int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void *ptr) +{ + return SSL_CTX_set_ex_data(ctx, 0, ptr); +} + +void* CryptoNative_SslCtxGetData(SSL_CTX* ctx) +{ + return SSL_CTX_get_ex_data(ctx, 0); +} + int32_t CryptoNative_SslSetAlpnProtos(SSL* ssl, const uint8_t* protos, uint32_t protos_len) { #if HAVE_OPENSSL_ALPN diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index faf96988b1b507..d6f3e3df4aad1b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -117,6 +117,13 @@ typedef int32_t (*SslCtxSetAlpnCallback)(SSL* ssl, const uint8_t* in, uint32_t inlen, void* arg); + +// the function pointer used for new session +typedef int32_t (*SslCtxNewSessionCallback)(SSL* ssl, SSL_SESSION* sesssion); + +// the function pointer used for new session +typedef void (*SslCtxRemoveSessionCallback)(SSL_CTX* ctx, SSL_SESSION* sesssion); + /* Ensures that libssl is correctly initialized and ready to use. */ @@ -150,7 +157,33 @@ PALEXPORT void CryptoNative_SslSetClientCertCallback(SSL* ssl, int set); /*======= Sets session caching. 0 is disabled. */ -PALEXPORT void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode); +PALEXPORT void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newCb, SslCtxRemoveSessionCallback removeCb); + +/* +Returns name associated with given ssl session. +OpenSSL holds reference to it and it must not be freed. +*/ +PALEXPORT const char* CryptoNative_SslGetServerName(SSL * ssl); + +/* +This function will attach existing ssl session for possible TLS resume. +*/ +PALEXPORT int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION *session); + +/* + * Frees SSL session. + */ +PALEXPORT void CryptoNative_SslSessionFree(SSL_SESSION *session); + +/* + * Get name associated with given SSL_SESSION. + */ +PALEXPORT const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session); + +/* + * Associate name with given SSL_SESSION. + */ +PALEXPORT int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname); /* Shims the SSL_new method. @@ -327,7 +360,7 @@ PALEXPORT void CryptoNative_SslCtxSetQuietShutdown(SSL_CTX* ctx); /* Shims the SSL_set_quiet_shutdown method. */ -PALEXPORT void CryptoNative_SslSetQuietShutdown(SSL* ctx, int mode); +PALEXPORT void CryptoNative_SslSetQuietShutdown(SSL* ssl, int mode); /* Shims the SSL_get_client_CA_list method. @@ -351,6 +384,16 @@ Shims SSL_get_ex_data to retrieve application context. */ PALEXPORT void* CryptoNative_SslGetData(SSL* ssl); +/* +Shims SSL_CTX_set_ex_data to attach application context. +*/ +PALEXPORT int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void *ptr); + +/* +Shims SSL_CTX_get_ex_data to retrieve application context. +*/ +PALEXPORT void* CryptoNative_SslCtxGetData(SSL_CTX* ctx); + /* Sets the specified encryption policy on the SSL_CTX. From f9a2eabef4df5bd3d7fa797c4e3ee4e97625294c Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 1 Feb 2022 14:31:44 -0800 Subject: [PATCH 02/10] shim functions introduced in 1.1.1 --- .../apibridge.c | 21 +++++++++++++++++++ .../apibridge.h | 2 ++ .../opensslshim.h | 10 +++++++-- .../osslcompat_111.h | 2 ++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.c b/src/native/libs/System.Security.Cryptography.Native/apibridge.c index 0a3705321eec4c..debb2d350f8066 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.c +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.c @@ -890,4 +890,25 @@ int local_EVP_PKEY_public_check(EVP_PKEY_CTX* ctx) } } +const char * local_SSL_SESSION_get0_hostname(const SSL_SESSION *s) +{ + return s->tlsext_hostname; +} + +int local_SSL_SESSION_set1_hostname(SSL_SESSION *s, const char *hostname) +{ + if (s->tlsext_hostname != NULL) + { + OPENSSL_free(s->tlsext_hostname); + } + + if (hostname == NULL) { + s->tlsext_hostname = NULL; + return 1; + } + + s->tlsext_hostname = OPENSSL_strdup(hostname); + return s->tlsext_hostname != NULL; +} + #endif diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.h b/src/native/libs/System.Security.Cryptography.Native/apibridge.h index 2079d81fd96816..8549dcf8448e0b 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.h +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.h @@ -57,3 +57,5 @@ const X509_ALGOR* local_X509_get0_tbs_sigalg(const X509* x509); X509_PUBKEY* local_X509_get_X509_PUBKEY(const X509* x509); int32_t local_X509_get_version(const X509* x509); int32_t local_X509_up_ref(X509* x509); +const char * local_SSL_SESSION_get0_hostname(const SSL_SESSION *s); +int local_SSL_SESSION_set1_hostname(SSL_SESSION *s, const char *hostname); diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index af059eb1ca1933..5e9a46dbc401b5 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -195,6 +195,8 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(BN_num_bits) \ REQUIRED_FUNCTION(BN_set_word) \ LEGACY_FUNCTION(CRYPTO_add_lock) \ + LEGACY_FUNCTION(CRYPTO_free) \ + LEGACY_FUNCTION(CRYPTO_strdup) \ LEGACY_FUNCTION(CRYPTO_num_locks) \ LEGACY_FUNCTION(CRYPTO_set_locking_callback) \ REQUIRED_FUNCTION(d2i_ASN1_BIT_STRING) \ @@ -508,8 +510,8 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ REQUIRED_FUNCTION(SSL_SESSION_free) \ - REQUIRED_FUNCTION(SSL_SESSION_get0_hostname) \ - REQUIRED_FUNCTION(SSL_SESSION_set1_hostname) \ + FALLBACK_FUNCTION(SSL_SESSION_get0_hostname) \ + FALLBACK_FUNCTION(SSL_SESSION_set1_hostname) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ @@ -664,6 +666,8 @@ FOR_ALL_OPENSSL_FUNCTIONS #define BN_num_bits BN_num_bits_ptr #define BN_set_word BN_set_word_ptr #define CRYPTO_add_lock CRYPTO_add_lock_ptr +#define CRYPTO_free CRYPTO_free_ptr +#define CRYPTO_strdup CRYPTO_strdup_ptr #define CRYPTO_num_locks CRYPTO_num_locks_ptr #define CRYPTO_set_locking_callback CRYPTO_set_locking_callback_ptr #define d2i_ASN1_BIT_STRING d2i_ASN1_BIT_STRING_ptr @@ -1187,6 +1191,8 @@ FOR_ALL_OPENSSL_FUNCTIONS #define RSA_test_flags local_RSA_test_flags #define SSL_CTX_set_security_level local_SSL_CTX_set_security_level #define SSL_is_init_finished local_SSL_is_init_finished +#define SSL_SESSION_get0_hostname local_SSL_SESSION_get0_hostname +#define SSL_SESSION_set1_hostname local_SSL_SESSION_set1_hostname #define X509_CRL_get0_nextUpdate local_X509_CRL_get0_nextUpdate #define X509_NAME_get0_der local_X509_NAME_get0_der #define X509_PUBKEY_get0_param local_X509_PUBKEY_get0_param diff --git a/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h b/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h index 59f0fc5f59d5df..96116e69583c35 100644 --- a/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h +++ b/src/native/libs/System.Security.Cryptography.Native/osslcompat_111.h @@ -79,6 +79,8 @@ int32_t X509_get_version(const X509* x509); int X509_set1_notAfter(X509* x509, const ASN1_TIME*); int X509_set1_notBefore(X509* x509, const ASN1_TIME*); int32_t X509_up_ref(X509* x509); +const char *SSL_SESSION_get0_hostname(const SSL_SESSION *s); +int SSL_SESSION_set1_hostname(SSL_SESSION *s, const char *hostname); #if OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_1_0_2_RTM int32_t X509_check_host(X509* x509, const char* name, size_t namelen, unsigned int flags, char** peername); From 9b87b4e15effff5a12201a810439cb4bfb241fb9 Mon Sep 17 00:00:00 2001 From: wfurt Date: Tue, 1 Feb 2022 15:44:11 -0800 Subject: [PATCH 03/10] add missing struct --- .../openssl_1_0_structs.h | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h index f730dd39387ffa..47d1be49dc44b3 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h +++ b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h @@ -169,7 +169,8 @@ struct x509_store_st X509_VERIFY_PARAM* param; }; -struct bio_st { +struct bio_st +{ const void* _ignored1; const void* _ignored2; const void* _ignored3; @@ -183,3 +184,41 @@ struct bio_st { const void*_ignored11; int references; }; + +#ifndef SSL_MAX_KRB5_PRINCIPAL_LENGTH +#define SSL_MAX_KRB5_PRINCIPAL_LENGTH 256 +#endif +struct ssl_session_st +{ + int _ignored0; + unsigned int _ignored2; + unsigned char _ignored3[SSL_MAX_KEY_ARG_LENGTH]; + int _ignored4; + unsigned char _ignored5[SSL_MAX_MASTER_KEY_LENGTH]; + unsigned int _ignored6; + unsigned char _ignored7[SSL_MAX_SSL_SESSION_ID_LENGTH]; + unsigned int _ignored8; + unsigned char _ignored9[SSL_MAX_SID_CTX_LENGTH]; +#ifndef OPENSSL_NO_KRB5 + unsigned int _ignored10; + unsigned char _ignored11[SSL_MAX_KRB5_PRINCIPAL_LENGTH]; +#endif +#ifndef OPENSSL_NO_PSK + char * _ignored12; + char * _ignored13; +#endif + int _ignored14; + void* _ignored15; + void* _ignored16; + long _ignored17; + int _ignored18; + long _ignored19; + long _ignored20; + unsigned int _ignored21; + void * _ignored22; + unsigned long _ignored23; + STACK_OF(SSL_CIPHER) *_ignored24; + CRYPTO_EX_DATA _ignored25; + struct ssl_session_st *prev, *next; + char *tlsext_hostname; +}; From fd5635df4922e3bf8207f6758bdef9e82a9974dc Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 2 Feb 2022 19:47:24 -0800 Subject: [PATCH 04/10] disable resume on old OpenSSL --- .../Interop.OpenSsl.cs | 8 ++++++++ .../System.Net.Security/src/System.Net.Security.csproj | 2 ++ 2 files changed, 10 insertions(+) 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 92876100efccdf..b34993ef33de57 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 @@ -60,6 +60,14 @@ private static bool DisableTlsResume return disableTlsResume != 0; } + // Resume does not work properly on older OpenSSL versions. + // This may be revisited but for now enable it only on 1.1.1 and above. + if (Interop.OpenSsl.OpenSslVersionNumber() < 0x10101000) + { + s_disableTlsResume = 1; + return true; + } + // First check for the AppContext switch, giving it priority over the environment variable. if (AppContext.TryGetSwitch(DisableTlsResumeCtxSwitch, out bool value)) { 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 047cbdcc98aea9..e903dcccac176f 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -314,6 +314,8 @@ Link="Common\Interop\Unix\System.Security.Cryptography.Native\Interop.Crypto.cs" /> + Date: Thu, 3 Feb 2022 16:04:52 -0800 Subject: [PATCH 05/10] feedback from review --- .../Interop.OpenSsl.cs | 75 +++++++++------ .../Interop.Ssl.cs | 3 + .../Interop.SslCtx.cs | 95 ++++++++++++------- .../apibridge.c | 22 ----- .../apibridge.h | 2 - .../openssl_1_0_structs.h | 38 -------- .../opensslshim.h | 12 +-- .../pal_ssl.c | 33 ++++++- .../pal_ssl.h | 4 +- 9 files changed, 146 insertions(+), 138 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 b34993ef33de57..0a7d4b1e62a409 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 @@ -21,6 +21,7 @@ internal static partial class OpenSsl { private const string DisableTlsResumeCtxSwitch = "System.Net.Security.DisableTlsResume"; private const string DisableTlsResumeEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_DISABLETLSRESUME"; + private const SslProtocols FakeAlpnSslProtocol = (SslProtocols)1; // used to distinguish server sessions with ALPN private static readonly IdnMapping s_idnMapping = new IdnMapping(); private static readonly ConcurrentDictionary s_clientSslContexts = new ConcurrentDictionary(); @@ -89,7 +90,7 @@ private static bool DisableTlsResume private static SslProtocols CalculateEffectiveProtocols(SslAuthenticationOptions sslAuthenticationOptions) { // make sure low bit is not set since we use it in context dictionary to distinguish use with ALPN - Debug.Assert(((int)sslAuthenticationOptions.EnabledSslProtocols & 1) == 0); + Debug.Assert((sslAuthenticationOptions.EnabledSslProtocols & FakeAlpnSslProtocol) == 0); SslProtocols protocols = sslAuthenticationOptions.EnabledSslProtocols & ~((SslProtocols)1); if (!Interop.Ssl.Capabilities.Tls13Supported) @@ -198,7 +199,8 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SafeFreeSslCreden } else { - Ssl.SslCtxSetCaching(sslCtx, 1, &NewSessionCallback, &RemoveSessionCallback); + int result = Ssl.SslCtxSetCaching(sslCtx, 1, &NewSessionCallback, &RemoveSessionCallback); + Debug.Assert(result == 1); sslCtx.EnableSessionCache(); } } @@ -285,22 +287,39 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia SafeSslContextHandle? newCtxHandle = null; SslProtocols protocols = CalculateEffectiveProtocols(sslAuthenticationOptions); bool hasAlpn = sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0; - bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && sslAuthenticationOptions.CipherSuitesPolicy == null && - ((sslAuthenticationOptions.IsServer && - sslAuthenticationOptions.CertificateContext != null && - sslAuthenticationOptions.CertificateContext.SslContexts != null) || - (sslAuthenticationOptions.IsClient && - // since SNI us our key, we don't wont to resume sessions without it. - !string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) && - // on client avoid resume with certificates - sslAuthenticationOptions.CertificateContext == null && - sslAuthenticationOptions.CertSelectionDelegate == null)); + bool cacheSslContext = !DisableTlsResume && sslAuthenticationOptions.EncryptionPolicy == EncryptionPolicy.RequireEncryption && sslAuthenticationOptions.CipherSuitesPolicy == null; + + if (cacheSslContext) + { + if (sslAuthenticationOptions.IsClient) + { + // we don't want to try on emtpy TargetName since that is our key. + // And we don't want to mess up with client authentication. It may be possible + // but it seems safe to get full new session. + if (string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) || + sslAuthenticationOptions.CertificateContext != null || + sslAuthenticationOptions.CertSelectionDelegate != null) + { + cacheSslContext = false; + } + } + else + { + // Server should always have certificate + Debug.Assert(sslAuthenticationOptions.CertificateContext != null); + if (sslAuthenticationOptions.CertificateContext == null || + sslAuthenticationOptions.CertificateContext.SslContexts == null) + { + cacheSslContext = false; + } + } + } if (cacheSslContext) { if (sslAuthenticationOptions.IsServer) { - sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (SslProtocols)(hasAlpn ? 1 : 0), out sslCtxHandle); + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(protocols | (hasAlpn ? FakeAlpnSslProtocol : SslProtocols.None), out sslCtxHandle); } else { @@ -317,8 +336,8 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia if (cacheSslContext) { bool added = sslAuthenticationOptions.IsServer ? - sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(protocols | (SslProtocols)(hasAlpn ? 1 : 0), newCtxHandle) : - s_clientSslContexts.TryAdd(protocols, newCtxHandle); + sslAuthenticationOptions.CertificateContext!.SslContexts!.TryAdd(protocols | (SslProtocols)(hasAlpn ? 1 : 0), newCtxHandle) : + s_clientSslContexts.TryAdd(protocols, newCtxHandle); if (added) { newCtxHandle = null; @@ -341,6 +360,7 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia { if (sslAuthenticationOptions.IsServer) { + Debug.Assert(Interop.Ssl.SslGetData(sslHandle) == IntPtr.Zero); alpnHandle = GCHandle.Alloc(sslAuthenticationOptions.ApplicationProtocols); Interop.Ssl.SslSetData(sslHandle, GCHandle.ToIntPtr(alpnHandle)); sslHandle.AlpnHandle = alpnHandle; @@ -675,13 +695,16 @@ private static unsafe int AlpnServerSelectCallback(IntPtr ssl, byte** outp, byte // If we return 1, the ownership is transfered to us and we will need to call SessionFree(). private static unsafe int NewSessionCallback(IntPtr ssl, IntPtr session) { + Debug.Assert(ssl != IntPtr.Zero); + Debug.Assert(session != IntPtr.Zero); + IntPtr ptr = Ssl.SslGetData(ssl); Debug.Assert(ptr != IntPtr.Zero); - GCHandle gch = (GCHandle)ptr; + GCHandle gch = GCHandle.FromIntPtr(ptr); SafeSslContextHandle? ctxHandle = gch.Target as SafeSslContextHandle; - Debug.Assert(ctxHandle != null); - + // There is no relation between SafeSslContextHandle and SafeSslHandle so the handle + // may be released while the ssl session is still active. if (ctxHandle != null && ctxHandle.TryAddSession(Ssl.SslGetServerName(ssl), session)) { // offered session was stored in our cache. @@ -698,25 +721,23 @@ private static unsafe void RemoveSessionCallback(IntPtr ctx, IntPtr session) Debug.Assert(ctx != IntPtr.Zero && session != IntPtr.Zero); IntPtr ptr = Ssl.SslCtxGetData(ctx); - Debug.Assert(ptr != IntPtr.Zero); - GCHandle gch = (GCHandle)ptr; - if (!gch.IsAllocated) + if (ptr == IntPtr.Zero) { + // Same as above, SafeSslContextHandle could be released while OpenSSL still holds refferecne. return; } + GCHandle gch = GCHandle.FromIntPtr(ptr); SafeSslContextHandle? ctxHandle = gch.Target as SafeSslContextHandle; - Debug.Assert(ctxHandle != null); if (ctxHandle == null) { return; } - string? name = Marshal.PtrToStringAnsi(Ssl.SessionGetHostname(session)); - if (!string.IsNullOrEmpty(name)) - { - ctxHandle.Remove(name, session); - } + //string? name = Marshal.PtrToStringAnsi(Ssl.SessionGetHostname(session));a + IntPtr name = Ssl.SessionGetHostname(session); + Debug.Assert(name != IntPtr.Zero); + ctxHandle.RemoveSession(name, session); } private static int BioRead(SafeBioHandle bio, byte[] buffer, int count) 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 ffe18dd25b348f..6d334dfb551c43 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 @@ -151,6 +151,9 @@ internal static partial class Ssl [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetData")] internal static partial IntPtr SslGetData(IntPtr ssl); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslGetData")] + internal static partial IntPtr SslGetData(SafeSslHandle ssl); + [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSetData")] internal static partial int SslSetData(SafeSslHandle ssl, IntPtr data); 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 a87099f1e8f470..2da1cc62056fe5 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 @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Net.Security; @@ -34,7 +33,7 @@ internal static partial class Ssl internal static unsafe partial void SslCtxSetAlpnSelectCb(SafeSslContextHandle ctx, delegate* unmanaged callback, IntPtr arg); [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCaching")] - internal static unsafe partial void SslCtxSetCaching(SafeSslContextHandle ctx, int mode, delegate* unmanaged neewSessionCallback, delegate* unmanaged removeSessionCallback); + internal static unsafe partial int SslCtxSetCaching(SafeSslContextHandle ctx, int mode, delegate* unmanaged neewSessionCallback, delegate* unmanaged removeSessionCallback); internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, X509Certificate2[] chain) { @@ -61,7 +60,8 @@ namespace Microsoft.Win32.SafeHandles { internal sealed class SafeSslContextHandle : SafeHandle { - private ConcurrentDictionary? _sslSessions; + // This is session cache keyed by SNI e.g. TargetHost + private Dictionary? _sslSessions; private GCHandle _gch; public SafeSslContextHandle() @@ -81,60 +81,74 @@ public override bool IsInvalid protected override bool ReleaseHandle() { - Interop.Ssl.SslCtxDestroy(handle); - SetHandle(IntPtr.Zero); - if (_gch.IsAllocated) - { - //Interop.Ssl.SslCtxSetData(this, (IntPtr)_gch); - _gch.Free(); - } - if (_sslSessions != null) { + // The SSL_CTX is ref counted and may not immediately die when we call SslCtxDestroy() + // Since there is no relation between SafeSslContextHandle and SafeSslHandle `this` can be release + // while we still have SSL session using it. + Interop.Ssl.SslCtxSetData(handle, IntPtr.Zero); + lock (_sslSessions) { - foreach (string name in _sslSessions.Keys) + foreach (IntPtr session in _sslSessions.Values) { - _sslSessions.Remove(name, out IntPtr session); Interop.Ssl.SessionFree(session); } + + _sslSessions.Clear(); } + + Debug.Assert(_gch.IsAllocated); + _gch.Free(); } + Interop.Ssl.SslCtxDestroy(handle); + SetHandle(IntPtr.Zero); + return true; } - public void EnableSessionCache() + internal void EnableSessionCache() { - _sslSessions = new ConcurrentDictionary(); + Debug.Assert(_sslSessions == null); + + _sslSessions = new Dictionary(); _gch = GCHandle.Alloc(this); - // This is needed so we can find the handle from session remove callback. + Debug.Assert(_gch.IsAllocated); + // This is needed so we can find the handle from session in SessionRemove callback. Interop.Ssl.SslCtxSetData(this, (IntPtr)_gch); } - public bool TryAddSession(IntPtr serverName, IntPtr session) + internal bool TryAddSession(IntPtr namePtr, IntPtr session) { Debug.Assert(_sslSessions != null && session != IntPtr.Zero); - if (_sslSessions == null || serverName == IntPtr.Zero) + if (_sslSessions == null || namePtr == IntPtr.Zero) { return false; } - string? name = Marshal.PtrToStringAnsi(serverName); - if (!string.IsNullOrEmpty(name)) + string? targetName = Marshal.PtrToStringAnsi(namePtr); + Debug.Assert(targetName != null); + + if (!string.IsNullOrEmpty(targetName)) { - Interop.Ssl.SessionSetHostname(session, serverName); + // We do this only for lookup in RemoveSession. + // Since this is part of chache manipulation and no function impact it is done here. + // This will use strdup() so it is safe to pass in raw pointer. + Interop.Ssl.SessionSetHostname(session, namePtr); lock (_sslSessions) { - IntPtr oldSession = _sslSessions.GetOrAdd(name, session); - if (oldSession != session) + if (!_sslSessions.TryAdd(targetName, session)) { - _sslSessions.Remove(name, out oldSession); - Interop.Ssl.SessionFree(oldSession); - oldSession = _sslSessions.GetOrAdd(name, session); - Debug.Assert(oldSession == session); + if (_sslSessions.Remove(targetName, out IntPtr oldSession)) + { + Interop.Ssl.SessionFree(oldSession); + } + + bool added = _sslSessions.TryAdd(targetName, session); + Debug.Assert(added); } } @@ -144,21 +158,33 @@ public bool TryAddSession(IntPtr serverName, IntPtr session) return false; } - public void Remove(string name, IntPtr session) + internal void RemoveSession(IntPtr namePtr, IntPtr session) { - if (_sslSessions != null) + Debug.Assert(_sslSessions != null); + + string? targetName = Marshal.PtrToStringAnsi(namePtr); + Debug.Assert(targetName != null); + + if (_sslSessions != null && targetName != null) { + IntPtr oldSession; + bool removed; lock (_sslSessions) { - if (!_sslSessions.Remove(name, out IntPtr oldSession)) - { - Interop.Ssl.SessionFree(oldSession); - } + removed = _sslSessions.Remove(targetName, out oldSession); + } + + if (removed) + { + // It seems like we may be called more than once. Since we grabbed only one refference + // when added to Dictionary, we will also drop exactly one when removed. + Interop.Ssl.SessionFree(oldSession); } + } } - public bool TrySetSession(SafeSslHandle sslHandle, string name) + internal bool TrySetSession(SafeSslHandle sslHandle, string name) { Debug.Assert(_sslSessions != null); @@ -169,6 +195,7 @@ public bool TrySetSession(SafeSslHandle sslHandle, string name) // even if we don't have matching session, we can get new one and we need // way how to link SSL back to `this`. + Debug.Assert(Interop.Ssl.SslGetData(sslHandle) == IntPtr.Zero); Interop.Ssl.SslSetData(sslHandle, (IntPtr)_gch); lock (_sslSessions) diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.c b/src/native/libs/System.Security.Cryptography.Native/apibridge.c index debb2d350f8066..73ba3e6cef5d67 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.c +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.c @@ -889,26 +889,4 @@ int local_EVP_PKEY_public_check(EVP_PKEY_CTX* ctx) return -1; } } - -const char * local_SSL_SESSION_get0_hostname(const SSL_SESSION *s) -{ - return s->tlsext_hostname; -} - -int local_SSL_SESSION_set1_hostname(SSL_SESSION *s, const char *hostname) -{ - if (s->tlsext_hostname != NULL) - { - OPENSSL_free(s->tlsext_hostname); - } - - if (hostname == NULL) { - s->tlsext_hostname = NULL; - return 1; - } - - s->tlsext_hostname = OPENSSL_strdup(hostname); - return s->tlsext_hostname != NULL; -} - #endif diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.h b/src/native/libs/System.Security.Cryptography.Native/apibridge.h index 8549dcf8448e0b..2079d81fd96816 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.h +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.h @@ -57,5 +57,3 @@ const X509_ALGOR* local_X509_get0_tbs_sigalg(const X509* x509); X509_PUBKEY* local_X509_get_X509_PUBKEY(const X509* x509); int32_t local_X509_get_version(const X509* x509); int32_t local_X509_up_ref(X509* x509); -const char * local_SSL_SESSION_get0_hostname(const SSL_SESSION *s); -int local_SSL_SESSION_set1_hostname(SSL_SESSION *s, const char *hostname); diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h index 47d1be49dc44b3..83942449bb3af6 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h +++ b/src/native/libs/System.Security.Cryptography.Native/openssl_1_0_structs.h @@ -184,41 +184,3 @@ struct bio_st const void*_ignored11; int references; }; - -#ifndef SSL_MAX_KRB5_PRINCIPAL_LENGTH -#define SSL_MAX_KRB5_PRINCIPAL_LENGTH 256 -#endif -struct ssl_session_st -{ - int _ignored0; - unsigned int _ignored2; - unsigned char _ignored3[SSL_MAX_KEY_ARG_LENGTH]; - int _ignored4; - unsigned char _ignored5[SSL_MAX_MASTER_KEY_LENGTH]; - unsigned int _ignored6; - unsigned char _ignored7[SSL_MAX_SSL_SESSION_ID_LENGTH]; - unsigned int _ignored8; - unsigned char _ignored9[SSL_MAX_SID_CTX_LENGTH]; -#ifndef OPENSSL_NO_KRB5 - unsigned int _ignored10; - unsigned char _ignored11[SSL_MAX_KRB5_PRINCIPAL_LENGTH]; -#endif -#ifndef OPENSSL_NO_PSK - char * _ignored12; - char * _ignored13; -#endif - int _ignored14; - void* _ignored15; - void* _ignored16; - long _ignored17; - int _ignored18; - long _ignored19; - long _ignored20; - unsigned int _ignored21; - void * _ignored22; - unsigned long _ignored23; - STACK_OF(SSL_CIPHER) *_ignored24; - CRYPTO_EX_DATA _ignored25; - struct ssl_session_st *prev, *next; - char *tlsext_hostname; -}; diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 5e9a46dbc401b5..ce65da4a4198e5 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -195,8 +195,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(BN_num_bits) \ REQUIRED_FUNCTION(BN_set_word) \ LEGACY_FUNCTION(CRYPTO_add_lock) \ - LEGACY_FUNCTION(CRYPTO_free) \ - LEGACY_FUNCTION(CRYPTO_strdup) \ LEGACY_FUNCTION(CRYPTO_num_locks) \ LEGACY_FUNCTION(CRYPTO_set_locking_callback) \ REQUIRED_FUNCTION(d2i_ASN1_BIT_STRING) \ @@ -266,7 +264,6 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(EC_POINT_mul) \ REQUIRED_FUNCTION(EC_POINT_new) \ REQUIRED_FUNCTION(EC_POINT_set_affine_coordinates_GFp) \ - REQUIRED_FUNCTION(ERR_print_errors_fp) \ REQUIRED_FUNCTION(ERR_clear_error) \ REQUIRED_FUNCTION(ERR_error_string_n) \ REQUIRED_FUNCTION(ERR_get_error) \ @@ -510,8 +507,8 @@ const EVP_CIPHER* EVP_chacha20_poly1305(void); REQUIRED_FUNCTION(SSL_renegotiate) \ REQUIRED_FUNCTION(SSL_renegotiate_pending) \ REQUIRED_FUNCTION(SSL_SESSION_free) \ - FALLBACK_FUNCTION(SSL_SESSION_get0_hostname) \ - FALLBACK_FUNCTION(SSL_SESSION_set1_hostname) \ + LIGHTUP_FUNCTION(SSL_SESSION_get0_hostname) \ + LIGHTUP_FUNCTION(SSL_SESSION_set1_hostname) \ FALLBACK_FUNCTION(SSL_session_reused) \ REQUIRED_FUNCTION(SSL_set_accept_state) \ REQUIRED_FUNCTION(SSL_set_bio) \ @@ -666,8 +663,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define BN_num_bits BN_num_bits_ptr #define BN_set_word BN_set_word_ptr #define CRYPTO_add_lock CRYPTO_add_lock_ptr -#define CRYPTO_free CRYPTO_free_ptr -#define CRYPTO_strdup CRYPTO_strdup_ptr #define CRYPTO_num_locks CRYPTO_num_locks_ptr #define CRYPTO_set_locking_callback CRYPTO_set_locking_callback_ptr #define d2i_ASN1_BIT_STRING d2i_ASN1_BIT_STRING_ptr @@ -737,7 +732,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define EC_POINT_mul EC_POINT_mul_ptr #define EC_POINT_new EC_POINT_new_ptr #define EC_POINT_set_affine_coordinates_GFp EC_POINT_set_affine_coordinates_GFp_ptr -#define ERR_print_errors_fp ERR_print_errors_fp_ptr #define ERR_clear_error ERR_clear_error_ptr #define ERR_error_string_n ERR_error_string_n_ptr #define ERR_get_error ERR_get_error_ptr @@ -1191,8 +1185,6 @@ FOR_ALL_OPENSSL_FUNCTIONS #define RSA_test_flags local_RSA_test_flags #define SSL_CTX_set_security_level local_SSL_CTX_set_security_level #define SSL_is_init_finished local_SSL_is_init_finished -#define SSL_SESSION_get0_hostname local_SSL_SESSION_get0_hostname -#define SSL_SESSION_set1_hostname local_SSL_SESSION_set1_hostname #define X509_CRL_get0_nextUpdate local_X509_CRL_get0_nextUpdate #define X509_NAME_get0_der local_X509_NAME_get0_der #define X509_PUBKEY_get0_param local_X509_PUBKEY_get0_param diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index d1fcaeab76e2d5..3f024e3c2c9f40 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -478,8 +478,22 @@ void CryptoNative_SslSetVerifyPeer(SSL* ssl) SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } -void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) +int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) { + int retValue = 1; + if (mode && !API_EXISTS(SSL_SESSION_get0_hostname)) + { + // Disable caching on old OpenSSL. + // While TLS resume is optional, none of this is critical. + mode = 0; + + if (newSessionCb != NULL || removeSessionCb != NULL) + { + // Indicate unwillingness to restore sessions + retValue = 0; + } + } + // We never reuse same CTX for both client and server SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_MODE, mode ? SSL_SESS_CACHE_BOTH : SSL_SESS_CACHE_OFF, NULL); if (mode == 0) @@ -487,15 +501,18 @@ void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallb SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); } - if (newSessionCb) + if (newSessionCb != NULL || removeSessionCb != NULL) + if (newSessionCb != NULL) { SSL_CTX_sess_set_new_cb(ctx, newSessionCb); } - if (removeSessionCb) + if (removeSessionCb != NULL) { SSL_CTX_sess_set_remove_cb(ctx, removeSessionCb); } + + return retValue; } const char* CryptoNative_SslGetServerName(SSL * ssl) @@ -515,11 +532,21 @@ void CryptoNative_SslSessionFree(SSL_SESSION *session) const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session) { + if (!API_EXISTS(SSL_SESSION_get0_hostname)) + { + return NULL; + } + return SSL_SESSION_get0_hostname(session); } int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname) { + if (!API_EXISTS(SSL_SESSION_set1_hostname)) + { + return 0; + } + return SSL_SESSION_set1_hostname(session, hostname); } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index d6f3e3df4aad1b..2b01338c0743e0 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -154,10 +154,10 @@ It will unset callback if set is zero. */ PALEXPORT void CryptoNative_SslSetClientCertCallback(SSL* ssl, int set); -/*======= +/* Sets session caching. 0 is disabled. */ -PALEXPORT void CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newCb, SslCtxRemoveSessionCallback removeCb); +PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallback newCb, SslCtxRemoveSessionCallback removeCb); /* Returns name associated with given ssl session. From 342d916a1f50627417118fd02bb79a6808cf429b Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 3 Feb 2022 22:06:26 -0800 Subject: [PATCH 06/10] fix source build --- .../pal_ssl.c | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 3f024e3c2c9f40..19d87d26bd5708 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -532,22 +532,29 @@ void CryptoNative_SslSessionFree(SSL_SESSION *session) const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session) { - if (!API_EXISTS(SSL_SESSION_get0_hostname)) +#ifdef SSL_SESSION_get0_hostname + if (API_EXISTS(SSL_SESSION_get0_hostname)) { - return NULL; + return SSL_SESSION_get0_hostname(session); } - - return SSL_SESSION_get0_hostname(session); +#else + (void*)session; +#endif + return NULL; } int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname) { - if (!API_EXISTS(SSL_SESSION_set1_hostname)) +#ifdef SSL_SESSION_set1_hostname + if (API_EXISTS(SSL_SESSION_set1_hostname)) { - return 0; + SSL_SESSION_set1_hostname(session, hostname); } - - return SSL_SESSION_set1_hostname(session, hostname); +#else + (void*)session; + (const void*)hostname; +#endif + return 0; } int32_t CryptoNative_SslCtxSetEncryptionPolicy(SSL_CTX* ctx, EncryptionPolicy policy) From 51ec5607461e62527891c744e8023c5f80d92f03 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 17 Feb 2022 17:36:54 -0800 Subject: [PATCH 07/10] update comment --- .../System.Security.Cryptography.Native/Interop.OpenSsl.cs | 3 +-- 1 file changed, 1 insertion(+), 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 19557d99ed8204..83247bbd137805 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 @@ -722,7 +722,7 @@ private static unsafe void RemoveSessionCallback(IntPtr ctx, IntPtr session) IntPtr ptr = Ssl.SslCtxGetData(ctx); if (ptr == IntPtr.Zero) { - // Same as above, SafeSslContextHandle could be released while OpenSSL still holds refferecne. + // Same as above, SafeSslContextHandle could be released while OpenSSL still holds reference. return; } @@ -733,7 +733,6 @@ private static unsafe void RemoveSessionCallback(IntPtr ctx, IntPtr session) return; } - //string? name = Marshal.PtrToStringAnsi(Ssl.SessionGetHostname(session));a IntPtr name = Ssl.SessionGetHostname(session); Debug.Assert(name != IntPtr.Zero); ctxHandle.RemoveSession(name, session); From 2170ec4a895e7f31932d4e03fcca511f248a5994 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 2 Mar 2022 22:48:14 -0800 Subject: [PATCH 08/10] feedback from review --- .../Interop.Ssl.cs | 3 --- .../pal_ssl.c | 19 +++++++++---------- .../pal_ssl.h | 16 ++++++++-------- 3 files changed, 17 insertions(+), 21 deletions(-) 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 cb895b2eb95b3e..499547de3fc235 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 @@ -181,9 +181,6 @@ internal static partial class Ssl [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionFree")] internal static partial void SessionFree(IntPtr session); - [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetHostname", CharSet = CharSet.Ansi)] - internal static partial int SessionSetHostname(IntPtr session, string name); - [GeneratedDllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslSessionSetHostname")] internal static partial int SessionSetHostname(IntPtr session, IntPtr name); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 3aa690726cc222..940e295e27e79c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -600,7 +600,6 @@ int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallba SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); } - if (newSessionCb != NULL || removeSessionCb != NULL) if (newSessionCb != NULL) { SSL_CTX_sess_set_new_cb(ctx, newSessionCb); @@ -614,22 +613,22 @@ int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSessionCallba return retValue; } -const char* CryptoNative_SslGetServerName(SSL * ssl) +const char* CryptoNative_SslGetServerName(SSL* ssl) { return SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); } -int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION *session) +int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION* session) { return SSL_set_session(ssl, session); } -void CryptoNative_SslSessionFree(SSL_SESSION *session) +void CryptoNative_SslSessionFree(SSL_SESSION* session) { SSL_SESSION_free(session); } -const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session) +const char* CryptoNative_SslSessionGetHostname(SSL_SESSION* session) { #ifdef SSL_SESSION_get0_hostname if (API_EXISTS(SSL_SESSION_get0_hostname)) @@ -642,7 +641,7 @@ const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session) return NULL; } -int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname) +int CryptoNative_SslSessionSetHostname(SSL_SESSION* session, const char* hostname) { #ifdef SSL_SESSION_set1_hostname if (API_EXISTS(SSL_SESSION_set1_hostname)) @@ -876,7 +875,7 @@ void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, #endif } -static int client_certificate_cb(SSL *ssl, void* state) +static int client_certificate_cb(SSL* ssl, void* state) { (void*)ssl; (void*)state; @@ -904,7 +903,7 @@ void CryptoNative_SslSetPostHandshakeAuth(SSL* ssl, int32_t val) #endif } -int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr) +int32_t CryptoNative_SslSetData(SSL* ssl, void* ptr) { ERR_clear_error(); return SSL_set_ex_data(ssl, 0, ptr); @@ -916,7 +915,7 @@ void* CryptoNative_SslGetData(SSL* ssl) return SSL_get_ex_data(ssl, 0); } -int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void *ptr) +int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void* ptr) { return SSL_CTX_set_ex_data(ctx, 0, ptr); } @@ -990,7 +989,7 @@ int32_t CryptoNative_SslGetCurrentCipherId(SSL* ssl, int32_t* cipherId) } // This function generates key pair and creates simple certificate. -static int MakeSelfSignedCertificate(X509 * cert, EVP_PKEY* evp) +static int MakeSelfSignedCertificate(X509* cert, EVP_PKEY* evp) { RSA* rsa = NULL; ASN1_TIME* time = ASN1_TIME_new(); diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h index 22bcf2d2186b9e..e88eb18444854e 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -168,27 +168,27 @@ PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, SslCtxNewSe Returns name associated with given ssl session. OpenSSL holds reference to it and it must not be freed. */ -PALEXPORT const char* CryptoNative_SslGetServerName(SSL * ssl); +PALEXPORT const char* CryptoNative_SslGetServerName(SSL* ssl); /* This function will attach existing ssl session for possible TLS resume. */ -PALEXPORT int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION *session); +PALEXPORT int32_t CryptoNative_SslSetSession(SSL* ssl, SSL_SESSION* session); /* * Frees SSL session. */ -PALEXPORT void CryptoNative_SslSessionFree(SSL_SESSION *session); +PALEXPORT void CryptoNative_SslSessionFree(SSL_SESSION* session); /* * Get name associated with given SSL_SESSION. */ -PALEXPORT const char* CryptoNative_SslSessionGetHostname(SSL_SESSION *session); +PALEXPORT const char* CryptoNative_SslSessionGetHostname(SSL_SESSION* session); /* * Associate name with given SSL_SESSION. */ -PALEXPORT int CryptoNative_SslSessionSetHostname(SSL_SESSION *session, const char *hostname); +PALEXPORT int CryptoNative_SslSessionSetHostname(SSL_SESSION* session, const char* hostname); /* Shims the SSL_new method. @@ -382,7 +382,7 @@ PALEXPORT void CryptoNative_SslSetVerifyPeer(SSL* ssl); /* Shims SSL_set_ex_data to attach application context. */ -PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void *ptr); +PALEXPORT int32_t CryptoNative_SslSetData(SSL* ssl, void* ptr); /* Shims SSL_get_ex_data to retrieve application context. @@ -392,7 +392,7 @@ PALEXPORT void* CryptoNative_SslGetData(SSL* ssl); /* Shims SSL_CTX_set_ex_data to attach application context. */ -PALEXPORT int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void *ptr); +PALEXPORT int32_t CryptoNative_SslCtxSetData(SSL_CTX* ctx, void* ptr); /* Shims SSL_CTX_get_ex_data to retrieve application context. @@ -460,7 +460,7 @@ PALEXPORT int32_t CryptoNative_SslAddClientCAs(SSL* ssl, X509** x509s, uint32_t /* Shims the ssl_ctx_set_alpn_select_cb method. */ -PALEXPORT void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void *arg); +PALEXPORT void CryptoNative_SslCtxSetAlpnSelectCb(SSL_CTX* ctx, SslCtxSetAlpnCallback cb, void* arg); /* Shims the ssl_set_alpn_protos method. From c2c8580116f259dc8b21f0f7675bf15e5a8fd8cc Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 3 Mar 2022 08:40:48 -0800 Subject: [PATCH 09/10] feedback from review --- src/native/libs/System.Security.Cryptography.Native/pal_ssl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c index 940e295e27e79c..6b849919df9ffd 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -630,7 +630,7 @@ void CryptoNative_SslSessionFree(SSL_SESSION* session) const char* CryptoNative_SslSessionGetHostname(SSL_SESSION* session) { -#ifdef SSL_SESSION_get0_hostname +#if defined NEED_OPENSSL_1_1 || defined NEED_OPENSSL_3_0 if (API_EXISTS(SSL_SESSION_get0_hostname)) { return SSL_SESSION_get0_hostname(session); @@ -643,7 +643,7 @@ const char* CryptoNative_SslSessionGetHostname(SSL_SESSION* session) int CryptoNative_SslSessionSetHostname(SSL_SESSION* session, const char* hostname) { -#ifdef SSL_SESSION_set1_hostname +#if defined NEED_OPENSSL_1_1 || defined NEED_OPENSSL_3_0 if (API_EXISTS(SSL_SESSION_set1_hostname)) { SSL_SESSION_set1_hostname(session, hostname); From 576d4d50bf33ddd175426e8b2e31f15e7bd46ed4 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 4 Mar 2022 17:48:46 -0800 Subject: [PATCH 10/10] avoild client resume on old OpenSSL --- .../System.Security.Cryptography.Native/Interop.OpenSsl.cs | 6 ++++-- 1 file changed, 4 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 6daa8b65b6fe2d..7639d7629c6b50 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 @@ -288,10 +288,12 @@ internal static SafeSslHandle AllocateSslHandle(SafeFreeSslCredentials credentia { if (sslAuthenticationOptions.IsClient) { - // we don't want to try on emtpy TargetName since that is our key. + // We don't support client resume on old OpenSSL versions. + // We don't want to try on empty TargetName since that is our key. // And we don't want to mess up with client authentication. It may be possible // but it seems safe to get full new session. - if (string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) || + if (!Interop.Ssl.Capabilities.Tls13Supported || + string.IsNullOrEmpty(sslAuthenticationOptions.TargetHost) || sslAuthenticationOptions.CertificateContext != null || sslAuthenticationOptions.CertSelectionDelegate != null) {