srdusr
aboutsummaryrefslogtreecommitdiffstats
path: root/internal
diff options
context:
space:
mode:
authorsrdusr <[email protected]>2026-05-16 11:49:00 +0200
committersrdusr <[email protected]>2026-05-16 11:49:00 +0200
commit1707cf827592bc3de7ea9c2805b7adea49b3cbc5 (patch)
treec61adf900307a76b557041c985b6a029393569c6 /internal
parentdd1621c0077e079e0693f16fe83f17d50338216e (diff)
downloadmitmux-1707cf827592bc3de7ea9c2805b7adea49b3cbc5.tar.gz
mitmux-1707cf827592bc3de7ea9c2805b7adea49b3cbc5.zip
Bound slow-loris connections and hung TLS handshakes
Neither the main proxy's http.Server nor the per-CONNECT-tunnel one had any timeouts - a client that opened a connection and either trickled request headers forever or, on the CONNECT/HTTPS path, completed the CONNECT handshake and then never sent a TLS ClientHello at all, held that connection and its goroutine open indefinitely. Confirmed live before the fix: partial headers with no terminator, and a completed CONNECT with no ClientHello, both held the connection open 10s+ with no sign of ever stopping. Low real-world risk at the 127.0.0.1 default, but multi-listener support means mitmuxd can now bind other interfaces, making this a real DoS-by-neglect surface rather than a purely theoretical one. clientHeaderTimeout (30s) is applied as ReadHeaderTimeout on both http.Server instances, and clientIdleTimeout (120s) as IdleTimeout on both - deliberately narrow, bounding only the pre-body header-parsing phase and idle time between keep-alive requests, not overall request duration, so a legitimately slow multi-minute upload/download still works exactly as before (verified: normal HTTP and HTTPS requests both still succeed after this change). The CONNECT-tunnel's TLS handshake specifically had no deadline at all before calling clientTLS.Handshake() - unlike the upstream leg, which already correctly calls conn.SetDeadline before its own round trip (see forward()). Now client.SetDeadline(...) is set with the same clientHeaderTimeout right before the handshake and cleared immediately after a successful one, so the request/response phase that follows doesn't inherit a stale handshake-only deadline. Verified live: a client sending a partial request line with no terminator was cut off at exactly 30.1s (previously indefinite); a client completing CONNECT and then never sending a ClientHello was cut off at exactly 30.1s (previously indefinite); normal HTTP and HTTPS requests through the proxy both still succeed afterward. go build/vet/gofmt/test/mod tidy all clean.
Diffstat (limited to 'internal')
-rw-r--r--internal/proxy/proxy.go39
1 files changed, 36 insertions, 3 deletions
diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go
index 08b8551..216e947 100644
--- a/internal/proxy/proxy.go
+++ b/internal/proxy/proxy.go
@@ -45,6 +45,24 @@ import (
// upstream exchange, once dialing has already succeeded.
const upstreamTimeout = 60 * time.Second
+// clientHeaderTimeout bounds how long a client connection can sit
+// sending request headers (or a TLS ClientHello, on the CONNECT-tunnel
+// leg) before mitmux gives up on it - a slow-loris style connection
+// that opens and then trickles bytes (or never sends a ClientHello at
+// all) would otherwise hold a connection and its goroutine open
+// indefinitely, with nothing else in the codebase bounding it. Narrow
+// on purpose: this only covers the pre-body phase, not overall request
+// duration - a legitimately slow multi-minute upload/download must
+// still work, so this is deliberately not a blanket ReadTimeout/
+// WriteTimeout on the whole connection.
+const clientHeaderTimeout = 30 * time.Second
+
+// clientIdleTimeout bounds how long a keep-alive client connection can
+// sit idle between requests before mitmux closes it - cleans up
+// abandoned idle connections without affecting any connection that's
+// actively mid-transfer.
+const clientIdleTimeout = 120 * time.Second
+
// hopByHopHeaders are stripped before forwarding a request or response,
// per RFC 7230 6.1 - they are meaningful only between a client and its
// immediate next hop, not end-to-end.
@@ -92,8 +110,10 @@ type Server struct {
func New(addrs []string, root *ca.CA, db *store.Store, upstreamProxy string) *Server {
s := &Server{Addrs: addrs, ca: root, store: db, UpstreamProxy: upstreamProxy}
s.server = &http.Server{
- Handler: http.HandlerFunc(s.handle),
- ConnContext: withClientTee,
+ Handler: http.HandlerFunc(s.handle),
+ ConnContext: withClientTee,
+ ReadHeaderTimeout: clientHeaderTimeout,
+ IdleTimeout: clientIdleTimeout,
}
return s
}
@@ -190,11 +210,19 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) {
NextProtos: []string{http2.NextProtoTLS, "http/1.1"},
MinVersion: tls.VersionTLS12,
})
+ // Bounded the same way the upstream leg already is (see forward's
+ // conn.SetDeadline): without this, a client that completes CONNECT
+ // and then never sends a ClientHello at all holds the connection and
+ // its goroutine open indefinitely. Cleared after a successful
+ // handshake - the request/response phase that follows has no
+ // business inheriting a short handshake-only deadline.
+ client.SetDeadline(time.Now().Add(clientHeaderTimeout))
if err := clientTLS.Handshake(); err != nil {
log.Printf("mitm handshake with client for %s: %v", hostname, err)
client.Close()
return
}
+ client.SetDeadline(time.Time{})
dial := func(ctx context.Context) (net.Conn, string, error) {
return dialUpstreamTLS(ctx, hostPort, hostname, s.UpstreamProxy)
@@ -208,7 +236,12 @@ func (s *Server) handleConnect(w http.ResponseWriter, r *http.Request) {
return
}
- h1 := &http.Server{Handler: handler, ConnContext: withClientTee}
+ h1 := &http.Server{
+ Handler: handler,
+ ConnContext: withClientTee,
+ ReadHeaderTimeout: clientHeaderTimeout,
+ IdleTimeout: clientIdleTimeout,
+ }
err = h1.Serve(newSingleConnListener(clientTLS))
if err != nil && !errors.Is(err, io.EOF) {
log.Printf("h1 serve for %s: %v", hostname, err)