Commitfe4b756fRecorded4 Aug 2026Repositorysigil-tls
tls: make the handshake bound a TOTAL, not a per-read timeout
Message
The first version of this bound did not work, and its own tests could not see that. It set mbedtls_ssl_conf_read_timeout to the remaining budget on each iteration of the handshake loop and called that a whole-handshake deadline. It is not one: mbedtls_ssl_handshake does not return between handshake steps, and mbedtls_ssl_fetch_input passes the full configured read_timeout to EVERY partial read. So the loop-level re-budget ran once per handshake instead of once per read.
The consequence is that a peer which DRIPS defeats it entirely, where a peer that goes silent does not. Measured: one byte every 600ms made a 1000ms bound take 13.06s to fire, and against a drip that never stopped it had not fired after 120s. That is the same whole-process freeze the bound exists to prevent, reachable by trickling instead of stalling.
The deadline now lives in the BIO callbacks, which are the only place that sees every individual read, and it clamps each wait to the time remaining. mbedtls_ssl_set_timer_cb would also have worked, but mbedTLS cancels that timer at several points during a handshake; a clamp on the read cannot be cancelled. The per-read read_timeout is kept underneath as a backstop, since the two bound different things.
The handshake WRITE goes through the same clamp. A peer that accepts and never reads fills the window and the client flight blocks in send(). Rare with a flight of a few hundred bytes, but "rare" is not a bound.
Both the old rig and the old tests were the problem: every stall case used a peer that sends NOTHING, and no peer of that shape can distinguish a per-read bound from a total one. The drip peer that can is in sigil-http's test/integration/, where a live TLS peer is available.
Also removes a test that claimed to guard the "connected" status and in fact repeated the failing call above it verbatim, so *status_out = "connected" could have been deleted with all tests still green. "connected" is now asserted where a live peer exists, in sigil-http's integration controls.
Reported by an adversarial review of my own diff, verified by measurement.
Changed
RELEASE_NOTES_v0.16.5.md | 14 ++++++++++++--
native/tls.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
src/sigil/tls.sgl | 10 +++++++++-
test/test-tls.sgl | 17 +++++++++++++----
4 files changed, 168 insertions(+), 30 deletions(-)Diff
RELEASE_NOTES_v0.16.5.mdmodified
@@ -17,8 +17,18 @@ two new procedures report why a connection attempt failed.
17
## Changes 18
19
- `tls-connect hostname port [connect-timeout-ms [handshake-timeout-ms]]`−20
bounds the handshake. The deadline covers the whole handshake rather than−21
each individual read, so a peer sending one byte at a time cannot extend it.+20
bounds the handshake. The deadline is a TOTAL over the handshake, enforced+21
on each individual read as a clamp on how long that read may wait.+22
+23
That distinction carries the whole value of this release, so it is worth+24
stating why. A bound applied only per read is defeated by a peer that DRIPS+25
rather than one that goes silent: mbedTLS does not return between handshake+26
steps and restarts its read timeout on every partial read, so each dribbled+27
byte resets the clock. Measured against an implementation that got this+28
wrong, a peer sending one byte every 600 ms made a 1000 ms bound take 13+29
seconds to fire, and it never fired at all while the drip continued. A+30
per-read timeout is still kept underneath as a backstop; the two bound+31
different things and neither replaces the other. 32
- `tls-upgrade socket hostname [handshake-timeout-ms]` gains the same bound. 33
An upgraded socket is already connected, so all of its handshake sits past 34
the connect phase and its unbounded window was strictly wider thannative/tls.cmodified
@@ -95,6 +95,11 @@ typedef struct {
95
* mode it must hand control back instead of spinning on a socket that 96
* will not accept bytes. */ 97
int nonblocking;+98
/* Absolute monotonic deadline (ms) for a handshake in progress, or 0 for+99
* none. Read by the deadline-aware BIO callbacks, which are the only+100
* place that sees every individual handshake read. Always back to 0+101
* before an established connection is handed to a caller. */+102
long handshake_deadline_ms; 103
} TlsConnectionData; 104
105
/* Symbol used as type tag for TLS connection foreign objects */@@ -351,6 +356,100 @@ static int sigil_tls_connect_timeout(mbedtls_net_context *ctx,
356
#define SIGIL_TLS_HS_TIMEOUT 1 357
#define SIGIL_TLS_HS_FAILED 2 358
+359
#ifndef _WIN32+360
/*+361
* Deadline-aware BIO callbacks, used ONLY while a bounded handshake is in+362
* progress. p_bio is the TlsConnectionData, so the deadline is reachable+363
* from inside the callback.+364
*+365
* WHY THE DEADLINE HAS TO LIVE HERE rather than in the caller's loop, which+366
* is where I first put it and where it did not work:+367
*+368
* mbedtls_ssl_handshake does not return between handshake steps, and+369
* mbedtls_ssl_fetch_input loops `while (in_left < nb_want)` passing the+370
* FULL configured read_timeout to every partial read. A deadline enforced+371
* in the caller's loop is therefore consulted once per handshake, not once+372
* per read, and a peer that DRIPS bytes resets a fresh full-length timeout+373
* on each one. Measured: against a peer sending one byte every 600 ms, a+374
* 1000 ms bound took 13.06 s to fire, and against a peer that never+375
* stopped dripping it had not fired at all after 120 s.+376
*+377
* mbedtls_ssl_check_timer would have caught it, but only if a timer+378
* callback is installed, and mbedTLS cancels that timer at several points+379
* in the handshake. Clamping the timeout on the read itself needs no timer+380
* and cannot be cancelled: this is the one place that sees every read.+381
*/+382
static int sigil_tls_bio_remaining(TlsConnectionData *conn, long *out)+383
{+384
if (conn->handshake_deadline_ms <= 0) {+385
*out = 0; /* no deadline in force */+386
return 0;+387
}+388
*out = conn->handshake_deadline_ms - sigil_tls_now_ms();+389
return 1;+390
}+391
+392
static int sigil_tls_bio_recv_timeout(void *ctx, unsigned char *buf, size_t len,+393
uint32_t timeout)+394
{+395
TlsConnectionData *conn = (TlsConnectionData *)ctx;+396
long remaining;+397
+398
if (sigil_tls_bio_remaining(conn, &remaining)) {+399
if (remaining <= 0) {+400
return MBEDTLS_ERR_SSL_TIMEOUT;+401
}+402
/* Never wait past the whole-handshake deadline, whatever mbedTLS+403
* asks for. This is the clamp that makes the bound a total. */+404
if (timeout == 0 || (long)timeout > remaining) {+405
timeout = (uint32_t)remaining;+406
}+407
}+408
return mbedtls_net_recv_timeout(&conn->server_fd, buf, len, timeout);+409
}+410
+411
static int sigil_tls_bio_recv(void *ctx, unsigned char *buf, size_t len)+412
{+413
/* mbedTLS prefers f_recv_timeout when set, so this is a fallback. Route+414
* it through the same clamp rather than letting it block unbounded. */+415
return sigil_tls_bio_recv_timeout(ctx, buf, len, 0);+416
}+417
+418
static int sigil_tls_bio_send(void *ctx, const unsigned char *buf, size_t len)+419
{+420
TlsConnectionData *conn = (TlsConnectionData *)ctx;+421
long remaining;+422
+423
/* The handshake WRITE can block too: a peer that accepts and never reads+424
* fills the window, and the client flight then blocks in send(). Rare+425
* with a flight of a few hundred bytes, but "rare" is not a bound. */+426
if (sigil_tls_bio_remaining(conn, &remaining)) {+427
fd_set wset;+428
struct timeval tv;+429
int sel;+430
+431
if (remaining <= 0) {+432
return MBEDTLS_ERR_SSL_TIMEOUT;+433
}+434
FD_ZERO(&wset);+435
FD_SET(conn->server_fd.fd, &wset);+436
tv.tv_sec = remaining / 1000L;+437
tv.tv_usec = (remaining % 1000L) * 1000L;+438
sel = select(conn->server_fd.fd + 1, NULL, &wset, NULL, &tv);+439
if (sel == 0) {+440
return MBEDTLS_ERR_SSL_TIMEOUT;+441
}+442
if (sel < 0) {+443
if (errno == EINTR) {+444
return MBEDTLS_ERR_SSL_WANT_WRITE;+445
}+446
return MBEDTLS_ERR_NET_SEND_FAILED;+447
}+448
}+449
return mbedtls_net_send(&conn->server_fd, buf, len);+450
}+451
#endif /* !_WIN32 */+452
453
/* 454
* Run the TLS handshake on an already-connected conn->server_fd. 455
*@@ -358,20 +457,21 @@ static int sigil_tls_connect_timeout(mbedtls_net_context *ctx,
457
* original blocking handshake: blocking mbedtls_net_recv as the BIO and a 458
* loop that only retries on WANT_READ/WANT_WRITE. 459
*−361
* With a positive handshake_timeout_ms, handshake reads are driven through−362
* mbedtls_net_recv_timeout, which selects on the socket and returns−363
* MBEDTLS_ERR_SSL_TIMEOUT when nothing arrives. This is the segment that−364
* connect-timeout-ms does NOT cover: once a peer has ACCEPTED the TCP−365
* connection, the connect phase is over and the handshake read that follows−366
* was previously unbounded. A peer that accepts and then never sends a−367
* ServerHello wedged a production monitoring service for 55 days.+460
* With a positive handshake_timeout_ms, the handshake's reads AND writes go+461
* through deadline-aware BIO callbacks that clamp every wait to the time+462
* remaining. This is the segment that connect-timeout-ms does NOT cover:+463
* once a peer has ACCEPTED the TCP connection the connect phase is over, and+464
* the handshake that follows was previously unbounded. A peer that accepts+465
* and then never sends a ServerHello wedged a production monitoring service+466
* for 55 days. 467
*−369
* The deadline is on the WHOLE handshake, not per read: each iteration−370
* re-budgets the remaining time, so a peer dripping one byte just under the−371
* interval cannot extend the wait indefinitely.+468
* The deadline is a TOTAL over the handshake. It is enforced inside the BIO+469
* rather than in the loop below, for the reason documented on those+470
* callbacks: a loop-level deadline is consulted once per handshake and a+471
* dripping peer walks straight through it. 472
*−373
* The read timeout is unwound before returning, so an established−374
* connection's subsequent tls-read calls behave exactly as before. Only the+473
* Everything is unwound before returning, so an established connection's+474
* subsequent tls-read and tls-write behave exactly as before. Only the 475
* handshake is bounded here. 476
*/ 477
static int sigil_tls_run_handshake(TlsConnectionData *conn, long handshake_timeout_ms)@@ -381,20 +481,14 @@ static int sigil_tls_run_handshake(TlsConnectionData *conn, long handshake_timeo
481
#ifndef _WIN32 482
if (handshake_timeout_ms > 0) { 483
int outcome = SIGIL_TLS_HS_OK;−384
long deadline = sigil_tls_now_ms() + handshake_timeout_ms; 484
−386
mbedtls_ssl_set_bio(&conn->ssl, &conn->server_fd,−387
mbedtls_net_send, mbedtls_net_recv,−388
mbedtls_net_recv_timeout);+485
conn->handshake_deadline_ms = sigil_tls_now_ms() + handshake_timeout_ms;+486
mbedtls_ssl_conf_read_timeout(&conn->conf, (uint32_t)handshake_timeout_ms);+487
mbedtls_ssl_set_bio(&conn->ssl, conn,+488
sigil_tls_bio_send, sigil_tls_bio_recv,+489
sigil_tls_bio_recv_timeout); 490
491
for (;;) {−391
long remaining = deadline - sigil_tls_now_ms();−392
if (remaining <= 0) {−393
outcome = SIGIL_TLS_HS_TIMEOUT;−394
break;−395
}−396
mbedtls_ssl_conf_read_timeout(&conn->conf, (uint32_t)remaining);−397
492
ret = mbedtls_ssl_handshake(&conn->ssl); 493
if (ret == 0) { 494
break;@@ -408,9 +502,16 @@ static int sigil_tls_run_handshake(TlsConnectionData *conn, long handshake_timeo
502
outcome = SIGIL_TLS_HS_FAILED; 503
break; 504
}+505
/* WANT_* on a blocking BIO means the deadline is the only thing+506
* that can end this; check it so the retry cannot spin. */+507
if (conn->handshake_deadline_ms - sigil_tls_now_ms() <= 0) {+508
outcome = SIGIL_TLS_HS_TIMEOUT;+509
break;+510
} 511
} 512
513
/* Restore unbounded reads + the plain BIO for the session. */+514
conn->handshake_deadline_ms = 0; 515
mbedtls_ssl_conf_read_timeout(&conn->conf, 0); 516
mbedtls_ssl_set_bio(&conn->ssl, &conn->server_fd, 517
mbedtls_net_send, mbedtls_net_recv, NULL);@@ -856,6 +957,12 @@ static Value native_tls_write(SigilVM *vm, int argc, Value *args)
957
} 958
959
if (ret < 0) {+960
/* Report bytes already COMMITTED rather than collapsing to #f:+961
* they are on the wire, and a caller that resent them would+962
* corrupt the stream. With nothing committed, #f as before. */+963
if (total_written > 0) {+964
break;+965
} 966
return SIGIL_FALSE; /* Error */ 967
} 968
@@ -1077,7 +1184,11 @@ cleanup_error:
1184
* mbedtls_net_free keeps mbedTLS's own bookkeeping out of a path where 1185
* server_fd may never have been handed to it. */ 1186
if (conn->server_fd.fd >= 0) {+1187
#ifdef _WIN32+1188
closesocket(conn->server_fd.fd);+1189
#else 1190
close(conn->server_fd.fd);+1191
#endif 1192
} 1193
conn->server_fd.fd = -1; /* Prevent double-close in mbedtls_net_free */ 1194
mbedtls_net_free(&conn->server_fd);src/sigil/tls.sglmodified
@@ -51,7 +51,15 @@
51
;;; sends a ServerHello previously blocked the handshake read forever. 52
;;; Under Sigil's cooperative scheduler that freezes the whole process, 53
;;; so this is the difference between a failed request and a dead−54
;;; service. The bound covers the whole handshake, not each read.+54
;;; service.+55
;;;+56
;;; The bound is a TOTAL over the handshake, not a per-read one. That+57
;;; distinction is the whole value: a per-read bound is defeated by a+58
;;; peer that DRIPS rather than one that goes silent, because mbedTLS+59
;;; restarts its read timeout on every partial read. Measured against an+60
;;; earlier build, one byte every 600 ms made a 1000 ms per-read bound+61
;;; take 13 s to fire, and never fire at all while the drip continued.+62
;;; 63
;;; Omitted or <= 0 keeps the original blocking handshake. 64
;;; 65
;;; ```schemetest/test-tls.sglmodified
@@ -90,10 +90,19 @@
90
(assert-equal "tcp-connect-failed" (car r)) 91
(assert-false (cdr r)))) 92
−93
(test "tls-connect/status reports connected only with a live peer"−94
;; Guards the other direction: "connected" is not produced by a failure.−95
(let ((r (tls-connect/status "127.0.0.1" 1 1000 1000)))−96
(assert-false (equal? "connected" (car r)))))+93
;; NOT TESTED HERE: the "connected" status, and the bound holding against a+94
;; peer that DRIPS rather than going silent. Both need a live TLS peer,+95
;; which this suite cannot stand up in-process, and both are covered by+96
;; sigil-http's test/integration/run-timeout-tests.sh:+97
;; "tls-connect/status reports connected for a live peer" and "the+98
;; handshake bound fires against a DRIPPING peer, not just a silent one".+99
;;+100
;; Said out loud because an earlier version of this file had a test here+101
;; claiming to guard the "connected" direction which in fact repeated the+102
;; failing call above verbatim. It asserted that a REFUSED connection is+103
;; not reported as connected, which is not the same claim at all, and+104
;; `*status_out = "connected"` could have been deleted from the C with all+105
;; 20 tests still green. 106
107
(test "handshake timeout bounds an accept-and-stall peer" 108
(let* ((server (tcp-listen 0))