From 6d03e39830acd369d2984584b59e3fb64ec963c3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 10:05:23 +0000 Subject: [PATCH] types, state: reduce a node's routes against the matchers once Reuse matcher membership and approved subnets across the route list. Preserve route order and HA co-router visibility. --- hscontrol/policy/policy.go | 10 +-- hscontrol/policy/policy_test.go | 73 +++++++++++++++++++ hscontrol/state/state.go | 15 ++-- hscontrol/types/node.go | 74 ++++++++++++------- hscontrol/types/node_test.go | 122 ++++++++++++++++++++++++++++++++ 5 files changed, 255 insertions(+), 39 deletions(-) diff --git a/hscontrol/policy/policy.go b/hscontrol/policy/policy.go index 260b0e469..55730cd18 100644 --- a/hscontrol/policy/policy.go +++ b/hscontrol/policy/policy.go @@ -16,15 +16,7 @@ func ReduceRoutes( routes []netip.Prefix, matchers []matcher.Match, ) []netip.Prefix { - var result []netip.Prefix - - for _, route := range routes { - if node.CanAccessRoute(matchers, route) { - result = append(result, route) - } - } - - return result + return node.CanAccessRoutes(matchers, routes) } // ApproveRoutesWithPolicy checks if the node can approve the announced routes diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go index 22d2cf7fd..a91513ef6 100644 --- a/hscontrol/policy/policy_test.go +++ b/hscontrol/policy/policy_test.go @@ -1619,3 +1619,76 @@ func TestReduceRoutes(t *testing.T) { }) } } + +func TestReduceRoutesDerivesNodeRoutesOnce(t *testing.T) { + routes := make([]netip.Prefix, 16) + + ip := netip.MustParseAddr("10.0.0.0") + for i := range routes { + routes[i] = netip.PrefixFrom(ip, 32) + ip = ip.Next() + } + + n := &types.Node{ + IPv4: ap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{RoutableIPs: routes}, + ApprovedRoutes: routes, + } + matchers := matcher.MatchesFromFilterRules([]tailcfg.FilterRule{{ + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{{ + IP: "192.0.2.0/24", Ports: tailcfg.PortRangeAny, + }}, + }}) + + var reduced []netip.Prefix + + single := testing.AllocsPerRun(3, func() { + reduced = ReduceRoutes(n.View(), routes[:1], matchers) + }) + + require.Nil(t, reduced) + + bulk := testing.AllocsPerRun(3, func() { + reduced = ReduceRoutes(n.View(), routes, matchers) + }) + + require.Nil(t, reduced) + require.LessOrEqual(t, bulk, single+1, + "route derivation must not repeat for each candidate", + ) +} + +func BenchmarkReduceRoutesOwnPrimaries(b *testing.B) { + routes := make([]netip.Prefix, 1000) + + ip := netip.MustParseAddr("10.0.0.0") + for i := range routes { + routes[i] = netip.PrefixFrom(ip, 32) + ip = ip.Next() + } + + n := &types.Node{ + IPv4: ap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{RoutableIPs: routes}, + ApprovedRoutes: routes, + } + matchers := matcher.MatchesFromFilterRules([]tailcfg.FilterRule{{ + SrcIPs: []string{"*"}, + DstPorts: []tailcfg.NetPortRange{{ + IP: "192.0.2.0/24", Ports: tailcfg.PortRangeAny, + }}, + }}) + + var reduced []netip.Prefix + + b.ReportAllocs() + + for b.Loop() { + reduced = ReduceRoutes(n.View(), routes, matchers) + } + + if len(reduced) != 0 { + b.Fatal("unmatched primaries must not be returned") + } +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 0ba2bc9cd..360b2aadb 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -43,6 +43,7 @@ import ( "tailscale.com/types/key" "tailscale.com/types/views" "tailscale.com/util/dnsname" + "tailscale.com/util/set" ) const ( @@ -1557,11 +1558,15 @@ func (s *State) RoutesForPeer( // that the peer is HA primary for, the viewer must see that route // regardless of matcher authorization. HA secondaries need this to // know which peer is primary for their shared prefix. - viewerSubnets := viewer.SubnetRoutes() - if len(viewerSubnets) > 0 { - for _, p := range globalPrimaries { - if slices.Contains(viewerSubnets, p) && !slices.Contains(reduced, p) { - reduced = append(reduced, p) + if len(globalPrimaries) > 0 { + viewerSubnets := set.SetOf(viewer.SubnetRoutes()) + if len(viewerSubnets) > 0 { + seen := set.SetOf(reduced) + for _, p := range globalPrimaries { + if viewerSubnets.Contains(p) && !seen.Contains(p) { + reduced = append(reduced, p) + seen.Add(p) + } } } } diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index f1843db9f..4d8c5be95 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -435,8 +435,13 @@ func (node *Node) canAccess( return false } -// CanAccessRoute determines whether a specific route prefix should be -// visible to this node based on the given matchers. +// CanAccessRoute reports whether [Node.CanAccessRoutes] keeps route. +func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) bool { + return len(node.CanAccessRoutes(matchers, []netip.Prefix{route})) > 0 +} + +// CanAccessRoutes returns the authorized routes in input order, evaluating the +// node's matcher membership once for the whole list. // // Unlike [Node.CanAccess], this function intentionally does NOT check // [matcher.Match.DestsIsTheInternet]. Exit routes (0.0.0.0/0, ::/0) are handled by @@ -451,40 +456,47 @@ func (node *Node) canAccess( // internet" from internet-targeted ACLs. Wildcard "*" dests produce // matchers where [matcher.Match.DestsOverlapsPrefixes](0.0.0.0/0) already returns // true, so the check would be redundant for that case. -func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) bool { - src := node.IPs() +func (node *Node) CanAccessRoutes( + matchers []matcher.Match, + routes []netip.Prefix, +) []netip.Prefix { + if len(routes) == 0 || len(matchers) == 0 { + return nil + } + + ips := node.IPs() subnetRoutes := node.SubnetRoutes() + sides := make([]struct{ isSrc, isDst bool }, len(matchers)) - for _, matcher := range matchers { - if matcher.SrcsContainsIPs(src...) && matcher.DestsOverlapsPrefixes(route) { - return true - } - - if matcher.SrcsOverlapsPrefixes(route) && matcher.DestsContainsIP(src...) { - return true - } - + for i, m := range matchers { // A subnet router acts on behalf of its advertised subnets. // If the node's approved subnet routes overlap the source set // and the route overlaps the destination set, the router needs // this route to forward traffic from its local subnet. - if len(subnetRoutes) > 0 { - if matcher.SrcsOverlapsPrefixes(subnetRoutes...) && - matcher.DestsOverlapsPrefixes(route) { - return true - } + sides[i].isSrc = m.SrcsContainsIPs(ips...) || + m.SrcsOverlapsPrefixes(subnetRoutes...) - // Reverse: traffic from the route's subnet is destined for - // this node's subnets; the router needs the route for return - // traffic. - if matcher.SrcsOverlapsPrefixes(route) && - matcher.DestsOverlapsPrefixes(subnetRoutes...) { - return true + // Reverse: traffic from the route's subnet is destined for + // this node's subnets; the router needs the route for return + // traffic. + sides[i].isDst = m.DestsContainsIP(ips...) || + m.DestsOverlapsPrefixes(subnetRoutes...) + } + + var result []netip.Prefix + + for _, route := range routes { + for i, m := range matchers { + if (sides[i].isSrc && m.DestsOverlapsPrefixes(route)) || + (sides[i].isDst && m.SrcsOverlapsPrefixes(route)) { + result = append(result, route) + + break } } } - return false + return result } func (nodes Nodes) FilterByIP(ip netip.Addr) Nodes { @@ -910,6 +922,18 @@ func (nv NodeView) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) return nv.ж.CanAccessRoute(matchers, route) } +// CanAccessRoutes is [Node.CanAccessRoutes] for a node view. +func (nv NodeView) CanAccessRoutes( + matchers []matcher.Match, + routes []netip.Prefix, +) []netip.Prefix { + if !nv.Valid() { + return nil + } + + return nv.ж.CanAccessRoutes(matchers, routes) +} + func (nv NodeView) AnnouncedRoutes() []netip.Prefix { if !nv.Valid() { return nil diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index c8cd3acfb..0ebbf0895 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -13,6 +13,7 @@ import ( "github.com/juanfont/headscale/hscontrol/policy/matcher" "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/require" + "pgregory.net/rapid" "tailscale.com/tailcfg" "tailscale.com/types/key" ) @@ -1222,3 +1223,124 @@ func TestIsExitNodeSubnetRouterAllocs(t *testing.T) { require.False(t, exit) require.Zero(t, allocs) } + +func TestCanAccessRoutesMatchesReference(t *testing.T) { + prefixStrings := []string{ + "100.64.0.1/32", "100.64.0.2/32", "100.64.0.0/24", + "10.0.0.0/24", "10.0.1.0/24", "10.0.0.0/8", "192.168.0.0/16", + "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128", "fd7a:115c:a1e0::/64", + "2001:db8::/32", "2001:db8:1::/64", + "fd7a:115c:a1e0:b1a:0:7:a00:0/120", + "0.0.0.0/0", "::/0", "0.0.0.0/1", "::/1", + } + prefixes := util.MustStringsToPrefixes(prefixStrings) + routePool := append(slices.Clone(prefixes), netip.Prefix{}) + matchPool := append([]string{"*"}, prefixStrings...) + prefixGen := rapid.SampledFrom(prefixes) + matchGen := rapid.SampledFrom(matchPool) + + rapid.Check(t, func(rt *rapid.T) { + n := Node{ + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: rapid.SliceOfN(prefixGen, 0, 8).Draw(rt, "announced"), + }, + ApprovedRoutes: rapid.SliceOfN(prefixGen, 0, 8).Draw(rt, "approved"), + } + if rapid.Bool().Draw(rt, "hasIPv4") { + ip := rapid.SampledFrom([]string{ + "100.64.0.1", "100.64.0.2", "10.0.0.1", "192.168.1.1", + }).Draw(rt, "ipv4") + n.IPv4 = new(netip.MustParseAddr(ip)) + } + + if rapid.Bool().Draw(rt, "hasIPv6") { + ip := rapid.SampledFrom([]string{ + "fd7a:115c:a1e0::1", "fd7a:115c:a1e0::2", "2001:db8::1", + }).Draw(rt, "ipv6") + n.IPv6 = new(netip.MustParseAddr(ip)) + } + + count := rapid.IntRange(0, 5).Draw(rt, "matchers") + matchers := make([]matcher.Match, 0, count) + + for i := range count { + sources := rapid.SliceOfN(matchGen, 0, 4).Draw( + rt, fmt.Sprintf("sources-%d", i), + ) + destinations := rapid.SliceOfN(matchGen, 0, 4).Draw( + rt, fmt.Sprintf("destinations-%d", i), + ) + + rule := tailcfg.FilterRule{SrcIPs: sources} + for _, dst := range destinations { + rule.DstPorts = append(rule.DstPorts, tailcfg.NetPortRange{ + IP: dst, Ports: tailcfg.PortRangeAny, + }) + } + + matchers = append(matchers, matcher.MatchFromFilterRule(rule)) + } + + routes := rapid.SliceOfN(rapid.SampledFrom(routePool), 0, 16).Draw(rt, "routes") + + var want []netip.Prefix + + for _, route := range routes { + allowed := canAccessRouteReference(&n, matchers, route) + require.Equal(rt, allowed, n.CanAccessRoute(matchers, route)) + + if allowed { + want = append(want, route) + } + } + + require.Equal(rt, want, n.CanAccessRoutes(matchers, routes)) + require.Equal(rt, want, n.View().CanAccessRoutes(matchers, routes)) + }) + + var invalid NodeView + + require.Nil(t, invalid.CanAccessRoutes( + matcher.MatchesFromFilterRules(tailcfg.FilterAllowAll), prefixes, + )) +} + +func canAccessRouteReference( + node *Node, + matchers []matcher.Match, + route netip.Prefix, +) bool { + src := node.IPs() + subnetRoutes := node.SubnetRoutes() + + for _, matcher := range matchers { + if matcher.SrcsContainsIPs(src...) && matcher.DestsOverlapsPrefixes(route) { + return true + } + + if matcher.SrcsOverlapsPrefixes(route) && matcher.DestsContainsIP(src...) { + return true + } + + // A subnet router acts on behalf of its advertised subnets. + // If the node's approved subnet routes overlap the source set + // and the route overlaps the destination set, the router needs + // this route to forward traffic from its local subnet. + if len(subnetRoutes) > 0 { + if matcher.SrcsOverlapsPrefixes(subnetRoutes...) && + matcher.DestsOverlapsPrefixes(route) { + return true + } + + // Reverse: traffic from the route's subnet is destined for + // this node's subnets; the router needs the route for return + // traffic. + if matcher.SrcsOverlapsPrefixes(route) && + matcher.DestsOverlapsPrefixes(subnetRoutes...) { + return true + } + } + } + + return false +}