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(); + } + } }