diff options
| author | srdusr <[email protected]> | 2026-05-16 11:49:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2026-05-16 11:49:00 +0200 |
| commit | 1707cf827592bc3de7ea9c2805b7adea49b3cbc5 (patch) | |
| tree | c61adf900307a76b557041c985b6a029393569c6 /internal | |
| parent | dd1621c0077e079e0693f16fe83f17d50338216e (diff) | |
| download | mitmux-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.go | 39 |
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) |