Skip to content

Commit b4a5428

Browse files
bootstrap: set ReadHeaderTimeout on the HTTP server
The *http.Server built by Runner.newServer left ReadHeaderTimeout unset, so request-header reads were unbounded -- a Slowloris-style connection exhaustion vector (gosec G112). The exporter-toolkit web package does not set server timeouts either, so every bootstrap-based exporter inherited the gap. Set ReadHeaderTimeout to one minute. It bounds only header reading, not the metrics handler, so it never affects legitimate scrapes. The value is a package var so tests can shorten it. Follow-up to prometheus/blackbox_exporter#1626, which made the same change on the caller side; setting it here fixes it once for all toolkit users. go build, go vet, and go test ./... pass; gosec G112 clears. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: randomizedcoder dave.seddon.ca@gmail.com <dave.seddon.ca@gmail.com>
1 parent 72861ea commit b4a5428

2 files changed

Lines changed: 89 additions & 1 deletion

File tree

bootstrap/bootstrap.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"log/slog"
2323
"net/http"
2424
"os"
25+
"time"
2526

2627
"github.com/alecthomas/kingpin/v2"
2728
"github.com/prometheus/common/promslog"
@@ -43,6 +44,12 @@ var (
4344
errNegativeMaxRequests = errors.New("web max requests must be greater than or equal to zero")
4445
)
4546

47+
// serverReadHeaderTimeout bounds how long the server waits to read request
48+
// headers, mitigating Slowloris-style connection exhaustion (gosec G112). It
49+
// covers only header reading, not the metrics handler, so a generous value is
50+
// safe for legitimate scrapes. It is a package var so tests can shorten it.
51+
var serverReadHeaderTimeout = time.Minute
52+
4653
// MetricsHandlerFactory builds an exporter-specific metrics handler after the
4754
// common toolkit flags have been parsed.
4855
type MetricsHandlerFactory func(*Bootstrap) (http.Handler, error)
@@ -250,7 +257,10 @@ func (t *Runner) newServer(metricsHandler http.Handler) (*http.Server, error) {
250257
mux.Handle("/", landingPage)
251258
}
252259

253-
return &http.Server{Handler: mux}, nil
260+
return &http.Server{
261+
Handler: mux,
262+
ReadHeaderTimeout: serverReadHeaderTimeout,
263+
}, nil
254264
}
255265

256266
func (t *Runner) defaultLandingConfig() web.LandingConfig {

bootstrap/bootstrap_test.go

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
package bootstrap
1515

1616
import (
17+
"net"
1718
"net/http"
1819
"net/http/httptest"
1920
"strings"
2021
"testing"
22+
"time"
2123

2224
"github.com/alecthomas/kingpin/v2"
2325
"github.com/prometheus/common/promslog"
@@ -129,3 +131,79 @@ func TestNewServerRegistersMetricsAndLandingPage(t *testing.T) {
129131
t.Fatalf("unexpected landing body: %q", body)
130132
}
131133
}
134+
135+
// TestNewServerSetsReadHeaderTimeout verifies that the server built by newServer
136+
// bounds request-header reads (ReadHeaderTimeout), so a connection whose headers
137+
// never complete is closed rather than held open indefinitely (a Slowloris
138+
// vector, gosec G112). A well-formed request must still succeed.
139+
func TestNewServerSetsReadHeaderTimeout(t *testing.T) {
140+
// Shorten the production timeout for the test; newServer reads this var.
141+
orig := serverReadHeaderTimeout
142+
serverReadHeaderTimeout = 250 * time.Millisecond
143+
t.Cleanup(func() { serverReadHeaderTimeout = orig })
144+
145+
tk := New(Config{
146+
App: kingpin.New("test", ""),
147+
Name: "test_exporter",
148+
Description: "test description",
149+
DefaultAddress: ":9100",
150+
Logger: promslog.NewNopLogger(),
151+
MetricsHandler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
152+
w.WriteHeader(http.StatusOK)
153+
}),
154+
})
155+
if err := tk.parse([]string{"--web.listen-address=:0"}); err != nil {
156+
t.Fatalf("unexpected parse error: %v", err)
157+
}
158+
handler, err := tk.resolveMetricsHandler()
159+
if err != nil {
160+
t.Fatalf("unexpected handler resolution error: %v", err)
161+
}
162+
server, err := tk.newServer(handler)
163+
if err != nil {
164+
t.Fatalf("unexpected server creation error: %v", err)
165+
}
166+
if server.ReadHeaderTimeout != serverReadHeaderTimeout {
167+
t.Fatalf("unexpected ReadHeaderTimeout: got %v, want %v", server.ReadHeaderTimeout, serverReadHeaderTimeout)
168+
}
169+
170+
ln, err := net.Listen("tcp", "127.0.0.1:0")
171+
if err != nil {
172+
t.Fatalf("listen: %v", err)
173+
}
174+
t.Cleanup(func() { _ = ln.Close() })
175+
go func() { _ = server.Serve(ln) }()
176+
t.Cleanup(func() { _ = server.Close() })
177+
178+
// Sanity: a well-formed request still succeeds.
179+
resp, err := http.Get("http://" + ln.Addr().String() + "/metrics")
180+
if err != nil {
181+
t.Fatalf("well-formed request failed: %v", err)
182+
}
183+
_ = resp.Body.Close()
184+
185+
// A client that starts request headers but never terminates them (no final
186+
// CRLF), holding the connection open.
187+
conn, err := net.Dial("tcp", ln.Addr().String())
188+
if err != nil {
189+
t.Fatalf("dial: %v", err)
190+
}
191+
t.Cleanup(func() { _ = conn.Close() })
192+
if _, err := conn.Write([]byte("GET /metrics HTTP/1.1\r\nHost: localhost\r\n")); err != nil {
193+
t.Fatalf("write partial request: %v", err)
194+
}
195+
196+
// The server must close the stalled connection once ReadHeaderTimeout
197+
// elapses. Read in a goroutine so the test never hangs if it does not.
198+
done := make(chan struct{})
199+
go func() {
200+
_, _ = conn.Read(make([]byte, 1)) // unblocks on server-side close
201+
close(done)
202+
}()
203+
select {
204+
case <-done:
205+
// Connection closed: ReadHeaderTimeout is effective.
206+
case <-time.After(5 * time.Second):
207+
t.Fatal("server did not close the stalled connection within 5s; ReadHeaderTimeout not effective")
208+
}
209+
}

0 commit comments

Comments
 (0)