Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a unified cache for SSL_CTX objects #112567

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+43 −66
Open

Use a unified cache for SSL_CTX objects #112567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0141432
Use the same cache for client and server SSL_CTX objects
rzikm Feb 14, 2025
fdb019a
Feedback
rzikm Feb 17, 2025
cc77e40
Minor improvements
rzikm Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 36 additions & 45 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading