Repository navigation
fix(tls,h3): TLS audit round 3 (#362-#373) with the retire-ring, QUIC-config and renegotiation clusters (#351, #359, #360, #374, #376) - #383
Merged
Conversation
loadCertChainMem exited its chain loop on the first nil from PEM_read_bio_X509 and cleared the error queue unconditionally. That function returns nil for every failure, not only end-of-data: bad base64, a truncated block, an ASN.1 decode failure, an allocation failure. So a certPem of leaf + mangled intermediate loaded as a successfully-built, silently leaf-only chain. buildTlsCtx only checks the return value and SSL_CTX_check_private_key validates the leaf against the key, never the chain, so the context was accepted and installed: the server started with no diagnostic, reloadTls returned true, and clients without the intermediate cached failed the handshake with "unable to get local issuer certificate". The two cases are distinguishable through the error queue, which is how OpenSSL's own SSL_CTX_use_certificate_chain_file tells them apart: a clean end of PEM data leaves PEM_R_NO_START_LINE (lib 9, reason 108), real corruption leaves something else (bad base64 decode is reason 100). loadCertChainMem now mirrors that loader: ERR_peek_last_error after the loop, clear and accept only for ERR_LIB_PEM / PEM_R_NO_START_LINE, and return false otherwise so buildTlsCtx raises and a reload is rejected. The queue is cleared on entry so the peek sees only this loader's errors, and a real error is left in place so buildTlsCtx reports its cause. ERR_peek_last_error joins the crypto FFI block; ERR_GET_LIB and ERR_GET_REASON are static inlines in <openssl/err.h>, so errGetLib/errGetReason mirror them in Nim, including the ERR_SYSTEM_ERROR special case. The h3 shim's loadCertChain had the identical loop and gets the identical treatment, so the QUIC context cannot be built from a chain the TCP context would reject. Tests: tests/test_tls_material_integrity.nim. A leaf + corrupt block + intermediate PEM is rejected at startup and by the QUIC engine, a clean leaf + intermediate still loads and serves 2 certificates (counted from openssl s_client -showcerts), and reloadTls rejects a corrupt chain file while keeping the running one. Verified both new cases fail against the unfixed sources (startup raised nothing; vq_engine_new accepted the corrupt chain). Fixes #367
loadCaMem treated every nil from PEM_read_bio_X509 as clean end-of-data and returned success whenever at least one CA had been added, so a corrupt or truncated clientCaPem installed a partial trust store and reported no error. With CA #1 and #2 parsed and CA #3 cut off mid-certificate, the loader added 2, cleared the error, broke, and returned true because added > 0. buildTlsCtx succeeded, the server started, and TLS looked healthy, while every client issued by CA #3 or later was rejected at handshake time with an unable-to-get-issuer alert: a self-inflicted partial outage that presents as a client-side problem, with nothing at startup to correlate against. This is the trust anchor set for client-certificate verification, so config-time validation of it is exactly what must not be skipped, and `added > 0` is too weak a contract for it. loadCaMem now applies the same error-queue discrimination as the chain loader (pemReadEndedCleanly: clean PEM end-of-data versus a real parse error) and additionally requires that the whole BIO was consumed cleanly, not merely that something loaded. A bundle that stops early is rejected, so buildTlsCtx raises and the caller's "bad material rejects the config/reload" promise holds for the CA source too. Tests: the client-CA-bundle cases in tests/test_tls_material_integrity.nim. A clean two-CA bundle is accepted and verifies a client certificate issued by the *last* CA in it; the same bundle truncated mid-certificate, and the same bundle with a mangled base64 block, are both rejected at startup. Both rejection cases were verified to fail against the unfixed loader (the server started with the partial store). Fixes #368
applyClientVerify loaded a CA only when one was configured, so verifyClient with both clientCaFile and clientCaPem empty fell through both branches and armed SSL_CTX_set_verify against a completely empty X509_STORE. OpenSSL 3 does not populate a new context's store and SSL_CTX_set_default_verify_paths is called nowhere in the tree, so the store really is empty. buildTlsCtx accepted that context and validateConfig had no check pairing verifyClient with a CA source, so the combination was reachable straight through initVortexConfig and the server started cleanly. Under Require every client presenting a certificate then failed with "unable to get local issuer certificate", rejecting 100% of connections with no startup diagnostic. Under Optional it is worse: clients that send no certificate connect fine, so the deployment looks healthy while client-certificate authentication is in fact non-functional and clientCertSubject is always "". Rejected in both places the issue offers, rather than one. validateConfig raises a named error, which is what an operator should see, and applyClientVerify returns false so buildTlsCtx raises: that covers direct TlsConfig users and every context rebuild. Falling back to the system trust store was not an option here; if that is ever wanted it has to be an explicit opt-in calling SSL_CTX_set_default_verify_paths, not the accidental behaviour of an empty store. reloadTlsConfig cannot drop the CA (it passes cfg.clientCaFile/cfg.clientCaPem through unchanged and nothing else mutates them), so a config that validated stays valid across reloads; the applyClientVerify guard covers that path regardless. Docs: README, HARDENING.md and the VortexConfig field comment now say a CA is required for a non-None verifyClient. Tests: three cases in the mTLS suite of tests/test_tls_advanced.nim (Require at startup, Optional at startup, and newTlsConfig directly for the context-level guard). All three were verified to fail against the unfixed code, where the server and the context both came up. Fixes #369
alpnSelect returned SSL_TLSEXT_ERR_NOACK when nothing in the client's
ALPN list matched the server's. OpenSSL implements NOACK as "behave as
if no callback were present": no ALPN extension goes back in the
ServerHello/EncryptedExtensions and the handshake completes. RFC 7301
3.2 is explicit that the server SHALL respond with a fatal
no_application_protocol alert instead, and OpenSSL's own documentation
for this callback assigns ALERT_FATAL to exactly this case and NOACK to
"no ALPN protocols are configured for this connection".
The practical effect: a client advertising only h3 that reached the TCP
port, or only a legacy name like spdy/3.1, completed the handshake,
driveHandshake recorded an empty c.alpn, and the connection was fed to
the HTTP/1 parser, which answered 400 or left it hanging until the idle
timeout. Any client that treats a completed handshake as proof its offer
was honoured, which RFC 7301 entitles it to, misframes the connection.
The project's own QUIC shim already returned ALERT_FATAL, so the TCP and
QUIC paths disagreed on the same question; they now agree.
The no-ALPN case is unaffected and must stay that way: OpenSSL does not
invoke the select callback when the ClientHello carries no ALPN
extension, so there is no list to have no overlap with. Verified
empirically against OpenSSL 3, and locked in by a test.
Tests, in the TLS suite of tests/test_tls.nim: an -alpn bogus client
gets alert number 120 ("no application protocol") and s_client exits
non-zero; a client offering no ALPN completes the handshake and is
served over HTTP/1.1; h2 and http/1.1 still negotiate. The alert case
was verified to fail against the unfixed callback (handshake succeeded,
exit 0). No existing test relied on the fallback. README and
HARDENING.md now state the behaviour.
Fixes #370
reloadTlsConfig's certFile branch clears pkcs12/pkcs12File; the keyFile branch did not. loadCertKey gives a PKCS#12 bundle unconditional precedence, so on a p12-configured server reloadTls(keyFile = ...) left the bundle in place, took the p12 branch, never opened the new key, rebuilt the *old* certificate and key, passed SSL_CTX_check_private_key because those two still match each other, and returned true. That is the one input combination that silently no-ops while reporting success, and the scenario it breaks is the worst one: an operator rotating a disclosed private key gets positive confirmation while the server keeps presenting the compromised key indefinitely. A secondary effect was that cfg.material then recorded the ignored keyFile, so a later cert-only reload paired a new certificate with a key path from that ignored call. Fixed by taking the issue's stronger option rather than only making the branches symmetric: a lone key against a p12-sourced certificate is meaningless, so keyFile.len > 0 with certFile.len == 0 on p12 material is rejected before anything else happens, and the bundle fields are cleared in the keyFile branch too so the two branches agree. Rejecting outright rather than relying on the follow-on empty-certFile failure keeps the reason at the point of the decision. Nothing is persisted on a rejection: `m` is a copy and `cfg.material = m` runs only after buildTlsCtx has returned a context, which this change preserves (the new rejection returns before any of it). Reported the same way as every other reload failure, as false, which server.reloadTls passes straight through and which also suppresses the h3 reload signal. Tests: a "PKCS#12 reload" suite in tests/test_tls_advanced.nim. A key-only reload on a p12 server returns false and the old bundle's certificate is still presented; a following cert-only reload also fails (proving the ignored keyFile was not persisted); a cert+key reload succeeds and the new certificate is served. Verified the suite fails against the unfixed code, where both the key-only and the cert-only reload returned true and the served subject changed. Fixes #363
The issue's code reading is correct: buildTlsCtx set no OpenSSL options
at all, SSL_CTX_set_options appeared nowhere in src/, and nothing in the
tree disabled renegotiation. Its premise about the consequence is not:
OpenSSL 3.0 already refuses *client*-initiated renegotiation unless
SSL_OP_ALLOW_CLIENT_RENEGOTIATION is set
(SSL_CTX_set_options(3ssl): "Client-initiated renegotiation is disabled
by default"), which an s_client -tls1_2 session sending R confirms
against the unpatched server: it gets a no_renegotiation alert. So the
unmetered-DoS scenario as described is not reachable on a current
OpenSSL 3.
The option is still worth setting rather than closing the issue,
because what stands between this server and that scenario is otherwise a
library default it does not state: a system openssl.cnf can turn client
renegotiation back on through the ssl_conf module, and the Linux dynlib
pattern this module binds ("libssl.so(.3|)") also matches a pre-3.0
libssl, where client-initiated renegotiation is permitted by default.
SSL_OP_NO_RENEGOTIATION makes the refusal this server's policy, covers
server-initiated renegotiation too, and costs one call. A client that
asks still gets a warning-level no_renegotiation alert and keeps its
connection; TLS 1.3, session resumption and tickets are untouched.
Not applied to the QUIC context in vq_ngtcp2.cpp makeCtx: it pins both
min and max proto version to TLS 1.3, which has no renegotiation
mechanism, so the option would be dead code there. TLS 1.3 KeyUpdate is
a separate mechanism and remains reachable on both paths, as the issue
notes.
Tests, in a new "TLS renegotiation" suite in tests/test_tls.nim: the
built context reports SSL_OP_NO_RENEGOTIATION (via a new
ctxRefusesRenegotiation introspection proc, in the style of the existing
ctxCertSubject), and an openssl s_client -tls1_2 session sending R is
answered with a no_renegotiation alert with exactly one handshake having
run, after which the listener still serves. The first case was verified
to fail without the SSL_CTX_set_options call; the second passes either
way on OpenSSL 3 precisely because of the default above, and is there to
pin the observable behaviour.
Docs: HARDENING.md now states that renegotiation is refused and why.
Fixes #376
reloadTlsConfig published the new SSL_CTX with an atomic exchange but
scanned and mutated the retire ring (cfg.retired / cfg.retiredAt) with
plain loads and stores, and read the stored material/OCSP fields at the
top of the call only to rewrite them at the bottom. Two threads reloading
at once, which the documented contract ("call from an ordinary thread, not
a raw signal handler") plainly allows, could therefore claim the same
retire slot: either both freed the same SSL_CTX, which is heap corruption,
or one overwrote the other's entry and that context leaked. The material
copy is worse than a lost update, since both threads walk the same
shared-memory strings while one of them is replacing them.
reloadTlsConfig now runs its whole body under a new reloadLock held in the
TlsConfig, initialised where the config is created and destroyed in
freeTlsConfig. Locking the entire call rather than just the ring keeps the
read-modify-write of the material and OCSP fields atomic too, and costs
nothing that matters: reloads are rare and already do file I/O, while loop
threads do not take this lock at all (newTlsSession keeps its lock-free
atomic load of cfg.ctx), so handshakes never block behind a reload.
Serialising is preferred over documenting the restriction away: a SIGHUP
thread plus an admin endpoint is a reasonable deployment, and the safe
reading of the contract should be the true one.
tests/test_tls_reload_race.nim runs four threads doing 60 reloads each
against one config, then checks every reload succeeded, the surviving ctx
still serves the expected certificate, and the config frees cleanly.
Without the lock it aborts on a double free inside the retire ring
(tls.nim:627) 3 runs out of 3; with it, 5 runs pass, including one built
with -d:useMalloc -fsanitize=address.
Fixes #360
The retire ring protected a displaced SSL_CTX with a time-based grace window, but its eviction path freed one unconditionally, and eviction only ran when no slot was free after the sweep, which by construction means all four retained contexts were displaced less than ctxGraceSec ago. So the one moment the ring had to break its invariant was the moment the invariant mattered. Refcounting did not cover it either: newTlsSession loaded cfg.ctx atomically and only gained a reference when SSL_new up-ref'd it, so between those two statements a loop thread held a bare pointer. A renewal hook, a config-file watcher firing per certbot write or a retried admin endpoint reloading five times inside the window could free the context such a thread had just loaded, destroying its cert, key, session cache and (through ocspExFree) its OCSP blob before SSL_new dereferenced it. newTlsSession now calls acquireCtx, which loads cfg.ctx and up-refs it under a new short-lived ctxLock, and drops that extra reference once SSL_new has taken its own. reloadTlsConfig publishes the new context under the same lock and then releases the old one immediately. A reader either loads the old context before the swap, holding a reference that turns the release into a decrement, or loads the new one after it, so a displaced context now lives exactly as long as the last session created on it. That is a real guarantee rather than an approximation, and it retires the slot cap, the grace window and the ring itself, along with the cross-thread read-modify-write the ring needed. In-flight connections keep the certificate they handshook with, however many reloads follow. Only the up-ref runs under ctxLock, not SSL_new: SSL_new costs about 1.7 us per session (300k sessions, release build, M-series), so holding the lock across it would serialise every loop thread on a 1.7 us critical section. Up-ref alone is ~30 ns and immeasurable here: the same benchmark reads 1688-1693 ns/op with the lock and 1737-1759 ns/op with the previous bare atomic load, i.e. the difference is inside the noise. ctxLock is taken from inside reloadTlsConfig's reloadLock and never the other way round, so the two cannot deadlock. ctxCertSubject, the other reader of cfg.ctx, now takes a reference too, since it hands out a borrowed X509 from the context. tests/test_tls_reload_race.nim gains a burst test (120 reloads on one thread while three others create and free sessions continuously) and an end-to-end one (40 reloadTls calls through the public API, far more than the four old slots and well inside the 5 s window, while three threads open connections that each make a loop thread build a session; a TLS connection established before the burst must still complete its request afterwards). Both pass, and pass under -d:useMalloc -fsanitize=address. Neither can be shown to fail before this commit: the ring evicted its *oldest* slot, so reproducing the use-after-free needs a loop thread descheduled between its load and SSL_new for the span of four reloads, which a stress test cannot force. They are regression tests for the lifetime rule, not reproductions of the window. Fixes #364
cbRecvStreamData handled a negative nghttp3_conn_read_stream by scheduling a CONNECTION_CLOSE and returning 0 to ngtcp2. nghttp3 documents that return as a connection error after which "calling nghttp3 API other than nghttp3_conn_del causes undefined behavior", but c->h3 stayed alive and reachable: returning 0 told ngtcp2 the callback succeeded, so ngtcp2_conn_read_pkt kept decoding the remaining frames of the same packet and the packets coalesced behind it in the datagram. Every one of those paths calls back into the poisoned connection (another read_stream, close_stream, add_ack_offset, unblock_stream), and the c->wantClose gate in vq_engine_recv only stops the NEXT datagram. One hostile datagram (a malformed frame sequence on one request stream, any bytes on a second) was therefore a remotely triggerable crash of the loop thread, taking every other connection on it down. failConn now deletes the nghttp3_conn and nulls c->h3 before doing anything else, which is the one call the library still allows, and cbRecvStreamData returns NGTCP2_ERR_CALLBACK_FAILURE so ngtcp2 abandons the rest of the datagram. Every other nghttp3 call site is guarded by `if (c->h3)`, so they all become no-ops; the one that was not (cbStreamOpen's set_stream_user_data) is now guarded too, and cbHandshakeCompleted no longer rebuilds an h3 for a connection that is already closing, so a killed nghttp3_conn is never resurrected. ~Conn already tolerated a null h3. vq_engine_recv turns the CALLBACK_FAILURE into the terminal packet ngtcp2 documents for that case, and because failConn already set c->wantClose the CONNECTION_CLOSE still carries the inferred HTTP/3 application error code rather than a transport error. The same nghttp3 contract applies to a negative nghttp3_conn_writev_stream in writeConn, which previously set c->closed and returned, leaving c->h3 alive and reachable from vq_stream_write / vq_submit_response until the next pump reaped the connection. It now goes through failConn and emits the CONNECTION_CLOSE (via the extracted sendConnClose helper) instead of going silent. Tested by tests/test_http3_malformed.nim, which replays the two STREAM frames of the hostile datagram into the shim's recv_stream_data callback (tests/vq_h3_malformed.cpp compiles the shim's translation unit to reach it; curl cannot emit a malformed HTTP/3 frame sequence and the aioquic harnesses in conformance/ need docker). Without the fix the suite dies with SIGSEGV on the second frame, which is the reported crash. Fixes #362
makeCtx configured exactly three things on the QUIC SSL_CTX: a hardcoded TLS 1.3 min/max, the ALPN callback, and the cert/key. settings. tlsCipherSuites, minTlsVersion and maxTlsVersion reached the TCP TlsConfig only, so an operator who restricted TLS 1.3 suites for a compliance requirement had that restriction enforced on HTTP/1.1 and HTTP/2 and silently not in force on any HTTP/3 connection, which negotiated whatever OpenSSL's defaults allowed. Nothing logged the discrepancy, so the configuration audited as applied. VqConfig now carries the suite list and the version ceiling into makeCtx, which calls SSL_CTX_set_ciphersuites and fails the engine on an invalid list (fail closed, as with a bad cert). Two settings are deliberately not carried, and the header says so: tlsCipherList is the TLS <= 1.2 cipher list, which no QUIC handshake can ever consult, and the configured *minimum* needs no field because QUIC mandates TLS 1.3 (RFC 9001 4.2), which clamps any lower floor up unconditionally. The ceiling cannot be clamped the same way: honoring maxTlsVersion = V12 would mean not serving HTTP/3 at all. Rather than let the policy be enforced on one transport and ignored on the other, validateConfig now rejects that combination with a message naming http3 = false as the fix, and makeCtx refuses the same combination as a fail-closed backstop (h3 then stays off and unadvertised). Pinning h3 to 1.3 while the TCP listener caps at 1.2 was the alternative, and it is the behaviour the issue calls out as the bug. tests/test_tls_polish.nim's TLS 1.2 cap case is updated to pass http3 = false, which is now how that policy is expressed. The conformance TLS harness already ran every version case with http3 = false. Tested by tests/test_tls_h3_policy.nim: a server restricted to TLS_CHACHA20_POLY1305_SHA256 negotiates exactly that suite over h3 (and over TCP), and the rejected version combination raises at start. Without the fix the h3 case negotiates TLS_AES_256_GCM_SHA384 (OpenSSL's default preference) while the TCP case honors the restriction, and the config case starts a server instead of raising. Fixes #359
makeCtx never called SSL_CTX_set_verify or loaded a client CA, and VqConfig had no fields to carry either, so verifyClient reached the TCP TlsConfig only. A server configured with ClientVerify.Require plus a clientCaFile advertised h3 through Alt-Svc and then completed the QUIC handshake with a client that presented no certificate at all: the mTLS requirement was enforced on TCP and silently absent on HTTP/3, which is an authentication bypass reachable by following the server's own advertisement. req.clientCertSubject returned "" over h3 as well (backend.nim left H3Conn.ssl nil), so an application could not even tell an absent client cert from the transport not reporting one. VqConfig now carries the SSL_VERIFY_* mode and the client CA, and makeCtx applies them through an applyClientVerify with the same shape and precedence as the TCP path's (in-memory PEM preferred over the file, no CA required, verify set last). It fails closed: CA material that will not load fails vq_engine_new rather than yielding an engine that accepts unauthenticated connections. The in-memory loader mirrors the error-queue discipline #368 adds on the Nim side: PEM_read_bio_X509 returning null is end of data only when the queue's last reason is PEM_R_NO_START_LINE, so a truncated bundle fails instead of installing the CAs before the damage. vq_conn_ssl exposes the connection's SSL object (as void*, keeping the header plain C), which cbAccept stores in H3Conn.ssl, so the existing request.nim path reads the peer certificate over h3 with no change. cbConnClose clears it alongside the VqConn, since the shim frees both right after it returns. ~Conn now calls SSL_set_app_data(ssl, NULL) before SSL_free, which ngtcp2's ossl backend documents as mandatory when the ngtcp2_conn does not outlive the SSL (it is deleted two lines earlier). SSL_free still runs the QUIC record-layer callbacks for crypto data OpenSSL never consumed, and those resolve the conn_ref in app data to reach the ngtcp2_conn and its ossl ctx. Nothing hit that before because no handshake failed that late; a rejected client certificate does, and it aborted the whole loop thread in crypto_ossl_ctx_release_crypto_data. The fix is small and required for this change to work at all, so it lands here rather than as a separate issue. Tested by tests/test_tls_h3_mtls.nim: an h3 request with no client certificate is refused, one with a valid certificate is served, its subject is visible through req.clientCertSubject, and the TCP listener behaves the same. Without the verify wiring the first h3 request is served (rc 0, body returned) and the subject is empty; with the verify wiring but H3Conn.ssl left nil the subject case alone fails; without the app-data fix the suite aborts on the refused handshake. Fixes #351
Confirmed against the source before fixing (the issue was filed unverified): makeCtx configured min/max protocol version, the ALPN callback and one cert/key, there was no SSL_CTX_set_tlsext_servername_callback anywhere in the shim, and ngSetup had no sni parameter. One SSL_CTX, one certificate, per engine. With http3 defaulting to true, a server using the README's own SNI snippet advertises h3 through Alt-Svc, and a browser that retries api.example.com over QUIC is served the DEFAULT certificate and aborts with a name mismatch, marking h3 broken, while the identical request over TCP gets the right certificate. Nothing logs. VqConfig now carries the per-host material (VqSniCert), and the engine keeps an owned copy of it plus one SSL_CTX per host, built by the same makeCtx as the default context so a host inherits the client verification, cipher suites and TLS 1.3 pinning rather than drifting from them. A servername callback on the default context switches the connection with SSL_set_SSL_CTX: exact host first, then a `*.example.com` wildcard over exactly one label, mirroring the TCP path's precedence. The QUIC match is case-insensitive. DNS names are case-insensitive and a client may send any casing, so an exact-byte comparison would silently fall back to the default certificate. (The TCP path is byte-exact today; that is issue #358 and is left alone here.) SSL_set_SSL_CTX is safe under the ngtcp2 ossl backend: it replaces the certificate and the context-level settings, while the QUIC record-layer callbacks and transport parameters that ngtcp2_crypto_ossl_configure_server_session installs live on the SSL object, so they survive the switch. The regression test completes h3 requests on a switched context, which exercises that end to end. vq_engine_reload_cert rebuilds every per-host context from the material it was configured with, so per-host certificate files replaced by the same renewal are picked up along with the default pair (reloadTls names only the default one, and the certbot pattern rotates all of them). The rebuild is all or nothing: a failure keeps the contexts that were serving, so a broken per-host certificate cannot quietly drop that host back to the default certificate. The contexts are unique_ptr members, so they are freed with the engine, and a connection that already switched to one keeps it alive through its own reference. Tested by tests/test_tls_h3_sni.nim (exact host, wildcard, default host, unmatched host, TCP parity, a request served after the switch, and the reload rebuild). Without the callback the SNI hosts are served CN=localhost over h3 while TCP serves the right certificate; without the reload rebuild the rotated per-host certificate is not picked up. Fixes #374
driveHandshake updated selector interest on only two of its four
outcomes. The tlsWantRead arm did nothing, so a handshake that had
already passed through a WANT_WRITE (its flight did not fit in the
socket send buffer, armWrite set interest to {Read, Write}) kept write
interest armed for the rest of the handshake.
The selector is level-triggered on both kqueue and epoll, and the
dispatcher routes every event on a handshaking connection straight back
into driveHandshake. So once the peer drained the blocked flight, the
fd was permanently writable: wake on Write, SSL_do_handshake, EAGAIN
read, WANT_READ, interest unchanged, wake again. One stalled handshake
pinned a whole loop thread at 100% CPU until headerTimeout (or
writeTimeout) reaped it, starving every other connection on that
thread.
The WANT_READ arm now calls disarmWrite: read interest is already
included by armWrite, so only the write side needs clearing, and the
same call drops the stale writeDeadline armWrite installed (the flight
it was watching has drained, so the handshake is no longer blocked on
the socket).
tests/test_tls_io.nim is new: a hand-built TLS 1.2 ClientHello from a
raw socket, a certificate chain fat enough that the server's flight
cannot fit in the socket buffers, and a client that drains that flight
and then never sends its second one. It samples process CPU
(getrusage) over a 1 s idle window. Without the fix that window costs
0.98 s of CPU; with it, 0.0002 s (threshold 0.3 s). The connection is
still reaped by headerTimeout, which the test also asserts.
Fixes #365
flushOut's tlsWantRead arm returned with c.writeArmed still true from
the preceding tlsWantWrite. Interest stayed {Read, Write} on a socket
that is writable (that is why the flush ran at all), and the selector
is level-triggered, so the dispatcher re-entered flushOut on every
selector pass: SSL_write, EAGAIN read, WANT_READ, repeat, until
writeTimeout reaped the connection. One core per such connection.
The arm also skipped the wsBackpressure and h2DrainResume that both
neighbouring stall arms perform, leaving a WebSocket waiting for its
backpressure signal and an HTTP/2 producer parked on the buffer cap
unnotified for the duration.
The arm now runs the same notifications, disarms write and keeps read
interest (handleRead's tail re-enters flushOut once the peer's bytes
arrive). The write-stall deadline is re-armed explicitly through the
new armWriteDeadline, split out of armWrite: this flush IS stalled, so
disarmWrite's "output fully flushed, no deadline" meaning does not
apply, and a peer that never sends the TLS input SSL_write is waiting
for must still be reaped by writeTimeout.
No regression test: the arm cannot be reached from a client against
OpenSSL 3. Three independent blockers, each verified on this host with
a raw OpenSSL 3.6.3 client driving a renegotiation against the server
while a 4 MiB response was stalled mid-flush:
* OpenSSL 3 servers refuse client-initiated renegotiation unless
SSL_OP_ALLOW_CLIENT_RENEGOTIATION is set, which vortex never sets;
the ClientHello never starts a handshake.
* with that option patched in temporarily, the renegotiation dies
earlier still: SSL_read cannot flush the partially written
application-data record that the stall left pending, and fails with
SSL_R_BAD_LENGTH ("bad length") before any handshake begins.
* the TLS 1.3 alternative (a KeyUpdate) is answered from OpenSSL's
write path, so it surfaces as WANT_WRITE from SSL_read (issue #372),
never as WANT_READ from SSL_write.
The fix is therefore hardening against a reachable-on-paper arm (a
future OpenSSL default, a server-initiated renegotiation, a build that
allows client renegotiation) and is free either way. The suite gains a
liveness test for the neighbouring, reachable stall instead: a TLS peer
that stops reading a 4 MiB response must park the flush without
spinning the loop (0.0002 s of CPU over a 1 s window) and the flush
must resume the moment it reads again.
Fixes #371
handleRead armed write interest when SSL_read returned WANT_WRITE, but nothing ever retried the read. OpenSSL's contract is that the *same* operation is repeated once the socket is writable: the bytes it must emit first (a TLS 1.3 KeyUpdate answer, a renegotiation flight, an alert) live in the SSL object's own write buffer, not in c.wbuf. The dispatcher's write branch ran flushOut instead, which with pendingOut == 0 skips its loop entirely, clears wbuf, disarms write and never touches the SSL object; handleRead is only reached when Event.Read is also set, which it is not on a pure writability wakeup. The record was swallowed and sat in OpenSSL's buffer until the connection timed out. The connection now records the debt in sslReadWantsWrite (set on the WANT_WRITE arm, cleared immediately before each retry of the same call), the dispatcher's write branch routes such a connection into handleRead before the flush, and flushOut holds write interest while the flag is set so the retry it depends on still gets its event. The flag is false on every plaintext connection, so the plaintext paths are unchanged. No regression test: the hang needs pendingOut == 0 while the socket send buffer is exactly full (any free space at all and OpenSSL's record goes out, and any pending output at all makes the next SSL_write flush it), and a client cannot arrange that, because the send-buffer capacity is not observable from outside the server and TCP window updates are not byte exact. Verified instead by injecting the condition in a throwaway build: the first TLS read on the loop thread consumes the peer's record out of the kernel (so no further readable event can fire for it) and reports WANT_WRITE, handing the record back on the retry. Without this commit the whole TLS suite's first request is never read and the client fails with "connection was closed before full request has been made"; with it, every suite passes. Fixes #372
tlsPending/SSL_pending were bound but never called, so nothing ever asked whether OpenSSL was holding decrypted bytes. The recv loop's two early exits both document the invariant "the bytes we did not take wait in the kernel as TCP backpressure until the next readable event", and both are false under TLS: tlsRead is called with wanted = rbuf room, and when that is smaller than the plaintext left in the current record SSL_read returns `wanted` and keeps the remainder in its own buffer while the socket has already been drained of that record. A level-triggered fd with an empty socket never fires again, and the resume paths only cleared their flag and waited for an event that cannot come. An HTTPS upload to an `await req.read()` handler stalled until bodyTimeout mid-request, and a pipelined HTTPS request decrypted behind a `blocking:` worker was never parsed after the unpin. handleRead's tail now settles the debt on the live pause/pin state (noteSslPending): drivable now, the connection goes on a per-loop sslReady queue (fd + generation, like the deferred flushes, so a recycled slot is skipped rather than read under its new occupant); still paused or pinned, the debt is remembered on the connection and re-queued from resumeBodyImpl (the body ack), releasePin (the worker unpin) and resumeAfterRespond (a deferred response that lifts a pause), none of which may re-enter the recv loop themselves: resumeBodyImpl can run inside feedBody inside handleRead, where a second recv loop would corrupt rbuf mid-move. The loop drains the queue once per pass after the event batch, skipping anything blocked again (which goes back on the flag, so this cannot spin), and does not wait on the selector at all while the queue is non-empty, so a paused-then-resumed body is not left until some unrelated event wakes the loop. An empty queue costs one length check per pass. The regression test covers the pinned-worker exit deterministically: a `blocking:` handler holds the connection while the client sends one pipelined request as a single full-size TLS record, which the server's SSL_read must drain from the kernel in full and can only partly return. Without the fix only the first response arrives and the second request is never served; with it both are. The read-ahead exit is the same mechanism and code path but cannot be forced from a client: it needs the 1 MiB read-ahead high-water (a compile-time constant) to be crossed inside a single receive pass, and whether that happens depends on how much the kernel has buffered when the loop runs. Fixes #366
beginLingerClose exempted TLS connections from the drain-before-close sequence on the grounds that "TLS has its own close_notify". But close_notify is a TLS-layer record: it does nothing to stop the kernel sending RST instead of FIN when close() runs with unread data still in the receive queue, and that RST discards the send queue, taking the response the client has not read yet and the close_notify with it. The window is wider under TLS than plaintext, not narrower, because close_notify is queued in the same doomed send buffer. So a pipelined HTTPS request whose predecessor was answered with a 4xx (respondError sets closeAfterFlush + lingerClose) left its ciphertext unread in the receive queue, and the client saw a connection reset with no body: exactly the truncation beginLingerClose exists to prevent, and exactly what a reverse proxy in front of the server reports as an incomplete upstream response. TLS now takes the same sequence as plaintext: tlsShutdown (close_notify into the send queue), shutdown(SHUT_WR), csDraining plus handleDrain until the peer's FIN or the 5 s drain deadline, then close. handleDrain reads raw bytes with recv(), which is right here: the point is only to empty the receive queue. The connection records that close_notify has been sent (tlsCloseNotified) so closeConn does not run a second SSL_shutdown over an already half-closed socket, where it could only fail into the error queue; the session is still freed exactly once, since closeConn is the only caller of freeTlsSession and nils c.ssl. The regression test floods a TLS connection's request head past maxHeaderSize and reads the 431 back. Without the fix the client's own write dies mid-flood with a reset on all five iterations; with it, all five close cleanly (FIN) with the 431 delivered. The RST is timing dependent on macOS, hence the repeats. THREAT_MODEL's truncated-error row now says HTTPS takes the same path. Fixes #373
…rder The union merge of the per-fix changelog entries appended eight of them below the Unreleased "Changed" heading. Move them back under "Fixed" and order the whole block by the commits that introduced it.
This was referenced Oct 2, 2026
cryo2010
added a commit
that referenced
this pull request
Oct 5, 2026
… the soak-harness and accept-drop findings (#387, #388) (#389) * fix(stress): keep a canary failure's cause on stdout (#387) A soak cell that failed for any reason outside `Fail` and `asyncio.TimeoutError` recorded no cause at all. The canary's unhandled exception left its traceback on stderr, the canary container ran without `2>&1`, and the harness is driven as `nimble stress | tee stress.log`, which tees only stdout, so the archived log held a bare `FAILED (exit 1)` and the cell was undiagnosable (the 2026-10-03 streamupload h1 cell). run.sh now merges the canary's stderr into the tee'd stream, and the client's `main` has a catch-all that prints `FAIL <workload>: unexpected <type>: <msg>` plus the full traceback to stdout before returning 1, with an `ExceptionGroup` arm because a library's `TaskGroup` can raise a group that neither existing handler matches (`asyncio.gather` itself re-raises only the first exception). `KeyboardInterrupt` and `SystemExit` still propagate. The fire-and-forget `reporter` and `loop_watchdog` tasks, which `main` cancels without awaiting, now report their own death on stdout: their exception was never retrieved, so a dead reporter and a healthy quiet run both looked like zero report lines. The chaos sidecar prints its traceback for the same reason and its catch-all covers `BaseException`, so a `CancelledError` escaping `run()` exits 2 (internal error) rather than 1, which run.sh reads as an fd leak. * fix(h3): size the QUIC receive buffer to the max_udp_payload_size we advertise (#380) ngReceive read datagrams into a 2048-byte buffer with flags 0, while acceptConn left tp.max_udp_payload_size at ngtcp2's 65527 default and so told every peer that datagrams up to 65527 bytes were fine. A conforming client on a large-MTU path (a 9000-byte VPC MTU, 65536 on loopback) took us at our word: the kernel truncated the datagram, header protection and AEAD failed, ngtcp2 dropped the packet, the client retransmitted the same oversize datagram, and the connection died on the idle timer with nothing logged at either end. kMaxUdpPayload (1452) is the send-side path-MTU guess and was never the receive bound. Both numbers now come from one shim constant, kMaxRecvUdpPayload, which acceptConn writes into the transport parameters and which ngSetup reads through vq_max_recv_udp_payload to size the per-loop receive buffer. The advertisement was deliberately not clamped down to 2048: that would cap every datagram the peer sends us and cost throughput on exactly the paths this is about. On Linux the receive passes MSG_TRUNC, so a datagram that still does not fit is counted and dropped rather than handed to ngtcp2 as apparent line corruption; macOS recvfrom has no MSG_TRUNC semantics, so there the larger buffer is the whole mitigation. * fix(h3): bound QUIC ingress per loop pass so a UDP flood cannot starve TCP (#381) ngReceive drained the UDP socket with `while true` until recvfrom returned <= 0, and every datagram is decrypted and parsed synchronously (vq_engine_recv -> ngtcp2_conn_read_pkt -> nghttp3) before the next recvfrom. A source whose datagrams cost the server more than they cost the sender therefore kept the loop thread inside the receive loop, and the HTTP/1.1 and HTTP/2 fds on that same thread were not serviced at all. Reproduced with a flood of Initial packets naming an unsupported QUIC version, each of which ngtcp2 answers with a Version Negotiation packet (a parse plus a sendto per datagram): a plain HTTP/1.1 request on the same single-loop server went unserved past curl's 2 s timeout. ngReceive now takes at most ngRecvBudget (256) datagrams and reports whether it stopped on the budget rather than on an empty socket. The loop records that in quicMorePending and, exactly as it does for its sslReady queue, does not wait on the selector while it is set: the backlog is still drained promptly, but the TCP fds get serviced between batches. The budget is a work quantum, not a rate limit, and is the shape of the accept-side suspendAccept backoff. Address validation is unchanged and still absent: a spoofed-source flood can make acceptConn commit per-connection state up to maxConnections. The comment there now says what does and does not bound that today. Retry tokens stay out of scope. * fix(loop,stress): count and explain every accept-path connection drop (#388) handleAccept had four ways to let go of a connection the kernel had already handed us, and none of them left a trace: the maxConnections cap closed the fd silently, startTls returning false (newTlsSession nil) closed it silently, registerHandle raising swallowed the reason in `except CatchableError`, and suspendAccept wrote one bare stderr line the stress harness never captured. At the client all four are a socket that opens and dies with nothing on it, which httpx reports as an empty ConnectError, so a server shedding load on purpose was indistinguishable from a broken network. A 1-hour soak lost a streamdownload h2 sync cell to exactly that 90 s in with nothing anywhere to say which path fired. Each drop now bumps a per-loop counter plus a process-wide atomic tally, readable as server.acceptDrops() / vortex.acceptDrops() (and a no-argument acceptDrops(), because a {.gcsafe.} handler cannot touch the Vortex ref), and writes one line saying why: which cap was hit, the OpenSSL reason behind the nil session, the selector's own message. The lines are rate-limited to one per cause per 5 s per loop thread so a flood cannot turn the diagnosis into its own denial of service, and the first occurrence of each cause is never suppressed. They go through a single new sink, opLog, which the other operator messages in the loop and in server.nim now use instead of bare stderr.writeLine. On the harness side the stress server exposes the tally on /drops (/stats keeps its exact three-field contract with the client) and prints it on SIGTERM, and run.sh dumps the last 200 lines of the server container's log before removing it when a cell fails, so the server's stderr survives in the tee'd run log instead of dying with the container. Re-running the failing cell for 120 s against this build passed clean, with every drop counter at zero; the host was near idle, where the original soak ran eight cells at load 20-50, so the observability is the deliverable here and the root cause stays open. * fix(tls): report the real reason for a private-key load failure (#377) loadKeyMem called ERR_clear_error() on its failure path, so the OpenSSL error queue was empty by the time buildTlsCtx read it with lastErrorMsg(). Every private-key problem -- a wrong keyPassword, a truncated PEM block, a key in an encoding OpenSSL's decoder rejects -- was therefore reported as "cannot load TLS certificate/key: unknown TLS error", which looks like a library fault rather than the configuration mistake it is. The certificate path never cleared the queue, which made the asymmetry easy to miss. loadKeyMem now clears on entry (so what it reports is its own error, not a leftover from an earlier step) and leaves the reason queued on failure, and lastErrorMsg drains the queue after reading the oldest entry so no stale error is attributed to a later failure or mistaken for corruption by pemReadEndedCleanly. * fix(tls): match SNI host names case-insensitively (#358) cstrEq and wildMatch compared the requested server name to the configured host byte for byte, but host names are case-insensitive (RFC 6066, and DNS generally). A client that sent Example.com for a configured example.com matched no host, fell through to the default certificate and failed the handshake on a name mismatch. Both helpers now fold ASCII case on both sides, so the configured host does not have to be lower-cased either. The fold is ASCII-only rather than locale-aware on purpose: a locale tolower maps I to a dotless i under tr_TR, which would make certificate selection depend on the server's locale. The h3 shim already folds case this way (hostEq/lcAscii), so the two paths now agree. * fix(tls): free the partial config when an SNI context fails to build (#361) buildTlsCtx raising inside newTlsConfigWith's per-host loop let the exception escape a half-initialized config: the default ctx, every per-host ctx built before the failing one, the createShared(TlsConfig) block and its two locks were all abandoned. A startup failure usually precedes process exit, so the practical impact is small, but an embedder that catches the raise and retries with corrected configuration leaks on every attempt, and a leak-checked test run reports noise that hides real findings. The loop now unwinds through freeTlsConfig (forward-declared for this) and re-raises with the host name prefixed. buildTlsCtx only ever sees the material, so without that an operator with several SNI entries could not tell which host was broken. * fix(tls): re-register the SNI servername callback on reload (#355) newTlsConfigWith installed three things on the default SSL_CTX that buildTlsCtx leaves to its caller -- the ALPN callback, the OCSP staple, and the servername callback plus its arg -- but reloadTlsConfig re-registered only ALPN on the replacement ctx. The servername callback therefore went away with the old ctx: from the first reloadTls() onwards every configured SNI host was served the default certificate and failed the handshake on a name mismatch, until the process restarted. The fault appears only after a reload, so it surfaces in production long after deployment. Both paths now call one installDefaultCbs helper, so the initial build and the reload cannot drift apart again. * fix(tls): rebuild the per-host SNI contexts on reload (#356) reloadTlsConfig rebuilt only the default ctx, so a per-host SNI context kept the certificate it loaded at startup for the lifetime of the process and eventually served an expired one. The per-host certificates renew on the same ~90-day cycle as the default, and there was no API to rotate them short of a restart. TlsConfig now stores each host's TlsMaterial next to its host name and ctx, and the reload rebuilds every per-host ctx from it, so a bare reloadTls() re-reads the per-host files too (the certbot pattern). reloadTlsConfig and server.reloadTls take an `sni` override that replaces the per-host material wholesale, host set included, persisted only on success like cfg.material. The reload is all-or-nothing with the default ctx: one per-host certificate that fails to build rejects it and leaves everything as it was, rather than half-rotating or dropping a host back to the default certificate, which the client would reject as a name mismatch. The new table is published and the old ctxs released under ctxLock, and servernameCb now holds that lock across its whole lookup rather than reading cfg.sniCtx bare: the reload swaps host names, material and ctxs in one critical section, so a handshake can neither index a ctx array that disagrees with the host list it scanned nor have its chosen ctx freed between the load and SSL_set_SSL_CTX, which up-refs what it is handed. The `sni` override reaches the TCP listener only; the h3 engine rebuilds its per-host contexts from its own configured material (a file re-read) on the same reload, which is how #374 already works. The asymmetry is documented on reloadTls and in HARDENING.md. * fix(tls): record and log why a certificate reload was rejected (#378) reloadTlsConfig caught the exception carrying the only diagnostic that existed and returned a bare false, and server.reloadTls passed that on without writing anything anywhere or exposing an accessor. A certbot deploy hook that got false had no way to learn whether the certificate was unreadable, the key mismatched, the OCSP file missing, the cipher string rejected, or the material fine. A large share of this audit's findings end in "the operator has no signal"; the reload path's own silence is the one that made the others hard to diagnose. Every false return now goes through reloadFailed, which stores the reason on the config under reloadLock and writes one stderr line "vortex: TLS reload failed: <reason>" (the convention applyQuicReload follows), so a hook that ignores the bool still leaves a trace. That covers contradictory OCSP arguments, an unreadable explicit ocspFile, a key-only reload against a PKCS#12 bundle, the default ctx build failure, and the per-host SNI failures, which name the host. A successful reload clears the reason. lastTlsReloadError reads it back on TlsConfig, Server and Vortex; under -d:plainHttp it names the reason reloadTls always has there. * fix(h3): initialize the ngtcp2 ossl backend once per process, not per engine ngtcp2_crypto_ossl_init allocates an OpenSSL ex_data index and stores it in a library-level global; it is documented as a once-per-process initializer and is not thread safe. vq_engine_new called it directly, and vq_engine_new runs once per loop thread as newLoop brings the loops up, so on a numThreads > 1 server several threads ran it at the same time. Each extra call leaked an index, and while they raced a session configured under one index could be read back under another, which yields a null crypto context and a failed handshake: only at startup, only under the race, and indistinguishable from a flaky client. It now runs under a std::call_once that captures the return code, so every engine sees the same verdict and a failure fails them all instead of leaving some loops on a half-initialized backend (#357). The race cannot be provoked on demand, so the test pins the invariant instead: tests/vq_h3_tls_ctx.cpp hammers the shared initializer from eight threads and reports how many times it actually ran, and tests/test_http3.nim adds a four-loop h3 server that must start and serve. * fix(h3): clear the QUIC context's certificate chain before loading a new one loadCertChain called SSL_CTX_use_certificate and then appended each following PEM block with SSL_CTX_add0_chain_cert. SSL_CTX_use_certificate does not touch the chain -- unlike SSL_CTX_use_certificate_chain_file, which clears it first -- so loading into a context that already held a certificate left the previous leaf's intermediates in place and stacked the new ones on top. loadPkcs12 has the same shape through SSL_CTX_add1_chain_cert. On the initial makeCtx the chain is empty, so this only bit an in-place reload: after a CA rotated its intermediate, h3 clients were handed the new leaf together with the old, no-longer-valid intermediates, and the chain grew on every subsequent reload. Both loaders now clear the chain first (#354). Per-host (buildSniCtxs) contexts were never affected: they are built fresh through makeCtx. Nothing observes the chain length from an h3 client, so the regression test drives the loaders directly through tests/vq_h3_tls_ctx.cpp: loading a leaf + intermediate five times must still leave one chain certificate, and likewise for a PKCS#12 bundle carrying its CA. * fix(h3): rebuild the QUIC context on reload instead of mutating it in place vq_engine_reload_cert wrote the new material into the live SSL_CTX, certificate first then key, with no validation and no rollback. OpenSSL's ssl_set_cert silently frees the existing private key when the new leaf does not match it, and ssl_set_pkey silently frees the existing certificate when the new key does not match, so an unreadable, missing or mismatched key left the loop's context holding one half of a pair: every later h3 handshake on that loop failed, for good, while applyQuicReload logged "keeping the current certificate on this loop", which is precisely what had not happened. A cert-only reload reported success. SSL_CTX_check_private_key was never called. The reload now builds a fresh context through makeCtx from the new material plus the engine's retained TLS policy (passphrase, verify mode, cipher suites, version pinning, client CA) and swaps it in only after everything loaded and the halves match. The per-host (SNI) rebuild joins the same transaction, so a failure anywhere leaves every context untouched. Releasing the engine's reference to the old context immediately is safe: SSL_new up-refs the SSL_CTX, so an in-flight connection keeps the certificate it handshook with. Error reporting goes with it (#383 follow-up 2): makeCtx and the loaders no longer swallow the reason with a bare ERR_clear_error, makeCtx records the step that failed plus OpenSSL's own text, and the new vq_engine_last_error carries it to Nim. applyQuicReload includes it in its stderr line, and so does the "HTTP/3 engine setup failed" line in newLoop, which had the same generic wording (#352). A reload that fails only on the QUIC side is one server.reloadTls rejects before it signals the loops, so the regression tests drive the engine ABI directly through tests/vq_h3_tls_ctx.cpp. With the fix reverted they show the old behaviour exactly: a mismatched key leaves the engine serving the new subject with no usable key, and a cert-only reload returns success. * fix(tls,h3): make a bare reloadTls() rotate the HTTP/3 certificate A reloadTls() with no arguments means "re-read the configured material", and the TCP listener has always honoured that. Nothing resolved it on the QUIC side: applyQuicReload read the two paths out of the CertReload signal and called readFile(cf) on them, so an empty path raised and the reload failed. That is exactly the certbot pattern the project documents, so after a renewal HTTP/1.1 and HTTP/2 picked up the new certificate and HTTP/3 kept serving the one loaded at startup until it expired -- a protocol-specific outage 90 days after deployment, with h1 and h2 healthy throughout. The engine already stored the per-host Material (#374); it now stores the default certificate's material the same way (cert/key paths, in-memory PEM, PKCS#12 bundle and file, passphrase), and vq_engine_reload_cert takes PEM file PATHS rather than PEM blobs, with empty/null meaning "rebuild from the configured material, re-reading any files" through the same fresh-context path. Explicit paths replace whatever the material was sourced from under the TCP path's reloadTlsConfig rules -- a certificate path clears the in-memory PEM and the bundle, a key-only reload against a PKCS#12-sourced certificate is refused -- and what loads successfully becomes what the next bare reload re-reads. For material configured as PEM bytes or a bundle there is nothing to re-read, so the rebuild is a correct no-op for the default certificate and still refreshes the per-host files. applyQuicReload passes the paths straight through: no readFile, nothing to raise (#353). Tests: an h3 server started from cert/key files, its files overwritten with a second certificate and reloaded with no arguments, must present the new subject over both --http3-only curl and the TCP listener; a certPem-configured server's bare reload must return true and keep serving. Both fail with the fix reverted (h3 stays on the old subject while TCP rotates). The white-box suite covers the material-merge rules the public API cannot reach. * fix(tls,h3): refuse certificate material outside its validity window Nothing checked notBefore/notAfter on either transport. buildTlsCtx verified that the key matched the certificate and nothing else, and the shim's makeCtx did the same, so an expired certificate loaded cleanly and reloadTls() returned true. A renewal hook racing a certbot symlink swap, or one pointed at archive/example.com/ instead of live/, got a success and every new connection from that moment failed at the client with certificate_expired, masked until the in-flight ones turned over. Verified against the source and reproduced: a reload to a cert dated 2020 (and to one dated 2040) was accepted on both transports and immediately served. The leaf's validity window is now checked right after the cert/key consistency check, in buildTlsCtx and in makeCtx, so it covers startup and reload, default and per-host certificates, TCP and QUIC. Hard failure, no clock-skew allowance, no warning-only mode: startup raises "certificate expired at <notAfter>" or "certificate not valid until <notBefore>", and a reload is rejected with the running certificate left serving. This is a behaviour change -- a server that previously started while serving an expired certificate now refuses to start -- so it is called out in README.md, HARDENING.md and the CHANGELOG (#379). Audited tests/, conformance/, demos/ and bench/ for static certificate fixtures first: there are none. Every certificate is minted at run time with openssl (-days 2 in the suites, -days 3650 in the conformance images), so nothing needed regenerating. * fix(h3): install a process-wide, rotating session-ticket key on every QUIC context Verified against the source: vq_engine_new runs per loop thread and each engine builds its own SSL_CTX through makeCtx, OpenSSL generates a fresh random TLS 1.3 ticket key per context, and SSL_CTX_set_tlsext_ticket_key_evp_cb appears nowhere in the tree. A ticket was therefore decryptable only by the loop that issued it, while which loop receives a returning client's first datagram is decided by the kernel's SO_REUSEPORT hash over its new 4-tuple. On an N-loop server roughly (N-1)/N of resumption attempts fell back to a full handshake, invisibly: the connection succeeded, one round trip slower, every time. The contexts stay per-loop, since the per-loop in-place reload is built on them. A process-wide ticket key is installed on every one instead -- default and per-host, initial and rebuilt -- generated once under the same std::call_once that initializes the ngtcp2 ossl backend. Rotation is part of this rather than deferred: the current key encrypts, the previous one still decrypts for one more lifetime (the callback returns 2, so OpenSSL reissues the ticket under the current key) and any older name is refused, costing that client one full handshake. The lifetime is one hour. The key state is behind a mutex, since the callback runs on every loop thread. An explicit session-id context goes on the QUIC contexts too, as on the TCP path, which is what lets an mTLS deployment resume at all. SSL_OP_NO_TICKET is never set and the ngtcp2 ossl backend does not change ticket issuance: it only registers the QUIC record-layer callbacks on the SSL, so the TLS stack builds and encrypts tickets as usual and they travel in CRYPTO frames. 0-RTT early data is not offered anywhere in the tree (nothing enables early data or sets an early-data context), so resumption is all that changes and the CHANGELOG says so (#382). No QUIC client in the tree can be made to resume on demand, so the regression test drives the ticket callback the way OpenSSL does through tests/vq_h3_tls_ctx.cpp: a name stamped by the encrypt side decrypts, an unknown name falls back to a full handshake rather than erroring, building a second engine does not mint another key, and after the lifetime elapses the encrypt side stamps a new name while the retired one still decrypts with a reissue and a two-lifetime-old one is refused. The rotation assertions fail with rotateTicketKeyIfDue stubbed out. * fix(loop,tls): route the last h3 operator lines through opLog and name an unreadable key file (#377, #388) Integration pass over the round-4 branch. The HTTP/3 engine-setup and certificate-reload failure lines in eventloop.nim, and server.nim's no-TLS reload rejection, were the three operator messages still written with a bare stderr.writeLine after #388 introduced opLog as the one sink; they now go through it. loadCertKey raised nothing and returned a bare false when the keyFile or pkcs12File could not be read, so that case still surfaced as "unknown TLS error" after #377 fixed the decoder-rejection case: a readMaterialFile helper carries the path and the OS reason out, and buildTlsCtx frees the partial ctx before re-raising it. Two doc comments still described the h3 reload as an in-place mutation; #352 made it a rebuild-and-swap. * fix(loop,stress): keep the accept() backoff out of the accept-drop total (#388) `AcceptDrops.total` summed `acceptSuspend` in with `cap`, `tls` and `register` and was documented as "connections this process accepted and then dropped". The first three are that; `acceptSuspend` is not. It counts the times accept() itself failed with EMFILE/ENFILE/ENOBUFS/ ENOMEM and the listener was deregistered for ~1s: nothing was accepted, nothing was dropped, and whatever is in the kernel's backlog waits for the listener to come back or times out there. Summing it made `total` read high against an operator's own connect-error rate, and one backoff could stand for any number of waiting connections. `total` is now `cap + tls + register`. `acceptSuspend` keeps its counter and its rate-limited operator line, and is reported beside the total as the backoff-event count it is ("raise the fd rlimit"). The field docs, the `Server.acceptDrops` accessor doc, HARDENING's "Seeing the limits fire" table, the README snippet and the stress server's `/drops` line and shutdown print all say so; `/drops` moved `acceptSuspend` after `total` so the grouping is visible in the soak log. test_security_dos pins the identity (`total == cap + tls + register`), and its no-argument-form check is now `>=` rather than `==`: both sides are reads of a process-wide counter that any loop thread of any server in the process can bump between them. * fix(loop): emit each operator-log line in one stderr write (#388) `opLog` used `stderr.writeLine`, which is two `fwrite` calls: the payload, then the newline. Every loop thread writes through this sink, so a line from another thread could land between the two and split a message across the log, which is the one thing a sink that exists to make drops visible must not do. Build the line (`msg & "\n"`) and write it once; the try/except shape is unchanged. Also reword the docstring, which read as if an operator could redirect the sink. It is internal and not exported from `vortex.nim`, so say that instead of promising a hook that does not exist. * fix(stress): join the loop threads on the stress server's SIGTERM path (#388) `srv.requestShutdown()` is non-blocking and it was the last statement of `isMainModule`, so main fell off the end and the process ran its exit sequence while the loop threads were still serving. That is a teardown race in the one binary whose whole purpose is to surface crashes, and it would be charged to vortex rather than to the harness. Use `srv.close()` (requestShutdown + waitFor), which blocks until the loops have drained and been joined, bounded by `shutdownHardTimeout` as everywhere else. The drop-counter print stays BEFORE the join so it is never lost if the join takes the grace period, and a "stopped" line after it marks an orderly exit. The module comment now says this path is for hand runs: run.sh tears a cell's server down with `docker rm -f`, a SIGKILL, so no handler runs there (which is why run.sh dumps the container's last 200 log lines on a failed cell). Verified by hand on the sync and chronos builds: SIGTERM prints the tally, then "stress_server: stopped", and the process is gone within a second. * docs(h3,stress): document QUIC ingress bounds and the stress /drops route (#380, #381, #388) HARDENING had nothing at all about HTTP/3 ingress, although it is the one listener with no accept queue and no backlog: every datagram is decrypted and parsed synchronously on a loop thread that also owns TCP connections, so it has to be bounded in work rather than memory. A new "QUIC ingress (HTTP/3)" subsection under the resource limits states what #380 and #381 actually guarantee -- a receive buffer sized to the advertised 65527-byte `max_udp_payload_size` from one shim constant, a 256-datagram budget per drain (`ngRecvBudget`), up to four drains per loop pass, and no selector wait while a budget was exhausted so the TCP fds on the thread keep being served under a flood -- and, just as important, the residual risk the #381 entry records: no Retry token yet, so a spoofed-source Initial flood can still make the shim commit per-connection state up to `maxConnections`, and Version Negotiation is one reply per unsupported-version Initial with no per-source limit (amplification below 1, so work rather than reflection). The HTTP/3 window rows link down to it. The stress README gained a section on `/drops`: its exact format, what each field means (including that `acceptSuspend` is not in `total`), that nothing polls it automatically, and that run.sh dumps the server container's last 200 log lines on a failed cell -- which is where the `vortex:` lines live, since `docker rm -f` is a SIGKILL and the server's own SIGTERM print never runs under the harness. * test(h3): size the UDP flood to the host and widen the latency bound (#381) The suite spun an unconditional 4 sender threads and gave curl `-m 2`, the same 2 s it then asserted the request completed inside. On a 2-vCPU CI runner that oversubscribes the host and measures its scheduler, and a curl timeout equal to the bound makes "served slowly" indistinguish- able from "never served": both come back as a nonzero exit at exactly the bound. Senders are now `max(1, countProcessors() - 1)`, so a core is always left for the loop thread and one sender is still enough to starve an unbounded drain (these datagrams cost the server more than the sender). curl gets `-m 10`, longer than the bound on purpose, so a slow answer shows up as a latency the test prints. The bound itself is 5 s: the pre-fix failure is total, not slow, so it does not need to be tight. Verified: five runs on a 14-core host (13 senders) took 418, 457, 663, 972, 1059, 1675 ms. With `ngRecvBudget` raised to 1e9 to undo the fix, the request was never served and curl returned at its full 10 s timeout, so the 5 s bound keeps the pre-fix signal. * fix(tls): format OpenSSL error reasons into a private buffer (#388, #377) lastErrorMsg formatted with ERR_error_string(e, nil), which writes into OpenSSL's single process-wide `static char buf[256]` and is documented as not thread-safe. #388 exported it as tlsLastErrorMsg() and the accept path calls it from every loop thread, so two threads reporting a failure at once could each be handed the other's message: a two-thread probe read 128 to 2,760 wrong messages per 400,000 samples. It now formats with ERR_error_string_n into a buffer of its own, trimmed at the NUL. newTlsSession also never cleared the thread's error queue, and tlsHandshake/tlsRead/tlsWrite leave their reason on it when a connection fails, so the accept path's "TLS session setup failed" line could name the failure of an earlier connection accepted on the same loop thread. It clears the queue on entry, and loadPkcs12 and buildTlsCtx do too, as the PEM loaders already did, so a rejected bundle, cipher string or protocol version is not reported with a leftover from unrelated OpenSSL work. tests/test_tls_error_queue.nim pins both: it pushes errors with known codes onto the queue (ERR_new + ERR_set_error) and reads them back from two threads, and it checks that a queued reason does not survive newTlsSession. Both cases fail with the fix reverted (9,180 wrong reads of 400,000, and the stale error:0A000064 reported after the new session). * fix(tls): reject a certificate-only reload of a PKCS#12 bundle (#377, #378) A PKCS#12 bundle carries the certificate and the key together, so a reloadTls(certFile = ...) against p12-sourced material cleared the bundle fields and left keyFile and keyPem empty. loadCertKey then called readMaterialFile("", "private key"), so the reload failed, as it must, but its reason was "cannot read private key : cannot open:" with both the path and the OS reason blank, naming neither the key nor the bundle. The symmetric keyFile-only case has had an explicit rejection since #378; the certFile-only case now gets the same one, before anything is built. readMaterialFile also treats an empty path as the configuration hole it is and raises "no private key configured" rather than asking the OS to open "". tests/test_tls_advanced.nim asserts the new reason through lastTlsReloadError and that the old blank one is gone, plus a case that reaches the empty-path message through the config. Both fail with the fix reverted, reporting exactly "cannot read private key : cannot open:". * fix(tls): switch the SNI context outside the config lock (#356) servernameCb held cfg.ctxLock across SSL_set_SSL_CTX, which in OpenSSL 3 is not a pointer store: it ssl_cert_dups the entire CERT of the context it is handed (an allocation plus an up-ref per chain entry and per key slot) and frees the connection's old one. acquireCtx takes the same lock on every accept, so every accepted connection on every loop thread queued behind one SNI handshake's certificate duplication. The lock now covers only what needs it: the host scan and an SSL_CTX_up_ref of the chosen context, which is what keeps a concurrent reload's release from destroying it (#356 swaps the whole per-host table under that lock). The switch runs outside the lock and takes its own reference, so the callback drops ours straight afterwards. Verified with tests/test_tls_reload_race.nim (3 SNI handshake threads against 40 reloads) and tests/test_tls_advanced.nim, and the race suite once more under NIM_SANITIZE=1, which reports no double free or use-after-free. * fix(tls): read the last reload reason without the reload lock (#378) lastTlsReloadError took cfg.reloadLock, which reloadTlsConfig holds across its file reads, key parsing, one buildTlsCtx per SNI host and a stderr write. A health handler on a loop thread reading the reason therefore stalled that whole loop for the length of a rotation. The field is now published by reloadFailed (and cleared with the ctx swap) under ctxLock, the short lock acquireCtx takes for an up-ref, and read under the same one; the writer still holds reloadLock, so the lock order reloadLock -> ctxLock is the one the swap already uses. Documented as cheap and safe from a handler in the accessor, HARDENING.md and README.md. Also fixes two stale comments noted in review: the TlsConfig field doc (which now also records that lastReloadError is a GC string in createShared memory, the same pre-existing pattern as material/ocsp/sni*), and the claim that the SNI-failure path's SSL_CTX_free releases an OCSP blob, which is not attached to that context yet. tests/test_tls_reload_race.nim gets a deterministic case: the reload is pointed at a FIFO as its ocspFile, so it sits in readFile holding reloadLock while a writer thread unblocks it after 2 s. The accessor must answer anyway; with the fix reverted it waits 2,004 ms, with it under 1 ms. * fix(tls): refuse a certificate whose validity time will not parse (#379) X509_cmp_current_time answers 0 only on failure: it reports an exact equality as -1, so 0 means an ASN.1 time it could not read. checkCertValidity compared `< 0` and `> 0`, so that 0 fell through as "inside the window" and the certificate was installed on the strength of a comparison that never happened. This is reachable, not theoretical: OpenSSL loads a certificate whose notAfter is a well-formed UTCTime that is not a real time, and its own x509 command prints "notAfter=Bad time value" for it. Such material now fails with "certificate validity time could not be parsed", at startup and on reload like any other bad material. The same change is going into the h3 shim. tests/test_tls_cert_validity.nim crafts one by rewriting the notAfter straight into the DER (same length, so the key still matches) and checks both paths. With the fix reverted the server starts on it and reloadTls returns true. The suite's dated-fixture gate now only guards the two suites that need openssl -not_before/-not_after, so the new case runs everywhere. * docs(hardening): correct what happens to the TCP ticket key (#382) HARDENING.md claimed "The TCP listener still uses OpenSSL's own per-context ticket key, generated once at startup and not rotated." The second half is wrong: reloadTlsConfig builds a replacement SSL_CTX on every reload, so the TCP ticket key and the TLS 1.2 session cache are regenerated there and every ticket or cached session issued before the reload stops resuming. Probed with openssl s_client -sess_out/-sess_in against a vortex server: the saved session comes back "Reused" before reloadTls() and "New" after it. Rewritten to say what is true, and to name the consequence an operator cares about: since #382 made the h3 key process-wide and rotating, HTTP/3 resumption survives a certificate rotation while TCP's does not. The #382 CHANGELOG entry gets the same clarification. Sharing the rotating key with the TCP listener is left as a follow-up. * fix(tls,h3): route the reloadTls sni override to the HTTP/3 engine (#356) The per-host override added by #356 reached the TCP listener only. server.reloadTls signalled the loops with requestCertReload(certFile, keyFile) alone, and the shim rebuilt its per-host contexts from the material vq_engine_new was handed, which is filled once at startup and never updated. So on an http3-enabled server reloadTls(sni = ...) returned true, TCP served the new host, and every h3 client that followed Alt-Svc was handed the DEFAULT certificate for that host and failed on a name mismatch -- the #374 bug, reintroduced for any host configured after startup -- while a host removed from the set kept being served its retired certificate over h3 for the life of the process. CertReload now carries the replacement set to the loop threads beside the paths: a serialised, GC-free blob published under the existing lock with the generation, freed and replaced by the next request, decoded by the loop under the lock exactly as the paths are. applyQuicReload hands it to ngReloadCert, and vq_engine_reload_cert takes a VqSniCert[] that, when non-empty, REPLACES the engine's per-host material and contexts in the same all-or-nothing transaction as the default certificate. An empty set keeps the old meaning, rebuild the configured material and re-read its files, so there is still no spelling for "drop every host" on either transport. server.reloadTls passes the set through only when the TCP reload succeeded, as it already did for the paths. Also fixes the shim's hostEq comment, which still called the TCP path's byte-exact SNI match issue #358 (fixed on this branch). * fix(h3): name unreadable material files and refuse a one-sided PKCS#12 reload Three diagnostics on the QUIC material path, mirrors of what the TCP side already does (#353, #377, #379). A missing or unreadable certificate, key or PKCS#12 FILE reached the operator as "cannot load TLS certificate/key: error:80000002:system library::No such file or directory", naming neither the path nor which half of the pair failed, because OpenSSL leaves nothing better in its error queue. makeCtx now opens each file up front and reports "cannot read private key <path>: <reason>", the shape readMaterialFile gives the TCP path. A certFile-only reload against a PKCS#12-configured engine cleared the bundle and left key_file empty, so it came back as a bare "certificate/key mismatch" instead of the actionable rejection the key-only case already got. It is now refused up front with the TCP path's wording. certValidityError treated X509_cmp_current_time == 0, which is a parse failure and not a verdict, as "in window", so a notBefore/notAfter OpenSSL cannot read installed as if it were valid. It is a rejection now ("certificate validity time could not be parsed"); the TCP side takes the same change. Also: sslErrStr's comment claimed it reported the queue's newest entry while ERR_get_error pops the oldest (behaviour unchanged), the #354 chain-clearing comments now say that SSL_CTX_clear_chain_certs clears the selected key slot's chain only (fine, because every live reload builds a fresh context), and the #353 notes stop calling a configured pkcs12File a no-op for a bare reload: it is a path, and it is re-read. * fix(h3): rotate the session-ticket key on the clock, and pin its install (#382) Rotation was driven only from the encrypt branch of ticketKeyCb, so the hourly lifetime held only under steady ticket traffic: on a server that went quiet, a ticket minted at t=0 was still accepted at t=10h because no ticket had been issued in between, and HARDENING's "a disclosed key exposes at most two hours of resumed sessions" was not true of an idle deployment. rotateTicketKeyIfDue now also runs from vq_engine_handle_expiry, which every loop enters at least once a second, at the cost of one steady_clock read under the mutex per tick. The three existing #382 cases call ticketKeyCb themselves, so they pass with the SSL_CTX_set_tlsext_ticket_key_evp_cb line deleted from makeCtx -- while every context would then fall back to its own OpenSSL-generated key and the cross-loop resumption the issue is about would be gone. A new case runs real TLS 1.3 handshakes over a BIO pair against the engines' own contexts: one against engine A to mint a ticket, one against engine B to resume it, asserting the shim's callback ran on both the encrypt and the decrypt side (two small counters on the shim make that observable) and that SSL_session_reused came back set. It fails with the install line removed. * fix(h3): share the QUIC receive counters and free the per-thread buffers gTruncDrops was a {.threadvar.}, so test_h3_recv_payload's `check ngTruncatedDrops() == 0` read the main thread's own copy, which is always zero whatever the loops did: a vacuous assertion. gRecvBufSize was the opposite problem, a plain shared global written by every loop thread, which is a data race even though all of them store the same number. Both are Atomics now, relaxed, since neither orders anything. gRecvBuf (a 64 KiB seq) and gReady were never released when a loop thread ended, so a process that starts and stops servers leaked 64 KiB per loop thread per cycle (25 cycles over 4 loops measured at 6 MiB). ngEngineFree clears them with the engine. Docs: gRecvBuf is 65527 bytes, which IS the largest possible UDP payload, so the MSG_TRUNC branch and gTruncDrops can never fire. The guard stays (a future smaller buffer would need it) but the #380 CHANGELOG entry and the ngTruncatedDrops doc comment stop describing it as a live path, and the test says it is asserting the guard stayed dormant (#380). ngRecvBudget's docstring and the #381 entry said "256 per pass of the event loop", but h3Drive can run up to four times per pass (the outbox, the main drive, the tick's idle sweep, the end-of-pass flush re-drive), so the budget is per drain and the per-pass ceiling is 1024. Reworded; the budget itself is unchanged (#381). * docs(changelog): state the measured flood latency honestly (#381) The #381 entry quoted the four-sender reproduction (tens of milliseconds); the regression test now sizes its senders to the host and measures about a second with every core but one blasting, so the entry gives both figures and the bound the test asserts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the TLS-audit issues #362 through #373, one commit per issue, plus the three clusters those issues cross-reference: the retire-ring lock (#360, the more reachable half of #364), the QUIC configuration inheritance gap (#351, #359, #374), and the renegotiation option (#376, the precondition for #371 and #372). Seventeen fixes, seventeen commits, each with its regression test where one is feasible and its changelog entry.
Fixes #351
Fixes #359
Fixes #360
Fixes #362
Fixes #363
Fixes #364
Fixes #365
Fixes #366
Fixes #367
Fixes #368
Fixes #369
Fixes #370
Fixes #371
Fixes #372
Fixes #373
Fixes #374
Fixes #376
TLS material loading (src/vortex/transport/tls.nim)
loadCertChainMemmirrors OpenSSL's ownSSL_CTX_use_certificate_chain_file: after the chain loop it peeks the error queue and accepts end-of-data only forPEM_R_NO_START_LINE; any other reason (bad base64, truncated block) rejects the material, at startup and on reload, with the OpenSSL reason in the error.ERR_peek_last_errorpluserrGetLib/errGetReason(mirroring theerr.hinlines,ERR_SYSTEM_ERRORincluded) join the FFI. The same discrimination is applied toloadCertChainin the QUIC shim.loadCaMemapplies the same discrimination and additionally requires the whole BIO to be consumed cleanly, so a truncated or mangled client-CA bundle is rejected instead of installing a partial trust store. A client issued by the last CA in a clean bundle verifies.verifyClient != Nonewith neitherclientCaFilenorclientCaPemis rejected in bothvalidateConfigandapplyClientVerify, so the empty-trust-store configuration cannot be reached through the public config or a directTlsConfig.alpnSelectreturnsSSL_TLSEXT_ERR_ALERT_FATALon no overlap, so a client offering only an unsupported protocol gets RFC 7301'sno_application_protocolalert instead of a silent HTTP/1.1 connection. A client that sends no ALPN at all still gets HTTP/1.1 (OpenSSL never invokes the callback in that case).reloadTlson a PKCS#12-configured server is rejected outright (and thekeyFilebranch now clears the bundle fields, symmetric withcertFile), instead of silently rebuilding the old bundle and returning true. Verifiedcfg.materialis only persisted after a successful build.SSL_OP_NO_RENEGOTIATIONis set on every context (newSSL_CTX_set_optionsFFI). Note from verification: OpenSSL 3 already refuses client-initiated renegotiation by default, so this makes the guarantee explicit rather than closing a live DoS; a systemopenssl.cnfor a pre-3.0 libssl could otherwise flip it. The consequence for fix(tls): flushOut returns on SSL_write WANT_READ without dropping write interest #371/fix(tls): SSL_read WANT_WRITE arms write interest but the write event never retries the read #372 is below.Context lifetime (#360, #364)
reloadTlsConfigruns under a per-configreloadLock, so two threads reloading at once (a SIGHUP thread plus an admin endpoint) no longer race the retire bookkeeping and thecfg.material/cfg.ocsp*strings. Loop threads never take this lock. Pre-fix the newtest_tls_reload_racesuite aborts 3/3 on a double free at the forced-eviction site; TSan reported 34 races there and in the material assignment, all gone.newTlsSession(andctxCertSubject) go throughacquireCtx, which loadscfg.ctxandSSL_CTX_up_refs it under a very shortctxLock(~1.7 us per session either way, inside the noise), andreloadTlsConfigpublishes the new context under the same lock and releases the old one immediately. A displaced context now lives exactly as long as the last session built on it. Lock order is alwaysreloadLockthenctxLock; readers only takectxLock.TLS I/O in the event loop (src/vortex/eventloop.nim)
driveHandshake's WANT_READ arm callsdisarmWrite, so a handshake that passed through a WANT_WRITE stops spinning once its flight drains. The newtest_tls_iosuite hand-builds a ClientHello against a fat chain, drains the server's flight and goes silent: 0.98 s of CPU per second before, 0.0002 s after; the connection is still reaped byheaderTimeout.flushOut's WANT_READ arm disarms write, re-arms the write-stall deadline by hand (the flush is stalled, sowriteTimeoutmust still reap it), and runs the samewsBackpressure/drainResumenotifications as the WANT_WRITE arm. No client-driven regression test: against OpenSSL 3 the arm is unreachable (renegotiation is refused by default, a patched-in renegotiation dies onSSL_R_BAD_LENGTHbehind a partial record, and a TLS 1.3 KeyUpdate surfaces as WANT_WRITE fromSSL_read, which is fix(tls): SSL_read WANT_WRITE arms write interest but the write event never retries the read #372). The suite gains a liveness test for the reachable neighbour instead: a peer that stops reading a 4 MiB response parks the flush without spinning and resumes the moment it reads.SSL_readWANT_WRITE recordssslReadWantsWrite; the dispatcher's write branch routes the connection intohandleRead(the same call, as OpenSSL requires) beforeflushOut, andflushOutkeeps write interest armed while the flag is set. Verified by injecting the condition into a throwaway build: without the retry the TLS suite's first request is never read. No client can arrange the exact-full-send-buffer state deterministically, so there is no black-box test.handleRead's tail callstlsPendingafter the recv loop's early exits. Drivable now, the connection goes on a per-loopsslReadyqueue (fd + generation, like the deferred flushes); still paused or pinned, the debt is kept on the connection and re-queued fromresumeBodyImpl,releasePinandresumeAfterRespond. The loop drains the queue once per pass and does not wait on the selector while it is non-empty. Test: ablocking:handler holds the connection while a pipelined request arrives as one full-size TLS record; without the fix the second request is never served.shutdown(SHUT_WR), drain to the peer's FIN or the drain deadline, then close.tlsCloseNotifiedstopscloseConnfrom running a secondSSL_shutdownover the half-closed socket; the session is still freed exactly once. Test: a 431 flooded over TLS is read back cleanly on all five iterations, where before the client saw a reset mid-flood.HTTP/3 (src/vortex/http3/ngtcp2)
failConndeletes thenghttp3_connand nullsc->h3before anything else, andcbRecvStreamDatareturnsNGTCP2_ERR_CALLBACK_FAILUREso ngtcp2 abandons the rest of the datagram. Every other nghttp3 call site is guarded byif (c->h3)(the one that was not,cbStreamOpen, now is) andcbHandshakeCompletedno longer resurrects an h3 for a closing connection. The negativenghttp3_conn_writev_streampath inwriteConngets the same treatment.test_http3_malformedreplays the issue's two-frame datagram into the real callback with a realnghttp3_conn: pre-fix it SIGSEGVs on the second frame.VqConfigcarriestlsCipherSuitesandmaxTlsVersion;makeCtxappliesSSL_CTX_set_ciphersuitesand fails the engine on an invalid list.maxTlsVersion = V12withhttp3 = trueis rejected at config time (QUIC cannot negotiate below 1.3), withmakeCtxrefusing the same combination as a backstop. This is a behaviour change for a config that capped TLS at 1.2 and lefthttp3at its default.tlsCipherList(TLS 1.2 only) and the minimum version (clamped to 1.3 by QUIC) are documented as not carried.verifyClientand the client CA are wired throughVqConfiginto a C++applyClientVerifywith the same mode mapping and precedence as the TCP path, and the in-memory CA loader uses the same error-queue discipline as fix(tls): loadCaMem accepts a corrupt CA bundle and trusts only the certificates before the damage #368. The per-connectionSSL*is exposed viavq_conn_ssl()and stored onH3Conn.ssl, soreq.clientCertSubjectworks over h3.~Connnow callsSSL_set_app_data(ssl, nullptr)beforeSSL_free, which ngtcp2's OpenSSL backend requires when thengtcp2_conndoes not outlive the SSL; without it a handshake rejected for a missing client certificate aborted the loop thread incrypto_ossl_ctx_release_crypto_data(a latent use-after-free for any connection freed with unconsumed crypto data, not only mTLS).settings.sniis passed throughVqConfig; the engine builds one QUIC-configuredSSL_CTXper host through the samemakeCtx(so hosts inherit verify, ciphers and version pinning) and a servername callback switches withSSL_set_SSL_CTX, exact match first, then a one-label wildcard, case-insensitively (DNS names are; the TCP side's byte-exact match is fix(tls): SNI hostname matching is case-sensitive #358 and is left alone).vq_engine_reload_certrebuilds the host contexts from their configured material, all or nothing.Decisions to double-check
maxTlsVersion = V12with h3 enabled rather than silently disabling h3. One existing test neededhttp3 = false.reloadTls.conformance/,demos/orbench/offers an ALPN outsideh2/http/1.1.ctxRefusesRenegotiationis new public surface ontransport/tls, added for the fix(tls): client-initiated renegotiation is enabled and unmetered on the loop thread #376 white-box test in the style ofctxCertSubject.Follow-ups found during the work (not changed here)
reloadTls's return value reflects the TCP reload only; a failed per-loop h3 apply is logged to stderr and invisible to the caller, andapplyQuicReloadruns on a one-second tick, so the h3 swap lands up to ~1 s after the TCP one.loadPkcs12andmakeCtxin the shim swallow errors with a bareERR_clear_error(), so an h3 context build failure loses its cause. Same shape as fix(tls): loadCertChainMem treats any chain parse error as end-of-data, loading a truncated chain #367/fix(tls): loadCaMem accepts a corrupt CA bundle and trusts only the certificates before the damage #368.loadPkcs12intls.nimreturns false on a partialSSL_CTRL_CHAIN_CERTfailure after installing the cert and key; harmless becausebuildTlsCtxfrees the ctx, but the same partially-applied shape as fix(tls): key-only reloadTls on a PKCS#12 server is silently ignored and returns true #363.libssl.so(.3|)dynlib pattern binds a pre-3.0 libssl on Linux, where the client-renegotiation default andERR_SYSTEM_FLAGlayout differ from what this module assumes.TlsConfigstores GC'd strings (material,ocsp*) increateSharedmemory; race-free under the new lock, but the neighbouringCertReloadstruct avoids GC strings deliberately.nimble testraceis-d:plainHttp, so the TSan runs for fix(tls): retire ring read-modify-written without synchronization #360 were manual; a TLS-enabled TSan entry would keep them.{.compile.}cache can hand a test a stalevq_ngtcp2.cppobject; verify shim changes with-f.Validation
bash tests/run.shon the integrated branch (orc): 88 suites, 656 checks, 0 failures, 4 environment skips (the compression builds). Every agent also ran the full suite on its own commits before integration.-d:plainHttpbuild ofsrc/vortex.nimand all 15conformance/tls/harness.nimconfigs pass.Stress smoke
One complete pass of
VORTEX_PROTO=all VORTEX_SERVER=all VORTEX_SECONDS=10 VORTEX_REPORT_SECONDS=2 nimble stresson the integrated branch (5 workloads x h1/h2/h3 x sync/async/async-await/chronos/chronos-await, 75 cells, canary plus chaos sidecar, 64 MiB streams): 75/75 cells passed, all 75 chaos sidecars passed, every checksum verified, and no panic, assertion, signal or unhandled-exception signature in any server log. Load average during the run was 4 to 7.Mean throughput over the five server runtimes, for a same-host comparison against the #350 numbers:
The h2 and h3 cells run over TLS, so every TLS I/O change (#365, #366, #371, #372, #373) and the QUIC context changes (#351, #359, #362, #374) were under load with the chaos sidecar's slow readers, stalled uploads, idle holds, mid-transfer aborts and vanishing connections.
Harness note, not changed here: the first chronos cell took ~19 minutes because nimble 0.22 inside the Docker image downloaded and built a Nim 2.2.12 toolchain to install chronos (it does not accept the image's system Nim 2.2.10, with or without
--solver:legacy, and--useSystemNimreports no system nim). Later cells reused the cached layer and ran at the usual ~1 minute each.