diff --git a/hscontrol/noise.go b/hscontrol/noise.go index 8961f2976..ad01f31e3 100644 --- a/hscontrol/noise.go +++ b/hscontrol/noise.go @@ -9,6 +9,7 @@ import ( "io" "net/http" "net/url" + "slices" "time" "github.com/go-chi/chi/v5" @@ -16,6 +17,7 @@ import ( "github.com/go-chi/metrics" "github.com/juanfont/headscale/hscontrol/capver" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util/zlog/zf" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "golang.org/x/net/http2" @@ -786,6 +788,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") @@ -821,7 +859,29 @@ func (ns *noiseServer) RegistrationHandler( return ®Req, regErr(err) } - resp, err = ns.headscale.handleRegister(req.Context(), regReq, ns.conn.Peer()) + if hi := regReq.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. + err = types.ValidateHostinfo(regReq.Hostinfo) + if err != nil { + return ®Req, regErr(err) + } + + // A waiting follow-up must not retain the decoder's excess slice capacity. + regReq.Hostinfo = regReq.Hostinfo.Clone() + + resp, err = ns.headscale.handleRegister(req.Context(), regReq, ns.machineKey) if err != nil { if httpErr, ok := errors.AsType[HTTPError](err); ok { resp = &tailcfg.RegisterResponse{ diff --git a/hscontrol/noise_test.go b/hscontrol/noise_test.go index 3cba9bc2f..a60de33bc 100644 --- a/hscontrol/noise_test.go +++ b/hscontrol/noise_test.go @@ -10,6 +10,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/netip" "net/url" "slices" "strconv" @@ -1262,3 +1263,248 @@ func TestPollNetMapHandler_ForeignMachineKeyStillRejected(t *testing.T) { assert.Equal(t, http.StatusNotFound, rec.Code, "body=%q", rec.Body.String()) } + +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) +} + +// serveRegister guards against panics so a handler that reaches a nil +// dependency fails its own row instead of the whole test binary. +func serveRegister(t *testing.T, ns *noiseServer, body any) *httptest.ResponseRecorder { + t.Helper() + + payload, err := json.Marshal(body) + require.NoError(t, err) + + req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "/machine/register", bytes.NewReader(payload)) + rec := httptest.NewRecorder() + + require.NotPanics(t, func() { + ns.RegistrationHandler(rec, req) + }) + + return rec +} + +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 019c14b62..482df66c2 100644 --- a/hscontrol/types/const.go +++ b/hscontrol/types/const.go @@ -14,3 +14,12 @@ const ( KeepAliveInterval = 60 * time.Second 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 ac4fb849e..0b7e73fc8 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 @@ -715,6 +717,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 9648ec31a..8c26f1264 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" @@ -1125,3 +1127,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") + }) + } +}