Skip to content

Commit 785b77c

Browse files
committed
[KLC-2434] bound REST slow-body reads and tighten header/body caps (GHSA-w4c6-7r69-w7j9)
Follow-up to KLC-2428 (slow-header fix): extend the slow-header hardening on the shared http.Server with a slow-body defense and tighter limits: - Add a per-request body read deadline (30s) via ResponseController.SetReadDeadline, applied only to body-bearing requests so the bodiless websocket handshakes (/log, /subscribe) are never wrapped, and cleared once the body is read so it never cancels a slow post-read handler. - Gate the exemption on bodiless requests instead of an Upgrade: websocket header, which a client could spoof on a POST to skip the deadline. - Set MaxHeaderBytes to 32 KiB; the previous 1 MiB matched Go's default and was a no-op. - Lower MaxBodyBytes to 2 MiB. - Clear the read deadline under sync.Once. Cover the body deadline (slow-body cut, slow-handler-not-cancelled, bodiless-upgrade skip), a live websocket round-trip through the hardened server, and an end-to-end slow-header drop on the real seednode routes.
1 parent a8d6682 commit 785b77c

3 files changed

Lines changed: 270 additions & 35 deletions

File tree

cmd/seednode/api/api_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package api
22

33
import (
4+
"bufio"
45
"encoding/json"
6+
"net"
57
"net/http"
68
"net/http/httptest"
79
"strings"
@@ -10,6 +12,7 @@ import (
1012

1113
"github.com/gin-gonic/gin"
1214
"github.com/klever-io/klever-go/core"
15+
"github.com/klever-io/klever-go/network/api/httpserver"
1316
)
1417

1518
type stubMessenger struct {
@@ -45,6 +48,41 @@ func setup(t *testing.T) (*gin.Engine, *stubMessenger) {
4548
return r, stub
4649
}
4750

51+
// TestSeednodeAPI_HardenedServerDropsSlowHeader serves the real seednode routes through
52+
// NewHardenedServer and confirms the seednode listener (the reporter's PoC path) drops a
53+
// slow-header connection — GHSA-w4c6-7r69-w7j9, verified end-to-end, not just wired.
54+
func TestSeednodeAPI_HardenedServerDropsSlowHeader(t *testing.T) {
55+
r, _ := setup(t)
56+
57+
ln, err := net.Listen("tcp", "127.0.0.1:0")
58+
if err != nil {
59+
t.Fatalf("listen: %v", err)
60+
}
61+
62+
srv := httpserver.NewHardenedServer(ln.Addr().String(), r.Handler())
63+
srv.ReadHeaderTimeout = 200 * time.Millisecond // tighten for a fast test
64+
go func() { _ = srv.Serve(ln) }()
65+
defer func() { _ = srv.Close() }()
66+
67+
conn, err := net.Dial("tcp", ln.Addr().String())
68+
if err != nil {
69+
t.Fatalf("dial: %v", err)
70+
}
71+
defer func() { _ = conn.Close() }()
72+
73+
// Partial header, never terminated: the seednode listener must drop it.
74+
if _, err := conn.Write([]byte("GET /node/status HTTP/1.1\r\nHost: x\r\n")); err != nil {
75+
t.Fatalf("write: %v", err)
76+
}
77+
78+
start := time.Now()
79+
_ = conn.SetReadDeadline(time.Now().Add(3 * time.Second))
80+
_, _ = bufio.NewReader(conn).ReadString('\n')
81+
if elapsed := time.Since(start); elapsed > time.Second {
82+
t.Fatalf("slow-header connection not dropped promptly: %v", elapsed)
83+
}
84+
}
85+
4886
func TestPeers_returnsCountsAndSortedAddresses(t *testing.T) {
4987
r, _ := setup(t)
5088

Lines changed: 74 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,102 @@
1-
// Package httpserver builds the hardened *http.Server shared by both REST start
2-
// paths (seednode and node). Gin's Engine.Run uses http.ListenAndServe with no
3-
// ReadHeaderTimeout, leaving it open to slow-header connection exhaustion
4-
// (GHSA-w4c6-7r69-w7j9); this helper hardens both listeners identically.
1+
// Package httpserver builds the hardened *http.Server shared by both REST start paths
2+
// (seednode and node). Gin's Engine.Run uses http.ListenAndServe with no
3+
// ReadHeaderTimeout, leaving it open to slow-header exhaustion (GHSA-w4c6-7r69-w7j9).
54
package httpserver
65

76
import (
7+
"io"
88
"net/http"
9+
"sync"
910
"time"
1011
)
1112

1213
const (
13-
// ReadHeaderTimeout is the slow-header (slowloris) mitigation: it bounds the
14-
// time to send the complete header. Header-only, so it is safe for the
15-
// long-lived websocket streams these APIs serve (cleared before hijack).
14+
// ReadHeaderTimeout bounds header read time — the slow-header (slowloris) fix.
1615
ReadHeaderTimeout = 10 * time.Second
1716

18-
// IdleTimeout bounds how long an idle keep-alive connection stays open.
17+
// BodyReadTimeout bounds body read time — the slow-body fix.
18+
BodyReadTimeout = 30 * time.Second
19+
20+
// IdleTimeout bounds idle keep-alive connections.
1921
IdleTimeout = 120 * time.Second
2022

21-
// MaxHeaderBytes caps request header size (Go's default, set explicitly).
22-
MaxHeaderBytes = 1 << 20 // 1 MiB
23+
// MaxHeaderBytes caps total request header size (Go's 1 MiB default is a no-op).
24+
MaxHeaderBytes = 32 << 10 // 32 KiB
2325

24-
// MaxBodyBytes caps the request body. A single tx is bounded by the ~960 KiB
25-
// P2P wire limit (~1.9 MiB once JSON-encoded), so 4 MiB covers the largest
26-
// legitimate request with margin; bulk /transaction/broadcast is additionally
27-
// bounded by an explicit tx count. Over-cap bodies are refused (400 on bind,
28-
// 413 raw). Bounds body size, not read time — see the body read-deadline follow-up.
29-
MaxBodyBytes = 4 << 20 // 4 MiB
26+
// MaxBodyBytes caps the request body; over-cap bodies are refused (400/413).
27+
MaxBodyBytes = 2 << 20 // 2 MiB
3028
)
3129

32-
// NewHardenedServer returns an *http.Server for addr serving handler, hardened
33-
// against slow-header exhaustion and oversized bodies. ReadTimeout/WriteTimeout
34-
// are left unset on purpose: a whole-connection deadline would sever the
35-
// long-lived websocket streams these APIs serve (/log, /subscribe).
30+
// NewHardenedServer returns an *http.Server hardened against slow-header/slow-body
31+
// exhaustion. ReadTimeout/WriteTimeout are left unset: a whole-connection deadline
32+
// would sever the long-lived websocket streams these APIs serve (/log, /subscribe).
3633
func NewHardenedServer(addr string, handler http.Handler) *http.Server {
3734
return &http.Server{
3835
Addr: addr,
39-
Handler: limitRequestBody(handler),
36+
Handler: guardRequestBody(handler),
4037
ReadHeaderTimeout: ReadHeaderTimeout,
4138
IdleTimeout: IdleTimeout,
4239
MaxHeaderBytes: MaxHeaderBytes,
4340
}
4441
}
4542

46-
// limitRequestBody caps every request body at MaxBodyBytes.
47-
func limitRequestBody(next http.Handler) http.Handler {
48-
return limitRequestBodyN(next, MaxBodyBytes)
49-
}
50-
51-
// limitRequestBodyN caps each request body at limit bytes. Applied ahead of gin so
52-
// w is the *http.response MaxBytesReader needs to close an over-cap connection.
53-
// Websocket upgrades hijack the connection and never read r.Body, so are unaffected.
54-
func limitRequestBodyN(next http.Handler, limit int64) http.Handler {
43+
// guardRequestBody caps body size (MaxBodyBytes) and body read time (BodyReadTimeout).
44+
// Applied ahead of gin so w is the *http.response both mechanisms need.
45+
func guardRequestBody(next http.Handler) http.Handler {
5546
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
56-
if r.Body != nil {
57-
r.Body = http.MaxBytesReader(w, r.Body, limit)
58-
}
47+
capRequestBody(w, r, MaxBodyBytes)
48+
applyBodyReadDeadline(w, r, BodyReadTimeout)
5949
next.ServeHTTP(w, r)
6050
})
6151
}
52+
53+
// capRequestBody limits r.Body to limit bytes.
54+
func capRequestBody(w http.ResponseWriter, r *http.Request, limit int64) {
55+
if r.Body != nil {
56+
r.Body = http.MaxBytesReader(w, r.Body, limit)
57+
}
58+
}
59+
60+
// applyBodyReadDeadline bounds body read time at d, clearing the deadline once the body
61+
// is read so it never cancels a slow post-read handler. Bodiless requests are skipped —
62+
// that covers the websocket handshakes (/log, /subscribe), which are bodiless GETs, and
63+
// leaves no Upgrade-header check for a body-bearing request to spoof.
64+
func applyBodyReadDeadline(w http.ResponseWriter, r *http.Request, d time.Duration) {
65+
if r.Body == nil || !requestHasBody(r) {
66+
return
67+
}
68+
rc := http.NewResponseController(w)
69+
if rc.SetReadDeadline(time.Now().Add(d)) != nil {
70+
return
71+
}
72+
r.Body = &deadlineClearingBody{ReadCloser: r.Body, rc: rc}
73+
}
74+
75+
func requestHasBody(r *http.Request) bool {
76+
return r.ContentLength != 0 // 0 = no body; -1 (chunked) and positive = body
77+
}
78+
79+
// deadlineClearingBody clears the read deadline once the body read completes (EOF or
80+
// error) or the body is closed, so the deadline never cancels a later slow handler.
81+
type deadlineClearingBody struct {
82+
io.ReadCloser
83+
rc *http.ResponseController
84+
clearOnce sync.Once
85+
}
86+
87+
func (b *deadlineClearingBody) clear() {
88+
b.clearOnce.Do(func() { _ = b.rc.SetReadDeadline(time.Time{}) })
89+
}
90+
91+
func (b *deadlineClearingBody) Read(p []byte) (int, error) {
92+
n, err := b.ReadCloser.Read(p)
93+
if err != nil {
94+
b.clear()
95+
}
96+
return n, err
97+
}
98+
99+
func (b *deadlineClearingBody) Close() error {
100+
b.clear()
101+
return b.ReadCloser.Close()
102+
}

network/api/httpserver/httpserver_test.go

Lines changed: 158 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"testing"
1212
"time"
1313

14+
"github.com/gorilla/websocket"
1415
"github.com/stretchr/testify/require"
1516
)
1617

@@ -125,13 +126,14 @@ func TestNewHardenedServer_CapsRequestBody(t *testing.T) {
125126

126127
// Status carries the read outcome, so assertions read only the response code —
127128
// no state shared across the server/test goroutines.
128-
handler := limitRequestBodyN(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
129+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
130+
capRequestBody(w, r, limit)
129131
if _, err := io.ReadAll(r.Body); err != nil {
130132
w.WriteHeader(http.StatusRequestEntityTooLarge)
131133
return
132134
}
133135
w.WriteHeader(http.StatusOK)
134-
}), limit)
136+
})
135137

136138
srv := httptest.NewServer(handler)
137139
defer srv.Close()
@@ -148,3 +150,157 @@ func TestNewHardenedServer_CapsRequestBody(t *testing.T) {
148150
_ = resp.Body.Close()
149151
require.Equal(t, http.StatusOK, resp.StatusCode, "body within the cap must be accepted")
150152
}
153+
154+
// TestApplyBodyReadDeadline_CutsSlowBody: a client that completes the header, promises
155+
// a large body, then stalls is cut at ~BodyReadTimeout instead of pinning the connection.
156+
func TestApplyBodyReadDeadline_CutsSlowBody(t *testing.T) {
157+
t.Parallel()
158+
159+
const deadline = 300 * time.Millisecond
160+
161+
readDone := make(chan error, 1)
162+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
163+
applyBodyReadDeadline(w, r, deadline)
164+
_, err := io.ReadAll(r.Body)
165+
readDone <- err
166+
})
167+
168+
srv := httptest.NewServer(handler)
169+
defer srv.Close()
170+
171+
conn, err := net.Dial("tcp", srv.Listener.Addr().String())
172+
require.Nil(t, err)
173+
defer func() { _ = conn.Close() }()
174+
175+
// Promise 1 MiB but send only a few bytes, then stall — the body read blocks.
176+
_, err = fmt.Fprint(conn, "POST / HTTP/1.1\r\nHost: x\r\nContent-Length: 1048576\r\n\r\npartial")
177+
require.Nil(t, err)
178+
179+
start := time.Now()
180+
select {
181+
case err := <-readDone:
182+
require.Error(t, err, "a stalled body read must be cut by the deadline")
183+
require.Less(t, time.Since(start), 2*time.Second, "body must be cut promptly (~BodyReadTimeout)")
184+
case <-time.After(2 * time.Second):
185+
t.Fatal("stalled body read was not cut by the deadline")
186+
}
187+
}
188+
189+
// TestApplyBodyReadDeadline_DoesNotCancelSlowHandler: a handler that reads the body then
190+
// works past the deadline still returns 200 — the deadline bounds the body read, not
191+
// post-read CPU work (e.g. a VM query) or the response write. (clear() has no observable
192+
// effect to assert in standard net/http, so this checks the property that matters.)
193+
func TestApplyBodyReadDeadline_DoesNotCancelSlowHandler(t *testing.T) {
194+
t.Parallel()
195+
196+
const deadline = 200 * time.Millisecond
197+
198+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
199+
applyBodyReadDeadline(w, r, deadline)
200+
if _, err := io.ReadAll(r.Body); err != nil {
201+
w.WriteHeader(http.StatusInternalServerError)
202+
return
203+
}
204+
time.Sleep(3 * deadline) // work well past the deadline
205+
w.WriteHeader(http.StatusOK)
206+
})
207+
208+
srv := httptest.NewServer(handler)
209+
defer srv.Close()
210+
211+
resp, err := http.Post(srv.URL, "text/plain", bytes.NewReader([]byte("hello")))
212+
require.Nil(t, err)
213+
_ = resp.Body.Close()
214+
require.Equal(t, http.StatusOK, resp.StatusCode,
215+
"the body deadline must not cancel a slow post-read handler")
216+
}
217+
218+
// TestRequestHasBody covers the predicate that gates the body deadline: bodiless requests
219+
// (including the bodiless-GET websocket handshakes) are skipped.
220+
func TestRequestHasBody(t *testing.T) {
221+
t.Parallel()
222+
223+
require.False(t, requestHasBody(&http.Request{ContentLength: 0}), "no body (incl. websocket handshake)")
224+
require.True(t, requestHasBody(&http.Request{ContentLength: 5}), "known body")
225+
require.True(t, requestHasBody(&http.Request{ContentLength: -1}), "chunked body")
226+
}
227+
228+
// TestApplyBodyReadDeadline_SkipsBodilessUpgrade: a bodiless GET with Upgrade: websocket
229+
// is not wrapped (no deadline armed), so the /log and /subscribe streams are never severed.
230+
func TestApplyBodyReadDeadline_SkipsBodilessUpgrade(t *testing.T) {
231+
t.Parallel()
232+
233+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
234+
before := r.Body
235+
applyBodyReadDeadline(w, r, BodyReadTimeout)
236+
// r.Body must be unchanged — a wrapped body would mean a deadline was armed.
237+
if r.Body != before {
238+
w.WriteHeader(http.StatusInternalServerError)
239+
return
240+
}
241+
w.WriteHeader(http.StatusOK)
242+
})
243+
244+
srv := httptest.NewServer(handler)
245+
defer srv.Close()
246+
247+
req, err := http.NewRequest(http.MethodGet, srv.URL, nil) // bodiless GET, like a handshake
248+
require.Nil(t, err)
249+
req.Header.Set("Connection", "Upgrade")
250+
req.Header.Set("Upgrade", "websocket")
251+
252+
resp, err := http.DefaultClient.Do(req)
253+
require.Nil(t, err)
254+
_ = resp.Body.Close()
255+
require.Equal(t, http.StatusOK, resp.StatusCode, "bodiless websocket handshake must not be wrapped")
256+
}
257+
258+
// TestNewHardenedServer_WebSocketStreamWorks: a real websocket upgrade survives the
259+
// hardened server — frames round-trip across idle gaps without the stream being severed.
260+
func TestNewHardenedServer_WebSocketStreamWorks(t *testing.T) {
261+
t.Parallel()
262+
263+
upgrader := websocket.Upgrader{CheckOrigin: func(*http.Request) bool { return true }}
264+
mux := http.NewServeMux()
265+
mux.HandleFunc("/ws", func(w http.ResponseWriter, r *http.Request) {
266+
conn, err := upgrader.Upgrade(w, r, nil)
267+
if err != nil {
268+
return
269+
}
270+
defer func() { _ = conn.Close() }()
271+
for { // echo until the client closes
272+
mt, msg, err := conn.ReadMessage()
273+
if err != nil {
274+
return
275+
}
276+
if err := conn.WriteMessage(mt, msg); err != nil {
277+
return
278+
}
279+
}
280+
})
281+
282+
ln, err := net.Listen("tcp", "127.0.0.1:0")
283+
require.Nil(t, err)
284+
srv := NewHardenedServer(ln.Addr().String(), mux)
285+
go func() { _ = srv.Serve(ln) }()
286+
defer func() { _ = srv.Close() }()
287+
288+
dialer := websocket.Dialer{}
289+
wsURL := "ws://" + ln.Addr().String() + "/ws"
290+
conn, resp, err := dialer.Dial(wsURL, nil)
291+
require.Nil(t, err, "websocket upgrade must succeed through the hardened server")
292+
require.Equal(t, http.StatusSwitchingProtocols, resp.StatusCode)
293+
defer func() { _ = conn.Close() }()
294+
295+
// Exchange frames with an idle gap between them to show the stream stays open and is
296+
// not cut by ReadHeaderTimeout/IdleTimeout/body deadline once upgraded.
297+
for i, gap := range []time.Duration{0, 250 * time.Millisecond, 250 * time.Millisecond} {
298+
time.Sleep(gap)
299+
want := fmt.Sprintf("frame-%d", i)
300+
require.Nil(t, conn.WriteMessage(websocket.TextMessage, []byte(want)))
301+
_ = conn.SetReadDeadline(time.Now().Add(2 * time.Second))
302+
_, got, err := conn.ReadMessage()
303+
require.Nil(t, err, "frame %d must round-trip", i)
304+
require.Equal(t, want, string(got))
305+
}
306+
}

0 commit comments

Comments
 (0)