From cd66e2414a9a2add456459f21671765f4576888b Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 29 Oct 2026 09:21:38 +0000 Subject: [PATCH] noise: restrict metrics to the metrics listener Fixes #3571 --- CHANGELOG.md | 4 ++ hscontrol/noise.go | 2 - hscontrol/noise_test.go | 122 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c52d7e582..7b3d015a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ **Minimum supported Tailscale client version: v1.84.0** +### Security + +- Metrics are no longer exposed to unregistered clients through the Noise protocol. They remain available only on the configured metrics listener; an empty `metrics_listen_addr` disables that listener. [#3571](https://github.com/juanfont/headscale/issues/3571) + ### v1 REST API replaced; gRPC and Protobuf removed The v1 REST API now provides an OpenAPI 3.1 specification at diff --git a/hscontrol/noise.go b/hscontrol/noise.go index 7df253399..a7f7730b5 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -160,8 +160,6 @@ func (h *Headscale) NoiseUpgradeHandler( r.Use(middleware.RequestLogger(&zerologRequestLogger{})) r.Use(middleware.Recoverer) - r.Handle("/metrics", metrics.Handler()) - r.Route("/machine", func(r chi.Router) { r.Post("/register", ns.RegistrationHandler) r.Post("/map", ns.PollNetMapHandler) diff --git a/hscontrol/noise_test.go b/hscontrol/noise_test.go index dadde22ef..eaaa8bf89 100644 --- a/hscontrol/noise_test.go +++ b/hscontrol/noise_test.go @@ -29,11 +29,133 @@ import ( "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/control/ts2021" + "tailscale.com/net/netmon" + "tailscale.com/net/tsdial" "tailscale.com/tailcfg" "tailscale.com/types/key" "tailscale.com/util/zstdframe" ) +var privateDiagnosticPaths = []string{ + "/metrics", + "/debug/", + "/debug/vars", + "/debug/varz", + "/debug/pprof/", + "/debug/pprof/goroutine", + "/debug/statsviz/", + "/debug/statsviz/ws", + "/debug/overview", + "/debug/config", + "/debug/policy", + "/debug/filter", + "/debug/ssh", + "/debug/derp", + "/debug/nodestore", + "/debug/registration-cache", + "/debug/routes", + "/debug/policy-manager", + "/debug/mapresponses", + "/debug/batcher", + "/debug/ping", +} + +// TestNoiseRouter_DoesNotExposeAdministrativeEndpoints covers https://github.com/juanfont/headscale/issues/3571. +func TestNoiseRouter_DoesNotExposeAdministrativeEndpoints(t *testing.T) { + t.Parallel() + + for _, metricsAddr := range []string{"", "127.0.0.1:9090"} { + t.Run("metrics_addr="+metricsAddr, func(t *testing.T) { + t.Parallel() + + serverKey := key.NewMachine() + app := &Headscale{ + cfg: &types.Config{MetricsAddr: metricsAddr}, + noisePrivateKey: &serverKey, + } + server := httptest.NewServer(http.HandlerFunc(app.NoiseUpgradeHandler)) + t.Cleanup(server.Close) + + dialer := tsdial.NewDialer(netmon.NewStatic()) + + t.Cleanup(func() { assert.NoError(t, dialer.Close()) }) + + // A fresh machine key has no registered node or credentials. + client, err := ts2021.NewClient(ts2021.ClientOpts{ + ServerURL: server.URL, + PrivKey: key.NewMachine(), + ServerPubKey: serverKey.Public(), + Dialer: dialer, + Logf: t.Logf, + }) + require.NoError(t, err) + t.Cleanup(func() { assert.NoError(t, client.Close()) }) + + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + + resp, err := client.DoWithBody(ctx, http.MethodGet, "/machine/whoami", key.NodePublic{}, nil) + require.NoError(t, err) + assert.Equal(t, http.StatusNotImplemented, resp.StatusCode, + "the Noise protocol router must remain reachable before registration") + require.NoError(t, resp.Body.Close()) + + paths := slices.Concat(privateDiagnosticPaths, []string{ + "/api/v1/node", + "/api/v1/apikey", + "/api/v1/debug/node", + "/api/v1/openapi.yaml", + "/api/v2/tailnet/-/settings", + "/api/v2/tailnet/-/keys", + "/api/v2/docs", + }) + for _, path := range paths { + for _, method := range []string{http.MethodGet, http.MethodHead} { + t.Run(method+" "+path, func(t *testing.T) { + resp, err := client.DoWithBody(ctx, method, path, key.NodePublic{}, nil) + require.NoError(t, err) + + defer resp.Body.Close() + + assert.Equal(t, 2, resp.ProtoMajor) + assert.Equal(t, http.StatusNotFound, resp.StatusCode, + "administrative endpoints must not be mounted on Noise") + }) + } + } + }) + } +} + +func TestMetricsRouterIsolation(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + public := app.HTTPHandler() + + for _, path := range privateDiagnosticPaths { + t.Run(path, func(t *testing.T) { + t.Parallel() + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, path, nil) + rec := httptest.NewRecorder() + public.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusNotFound, rec.Code, + "diagnostic endpoints must not be mounted on the public router") + }) + } + + private := app.debugHTTPServer().Handler + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/metrics", nil) + rec := httptest.NewRecorder() + private.ServeHTTP(rec, req) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), "# HELP") +} + // newNoiseRouterWithBodyLimit builds a chi router with the same body-limit // middleware used in the real Noise router but wired to a test handler that // captures the [io.ReadAll] result. This lets us verify the limit without