From c59e19be216a3c3c3f1e8a35710e15f279586b53 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 31 Oct 2026 14:00:13 +0000 Subject: [PATCH] policy/v2: check that captures agree on routes Parts of a capture are read seconds apart, so an approval can split them. Also refuses tool failures, lost nodes, unknown peers, lagging viewers and approvals that never landed. --- .../v2/tailscale_routes_data_compat_test.go | 361 ++++++++++++++++++ 1 file changed, 361 insertions(+) diff --git a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go index f01bc219c..49e486239 100644 --- a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go @@ -26,7 +26,9 @@ package v2 import ( + "encoding/json" "fmt" + "io/fs" "net/http" "net/netip" "path/filepath" @@ -43,8 +45,12 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tailscale/hujson" "go4.org/netipx" + "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" + "tailscale.com/types/netmap" + "tailscale.com/types/views" ) // loadRoutesTestFile loads and parses a single routes capture HuJSON file. @@ -1901,3 +1907,358 @@ func assertRoutesErrorContains( errStr, ) } + +// TestCapturesAgreeOnRoutes holds every capture to itself. The capture tool +// reads the topology and each node's netmap seconds apart, and an approval can +// land or reach a viewer in between, so a capture can straddle it. The compat +// tests would then replay a router with no routes against a netmap holding them. +func TestCapturesAgreeOnRoutes(t *testing.T) { + t.Parallel() + + var files []string + + err := filepath.WalkDir("testdata", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if !d.IsDir() && filepath.Ext(path) == ".hujson" { + files = append(files, path) + } + + return nil + }) + require.NoError(t, err) + require.NotEmpty(t, files) + + routes := func(n tailcfg.NodeView) []string { + var out []string + + for _, pfx := range n.AllowedIPs().All() { + if !views.SliceContains(n.Addresses(), pfx) { + out = append(out, pfx.String()) + } + } + + return out + } + + for _, file := range files { + t.Run(strings.TrimSuffix(filepath.Base(file), ".hujson"), func(t *testing.T) { + t.Parallel() + + c, err := testcapture.Read(file) + require.NoError(t, err) + assert.Empty(t, c.CaptureError, "the capture tool reported a failure") + + if len(c.Captures) == 0 { + require.Truef(t, slices.ContainsFunc(c.Steps, func(s testcapture.Step) bool { return s.Name == "validate" }), + "no node was captured and no step says that was the intent") + t.Skip("validation only, no node was asked for its state") + } + + assert.Len(t, c.Captures, len(c.Topology.Nodes), "captures against the topology") + + if !c.Error { + for name, topo := range c.Topology.Nodes { + assert.Subsetf(t, topo.ApprovedRoutes, tagApprovedRoutes(t, c.Input.FullPolicy, topo), + "%s: the policy approves routes the capture does not", name) + } + } + + for name, topo := range c.Topology.Nodes { + node, captured := c.Captures[name] + if !assert.Truef(t, captured, "%s is in the topology but was not captured", name) { + continue + } + + if !assert.Truef(t, node.Netmap != nil && node.Netmap.SelfNode.Valid(), + "%s was captured without its own netmap", name) { + continue + } + + assert.ElementsMatchf(t, topo.ApprovedRoutes, routes(node.Netmap.SelfNode), + "%s: approved routes against its own netmap", name) + + // The lag check below reads the filter, which is read apart + // from the netmap. + assert.Emptyf(t, cmp.Diff(node.PacketFilterRules, node.Netmap.PacketFilterRules.AsSlice(), append(cmpOptions(), cmpopts.EquateEmpty())...), + "%s: its filter against the one in its netmap", name) + + for _, peer := range node.Netmap.Peers { + peerName := peer.ComputedName() + + owner, known := c.Topology.Nodes[peerName] + if !assert.Truef(t, known, "%s has a peer outside the topology: %s", name, peerName) { + continue + } + + seen := routes(peer) + assert.Subsetf(t, owner.ApprovedRoutes, seen, + "%s sees routes on %s that it does not hold", name, peerName) + + // A policy can hide a route, but not one the filters use. + for _, route := range routesViewerMustSee(t, node, c.Captures[peerName], owner, c.Topology) { + assert.Containsf(t, seen, route, "%s lags: %s holds a route the filters give it", name, peerName) + } + } + } + }) + } +} + +// tagApprovedRoutes lists the routes node advertises that policy's +// autoApprovers grant to one of its tags. It reads the policy itself, so that +// the capture check does not lean on the approval code the replays test. +func tagApprovedRoutes(t *testing.T, policy string, node testcapture.TopologyNode) []string { + t.Helper() + + std, err := hujson.Standardize([]byte(policy)) + require.NoError(t, err) + + var pol struct { + AutoApprovers struct { + Routes map[string][]string `json:"routes"` + ExitNode []string `json:"exitNode"` + } `json:"autoApprovers"` + } + + require.NoError(t, json.Unmarshal(std, &pol)) + + hasTag := func(approvers []string) bool { + return slices.ContainsFunc(approvers, func(a string) bool { return slices.Contains(node.Tags, a) }) + } + + var out []string + + for _, advertised := range node.RoutableIPs { + route, err := netip.ParsePrefix(advertised) + require.NoError(t, err) + + approved := route.Bits() == 0 && hasTag(pol.AutoApprovers.ExitNode) + + for covering, approvers := range pol.AutoApprovers.Routes { + pfx, err := netip.ParsePrefix(covering) + require.NoError(t, err) + + if route.Bits() > 0 && pfx.Bits() <= route.Bits() && pfx.Contains(route.Addr()) && hasTag(approvers) { + approved = true + } + } + + if approved { + out = append(out, advertised) + } + } + + return out +} + +// routesViewerMustSee lists the owner's subnet routes that a viewer's netmap +// has to show: those the owner's filter admits the viewer to, and those the +// viewer's own filter admits traffic from. A route with more than one approver +// is left out, as only its primary shows it, and so is one inside the tailnet's +// own ranges, which TestRoutesTailnetOverlapCompat covers. +func routesViewerMustSee( + t *testing.T, + viewer, owner testcapture.Node, + ownerTopo testcapture.TopologyNode, + topo testcapture.Topology, +) []string { + t.Helper() + + if viewer.Netmap == nil || !viewer.Netmap.SelfNode.Valid() { + return nil + } + + overlaps := func(cidrs []string, route netip.Prefix) bool { + return slices.ContainsFunc(cidrs, func(cidr string) bool { + set, err := util.ParseIPSet(cidr, nil) + require.NoError(t, err) + + return set.OverlapsPrefix(route) + }) + } + + var out []string + + for _, approved := range ownerTopo.ApprovedRoutes { + route, err := netip.ParsePrefix(approved) + require.NoError(t, err) + + approvers := 0 + + for _, node := range topo.Nodes { + if slices.Contains(node.ApprovedRoutes, approved) { + approvers++ + } + } + + if route.Bits() == 0 || approvers > 1 || tsaddr.CGNATRange().Overlaps(route) || + tsaddr.TailscaleULARange().Overlaps(route) || tsaddr.CGNATRange().Contains(route.Addr().Unmap()) { + continue + } + + admitted := slices.ContainsFunc(owner.PacketFilterRules, func(rule tailcfg.FilterRule) bool { + return slices.ContainsFunc(viewer.Netmap.SelfNode.Addresses().AsSlice(), func(addr netip.Prefix) bool { + return overlaps(rule.SrcIPs, addr) + }) && slices.ContainsFunc(rule.DstPorts, func(dst tailcfg.NetPortRange) bool { + return overlaps([]string{dst.IP}, route) + }) + }) + + sourced := slices.ContainsFunc(viewer.PacketFilterRules, func(rule tailcfg.FilterRule) bool { + return !slices.Contains(rule.SrcIPs, "*") && overlaps(rule.SrcIPs, route) + }) + + if admitted || sourced { + out = append(out, approved) + } + } + + return out +} + +func TestRoutesViewerMustSee(t *testing.T) { + t.Parallel() + + const ( + viewerAddr = "100.64.0.1" + route = "10.33.0.0/16" + ) + + viewer := func(rules ...tailcfg.FilterRule) testcapture.Node { + return testcapture.Node{ + PacketFilterRules: rules, + Netmap: &netmap.NetworkMap{SelfNode: (&tailcfg.Node{ + Addresses: []netip.Prefix{netip.MustParsePrefix(viewerAddr + "/32")}, + }).View()}, + } + } + + allow := func(src, dst string) tailcfg.FilterRule { + return tailcfg.FilterRule{SrcIPs: []string{src}, DstPorts: []tailcfg.NetPortRange{{IP: dst, Ports: tailcfg.PortRangeAny}}} + } + + tests := []struct { + name string + viewer testcapture.Node + owner testcapture.Node + approved []string + others int + want []string + }{ + { + name: "the router's filter admits the viewer", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, route)}}, + approved: []string{route}, + want: []string{route}, + }, + { + name: "the router's filter admits someone else", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow("100.64.0.2", route)}}, + approved: []string{route}, + }, + { + name: "the router's filter admits the viewer elsewhere", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, "10.99.0.0/16")}}, + approved: []string{route}, + }, + { + name: "the viewer's filter admits traffic from the route", + viewer: viewer(allow(route, viewerAddr)), + approved: []string{route}, + want: []string{route}, + }, + { + name: "a wildcard source says nothing of the route", + viewer: viewer(allow("*", viewerAddr)), + approved: []string{route}, + }, + { + name: "two routers hold it, so only the primary shows it", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, route)}}, + approved: []string{route}, + others: 1, + }, + { + name: "exit routes are not subnet routes", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, "*")}}, + approved: []string{"0.0.0.0/0", "::/0"}, + }, + { + name: "a route inside the tailnet's range", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, "100.64.0.11/32")}}, + approved: []string{"100.64.0.11/32"}, + }, + { + name: "an IPv4-mapped route inside the tailnet's range", + viewer: viewer(), + owner: testcapture.Node{PacketFilterRules: []tailcfg.FilterRule{allow(viewerAddr, "::ffff:100.64.0.11/128")}}, + approved: []string{"::ffff:100.64.0.11/128"}, + }, + { + name: "a viewer with no netmap", + approved: []string{route}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + owner := testcapture.TopologyNode{ApprovedRoutes: tt.approved} + topo := testcapture.Topology{Nodes: map[string]testcapture.TopologyNode{"owner": owner}} + + for i := range tt.others { + topo.Nodes[fmt.Sprintf("other-%d", i)] = owner + } + + assert.Equal(t, tt.want, routesViewerMustSee(t, tt.viewer, tt.owner, owner, topo)) + }) + } +} + +func TestTagApprovedRoutes(t *testing.T) { + t.Parallel() + + const policy = `{ + // A comment, as a policy file can hold. + "autoApprovers": { + "routes": {"10.0.0.0/8": ["tag:router"], "192.168.0.0/24": ["alice@example.com"]}, + "exitNode": ["tag:exit"], + }, + }` + + node := func(routes []string, tags ...string) testcapture.TopologyNode { + return testcapture.TopologyNode{RoutableIPs: routes, Tags: tags} + } + + tests := []struct { + name string + node testcapture.TopologyNode + want []string + }{ + {"a wider approver covers it", node([]string{"10.33.0.0/16"}, "tag:router"), []string{"10.33.0.0/16"}}, + {"another tag", node([]string{"10.33.0.0/16"}, "tag:other"), nil}, + {"a route the approver does not cover", node([]string{"172.16.0.0/12"}, "tag:router"), nil}, + {"a route wider than the approver", node([]string{"10.0.0.0/7"}, "tag:router"), nil}, + {"a user approver is out of scope", node([]string{"192.168.0.0/24"}), nil}, + {"exit routes need the exit approver", node([]string{"0.0.0.0/0", "::/0"}, "tag:exit"), []string{"0.0.0.0/0", "::/0"}}, + {"a route approver does not approve exit", node([]string{"0.0.0.0/0"}, "tag:router"), nil}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, tagApprovedRoutes(t, policy, tt.node)) + }) + } +}