caddyhttp: mitigate slowloris via idle read/write deadlines - #7913
Open
dunglas wants to merge 10 commits into
Open
caddyhttp: mitigate slowloris via idle read/write deadlines#7913dunglas wants to merge 10 commits into
dunglas wants to merge 10 commits into
Conversation
ReadTimeout and WriteTimeout previously applied as a single hard deadline over the whole body/response through http.Server, so any non-zero value also killed large transfers from legitimately slow clients. Reset the deadline on every successful read/write instead (via http.ResponseController), and give both a sane 1m default now that doing so no longer penalizes slow-but-progressing clients.
Reworking ReadTimeout/WriteTimeout's own semantics was an unwanted behavior change for existing configs relying on the hard deadline. Leave them untouched and add ReadIdleTimeout/WriteIdleTimeout instead, reset on every successful read/write; both default to 1m since, being new, no existing config could have depended on a different value. Combining an idle timeout with its hard counterpart now gives the same base+ceiling shape as Apache's mod_reqtimeout, for free.
Deadlines are a single absolute value on the connection, not a min of several: ReadTimeout/WriteTimeout's own hard deadline, set once by net/http before the handler runs, was silently getting overwritten by the first idle-reset Read/Write, voiding it entirely. Clamp the idle-reset deadline to the hard one when both are set, so combining them actually behaves like the advertised base+ceiling.
Pure idle-reset alone doesn't bound a trickle that sends just enough to never go idle. ReadMinRate/WriteMinRate (bytes/second) grow the allowed deadline from a fixed start based on bytes transferred so far instead of resetting to a flat window on every call, so a transfer that doesn't sustain the configured rate falls behind real time and gets cut, matching Apache mod_reqtimeout's MinRate. Zero (default) keeps the existing flat idle-reset behavior unchanged.
Matches the named-return style already used by ResponseWriterWrapper.ReadFrom.
SetWriteDeadline bounds the whole call it precedes, not just a stall within it. net.Conn.Write loops internally until a buffer is fully sent (unlike Read, which returns after one syscall), and ResponseWriter.ReadFrom hands the entire remaining source to the connection in one call. A single large Write, or any body copied via io.Copy triggering the ReadFrom fast path (http.ServeContent, static file serving), had its whole transfer bounded by one deadline, silently truncating a slow-but-healthy transfer exactly like a hard WriteTimeout would - the same bug found and fixed the same way in FrankenPHP's go_ub_write (php/frankenphp#2574). Cap each underlying call at 64 KiB and reset the deadline between chunks instead. net/sendfile.go special-cases *io.LimitedReader, so chunking ReadFrom still uses the sendfile fast path per chunk.
Export IdleTimeoutReader/IdleTimeoutWriter/IdleDeadline so other packages (request_body next) can reuse the same idle-reset mechanism instead of reimplementing it, and turn the hardcoded 64 KiB write chunk size into a configurable MaxWriteChunk field defaulting to the same value - nginx's sendfile_max_chunk exists for the identical reason and is admin-tunable rather than fixed.
…eChunk ReadTimeout/WriteTimeout set a single deadline once, so any transfer running longer than the timeout got cut regardless of whether it was actually stalled - the same bug the server-wide timeouts had before switching to idle-reset. Reuse caddyhttp.IdleTimeoutReader/Writer here too, giving per-route granularity nginx/Apache have via location/ directory scoping and Caddy's server-wide timeouts don't: a route matching this handler can now set its own idle window independently from the rest of the server block.
Two directives per rate (read_body_idle + read_body_min_rate) for a value that's meaningless without the other. Fold min_rate into the idle-timeout directive as an optional second argument instead.
…andler request_body is a request-body concern (max_size, set); ReadTimeout/ WriteTimeout/MinRate/MaxWriteChunk pace both directions, and write pacing has nothing to do with the request body. Move all of it to a dedicated http.handlers.timeouts module instead, mirroring the server-wide timeouts option one level down.
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.
Summary
Adds
ReadIdleTimeout/WriteIdleTimeout(read_idle_timeout/write_idle_timeoutin JSON,read_body_idle/write_idlein the Caddyfiletimeoutsblock), reset on every successful read/write viahttp.ResponseController. A stalled connection is killed; a slow-but-progressing one isn't — matching nginx'sclient_body_timeout/send_timeoutsemantics.The existing
ReadTimeout/WriteTimeoutfields are untouched: same hard-deadline-over-the-whole-transfer semantics, same default (0/unbounded) as before. No behavior change for existing configs. Combine an idle timeout with its hard counterpart for a ceiling on top — the idle-reset deadline is capped so it can't silently push past an explicitly configuredReadTimeout/WriteTimeout.Also adds
ReadMinRate/WriteMinRate(bytes/second, 0 = disabled), matching Apachemod_reqtimeout'sMinRate. Idle-reset alone doesn't bound a client that trickles just enough data to never go idle; with a min rate set, the allowed deadline grows from a fixed start based on bytes transferred so far instead of resetting to a flat window on every call, so a transfer that doesn't sustain the configured rate falls behind real time and gets cut, even though no single read/write ever stalls. In the Caddyfile, min rate is a second, optional argument on the idle-timeout directive rather than a separate one:read_body_idle 60s 100/write_idle 60s 100.All new fields default to 1 minute (idle) / 0 (min rate, opt-in) — safe to default the idle timeouts since they're new fields, so no existing config could have depended on a different value.
Write chunking
SetWriteDeadlinebounds the whole call it precedes, not just a stall within it:net.Conn.Writeloops internally until a buffer is fully sent, andResponseWriter.ReadFromhands the entire remaining source to the connection in one call (sendfile or an internal buffered copy loop — the latter always for TLS, since*tls.Connisn't anio.ReaderFrom). Without chunking, a single largeWrite, or any response body copied viaio.Copytriggering theReadFromfast path (http.ServeContent, static file serving), had its whole transfer bounded by one deadline — silently truncating a slow-but-healthy transfer exactly like a hardWriteTimeoutwould. This is the same bug independently found and fixed the same way in FrankenPHP'sgo_ub_write(php/frankenphp#2574), and nginx hit it historically too (sendfile_max_chunk, added after a single fast connection could seize a worker process entirely).Fixed by capping each underlying
Write/ReadFromcall and resetting the deadline between chunks. The cap is configurable (MaxWriteChunk/max_write_chunkonServer,write_max_chunkin the Caddyfiletimeoutsblock), defaulting to 64 KiB — matching nginx's own tunablesendfile_max_chunk.net/sendfile.gospecial-cases*io.LimitedReader, so chunkedReadFromstill gets the sendfile fast path per chunk.Per-route granularity
New
timeoutshandler (http.handlers.timeouts, Caddyfile directivetimeouts) applies the same idle-resetReadTimeout/WriteTimeout/ReadMinRate/WriteMinRate/MaxWriteChunkper-route, independent of the rest of the server block — matching nginx'slocation{}and Apache's<Directory>scoping. Same Caddyfile shape as the server-wide option:read_timeout 60s 100/write_timeout 60s 100take the min rate as an optional second argument.Kept separate from
request_bodyrather than bolted onto it:request_bodyis about the request body specifically (max_size,set), while write-side pacing is a response concern that has nothing to do with the request body. A dedicated handler keeps that boundary clean and mirrors the server-widetimeoutsoption one level down.Covers HTTP/1.1, HTTP/2, and HTTP/3 (quic-go's
http3.responseWriterimplementsSetReadDeadline/SetWriteDeadlinedirectly, on the same stream used for the request body).Assistance Disclosure
This PR has been designed and reviewed by me, the code has been written by Claude Code.