From 8c3b615bda69a32c81596b37f3c418d6e13ae176 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 22 Oct 2026 05:29:57 +0000 Subject: [PATCH] api, policy/v2: refuse route prefixes with host bits Routes are matched as written, so such a prefix never equals the one a client announces. The approval APIs answer 400 and autoApprovers refuses it at load and write, with Tailscale's text. --- hscontrol/api/v1/nodes.go | 5 +++++ hscontrol/api/v2/devices.go | 5 +++++ hscontrol/apiv2_devices_test.go | 17 +---------------- .../policy/v2/tailscale_acl_data_compat_test.go | 4 ---- hscontrol/policy/v2/types.go | 7 ++++++- hscontrol/servertest/policy_test.go | 3 --- hscontrol/util/addr.go | 13 +++++++++++++ 7 files changed, 30 insertions(+), 24 deletions(-) diff --git a/hscontrol/api/v1/nodes.go b/hscontrol/api/v1/nodes.go index b07cdd118..7b4521550 100644 --- a/hscontrol/api/v1/nodes.go +++ b/hscontrol/api/v1/nodes.go @@ -429,6 +429,11 @@ func registerNodeAdminOps(api huma.API, b Backend) { return nil, huma.Error400BadRequest("parsing route", parseErr) } + maskErr := util.CheckRouteMasked(prefix) + if maskErr != nil { + return nil, huma.Error400BadRequest(prefix.String() + " " + maskErr.Error()) + } + // One exit route implies both families, else the client won't // annotate the node as an exit node. if prefix == tsaddr.AllIPv4() || prefix == tsaddr.AllIPv6() { diff --git a/hscontrol/api/v2/devices.go b/hscontrol/api/v2/devices.go index d5afd78b4..ce803401a 100644 --- a/hscontrol/api/v2/devices.go +++ b/hscontrol/api/v2/devices.go @@ -393,6 +393,11 @@ func parseRoutes(routes []string) ([]netip.Prefix, error) { return nil, huma.Error400BadRequest("parsing route", err) } + err = util.CheckRouteMasked(prefix) + if err != nil { + return nil, huma.Error400BadRequest(prefix.String() + " " + err.Error()) + } + if prefix == tsaddr.AllIPv4() || prefix == tsaddr.AllIPv6() { approved = append(approved, tsaddr.AllIPv4(), tsaddr.AllIPv6()) } else { diff --git a/hscontrol/apiv2_devices_test.go b/hscontrol/apiv2_devices_test.go index 8f9867d97..7864fb9fe 100644 --- a/hscontrol/apiv2_devices_test.go +++ b/hscontrol/apiv2_devices_test.go @@ -354,22 +354,7 @@ func containsDeviceID(devs []apiv2.Device, id string) bool { // from SaaS, keyed ///, where is the call's index // in its step. A listed check that matches fails, so a fix must delete its // entry. -var routesAPICompatSkipReasons = map[string]string{ - // The GET after each refused probe reads back what headscale stored. - "routes-t4-admin/probe/0/status": routeHostBitsAccepted, - "routes-t4-admin/probe/1/enabled": routeHostBitsAccepted, - "routes-t4-admin/probe/2/status": routeHostBitsAccepted, - "routes-t4-admin/probe/3/enabled": routeHostBitsAccepted, - "routes-t4-admin/probe/0/v1-status": routeHostBitsAccepted, - "routes-t4-admin/probe/1/v1-enabled": routeHostBitsAccepted, - "routes-t4-admin/probe/2/v1-status": routeHostBitsAccepted, - "routes-t4-admin/probe/3/v1-enabled": routeHostBitsAccepted, - - "routes-t5-oauth-routes-scope/probe/0/status": routeHostBitsAccepted, - "routes-t5-oauth-routes-scope/probe/1/enabled": routeHostBitsAccepted, -} - -const routeHostBitsAccepted = "ROUTE_HOST_BITS_ACCEPTED: SaaS refuses a route with host bits set with 400, headscale approves it as sent" +var routesAPICompatSkipReasons = map[string]string{} // routeCall is one routes API answer, as the compat replays compare it. type routeCall struct { diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index 3af51c138..9e8e6bdc2 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -204,9 +204,6 @@ func loadACLTestFile(t *testing.T, path string) *testcapture.Capture { // aclSkipReasons lists checks where headscale differs from SaaS, keyed // [/]/. var aclSkipReasons = map[string]string{ - "acl-err10/refused": routeHostBits, - "acl-err11/refused": routeHostBits, - "acl-n01/beedrill/filter": allowAllForm, "acl-n01/bulbasaur/filter": allowAllForm, "acl-n01/charmander/filter": allowAllForm, @@ -222,7 +219,6 @@ var aclSkipReasons = map[string]string{ } const ( - routeHostBits = "ROUTE_HOST_BITS: SaaS refuses an autoApprovers route with host bits; headscale accepts it" allowAllForm = "ALLOW_ALL_FORM: SaaS writes the allow-all default's sources as the tailnet ranges, headscale as \"*\"" srcAddressForm = "FILTER_ADDRESS_FORM: SaaS lists contiguous source addresses one by one, headscale as a range" ) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 2b6d29793..fe4bab998 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -2888,7 +2888,12 @@ func (p *Policy) validate() error { } } - for _, approvers := range p.AutoApprovers.Routes { + for prefix, approvers := range p.AutoApprovers.Routes { + err := util.CheckRouteMasked(prefix) + if err != nil { + errs = append(errs, fmt.Errorf("autoApprovers.routes[%q]: %w", prefix.String(), err)) + } + for _, approver := range approvers { switch approver := approver.(type) { case *Group: diff --git a/hscontrol/servertest/policy_test.go b/hscontrol/servertest/policy_test.go index 70bcb9444..4baa4dd56 100644 --- a/hscontrol/servertest/policy_test.go +++ b/hscontrol/servertest/policy_test.go @@ -316,9 +316,6 @@ var rejectedPolicySkipReasons = map[string]string{ "ssh-e1/refused": noTailnetDomains, "ssh-e2/refused": noTailnetDomains, "ssh-malformed-user-localpart-multi-glob/refused": noTailnetDomains, - - "acl-err10/refused": "ROUTE_HOST_BITS: headscale accepts an autoApprovers route with host bits", - "acl-err11/refused": "ROUTE_HOST_BITS: headscale accepts an autoApprovers route with host bits", } const noTailnetDomains = "NO_TAILNET_DOMAINS: headscale does not check user:*@domain or localpart:*@domain against the tailnet's domains" diff --git a/hscontrol/util/addr.go b/hscontrol/util/addr.go index 456e569cf..2b9a1961e 100644 --- a/hscontrol/util/addr.go +++ b/hscontrol/util/addr.go @@ -146,6 +146,19 @@ func StringToIPPrefix(prefixes []string) ([]netip.Prefix, error) { return result, nil } +var ErrRouteHostBits = errors.New("has non-address bits set") + +// CheckRouteMasked refuses a route with bits set past its length: routes are +// matched as written, so it would never equal the masked prefix a client +// announces. +func CheckRouteMasked(p netip.Prefix) error { + if masked := p.Masked(); p != masked { + return fmt.Errorf("%w; expected %s", ErrRouteHostBits, masked) + } + + return nil +} + // IPSetAddrIter returns a function that iterates over all the IPs in the [netipx.IPSet]. func IPSetAddrIter(ipSet *netipx.IPSet) iter.Seq[netip.Addr] { return func(yield func(netip.Addr) bool) {