Area
RESP2 (Redis-compatible)
Problem or motivation
tlsHandshakeTimeout (10s, internal/resp/server.go:34) is meant to drop connections that never complete the TLS handshake. But st.handshakeDeadline is only checked inside OnTraffic (server.go:199), which gnet invokes only when new bytes arrive on the connection.
A client that sends STARTTLS, receives +OK, then sends nothing (or half a ClientHello and stalls) holds an open fd plus the allocated TLS state (tlslib.Conn, GnetConnAdapter, 4 KB readBuf) indefinitely — the deadline expires but nothing closes the socket because there is no further traffic. This is a slow-loris-style resource-exhaustion vector: an unauthenticated plaintext client can open many connections, STARTTLS each, and stall. The same flaw exists in the pre-existing implicit-TLS path (deadline set in OnOpen at server.go:169), but STARTTLS broadens reach because any plaintext client can trigger TLS-state allocation.
Proposed solution
Actively enforce the handshake deadline regardless of traffic:
- Maintain a registry of pre-handshake connections (added in
OnOpen, removed in OnClose, and on handshake completion), including both the STARTTLS-upgrade and implicit-TLS-accept paths.
- Drive cleanup from gnet's
OnTick() ticker (gnet.WithTicker): each tick closes connections past st.handshakeDeadline. Conn.Close() is safe from another goroutine — it schedules onto the owning event loop via Trigger(queue.LowPriority, ...) — which matters because the ticker runs on a single goroutine while connections span multicore event loops.
- Do not use socket deadlines: gnet v2.10 returns
ErrUnsupportedOp for SetDeadline/SetReadDeadline on Linux (connection_unix.go:554), which is why GnetConnAdapter no-ops them.
- Post-handshake connections must be unaffected (the timeout applies to the handshake window only); the zero-allocation hot path must not regress (registry/ticker touch only pre-handshake connections).
Acceptance criteria:
Alternatives considered
- Per-connection
time.AfterFunc timer — rejected; closing must be scheduled onto the owning event loop, and a ticker sweep keeps all mutations inside gnet callback context.
- Socket read deadline via
GnetConnAdapter — rejected; gnet unix deadlines are unsupported.
- Accept and document as a known limitation — acceptable fallback if enforcement proves too costly for the hot path.
Additional context
Follow-up to #15 . Finding from PR review; gnet v2.10 ticker runs on the main event loop only (server_unix.go:178-180).
Area
RESP2 (Redis-compatible)
Problem or motivation
tlsHandshakeTimeout(10s,internal/resp/server.go:34) is meant to drop connections that never complete the TLS handshake. Butst.handshakeDeadlineis only checked insideOnTraffic(server.go:199), which gnet invokes only when new bytes arrive on the connection.A client that sends
STARTTLS, receives+OK, then sends nothing (or half a ClientHello and stalls) holds an open fd plus the allocated TLS state (tlslib.Conn,GnetConnAdapter, 4 KBreadBuf) indefinitely — the deadline expires but nothing closes the socket because there is no further traffic. This is a slow-loris-style resource-exhaustion vector: an unauthenticated plaintext client can open many connections,STARTTLSeach, and stall. The same flaw exists in the pre-existing implicit-TLS path (deadline set inOnOpenatserver.go:169), but STARTTLS broadens reach because any plaintext client can trigger TLS-state allocation.Proposed solution
Actively enforce the handshake deadline regardless of traffic:
OnOpen, removed inOnClose, and on handshake completion), including both the STARTTLS-upgrade and implicit-TLS-accept paths.OnTick()ticker (gnet.WithTicker): each tick closes connections pastst.handshakeDeadline.Conn.Close()is safe from another goroutine — it schedules onto the owning event loop viaTrigger(queue.LowPriority, ...)— which matters because the ticker runs on a single goroutine while connections span multicore event loops.ErrUnsupportedOpforSetDeadline/SetReadDeadlineon Linux (connection_unix.go:554), which is whyGnetConnAdapterno-ops them.Acceptance criteria:
STARTTLS, receives+OK, and sends no further bytes is closed ~10s after the upgrade.internal/resp/covering the stalled-upgrade case (and stalled implicit accept).task check(vet + race tests) passes; hot path stays allocation-free.Alternatives considered
time.AfterFunctimer — rejected; closing must be scheduled onto the owning event loop, and a ticker sweep keeps all mutations inside gnet callback context.GnetConnAdapter— rejected; gnet unix deadlines are unsupported.Additional context
Follow-up to #15 . Finding from PR review; gnet v2.10 ticker runs on the main event loop only (
server_unix.go:178-180).