diff --git a/hscontrol/servertest/routes_test.go b/hscontrol/servertest/routes_test.go index 73278c8b9..5c2bdacdb 100644 --- a/hscontrol/servertest/routes_test.go +++ b/hscontrol/servertest/routes_test.go @@ -371,32 +371,6 @@ var routeOverlapCompatTests = []string{ // keyed //. A listed check that matches fails, so a fix // must delete its entry. var routeOverlapSkipReasons = map[string]string{ - "routes-t1-auto-v4-default/ovl-db/peer-routes": tailnetSingleServed, - "routes-t1-auto-v4-default/ovl-observer/peer-routes": tailnetSingleServed, - "routes-t1-auto-v4-default/ovl-r-wide-in/peer-routes": tailnetSingleServed, - "routes-t1-auto-v4-default/ovl-r-wide-out/peer-routes": tailnetSingleServed, - "routes-t1-auto-v4-default/ovl-viewer/peer-routes": tailnetSingleServed, - "routes-t2-auto-v6-default/ovl-db/peer-routes": tailnetSingleServed, - "routes-t2-auto-v6-default/ovl-observer/peer-routes": tailnetSingleServed, - "routes-t2-auto-v6-default/ovl-r-wide-in/peer-routes": tailnetSingleServed, - "routes-t2-auto-v6-default/ovl-r-wide-out/peer-routes": tailnetSingleServed, - "routes-t2-auto-v6-default/ovl-viewer/peer-routes": tailnetSingleServed, - "routes-t3-auto-narrow-keys/ovl-db/peer-routes": tailnetSingleServed, - "routes-t3-auto-narrow-keys/ovl-observer/peer-routes": tailnetSingleServed, - "routes-t3-auto-narrow-keys/ovl-r-wide-in/peer-routes": tailnetSingleServed, - "routes-t3-auto-narrow-keys/ovl-r-wide-out/peer-routes": tailnetSingleServed, - "routes-t3-auto-narrow-keys/ovl-viewer/peer-routes": tailnetSingleServed, - "routes-t4-admin/ovl-db/peer-routes": tailnetSingleServed, - "routes-t4-admin/ovl-observer/peer-routes": tailnetSingleServed, - "routes-t4-admin/ovl-r-wide-in/peer-routes": tailnetSingleServed, - "routes-t4-admin/ovl-r-wide-out/peer-routes": tailnetSingleServed, - "routes-t4-admin/ovl-viewer/peer-routes": tailnetSingleServed, - "routes-t5-oauth-routes-scope/ovl-db/peer-routes": tailnetSingleServed, - "routes-t5-oauth-routes-scope/ovl-observer/peer-routes": tailnetSingleServed, - "routes-t5-oauth-routes-scope/ovl-r-wide-in/peer-routes": tailnetSingleServed, - "routes-t5-oauth-routes-scope/ovl-r-wide-out/peer-routes": tailnetSingleServed, - "routes-t5-oauth-routes-scope/ovl-viewer/peer-routes": tailnetSingleServed, - // Once a router is sent its own routes, its self-primary also needs the // approved singles in the tailnet range left unserved. "routes-t1-auto-v4-default/ovl-r-ctrl/self-allowed": selfRoutesReduced, @@ -489,8 +463,6 @@ var routeOverlapNotCompared = []string{ "routes-t5-oauth-routes-scope/ovl-r-via/peers", } -const tailnetSingleServed = "TAILNET_SINGLE_SERVED: headscale serves an approved single address inside the tailnet range, which SaaS approves but never serves" - // TestRoutesTailnetOverlapCompat replays routers whose routes overlap the // tailnet's addresses and compares approvals, served routes, peers and // filters with SaaS. Headscale must hand out SaaS's addresses, so a route @@ -665,3 +637,90 @@ func prefixList(pfxs views.Slice[netip.Prefix]) string { func sortedStrings(s []string) string { return fmt.Sprint(slices.Sorted(slices.Values(s))) } + +// TestRoutesNodeRangeSingleNotServed checks that a router's approved single +// addresses inside the node ranges, in either spelling, reach no viewer +// through a plain grant or a via grant. They stay approved, the router's own +// entry keeps them, and its wider route is still served. +func TestRoutesNodeRangeSingleNotServed(t *testing.T) { + t.Parallel() + + subnet := netip.MustParsePrefix("10.9.0.0/24") + singles := []netip.Prefix{ + netip.MustParsePrefix("100.64.0.50/32"), + netip.MustParsePrefix("fd7a:115c:a1e0::50/128"), + netip.MustParsePrefix("::ffff:100.64.0.50/128"), + } + routes := append([]netip.Prefix{subnet}, singles...) + + for name, grant := range map[string]string{ + "plain": `{"src": ["tag:client"], "dst": ["*"], "ip": ["*"]}`, + "via": `{"src": ["tag:client"], "ip": ["*"], "via": ["tag:router"], + "dst": ["10.9.0.0/24", "100.64.0.0/10", "fd7a:115c:a1e0::/48", "::ffff:0:0/96"]}`, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + + srv := servertest.NewServer(t) + user := srv.CreateUser(t, "routes-user") + + changed, err := srv.State().SetPolicy([]byte(`{ + "tagOwners": {"tag:client": ["routes-user@"], "tag:router": ["routes-user@"]}, + "grants": [ + {"src": ["tag:client"], "dst": ["tag:router"], "ip": ["*"]}, + {"src": ["tag:router"], "dst": ["*"], "ip": ["*"]}, + ` + grant + ` + ] + }`)) + require.NoError(t, err) + + if changed { + changes, err := srv.State().ReloadPolicy() + require.NoError(t, err) + srv.App.Change(changes...) + } + + router := servertest.NewClient(t, srv, "router", + servertest.WithUser(user), servertest.WithTags("tag:router"), + servertest.WithHostinfo(&tailcfg.Hostinfo{ + BackendLogID: "servertest-router", Hostname: "router", RoutableIPs: routes, + })) + viewer := servertest.NewClient(t, srv, "viewer", + servertest.WithUser(user), servertest.WithTags("tag:client")) + + viewer.WaitForPeers(t, 1, 15*time.Second) + + routerID := findNodeID(t, srv, "router") + _, routeChange, err := srv.State().SetApprovedRoutes(routerID, routes) + require.NoError(t, err) + srv.App.Change(routeChange) + + viewer.WaitForCondition(t, "viewer is served the subnet", 15*time.Second, + func(nm *netmap.NetworkMap) bool { + p, ok := viewer.PeerByName("router") + + return ok && views.SliceContains(p.PrimaryRoutes(), subnet) + }) + router.WaitForCondition(t, "router holds its routes", 15*time.Second, + func(nm *netmap.NetworkMap) bool { + return views.SliceContains(nm.SelfNode.AllowedIPs(), subnet) + }) + + peer, _ := viewer.PeerByName("router") + self := router.Netmap().SelfNode + + for _, single := range singles { + assert.Falsef(t, views.SliceContains(peer.AllowedIPs(), single), + "%s served in AllowedIPs", single) + assert.Falsef(t, views.SliceContains(peer.PrimaryRoutes(), single), + "%s served in PrimaryRoutes", single) + assert.Truef(t, views.SliceContains(self.AllowedIPs(), single), + "%s missing from the router's own AllowedIPs", single) + } + + node, ok := srv.State().GetNodeByID(routerID) + require.True(t, ok) + assert.ElementsMatch(t, routes, node.ApprovedRoutes().AsSlice()) + }) + } +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 1fd1b3bbe..cf7d630cb 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -1359,7 +1359,11 @@ func (s *State) RoutesForPeer( } } - return reduced + // Via inclusions come from the peer's routes, not the election, so + // the serving rule is applied here, to everything the viewer gets. + return slices.DeleteFunc(reduced, func(p netip.Prefix) bool { + return !s.cfg.ServesRoute(p) + }) } // PrimaryRoutesString renders the current prefix→primary assignment diff --git a/hscontrol/types/config.go b/hscontrol/types/config.go index e69b6a479..a36ef1575 100644 --- a/hscontrol/types/config.go +++ b/hscontrol/types/config.go @@ -399,6 +399,21 @@ func (c *Config) Domain() string { return u.Hostname() } +// ServesRoute reports whether an approved route is handed to clients. A +// single address inside the node ranges, in either spelling, names a node +// rather than a subnet, so it stays approved but is never served. 4via6 +// addresses stand for a site's IPv4 subnet, not a node. +func (c *Config) ServesRoute(p netip.Prefix) bool { + if !p.IsSingleIP() || tsaddr.IsViaPrefix(p) { + return true + } + + addr := p.Addr().Unmap() + + return (c.PrefixV4 == nil || !c.PrefixV4.Contains(addr)) && + (c.PrefixV6 == nil || !c.PrefixV6.Contains(addr)) +} + // LoadConfig prepares and loads the Headscale configuration into Viper. // This means it sets the default values, reads the configuration file and // environment variables, and handles deprecated configuration options. diff --git a/hscontrol/types/config_test.go b/hscontrol/types/config_test.go index a9374732f..c0476bc00 100644 --- a/hscontrol/types/config_test.go +++ b/hscontrol/types/config_test.go @@ -729,3 +729,33 @@ func TestExtraRecordsAreLowercased(t *testing.T) { t.Errorf("SetExtraRecords mismatch (-want +got):\n%s", diff) } } + +func TestServesRoute(t *testing.T) { + v4 := netip.MustParsePrefix("100.64.0.0/10") + v6 := netip.MustParsePrefix("fd7a:115c:a1e0::/48") + cfg := &Config{PrefixV4: &v4, PrefixV6: &v6} + + for route, want := range map[string]bool{ + "100.64.0.11/32": false, + "fd7a:115c:a1e0::b/128": false, + "::ffff:100.64.0.11/128": false, + "100.64.1.2/31": true, + "100.64.2.0/24": true, + "fd7a:115c:a1e0::1:2/127": true, + "fd7a:115c:a1e0::2:0/120": true, + "::ffff:100.64.0.0/112": true, + "100.64.0.8/29": true, + "0.0.0.0/1": true, + "0.0.0.0/0": true, + "10.9.0.1/32": true, + "::ffff:10.9.0.1/128": true, + "fd00::1/128": true, + "fd7a:115c:a1e0:b1a:0:7:a09:5/128": true, + } { + assert.Equalf(t, want, cfg.ServesRoute(netip.MustParsePrefix(route)), "%s", route) + } + + // Only the configured ranges hold nodes. + narrow := netip.MustParsePrefix("100.64.0.0/24") + assert.True(t, (&Config{PrefixV4: &narrow}).ServesRoute(netip.MustParsePrefix("100.100.0.1/32"))) +}