From bc3ac275b85a4f2bb0f26f0bcf4ffbacbb59ee70 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 28 Oct 2026 13:36:42 +0000 Subject: [PATCH] noise: time out stalled bodies and idle tunnels The hijack drops the outer server's deadlines; any machine key gets a tunnel. Fixes ANT-2026-K903ZEUA (cherry picked from commit 28959cb27ca8ce6567e7c8d4172f03db23c0de44, adapted: 0.29 serves via http2.ServeConn, idle timeout set on http2.Server) --- hscontrol/noise.go | 23 ++++-- hscontrol/noise_test.go | 159 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 5 deletions(-) diff --git a/hscontrol/noise.go b/hscontrol/noise.go index ad01f31e3..585f02acc 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -207,11 +207,7 @@ func (h *Headscale) NoiseUpgradeHandler( r.Post("/c2n", ns.NotImplementedHandler) }) - ns.httpBaseConfig = &http.Server{ - Handler: r, - ReadHeaderTimeout: types.HTTPTimeout, - } - ns.http2Server = &http2.Server{} + ns.httpBaseConfig, ns.http2Server = newNoiseHTTPServers(r, types.HTTPTimeout) ns.http2Server.ServeConn( noiseConn, @@ -221,6 +217,23 @@ func (h *Headscale) NoiseUpgradeHandler( ) } +// newNoiseHTTPServers builds the servers for one Noise tunnel. The hijack +// drops the outer server's deadlines and any machine key gets a tunnel, so +// this is where a stalled request body (ReadTimeout) and a tunnel with no open +// stream (IdleTimeout) are bounded. WriteTimeout stays unset: it would cut +// long polls and a follow-up waiting for its registration. ServeConn takes the +// idle timeout from the [http2.Server], not from BaseConfig. +func newNoiseHTTPServers( + handler http.Handler, + timeout time.Duration, +) (*http.Server, *http2.Server) { //nolint:staticcheck // 0.29 serves tunnels via http2.ServeConn + return &http.Server{ + Handler: handler, + ReadHeaderTimeout: timeout, + ReadTimeout: timeout, + }, &http2.Server{IdleTimeout: timeout} //nolint:staticcheck // see above +} + func unsupportedClientError(version tailcfg.CapabilityVersion) error { return fmt.Errorf("%w: %s (%d)", ErrUnsupportedClientVersion, capver.TailscaleVersion(version), version) } diff --git a/hscontrol/noise_test.go b/hscontrol/noise_test.go index cb24c69a0..9492969bd 100644 --- a/hscontrol/noise_test.go +++ b/hscontrol/noise_test.go @@ -8,10 +8,12 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/http/httptest" "net/netip" "net/url" + "os" "slices" "strconv" "strings" @@ -25,6 +27,7 @@ import ( "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/http2" "tailscale.com/control/ts2021" "tailscale.com/net/netmon" "tailscale.com/net/tsdial" @@ -1544,3 +1547,159 @@ func TestRegistrationHandler_ValidatesClientReport(t *testing.T) { assert.False(t, stored.Used) }) } + +// TestNoiseHTTPServerTimeouts pins the limits of the server inside a Noise +// tunnel. Any machine key gets a tunnel, so without them a stalled request +// body or an empty tunnel lives as long as the TCP connection. +func TestNoiseHTTPServerTimeouts(t *testing.T) { + t.Parallel() + + const timeout = 200 * time.Millisecond + + // serve runs handler the way NoiseUpgradeHandler serves a tunnel. It + // returns a client for the tunnel and a channel closed once the server + // has closed the connection. + serve := func(t *testing.T, handler http.Handler) (*http.Client, <-chan struct{}) { + t.Helper() + + serverConn, clientConn := net.Pipe() + + t.Cleanup(func() { + serverConn.Close() + clientConn.Close() + }) + + closed := make(chan struct{}) + base, h2 := newNoiseHTTPServers(handler, timeout) + + go func() { + // ServeConn returns once the server has closed the connection. + h2.ServeConn(serverConn, &http2.ServeConnOpts{BaseConfig: base}) //nolint:staticcheck // as NoiseUpgradeHandler + close(closed) + }() + + protocols := new(http.Protocols) + protocols.SetUnencryptedHTTP2(true) + + transport := &http.Transport{ + Protocols: protocols, + DialContext: func(context.Context, string, string) (net.Conn, error) { + return clientConn, nil + }, + } + t.Cleanup(transport.CloseIdleConnections) + + return &http.Client{Transport: transport}, closed + } + + t.Run("stalled body times out", func(t *testing.T) { + t.Parallel() + + readErr := make(chan error, 1) + client, _ := serve(t, http.HandlerFunc(func( + _ http.ResponseWriter, r *http.Request, + ) { + _, err := io.ReadAll(r.Body) + readErr <- err + })) + + // One byte of body, then silence. + body, bodyWriter := io.Pipe() + + t.Cleanup(func() { bodyWriter.Close() }) + + go func() { _, _ = bodyWriter.Write([]byte("{")) }() + + ctx, cancel := context.WithTimeout(context.Background(), 10*timeout) + defer cancel() + + req, err := http.NewRequestWithContext( + ctx, http.MethodPost, "http://noise/machine/register", body, + ) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err, "the handler must not stay blocked on a stalled body") + resp.Body.Close() + + require.ErrorIs(t, <-readErr, os.ErrDeadlineExceeded) + }) + + // A register follow-up answers only once the user has authenticated, + // which can take longer than the timeout, and writes nothing until then. + t.Run("quiet answer past the timeout is delivered", func(t *testing.T) { + t.Parallel() + + client, _ := serve(t, http.HandlerFunc(func( + w http.ResponseWriter, r *http.Request, + ) { + _, _ = io.ReadAll(r.Body) + + select { + case <-time.After(3 * timeout): + case <-r.Context().Done(): + return + } + + _, _ = io.WriteString(w, "{}") + })) + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, "http://noise/machine/register", + strings.NewReader("{}"), + ) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err) + + got, err := io.ReadAll(resp.Body) + resp.Body.Close() + require.NoError(t, err) + assert.Equal(t, "{}", string(got)) + }) + + t.Run("long poll is kept, then the idle tunnel closes", func(t *testing.T) { + t.Parallel() + + const chunks = 5 + + client, closed := serve(t, http.HandlerFunc(func( + w http.ResponseWriter, r *http.Request, + ) { + _, _ = io.ReadAll(r.Body) + + for range chunks { + _, _ = io.WriteString(w, "chunk\n") + w.(http.Flusher).Flush() //nolint:forcetypeassert // h2 writers flush + + select { + case <-time.After(timeout): + case <-r.Context().Done(): + return + } + } + })) + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodPost, "http://noise/machine/map", + strings.NewReader("{}"), + ) + require.NoError(t, err) + + resp, err := client.Do(req) + require.NoError(t, err) + + got, err := io.ReadAll(resp.Body) + resp.Body.Close() + require.NoError(t, err) + assert.Equal(t, strings.Repeat("chunk\n", chunks), string(got), + "a stream that outlives the timeout must not be cut") + + select { + case <-closed: + case <-time.After(10 * timeout): + t.Fatal("the server kept a tunnel with no open stream") + } + }) +}