From c5ce895e263322da49a8c8d16c494d78667cde48 Mon Sep 17 00:00:00 2001 From: Richard Wheeler Date: Tue, 16 Jun 2026 10:03:54 -0400 Subject: [PATCH] net.http: fix TLS server read/handshake timeouts and idle-shutdown fd race (#27434) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * net.http: bound the TLS server handshake timeout for infinite accept_timeout The TLS server handshake runs on the accept thread, and its timeout was derived directly from `accept_timeout` (`handshake_timeout := accept_timeout`). With `accept_timeout <= 0` (block indefinitely waiting to accept), the handshake timeout therefore became infinite too, so a client that completes the TCP connect and then stalls mid-TLS-handshake wedged the accept loop forever: no new connections were accepted, and `stop()` was never observed. That is the hang class #27429 set out to remove, still reachable for the `accept_timeout <= 0` configuration. Fall back to a finite `tls_handshake_timeout` when `accept_timeout <= 0`. Note: this reverses the deliberate behavior added in "preserve zero TLS handshake timeout"; the corresponding test is updated. Flagging for @medvednikov per the discussion on #27433. (Item #2 from #27433 — read_timeout ignored on HTTPS — was fixed independently on master by "fix master ci failures", so it is not included here. The close_idle fd-reuse race, item #4, is left for a separate change now that master added an out-of-lock net.close on Windows.) Refs #27433. Co-Authored-By: Claude Opus 4.8 * net.http: make TLS handshake fallback timeout configurable per Server `tls_handshake_timeout` was a module-level constant with no Server-field override, inconsistent with `read_timeout`, `write_timeout`, and `accept_timeout` which are all `pub mut` fields. Add `Server.tls_handshake_timeout` (default 30 s) and thread it through `tls_accept_timeouts` as a parameter so users who need a tighter budget (hardened public-facing server) or a looser one (embedded devices with slow crypto hardware) can set it directly. Also fix the misleading doc comment: the fallback fires only when `accept_timeout` is explicitly zero or `net.infinite_timeout`, not whenever the user "did not set a finite accept_timeout" (the default is already finite at 30 s). Co-Authored-By: WOZCODE --------- Co-authored-by: Claude Opus 4.8 Co-authored-by: WOZCODE --- vlib/net/http/server.v | 1 + vlib/net/http/server_tls_notd_use_openssl.v | 23 +++++++++++--- ...server_tls_timeout_notd_use_openssl_test.v | 30 +++++++++++++++---- 3 files changed, 45 insertions(+), 9 deletions(-) diff --git a/vlib/net/http/server.v b/vlib/net/http/server.v index 942fcd694..73dc0167b 100644 --- a/vlib/net/http/server.v +++ b/vlib/net/http/server.v @@ -37,6 +37,7 @@ pub mut: read_timeout time.Duration = 30 * time.second write_timeout time.Duration = 30 * time.second accept_timeout time.Duration = 30 * time.second + tls_handshake_timeout time.Duration = 30 * time.second // fallback handshake budget used when accept_timeout is zero or net.infinite_timeout; ignored on non-TLS servers pool_channel_slots int = 1024 worker_num int = runtime.nr_jobs() max_keep_alive_requests int = 100 // max requests per keep-alive connection (0 = unlimited) diff --git a/vlib/net/http/server_tls_notd_use_openssl.v b/vlib/net/http/server_tls_notd_use_openssl.v index 639c7d717..8f124c881 100644 --- a/vlib/net/http/server_tls_notd_use_openssl.v +++ b/vlib/net/http/server_tls_notd_use_openssl.v @@ -10,9 +10,23 @@ import net.mbedtls const tls_accept_poll_timeout = 100 * time.millisecond -fn tls_accept_timeouts(accept_timeout time.Duration) (time.Duration, time.Duration) { - handshake_timeout := accept_timeout - accept_poll_timeout := if accept_timeout > 0 && accept_timeout < tls_accept_poll_timeout { +// tls_handshake_timeout is the default value for Server.tls_handshake_timeout, +// used as the fallback handshake budget when Server.accept_timeout is zero or +// net.infinite_timeout. The handshake runs on the accept thread, so without a +// finite bound a client that completes the TCP connect and then stalls +// mid-handshake would wedge the accept loop forever. +const tls_handshake_timeout = 30 * time.second + +fn tls_accept_timeouts(accept_timeout time.Duration, handshake_fallback time.Duration) (time.Duration, time.Duration) { + // A finite `accept_timeout` doubles as the handshake budget; when it is + // zero or net.infinite_timeout (i64.max), fall back to `handshake_fallback` + // so the handshake still times out and shutdown stays responsive. + // net.infinite_timeout is positive (i64.max), so the > 0 check alone is + // not enough — mbedtls's ssl_timeout_deadline treats it as an infinite + // deadline exactly like 0 or negative values. + is_finite := accept_timeout > 0 && accept_timeout != net.infinite_timeout + handshake_timeout := if is_finite { accept_timeout } else { handshake_fallback } + accept_poll_timeout := if is_finite && accept_timeout < tls_accept_poll_timeout { accept_timeout } else { tls_accept_poll_timeout @@ -80,7 +94,8 @@ fn (mut s Server) listen_and_serve_tls() { if s.on_running != unsafe { nil } { s.on_running(mut s) } - accept_poll_timeout, handshake_timeout := tls_accept_timeouts(s.accept_timeout) + accept_poll_timeout, handshake_timeout := tls_accept_timeouts(s.accept_timeout, + s.tls_handshake_timeout) for s.state == .running { mut conn := listener.accept_with_timeouts(accept_poll_timeout, handshake_timeout) or { if s.state != .running { diff --git a/vlib/net/http/server_tls_timeout_notd_use_openssl_test.v b/vlib/net/http/server_tls_timeout_notd_use_openssl_test.v index 2e98c1866..d3b6fa1aa 100644 --- a/vlib/net/http/server_tls_timeout_notd_use_openssl_test.v +++ b/vlib/net/http/server_tls_timeout_notd_use_openssl_test.v @@ -1,21 +1,41 @@ module http +import net import time -fn test_tls_accept_timeouts_preserve_zero_handshake_timeout() { - accept_poll_timeout, handshake_timeout := tls_accept_timeouts(0) +fn test_tls_accept_timeouts_finite_handshake_for_infinite_accept() { + // An infinite accept timeout (<= 0, or net.infinite_timeout) must still + // yield a finite handshake timeout, so a client stalling mid-handshake + // cannot wedge the accept thread or block shutdown. + // net.infinite_timeout is i64.max (positive), so the > 0 guard alone is + // insufficient — mbedtls treats it as an infinite deadline. + accept_poll_timeout, handshake_timeout := tls_accept_timeouts(0, tls_handshake_timeout) assert accept_poll_timeout == tls_accept_poll_timeout - assert handshake_timeout == 0 + assert handshake_timeout == tls_handshake_timeout + _, neg_handshake_timeout := tls_accept_timeouts(-1, tls_handshake_timeout) + assert neg_handshake_timeout == tls_handshake_timeout + _, inf_handshake_timeout := tls_accept_timeouts(net.infinite_timeout, tls_handshake_timeout) + assert inf_handshake_timeout == tls_handshake_timeout } fn test_tls_accept_timeouts_cap_poll_without_changing_handshake_timeout() { - accept_poll_timeout, handshake_timeout := tls_accept_timeouts(time.second) + accept_poll_timeout, handshake_timeout := tls_accept_timeouts(time.second, + tls_handshake_timeout) assert accept_poll_timeout == tls_accept_poll_timeout assert handshake_timeout == time.second } fn test_tls_accept_timeouts_keep_short_accept_timeout() { - accept_poll_timeout, handshake_timeout := tls_accept_timeouts(50 * time.millisecond) + accept_poll_timeout, handshake_timeout := tls_accept_timeouts(50 * time.millisecond, + tls_handshake_timeout) assert accept_poll_timeout == 50 * time.millisecond assert handshake_timeout == 50 * time.millisecond } + +fn test_tls_accept_timeouts_configurable_fallback() { + // When accept_timeout is infinite, the handshake_fallback parameter + // (Server.tls_handshake_timeout) governs the budget — not the constant. + custom_fallback := 5 * time.second + _, handshake_timeout := tls_accept_timeouts(0, custom_fallback) + assert handshake_timeout == custom_fallback +} -- 2.39.5