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 22df631fe..d503d7436 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" "tailscale.com/util/truncate" ) @@ -1671,11 +1672,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 3e716c9e0..dd357eaa8 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -23,6 +23,7 @@ import ( "tailscale.com/types/key" "tailscale.com/types/views" "tailscale.com/util/dnsname" + "tailscale.com/util/set" ) var ( @@ -346,15 +347,7 @@ func (node *Node) Prefixes() []netip.Prefix { // viewer uses an exit node; excluded from [Node.CanAccessRoute] which only // handles non-exit routing. func (node *Node) ExitRoutes() []netip.Prefix { - var routes []netip.Prefix - - for _, route := range node.AnnouncedRoutes() { - if tsaddr.IsExitRoute(route) && slices.Contains(node.ApprovedRoutes, route) { - routes = append(routes, route) - } - } - - return routes + return node.approvedRoutes(true) } // IsExitNode reports whether the node has any approved exit routes. @@ -447,8 +440,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 @@ -463,40 +461,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 { @@ -603,15 +608,34 @@ func (node *Node) AnnouncedRoutes() []netip.Prefix { // responses that need the routes actively served by the node must // populate SubnetRoutes from the primary-route election instead. func (node *Node) SubnetRoutes() []netip.Prefix { + return node.approvedRoutes(false) +} + +func (node *Node) approvedRoutes(exit bool) []netip.Prefix { + // Only the requested kind goes in the set, so finding the exit routes of + // a large subnet router stays cheap. + var approved set.Set[netip.Prefix] + + for _, route := range node.ApprovedRoutes { + if tsaddr.IsExitRoute(route) == exit { + if approved == nil { + approved = make(set.Set[netip.Prefix]) + } + + approved.Add(route) + } + } + + if len(approved) == 0 { + return nil + } + var routes []netip.Prefix for _, route := range node.AnnouncedRoutes() { - if tsaddr.IsExitRoute(route) { - continue - } - - if slices.Contains(node.ApprovedRoutes, route) { + if approved.Contains(route) { routes = append(routes, route) + approved.Delete(route) } } @@ -961,6 +985,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_benchmark_test.go b/hscontrol/types/node_benchmark_test.go index e73203797..0b97075bc 100644 --- a/hscontrol/types/node_benchmark_test.go +++ b/hscontrol/types/node_benchmark_test.go @@ -3,6 +3,7 @@ package types import ( "fmt" "net/netip" + "slices" "testing" "github.com/juanfont/headscale/hscontrol/policy/matcher" @@ -71,3 +72,60 @@ func BenchmarkNodeViewCanAccess(b *testing.B) { } }) } + +func BenchmarkNodeDerivedRoutes(b *testing.B) { + for _, count := range []int{1024, 8192} { + announced := make([]netip.Prefix, count, count+2) + + ip := netip.MustParseAddr("10.0.0.0") + for i := range announced { + announced[i] = netip.PrefixFrom(ip, 32) + ip = ip.Next() + } + + announced = append(announced, + netip.MustParsePrefix("0.0.0.0/0"), netip.MustParsePrefix("::/0")) + + for _, approved := range []bool{false, true} { + name := fmt.Sprintf("approved=%t/routes=%d", approved, count) + b.Run(name, func(b *testing.B) { + n := Node{Hostinfo: &tailcfg.Hostinfo{RoutableIPs: announced}} + if approved { + n.ApprovedRoutes = slices.Clone(announced) + } + + var subnets, exits []netip.Prefix + + b.ReportAllocs() + + for b.Loop() { + subnets = n.SubnetRoutes() + exits = n.ExitRoutes() + } + + if approved { + if len(subnets) != count || len(exits) != 2 { + b.Fatalf("unexpected derived routes: %d subnets, %d exits", + len(subnets), len(exits)) + } + } else if len(subnets) != 0 || len(exits) != 0 { + b.Fatal("unapproved routes must not be served") + } + }) + } + } +} + +func BenchmarkNodeIsExitNodeSubnetRouter(b *testing.B) { + for _, count := range []int{4096, 16384} { + b.Run(fmt.Sprintf("routes=%d", count), func(b *testing.B) { + n := subnetRouter(count) + + b.ReportAllocs() + + for b.Loop() { + n.IsExitNode() + } + }) + } +} diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index 3a561dd6f..d8d2f5c5f 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -14,6 +14,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" ) @@ -1124,6 +1125,227 @@ func TestHasPolicyChangeSharedRouteInputs(t *testing.T) { require.Zero(t, allocs, "unchanged route inputs should not derive route lists") } +func TestNodeDerivedRoutes(t *testing.T) { + a := netip.MustParsePrefix("10.0.0.0/24") + b := netip.MustParsePrefix("10.1.0.0/24") + x := netip.MustParsePrefix("10.2.0.0/24") + exit4 := netip.MustParsePrefix("0.0.0.0/0") + exit6 := netip.MustParsePrefix("::/0") + + for _, tt := range []struct { + name string + announced, approved, subnets, exits []netip.Prefix + }{ + { + name: "subnets follow announcement order without duplicates", + announced: []netip.Prefix{b, a, x, b, a, b}, + approved: []netip.Prefix{a, b, b}, + subnets: []netip.Prefix{b, a}, + }, + { + name: "exit routes follow announcement order without duplicates", + announced: []netip.Prefix{exit6, exit4, exit6, exit4, a}, + approved: []netip.Prefix{exit4, exit6, exit4}, + exits: []netip.Prefix{exit6, exit4}, + }, + { + name: "mixed routes", + announced: []netip.Prefix{b, exit6, x, exit6, a, b, exit4}, + approved: []netip.Prefix{exit4, a, exit6, b}, + subnets: []netip.Prefix{b, a}, + exits: []netip.Prefix{exit6, exit4}, + }, + {name: "nothing approved", announced: []netip.Prefix{b, exit6, a, exit4}}, + { + name: "empty approvals", + announced: []netip.Prefix{a, exit4}, + approved: []netip.Prefix{}, + }, + {name: "nothing announced", approved: []netip.Prefix{a, exit4}}, + { + name: "unannounced approvals", + announced: []netip.Prefix{a}, + approved: []netip.Prefix{b, exit4}, + }, + { + name: "exit families differ", + announced: []netip.Prefix{exit4}, + approved: []netip.Prefix{exit6}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + announced := slices.Clone(tt.announced) + approved := slices.Clone(tt.approved) + n := Node{ + Hostinfo: &tailcfg.Hostinfo{RoutableIPs: tt.announced}, + ApprovedRoutes: tt.approved, + } + + require.Equal(t, tt.subnets, n.SubnetRoutes()) + require.Equal(t, tt.exits, n.ExitRoutes()) + require.Equal(t, announced, n.AnnouncedRoutes(), + "announcements must remain immutable") + require.Equal(t, Prefixes(approved), n.ApprovedRoutes, + "approvals must remain immutable") + }) + } + + n := Node{ApprovedRoutes: []netip.Prefix{a, exit4}} + require.Nil(t, n.SubnetRoutes()) + require.Nil(t, n.ExitRoutes()) +} + +func subnetRouter(count int) Node { + routes := make([]netip.Prefix, count) + + ip := netip.MustParseAddr("10.0.0.0") + for i := range routes { + routes[i] = netip.PrefixFrom(ip, 32) + ip = ip.Next() + } + + return Node{ + Hostinfo: &tailcfg.Hostinfo{RoutableIPs: routes}, + ApprovedRoutes: routes, + } +} + +// IsExitNode runs per viewer and peer pair; a subnet router must answer it +// without collecting its subnet routes. +func TestIsExitNodeSubnetRouterAllocs(t *testing.T) { + n := subnetRouter(4096) + + var exit bool + + allocs := testing.AllocsPerRun(10, func() { + exit = n.IsExitNode() + }) + + 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 +} + func TestValidateHostinfo(t *testing.T) { t.Parallel()