From b0d38a32e54dc85837fcc0e7b00bebba57ea0192 Mon Sep 17 00:00:00 2001 From: Diogo Martins Date: Mon, 12 Oct 2026 23:47:21 +0100 Subject: [PATCH] httpclient: HandshakeTimeoutMs ends a TLS handshake whose peer accepted and went silent The handshake deadline was checked only between flights, so a TLS connect parked in RecvAsync for the server's first flight waited forever when the server accepted and never answered. Through the pool only the acquire timeout bounded it, and each such open kept _live and _opening raised, so PoolSize of them stopped the origin's pool for as long as the origin held the sockets. A direct TlsClientContext.ConnectAsync caller had no bound at all. The handshake now arms one ring timeout of HandshakeTimeoutMs on the socket's own ring. If it fires while the stream is not disposed and SSL_in_init still says the handshake is running, it shuts the socket down (SHUT_RDWR), the move the server's handshake sweep makes. The parked recv then completes, and the loop reports "TLS handshake to '' did not complete within ms". The between-flights check goes: the timer bounds a dribbling peer too. RingSocket gains a public Host accessor, so the stream reaches the ring from the socket it is given, and direct ConnectAsync callers get the deadline as well as the pool. The pending test "tls client: HandshakeTimeoutMs bounds a handshake whose peer accepted and went silent" is now a Test, and "tls client: HandshakeTimeoutMs bounds a direct ConnectAsync whose peer accepted and went silent" pins the path with no pool behind it. --- .../ioxide.httpclient/Tls/TlsClientStream.cs | 28 +++-- src/ioxide/Client/RingSocket.cs | 2 + .../TlsClientPostureTests.cs | 101 ++++++++++++++---- 3 files changed, 100 insertions(+), 31 deletions(-) diff --git a/src/clients/ioxide.httpclient/Tls/TlsClientStream.cs b/src/clients/ioxide.httpclient/Tls/TlsClientStream.cs index e9278cd3..87f6d679 100644 --- a/src/clients/ioxide.httpclient/Tls/TlsClientStream.cs +++ b/src/clients/ioxide.httpclient/Tls/TlsClientStream.cs @@ -142,7 +142,8 @@ private static void ConfigureNames(nint ssl, TlsClientOptions options, string se private async ValueTask RunHandshakeAsync(TlsClientOptions options, string serverName) { - long deadlineMs = Native.MonotonicMs + options.HandshakeTimeoutMs; + var deadline = new HandshakeDeadline(this); + _socket.Host.SubmitTimeout(options.HandshakeTimeoutMs * 1_000_000L, deadline); while (true) { @@ -159,17 +160,12 @@ private async ValueTask RunHandshakeAsync(TlsClientOptions options, string serve throw new IOException($"TLS handshake to '{serverName}' failed: {OpenSsl.LastError()}"); } - // The deadline is checked between flights rather than raced against the recv: a ring - // operation cannot be abandoned mid-flight without its completion landing on a source - // someone else has since taken. A peer that goes completely silent is bounded by the - // caller's own acquire timeout instead. - if (Native.MonotonicMs >= deadlineMs) + int n = await _socket.RecvAsync(_inbound, CiphertextBufferSize); + if (deadline.Expired) { throw new IOException( $"TLS handshake to '{serverName}' did not complete within {options.HandshakeTimeoutMs} ms"); } - - int n = await _socket.RecvAsync(_inbound, CiphertextBufferSize); if (n <= 0) { throw new IOException(n == 0 @@ -360,4 +356,20 @@ public void Dispose() FreeCiphertextBuffer(_outbound); _socket.Dispose(); } + + // Shuts down a handshake still running at HandshakeTimeoutMs, which completes the recv it is parked on. + private sealed class HandshakeDeadline(TlsClientStream stream) : IRingCompletion + { + public bool Expired { get; private set; } + + public void Complete(int result) + { + // -ETIME: the wait ran its course. Disposed is checked first: by then SSL_free has run. + if (result is -62 or 0 && !stream._disposed && OpenSsl.SSL_in_init(stream._ssl) != 0) + { + Expired = true; + Native.shutdown(stream._socket.Fd, Native.SHUT_RDWR); + } + } + } } diff --git a/src/ioxide/Client/RingSocket.cs b/src/ioxide/Client/RingSocket.cs index 73682969..9645f2d9 100644 --- a/src/ioxide/Client/RingSocket.cs +++ b/src/ioxide/Client/RingSocket.cs @@ -22,6 +22,8 @@ public sealed unsafe class RingSocket : IDisposable public int Fd { get; } + public IRingHost Host => _host; + private RingSocket(IRingHost host, int fd) { _host = host; diff --git a/tests/Ioxide.Tests.Http/TlsClientPostureTests.cs b/tests/Ioxide.Tests.Http/TlsClientPostureTests.cs index 57d842a2..52b07ae1 100644 --- a/tests/Ioxide.Tests.Http/TlsClientPostureTests.cs +++ b/tests/Ioxide.Tests.Http/TlsClientPostureTests.cs @@ -15,14 +15,6 @@ namespace Ioxide.Tests; /// while one inside it is served - not that setting it was accepted. /// /// -/// Two live defects ride as Pending: -/// -/// - BuildAlpnWire casts each UTF-16 unit to a byte, so a non-ASCII protocol name is OFFERED as -/// a different protocol made of its chars' low bytes - the client twin of the server-side -/// defect in TlsService.BuildAlpnWire, proven there today; -/// - HandshakeTimeoutMs is only checked between handshake flights, so the one peer a handshake -/// timeout exists for - accepted the connect, then went silent - is not bounded by it at all. -/// /// Examined without a failing result, stated so they are not re-litigated: MinimumVersion = 0 is /// accepted (OpenSSL's "no floor"), but this box's system-wide MinProtocol makes the weakened /// floor unobservable, so no test can discriminate it here; a ServerName over 255 bytes makes the @@ -202,16 +194,16 @@ public static void Register(Runner runner) + "wire as the bytes 'h2' and the client negotiates - and NegotiatedAlpn reports - a " + "protocol nobody configured"); - runner.Pending("tls client: HandshakeTimeoutMs bounds a handshake whose peer accepted and went silent", () => + runner.Test("tls client: HandshakeTimeoutMs bounds a handshake whose peer accepted and went silent", () => { // The option says "how long the handshake may take before the connect fails", but - // RunHandshakeAsync checks its deadline only BETWEEN flights. A peer that accepts the - // TCP connect and then never answers the ClientHello - the one peer a handshake - // timeout exists for - leaves the connect parked in RecvAsync, where the deadline is - // never consulted. Through the pool that surfaces as the ACQUIRE timeout with no - // reason (the open never failed - it is still parked); for a direct - // TlsClientContext.ConnectAsync caller there is no second timeout, and the await - // simply never completes. + // RunHandshakeAsync used to check its deadline only BETWEEN flights. A peer that accepts + // the TCP connect and then never answers the ClientHello - the one peer a handshake + // timeout exists for - left the connect parked in RecvAsync, where the deadline was + // never consulted. Through the pool that surfaced as the ACQUIRE timeout with no + // reason (the open never failed - it was still parked); for a direct + // TlsClientContext.ConnectAsync caller there was no second timeout, and the await + // never completed. using var tarpit = new TarpitOrigin(); (string certPath, _) = TestCert.Ensure(); @@ -231,15 +223,35 @@ public static void Register(Runner runner) // Guard against passing vacuously: the tarpit really was reached and held the socket. Assert.True(tarpit.Accepted > 0, "the connect never reached the tarpit, so nothing was proven"); - // The claim: the failure the caller sees names the handshake deadline (the message the - // dribbling-peer path already produces). Today it is the pool's own acquire timeout - // with no cause attached, because the open is still parked when it fires. + // The claim: the failure the caller sees names the handshake deadline. It used to be + // the pool's own acquire timeout with no cause attached, because the open was still + // parked when it fired. Assert.True(body.Contains("did not complete within"), $"the handshake deadline should have fired and been named, got: {body}"); - }, "the deadline is only checked between handshake flights, so a peer that accepts and " - + "goes silent parks the connect in RecvAsync forever - HandshakeTimeoutMs never fires, " - + "and only the pool's acquire timeout (which a direct ConnectAsync caller does not have) " - + "bounds it"); + }); + + runner.Test("tls client: HandshakeTimeoutMs bounds a direct ConnectAsync whose peer accepted and went silent", () => + { + // The same tarpit without the pool: a direct TlsClientContext.ConnectAsync has no + // acquire timeout behind it, so the handshake deadline is the only bound it has. + using var tarpit = new TarpitOrigin(); + (string certPath, _) = TestCert.Ensure(); + TlsClientContext tls = TlsClientContext.Create(new TlsClientOptions + { + ServerName = "localhost", + AlpnProtocols = ["http/1.1"], + CaFile = certPath, + HandshakeTimeoutMs = 750, + }); + + int port = TestServer.Start((reactor, connection) => DirectConnectHandler(reactor, connection, tarpit.Port, tls)); + + (int status, string body) = Client.Get(port, "/tarpit-direct", timeoutMs: 20_000); + Assert.Equal(200, status); + Assert.True(tarpit.Accepted > 0, "the connect never reached the tarpit, so nothing was proven"); + Assert.True(body.Contains("did not complete within 750 ms"), + $"the handshake deadline should have ended the connect, got: {body}"); + }); } // --------------------------------------------------------------------------------------------- @@ -459,4 +471,47 @@ private static async Task ProxyHandler(Reactor reactor, TcpConnection connection connection.DecRef(); } } + + // One TLS connect straight to the origin, no pool, answered with how it ended. + private static async Task DirectConnectHandler(Reactor reactor, TcpConnection connection, int originPort, + TlsClientContext tls) + { + try + { + RecvSnapshot snapshot = await connection.ReadAsync(); + if (snapshot.IsClosed) + { + return; + } + Wire.ReadPath(connection, snapshot); + + string detail; + RingSocket socket = RingSocket.CreateTcp(reactor); + int connected = await socket.ConnectAsync("127.0.0.1", (ushort)originPort); + if (connected < 0) + { + socket.Dispose(); + detail = $"connect failed: errno {-connected}"; + } + else + { + try + { + using TlsClientStream stream = await tls.ConnectAsync(socket); // owns the socket from here + detail = "handshake completed"; + } + catch (IOException e) + { + detail = e.Message; + } + } + + Wire.Write(connection, 200, detail); + await connection.FlushAsync(); + } + finally + { + connection.DecRef(); + } + } }