From caf65d5ac99303a204c8255e3a53444bfa64fe64 Mon Sep 17 00:00:00 2001 From: wfurt Date: Sun, 11 Sep 2022 22:43:28 -0700 Subject: [PATCH 1/4] set session ID when TLS resume is enabled --- .../opensslshim.h | 2 ++ .../System.Security.Cryptography.Native/pal_ssl.c | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h index 1fbf3ce6660422..efb0a9378ae77c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/opensslshim.h +++ b/src/native/libs/System.Security.Cryptography.Native/opensslshim.h @@ -486,6 +486,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) \ + REQUIRED_FUNCTION(SSL_CTX_set_session_id_context) \ REQUIRED_FUNCTION(SSL_CTX_set_verify) \ REQUIRED_FUNCTION(SSL_CTX_use_certificate) \ REQUIRED_FUNCTION(SSL_CTX_use_PrivateKey) \ @@ -965,6 +966,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_CTX_set_session_id_context SSL_CTX_set_session_id_context_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 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 57f1b368b62abf..42349730b7c805 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -11,6 +11,7 @@ #include #include #include +#include c_static_assert(PAL_SSL_ERROR_NONE == SSL_ERROR_NONE); c_static_assert(PAL_SSL_ERROR_SSL == SSL_ERROR_SSL); @@ -678,9 +679,16 @@ int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, SslCtxN { SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); } - else if (cacheSize >= 0) + else { - SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_SIZE, (long)cacheSize, NULL); + unsigned char id[SSL_MAX_SID_CTX_LENGTH]; + getrandom(id, sizeof(id), GRND_RANDOM); + SSL_CTX_set_session_id_context(ctx, id, sizeof(id)); + + if (cacheSize >= 0) + { + SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_SIZE, (long)cacheSize, NULL); + } } if (newSessionCb != NULL) From 3548531acafe2b6c0002267e8f38ecdce3372f01 Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 12 Sep 2022 14:00:10 -0700 Subject: [PATCH 2/4] feedback from review --- .../Interop.OpenSsl.cs | 8 +++++--- .../Interop.SslCtx.cs | 2 +- .../pal_ssl.c | 16 +++++++--------- .../pal_ssl.h | 2 +- 4 files changed, 14 insertions(+), 14 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 f22665ebc260fc..21bf5905f45ec5 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 @@ -203,18 +203,20 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication { if (sslAuthenticationOptions.IsServer) { - Ssl.SslCtxSetCaching(sslCtx, 1, s_cacheSize, null, null); + Span contextId = stackalloc byte[32]; + RandomNumberGenerator.Fill(contextId); + Ssl.SslCtxSetCaching(sslCtx, 1, s_cacheSize, contextId.Length, contextId, null, null); } else { - int result = Ssl.SslCtxSetCaching(sslCtx, 1, s_cacheSize, &NewSessionCallback, &RemoveSessionCallback); + int result = Ssl.SslCtxSetCaching(sslCtx, 1, s_cacheSize, 0, null, &NewSessionCallback, &RemoveSessionCallback); Debug.Assert(result == 1); sslCtx.EnableSessionCache(); } } else { - Ssl.SslCtxSetCaching(sslCtx, 0, -1, null, null); + Ssl.SslCtxSetCaching(sslCtx, 0, -1, 0, null, null, null); } if (sslAuthenticationOptions.IsServer && sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0) 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 18a6b27f825e12..4ed5c1881328eb 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 @@ -33,7 +33,7 @@ internal static partial class Ssl internal static unsafe partial void SslCtxSetAlpnSelectCb(SafeSslContextHandle ctx, delegate* unmanaged callback, IntPtr arg); [LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_SslCtxSetCaching")] - internal static unsafe partial int SslCtxSetCaching(SafeSslContextHandle ctx, int mode, int cacheSize, delegate* unmanaged neewSessionCallback, delegate* unmanaged removeSessionCallback); + internal static unsafe partial int SslCtxSetCaching(SafeSslContextHandle ctx, int mode, int cacheSize, int contextIdLength, Span contextId, delegate* unmanaged neewSessionCallback, delegate* unmanaged removeSessionCallback); internal static bool AddExtraChainCertificates(SafeSslContextHandle ctx, X509Certificate2[] chain) { 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 42349730b7c805..3761913b7663d0 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -656,7 +656,7 @@ void CryptoNative_SslSetVerifyPeer(SSL* ssl) SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } -int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) +int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, unsigned char* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) { int retValue = 1; if (mode && !API_EXISTS(SSL_SESSION_get0_hostname)) @@ -679,16 +679,14 @@ int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, SslCtxN { SSL_CTX_set_options(ctx, SSL_OP_NO_TICKET); } - else + else if (cacheSize >= 0) { - unsigned char id[SSL_MAX_SID_CTX_LENGTH]; - getrandom(id, sizeof(id), GRND_RANDOM); - SSL_CTX_set_session_id_context(ctx, id, sizeof(id)); + SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_SIZE, (long)cacheSize, NULL); + } - if (cacheSize >= 0) - { - SSL_CTX_ctrl(ctx, SSL_CTRL_SET_SESS_CACHE_SIZE, (long)cacheSize, NULL); - } + if (contextIdLength > 0 && contextId != NULL) + { + SSL_CTX_set_session_id_context(ctx, contextId, contextIdLength <= SSL_MAX_SID_CTX_LENGTH ? (unsigned int)contextIdLength : SSL_MAX_SID_CTX_LENGTH); } if (newSessionCb != NULL) 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 3b186a96cbb7d0..2fc0fa0c373507 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -162,7 +162,7 @@ PALEXPORT void CryptoNative_SslSetPostHandshakeAuth(SSL* ssl, int32_t val); /* Sets session caching. 0 is disabled. */ -PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, SslCtxNewSessionCallback newCb, SslCtxRemoveSessionCallback removeCb); +PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, unsigned char* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb); /* Returns name associated with given ssl session. From 10a2c2807d8a3ef4ef19a524079516611431b72d Mon Sep 17 00:00:00 2001 From: wfurt Date: Mon, 12 Sep 2022 14:04:39 -0700 Subject: [PATCH 3/4] remove random.h --- src/native/libs/System.Security.Cryptography.Native/pal_ssl.c | 1 - 1 file changed, 1 deletion(-) 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 3761913b7663d0..7d1ce8334e6240 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -11,7 +11,6 @@ #include #include #include -#include c_static_assert(PAL_SSL_ERROR_NONE == SSL_ERROR_NONE); c_static_assert(PAL_SSL_ERROR_SSL == SSL_ERROR_SSL); From 6f0d0e0ba0cbd6fb8f4757ccfe9d1d3d2eb95887 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Mon, 12 Sep 2022 14:29:18 -0700 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Jeremy Barton --- src/native/libs/System.Security.Cryptography.Native/pal_ssl.c | 2 +- src/native/libs/System.Security.Cryptography.Native/pal_ssl.h | 2 +- 2 files 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 7d1ce8334e6240..431c2b8ec06722 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.c @@ -655,7 +655,7 @@ void CryptoNative_SslSetVerifyPeer(SSL* ssl) SSL_set_verify(ssl, SSL_VERIFY_PEER, verify_callback); } -int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, unsigned char* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) +int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, uint8_t* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb) { int retValue = 1; if (mode && !API_EXISTS(SSL_SESSION_get0_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 2fc0fa0c373507..738511d31a1d96 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h +++ b/src/native/libs/System.Security.Cryptography.Native/pal_ssl.h @@ -162,7 +162,7 @@ PALEXPORT void CryptoNative_SslSetPostHandshakeAuth(SSL* ssl, int32_t val); /* Sets session caching. 0 is disabled. */ -PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, unsigned char* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb); +PALEXPORT int CryptoNative_SslCtxSetCaching(SSL_CTX* ctx, int mode, int cacheSize, int contextIdLength, uint8_t* contextId, SslCtxNewSessionCallback newSessionCb, SslCtxRemoveSessionCallback removeSessionCb); /* Returns name associated with given ssl session.