From a80d93dc5ba121f81b55f3948163460d19bff858 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Thu, 20 Aug 2026 15:53:00 +0200 Subject: [PATCH 01/16] user callbacks running on thread pool --- .../src/ClientCertificateValidator.cs | 12 +- .../Connection/DirectTlsConnectionListener.cs | 10 +- .../src/DirectTlsTransportFactory.cs | 3 +- .../src/HandshakeUserCallback.cs | 211 +++++++ .../src/Interop/NativeTls.cs | 12 + .../src/TlsEventPump.UserCallbacks.cs | 304 +++++++++ .../Transport.DirectTls/src/TlsEventPump.cs | 583 +++++++++++------- .../src/TlsEventPumpPool.cs | 6 +- .../DirectTlsContextResolverFastPathTests.cs | 132 ++++ .../DirectTlsUserCallbackDispatchTests.cs | 278 +++++++++ 10 files changed, 1318 insertions(+), 233 deletions(-) create mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/ClientCertificateValidator.cs b/src/Servers/Kestrel/Transport.DirectTls/src/ClientCertificateValidator.cs index 47c575331f61..7baeb3266423 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/ClientCertificateValidator.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/ClientCertificateValidator.cs @@ -21,17 +21,17 @@ internal static class ClientCertificateValidator /// 's default server-side client-certificate validation policy. /// /// - /// The returned chain is configured for use on the pump thread: + /// The chain build and the endpoint callback run on the thread pool, never on the pump thread. /// /// - /// avoids blocking the pump thread on CRL/OCSP network I/O and - /// matches the transport default (CheckCertificateRevocation == false). + /// avoids blocking on CRL/OCSP network I/O and matches the + /// transport default (CheckCertificateRevocation == false). /// /// /// is so the chain - /// engine never makes synchronous AIA fetches for missing intermediates. - /// runs on the pump thread, so an attacker-supplied leaf whose Authority Information Access extension - /// points at an unreachable or slow URL could otherwise stall every connection this pump owns. + /// engine never makes synchronous AIA fetches for missing intermediates. Otherwise a supplied + /// leaf whose Authority Information Access extension points at an unreachable or slow URL would occupy a + /// thread pool thread for the lifetime of that fetch, one per connection. /// sets the same flag on the server side for this reason. /// Legitimate clients send their intermediates in the handshake, which are supplied here via . /// diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs index 69349dfeddf2..2fa0a92d8d91 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs @@ -31,6 +31,9 @@ internal sealed class DirectTlsConnectionListener : IConnectionListener private readonly TlsEventPumpPool _pumpPool; private readonly Action>? _clientHelloCallback; + // Decides whether the listener will try to select a server certificate for incoming connections. + private readonly bool _serverCertificateSelectorConfigured; + // Native OpenSSL server credentials (bootstrap + per-SNI contexts) owned by this listener. Disposed once, // at the end of DisposeAsync, after the pump threads are joined. Null only in tests that don't wire them. private readonly IDisposable? _ownedServerContexts; @@ -62,7 +65,8 @@ public DirectTlsConnectionListener( MemoryPool memoryPool, IHostApplicationLifetime applicationLifetime, Action>? clientHelloCallback = null, - IDisposable? ownedServerContexts = null) + IDisposable? ownedServerContexts = null, + bool serverCertificateSelectorConfigured = true) { ArgumentNullException.ThrowIfNull(tlsContext); ArgumentNullException.ThrowIfNull(applicationLifetime); @@ -75,6 +79,7 @@ public DirectTlsConnectionListener( _tlsContext = tlsContext; _contextResolver = contextResolver; _clientHelloCallback = clientHelloCallback; + _serverCertificateSelectorConfigured = serverCertificateSelectorConfigured; _ownedServerContexts = ownedServerContexts; _appLifetime = applicationLifetime; EndPoint = endpoint; @@ -133,7 +138,8 @@ internal void Bind() _options.MaxWriteBufferSize ?? 0, OnPumpFatalError, _clientHelloCallback, - _connectionTracker); + _connectionTracker, + _serverCertificateSelectorConfigured); _logger.LogInformation("DirectTls listener started with EPOLLEXCLUSIVE worker accept"); } diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs b/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs index ae48cc7018aa..b9d69339f1a6 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs @@ -163,7 +163,8 @@ public async ValueTask BindAsync(EndPoint endpoint, Cancell memoryPool, _applicationLifetime, clientHelloCallback, - ownedServerContexts); + ownedServerContexts, + serverCertificateSelectorConfigured: endpointOptions.ServerCertificateSelector is not null); _logger.LogInformation("DirectTls listener bound for endpoint {Endpoint}.", endpoint); diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs b/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs new file mode 100644 index 000000000000..faea9e6d3486 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs @@ -0,0 +1,211 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; + +/// +/// One suspended handshake's user code, executed on the thread pool instead of on the pump (epoll) thread. +/// +/// +/// User-supplied handshake callbacks - the ClientHello listener, the server-certificate selector, and the +/// client-certificate validation callback - can block for an arbitrarily long time (a disk read, a key vault +/// round trip, a lock). A pump thread owns accept plus all I/O readiness for every connection assigned to it, +/// so running any of them inline stalls every one of those connections. Instead the pump parks the handshake +/// (de-registering its fd from the epoll set so it cannot generate pump work while parked) and queues this +/// work item. Everything a derived item touches was copied out of the session on the pump thread beforehand: +/// it never calls into TlsSocketSession, which stays single-threaded and owned by its pump. When the +/// user code returns - or throws - the result is handed back to the owning pump through +/// , which resumes the handshake on the pump thread. +/// +/// Each suspension point has its own derived type carrying only its own state, so the pump resumes by +/// switching on the work item's type. is deliberately not virtual: the try/catch/finally +/// it wraps every callback in is what guarantees that a suspended handshake reports back exactly once, whether +/// the user code returns or throws, so a derived type must not be able to replace it. +/// +/// +internal abstract class HandshakeUserCallback : IThreadPoolWorkItem +{ + private readonly TlsEventPump _pump; + + protected HandshakeUserCallback(TlsEventPump pump, int fd, DirectTlsConnection? connection) + { + _pump = pump; + Fd = fd; + Connection = connection; + } + + /// The handshaking file descriptor this callback belongs to. + public int Fd { get; } + + /// + /// The connection allocated early (at NeedsTlsContext) so user code sees a stable + /// . Null when nothing needed one that early: the pump resolved the TLS + /// context inline because no user code runs at NeedsTlsContext (so only the client-certificate + /// suspension is reachable, and it does not use this), or the pump has no memory pool (tests). In both + /// cases the connection is allocated when the handshake completes instead. + /// + public DirectTlsConnection? Connection { get; } + + /// The exception the user code threw, if any. Non-null means the pump drops the connection. + public Exception? Failure { get; private set; } + + /// + public void Execute() + { + try + { + RunUserCode(); + } + catch (Exception ex) + { + // A throwing user callback (or a selector that resolved no certificate) fails this one connection. + // The pump logs it and drops the handshake when it picks the result up; it must never escape onto + // a thread pool thread, where it would tear the process down. + Failure = ex; + } + finally + { + ReleaseTransientState(); + + // Hand the result back to the owning pump. Nothing here may touch the session, the epoll set, or + // the handshake bookkeeping - those are pump-thread-only. + _pump.CompleteUserCallback(this); + } + } + + /// + /// Runs the endpoint-supplied callback on the thread pool and records its result on this instance. Any + /// throw is captured by as . + /// + protected abstract void RunUserCode(); + + /// + /// Releases anything borrowed for the duration of the callback, whether it returned or threw. Runs before + /// the result is handed back to the pump. + /// + protected virtual void ReleaseTransientState() + { + } +} + +/// +/// The NeedsTlsContext suspension: the optional ClientHello listener followed by the server-certificate +/// selector. +/// +internal sealed class ResolveTlsContextCallback : HandshakeUserCallback +{ + private readonly Func _contextResolver; + private readonly Action>? _clientHelloCallback; + private readonly string? _targetHostName; + + // ClientHello record copied out of the session on the pump thread. Rented from the shared pool and + // returned as soon as the (synchronous) user callback returns, matching the transient-buffer contract of + // the socket-transport TlsListener. + private byte[]? _clientHelloBuffer; + private readonly int _clientHelloLength; + + /// + /// Creates the work item for the NeedsTlsContext suspension. + /// holds the ClientHello record already copied out of the session on the pump thread (null when there is + /// no listener, or nothing was captured); this work item returns it to + /// once the listener has run. + /// + public ResolveTlsContextCallback( + TlsEventPump pump, + int fd, + DirectTlsConnection? connection, + string? targetHostName, + Func contextResolver, + Action>? clientHelloCallback, + byte[]? clientHelloBuffer, + int clientHelloLength) + : base(pump, fd, connection) + { + _targetHostName = targetHostName; + _contextResolver = contextResolver; + _clientHelloCallback = clientHelloCallback; + _clientHelloBuffer = clientHelloBuffer; + _clientHelloLength = clientHelloLength; + } + + /// The TLS context the certificate selector resolved. + public TlsContext? ResolvedContext { get; private set; } + + /// The client-certificate validation callback that came back with the resolved context. + public RemoteCertificateValidationCallback? ResolvedClientCertificateValidation { get; private set; } + + /// + protected override void RunUserCode() + { + // Fire the observe-only ClientHello listener first: the listener sees the ClientHello before the real context is installed. + if (_clientHelloCallback is not null && Connection is not null && _clientHelloBuffer is not null && _clientHelloLength > 0) + { + _clientHelloCallback(Connection, new ReadOnlySequence(_clientHelloBuffer, 0, _clientHelloLength)); + } + + var (context, clientCertificateValidation) = _contextResolver(Connection, _targetHostName); + ResolvedContext = context; + ResolvedClientCertificateValidation = clientCertificateValidation; + } + + /// + protected override void ReleaseTransientState() + { + if (_clientHelloBuffer is { } buffer) + { + _clientHelloBuffer = null; + ArrayPool.Shared.Return(buffer); + } + } +} + +/// +/// The client-certificate validation suspension: the chain build plus the endpoint's validation callback, run +/// once the handshake reports Complete. +/// +internal sealed class ValidateClientCertificateCallback : HandshakeUserCallback +{ + private readonly RemoteCertificateValidationCallback _validateClientCertificate; + private readonly object _validationSender; + private readonly X509Certificate2Collection? _intermediates; + + /// + /// Creates the work item for the client-certificate validation suspension. The certificates were read from + /// the session on the pump thread; the chain build and the endpoint's callback run here. + /// + public ValidateClientCertificateCallback( + TlsEventPump pump, + int fd, + DirectTlsConnection? connection, + object validationSender, + X509Certificate2? presentedCertificate, + X509Certificate2Collection? intermediates, + RemoteCertificateValidationCallback validateClientCertificate) + : base(pump, fd, connection) + { + _validationSender = validationSender; + PresentedCertificate = presentedCertificate; + _intermediates = intermediates; + _validateClientCertificate = validateClientCertificate; + } + + /// The peer's leaf certificate, or null when it presented none. + public X509Certificate2? PresentedCertificate { get; } + + /// Whether the endpoint's validation callback accepted the peer's certificate. + public bool CertificateAccepted { get; private set; } + + /// + protected override void RunUserCode() + => CertificateAccepted = ClientCertificateValidator.Validate( + _validationSender, + PresentedCertificate, + _intermediates, + _validateClientCertificate); +} diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/Interop/NativeTls.cs b/src/Servers/Kestrel/Transport.DirectTls/src/Interop/NativeTls.cs index 7311ae826096..f6959c7207c0 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/Interop/NativeTls.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/Interop/NativeTls.cs @@ -32,6 +32,14 @@ internal static partial class NativeTls // Reporting EAGAIN/EBADF/EINTR as errno return values lets the accept loop drain without exceptions. [LibraryImport(LIBC, SetLastError = true)] public static partial int accept4(int sockfd, IntPtr addr, IntPtr addrlen, int flags); + // eventfd: the pump's cross-thread wakeup. A thread pool thread may write to same fd and make it wake up on epoll_wait. + // The counter is drained with a single 8-byte read (EFD_NONBLOCK makes an empty read return EAGAIN rather than block the pump). + [LibraryImport(LIBC, SetLastError = true)] public static partial int eventfd(uint initval, int flags); + + [LibraryImport(LIBC, SetLastError = true)] public static partial nint read(int fd, ref long buf, nuint count); + + [LibraryImport(LIBC, SetLastError = true)] public static partial nint write(int fd, ref long buf, nuint count); + [LibraryImport(LIBC, SetLastError = true)] private static partial int epoll_ctl(int epfd, int op, int fd, ref EpollEventPacked ev); @@ -109,6 +117,10 @@ public EpollEvent this[int index] public const int SOCK_CLOEXEC = 0x80000; // Linux O_CLOEXEC public const int EPOLL_CLOEXEC = 0x80000; // Linux O_CLOEXEC + // eventfd flags (Linux eventfd2). Same values as O_NONBLOCK / O_CLOEXEC. + public const int EFD_NONBLOCK = 0x800; + public const int EFD_CLOEXEC = 0x80000; + // errno values the accept loop distinguishes (Linux asm-generic/errno-base.h). EWOULDBLOCK == EAGAIN on Linux. public const int EINTR = 4; // interrupted by a signal before a connection was accepted - retry public const int EBADF = 9; // listen fd closed underneath the pump during shutdown diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs new file mode 100644 index 000000000000..31af15119b6e --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -0,0 +1,304 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Concurrent; +using System.Diagnostics; +using System.Net.Security; +using System.Runtime.InteropServices; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Interop; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; + +/// +/// The half of the pump that runs endpoint-supplied handshake callbacks off the event loop. +/// +/// +/// A user callback (certificate selector, ClientHello listener, client-certificate validation) may block for +/// an unbounded time, and a pump thread owns the accept path plus the I/O readiness of every connection +/// assigned to it, so running one inline would stall all of them. Instead the handshake is parked, the +/// callback runs on the thread pool, and the result is handed back for the pump to resume. The split is +/// strictly by thread ownership: everything below either runs on the pump thread or does nothing but move a +/// result towards it - the TLS session, the epoll set and the handshake bookkeeping stay pump-thread-only. +/// +internal partial class TlsEventPump +{ + private readonly ConcurrentQueue _completedCallbacks = new(); + + // Number of user callbacks queued to the thread pool that have not reported back yet. The pump's fds are + // closed and _exitSignal is completed only once the loop has exited AND this reaches zero, so a callback + // can never write to a recycled wakeup fd, and a caller that observes the exit signal knows no thread can + // still be running user code that reaches this pump's resources. + private int _outstandingUserCallbacks; + + // Handshakes that were parked on a user callback when the pump loop exited. Their teardown is deferred to + // the drained-shutdown path so the session (and the certificate handed to the validation callback) is not + // disposed underneath live user code. Only ever touched by the thread that exits the loop and then by the + // single thread that runs the one-shot shutdown completion, so it needs no synchronization of its own. + private readonly List _handshakesAwaitingCallback = []; + + // Whether resolving the TLS context can run user code: an endpoint-supplied certificate selector, or the + // ClientHello listener that runs just before it. When neither is configured the resolver is the transport's + // own lambda over a static certificate and a per-certificate context cache, so there is nothing to move off + // the event loop and the pump resolves inline instead of suspending. Defaults to true so a caller that does + // not report its selector fails safe onto the suspending path. + private bool _contextResolverRunsUserCode = true; + + // Creates the eventfd a thread pool thread writes to when a user callback has finished, and registers it + // in this pump's epoll set so the write wakes the loop. Called from the constructor once _epollFd exists; + // it returns the fd rather than assigning it so the field can stay readonly. On failure it closes what it + // opened (including the caller's epoll fd) before throwing, because the pump is never constructed and so + // nothing else will ever run its teardown. + private int CreateWakeupFd() + { + int wakeupFd = NativeTls.eventfd(0, NativeTls.EFD_NONBLOCK | NativeTls.EFD_CLOEXEC); + if (wakeupFd < 0) + { + int errno = Marshal.GetLastWin32Error(); + NativeTls.close(_epollFd); + throw new InvalidOperationException($"eventfd failed: errno={errno}"); + } + + var wakeupEvent = new EpollEvent + { + Events = NativeTls.EPOLLIN, + Data = new EpollData { Fd = wakeupFd } + }; + + if (NativeTls.epoll_ctl(_epollFd, NativeTls.EPOLL_CTL_ADD, wakeupFd, ref wakeupEvent) < 0) + { + int errno = Marshal.GetLastWin32Error(); + NativeTls.close(wakeupFd); + NativeTls.close(_epollFd); + throw new InvalidOperationException($"Failed to add the pump wakeup fd to epoll: errno={errno}"); + } + + return wakeupFd; + } + + // Releases the handshakes that were parked on user code when the loop exited. Called exactly once, from the + // one-shot drained-shutdown path, by whichever thread observes the last in-flight callback completing - so + // by then the loop is gone and no user callback is running, giving this thread sole ownership. + private void ReleaseDeferredHandshakes() + { + foreach (var conn in _handshakesAwaitingCallback) + { + ReleaseHandshakeResources(conn); + } + + _handshakesAwaitingCallback.Clear(); + } + + // Finishes the pump's teardown once BOTH the event loop has exited and every user callback it dispatched to + // the thread pool has reported back: closes the fds it owns and completes the exit signal. Called by the + // pump thread when the loop ends and by each callback completion, so whichever happens last does the work. + // Deferring the close until the callbacks have drained is what makes CompleteUserCallback's wakeup write safe: + // the wakeup fd cannot have been closed (and its number recycled by an unrelated fd) while a callback is + // still in flight. + private void CompletePumpShutdownIfDrained() + { + if (!_loopExited || Volatile.Read(ref _outstandingUserCallbacks) != 0) + { + return; + } + + if (Interlocked.Exchange(ref _shutdownCompleted, 1) != 0) + { + return; + } + + CloseOwnedFds(); + ReleaseDeferredHandshakes(); + _exitSignal.TrySetResult(); + } + + /// + /// Parks a handshake while its user callback runs on the thread pool. + /// + /// + /// The fd is removed from this pump's epoll set for the whole suspension, so a parked connection cannot + /// generate pump work (a peer that keeps writing would otherwise re-fire level-triggered EPOLLIN on every + /// epoll_wait and spin the pump). The work item is recorded on the handshaking entry: it is both the + /// "suspended" marker the event loop and the timeout sweep honour, and the token + /// matches on so a completion can never resume a handshake that was + /// torn down (or an unrelated connection that reused the fd number) while the callback ran. + /// Runs on the pump thread only. + /// + private void SuspendHandshake(int fd, ref HandshakingConnection conn, HandshakeUserCallback callback) + { + DeregisterFromEpoll(fd); + + conn.PendingUserCallback = callback; + conn.CurrentEpollInterest = 0; + _handshaking[fd] = conn; + + // Counted before the item is queued so the pump's shutdown can never observe zero in-flight callbacks + // while one is about to run. + Interlocked.Increment(ref _outstandingUserCallbacks); + + // preferLocal: false - this must not land on the pump thread's local queue; the whole point is to get + // the user code onto a different thread. + ThreadPool.UnsafeQueueUserWorkItem(callback, preferLocal: false); + } + + /// + /// Called from a thread pool thread once a suspended handshake's user callback has finished (or thrown). + /// Hands the result to the owning pump and wakes it; the handshake itself is only ever resumed on the pump + /// thread, which is the sole owner of the TLS session. + /// + internal void CompleteUserCallback(HandshakeUserCallback callback) + { + _completedCallbacks.Enqueue(callback); + + // Wake the pump before releasing the in-flight count: the wakeup fd is only closed once the loop has + // exited AND that count reaches zero, so this write can never hit a recycled descriptor. + Wakeup(); + + Interlocked.Decrement(ref _outstandingUserCallbacks); + CompletePumpShutdownIfDrained(); + } + + // Nudges the pump out of epoll_wait by making its eventfd readable. Called from thread-pool threads. + private void Wakeup() + { + long value = 1; + if (NativeTls.write(_wakeupFd, ref value, sizeof(long)) < 0) + { + // EAGAIN only happens if the 64-bit counter is saturated (2^64-1 pending wakeups), which cannot + // happen here; anything else means the fd is gone, in which case the pump is already shutting down. + _logger.LogDebug("Pump {Id}: writing the wakeup fd failed: errno={Errno}", _id, Marshal.GetLastWin32Error()); + } + } + + // Consumes the eventfd counter so the level-triggered wakeup fd stops firing. Runs on the pump thread. + private void DrainWakeup() + { + long value = 0; + long read = NativeTls.read(_wakeupFd, ref value, sizeof(long)); + + // The counter is not a message and carries no payload: an eventfd created without EFD_SEMAPHORE + // returns the accumulated sum of every write since the last read and resets it to zero, so N callbacks + // completing between two polls surface as a single readable event with value N. The value is + // deliberately not used to decide how much work to do - the completion queue is the source of truth, + // and this fd only says "look at it". epoll reported EPOLLIN and the pump thread is the only reader, + // so the counter cannot have been consumed in between: a short read or a zero counter would mean the + // fd is not the eventfd we registered. + Debug.Assert(read == sizeof(long), $"Reading the wakeup fd returned {read} bytes."); + Debug.Assert(value >= 1, $"The wakeup fd reported a counter of {value}."); + } + + // Resumes every handshake whose user callback has reported back. Runs on the pump thread only. + private void DrainCompletedUserCallbacks() + { + while (_completedCallbacks.TryDequeue(out var callback)) + { + ResumeSuspendedHandshake(callback); + } + } + + /// + /// Resumes one suspended handshake with the result of its user callback. Runs on the pump thread only. + /// + /// + /// A completion is only acted on when the handshaking entry for the fd still points at this exact work + /// item. Anything else - the connection was dropped (handshake timeout, listener shutdown, the peer went + /// away), or the fd number was recycled by a newer connection - means this result is stale, and it is + /// discarded. The resources of the dropped connection were already released exactly once by the teardown + /// path that removed it, so there is nothing to release here. + /// + private void ResumeSuspendedHandshake(HandshakeUserCallback callback) + { + int fd = callback.Fd; + if (!_handshaking.TryGetValue(fd, out var conn) || !ReferenceEquals(conn.PendingUserCallback, callback)) + { + _logger.LogDebug("Pump {Id}: discarding a user callback result for fd={Fd}; the handshake is gone.", _id, fd); + return; + } + + // Clear the suspension marker first, so this handshake can never be resumed twice. + conn.PendingUserCallback = null; + _handshaking[fd] = conn; + + if (callback.Failure is { } failure) + { + // The ClientHello listener, the certificate selector, or the client-certificate validation callback + // threw. Fail this one connection (matching the socket-transport TlsListener) and log it. + _logger.LogDebug(failure, "A TLS handshake callback failed for fd={Fd}; dropping connection.", fd); + DropHandshake(fd, conn); + return; + } + + // Re-arm the fd before touching the session again: from here on the handshake may need more epoll + // round-trips, and a completed handshake is promoted with EPOLL_CTL_MOD, which requires registration. + if (!TryArmHandshakeInterest(fd, DefaultEpollInterest)) + { + DropHandshake(fd, conn); + return; + } + + conn.CurrentEpollInterest = DefaultEpollInterest; + _handshaking[fd] = conn; + + switch (callback) + { + case ResolveTlsContextCallback resolvedContext: + try + { + conn.Session.SetContext(resolvedContext.ResolvedContext!); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Installing the resolved TLS context failed for fd={Fd}", fd); + DropHandshake(fd, conn); + return; + } + + // The endpoint's client-certificate validation callback is resolved with the context. Persist + // it on the handshaking entry so the Complete branch can drive mTLS validation, even if the + // handshake needs several more epoll round-trips (each re-reads _handshaking[fd]). + conn.ClientCertificateValidation = resolvedContext.ResolvedClientCertificateValidation; + _handshaking[fd] = conn; + + // Real context is now set; continue the handshake immediately. + TryAdvanceHandshake(fd, conn); + return; + + case ValidateClientCertificateCallback validation: + if (!validation.CertificateAccepted) + { + _logger.LogDebug("Client certificate rejected for fd={Fd} (presented={Presented}).", fd, validation.PresentedCertificate is not null); + DropHandshake(fd, conn); + return; + } + + // Record the accepted result so the runtime promotes the leaf into its canonical remote-cert + // slot and clears its pending-validation state. + try + { + conn.Session.SetRemoteCertificateValidationResult(SslPolicyErrors.None); + } + catch (InvalidOperationException) + { + // Validation was already resolved (e.g. a buffered PAL that surfaced + // NeedsCertificateValidation before reaching Complete). + } + + // Surface the accepted certificate to Kestrel via ITlsConnectionFeature. This is the same + // instance the runtime just promoted into its canonical remote-cert slot (on the accept path + // SetRemoteCertificateValidationResult moves _externalPendingCert into _remoteCertificate + // without reallocating), and null when the client presented none on an AllowCertificate + // endpoint. + CompleteHandshake(fd, conn, validation.PresentedCertificate); + return; + + default: + // Unreachable: every suspension point has a case above. Guard it anyway - the fd has just been + // re-armed, so falling out of the switch would leave the handshake registered but never + // advanced, stalling it until the timeout sweep instead of failing it here. + Debug.Assert(false, $"Unhandled handshake user callback type {callback.GetType()}."); + _logger.LogWarning("fd={Fd}: unhandled handshake user callback type; dropping.", fd); + DropHandshake(fd, conn); + return; + } + } +} diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 76c34b921437..0fd77aa4807d 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; /// TLS event pump that handles accept, handshake, and I/O events on a dedicated thread. /// Uses EPOLLEXCLUSIVE on the listen socket to distribute accept load across workers. /// -internal class TlsEventPump : IDisposable +internal partial class TlsEventPump : IDisposable { private readonly ILogger _logger; private readonly int _id; @@ -62,9 +62,9 @@ internal class TlsEventPump : IDisposable // (constructed by tests) can be detected and short-circuited. private readonly TaskCompletionSource _exitSignal = new(TaskCreationOptions.RunContinuationsAsynchronously); private bool _threadStarted; - // Guards the one-time epoll fd close, which happens either in the pump thread's finally (started pump) or - // in StopAndJoinAsync (never-started pump) - never both. Interlocked so a double close can't hit an - // unrelated fd whose number was recycled. + // Guards the one-time close of the fds this pump owns (epoll + wakeup), which happens either once the pump + // thread and its dispatched user callbacks have finished (started pump) or in StopAndJoinAsync (never-started + // pump) - never both. Interlocked so a double close can't hit an unrelated fd whose number was recycled. private int _epollClosed; // Memoizes StopAndJoinAsync so repeated/concurrent stop calls observe one shutdown, not a re-run. private readonly object _stopLock = new(); @@ -73,8 +73,15 @@ internal class TlsEventPump : IDisposable // Listen socket (added with EPOLLEXCLUSIVE). Volatile: written by StopAccepting() (on the disposing // thread) and read by the pump thread in PumpLoop/AcceptConnections. private volatile int _listenFd = -1; + + // Cross-thread wakeup for handshakes suspended on user code. A thread pool thread that finished a user + // callback enqueues its result on _completedCallbacks and writes to this eventfd, which is registered in + // this pump's epoll set, so the pump wakes immediately and resumes the handshake on its own thread. + private readonly int _wakeupFd; + private TlsContext? _tlsContext; private Func? _contextResolver; + private ChannelWriter? _readyConnections; private MemoryPool? _memoryPool; private ILoggerFactory _loggerFactory = NullLoggerFactory.Instance; @@ -108,6 +115,13 @@ internal class TlsEventPump : IDisposable // Cached listen endpoint to avoid getsockname syscall per connection private EndPoint? _listenEndPoint; + // Set by the pump thread as it leaves the loop. Not redundant with _outstandingUserCallbacks: shutdown is + // also reconsidered every time a user callback reports back, which happens throughout normal operation, so + // without this flag the first callback to complete on a healthy pump would observe zero in-flight callbacks + // and close the epoll and wakeup fds underneath the still-running loop. + private volatile bool _loopExited; + private int _shutdownCompleted; + /// /// Lightweight struct to track TLS connections during handshake. /// Uses less memory than ConnectionIoState since we don't need full read/write machinery. @@ -139,6 +153,13 @@ internal struct HandshakingConnection /// The fd's current epoll interest set, mirrored from the last epoll_ctl issued for this handshaking socket. /// public uint CurrentEpollInterest; + /// + /// Non-null while this handshake is suspended waiting on user code running on the thread pool. The fd is + /// de-registered from epoll for that whole window, so the connection generates no pump work, and the + /// instance doubles as the resume token: a completion whose work item is not reference-equal to this one + /// (fd recycled, connection already torn down) is discarded instead of resuming a stale handshake. + /// + public HandshakeUserCallback? PendingUserCallback; } public TlsEventPump(ILogger tlsPumpLogger, int id, TimeSpan handshakeTimeout) @@ -155,6 +176,8 @@ public TlsEventPump(ILogger tlsPumpLogger, int id, TimeSpan handshakeTimeout) throw new InvalidOperationException($"epoll_create1 failed: {Marshal.GetLastWin32Error()}"); } + _wakeupFd = CreateWakeupFd(); + _pumpThread = new Thread(PumpLoop) { Name = $"TlsEventPump-{id}", @@ -179,7 +202,8 @@ public void StartWithListenSocket( long maxWriteBufferSize, Action onFatalError, Action>? clientHelloCallback = null, - ConnectionTracker? connectionTracker = null) + ConnectionTracker? connectionTracker = null, + bool serverCertificateSelectorConfigured = true) { _listenFd = listenFd; ArgumentNullException.ThrowIfNull(tlsContext); @@ -193,6 +217,7 @@ public void StartWithListenSocket( _maxReadBufferSize = maxReadBufferSize; _maxWriteBufferSize = maxWriteBufferSize; _clientHelloCallback = clientHelloCallback; + _connectionTracker = connectionTracker ?? ConnectionTracker.Unlimited; _onFatalError = onFatalError; _listenEndPoint = listenEndPoint; @@ -201,6 +226,9 @@ public void StartWithListenSocket( _connectionIoStateLogger = loggerFactory.CreateLogger(); _directTlsConnectionLogger = loggerFactory.CreateLogger(); + // whether to suspend the handshake on the pump thread while resolving the TLS context (server certificate selection, SNI, etc.) + _contextResolverRunsUserCode = serverCertificateSelectorConfigured || clientHelloCallback is not null; + // Add listen socket with EPOLLEXCLUSIVE - only one worker wakes per connection var ev = new EpollEvent { @@ -404,10 +432,25 @@ private void PumpLoop() continue; } + // Cross-thread wakeup: a user callback finished on the thread pool. Consume the eventfd + // counter here; the queue itself is drained once below, after the whole batch. + if (fd == _wakeupFd) + { + DrainWakeup(); + continue; + } + // Check if this is a handshaking connection if (_handshaking.TryGetValue(fd, out var handshakingConn)) { - TryAdvanceHandshake(fd, handshakingConn); + // A handshake suspended on user code has its fd de-registered from epoll, but an + // event for it may already be sitting in this batch (it was suspended earlier in + // the same iteration). Ignore it: only the resume path may touch the session. + if (handshakingConn.PendingUserCallback is null) + { + TryAdvanceHandshake(fd, handshakingConn); + } + continue; } @@ -415,6 +458,11 @@ private void PumpLoop() HandleConnectionEvent(fd, mask); } + // Resume handshakes whose user callback completed. Done after the event batch so a resumed + // handshake is driven with the freshest state, and unconditionally (not only on a wakeup + // event) so a result that raced the eventfd read is never left parked. + DrainCompletedUserCallbacks(); + // Drop connections whose handshake has taken too long. While a finite handshake timeout is // configured and any handshake is in flight the epoll_wait timeout above is short (see // ComputePollTimeoutMs), so a stalled handshake (e.g. a slow-loris ClientHello) is swept @@ -432,14 +480,16 @@ private void PumpLoop() } finally { - // The thread owns its own teardown: release half-open handshakes, then close the epoll fd it created - // in the constructor, and only then signal exit. Ordering matters - _exitSignal is the proof - // StopAndJoinAsync waits on before the listener frees the TLS contexts and memory pool, so it must be - // the last thing this thread does after every resource access here. In a finally so a stray escape - // (or a break above) still signals, otherwise the awaiter would hang until its timeout and leak. + // The thread owns its own teardown: release half-open handshakes, then hand the remaining teardown + // (closing the fds it created in the constructor and signalling exit) to CompletePumpShutdownIfDrained. + // Ordering matters - _exitSignal is the proof StopAndJoinAsync waits on before the listener frees the + // TLS contexts and memory pool, so it must only fire once this thread is done AND no user callback is + // still running on the thread pool (that callback would otherwise keep running against freed + // resources, and could write to a wakeup fd whose number the OS had already recycled). In a finally so + // a stray escape (or a break above) still signals, otherwise the awaiter would hang until its timeout. ReleasePendingHandshakes(); - CloseEpollFd(); - _exitSignal.TrySetResult(); + _loopExited = true; + CompletePumpShutdownIfDrained(); } } @@ -447,9 +497,19 @@ private void PumpLoop() // ordinary handshake failures so a DirectTlsConnection allocated at NeedsTlsContext is aborted as well. internal void ReleasePendingHandshakes() { - foreach (var connection in _handshaking.Values) + foreach (var kvp in _handshaking) { - ReleaseHandshakeResources(connection); + // A handshake parked on user code must not be torn down yet: its work item may still be running, + // and the certificate and validation sender it was handed belong to this session. Hold it aside and + // release it once every dispatched callback has reported back (see CompletePumpShutdownIfDrained), + // at which point nothing else can observe it. + if (kvp.Value.PendingUserCallback is not null) + { + _handshakesAwaitingCallback.Add(kvp.Value); + continue; + } + + ReleaseHandshakeResources(kvp.Value); } _handshaking.Clear(); @@ -660,17 +720,8 @@ internal virtual void ProcessAcceptedSocket(Socket accepted) } // Register client socket with epoll for handshake events - var ev = new EpollEvent + if (!TryArmHandshakeInterest(clientFd, DefaultEpollInterest)) { - Events = DefaultEpollInterest, - Data = new EpollData { Fd = clientFd } - }; - - int result = NativeTls.epoll_ctl(_epollFd, NativeTls.EPOLL_CTL_ADD, clientFd, ref ev); - if (result < 0) - { - int errno = Marshal.GetLastWin32Error(); - _logger.LogWarning("epoll_ctl ADD failed for handshaking fd={Fd}: errno={Errno}", clientFd, errno); session.Dispose(); _connectionTracker.ReleaseHandshake(); return; @@ -736,133 +787,43 @@ private void TryAdvanceHandshake( if (status == TlsOperationStatus.Complete) { - // Handshake complete: validate any client certificate, build the connection, and promote the fd from handshaking to established. - X509Certificate2? clientCertificate = null; - var earlyConnection = conn.Connection; - ConnectionIoState connectionState; - DirectTlsConnection directConnection; - - try - { - // Mutual TLS (client certificate) handling. The endpoint opts in via - // HttpsConnectionAdapterOptions.ClientCertificateMode (Allow/Require), which makes - // CreateStreamTransportOptions set ClientCertificateRequired and install a - // RemoteCertificateValidationCallback; conn.ClientCertificateValidation carries that callback - // (null for server-auth-only endpoints, which skip this block entirely). The Linux fd fast - // handshake path reports Complete directly - it does not surface NeedsCertificateValidation like - // the buffered PALs do, OpenSSL only enforces SSL_VERIFY_PEER (not FAIL_IF_NO_PEER_CERT), and the - // fd read/write fast paths bypass the runtime's pending-validation fault. So the runtime cannot - // enforce the accept/reject decision on this path. The transport runs the endpoint's validation - // callback here, records the verdict on the session, and tears down rejected connections before - // they are ever surfaced to Kestrel. - if (conn.ClientCertificateValidation is { } validateClientCertificate) - { - // The peer's leaf certificate, or null when the client presented none. On the fd fast path - // this is the runtime's pending external-validation certificate. Intermediates are only - // fetched when a leaf is present (they feed the chain's ExtraStore). The chain build, - // policy, and callback invocation live in ClientCertificateValidator so they can be unit - // tested without epoll or a live session - see its remarks for why AIA downloads are - // disabled on this pump thread. - var presentedCertificate = conn.Session.GetRemoteCertificate(); - var intermediates = presentedCertificate is null ? null : conn.Session.GetRemoteCertificates(); - - var accepted = ClientCertificateValidator.Validate(conn.Session, presentedCertificate, intermediates, validateClientCertificate); - - if (!accepted) - { - _logger.LogDebug("Client certificate rejected for fd={Fd} (presented={Presented}).", fd, presentedCertificate is not null); - DropHandshake(fd, conn); - return; - } - - // Record the accepted result so the runtime promotes the leaf into its canonical remote-cert - // slot and clears its pending-validation state. - try - { - conn.Session.SetRemoteCertificateValidationResult(SslPolicyErrors.None); - } - catch (InvalidOperationException) - { - // Validation was already resolved (e.g. a buffered PAL that surfaced - // NeedsCertificateValidation before reaching Complete). - } - - // Surface the accepted certificate to Kestrel via ITlsConnectionFeature. This is the same - // instance the runtime just promoted into its canonical remote-cert slot (on the accept path - // SetRemoteCertificateValidationResult moves _externalPendingCert into _remoteCertificate - // without reallocating), and null when the client presented none on an AllowCertificate - // endpoint - so we reuse presentedCertificate instead of re-reading it from the session. - clientCertificate = presentedCertificate; - } - - // Both are set before the pump thread starts and never cleared, so this is unreachable. - if (_readyConnections is null || _memoryPool is null) - { - Debug.Assert(false, "Handshake completed before the pump was initialized."); - _logger.LogWarning("fd={Fd}: handshake completed before the pump was initialized; dropping.", fd); - DropHandshake(fd, conn); - return; - } - - // Reuse the DirectTlsConnection allocated early for the ClientHello listener (at - // NeedsTlsContext), if any, so the connection surfaced to Kestrel keeps the same - // ConnectionId the listener already observed. Otherwise create both now. Its ConnectionIoState - // has Pump already set (early path) or set here (default path). - connectionState = earlyConnection?.ConnectionState - ?? new ConnectionIoState(fd, conn.Session, _connectionIoStateLogger) { Pump = this }; - connectionState.SetHandshakeComplete(); - - // Create DirectTlsConnection using fd directly (no Socket wrapper). - // This avoids ~5+ syscalls per connection (fstat, getsockopt, fcntl, etc.) - if (earlyConnection is not null) - { - // Promote the early connection: publish the ALPN protocol and validated client cert - // that were unknown when it was allocated (the ClientHello listener has already run). - directConnection = earlyConnection; - directConnection.CompleteHandshake(conn.Session.NegotiatedApplicationProtocol, clientCertificate); - } - else - { - directConnection = new DirectTlsConnection( - connectionState, - this, - _listenEndPoint, // Cached - avoids getsockname syscall - conn.RemoteEndPoint, // Captured from Socket.RemoteEndPoint at accept time - _memoryPool, - _maxReadBufferSize, - _maxWriteBufferSize, - _directTlsConnectionLogger!, - negotiatedApplicationProtocol: conn.Session.NegotiatedApplicationProtocol, - clientCertificate: clientCertificate); // Non-null only when the peer presented a client cert (mTLS) - } - } - catch (Exception ex) + // Mutual TLS (client certificate) handling. The endpoint opts in via + // HttpsConnectionAdapterOptions.ClientCertificateMode (Allow/Require), which makes + // CreateStreamTransportOptions set ClientCertificateRequired and install a + // RemoteCertificateValidationCallback; conn.ClientCertificateValidation carries that callback + // (null for server-auth-only endpoints, which skip this block entirely). The Linux fd fast + // handshake path reports Complete directly - it does not surface NeedsCertificateValidation like + // the buffered PALs do, OpenSSL only enforces SSL_VERIFY_PEER (not FAIL_IF_NO_PEER_CERT), and the + // fd read/write fast paths bypass the runtime's pending-validation fault. So the runtime cannot + // enforce the accept/reject decision on this path. The transport runs the endpoint's validation + // callback itself, records the verdict on the session, and tears down rejected connections before + // they are ever surfaced to Kestrel. + // + // The certificates are read from the session here (pump thread only), but the chain build and the + // endpoint's callback are user-controlled work, so they are suspended onto the thread pool and the + // handshake resumes in ResumeSuspendedHandshake. + if (conn.ClientCertificateValidation is { } validateClientCertificate) { - // Post-handshake activities failed (like cert validation). De-register fd here - _logger.LogDebug(ex, "Completing handshake threw for fd={Fd}", fd); - DropHandshake(fd, conn); - return; - } + // The peer's leaf certificate, or null when the client presented none. On the fd fast path + // this is the runtime's pending external-validation certificate. Intermediates are only + // fetched when a leaf is present (they feed the chain's ExtraStore). + var presentedCertificate = conn.Session.GetRemoteCertificate(); + var intermediates = presentedCertificate is null ? null : conn.Session.GetRemoteCertificates(); - if (!PromoteHandshakeToConnection(fd, connectionState)) - { - // The socket could not be re-armed to the established interest set, so don't surface a - // connection whose epoll interest is wrong (it would spin the pump on a stuck EPOLLOUT). It was - // built but never Started and the fd is still registered as handshaking, so tear it down on - // that path. - DropCompletedHandshake(fd, directConnection); + var validationCallback = new ValidateClientCertificateCallback( + this, + fd, + conn.Connection, + conn.Session, + presentedCertificate, + intermediates, + validateClientCertificate); + + SuspendHandshake(fd, ref conn, validationCallback); return; } - directConnection.Start(); - - if (!_readyConnections.TryWrite(directConnection)) - { - // Channel closed (shutting down) - dispose connection - _connectionTracker.ReleaseHandshake(); - _ = DisposeAbandonedConnectionAsync(directConnection); - } - + CompleteHandshake(fd, conn, clientCertificate: null); return; } @@ -904,9 +865,12 @@ private void TryAdvanceHandshake( if (status == TlsOperationStatus.NeedsTlsContext) { - // Deferred SNI flow: the session parsed the ClientHello and needs the real - // per-host TLS context before it can continue. Resolve it from the SNI host - // name and hand it back via SetContext, then re-drive the handshake. + // Deferred SNI flow: the session parsed the ClientHello and needs the real per-host TLS context + // before it can continue. Both the ClientHello listener and the certificate selector are user code + // that can block for an unbounded time, so the pump copies everything they need off the session + // here and then suspends the handshake: the fd leaves this pump's epoll set and the callbacks run + // on the thread pool. ResumeSuspendedHandshake installs the resolved context and re-drives the + // handshake back on the pump thread. if (_contextResolver is null) { // No selector configured but the session still deferred — misconfiguration. @@ -915,14 +879,25 @@ private void TryAdvanceHandshake( return; } + if (!_contextResolverRunsUserCode) + { + // Neither a certificate selector nor a ClientHello listener is configured, so resolution cannot + // reach user code and there is nothing to move off the event loop. Resolve inline: this keeps + // the fd armed and skips the suspension, the thread-pool hop and the early DirectTlsConnection + // allocation the suspending path needs to give user code a stable ConnectionContext. + ResolveTlsContextInline(fd, conn); + return; + } + // Allocate the DirectTlsConnection now (its handshake is not yet complete) so both the - // certificate selector below and the optional ClientHello listener see the same + // certificate selector and the optional ClientHello listener see the same // ConnectionContext / ConnectionId that will later serve the request; it is reused in the // Complete branch. The Connection-is-null guard makes this run exactly once even if the // handshake needs several more epoll round-trips. Because the bootstrap context carries no // credentials, every connection reaches NeedsTlsContext, so this early allocation is net-neutral // (moved from Complete, not added). - if (conn.Connection is null && _memoryPool is not null) + bool firstSuspension = conn.Connection is null; + if (firstSuspension && _memoryPool is not null) { var earlyState = new ConnectionIoState(fd, conn.Session, _connectionIoStateLogger) { Pump = this }; var earlyConnection = new DirectTlsConnection( @@ -936,42 +911,44 @@ private void TryAdvanceHandshake( _directTlsConnectionLogger!); conn.Connection = earlyConnection; _handshaking[fd] = conn; + } - // Fire the optional ClientHello listener as early as possible - the session has parsed the - // ClientHello (which is what produced this NeedsTlsContext suspension), but the real context - // has not been installed yet and OpenSSL has not run the expensive key exchange / certificate - // signing. The listener is observable-only today; it does not decide whether the handshake - // proceeds - but a throwing callback fails the connection (matching the socket-transport - // TlsListener) rather than being swallowed. - if (_clientHelloCallback is not null && !InvokeClientHelloListener(earlyConnection, conn.Session)) - { - DropHandshake(fd, conn); - return; - } + // Copy the parsed ClientHello record out of the session while we are still on the pump thread; the + // listener itself runs on the thread pool against this copy. Only on the first suspension, so a + // handshake that needs several context round-trips still fires the listener exactly once. + byte[]? clientHelloBuffer = null; + int clientHelloLength = 0; + if (firstSuspension && _clientHelloCallback is not null && conn.Connection is not null && + !TryCaptureClientHello(conn.Session, out clientHelloBuffer, out clientHelloLength)) + { + _logger.LogDebug("Capturing the ClientHello record failed for fd={Fd}; dropping connection.", fd); + DropHandshake(fd, conn); + return; } + string? targetHostName; try { - var (resolvedContext, clientCertificateValidation) = _contextResolver(conn.Connection, conn.Session.TargetHostName); - conn.Session.SetContext(resolvedContext); - - // The endpoint's client-certificate validation callback is resolved with the context. Persist - // it on the handshaking entry so the Complete branch can drive mTLS validation, even if the - // handshake needs several more epoll round-trips (each re-reads _handshaking[fd]). - conn.ClientCertificateValidation = clientCertificateValidation; - _handshaking[fd] = conn; + targetHostName = conn.Session.TargetHostName; } catch (Exception ex) { - // A bad SNI host, a selector that returned no certificate, or a credential - // acquisition failure must drop only this connection, not the pump. - _logger.LogDebug(ex, "SNI certificate resolution failed for fd={Fd}", fd); + _logger.LogDebug(ex, "Reading the SNI host name failed for fd={Fd}", fd); DropHandshake(fd, conn); return; } - // Real context is now set; continue the handshake immediately. - TryAdvanceHandshake(fd, conn); + var contextCallback = new ResolveTlsContextCallback( + this, + fd, + conn.Connection, + targetHostName, + _contextResolver, + clientHelloBuffer is null ? null : _clientHelloCallback, + clientHelloBuffer, + clientHelloLength); + + SuspendHandshake(fd, ref conn, contextCallback); return; } @@ -980,6 +957,151 @@ private void TryAdvanceHandshake( DropHandshake(fd, conn); } + // Resolves the TLS context on the pump thread and drives the handshake straight on. Only valid when the + // resolver provably runs no user code (see _contextResolverRunsUserCode): it is the transport's own lambda + // over a static certificate and a per-certificate TlsContext cache, so the only unbounded work is creating + // the context on the first connection. Mirrors the ResolveTlsContextCallback arm of ResumeSuspendedHandshake, + // minus the re-arm (the fd was never de-armed because the handshake never suspended). + private void ResolveTlsContextInline(int fd, HandshakingConnection conn) + { + Debug.Assert(_contextResolver is not null, "ResolveTlsContextInline ran without a certificate resolver."); + + TlsContext context; + RemoteCertificateValidationCallback? clientCertificateValidation; + try + { + // conn.Connection is null here and stays null: with no selector the resolver ignores it, and with no + // ClientHello listener nothing else needs a ConnectionContext this early. CompleteHandshake + // allocates the DirectTlsConnection once the handshake is done. + (context, clientCertificateValidation) = _contextResolver(conn.Connection, conn.Session.TargetHostName); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Resolving the TLS context failed for fd={Fd}; dropping connection.", fd); + DropHandshake(fd, conn); + return; + } + + try + { + conn.Session.SetContext(context); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Installing the resolved TLS context failed for fd={Fd}", fd); + DropHandshake(fd, conn); + return; + } + + // Persist the validation callback that came back with the context so the Complete branch can drive mTLS + // validation even if the handshake needs several more epoll round-trips (each re-reads _handshaking[fd]). + conn.ClientCertificateValidation = clientCertificateValidation; + _handshaking[fd] = conn; + + TryAdvanceHandshake(fd, conn); + } + + // Completes a handshake that has passed every user-code gate: builds (or promotes) the DirectTlsConnection, + // moves the fd from handshaking to established, and hands the connection to Kestrel. Split out of + // TryAdvanceHandshake so the client-certificate resume path can reach it directly without re-driving the + // native handshake. Runs on the pump thread only. + private void CompleteHandshake(int fd, HandshakingConnection conn, X509Certificate2? clientCertificate) + { + var earlyConnection = conn.Connection; + ConnectionIoState connectionState; + DirectTlsConnection directConnection; + + try + { + // Both are set before the pump thread starts and never cleared, so this is unreachable. + if (_readyConnections is null || _memoryPool is null) + { + Debug.Assert(false, "Handshake completed before the pump was initialized."); + _logger.LogWarning("fd={Fd}: handshake completed before the pump was initialized; dropping.", fd); + DropHandshake(fd, conn); + return; + } + + // Reuse the DirectTlsConnection allocated early for the ClientHello listener (at + // NeedsTlsContext), if any, so the connection surfaced to Kestrel keeps the same + // ConnectionId the listener already observed. Otherwise create both now. Its ConnectionIoState + // has Pump already set (early path) or set here (default path). + connectionState = earlyConnection?.ConnectionState + ?? new ConnectionIoState(fd, conn.Session, _connectionIoStateLogger) { Pump = this }; + connectionState.SetHandshakeComplete(); + + // Create DirectTlsConnection using fd directly (no Socket wrapper). + // This avoids ~5+ syscalls per connection (fstat, getsockopt, fcntl, etc.) + if (earlyConnection is not null) + { + // Promote the early connection: publish the ALPN protocol and validated client cert + // that were unknown when it was allocated (the ClientHello listener has already run). + directConnection = earlyConnection; + directConnection.CompleteHandshake(conn.Session.NegotiatedApplicationProtocol, clientCertificate); + } + else + { + directConnection = new DirectTlsConnection( + connectionState, + this, + _listenEndPoint, // Cached - avoids getsockname syscall + conn.RemoteEndPoint, // Captured from Socket.RemoteEndPoint at accept time + _memoryPool, + _maxReadBufferSize, + _maxWriteBufferSize, + _directTlsConnectionLogger!, + negotiatedApplicationProtocol: conn.Session.NegotiatedApplicationProtocol, + clientCertificate: clientCertificate); // Non-null only when the peer presented a client cert (mTLS) + } + } + catch (Exception ex) + { + // Post-handshake activities failed. De-register fd here + _logger.LogDebug(ex, "Completing handshake threw for fd={Fd}", fd); + DropHandshake(fd, conn); + return; + } + + if (!PromoteHandshakeToConnection(fd, connectionState)) + { + // The socket could not be re-armed to the established interest set, so don't surface a + // connection whose epoll interest is wrong (it would spin the pump on a stuck EPOLLOUT). It was + // built but never Started and the fd is still registered as handshaking, so tear it down on + // that path. + DropCompletedHandshake(fd, directConnection); + return; + } + + directConnection.Start(); + + if (!_readyConnections.TryWrite(directConnection)) + { + // Channel closed (shutting down) - dispose connection + _connectionTracker.ReleaseHandshake(); + _ = DisposeAbandonedConnectionAsync(directConnection); + } + } + + // Registers a handshaking fd in this pump's epoll set (EPOLL_CTL_ADD). Used when a connection is first + // accepted and when a suspended handshake is resumed. internal virtual so tests can observe/reject the + // registration without a live epoll instance. + internal virtual bool TryArmHandshakeInterest(int fd, uint events) + { + var ev = new EpollEvent + { + Events = events, + Data = new EpollData { Fd = fd } + }; + + if (NativeTls.epoll_ctl(_epollFd, NativeTls.EPOLL_CTL_ADD, fd, ref ev) < 0) + { + _logger.LogWarning("epoll_ctl ADD failed for handshaking fd={Fd}: errno={Errno}", fd, Marshal.GetLastWin32Error()); + return false; + } + + return true; + } + // Adjusts an in-progress handshake's epoll interest set for a NeedMoreData / DestinationTooSmall step. // Established sockets are level-triggered, so EPOLLOUT must be armed only while there is pending handshake // output the socket send buffer could not accept (DestinationTooSmall), and cleared the moment the @@ -1110,6 +1232,15 @@ internal int SweepExpiredHandshakes(long nowTimestamp) int count = 0; foreach (var kvp in _handshaking) { + // A handshake parked on user code is not stalled on the peer, and its work item may still be + // running on the thread pool - dropping it here would release the connection underneath live user + // code. It is swept on a later pass once it has resumed (its deadline is not extended, so an + // over-long callback still costs the connection its handshake budget). + if (kvp.Value.PendingUserCallback is not null) + { + continue; + } + long deadline = kvp.Value.HandshakeDeadlineTimestamp; if (deadline != long.MaxValue && deadline <= nowTimestamp) { @@ -1134,52 +1265,50 @@ internal int SweepExpiredHandshakes(long nowTimestamp) return count; } - // Copies the raw parsed ClientHello record from the session and hands it to the - // UseTlsClientHelloListener callback. Runs synchronously on the pump (epoll) thread at - // NeedsTlsContext (before the handshake's key exchange), so the callback must not block. + // Copies the raw parsed ClientHello record out of the session so the UseTlsClientHelloListener callback can + // run off the pump thread. Must be called on the pump thread (it touches the session); the copy is then + // handed to a HandshakeUserCallback, which invokes the listener on the thread pool and returns the buffer + // to the pool afterwards - so the callback still only sees a transient buffer, matching the + // socket-transport TlsListener contract. // - // Returns true when the handshake should continue, false when the caller must drop the connection. - // Any failure - the session being unable to produce the ClientHello record, or the user callback throwing - - // fails the connection, matching the socket-transport TlsListener where the ClientHello callback is not - // guarded and an exception fails the connection rather than being swallowed. A session that simply has no - // ClientHello bytes to hand over (a non-exceptional empty result) is not a failure and continues. - private bool InvokeClientHelloListener(DirectTlsConnection connection, TlsSocketSession session) + // Returns true when the handshake should continue - including when the session simply has no ClientHello + // bytes to hand over, a non-exceptional empty result, in which case is null. + // Returns false when the session could not produce the record, so the caller drops the connection (the + // socket-transport TlsListener also fails the connection rather than swallowing this). + private static bool TryCaptureClientHello(TlsSocketSession session, out byte[]? buffer, out int length) { - byte[]? buffer = null; + buffer = null; + length = 0; + try { - var length = session.GetClientHelloLength(); - if (length <= 0) + var helloLength = session.GetClientHelloLength(); + if (helloLength <= 0) { - // Nothing was captured; the observe-only listener has nothing to hand over. Not a failure. return true; } - buffer = ArrayPool.Shared.Rent(length); - if (!session.TryGetClientHelloBytes(buffer.AsSpan(0, length), out var written) || written <= 0) + var rented = ArrayPool.Shared.Rent(helloLength); + if (!session.TryGetClientHelloBytes(rented.AsSpan(0, helloLength), out var written) || written <= 0) { + ArrayPool.Shared.Return(rented); return true; } - // The buffer is only valid for the duration of this synchronous call; it is returned to - // the pool immediately afterwards. This matches the transient-buffer contract of the - // socket-transport TlsListener middleware. - _clientHelloCallback!(connection, new ReadOnlySequence(buffer, 0, written)); + buffer = rented; + length = written; return true; } - catch (Exception ex) - { - // Either the session invocation failed (unable to read the ClientHello) or the user callback threw. - // Both fail the connection, matching the socket-transport TlsListener. - _logger.LogDebug(ex, "TLS ClientHello listener failed for fd={Fd}; dropping connection.", connection.ConnectionState.Fd); - return false; - } - finally + catch { if (buffer is not null) { ArrayPool.Shared.Return(buffer); + buffer = null; + length = 0; } + + return false; } } @@ -1197,11 +1326,13 @@ private bool InvokeClientHelloListener(DirectTlsConnection connection, TlsSocket /// /// Bounds the wait; on cancellation the method reports the thread as still running. /// - /// if the pump thread has exited (or was never started), meaning it can no longer - /// touch the epoll fd, the TLS contexts, or the memory pool, so the owner may safely release them. - /// if the thread is still running when the wait is canceled - for example stuck in a - /// blocking user callback (certificate selector, certificate validation, or ClientHello listener). In that - /// case the caller MUST NOT release any resource the pump can still reach, or it risks a use-after-free. + /// if the pump thread has exited (or was never started) and no user callback it + /// dispatched is still running, meaning nothing can touch the epoll fd, the TLS contexts, or the memory + /// pool any more, so the owner may safely release them. + /// if the thread is still running, or a user callback (certificate selector, + /// certificate validation, or ClientHello listener) it queued to the thread pool is still blocked, when the + /// wait is canceled. In that case the caller MUST NOT release any resource the pump can still reach, or it + /// risks a use-after-free. /// public Task StopAndJoinAsync(CancellationToken cancellationToken) { @@ -1220,34 +1351,37 @@ private async Task StopAndJoinCoreAsync(CancellationToken cancellationToke if (!_threadStarted) { - // The thread never ran, so PumpLoop's finally will never fire: close the epoll fd here instead. - // Nothing else (contexts, pool) was ever handed to the loop, so this is all the cleanup needed. - CloseEpollFd(); + // The thread never ran, so PumpLoop's finally will never fire: close the pump's fds here instead. + // Nothing else (contexts, pool) was ever handed to the loop, and no user callback can be in flight, + // so this is all the cleanup needed. + CloseOwnedFds(); return true; } try { - // The pump thread completes _exitSignal only after it has released its handshakes and closed its - // own epoll fd, so returning here proves it can no longer reach any owner-shared resource. + // The pump thread completes _exitSignal only after it has released its handshakes, every user + // callback it dispatched has reported back, and its fds are closed - so returning here proves + // nothing can reach an owner-shared resource any more. await _exitSignal.Task.WaitAsync(cancellationToken).ConfigureAwait(false); return true; } catch (OperationCanceledException) { - // The wait was canceled - a user callback on the pump thread is blocking. It still owns its - // epoll fd and may still reach the TLS contexts and the memory pool, so leave every resource intact - // (the thread closes its own epoll fd if the callback ever returns; the listener leaks the - // contexts/pool). The OS reclaims all of it at process exit; freeing it now would be a use-after-free. - _logger.LogWarning("Pump {Id} thread did not exit (a TLS certificate, validation, or ClientHello callback may be blocking); deferring resource release to avoid a use-after-free.", _id); + // The wait was canceled - the pump thread, or a user callback it dispatched to the thread pool, is + // still running. It may still reach the TLS contexts and the memory pool, so leave every resource + // intact (the fds are closed once everything has finished; the listener leaks the contexts/pool). The OS + // reclaims all of it at process exit; freeing it now would be a use-after-free. + _logger.LogWarning("Pump {Id} did not finish (a TLS certificate, validation, or ClientHello callback may be blocking); deferring resource release to avoid a use-after-free.", _id); return false; } } - // Closes the pump-owned epoll fd exactly once. Called by the pump thread in PumpLoop's finally for a started - // pump, or by StopAndJoinCoreAsync for a never-started one - the Interlocked guard makes a stray double call - // a no-op so it can never close an unrelated fd whose number was recycled. - private void CloseEpollFd() + // Closes the epoll and wakeup fds this pump created in its constructor, exactly once. Reached from the + // drained-shutdown path for a started pump, or from StopAndJoinCoreAsync for a never-started one - the + // Interlocked guard makes a stray double call a no-op so it can never close an unrelated fd whose number + // was recycled. + private void CloseOwnedFds() { if (Interlocked.Exchange(ref _epollClosed, 1) != 0) { @@ -1257,6 +1391,11 @@ private void CloseEpollFd() // close() is intentionally not retried: on Linux the fd is released even when close returns EINTR, so a // retry could close an unrelated fd. A failure here (realistically only EBADF) signals a lifecycle bug // rather than a leak, so log it for diagnostics but don't act on it. + if (NativeTls.close(_wakeupFd) < 0) + { + _logger.LogDebug("close(wakeupFd={WakeupFd}) failed: errno={Errno}", _wakeupFd, Marshal.GetLastWin32Error()); + } + if (NativeTls.close(_epollFd) < 0) { _logger.LogDebug("close(epollFd={EpollFd}) failed: errno={Errno}", _epollFd, Marshal.GetLastWin32Error()); diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPumpPool.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPumpPool.cs index 0ae3f157e709..606b11ef789f 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPumpPool.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPumpPool.cs @@ -57,7 +57,8 @@ public void StartWithListenSocket( long maxWriteBufferSize, Action onFatalError, Action>? clientHelloCallback = null, - ConnectionTracker? connectionTracker = null) + ConnectionTracker? connectionTracker = null, + bool serverCertificateSelectorConfigured = true) { foreach (var pump in _pumps) { @@ -74,7 +75,8 @@ public void StartWithListenSocket( maxWriteBufferSize, onFatalError, clientHelloCallback, - connectionTracker); + connectionTracker, + serverCertificateSelectorConfigured); } } diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs new file mode 100644 index 000000000000..2c224ac809d9 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs @@ -0,0 +1,132 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Buffers; +using System.Net; +using System.Net.Security; +using System.Net.Sockets; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; + +/// +/// Covers the complement of DirectTlsUserCallbackDispatchTests: when resolving the TLS context provably +/// cannot reach user code - no endpoint certificate selector and no ClientHello listener - the pump must resolve +/// inline instead of suspending the handshake onto the thread pool. The bootstrap context carries no credentials, +/// so every connection still reaches NeedsTlsContext; the question these tests answer is only which thread +/// the resolver runs on. Pump threads are named TlsEventPump-{id}, which makes that directly observable. +/// +public class DirectTlsContextResolverFastPathTests +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(30); + + private const string PumpThreadNamePrefix = "TlsEventPump-"; + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ContextResolver_WithoutSelectorOrClientHelloListener_RunsOnThePumpThread() + { + // The fast path: a static certificate behind the transport's own resolver. Nothing here can block for an + // unbounded time, so suspending would buy nothing and cost a thread-pool round trip per connection. + var threadName = await CaptureResolverThreadNameAsync(serverCertificateSelectorConfigured: false); + + Assert.StartsWith(PumpThreadNamePrefix, threadName); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ContextResolver_WithCertificateSelector_RunsOffThePumpThread() + { + // The endpoint supplied a selector, so the resolver closes over user code and must be suspended. + var threadName = await CaptureResolverThreadNameAsync(serverCertificateSelectorConfigured: true); + + Assert.DoesNotContain(PumpThreadNamePrefix, threadName ?? string.Empty); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ContextResolver_WithClientHelloListenerOnly_RunsOffThePumpThread() + { + // No selector, but a ClientHello listener - which the pump runs immediately before resolving the context. + // The pump must combine that with the selector flag rather than trusting the flag alone, otherwise a + // listener-only endpoint would silently run user code on the event loop. + var threadName = await CaptureResolverThreadNameAsync( + serverCertificateSelectorConfigured: false, + withClientHelloListener: true); + + Assert.DoesNotContain(PumpThreadNamePrefix, threadName ?? string.Empty); + } + + // Drives one real TLS handshake against a listener whose context resolver records the thread it was invoked + // on, and returns that thread's name. Only the resolver call is awaited: whether the handshake goes on to + // succeed is irrelevant to which thread resolved the context. + private static async Task CaptureResolverThreadNameAsync( + bool serverCertificateSelectorConfigured, + bool withClientHelloListener = false) + { + using var certificate = TestResources.GetTestCertificate(); + using var bootstrapContext = TlsContext.CreateServer(new SslServerAuthenticationOptions()); + using var serverContext = TlsContext.CreateServer(new SslServerAuthenticationOptions + { + ServerCertificate = certificate, + }); + + var resolverThreadName = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + var listener = new DirectTlsConnectionListener( + NullLoggerFactory.Instance, + bootstrapContext, + (connection, hostName) => + { + resolverThreadName.TrySetResult(Thread.CurrentThread.Name); + return (serverContext, null); + }, + new TlsEventPumpPool(pumpCount: 1, NullLoggerFactory.Instance), + new IPEndPoint(IPAddress.Loopback, 0), + new DirectTlsTransportOptions(), + MemoryPool.Shared, + new TestHostApplicationLifetime(), + withClientHelloListener ? static (_, _) => { } : null, + ownedServerContexts: null, + serverCertificateSelectorConfigured); + + listener.Bind(); + + // Keep the ready-connection channel drained so a completed handshake does not park a connection for the + // lifetime of the test. Ends on its own when DisposeAsync completes the channel. + var drain = Task.Run(async () => + { + while (await listener.AcceptAsync() is { } connection) + { + await connection.DisposeAsync(); + } + }); + + try + { + using var client = new TcpClient(); + await client.ConnectAsync((IPEndPoint)listener.EndPoint); + + using var sslStream = new SslStream(client.GetStream(), leaveInnerStreamOpen: false, (_, _, _, _) => true); + var handshake = sslStream.AuthenticateAsClientAsync("localhost"); + + var threadName = await resolverThreadName.Task.WaitAsync(Timeout); + + // Observe the handshake so a failure after the resolver ran does not surface as an unobserved + // exception; the assertion under test is about the resolver's thread, not the handshake's outcome. + await handshake.ContinueWith(static _ => { }, TaskScheduler.Default); + + return threadName; + } + finally + { + await listener.DisposeAsync(); + await drain.ContinueWith(static _ => { }, TaskScheduler.Default); + } + } +} diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs new file mode 100644 index 000000000000..3b753cc1eba9 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs @@ -0,0 +1,278 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#pragma warning disable ASPNETCORE_DIRECTTLS_001 // Experimental API + +using System.Buffers; +using System.Net; +using System.Net.Security; +using System.Net.Sockets; +using System.Security.Cryptography.X509Certificates; +using System.Text; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.Extensions.Hosting; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Tests; + +/// +/// Covers the guarantee that user-supplied handshake callbacks - the server-certificate selector and the +/// ClientHello listener - never run on a pump's epoll thread. Every test pins the transport to a single pump +/// (WorkerCount = 1) so all connections are provably owned by the same event loop: if a callback were +/// still invoked inline, one slow or throwing connection would stall or break every other connection here. +/// +public class DirectTlsUserCallbackDispatchTests +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(30); + + // Deliberately much shorter than Timeout: this is what "the pump is not stalled" means. A connection that + // has to wait for the blocked callback would need the full release delay, which never arrives on its own. + private static readonly TimeSpan ProgressTimeout = TimeSpan.FromSeconds(15); + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task BlockingCertificateSelector_DoesNotStallHandshakesOnTheSamePump() + { + var certificate = TestResources.GetTestCertificate(); + using var selectorEntered = new SemaphoreSlim(0); + using var releaseSelector = new ManualResetEventSlim(false); + var blockedOnce = 0; + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificateSelector = (connection, hostName) => + { + // Only the first connection blocks; every later one resolves immediately. + if (Interlocked.Exchange(ref blockedOnce, 1) == 0) + { + selectorEntered.Release(); + releaseSelector.Wait(Timeout); + } + + return certificate; + }; + + using var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + var port = host.GetPort(); + + try + { + var blockedHandshake = ConnectAsync(port); + Assert.True(await selectorEntered.WaitAsync(Timeout), "The certificate selector was never entered."); + + // The pump that owns the blocked handshake also owns this one. It must accept it and drive its + // handshake to completion while the first connection's user code is still parked. + using var progressing = await ConnectAsync(port).WaitAsync(ProgressTimeout); + Assert.Contains("200 OK", await SendRequestAsync(progressing)); + + releaseSelector.Set(); + + // The parked handshake resumes on the pump once its callback returns. + using var resumed = await blockedHandshake.WaitAsync(Timeout); + Assert.Contains("200 OK", await SendRequestAsync(resumed)); + } + finally + { + releaseSelector.Set(); + } + + await host.StopAsync().WaitAsync(Timeout); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task BlockingClientHelloCallback_DoesNotStallHandshakesOnTheSamePump() + { + using var callbackEntered = new SemaphoreSlim(0); + using var releaseCallback = new ManualResetEventSlim(false); + var blockedOnce = 0; + var observedClientHelloLength = 0L; + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificate = TestResources.GetTestCertificate(); + endpoint.Options.TlsClientHelloBytesCallback = (connection, clientHelloBytes) => + { + Interlocked.Exchange(ref observedClientHelloLength, clientHelloBytes.Length); + + if (Interlocked.Exchange(ref blockedOnce, 1) == 0) + { + callbackEntered.Release(); + releaseCallback.Wait(Timeout); + } + }; + + using var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + var port = host.GetPort(); + + try + { + var blockedHandshake = ConnectAsync(port); + Assert.True(await callbackEntered.WaitAsync(Timeout), "The ClientHello callback was never entered."); + + using var progressing = await ConnectAsync(port).WaitAsync(ProgressTimeout); + Assert.Contains("200 OK", await SendRequestAsync(progressing)); + + releaseCallback.Set(); + + using var resumed = await blockedHandshake.WaitAsync(Timeout); + Assert.Contains("200 OK", await SendRequestAsync(resumed)); + } + finally + { + releaseCallback.Set(); + } + + // The callback still sees the real ClientHello record even though it now runs off the pump against a + // copy taken there. + Assert.True(Interlocked.Read(ref observedClientHelloLength) > 0, "The ClientHello callback saw no bytes."); + + await host.StopAsync().WaitAsync(Timeout); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ThrowingCertificateSelector_FailsOnlyThatConnection() + { + var certificate = TestResources.GetTestCertificate(); + var throwOnce = 1; + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificateSelector = (connection, hostName) => + Interlocked.Exchange(ref throwOnce, 0) == 1 + ? throw new InvalidOperationException("Certificate selection failed.") + : certificate; + + using var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + var port = host.GetPort(); + + // The exception escaped user code on a thread pool thread; it must fail this handshake rather than + // reach the pump (which would take the process, and every other connection, down with it). + await Assert.ThrowsAnyAsync(() => ConnectAsync(port).WaitAsync(ProgressTimeout)); + + using var healthy = await ConnectAsync(port).WaitAsync(ProgressTimeout); + Assert.Contains("200 OK", await SendRequestAsync(healthy)); + + await host.StopAsync().WaitAsync(Timeout); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ThrowingClientHelloCallback_FailsOnlyThatConnection() + { + var throwOnce = 1; + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificate = TestResources.GetTestCertificate(); + endpoint.Options.TlsClientHelloBytesCallback = (connection, clientHelloBytes) => + { + if (Interlocked.Exchange(ref throwOnce, 0) == 1) + { + throw new InvalidOperationException("ClientHello inspection failed."); + } + }; + + using var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + var port = host.GetPort(); + + await Assert.ThrowsAnyAsync(() => ConnectAsync(port).WaitAsync(ProgressTimeout)); + + using var healthy = await ConnectAsync(port).WaitAsync(ProgressTimeout); + Assert.Contains("200 OK", await SendRequestAsync(healthy)); + + await host.StopAsync().WaitAsync(Timeout); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task ShutdownWhileCallbackIsParked_CompletesWithoutLeakingTheConnection() + { + var certificate = TestResources.GetTestCertificate(); + using var selectorEntered = new SemaphoreSlim(0); + using var releaseSelector = new ManualResetEventSlim(false); + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificateSelector = (connection, hostName) => + { + selectorEntered.Release(); + releaseSelector.Wait(Timeout); + return certificate; + }; + + var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + + try + { + _ = ConnectAsync(host.GetPort()); + Assert.True(await selectorEntered.WaitAsync(Timeout), "The certificate selector was never entered."); + + // Tear the server down while the handshake is parked on user code. The pump loop must exit, and the + // parked handshake's resources must be released once its callback reports back - without the resume + // path touching a session that shutdown already disposed. + var stop = host.StopAsync(); + releaseSelector.Set(); + await stop.WaitAsync(Timeout); + } + finally + { + releaseSelector.Set(); + host.Dispose(); + } + } + + private static async Task StartHostAsync(DirectTlsEndpoint endpoint, Action configureTransport) + { + var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .UseKestrel() + .UseDirectTls(configureTransport) + .ConfigureKestrel(options => options.Listen(endpoint)) + .Configure(appBuilder => appBuilder.Run(context => context.Response.WriteAsync("ok"))); + }) + .Build(); + + await host.StartAsync().WaitAsync(Timeout); + return host; + } + + private static async Task ConnectAsync(int port) + { + var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + await socket.ConnectAsync(IPAddress.Loopback, port); + + var sslStream = new SslStream( + new NetworkStream(socket, ownsSocket: true), + leaveInnerStreamOpen: false, + userCertificateValidationCallback: (sender, certificate, chain, errors) => true); + + try + { + await sslStream.AuthenticateAsClientAsync(new SslClientAuthenticationOptions + { + TargetHost = "localhost", + ApplicationProtocols = [SslApplicationProtocol.Http11], + }); + } + catch + { + await sslStream.DisposeAsync(); + throw; + } + + return sslStream; + } + + private static async Task SendRequestAsync(SslStream sslStream) + { + var request = "GET / HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n\r\n"; + await sslStream.WriteAsync(Encoding.ASCII.GetBytes(request)); + await sslStream.FlushAsync(); + + using var reader = new StreamReader(sslStream, Encoding.ASCII, detectEncodingFromByteOrderMarks: false, bufferSize: 1024, leaveOpen: true); + return await reader.ReadToEndAsync().WaitAsync(ProgressTimeout); + } +} From 314c0581486874dadf4814a426efc25411b6dbae Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 09:56:11 +0200 Subject: [PATCH 02/16] fix leaking pool problems --- .../Transport.DirectTls/src/TlsEventPump.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 0fd77aa4807d..5da24d319a20 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -1280,6 +1280,9 @@ private static bool TryCaptureClientHello(TlsSocketSession session, out byte[]? buffer = null; length = 0; + // Tracked outside the try so the catch returns the array even when the throw happened between renting it + // and publishing it to buffer. + byte[]? rented = null; try { var helloLength = session.GetClientHelloLength(); @@ -1288,11 +1291,13 @@ private static bool TryCaptureClientHello(TlsSocketSession session, out byte[]? return true; } - var rented = ArrayPool.Shared.Rent(helloLength); + rented = ArrayPool.Shared.Rent(helloLength); if (!session.TryGetClientHelloBytes(rented.AsSpan(0, helloLength), out var written) || written <= 0) { + // The session reported a record but then could not hand it over, so the listener would silently + // miss a ClientHello it was configured to see. Treat it as a capture failure, not as "no bytes". ArrayPool.Shared.Return(rented); - return true; + return false; } buffer = rented; @@ -1301,13 +1306,13 @@ private static bool TryCaptureClientHello(TlsSocketSession session, out byte[]? } catch { - if (buffer is not null) + if (rented is not null) { - ArrayPool.Shared.Return(buffer); - buffer = null; - length = 0; + ArrayPool.Shared.Return(rented); } + buffer = null; + length = 0; return false; } } From 60cb7e5b5d572fec2852dd65ccbe28afcf495b9b Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:00:57 +0200 Subject: [PATCH 03/16] nit comments --- .../src/Connection/DirectTlsConnectionListener.cs | 2 +- src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs index 2fa0a92d8d91..88041be24659 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs @@ -31,7 +31,7 @@ internal sealed class DirectTlsConnectionListener : IConnectionListener private readonly TlsEventPumpPool _pumpPool; private readonly Action>? _clientHelloCallback; - // Decides whether the listener will try to select a server certificate for incoming connections. + // Whether the endpoint supplied a ServerCertificateSelector, i.e. whether resolving the TLS context can run user code private readonly bool _serverCertificateSelectorConfigured; // Native OpenSSL server credentials (bootstrap + per-SNI contexts) owned by this listener. Disposed once, diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 5da24d319a20..8fe277cecc15 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -226,7 +226,7 @@ public void StartWithListenSocket( _connectionIoStateLogger = loggerFactory.CreateLogger(); _directTlsConnectionLogger = loggerFactory.CreateLogger(); - // whether to suspend the handshake on the pump thread while resolving the TLS context (server certificate selection, SNI, etc.) + // Either of these makes context resolution run user code, so the handshake must leave the event loop before resolving. _contextResolverRunsUserCode = serverCertificateSelectorConfigured || clientHelloCallback is not null; // Add listen socket with EPOLLEXCLUSIVE - only one worker wakes per connection From afc600181331f2468d776118b29abead1227d50a Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:08:56 +0200 Subject: [PATCH 04/16] more debugable DrainWakeup --- .../src/TlsEventPump.UserCallbacks.cs | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index 31af15119b6e..013cfdc3ab58 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -171,20 +171,47 @@ private void Wakeup() } // Consumes the eventfd counter so the level-triggered wakeup fd stops firing. Runs on the pump thread. + // + // The counter is not a message and carries no payload: an eventfd created without EFD_SEMAPHORE returns the + // accumulated sum of every write since the last read and resets it to zero, so N callbacks completing + // between two polls surface as a single readable event with value N. The value is deliberately not used to + // decide how much work to do - the completion queue is the source of truth, and this fd only says "look at + // it". PumpLoop drains that queue after every batch whether or not this read succeeded, so a failure here + // costs a diagnostic, never a parked handshake. + // + // Failing to consume the counter cannot spin the loop. EAGAIN means the counter is already zero, so the fd + // is no longer readable and epoll will not report it again. EINTR leaves the counter set, so the + // level-triggered registration re-reports EPOLLIN and the next iteration retries this read - one extra + // iteration rather than an inner retry loop on the pump thread. A closed fd leaves the epoll set entirely. private void DrainWakeup() { long value = 0; - long read = NativeTls.read(_wakeupFd, ref value, sizeof(long)); - - // The counter is not a message and carries no payload: an eventfd created without EFD_SEMAPHORE - // returns the accumulated sum of every write since the last read and resets it to zero, so N callbacks - // completing between two polls surface as a single readable event with value N. The value is - // deliberately not used to decide how much work to do - the completion queue is the source of truth, - // and this fd only says "look at it". epoll reported EPOLLIN and the pump thread is the only reader, - // so the counter cannot have been consumed in between: a short read or a zero counter would mean the - // fd is not the eventfd we registered. - Debug.Assert(read == sizeof(long), $"Reading the wakeup fd returned {read} bytes."); - Debug.Assert(value >= 1, $"The wakeup fd reported a counter of {value}."); + nint read = NativeTls.read(_wakeupFd, ref value, sizeof(long)); + + if (read == sizeof(long)) + { + // epoll reported EPOLLIN and the pump thread is the only reader, so the counter cannot have been + // consumed in between; a zero counter would mean this is not the eventfd we registered. + Debug.Assert(value >= 1, $"The wakeup fd reported a counter of {value}."); + return; + } + + if (read < 0) + { + int errno = Marshal.GetLastWin32Error(); + if (errno is NativeTls.EAGAIN or NativeTls.EINTR) + { + return; + } + + _logger.LogDebug("Pump {Id}: reading the wakeup fd failed: errno={Errno}", _id, errno); + return; + } + + // An eventfd read is all-or-nothing (the kernel rejects a count below 8 bytes with EINVAL), so a + // partial read means this fd is not the eventfd we registered. + Debug.Assert(false, $"Reading the wakeup fd returned {read} bytes."); + _logger.LogDebug("Pump {Id}: reading the wakeup fd returned {Read} bytes.", _id, read); } // Resumes every handshake whose user callback has reported back. Runs on the pump thread only. From 24af91cece5f0357a250fec28f07e3171674c4fb Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:17:29 +0200 Subject: [PATCH 05/16] tests & nits --- .../DirectTlsContextResolverFastPathTests.cs | 11 ++- .../test/DirectTlsFunctionalTests.cs | 2 +- .../DirectTlsUserCallbackDispatchTests.cs | 81 +++++++++++++++++-- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs index 2c224ac809d9..b121e9a85d18 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsContextResolverFastPathTests.cs @@ -117,16 +117,19 @@ public async Task ContextResolver_WithClientHelloListenerOnly_RunsOffThePumpThre var threadName = await resolverThreadName.Task.WaitAsync(Timeout); - // Observe the handshake so a failure after the resolver ran does not surface as an unobserved - // exception; the assertion under test is about the resolver's thread, not the handshake's outcome. - await handshake.ContinueWith(static _ => { }, TaskScheduler.Default); + // The resolver ran off the pump, but the handshake itself must still succeed: these tests use a + // real certificate and a permissive client callback, so a failure here means something broke. + await handshake.WaitAsync(Timeout); return threadName; } finally { await listener.DisposeAsync(); - await drain.ContinueWith(static _ => { }, TaskScheduler.Default); + + // DisposeAsync completes the accept channel, so the drain loop ends on its own. Awaiting it keeps + // the loop from outliving the test and surfaces any failure it hit. + await drain.WaitAsync(Timeout); } } } diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsFunctionalTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsFunctionalTests.cs index 7715719d06d3..df64aa162ddb 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsFunctionalTests.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsFunctionalTests.cs @@ -268,7 +268,7 @@ public async Task ClientCertificate_RequiredAndValidated_AllowsRequest() [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] public async Task ClientCertificateValidation_ThrowingCallback_DropsConnection_AndKeepsPumpHealthy() { - // The endpoint's client-certificate validation callback runs on the pump thread right after the fd-path + // The endpoint's client-certificate validation callback is dispatched off the pump once the fd-path // handshake reports Complete. If it throws, the transport must drop the connection - dispose the session // and de-register the fd - rather than leaving the fd epoll-registered but in neither the handshaking nor // the connection table, which would spin the pump on the level-triggered socket and hang the request. The diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs index 3b753cc1eba9..08c226b23df4 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsUserCallbackDispatchTests.cs @@ -15,13 +15,14 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.InternalTesting; using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.Extensions.Hosting; namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Tests; /// -/// Covers the guarantee that user-supplied handshake callbacks - the server-certificate selector and the -/// ClientHello listener - never run on a pump's epoll thread. Every test pins the transport to a single pump +/// Covers the guarantee that user-supplied handshake callbacks - the server-certificate selector, the +/// ClientHello listener and client-certificate validation - never run on a pump's epoll thread. Every test pins the transport to a single pump /// (WorkerCount = 1) so all connections are provably owned by the same event loop: if a callback were /// still invoked inline, one slow or throwing connection would stall or break every other connection here. /// @@ -185,6 +186,69 @@ public async Task ThrowingClientHelloCallback_FailsOnlyThatConnection() await host.StopAsync().WaitAsync(Timeout); } + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task BlockingClientCertificateValidation_DoesNotStallHandshakesOnTheSamePump() + { + // Client-certificate validation is the third suspension site, and the odd one out: it runs after the + // handshake already reported Complete, so the blocked connection is one the pump has not yet surfaced + // to Kestrel rather than one still negotiating. Parking it must not stop the pump from driving other + // connections to a served request. + var blockedOnce = 0; + string validationThreadName = null; + using var validationEntered = new SemaphoreSlim(0); + using var releaseValidation = new ManualResetEventSlim(false); + + var endpoint = new DirectTlsEndpoint(IPAddress.Loopback, 0); + endpoint.Options.ServerCertificate = TestResources.GetTestCertificate(); + endpoint.Options.ClientCertificateMode = ClientCertificateMode.RequireCertificate; + endpoint.Options.ClientCertificateValidation = (certificate, chain, errors) => + { + // Only the first connection blocks; everything after it validates immediately. + if (Interlocked.Exchange(ref blockedOnce, 1) == 0) + { + // Published before the release below, so the test thread observes it after the wait. + validationThreadName = Thread.CurrentThread.Name; + validationEntered.Release(); + releaseValidation.Wait(Timeout); + } + + return true; + }; + + try + { + using var host = await StartHostAsync(endpoint, options => options.WorkerCount = 1); + var port = host.GetPort(); + var clientCertificate = TestResources.GetTestCertificate("eku.client.pfx"); + + // The TLS handshake can finish on the client before the server runs validation, so this connect may + // complete while the server side is still parked. Either way the request cannot be served until the + // callback returns, so the response is only read after the release below. + var blocked = ConnectAsync(port, clientCertificate); + Assert.True(await validationEntered.WaitAsync(Timeout)); + + // The pump owns both connections (WorkerCount = 1). If validation were still inline, this second + // handshake could not even start, let alone be served. + using var progressing = await ConnectAsync(port, clientCertificate).WaitAsync(ProgressTimeout); + Assert.Contains("200 OK", await SendRequestAsync(progressing)); + + releaseValidation.Set(); + + using var resumed = await blocked.WaitAsync(Timeout); + Assert.Contains("200 OK", await SendRequestAsync(resumed)); + + // Pump threads are named TlsEventPump-{id}; the callback must have run somewhere else. + Assert.DoesNotContain("TlsEventPump-", validationThreadName ?? string.Empty); + + await host.StopAsync().WaitAsync(Timeout); + } + finally + { + releaseValidation.Set(); + } + } + [ConditionalFact] [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] public async Task ShutdownWhileCallbackIsParked_CompletesWithoutLeakingTheConnection() @@ -239,7 +303,7 @@ private static async Task StartHostAsync(DirectTlsEndpoint endpoint, Acti return host; } - private static async Task ConnectAsync(int port) + private static async Task ConnectAsync(int port, X509Certificate2 clientCertificate = null) { var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); await socket.ConnectAsync(IPAddress.Loopback, port); @@ -251,11 +315,18 @@ private static async Task ConnectAsync(int port) try { - await sslStream.AuthenticateAsClientAsync(new SslClientAuthenticationOptions + var clientOptions = new SslClientAuthenticationOptions { TargetHost = "localhost", ApplicationProtocols = [SslApplicationProtocol.Http11], - }); + }; + + if (clientCertificate is not null) + { + clientOptions.ClientCertificates = [clientCertificate]; + } + + await sslStream.AuthenticateAsClientAsync(clientOptions); } catch { From 143b4479a8a55855892690ba6318527898ca4b59 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:22:16 +0200 Subject: [PATCH 06/16] public API comments about thread pool dispatch --- .../Core/src/DirectTlsEndpointOptions.cs | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs b/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs index d1a1516d035d..2dce4a14d8a0 100644 --- a/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs +++ b/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs @@ -38,8 +38,12 @@ public sealed class DirectTlsEndpointOptions /// allocated so it carries the same connection id that will later serve the request); the second is the /// requested SNI host name, or when the client did not send one. /// - /// This callback runs synchronously on the epoll worker thread that owns the connection and must not block. - /// A blocking or long-running callback stalls the handshake and I/O of every connection assigned to that worker. + /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and + /// the callback is dispatched to the , so a slow callback no longer stalls the + /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, + /// and the time spent in it counts against - a callback that overruns that + /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent + /// invocation occupies a thread-pool thread. /// /// public Func? ServerCertificateSelector { get; set; } @@ -65,8 +69,12 @@ public sealed class DirectTlsEndpointOptions /// when it produced no . /// /// - /// This callback runs synchronously on the epoll worker thread that owns the connection and must not block. - /// A blocking or long-running callback stalls the handshake and I/O of every connection assigned to that worker. + /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and + /// the callback is dispatched to the , so a slow callback no longer stalls the + /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, + /// and the time spent in it counts against - a callback that overruns that + /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent + /// invocation occupies a thread-pool thread. /// public Func? ClientCertificateValidation { get; set; } @@ -79,8 +87,12 @@ public sealed class DirectTlsEndpointOptions /// (for example with ToArray()) if they must outlive the call. The first argument is the /// for the connection being negotiated. /// - /// This callback runs synchronously on the epoll worker thread that owns the connection and must not block. - /// A blocking or long-running callback stalls the handshake and I/O of every connection assigned to that worker. + /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and + /// the callback is dispatched to the , so a slow callback no longer stalls the + /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, + /// and the time spent in it counts against - a callback that overruns that + /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent + /// invocation occupies a thread-pool thread. /// /// public Action>? TlsClientHelloBytesCallback { get; set; } @@ -137,7 +149,7 @@ public TimeSpan HandshakeTimeout /// /// The HTTP protocols (ALPN) advertised for this endpoint, - /// sourced from after the endpoint has been configured. + /// sourced from after the endpoint has been configured. /// internal HttpProtocols HttpProtocols { get; set; } = HttpProtocols.Http1AndHttp2; } From 240fca3eaa6c8876cd3876a2d0bd483ae1bd5da6 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:32:01 +0200 Subject: [PATCH 07/16] split up types --- .../src/HandshakeUserCallback.cs | 211 ------------------ .../src/TlsEventPump.UserCallbacks.cs | 1 + .../Transport.DirectTls/src/TlsEventPump.cs | 1 + .../UserCallbacks/HandshakeUserCallback.cs | 92 ++++++++ .../ResolveTlsContextCallback.cs | 80 +++++++ .../ValidateClientCertificateCallback.cs | 53 +++++ 6 files changed, 227 insertions(+), 211 deletions(-) delete mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/HandshakeUserCallback.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ResolveTlsContextCallback.cs create mode 100644 src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ValidateClientCertificateCallback.cs diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs b/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs deleted file mode 100644 index faea9e6d3486..000000000000 --- a/src/Servers/Kestrel/Transport.DirectTls/src/HandshakeUserCallback.cs +++ /dev/null @@ -1,211 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Buffers; -using System.Net.Security; -using System.Security.Cryptography.X509Certificates; -using Microsoft.AspNetCore.Connections; -using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; - -namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; - -/// -/// One suspended handshake's user code, executed on the thread pool instead of on the pump (epoll) thread. -/// -/// -/// User-supplied handshake callbacks - the ClientHello listener, the server-certificate selector, and the -/// client-certificate validation callback - can block for an arbitrarily long time (a disk read, a key vault -/// round trip, a lock). A pump thread owns accept plus all I/O readiness for every connection assigned to it, -/// so running any of them inline stalls every one of those connections. Instead the pump parks the handshake -/// (de-registering its fd from the epoll set so it cannot generate pump work while parked) and queues this -/// work item. Everything a derived item touches was copied out of the session on the pump thread beforehand: -/// it never calls into TlsSocketSession, which stays single-threaded and owned by its pump. When the -/// user code returns - or throws - the result is handed back to the owning pump through -/// , which resumes the handshake on the pump thread. -/// -/// Each suspension point has its own derived type carrying only its own state, so the pump resumes by -/// switching on the work item's type. is deliberately not virtual: the try/catch/finally -/// it wraps every callback in is what guarantees that a suspended handshake reports back exactly once, whether -/// the user code returns or throws, so a derived type must not be able to replace it. -/// -/// -internal abstract class HandshakeUserCallback : IThreadPoolWorkItem -{ - private readonly TlsEventPump _pump; - - protected HandshakeUserCallback(TlsEventPump pump, int fd, DirectTlsConnection? connection) - { - _pump = pump; - Fd = fd; - Connection = connection; - } - - /// The handshaking file descriptor this callback belongs to. - public int Fd { get; } - - /// - /// The connection allocated early (at NeedsTlsContext) so user code sees a stable - /// . Null when nothing needed one that early: the pump resolved the TLS - /// context inline because no user code runs at NeedsTlsContext (so only the client-certificate - /// suspension is reachable, and it does not use this), or the pump has no memory pool (tests). In both - /// cases the connection is allocated when the handshake completes instead. - /// - public DirectTlsConnection? Connection { get; } - - /// The exception the user code threw, if any. Non-null means the pump drops the connection. - public Exception? Failure { get; private set; } - - /// - public void Execute() - { - try - { - RunUserCode(); - } - catch (Exception ex) - { - // A throwing user callback (or a selector that resolved no certificate) fails this one connection. - // The pump logs it and drops the handshake when it picks the result up; it must never escape onto - // a thread pool thread, where it would tear the process down. - Failure = ex; - } - finally - { - ReleaseTransientState(); - - // Hand the result back to the owning pump. Nothing here may touch the session, the epoll set, or - // the handshake bookkeeping - those are pump-thread-only. - _pump.CompleteUserCallback(this); - } - } - - /// - /// Runs the endpoint-supplied callback on the thread pool and records its result on this instance. Any - /// throw is captured by as . - /// - protected abstract void RunUserCode(); - - /// - /// Releases anything borrowed for the duration of the callback, whether it returned or threw. Runs before - /// the result is handed back to the pump. - /// - protected virtual void ReleaseTransientState() - { - } -} - -/// -/// The NeedsTlsContext suspension: the optional ClientHello listener followed by the server-certificate -/// selector. -/// -internal sealed class ResolveTlsContextCallback : HandshakeUserCallback -{ - private readonly Func _contextResolver; - private readonly Action>? _clientHelloCallback; - private readonly string? _targetHostName; - - // ClientHello record copied out of the session on the pump thread. Rented from the shared pool and - // returned as soon as the (synchronous) user callback returns, matching the transient-buffer contract of - // the socket-transport TlsListener. - private byte[]? _clientHelloBuffer; - private readonly int _clientHelloLength; - - /// - /// Creates the work item for the NeedsTlsContext suspension. - /// holds the ClientHello record already copied out of the session on the pump thread (null when there is - /// no listener, or nothing was captured); this work item returns it to - /// once the listener has run. - /// - public ResolveTlsContextCallback( - TlsEventPump pump, - int fd, - DirectTlsConnection? connection, - string? targetHostName, - Func contextResolver, - Action>? clientHelloCallback, - byte[]? clientHelloBuffer, - int clientHelloLength) - : base(pump, fd, connection) - { - _targetHostName = targetHostName; - _contextResolver = contextResolver; - _clientHelloCallback = clientHelloCallback; - _clientHelloBuffer = clientHelloBuffer; - _clientHelloLength = clientHelloLength; - } - - /// The TLS context the certificate selector resolved. - public TlsContext? ResolvedContext { get; private set; } - - /// The client-certificate validation callback that came back with the resolved context. - public RemoteCertificateValidationCallback? ResolvedClientCertificateValidation { get; private set; } - - /// - protected override void RunUserCode() - { - // Fire the observe-only ClientHello listener first: the listener sees the ClientHello before the real context is installed. - if (_clientHelloCallback is not null && Connection is not null && _clientHelloBuffer is not null && _clientHelloLength > 0) - { - _clientHelloCallback(Connection, new ReadOnlySequence(_clientHelloBuffer, 0, _clientHelloLength)); - } - - var (context, clientCertificateValidation) = _contextResolver(Connection, _targetHostName); - ResolvedContext = context; - ResolvedClientCertificateValidation = clientCertificateValidation; - } - - /// - protected override void ReleaseTransientState() - { - if (_clientHelloBuffer is { } buffer) - { - _clientHelloBuffer = null; - ArrayPool.Shared.Return(buffer); - } - } -} - -/// -/// The client-certificate validation suspension: the chain build plus the endpoint's validation callback, run -/// once the handshake reports Complete. -/// -internal sealed class ValidateClientCertificateCallback : HandshakeUserCallback -{ - private readonly RemoteCertificateValidationCallback _validateClientCertificate; - private readonly object _validationSender; - private readonly X509Certificate2Collection? _intermediates; - - /// - /// Creates the work item for the client-certificate validation suspension. The certificates were read from - /// the session on the pump thread; the chain build and the endpoint's callback run here. - /// - public ValidateClientCertificateCallback( - TlsEventPump pump, - int fd, - DirectTlsConnection? connection, - object validationSender, - X509Certificate2? presentedCertificate, - X509Certificate2Collection? intermediates, - RemoteCertificateValidationCallback validateClientCertificate) - : base(pump, fd, connection) - { - _validationSender = validationSender; - PresentedCertificate = presentedCertificate; - _intermediates = intermediates; - _validateClientCertificate = validateClientCertificate; - } - - /// The peer's leaf certificate, or null when it presented none. - public X509Certificate2? PresentedCertificate { get; } - - /// Whether the endpoint's validation callback accepted the peer's certificate. - public bool CertificateAccepted { get; private set; } - - /// - protected override void RunUserCode() - => CertificateAccepted = ClientCertificateValidator.Validate( - _validationSender, - PresentedCertificate, - _intermediates, - _validateClientCertificate); -} diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index 013cfdc3ab58..d9b3f8c229d1 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -6,6 +6,7 @@ using System.Net.Security; using System.Runtime.InteropServices; using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Interop; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.UserCallbacks; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 8fe277cecc15..800d72ce5a46 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Interop; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.UserCallbacks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/HandshakeUserCallback.cs b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/HandshakeUserCallback.cs new file mode 100644 index 000000000000..dc08eb88b54d --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/HandshakeUserCallback.cs @@ -0,0 +1,92 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.UserCallbacks; + +/// +/// One suspended handshake's user code, executed on the thread pool instead of on the pump (epoll) thread. +/// +/// +/// User-supplied handshake callbacks - the ClientHello listener, the server-certificate selector, and the +/// client-certificate validation callback - can block for an arbitrarily long time (a disk read, a key vault +/// round trip, a lock). A pump thread owns accept plus all I/O readiness for every connection assigned to it, +/// so running any of them inline stalls every one of those connections. Instead the pump parks the handshake +/// (de-registering its fd from the epoll set so it cannot generate pump work while parked) and queues this +/// work item. Everything a derived item touches was copied out of the session on the pump thread beforehand: +/// it never calls into TlsSocketSession, which stays single-threaded and owned by its pump. When the +/// user code returns - or throws - the result is handed back to the owning pump through +/// , which resumes the handshake on the pump thread. +/// +/// Each suspension point has its own derived type carrying only its own state, so the pump resumes by +/// switching on the work item's type. is deliberately not virtual: the try/catch/finally +/// it wraps every callback in is what guarantees that a suspended handshake reports back exactly once, whether +/// the user code returns or throws, so a derived type must not be able to replace it. +/// +/// +internal abstract class HandshakeUserCallback : IThreadPoolWorkItem +{ + private readonly TlsEventPump _pump; + + protected HandshakeUserCallback(TlsEventPump pump, int fd, DirectTlsConnection? connection) + { + _pump = pump; + Fd = fd; + Connection = connection; + } + + /// The handshaking file descriptor this callback belongs to. + public int Fd { get; } + + /// + /// The connection allocated early (at NeedsTlsContext) so user code sees a stable + /// . Null when nothing needed one that early: the pump resolved the TLS + /// context inline because no user code runs at NeedsTlsContext (so only the client-certificate + /// suspension is reachable, and it does not use this), or the pump has no memory pool (tests). In both + /// cases the connection is allocated when the handshake completes instead. + /// + public DirectTlsConnection? Connection { get; } + + /// The exception the user code threw, if any. Non-null means the pump drops the connection. + public Exception? Failure { get; private set; } + + /// + public void Execute() + { + try + { + RunUserCode(); + } + catch (Exception ex) + { + // A throwing user callback (or a selector that resolved no certificate) fails this one connection. + // The pump logs it and drops the handshake when it picks the result up; it must never escape onto + // a thread pool thread, where it would tear the process down. + Failure = ex; + } + finally + { + ReleaseTransientState(); + + // Hand the result back to the owning pump. Nothing here may touch the session, the epoll set, or + // the handshake bookkeeping - those are pump-thread-only. + _pump.CompleteUserCallback(this); + } + } + + /// + /// Runs the endpoint-supplied callback on the thread pool and records its result on this instance. Any + /// throw is captured by as . + /// + protected abstract void RunUserCode(); + + /// + /// Releases anything borrowed for the duration of the callback, whether it returned or threw. Runs before + /// the result is handed back to the pump. + /// + protected virtual void ReleaseTransientState() + { + } +} diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ResolveTlsContextCallback.cs b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ResolveTlsContextCallback.cs new file mode 100644 index 000000000000..df8eac194e77 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ResolveTlsContextCallback.cs @@ -0,0 +1,80 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers; +using System.Net.Security; +using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.UserCallbacks; + +/// +/// The NeedsTlsContext suspension: the optional ClientHello listener followed by the server-certificate +/// selector. +/// +internal sealed class ResolveTlsContextCallback : HandshakeUserCallback +{ + private readonly Func _contextResolver; + private readonly Action>? _clientHelloCallback; + private readonly string? _targetHostName; + + // ClientHello record copied out of the session on the pump thread. Rented from the shared pool and + // returned as soon as the (synchronous) user callback returns, matching the transient-buffer contract of + // the socket-transport TlsListener. + private byte[]? _clientHelloBuffer; + private readonly int _clientHelloLength; + + /// + /// Creates the work item for the NeedsTlsContext suspension. + /// holds the ClientHello record already copied out of the session on the pump thread (null when there is + /// no listener, or nothing was captured); this work item returns it to + /// once the listener has run. + /// + public ResolveTlsContextCallback( + TlsEventPump pump, + int fd, + DirectTlsConnection? connection, + string? targetHostName, + Func contextResolver, + Action>? clientHelloCallback, + byte[]? clientHelloBuffer, + int clientHelloLength) + : base(pump, fd, connection) + { + _targetHostName = targetHostName; + _contextResolver = contextResolver; + _clientHelloCallback = clientHelloCallback; + _clientHelloBuffer = clientHelloBuffer; + _clientHelloLength = clientHelloLength; + } + + /// The TLS context the certificate selector resolved. + public TlsContext? ResolvedContext { get; private set; } + + /// The client-certificate validation callback that came back with the resolved context. + public RemoteCertificateValidationCallback? ResolvedClientCertificateValidation { get; private set; } + + /// + protected override void RunUserCode() + { + // Fire the observe-only ClientHello listener first: the listener sees the ClientHello before the real context is installed. + if (_clientHelloCallback is not null && Connection is not null && _clientHelloBuffer is not null && _clientHelloLength > 0) + { + _clientHelloCallback(Connection, new ReadOnlySequence(_clientHelloBuffer, 0, _clientHelloLength)); + } + + var (context, clientCertificateValidation) = _contextResolver(Connection, _targetHostName); + ResolvedContext = context; + ResolvedClientCertificateValidation = clientCertificateValidation; + } + + /// + protected override void ReleaseTransientState() + { + if (_clientHelloBuffer is { } buffer) + { + _clientHelloBuffer = null; + ArrayPool.Shared.Return(buffer); + } + } +} diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ValidateClientCertificateCallback.cs b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ValidateClientCertificateCallback.cs new file mode 100644 index 000000000000..7d03ea5fee37 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/src/UserCallbacks/ValidateClientCertificateCallback.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.UserCallbacks; + +/// +/// The client-certificate validation suspension: the chain build plus the endpoint's validation callback, run +/// once the handshake reports Complete. +/// +internal sealed class ValidateClientCertificateCallback : HandshakeUserCallback +{ + private readonly RemoteCertificateValidationCallback _validateClientCertificate; + private readonly object _validationSender; + private readonly X509Certificate2Collection? _intermediates; + + /// + /// Creates the work item for the client-certificate validation suspension. The certificates were read from + /// the session on the pump thread; the chain build and the endpoint's callback run here. + /// + public ValidateClientCertificateCallback( + TlsEventPump pump, + int fd, + DirectTlsConnection? connection, + object validationSender, + X509Certificate2? presentedCertificate, + X509Certificate2Collection? intermediates, + RemoteCertificateValidationCallback validateClientCertificate) + : base(pump, fd, connection) + { + _validationSender = validationSender; + PresentedCertificate = presentedCertificate; + _intermediates = intermediates; + _validateClientCertificate = validateClientCertificate; + } + + /// The peer's leaf certificate, or null when it presented none. + public X509Certificate2? PresentedCertificate { get; } + + /// Whether the endpoint's validation callback accepted the peer's certificate. + public bool CertificateAccepted { get; private set; } + + /// + protected override void RunUserCode() + => CertificateAccepted = ClientCertificateValidator.Validate( + _validationSender, + PresentedCertificate, + _intermediates, + _validateClientCertificate); +} From 41997a83187f36996961bf4dc36bdcc8d47637af Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 10:42:33 +0200 Subject: [PATCH 08/16] fix race condition --- .../src/DirectTlsTransportFactory.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs b/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs index b9d69339f1a6..273d19e9bd53 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/DirectTlsTransportFactory.cs @@ -118,11 +118,11 @@ public async ValueTask BindAsync(EndPoint endpoint, Cancell $"No server certificate was resolved for SNI host name '{hostName}'."); } - var context = contextCache.GetOrAdd(certificate, cert => + if (!contextCache.TryGetValue(certificate, out var context)) { var authenticationOptions = new SslServerAuthenticationOptions { - ServerCertificate = cert, + ServerCertificate = certificate, EnabledSslProtocols = endpointOptions.SslProtocols, ApplicationProtocols = applicationProtocols, ClientCertificateRequired = requireClientCertificate, @@ -133,8 +133,16 @@ public async ValueTask BindAsync(EndPoint endpoint, Cancell authenticationOptions.RemoteCertificateValidationCallback = clientCertificateValidation; } - return TlsContext.CreateServer(authenticationOptions); - }); + var candidate = TlsContext.CreateServer(authenticationOptions); + context = contextCache.GetOrAdd(certificate, candidate); + + // Multiple threads racing to add the same certificate can create multiple candidates, + // so make sure we dont leak TlsContext (with cert/key handles) by disposing the non-cached candidate. + if (!ReferenceEquals(context, candidate)) + { + candidate.Dispose(); + } + } return (context, clientCertificateValidation); }; From a18f227d1e3bdb0557492e24c5eb81ad2ff6a172 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:05:55 +0200 Subject: [PATCH 09/16] fix async disposal of directtlsconnection & exitSignal relation --- .../src/TlsEventPump.UserCallbacks.cs | 25 ++++-- .../Transport.DirectTls/src/TlsEventPump.cs | 25 +++++- .../test/TlsEventPumpStopTests.cs | 83 +++++++++++++++++++ 3 files changed, 123 insertions(+), 10 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index d9b3f8c229d1..9170ac088fc0 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -32,6 +32,15 @@ internal partial class TlsEventPump // still be running user code that reaches this pump's resources. private int _outstandingUserCallbacks; + // Number of abandoned-connection disposals that have been started but have not finished. + // DirectTlsConnection.DisposeAsync resumes the connection's send/receive loops asynchronously, so that work + // outlives this pump's event loop while still reaching the memory pool and the OpenSSL contexts - which the + // listener frees as soon as _exitSignal completes. Gating the signal on this as well is what keeps that from + // becoming a use-after-free. Counted separately from _outstandingUserCallbacks because these never write to + // the wakeup fd, so the argument above does not apply to them, and because it makes plain which kind of + // off-pump work is holding shutdown open. + private int _outstandingConnectionDisposals; + // Handshakes that were parked on a user callback when the pump loop exited. Their teardown is deferred to // the drained-shutdown path so the session (and the certificate handed to the validation callback) is not // disposed underneath live user code. Only ever touched by the thread that exits the loop and then by the @@ -90,15 +99,17 @@ private void ReleaseDeferredHandshakes() _handshakesAwaitingCallback.Clear(); } - // Finishes the pump's teardown once BOTH the event loop has exited and every user callback it dispatched to - // the thread pool has reported back: closes the fds it owns and completes the exit signal. Called by the - // pump thread when the loop ends and by each callback completion, so whichever happens last does the work. - // Deferring the close until the callbacks have drained is what makes CompleteUserCallback's wakeup write safe: - // the wakeup fd cannot have been closed (and its number recycled by an unrelated fd) while a callback is - // still in flight. + // Finishes the pump's teardown once the event loop has exited AND every piece of off-pump work it started - + // dispatched user callbacks and abandoned-connection disposals - has reported back: closes the fds it owns + // and completes the exit signal. Called by the pump thread when the loop ends and by each piece of off-pump + // work as it finishes, so whichever happens last does the work. Deferring the close until the callbacks have + // drained is what makes CompleteUserCallback's wakeup write safe: the wakeup fd cannot have been closed (and + // its number recycled by an unrelated fd) while a callback is still in flight. private void CompletePumpShutdownIfDrained() { - if (!_loopExited || Volatile.Read(ref _outstandingUserCallbacks) != 0) + if (!_loopExited + || Volatile.Read(ref _outstandingUserCallbacks) != 0 + || Volatile.Read(ref _outstandingConnectionDisposals) != 0) { return; } diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 800d72ce5a46..9fe9b8b5c646 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -1161,17 +1161,36 @@ private void DropCompletedHandshake(int fd, DirectTlsConnection connection) connection.AbortBeforeStart(); } - // Fire-and-forget teardown for a connection - private async Task DisposeAbandonedConnectionAsync(DirectTlsConnection connection) + // Disposes the abandoned connection itself. internal virtual so tests can hold the disposal open and observe + // that shutdown waits for it, without needing a live TLS session behind the connection. + internal virtual ValueTask DisposeConnectionAsync(DirectTlsConnection connection) => connection.DisposeAsync(); + + // Disposes a connection that finished its handshake just as the listener stopped accepting. It never reached + // the ready channel, so nothing else will ever dispose it - and DisposeAsync is not quick: it awaits the + // send/receive loops (which hold pooled buffers) and then sends close_notify through the TLS session, so the + // work keeps touching the memory pool and the OpenSSL contexts after this method has yielded to its caller. + // The listener frees both as soon as this pump reports that it has exited, so the disposal has to be part of + // that report. The count is taken in the synchronous part of the method, which runs before the first await: + // keeping it here rather than at the call site means a future caller cannot start a disposal without it + // being counted. The returned task is deliberately not retained; the counter is what shutdown waits on. + // internal so tests can drive this path without completing a real handshake. + internal async Task DisposeAbandonedConnectionAsync(DirectTlsConnection connection) { + Interlocked.Increment(ref _outstandingConnectionDisposals); + try { - await connection.DisposeAsync().ConfigureAwait(false); + await DisposeConnectionAsync(connection).ConfigureAwait(false); } catch (Exception ex) { _logger.LogDebug(ex, "Disposing a connection abandoned during shutdown threw."); } + finally + { + Interlocked.Decrement(ref _outstandingConnectionDisposals); + CompletePumpShutdownIfDrained(); + } } // Tears down a handshake we will not surface to Kestrel - whether it failed (the handshake or the diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/TlsEventPumpStopTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/TlsEventPumpStopTests.cs index fca3495f71a9..593daf9cc4c6 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/test/TlsEventPumpStopTests.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/test/TlsEventPumpStopTests.cs @@ -98,6 +98,89 @@ public async Task StopAndJoinAsync_ThreadBlockedInCallback_ReturnsFalse() Thread.Sleep(200); } + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task StopAndJoinAsync_AbandonedConnectionDisposalInFlight_ReturnsFalse() + { + var disposalStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseDisposal = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var pump = new ControllableDisposalPump(disposalStarted, releaseDisposal); + + using var listenSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); + listenSocket.Bind(new IPEndPoint(IPAddress.Loopback, 0)); + listenSocket.Listen(backlog: 16); + + // Stands in for the OpenSSL server credentials the listener frees as soon as the pump confirms exit. + using var bootstrap = TlsContext.CreateServer(new SslServerAuthenticationOptions()); + var readyConnections = Channel.CreateUnbounded(); + + pump.StartWithListenSocket( + (int)listenSocket.Handle, + (IPEndPoint)listenSocket.LocalEndPoint!, + bootstrap, + contextResolver: null, + readyConnections.Writer, + MemoryPool.Shared, + NullLoggerFactory.Instance, + noDelay: false, + maxReadBufferSize: 0, + maxWriteBufferSize: 0, + onFatalError: static _ => { }); + + var connectionState = new ConnectionIoState( + fd: 101, + session: null!, + NullLogger.Instance); + var connection = new DirectTlsConnection( + connectionState, + pump, + localEndPoint: null, + remoteEndPoint: null, + MemoryPool.Shared, + maxReadBufferSize: 0, + maxWriteBufferSize: 0, + NullLogger.Instance); + + // A connection that completed its handshake just as the listener stopped accepting. Its DisposeAsync + // resumes the send/receive loops asynchronously, so it outlives the event loop. + _ = pump.DisposeAbandonedConnectionAsync(connection); + await disposalStarted.Task.WaitAsync(TimeSpan.FromSeconds(5)); + + // The event loop is free to wind down, but the disposal is provably still running and still reaches the + // memory pool and the OpenSSL contexts, so the pump must not yet report itself as exited - that report is + // exactly what lets the listener free them. + pump.SignalStop(); + using var stopCts = new CancellationTokenSource(TimeSpan.FromSeconds(2)); + Assert.False(await pump.StopAndJoinAsync(stopCts.Token)); + + // Let the disposal finish so the pump closes its own fds before the bootstrap context goes away. + releaseDisposal.SetResult(); + Thread.Sleep(200); + } + + /// + /// A pump whose abandoned-connection disposal is held open by the test, standing in for send/receive loops + /// that are still unwinding after the event loop has gone. The real TLS session is never touched. + /// + private sealed class ControllableDisposalPump : TlsEventPump + { + private readonly TaskCompletionSource _started; + private readonly TaskCompletionSource _release; + + public ControllableDisposalPump(TaskCompletionSource started, TaskCompletionSource release) + : base(NullLogger.Instance, id: 0, handshakeTimeout: Timeout.InfiniteTimeSpan) + { + _started = started; + _release = release; + } + + internal override async ValueTask DisposeConnectionAsync(DirectTlsConnection connection) + { + _started.TrySetResult(); + await _release.Task; + } + } + /// /// A pump whose accept path blocks on the first call, standing in for a user callback that never returns. /// Once released it reports a drained backlog so the loop can wind down. The native TLS session is never From 69c04f12df42ee1bdd29df03a4b0e7e524b40508 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:11:54 +0200 Subject: [PATCH 10/16] simplify SNI read --- .../Transport.DirectTls/src/TlsEventPump.cs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index 9fe9b8b5c646..e8fc3c2b70a6 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -927,23 +927,11 @@ private void TryAdvanceHandshake( return; } - string? targetHostName; - try - { - targetHostName = conn.Session.TargetHostName; - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Reading the SNI host name failed for fd={Fd}", fd); - DropHandshake(fd, conn); - return; - } - var contextCallback = new ResolveTlsContextCallback( this, fd, conn.Connection, - targetHostName, + conn.Session.TargetHostName, _contextResolver, clientHelloBuffer is null ? null : _clientHelloCallback, clientHelloBuffer, From 0508102c7bf27220f5ba19e3120fb7ee3740a36e Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:16:20 +0200 Subject: [PATCH 11/16] drop handshake on SetRemoteCertificateValidationResult failure --- .../src/TlsEventPump.UserCallbacks.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index 9170ac088fc0..95e45ffced80 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -311,15 +311,16 @@ private void ResumeSuspendedHandshake(HandshakeUserCallback callback) } // Record the accepted result so the runtime promotes the leaf into its canonical remote-cert - // slot and clears its pending-validation state. + // slot and clears its pending-validation state. Throws only in incorrect state of session, so we must drop the handshake. try { conn.Session.SetRemoteCertificateValidationResult(SslPolicyErrors.None); } - catch (InvalidOperationException) + catch (Exception ex) { - // Validation was already resolved (e.g. a buffered PAL that surfaced - // NeedsCertificateValidation before reaching Complete). + _logger.LogDebug(ex, "Recording the client certificate verdict failed for fd={Fd}; dropping connection.", fd); + DropHandshake(fd, conn); + return; } // Surface the accepted certificate to Kestrel via ITlsConnectionFeature. This is the same From 8b35f16ebdf461d8d9641e0ed058f77ea0aa1e90 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:27:36 +0200 Subject: [PATCH 12/16] unreachable for non-fd flow --- .../Transport.DirectTls/src/TlsEventPump.cs | 43 +++++++------------ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs index e8fc3c2b70a6..0bec61922196 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.cs @@ -473,6 +473,16 @@ private void PumpLoop() SweepExpiredHandshakes(Environment.TickCount64); } } + catch (UnreachableException ex) + { + _logger.LogCritical(ex, "Pump {Id} reached an unreachable state in PumpLoop", _id); + + if (_running) + { + _onFatalError.Invoke(ex); + } + break; + } catch (Exception ex) { _logger.LogError(ex, "Pump {Id} encountered an exception in PumpLoop", _id); @@ -792,13 +802,7 @@ private void TryAdvanceHandshake( // HttpsConnectionAdapterOptions.ClientCertificateMode (Allow/Require), which makes // CreateStreamTransportOptions set ClientCertificateRequired and install a // RemoteCertificateValidationCallback; conn.ClientCertificateValidation carries that callback - // (null for server-auth-only endpoints, which skip this block entirely). The Linux fd fast - // handshake path reports Complete directly - it does not surface NeedsCertificateValidation like - // the buffered PALs do, OpenSSL only enforces SSL_VERIFY_PEER (not FAIL_IF_NO_PEER_CERT), and the - // fd read/write fast paths bypass the runtime's pending-validation fault. So the runtime cannot - // enforce the accept/reject decision on this path. The transport runs the endpoint's validation - // callback itself, records the verdict on the session, and tears down rejected connections before - // they are ever surfaced to Kestrel. + // (null for server-auth-only endpoints, which skip this block entirely). // // The certificates are read from the session here (pump thread only), but the chain build and the // endpoint's callback are user-controlled work, so they are suspended onto the thread pool and the @@ -841,27 +845,10 @@ private void TryAdvanceHandshake( if (status == TlsOperationStatus.NeedsCertificateValidation) { - // Buffered / non-fd PALs surface this suspension so the caller runs client-certificate - // validation mid-handshake. (The Linux fd fast path our transport uses does not: it reports - // Complete directly and we validate + surface the certificate in the Complete branch above.) - // Resolve validation here so the re-driven handshake can finish (accept) or fail (reject); the - // Complete branch then observes it as already-validated. AcceptWithDefaultValidation runs the - // default chain build plus the RemoteCertificateValidationCallback configured in - // HttpsConnectionMiddleware.CreateStreamTransportOptions. - try - { - conn.Session.AcceptWithDefaultValidation(); - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Client certificate validation failed for fd={Fd}", fd); - DropHandshake(fd, conn); - return; - } - - // Re-drive so the handshake completes (accept) or fails (reject). - TryAdvanceHandshake(fd, conn); - return; + // The Linux fd fast handshake path reports Complete directly - it does not surface NeedsCertificateValidation like + // the buffered PALs do, OpenSSL only enforces SSL_VERIFY_PEER (not FAIL_IF_NO_PEER_CERT), and the + // fd read/write fast paths bypass the runtime's pending-validation fault. + throw new UnreachableException($"The DirectTls handshake path reported {nameof(TlsOperationStatus.NeedsCertificateValidation)} for fd={fd}."); } if (status == TlsOperationStatus.NeedsTlsContext) From cbca72f4f012599bce2f9e364452af08a07b9506 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:33:48 +0200 Subject: [PATCH 13/16] ResumeSuspendedHandshake not finding fd should be unreachable --- .../src/TlsEventPump.UserCallbacks.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index 95e45ffced80..aba6c8cf5fbb 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -238,20 +238,12 @@ private void DrainCompletedUserCallbacks() /// /// Resumes one suspended handshake with the result of its user callback. Runs on the pump thread only. /// - /// - /// A completion is only acted on when the handshaking entry for the fd still points at this exact work - /// item. Anything else - the connection was dropped (handshake timeout, listener shutdown, the peer went - /// away), or the fd number was recycled by a newer connection - means this result is stale, and it is - /// discarded. The resources of the dropped connection were already released exactly once by the teardown - /// path that removed it, so there is nothing to release here. - /// private void ResumeSuspendedHandshake(HandshakeUserCallback callback) { int fd = callback.Fd; if (!_handshaking.TryGetValue(fd, out var conn) || !ReferenceEquals(conn.PendingUserCallback, callback)) { - _logger.LogDebug("Pump {Id}: discarding a user callback result for fd={Fd}; the handshake is gone.", _id, fd); - return; + throw new UnreachableException($"Pump {_id}: a user callback completed for fd={fd}, but the handshake it was parked on is gone or is parked on a different callback."); } // Clear the suspension marker first, so this handshake can never be resumed twice. From f0770f7896819fc983ca8798e8002310fd404437 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:34:53 +0200 Subject: [PATCH 14/16] unrechable for non-existing callback type --- .../Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs index aba6c8cf5fbb..556a69ceece3 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/TlsEventPump.UserCallbacks.cs @@ -324,13 +324,7 @@ private void ResumeSuspendedHandshake(HandshakeUserCallback callback) return; default: - // Unreachable: every suspension point has a case above. Guard it anyway - the fd has just been - // re-armed, so falling out of the switch would leave the handshake registered but never - // advanced, stalling it until the timeout sweep instead of failing it here. - Debug.Assert(false, $"Unhandled handshake user callback type {callback.GetType()}."); - _logger.LogWarning("fd={Fd}: unhandled handshake user callback type; dropping.", fd); - DropHandshake(fd, conn); - return; + throw new UnreachableException($"Pump {_id}: unhandled handshake user callback type {callback.GetType()} for fd={fd}."); } } } From 3ef8397b8be6cc247a61809a7ab12c76e1925a78 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 11:44:06 +0200 Subject: [PATCH 15/16] stable channelRead on acceptAsync & wrapping _connectionClosedTokenSource cancellation --- .../src/Connection/DirectTlsConnection.cs | 17 ++++- .../Connection/DirectTlsConnectionListener.cs | 34 ++------- .../test/DirectTlsConnectionDisposeTests.cs | 73 +++++++++++++++++++ 3 files changed, 95 insertions(+), 29 deletions(-) create mode 100644 src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsConnectionDisposeTests.cs diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnection.cs b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnection.cs index 228cd4d6d3c5..e428b28abbc3 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnection.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnection.cs @@ -199,7 +199,7 @@ internal void AbortBeforeStart() _logger.LogDebug(ex, "Failed to dispose half-open connection backend for fd={Fd}", _connectionState.Fd); } - _connectionClosedTokenSource.Cancel(); + CancelConnectionClosedToken(); // A half-open handshake never reached mTLS validation, so _ownedClientCertificate is normally null // here; dispose defensively (no-op when null) to keep both teardown paths symmetric. @@ -404,6 +404,19 @@ private void OnTlsFatalError(Exception ex) Abort(new ConnectionAbortedException("TLS connection error", ex)); } + private void CancelConnectionClosedToken() + { + try + { + _connectionClosedTokenSource.Cancel(); + } + catch (Exception ex) + { + // a throwing callback must not escape and abandon the rest of the teardown + _logger.LogError(0, ex, $"Unexpected exception in {nameof(DirectTlsConnection)}.{nameof(CancelConnectionClosedToken)}."); + } + } + public override async ValueTask DisposeAsync() { // Thread-safe check: only one call to DisposeAsync proceeds @@ -452,7 +465,7 @@ public override async ValueTask DisposeAsync() } // 7. Signal connection closed - _connectionClosedTokenSource.Cancel(); + CancelConnectionClosedToken(); _connectionClosedTokenSource.Dispose(); // 8. Dispose the accepted client certificate. The runtime's TlsSession transferred ownership of the diff --git a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs index 88041be24659..be0ee657035c 100644 --- a/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.DirectTls/src/Connection/DirectTlsConnectionListener.cs @@ -51,7 +51,6 @@ internal sealed class DirectTlsConnectionListener : IConnectionListener private readonly IHostApplicationLifetime _appLifetime; private int _fatalErrorReported; - private Exception? _fatalError; public EndPoint EndPoint { get; private set; } @@ -155,8 +154,6 @@ internal void OnPumpFatalError(Exception error) return; } - // Publish the error before completing the channel so AcceptAsync observes it when it wakes. - _fatalError = error; _logger.LogCritical(error, "A DirectTls pump thread failed unrecoverably; stopping the application."); _readyConnections.Writer.TryComplete(error); @@ -165,26 +162,16 @@ internal void OnPumpFatalError(Exception error) public async ValueTask AcceptAsync(CancellationToken cancellationToken = default) { - try - { - // Wait for a connection that has completed handshake - var connection = await _readyConnections.Reader.ReadAsync(cancellationToken); - _connectionTracker.ReleaseHandshake(); - return connection; - } - catch (ChannelClosedException) + while (await _readyConnections.Reader.WaitToReadAsync(cancellationToken)) { - if (_fatalError is not null) + if (_readyConnections.Reader.TryRead(out var connection)) { - throw _fatalError; + _connectionTracker.ReleaseHandshake(); + return connection; } - - return null; - } - catch (OperationCanceledException) - { - return null; } + + return null; } public async ValueTask DisposeAsync() @@ -207,14 +194,7 @@ public async ValueTask DisposeAsync() // Drain any remaining connections from the channel while (_readyConnections.Reader.TryRead(out var connection)) { - try - { - await connection.DisposeAsync(); - } - catch - { - // Ignore errors during cleanup - } + await connection.DisposeAsync(); } // This listener owns its pump pool; stop the pump threads and release their epoll fds. Bound the wait so diff --git a/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsConnectionDisposeTests.cs b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsConnectionDisposeTests.cs new file mode 100644 index 000000000000..ac1dc0a68140 --- /dev/null +++ b/src/Servers/Kestrel/Transport.DirectTls/test/DirectTlsConnectionDisposeTests.cs @@ -0,0 +1,73 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Buffers; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls.Connection; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Microsoft.AspNetCore.Server.Kestrel.Transport.DirectTls; + +/// +/// Unit tests for teardown that has to survive application code. +/// Cancelling ConnectionClosed runs whatever the application registered on it, +/// and the steps after it release resources the process does not get back on its own - most importantly the +/// accepted client certificate, which owns a native key handle the transport is solely responsible for freeing. +/// Linux-only, matching the rest of the DirectTls suite. +/// +public class DirectTlsConnectionDisposeTests +{ + [ConditionalFact] + [OSSkipCondition(OperatingSystems.Windows | OperatingSystems.MacOSX)] + public async Task DisposeAsync_ConnectionClosedCallbackThrows_StillDisposesClientCertificate() + { + using var pump = new TlsEventPump(NullLogger.Instance, id: 0, Timeout.InfiniteTimeSpan); + var certificate = CreateSelfSignedCertificate(); + var connection = CreateConnection(pump, certificate); + + connection.ConnectionClosed.Register(static () => throw new InvalidOperationException("ConnectionClosed callback failed.")); + + // The throwing callback must not escape and abandon the rest of the teardown. + await connection.DisposeAsync(); + + Assert.True(connection.ConnectionClosed.IsCancellationRequested); + + // A disposed X509Certificate2 reports a null handle. Reaching this means disposal continued past the + // failing callback, so the native key handle is released instead of leaking once per accepted mTLS + // connection. + Assert.Equal(IntPtr.Zero, certificate.Handle); + } + + private static DirectTlsConnection CreateConnection(TlsEventPump pump, X509Certificate2 clientCertificate) + { + // The session is only reached by the graceful-shutdown step, which is already guarded, so the fd and + // session can stay fake: this test is about the ordering of the steps that follow the callback. + var connectionState = new ConnectionIoState( + fd: 101, + session: null!, + NullLogger.Instance); + + return new DirectTlsConnection( + connectionState, + pump, + localEndPoint: null, + remoteEndPoint: null, + MemoryPool.Shared, + maxReadBufferSize: 0, + maxWriteBufferSize: 0, + NullLogger.Instance, + clientCertificate: clientCertificate); + } + + private static X509Certificate2 CreateSelfSignedCertificate() + { + using var key = RSA.Create(2048); + var request = new CertificateRequest("CN=directtls-test-client", key, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + var now = DateTimeOffset.UtcNow; + return request.CreateSelfSigned(now.AddDays(-1), now.AddDays(1)); + } +} From 214f95450d93b822dde52fdc9c9361110bbe7472 Mon Sep 17 00:00:00 2001 From: Dmitrii Korolev Date: Fri, 21 Aug 2026 12:58:17 +0200 Subject: [PATCH 16/16] rephrase the comment --- .../Core/src/DirectTlsEndpointOptions.cs | 30 ++++++++----------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs b/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs index 2dce4a14d8a0..b997b5d2742a 100644 --- a/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs +++ b/src/Servers/Kestrel/Core/src/DirectTlsEndpointOptions.cs @@ -38,12 +38,10 @@ public sealed class DirectTlsEndpointOptions /// allocated so it carries the same connection id that will later serve the request); the second is the /// requested SNI host name, or when the client did not send one. /// - /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and - /// the callback is dispatched to the , so a slow callback no longer stalls the - /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, - /// and the time spent in it counts against - a callback that overruns that - /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent - /// invocation occupies a thread-pool thread. + /// The callback is invoked on the , so a slow one does not hold up the handshakes or + /// I/O of other connections. It does still delay this connection, and the time spent in it counts against + /// - a callback that overruns that budget costs this connection its handshake. + /// Prefer fast, non-blocking work regardless: each concurrent invocation occupies a thread-pool thread. /// /// public Func? ServerCertificateSelector { get; set; } @@ -69,12 +67,10 @@ public sealed class DirectTlsEndpointOptions /// when it produced no . /// /// - /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and - /// the callback is dispatched to the , so a slow callback no longer stalls the - /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, - /// and the time spent in it counts against - a callback that overruns that - /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent - /// invocation occupies a thread-pool thread. + /// The callback is invoked on the , so a slow one does not hold up the handshakes or + /// I/O of other connections. It does still delay this connection, and the time spent in it counts against + /// - a callback that overruns that budget costs this connection its handshake. + /// Prefer fast, non-blocking work regardless: each concurrent invocation occupies a thread-pool thread. /// public Func? ClientCertificateValidation { get; set; } @@ -87,12 +83,10 @@ public sealed class DirectTlsEndpointOptions /// (for example with ToArray()) if they must outlive the call. The first argument is the /// for the connection being negotiated. /// - /// This callback does not run on the epoll worker thread. The handshake for the connection is suspended and - /// the callback is dispatched to the , so a slow callback no longer stalls the - /// handshakes and I/O of the other connections assigned to that worker. It does still delay this connection, - /// and the time spent in it counts against - a callback that overruns that - /// budget costs this connection its handshake. Prefer fast, non-blocking work regardless: each concurrent - /// invocation occupies a thread-pool thread. + /// The callback is invoked on the , so a slow one does not hold up the handshakes or + /// I/O of other connections. It does still delay this connection, and the time spent in it counts against + /// - a callback that overruns that budget costs this connection its handshake. + /// Prefer fast, non-blocking work regardless: each concurrent invocation occupies a thread-pool thread. /// /// public Action>? TlsClientHelloBytesCallback { get; set; }