From 0a970a50e6e68825f8d1d0af2f5040898fcd755d Mon Sep 17 00:00:00 2001 From: Feng Ruohang Date: Wed, 16 Sep 2026 00:14:43 +0800 Subject: [PATCH] fix(http): enforce absolute read header timeout against slow HTTP DoS Slow HTTP DoS scanners (issue #183) hold connections open by dribbling request bytes, and two inherited gaps let them: 1. deadlineconn re-extended the read deadline on every partial read, converting net/http's absolute ReadHeaderTimeout into an inter-byte activity timeout that a trickle bypasses entirely. 2. the --read-header-timeout flag (default 30s) was never copied into the server context, so the server actually ran with header timeout disabled (upstream shares this bug). Keep deadlineconn on the write side only so response writes stay activity-based, and leave ReadTimeout at zero now that native read deadlines are honored, since an absolute cap would terminate large uploads. Request bodies keep an activity-based bound instead via a per-read deadline refresh in xhttp, and the flag is wired through common-main. With this, a slow-header connection is cut at ReadHeaderTimeout (verified live: 33s kill vs. unlimited before), while slow progressing uploads (32s for 2 MiB), stalled bodies (30s cleanup), slow readers (~30s write cutoff) and plain S3 traffic are unaffected. Fixes #183 Signed-off-by: Feng Ruohang --- cmd/common-main.go | 1 + cmd/server-main.go | 6 +- internal/http/listener.go | 7 +- internal/http/server-deadline_test.go | 157 ++++++++++++++++++++++++++ internal/http/server.go | 36 ++++++ 5 files changed, 205 insertions(+), 2 deletions(-) create mode 100644 internal/http/server-deadline_test.go diff --git a/cmd/common-main.go b/cmd/common-main.go index 393ab17d0..c127759fc 100644 --- a/cmd/common-main.go +++ b/cmd/common-main.go @@ -445,6 +445,7 @@ func buildServerCtxt(ctx *cli.Context, ctxt *serverCtxt) (err error) { ctxt.SendBufSize = ctx.Int("send-buf-size") ctxt.RecvBufSize = ctx.Int("recv-buf-size") ctxt.IdleTimeout = ctx.Duration("idle-timeout") + ctxt.ReadHeaderTimeout = ctx.Duration("read-header-timeout") ctxt.UserTimeout = ctx.Duration("conn-user-timeout") if conf := ctx.String("config"); len(conf) > 0 { diff --git a/cmd/server-main.go b/cmd/server-main.go index 48ed0f87d..aa450faee 100644 --- a/cmd/server-main.go +++ b/cmd/server-main.go @@ -905,7 +905,11 @@ func serverMain(ctx *cli.Context) { UseHandler(setCriticalErrorHandler(corsHandler(handler))). UseTLSConfig(newTLSConfig(getCert)). UseIdleTimeout(globalServerCtxt.IdleTimeout). - UseReadTimeout(globalServerCtxt.IdleTimeout). + // WriteTimeout only resets the activity-based write deadline + // that deadlineconn enforces per response write. ReadTimeout is + // left at zero: with native read deadlines honored again it would + // cap the whole request including large uploads; request bodies + // are bounded per-read by xhttp instead. UseWriteTimeout(globalServerCtxt.IdleTimeout). UseReadHeaderTimeout(globalServerCtxt.ReadHeaderTimeout). UseBaseContext(GlobalContext). diff --git a/internal/http/listener.go b/internal/http/listener.go index bc6de3af9..79345420d 100644 --- a/internal/http/listener.go +++ b/internal/http/listener.go @@ -70,7 +70,12 @@ func (listener *httpListener) Accept() (conn net.Conn, err error) { if result.err != nil { return nil, result.err } - return deadlineconn.New(result.conn).WithReadDeadline(listener.opts.IdleTimeout).WithWriteDeadline(listener.opts.IdleTimeout), result.err + // The read side must stay on native deadlines: an activity-based + // read deadline would extend the absolute ReadHeaderTimeout set by + // net/http on every partial read, letting slow-header clients keep + // connections open indefinitely (slowloris). Request bodies are + // bounded per-read by idleTimeoutBody in the server instead. + return deadlineconn.New(result.conn).WithWriteDeadline(listener.opts.IdleTimeout), result.err case <-listener.ctxDoneCh: } return nil, syscall.EINVAL diff --git a/internal/http/server-deadline_test.go b/internal/http/server-deadline_test.go new file mode 100644 index 000000000..8bfb7bc53 --- /dev/null +++ b/internal/http/server-deadline_test.go @@ -0,0 +1,157 @@ +// Copyright (c) 2026 PGSTY +// SPDX-License-Identifier: AGPL-3.0-or-later + +package http + +import ( + "context" + "io" + "net" + "net/http" + "strings" + "testing" + "time" +) + +// startDeadlineTestServer starts a full xhttp server (listener + wrapped +// handler, mirroring the server-main timeout layout) on an ephemeral port. +func startDeadlineTestServer(t *testing.T, handler http.Handler, idle, readHeaderTimeout time.Duration) string { + t.Helper() + + srv := NewServer([]string{"127.0.0.1:0"}). + UseHandler(handler). + UseIdleTimeout(idle). + UseWriteTimeout(idle). + UseReadHeaderTimeout(readHeaderTimeout). + // Mirror server-main, which routes the idle timeout to the listener + // (and from there to deadlineconn) through TCPOptions. + UseTCPOptions(TCPOptions{IdleTimeout: idle}) + + serveFn, err := srv.Init(context.Background(), func(listenAddr string, err error) { + t.Fatalf("listen %s: %v", listenAddr, err) + }) + if err != nil { + t.Fatalf("Init: %v", err) + } + go serveFn() + t.Cleanup(func() { srv.Shutdown() }) + + srv.listenerMutex.Lock() + defer srv.listenerMutex.Unlock() + return srv.listener.Addr().String() +} + +// A client that keeps trickling header bytes must have its connection cut +// once ReadHeaderTimeout elapses in total, not per byte. +func TestServerSlowHeaderConnectionKilled(t *testing.T) { + addr := startDeadlineTestServer(t, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}), + 5*time.Second, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + // Send a partial header line, then dribble one byte at a time well + // within the idle timeout, so only the absolute deadline can kill it. + if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: 127.0.0.1\r\nX-Slow: ")); err != nil { + t.Fatalf("write header: %v", err) + } + go func() { + for i := 0; ; i++ { + if _, err := conn.Write([]byte{byte('a' + i%26)}); err != nil { + return + } + time.Sleep(100 * time.Millisecond) + } + }() + + conn.SetReadDeadline(time.Now().Add(4 * time.Second)) + start := time.Now() + buf := make([]byte, 1) + for { + _, err := conn.Read(buf) + if err != nil { + break + } + } + if elapsed := time.Since(start); elapsed > 2*time.Second { + t.Fatalf("slow-header connection survived %v, want it closed within ReadHeaderTimeout+slack", elapsed) + } +} + +// A request whose body stalls mid-transfer must be cut off by the per-read +// idle deadline, while the handler observes the read error. +func TestServerStalledRequestBodyKilled(t *testing.T) { + bodyResult := make(chan error, 1) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := io.Copy(io.Discard, r.Body) + bodyResult <- err + }) + addr := startDeadlineTestServer(t, handler, 1*time.Second, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + head := "POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 100\r\n\r\n" + if _, err := conn.Write([]byte(head + strings.Repeat("x", 10))); err != nil { + t.Fatalf("write: %v", err) + } + // Stall: no further body bytes are ever sent. + + select { + case err := <-bodyResult: + if err == nil { + t.Fatal("handler completed a request whose body stalled forever") + } + case <-time.After(3 * time.Second): + t.Fatal("stalled body kept the handler blocked past the idle deadline") + } +} + +// A body that keeps making progress, even slower than the idle timeout in +// total, must complete: the deadline is per-read activity, never absolute. +func TestServerSlowProgressingBodyAccepted(t *testing.T) { + bodyResult := make(chan int, 1) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + n, _ := io.Copy(io.Discard, r.Body) + bodyResult <- int(n) + }) + addr := startDeadlineTestServer(t, handler, 500*time.Millisecond, 1*time.Second) + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + const total = 100 + head := "POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Length: 100\r\n\r\n" + if _, err := conn.Write([]byte(head)); err != nil { + t.Fatalf("write: %v", err) + } + // 20ms per byte: the whole body takes 2s, four times the idle timeout, + // but every inter-byte gap stays far below it. + go func() { + for i := 0; i < total; i++ { + if _, err := conn.Write([]byte{'y'}); err != nil { + return + } + time.Sleep(20 * time.Millisecond) + } + }() + + select { + case n := <-bodyResult: + if n != total { + t.Fatalf("handler read %d bytes, want %d", n, total) + } + case <-time.After(5 * time.Second): + t.Fatal("progressing slow body was terminated") + } +} diff --git a/internal/http/server.go b/internal/http/server.go index 2934fda6c..c63b59645 100644 --- a/internal/http/server.go +++ b/internal/http/server.go @@ -21,6 +21,7 @@ import ( "context" "crypto/tls" "errors" + "io" "log" "net" "net/http" @@ -114,6 +115,19 @@ func (srv *Server) Init(listenCtx context.Context, listenErrCallback func(listen atomic.AddInt32(&srv.requestCount, 1) defer atomic.AddInt32(&srv.requestCount, -1) + // Bound request bodies by read activity: refresh the connection + // read deadline ahead of every body read so a stalled upload is + // cut off while a slow but progressing one is never terminated. + // This replaces the connection-level read idle timeout that had + // to be dropped to keep ReadHeaderTimeout absolute (slowloris). + if idle := srv.IdleTimeout; idle > 0 && r.Body != nil && r.Body != http.NoBody { + r.Body = &idleTimeoutBody{ + rc: http.NewResponseController(w), + body: r.Body, + idle: idle, + } + } + // Handle request using passed handler. handler.ServeHTTP(w, r) }) @@ -217,6 +231,28 @@ func (srv *Server) UseTCPOptions(opts TCPOptions) *Server { return srv } +// idleTimeoutBody wraps a request body and refreshes the connection read +// deadline ahead of every read, giving bodies an activity-based timeout: +// reads that stall longer than idle fail with a timeout error while reads +// that keep making progress extend the deadline indefinitely. +type idleTimeoutBody struct { + rc *http.ResponseController + body io.ReadCloser + idle time.Duration +} + +func (b *idleTimeoutBody) Read(p []byte) (n int, err error) { + // Best effort: on connections where deadline control is unavailable + // (e.g. hijacked) the call is a no-op and reads stay unbounded, as + // they were before. + _ = b.rc.SetReadDeadline(time.Now().Add(b.idle)) + return b.body.Read(p) +} + +func (b *idleTimeoutBody) Close() error { + return b.body.Close() +} + // NewServer - creates new HTTP server using given arguments. func NewServer(addrs []string) *Server { httpServer := &Server{