Skip to content

Commit

Permalink
Use a unified cache for SSL_CTX objects (#112567)
Browse files Browse the repository at this point in the history 
* Use the same cache for client and server SSL_CTX objects

* Feedback

* Minor improvements
  • Loading branch information
@rzikm
rzikm authored Feb 24, 2025
1 parent 8e492e4 commit 1d81756
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 66 deletions.
81 changes: 36 additions & 45 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,25 @@ internal static partial class OpenSsl

private sealed class SafeSslContextCache : SafeHandleCache<SslContextCacheKey, SafeSslContextHandle> { }

private static readonly SafeSslContextCache s_clientSslContexts = new();
private static readonly SafeSslContextCache s_sslContexts = new();

internal readonly struct SslContextCacheKey : IEquatable<SslContextCacheKey>
{
public readonly bool IsClient;
public readonly byte[]? CertificateThumbprint;
public readonly SslProtocols SslProtocols;

public SslContextCacheKey(SslProtocols sslProtocols, byte[]? certificateThumbprint)
public SslContextCacheKey(bool isClient, SslProtocols sslProtocols, byte[]? certificateThumbprint)
{
IsClient = isClient;
SslProtocols = sslProtocols;
CertificateThumbprint = certificateThumbprint;
}

public override bool Equals(object? obj) => obj is SslContextCacheKey key && Equals(key);

public bool Equals(SslContextCacheKey other) =>
IsClient == other.IsClient &&
SslProtocols == other.SslProtocols &&
(CertificateThumbprint == null && other.CertificateThumbprint == null ||
CertificateThumbprint != null && other.CertificateThumbprint != null && CertificateThumbprint.AsSpan().SequenceEqual(other.CertificateThumbprint));
Expand All @@ -55,6 +58,7 @@ public override int GetHashCode()
{
HashCode hash = default;

hash.Add(IsClient);
hash.Add(SslProtocols);
if (CertificateThumbprint != null)
{
Expand Down Expand Up @@ -161,41 +165,19 @@ internal static SafeSslContextHandle GetOrCreateSslContextHandle(SslAuthenticati
return AllocateSslContext(sslAuthenticationOptions, protocols, allowCached);
}

if (sslAuthenticationOptions.IsClient)
{
var key = new SslContextCacheKey(protocols, sslAuthenticationOptions.CertificateContext?.TargetCertificate.GetCertHash(HashAlgorithmName.SHA512));
return s_clientSslContexts.GetOrCreate(key, static (args) =>
{
var (sslAuthOptions, protocols, allowCached) = args;
return AllocateSslContext(sslAuthOptions, protocols, allowCached);
}, (sslAuthenticationOptions, protocols, allowCached));
}

// cache in SslStreamCertificateContext is bounded and there is no eviction
// so the handle should always be valid,

bool hasAlpn = sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0;

SslProtocols serverCacheKey = protocols | (hasAlpn ? FakeAlpnSslProtocol : SslProtocols.None);
if (!sslAuthenticationOptions.CertificateContext!.SslContexts!.TryGetValue(serverCacheKey, out SafeSslContextHandle? handle))
{
// not found in cache, create and insert
handle = AllocateSslContext(sslAuthenticationOptions, protocols, allowCached);

SafeSslContextHandle cached = sslAuthenticationOptions.CertificateContext!.SslContexts!.GetOrAdd(serverCacheKey, handle);

if (handle != cached)
{
// lost the race, another thread created the SSL_CTX meanwhile, prefer the cached one
handle.Dispose();
Debug.Assert(handle.IsClosed);
handle = cached;
}
}
SslProtocols serverProtocolCacheKey = protocols | (hasAlpn ? FakeAlpnSslProtocol : SslProtocols.None);

Debug.Assert(!handle.IsClosed);
handle.TryAddRentCount();
return handle;
var key = new SslContextCacheKey(
sslAuthenticationOptions.IsClient,
sslAuthenticationOptions.IsClient ? protocols : serverProtocolCacheKey,
sslAuthenticationOptions.CertificateContext?.TargetCertificate.GetCertHash(HashAlgorithmName.SHA512));
return s_sslContexts.GetOrCreate(key, static (args) =>
{
var (sslAuthOptions, protocols, allowCached) = args;
return AllocateSslContext(sslAuthOptions, protocols, allowCached);
}, (sslAuthenticationOptions, protocols, allowCached));
}

// This essentially wraps SSL_CTX* aka SSL_CTX_new + setting
Expand Down Expand Up @@ -367,8 +349,7 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
{
// Server should always have certificate
Debug.Assert(sslAuthenticationOptions.CertificateContext != null);
if (sslAuthenticationOptions.CertificateContext == null ||
sslAuthenticationOptions.CertificateContext.SslContexts == null)
if (sslAuthenticationOptions.CertificateContext == null)
{
cacheSslContext = false;
}
Expand All @@ -395,6 +376,25 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
throw CreateSslException(SR.net_allocate_ssl_context_failed);
}

if (cacheSslContext)
{
// For non-cached SSL_CTX instances, we free the `sslCtxHandle`
// after creating the SSL instance and don't use it again. We don't
// access it afterwards and OpenSSL has internal refcount which
// keeps it alive until the last SSL using it is freed.
//
// For cached SSL_CTX instances, we want to keep an outstanding
// up-ref to indicate that it is in use and does not get
// evicted from the cache.
//
// This call should always succeed because we already
// increased the rent count when getting the context from
// the cache.
bool success = sslCtxHandle.TryAddRentCount();
Debug.Assert(success);
sslHandle.SslContextHandle = sslCtxHandle;
}

if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)
{
if (sslAuthenticationOptions.IsServer)
Expand Down Expand Up @@ -426,15 +426,6 @@ internal static SafeSslHandle AllocateSslHandle(SslAuthenticationOptions sslAuth
if (cacheSslContext)
{
sslCtxHandle.TrySetSession(sslHandle, sslAuthenticationOptions.TargetHost);

// Maintain additional rent count for the context so
// that it is not evicted from the cache and future
// SSL objects can reuse it. This call should always
// succeed because already have increased rent count
// when getting the context from the cache
bool success = sslCtxHandle.TryAddRentCount();
Debug.Assert(success);
sslHandle.SslContextHandle = sslCtxHandle;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,8 @@ internal sealed class SafeSslHandle : SafeDeleteSslContext
private bool _handshakeCompleted;

public GCHandle AlpnHandle;
// Reference to the parent SSL_CTX handle in the SSL_CTX is being cached. Only used for
// refcount management.
public SafeSslContextHandle? SslContextHandle;

public bool IsServer
Expand Down Expand Up @@ -445,8 +447,6 @@ protected override bool ReleaseHandle()
Disconnect();
}

// drop reference to any SSL_CTX handle, any handle present here is being
// rented from (client) SSL_CTX cache.
SslContextHandle?.Dispose();

if (AlpnHandle.IsAllocated)
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.SslCtx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ protected override bool ReleaseHandle()
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.
// Since there is no relation between SafeSslContextHandle and SafeSslHandle `this`
// can be released while we still have SSL session using it.
Interop.Ssl.SslCtxSetData(handle, IntPtr.Zero);

lock (_sslSessions)
Expand Down
20 changes: 3 additions & 17 deletions ...ibraries/System.Net.Security/src/System/Net/Security/SslStreamCertificateContext.Linux.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,11 @@ public partial class SslStreamCertificateContext

private const bool TrimRootCertificate = true;
private const bool ChainBuildNeedsTrustedRoot = false;
internal ConcurrentDictionary<SslProtocols, SafeSslContextHandle> SslContexts
{
get
{
ConcurrentDictionary<SslProtocols, SafeSslContextHandle>? sslContexts = _sslContexts;
if (sslContexts is null)
{
Interlocked.CompareExchange(ref _sslContexts, new(), null);
sslContexts = _sslContexts;
}

return sslContexts;
}
}

private ConcurrentDictionary<SslProtocols, SafeSslContextHandle>? _sslContexts;
internal readonly SafeX509Handle CertificateHandle;
internal readonly SafeEvpPKeyHandle KeyHandle;

private object SyncObject => KeyHandle;

private bool _staplingForbidden;
private byte[]? _ocspResponse;
private DateTimeOffset _ocspExpiration;
Expand Down Expand Up @@ -239,7 +225,7 @@ partial void AddRootCertificate(X509Certificate2? rootCertificate, ref bool tran
return new ValueTask<byte[]?>((byte[]?)null);
}

lock (SslContexts)
lock (SyncObject)
{
pending = _pendingDownload;

Expand Down

0 comments on commit 1d81756

Please sign in to comment.