From 2e563635d060bd739666bcf0215171897834dca5 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 28 Oct 2026 13:34:18 +0000 Subject: [PATCH] types, noise: bound the Hostinfo and endpoints a client reports Keep the first 256 endpoints and 4096 routes; refuse longer hostnames, too many tags, zoned endpoints and >64 KiB NetInfo or other Hostinfo. The body limit counted wire bytes, not what NodeStore and peers keep. Fixes ANT-2026-RQS7JAPP Fixes ANT-2026-5B6KE87K Fixes ANT-2026-KFCFFM34 --- CHANGELOG.md | 1 + hscontrol/noise.go | 60 +++++++++ hscontrol/noise_test.go | 228 +++++++++++++++++++++++++++++++++++ hscontrol/types/const.go | 9 ++ hscontrol/types/node.go | 41 +++++++ hscontrol/types/node_test.go | 61 ++++++++++ 6 files changed, 400 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc2aa6971..8482c3e70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`, - Another user's key or login on a tagged machine now creates a new device for that user instead of re-authenticating the tagged node - Fix one node with a stored key expiry outside the years 0 to 9999 in UTC failing the map of every node that can see it; it is now left out of its peers' maps and the startup check names it with the fix command - Refuse a key expiry outside the years 0 to 9999 in UTC on registration, on the OIDC token-expiry path and in `headscale nodes expire` / `POST /api/v1/node/{nodeId}/expire` (400); the latest accepted expiry no longer fails node listings on PostgreSQL in a zone east of UTC +- Keep the first 256 endpoints and 4096 announced routes from client reports; oversized hostnames, requested tags or other metadata return a registration error or HTTP 413 on map updates. Zoned IPv6 endpoints are refused ## 0.29.7 (202x-xx-xx) diff --git a/hscontrol/noise.go b/hscontrol/noise.go index a7f7730b5..cf2d8a62a 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -10,6 +10,7 @@ import ( "net/http" "net/url" "regexp" + "slices" "time" "github.com/go-chi/chi/v5" @@ -18,6 +19,7 @@ import ( "github.com/juanfont/headscale/hscontrol/capver" "github.com/juanfont/headscale/hscontrol/dns" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util/zlog/zf" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "tailscale.com/control/controlbase" @@ -765,6 +767,42 @@ func (ns *noiseServer) PollNetMapHandler( return } + // Keep preferred endpoints first; clients retry rejected reports unchanged. + count := min(len(mapRequest.Endpoints), types.MaxEndpoints) + mapRequest.Endpoints = slices.Clone(mapRequest.Endpoints[:count]) + + mapRequest.EndpointTypes = slices.Clone(mapRequest.EndpointTypes[:min(len(mapRequest.EndpointTypes), types.MaxEndpoints)]) + for _, endpoint := range mapRequest.Endpoints { + if endpoint.Addr().Zone() != "" { + httpError(writer, NewHTTPError(http.StatusBadRequest, "endpoint must not include an IPv6 zone", nil)) + return + } + } + + if hi := mapRequest.Hostinfo; hi != nil { + if len(hi.RoutableIPs) > types.MaxRoutableIPs { + log.Warn().Uint64(zf.NodeID, uint64(nv.ID())).Int(zf.RoutableIPCount, len(hi.RoutableIPs)). + Msgf("client announced too many routes, keeping the first %d", types.MaxRoutableIPs) + hi.RoutableIPs = hi.RoutableIPs[:types.MaxRoutableIPs] + } + + if hi.NetInfo != nil { + // Latency measurements are not used by the control server. + hi.NetInfo.DERPLatency = nil + } + } + + // Reject before the session, the node or its peers keep the report. + err = types.ValidateHostinfo(mapRequest.Hostinfo) + if err != nil { + httpError(writer, NewHTTPError(http.StatusRequestEntityTooLarge, err.Error(), err)) + return + } + + // A repeated JSON key can leave a short slice backed by a larger array. + mapRequest.Hostinfo = mapRequest.Hostinfo.Clone() + mapRequest.DebugFlags = nil + sess := ns.headscale.newMapSession(req.Context(), mapRequest, writer, nv.AsStruct()) sess.log.Trace().Caller().Msg("a node sending a MapRequest with Noise protocol") @@ -906,6 +944,25 @@ func (ns *noiseServer) RegistrationHandler( var registerRequest tailcfg.RegisterRequest decodeErr := json.NewDecoder(req.Body).Decode(®isterRequest) + if decodeErr == nil { + if hi := registerRequest.Hostinfo; hi != nil { + if len(hi.RoutableIPs) > types.MaxRoutableIPs { + log.Warn().Str(zf.MachineKey, ns.machineKey.ShortString()). + Int(zf.RoutableIPCount, len(hi.RoutableIPs)). + Msgf("client announced too many routes, keeping the first %d", types.MaxRoutableIPs) + hi.RoutableIPs = hi.RoutableIPs[:types.MaxRoutableIPs] + } + + if hi.NetInfo != nil { + hi.NetInfo.DERPLatency = nil + } + } + } + + // Reject before a pending registration, a waiting follow-up or the node keeps it. + if decodeErr == nil { + decodeErr = types.ValidateHostinfo(registerRequest.Hostinfo) + } // The floor must be enforced before handleRegister: a logout, pre-auth // key use or auth-cache write it performs is not undone by a later 400. @@ -919,6 +976,9 @@ func (ns *noiseServer) RegistrationHandler( return regErr(decodeErr) } + // A waiting follow-up must not retain the decoder's excess slice capacity. + registerRequest.Hostinfo = registerRequest.Hostinfo.Clone() + resp, err := ns.headscale.handleRegister(req.Context(), registerRequest, ns.machineKey) if err != nil { if httpErr, ok := errors.AsType[HTTPError](err); ok { diff --git a/hscontrol/noise_test.go b/hscontrol/noise_test.go index eaaa8bf89..2ee46e9fe 100644 --- a/hscontrol/noise_test.go +++ b/hscontrol/noise_test.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/netip" "net/url" "slices" "strconv" @@ -1795,3 +1796,230 @@ func TestSetDNSHandlerDisabled(t *testing.T) { }) assert.Equal(t, http.StatusNotImplemented, rec.Code) } + +func manyServices(n int) []tailcfg.Service { + services := make([]tailcfg.Service, n) + for i := range services { + services[i] = tailcfg.Service{Proto: tailcfg.TCP, Port: uint16(i)} + } + + return services +} + +func TestPollNetMapHandler_AcceptsClientReport(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + user := app.state.CreateUserForTest("report-map-user") + node := putTestNodeInStore(t, app, user, "report-map-node") + + endpoints := make([]netip.AddrPort, types.MaxEndpoints+1) + for i := range endpoints { + endpoints[i] = netip.AddrPortFrom(netip.MustParseAddr("192.0.2.1"), uint16(i)) + } + + report := tailcfg.MapRequest{ + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: node.NodeKey, + DiscoKey: node.DiscoKey, + OmitPeers: true, + Endpoints: endpoints, + EndpointTypes: make([]tailcfg.EndpointType, types.MaxEndpoints+1), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "report-map-node", + Services: manyServices(300), + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1, DERPLatency: map[string]float64{"1-v4": 0.01}}, + RoutableIPs: slices.Repeat([]netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, types.MaxRoutableIPs+1), + }, + } + + ns := &noiseServer{headscale: app, machineKey: node.MachineKey} + rec := httptest.NewRecorder() + ns.PollNetMapHandler(rec, newMapRequest(t, report)) + require.Equal(t, http.StatusOK, rec.Code, "body=%q", rec.Body.String()) + + stored, ok := app.state.GetNodeByID(node.ID) + require.True(t, ok) + assert.Equal(t, report.Hostinfo.Hostname, stored.Hostname()) + report.Endpoints = report.Endpoints[:types.MaxEndpoints] + report.EndpointTypes = report.EndpointTypes[:types.MaxEndpoints] + report.Hostinfo.RoutableIPs = report.Hostinfo.RoutableIPs[:types.MaxRoutableIPs] + report.Hostinfo.NetInfo.DERPLatency = nil + assert.Equal(t, report.Endpoints, stored.Endpoints().AsSlice()) + assert.True(t, report.Hostinfo.Equal(stored.Hostinfo().AsStruct()), + "accepted Hostinfo must remain unchanged, including lists longer than 256 entries") + + c, err := app.state.UpdateNodeFromMapRequest(node.ID, report) + require.NoError(t, err) + assert.True(t, c.IsEmpty(), + "an unchanged report must not be a change, got %s", c.Type()) +} + +func TestPollNetMapHandler_RejectsOversizedClientReport(t *testing.T) { + t.Parallel() + + for _, tt := range []struct { + name string + report tailcfg.MapRequest + status int + }{ + { + name: "zoned IPv6 endpoint", + report: tailcfg.MapRequest{Endpoints: []netip.AddrPort{netip.MustParseAddrPort("[fe80::1%eth0]:41641")}}, + status: http.StatusBadRequest, + }, + { + name: "hostname", + report: tailcfg.MapRequest{Hostinfo: &tailcfg.Hostinfo{ + Hostname: strings.Repeat("h", types.MaxHostnameLength+1), + }}, + }, + { + name: "requested tags", + report: tailcfg.MapRequest{Hostinfo: &tailcfg.Hostinfo{ + RequestTags: slices.Repeat([]string{"tag:test"}, types.MaxRequestTags+1), + }}, + }, + { + name: "metadata", + report: tailcfg.MapRequest{Hostinfo: &tailcfg.Hostinfo{ + OSVersion: strings.Repeat("v", types.MaxHostinfoSize), + }}, + }, + { + name: "netinfo", + report: tailcfg.MapRequest{Hostinfo: &tailcfg.Hostinfo{ + NetInfo: &tailcfg.NetInfo{LinkType: strings.Repeat("l", types.MaxHostinfoSize)}, + }}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + user := app.state.CreateUserForTest("rejected-report") + node := putTestNodeInStore(t, app, user, "rejected-report") + before, ok := app.state.GetNodeByID(node.ID) + require.True(t, ok) + + ns := &noiseServer{headscale: app, machineKey: node.MachineKey} + rec := httptest.NewRecorder() + tt.report.Version = tailcfg.CurrentCapabilityVersion + + tt.report.NodeKey = node.NodeKey + if tt.status == 0 { + tt.status = http.StatusRequestEntityTooLarge + } + + ns.PollNetMapHandler(rec, newMapRequest(t, tt.report)) + assert.Equal(t, tt.status, rec.Code, "body=%q", rec.Body.String()) + + after, ok := app.state.GetNodeByID(node.ID) + require.True(t, ok) + assert.Equal(t, before, after, "rejection must not mutate the node") + }) + } +} + +func TestPollNetMapHandler_UnknownNodeBeforeReportValidation(t *testing.T) { + t.Parallel() + app := createTestApp(t) + ns := &noiseServer{headscale: app, machineKey: key.NewMachine().Public()} + rec := httptest.NewRecorder() + ns.PollNetMapHandler(rec, newMapRequest(t, tailcfg.MapRequest{ + Version: tailcfg.CurrentCapabilityVersion, NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: strings.Repeat("h", types.MaxHostnameLength+1)}, + })) + require.Equal(t, http.StatusNotFound, rec.Code) +} + +func TestRegistrationHandler_ValidatesClientReport(t *testing.T) { + t.Parallel() + + t.Run("oversized report", func(t *testing.T) { + t.Parallel() + + // A nil headscale proves rejection happens before any state access. + ns := &noiseServer{machineKey: key.NewMachine().Public()} + + rec := serveRegister(t, ns, tailcfg.RegisterRequest{ + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{ + OSVersion: strings.Repeat("v", types.MaxHostinfoSize), + }, + }) + require.Equal(t, http.StatusOK, rec.Code, "body=%q", rec.Body.String()) + + var resp tailcfg.RegisterResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Contains(t, resp.Error, types.ErrHostinfoTooLarge.Error()) + assert.Empty(t, resp.AuthURL) + }) + + t.Run("pre-auth key trims routes and drops unused latency", func(t *testing.T) { + t.Parallel() + app := createTestApp(t) + user := app.state.CreateUserForTest("trim-pak-user") + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) + require.NoError(t, err) + + nodeKey := key.NewNode().Public() + ns := &noiseServer{headscale: app, machineKey: key.NewMachine().Public()} + rec := serveRegister(t, ns, tailcfg.RegisterRequest{ + Version: tailcfg.CurrentCapabilityVersion, NodeKey: nodeKey, + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "trim-pak-node", + RoutableIPs: slices.Repeat([]netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, types.MaxRoutableIPs+1), + NetInfo: &tailcfg.NetInfo{PreferredDERP: 1, DERPLatency: map[string]float64{"1-v4": 0.01}}, + }, + }) + require.Equal(t, http.StatusOK, rec.Code) + + var resp tailcfg.RegisterResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + require.Empty(t, resp.Error) + + node, ok := app.state.GetNodeByNodeKey(nodeKey) + require.True(t, ok) + + hi := node.Hostinfo().AsStruct() + assert.Len(t, hi.RoutableIPs, types.MaxRoutableIPs) + assert.Equal(t, tailcfg.DERPRegionID(1), hi.NetInfo.PreferredDERP) + assert.Nil(t, hi.NetInfo.DERPLatency) + }) + + t.Run("oversized report leaves pre-auth key unused", func(t *testing.T) { + t.Parallel() + + app := createTestApp(t) + user := app.state.CreateUserForTest("report-pak-user") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, nil) + require.NoError(t, err) + + nodeKey := key.NewNode().Public() + ns := &noiseServer{headscale: app, machineKey: key.NewMachine().Public()} + + rec := serveRegister(t, ns, tailcfg.RegisterRequest{ + Version: tailcfg.CurrentCapabilityVersion, + NodeKey: nodeKey, + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + Hostinfo: &tailcfg.Hostinfo{Hostname: strings.Repeat("p", types.MaxHostnameLength+1)}, + }) + require.Equal(t, http.StatusOK, rec.Code, "body=%q", rec.Body.String()) + + var resp tailcfg.RegisterResponse + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Contains(t, resp.Error, types.ErrHostinfoTooLarge.Error()) + assert.Empty(t, resp.AuthURL) + + _, ok := app.state.GetNodeByNodeKey(nodeKey) + assert.False(t, ok) + + stored, err := app.state.GetPreAuthKey(pak.Key) + require.NoError(t, err) + assert.False(t, stored.Used) + }) +} diff --git a/hscontrol/types/const.go b/hscontrol/types/const.go index 1b75c89d9..baf6cd3f5 100644 --- a/hscontrol/types/const.go +++ b/hscontrol/types/const.go @@ -14,3 +14,12 @@ const ( MaxHostnameLength = 255 ) + +// Client report limits are above what a stock client sends. +// MaxHostinfoSize applies separately to NetInfo and the remaining fields without routes. +const ( + MaxEndpoints = 256 + MaxRequestTags = 256 + MaxRoutableIPs = 4096 + MaxHostinfoSize = 64 << 10 +) diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index fb5834b02..c92166ffd 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -1,6 +1,7 @@ package types import ( + "encoding/json" "errors" "fmt" "maps" @@ -34,6 +35,7 @@ var ( ErrExpiryOutOfRange = errors.New( "key expiry is outside the years 0 to 9999 in UTC", ) + ErrHostinfoTooLarge = errors.New("hostinfo too large") ) // RouteFunc is a function that takes a node ID and returns a list of @@ -654,6 +656,45 @@ func (node *Node) MarshalZerologObject(e *zerolog.Event) { } } +// ValidateHostinfo checks the report before it is retained or sent to peers. +// Routes have a count limit; their encoded size depends on the address family. +// NetInfo has a separate budget because omitted NetInfo preserves the old value. +func ValidateHostinfo(hi *tailcfg.Hostinfo) error { + if hi == nil { + return nil + } + + if len(hi.Hostname) > MaxHostnameLength { + return fmt.Errorf("%w: hostname exceeds %d bytes", ErrHostinfoTooLarge, MaxHostnameLength) + } + + if len(hi.RoutableIPs) > MaxRoutableIPs { + return fmt.Errorf("%w: more than %d routes", ErrHostinfoTooLarge, MaxRoutableIPs) + } + + if len(hi.RequestTags) > MaxRequestTags { + return fmt.Errorf("%w: more than %d requested tags", ErrHostinfoTooLarge, MaxRequestTags) + } + + // Encoding also counts fields added upstream without a local field list. + rest := *hi + rest.RoutableIPs = nil + rest.NetInfo = nil + + for _, part := range []any{&rest, hi.NetInfo} { + encoded, err := json.Marshal(part) + if err != nil { + return fmt.Errorf("encoding hostinfo: %w", err) + } + + if len(encoded) > MaxHostinfoSize { + return fmt.Errorf("%w: %d bytes, limit %d", ErrHostinfoTooLarge, len(encoded), MaxHostinfoSize) + } + } + + return nil +} + // PeerChangeFromMapRequest takes a [tailcfg.MapRequest] and compares it to the node // to produce a [tailcfg.PeerChange] struct that can be used to updated the node and // inform peers about smaller changes to the node. diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index 9a4b537db..3a1a92b21 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -1,8 +1,10 @@ package types import ( + "encoding/json" "fmt" "net/netip" + "slices" "strings" "testing" "time" @@ -1073,3 +1075,62 @@ func TestNodeIsExpiredAt(t *testing.T) { "an invalid view counts as expired, like IsExpired") }) } + +func TestValidateHostinfo(t *testing.T) { + t.Parallel() + + routes := make([]netip.Prefix, MaxRoutableIPs) + for i := range routes { + routes[i] = netip.PrefixFrom(netip.AddrFrom16([16]byte{0xfd, byte(i >> 8), byte(i)}), 64) + } + + latencies := make(map[string]float64, 300) + for i := range 300 { + latencies[fmt.Sprintf("%d-v4", i)] = 0.01 + } + + base, err := json.Marshal(&tailcfg.Hostinfo{OSVersion: "x"}) + require.NoError(t, err) + + exact := strings.Repeat("x", MaxHostinfoSize-len(base)+1) + + for _, tt := range []struct { + name string + hostinfo *tailcfg.Hostinfo + wantErr bool + }{ + {name: "nil"}, + {name: "hostname at limit", hostinfo: &tailcfg.Hostinfo{Hostname: strings.Repeat("h", 255)}}, + {name: "hostname over limit", hostinfo: &tailcfg.Hostinfo{Hostname: strings.Repeat("h", 256)}, wantErr: true}, + {name: "hostname bytes", hostinfo: &tailcfg.Hostinfo{Hostname: strings.Repeat("é", 128)}, wantErr: true}, + {name: "routes at limit", hostinfo: &tailcfg.Hostinfo{RoutableIPs: routes}}, + {name: "routes over limit", hostinfo: &tailcfg.Hostinfo{RoutableIPs: append(slices.Clone(routes), routes[0])}, wantErr: true}, + {name: "tags at limit", hostinfo: &tailcfg.Hostinfo{RequestTags: make([]string, 256)}}, + {name: "tags over limit", hostinfo: &tailcfg.Hostinfo{RequestTags: make([]string, 257)}, wantErr: true}, + {name: "metadata at byte limit", hostinfo: &tailcfg.Hostinfo{OSVersion: exact}}, + {name: "metadata over byte limit", hostinfo: &tailcfg.Hostinfo{OSVersion: exact + "x"}, wantErr: true}, + {name: "NetInfo over byte limit", hostinfo: &tailcfg.Hostinfo{NetInfo: &tailcfg.NetInfo{LinkType: strings.Repeat("n", MaxHostinfoSize)}}, wantErr: true}, + {name: "budgets compose", hostinfo: &tailcfg.Hostinfo{OSVersion: strings.Repeat("o", 60<<10), NetInfo: &tailcfg.NetInfo{LinkType: strings.Repeat("n", 60<<10)}}}, + {name: "small entries need no field-specific truncation", hostinfo: &tailcfg.Hostinfo{ + Services: make([]tailcfg.Service, 300), + WoLMACs: make([]string, 300), + SSH_HostKeys: make([]string, 300), + NetInfo: &tailcfg.NetInfo{DERPLatency: latencies}, + }}, + } { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + want := tt.hostinfo.Clone() + + err := ValidateHostinfo(tt.hostinfo) + if tt.wantErr { + require.ErrorIs(t, err, ErrHostinfoTooLarge) + } else { + require.NoError(t, err) + } + + require.True(t, want.Equal(tt.hostinfo), "validation must not change the report") + }) + } +}