Drive the inbound TLS handshake by socket events; kqueue: deliver unread data before EOF

[MrIron-no]

Summary

Two independent fixes, split into two commits.

1. Inbound TLS handshake busy-loop (dff3c84)

add_connection() registered inbound TLS sockets as WRITABLE only and drove the handshake from ET_WRITE. A writable socket is level-triggered and always ready, so every event-loop iteration re-ran SSL_accept() → WANT_READ until the peer's flight arrived: one RTT of 100% CPU per inbound TLS connection (user and server ports), and under -x thousands of ssl_handle_error: SSL_get_error=2 lines per second.

History: the 2019 TLS code set READABLE/WRITABLE from ssl_handle_error(). 7cb71c8 moved to the WRITABLE-only registration so start_auth() could be deferred past the handshake; 046b2ed removed the ssl_handle_error() calls because they used SOCK_ACTION_SET from the data path too (an SSL_write WANT_WRITE replaced the mask with WRITABLE only and starved reads). That left the spin as the only thing driving the handshake.

Now:

• add_connection() registers READABLE.
• ircd_tls_negotiate() gains an int *wants_write out-parameter (OpenSSL SSL_ERROR_WANT_WRITE, GnuTLS gnutls_record_get_direction() plus the "call again now" returns, libtls TLS_WANT_POLLOUT). Backends no longer touch socket events.
• The two handshake callers ADD/DEL WRITABLE from that hint only, so update_write() keeps owning WRITABLE for the data path.
• A silent peer now generates no events, and inbound TLS clients are not in LocalClientArray until start_auth() runs after the handshake, so check_pings() cannot see them either. TLS_HANDSHAKE_TIMEOUT is enforced from the connection's con_proc timer, armed in add_connection() and cancelled in tls_handshake_succeeded() (con_proc is unused until read_packet(), which cannot precede the handshake).

2. kqueue: unread data dropped on EOF (eba98ff)

kqueue sets EV_EOF on EVFILT_READ as soon as the peer's FIN arrives, even while evt->data bytes are still unread. engine_kqueue.c turned that straight into ET_EOF, so a peer's final ERROR/SQUIT line was dropped and the hub reported Server X closed the connection (<>) with no reason. Generate ET_READ while data remains; the drained socket comes back as EV_EOF with data == 0 and becomes the real ET_EOF.

Not buildable on Linux (no sys/event.h) — reviewed by hand, needs a BSD build.

Also adds tests/tls/test_tls_s2s_burst.py: links tls-hub and tls-leaf with the hub in both TLS roles and a populated burst.

Testing

• tests/tls/ suite: 75/75 (Linux/epoll, OpenSSL 3.5). test_stalled_handshake_times_out and test_stalled_handshake_after_clienthello_times_out fail without the con_proc timer.
• pr_websocket + cap: 92/92 (same add_connection path).
• New S2S burst test also passed with SSL_write capped to 16 bytes and with SO_SNDBUF forced to 1 KB (real WANT_WRITE under an 80-user burst), both link directions; those harness edits are not committed.
• GnuTLS / libtls backends compile-checked only (-fsyntax-only).

Review notes

Independently reviewed; timer lifecycle (GEN_MARKED, FREEFLAG_TIMER, exit from within the timer callback), ADD/DEL semantics, and the ET_WRITE-success-without-read_packet() path were traced and found sound. One pre-existing issue noted for a separate follow-up: client_sock_callback ET_READ runs read_packet() after tls_handshake_succeeded() may have exit_client()'d a failed outbound link (present since #99, not widened here).

[Ratler]

@MrIron-no can you take a look at these findings?

High — crashes and hangs

1. Use-after-free in client_sock_callback ET_READ arm (ircd/s_bsd.c:1298)
   After tls_handshake_succeeded(), the code falls through to read_packet(cptr, 1). But tls_handshake_succeeded() → start_auth() can synchronously exit_client()/free the client (peer RST making os_get_peername() fail, K-line, class full, IPcheck limit).
   read_packet then dereferences con_max_flood(NULL) → SIGSEGV. This path is newly reachable because inbound handshakes now complete on ET_READ; the ET_WRITE arm returns after success and this one doesn't. Fix: return (or check IsDead(cptr)) after
   tls_handshake_succeeded().
2. 100% CPU spin in kqueue engine for dead-socket clients (ircd/engine_kqueue.c:418)
   Suppressing ET_EOF when EV_EOF is set with data > 0 assumes the consumer will read. A client with FLAG_DEADSOCKET (sendQ overflow via dead_link()) whose peer then FINs with unread bytes hits a no-op ET_READ arm (if (!IsDead(cptr))), and the
   level-triggered filter re-fires every kevent() pass — busy loop until check_pings() reaps it, up to 120s later. Fix: emit ET_EOF (or drain) when the socket owner is dead.
3. Blocked write misreported as read-blocked in OpenSSL backend (ircd/tls_openssl.c:871)
   wants_write is derived only from SSL_ERROR_WANT_WRITE, but ssl_handle_error() also returns IO_BLOCKED for SSL_ERROR_SYSCALL with EINTR/EAGAIN. A blocked write then gets reported as read-blocked, tls_negotiation_events() drops WRITABLE, and the
   handshake deadlocks waiting for peer data that never comes — inbound dies at the ~6s timer, outbound idles for 90s. The old permanent-WRITABLE registration would have retried. Fix: treat IO_BLOCKED + SSL_ERROR_SYSCALL as wants_write = 1.

Medium — the fix doesn't cover Linux, plus DoS-adjacent issues

4. The "deliver unread data before EOF" fix is kqueue-only (ircd/engine_epoll.c:257)
   epoll's else if (events & EPOLLHUP) still swallows EPOLLIN when both are set, and poll/devpoll short-circuit POLLHUP into ET_EOF before their MSG_PEEK probe. So on Linux — the production engine and what the Docker test suite runs — the peer's final
   ERROR/SQUIT line is still lost, the exact symptom commit eba98ff claims to fix. Even on kqueue, the SO_ERROR pre-check (engine_kqueue.c:381-391) bypasses the new ordering on an RST.
5. Attacker-triggerable handshake spin: READABLE stays armed during WANT_WRITE (ircd/s_bsd.c:1287)
   tls_negotiation_events() only toggles WRITABLE; READABLE stays registered all handshake. A malicious peer that pipelines junk after ClientHello and stops reading (parking the backend in WANT_WRITE against the 2048-byte SO_SNDBUF) makes the
   level-triggered read event re-run a can't-progress handshake every loop pass for the full 5s timeout, per connection. Fix: while the backend reports wants_write, drop READABLE via SOCK_ACTION_SET — the mask is already being modified there.
6. GnuTLS backend abuses wants_write = 1 as a "retry" signal (ircd/tls_gnutls.c:619)
   The blanket default: non-fatal branch and the warning-alert case set wants_write = 1 purely to get called again via the always-ready writable event — an unbounded full-speed retry loop whose only backstop is the 5s deadline. The comment at 483-486 is
   also wrong: GNUTLS_E_GOT_APPLICATION_DATA is post-handshake-only (unreachable here) and its record is buffered, not consumed. Fix: loop on gnutls_handshake() until AGAIN/fatal inside ircd_tls_negotiate() instead of encoding "retry" as a fake socket
   direction.
7. Handshake deadline anchored at client allocation, never reset (ircd/tls_openssl.c:764, duplicated in gnutls/libtls)
   CurrentTime - cli_firsttime(cptr) > TLS_HANDSHAKE_TIMEOUT measures from make_client(), which for outbound links precedes the TCP connect. On a lossy path, SYN retransmits (1s/3s/7s) burn the whole 5s budget before ET_CONNECT, so the first negotiate
   call fails with "TLS handshake timed out" before a single TLS byte is sent — and every auto-reconnect fails identically, so the link can never come up. Pre-existing, but the PR rewrites and relies on this contract. Fix: stamp the deadline when the
   handshake actually starts.
8. Resource leak on throttled/failed TLS accepts (ircd/s_bsd.c:619)
   add_connection() allocates the SSL session and Client/Connection before the IPcheck throttle and socket_add failure checks, and both early returns just close the fd — the session isn't attached to the socket until after both, so nothing can ever free
   it. A sustained reconnect flood to a TLS port grows the process without bound. Pre-existing, but in a function this PR touches. Fix: free both in the early-return paths, or defer the allocations until after the checks.

Low — maintainability

9. Outbound path duplicates tls_negotiate_client() inline (ircd/s_bsd.c:381)
   completed_connection() carries a hand-mirrored copy (its own comments admit it), and this diff had to add the wants_write plumbing to both — which have already drifted (ircd_tls_close reason NULL vs "TLS negotiation failed"). Route the outbound path
   through tls_negotiate_client() and keep the opmask notice at the call site.
10. Timeout timer branch is a third negotiate-dispatch copy with unreachable arms (ircd/s_bsd.c:1350)
    Every backend fails on its deadline check before touching the session once the timer fires, so the res > 0 / res == 0 arms are dead in practice, and the res == 0 fallback fabricates a timeout message while re-arming socket interest on a connection
    it's about to kill. Simpler: the timer only fires past the deadline, so abort directly with a small tls_handshake_abort() helper — no backend round-trip.

[MrIron-no]

Thanks — addressed in c8edcd2, fe30688 and 63f5d94:

• Findings 1, 2, 10, 9: fixed as suggested. For finding 1 the only synchronous exit reachable from start_auth() is os_get_peername() failing (K-line / class / IPcheck are gated on NICK+USER or already ran), but that is real — a TLS 1.3 peer that closes right after Finished leaves our NewSessionTicket unread and RSTs — so the ET_READ arm now returns after tls_handshake_succeeded() like the ET_WRITE arm. Finding 2 is fixed at the owner: a dead client that is still readable is exited from the ET_READ arm instead of spinning until check_pings() (the same spin existed on every engine for "data pending, no FIN").
• Findings 7 + 5 + 10 (one commit): the three per-backend deadline checks are gone; the con_proc timer is armed where the handshake actually starts for both directions (tls_handshake_timer_arm()), so SYN retransmits no longer eat the budget. tls_negotiation_events() now SETs exactly one direction. Finding 5 is only safe with the outbound timer in place, hence the same commit. The timer aborts through tls_handshake_abort() without a backend round-trip.
• Finding 8: fixed — IPcheck runs before make_client(), the session is freed on the throttle path, and session + IPcheck count + Client are released on socket_add() failure (also covered the plaintext-port Client leak and the IPcheck count). While writing tests for that path I found that IPcheck itself has worse problems (exempt-address disconnects can abort the server, exemptions survive a rehash, 16-bit/unsigned arithmetic bugs); those are in IPcheck: unit + integration tests; fix exempt-address crash, rehash, and 16-bit/unsigned arithmetic bugs #102 together with unit and integration tests for IPcheck, kept separate from this PR.
• Finding 6: replaced the fake-write retry with a bounded gnutls_handshake() loop; you are right about GOT_APPLICATION_DATA being rehandshake-only.
• Finding 3: with SSL_set_fd (socket BIO) OpenSSL turns EAGAIN/EINTR into WANT_READ/WANT_WRITE via the BIO retry flag, so SSL_ERROR_SYSCALL+EAGAIN cannot reach ssl_handle_error(); adopted the safe default anyway (any non-WANT_READ block → write, since a wrong "write" costs one loop pass and a wrong "read" costs the deadline).
• Finding 4: not changed. On Linux tcp_poll() sets EPOLLHUP only on full shutdown / TCP_CLOSE; a FIN yields EPOLLIN|EPOLLRDHUP, so epoll already reads the peer's last line — the kqueue change is BSD-specific. Loss on RST (peer closes with our bytes unread) is engine-independent: every engine checks SO_ERROR/EPOLLERR before reading. That needs a recv-before-error design and I would rather do it as its own change; happy to file it.

tests/tls/ 75/75, websocket + CAP + throttle sanity 95/95. engine_kqueue.c still needs a BSD build.

[MrIron-no]

Two more commits pushed (a323e62, 02a4c5f).

Misbehaving-peer harness (tests/tls/test_tls_bogus_peer.py, bogus_peer.py, bogus_server_main.py): drives OpenSSL through ssl.MemoryBIO so each test decides which bytes hit the wire and when, parking ircd in one handshake state at a time. For every scenario it checks the 5 s deadline (or prompt failure) with nothing written before the close, no CPU spin (docker stats on the hub), and that a healthy client keeps getting PONGs. Inbound scenarios cover silent peers on client and server ports, stall after ClientHello, a ClientHello dribbled one byte at a time, garbage, RST/FIN mid-handshake, a peer with a 1 KB receive window that never reads (write-blocked server flight), a post-handshake flood without reading, plus two controls. Outbound scenarios make the hub CONNECT bogus.test.net <port> to a server we control (a python:3-alpine sidecar on the test network, new Connect block in ircd-tls-hub.conf): silent server, truncated server flight, garbage and immediate close both during and after the connect step, and a full handshake against a foreign TLS stack that receives PASS/SERVER. BOGUS_TLS_HOST/PORT/SERVER_PORT point the inbound scenarios at a real server.

Two things it found, fixed in a323e62:

• An outbound handshake that failed on a later socket event (peer closed or sent garbage after the hub had parked waiting for the server flight, i.e. the ET_READ path) was torn down with no operator notice at all — "TLS negotiation failed to …" only existed in completed_connection(). It now comes from tls_negotiation_failed() for both directions and all detection points.
• exit_client()'s "Link with %s canceled" notices test IsConnecting(victim) but sit under an IsClient(victim) guard, and IsClient() does not include STAT_CONNECTING, so they never fired for a connecting link (pre-existing). A link reset between connect() and registration — e.g. ECONNRESET during the TLS handshake — was invisible to the oper who issued the CONNECT. The guard now includes IsConnecting(); the inner branches already skip the ERROR line for connecting clients.

No spin and no deadline miss was found in any scenario on this branch (Linux/epoll). tests/tls/: 92 passed (75 existing + 17 new).

[MrIron-no]

Superseded by #105, which restructures the TLS layer and carries these fixes inside the new design. The remaining part of finding 4 (RST drops a peer's final pending line, engine-independent) is tracked in #104.