From 49d842463421a01fd993d1895b3c40db0ba9586a Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 22 Oct 2026 05:40:54 +0000 Subject: [PATCH] policy: reduce merged destination prefixes to the node's own addresses Adjacent node addresses merge into one prefix, and each node's filter kept all of it. A prefix made only of node addresses now shrinks to the node's own; a prefix the policy wrote, or one the node routes, stays. --- hscontrol/policy/policyutil/reduce.go | 40 ++++++++++++- hscontrol/policy/policyutil/reduce_test.go | 57 +++++++++++++++++-- hscontrol/policy/v2/compiled.go | 14 +++++ hscontrol/policy/v2/filter_test.go | 6 +- hscontrol/policy/v2/policy.go | 6 +- .../v2/tailscale_acl_data_compat_test.go | 1 + .../policy/v2/tailscale_grants_compat_test.go | 1 + .../v2/tailscale_routes_data_compat_test.go | 1 + hscontrol/servertest/mapstream_compat_test.go | 9 --- hscontrol/servertest/via_compat_test.go | 7 --- 10 files changed, 115 insertions(+), 27 deletions(-) diff --git a/hscontrol/policy/policyutil/reduce.go b/hscontrol/policy/policyutil/reduce.go index 1123d5d76..a6e803df0 100644 --- a/hscontrol/policy/policyutil/reduce.go +++ b/hscontrol/policy/policyutil/reduce.go @@ -13,11 +13,19 @@ import ( // ReduceFilterRules takes a node and a set of global filter rules and removes all rules // and destinations that are not relevant to that particular node. // +// nodeAddrs holds every node's addresses. A destination prefix made only of +// them merges several nodes' addresses, and the node keeps just its own, as +// Tailscale lists them; any other prefix stays whole. nil keeps all whole. +// // IMPORTANT: This function is designed for global filters only. Per-node filters // (from autogroup:self policies) are already node-specific and should not be passed // to this function. Use [policy.PolicyManager.FilterForNode] instead, which handles // both cases. -func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcfg.FilterRule { +func ReduceFilterRules( + node types.NodeView, + rules []tailcfg.FilterRule, + nodeAddrs *netipx.IPSet, +) []tailcfg.FilterRule { ret := []tailcfg.FilterRule{} nodeIPs := node.IPs() subnetRoutes := node.SubnetRoutes() @@ -40,7 +48,7 @@ func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcf for _, dest := range rule.DstPorts { if destReachesNode(dest.IP, nodeIPs, subnetRoutes, exitRoutes) { - dests = append(dests, dest) + dests = appendOwnDests(dests, dest, nodeIPs, subnetRoutes, exitRoutes, nodeAddrs) } } @@ -83,6 +91,34 @@ func destReachesNode(dst string, nodeIPs []netip.Addr, subnetRoutes, exitRoutes slices.ContainsFunc(exitRoutes, overlaps) } +// appendOwnDests appends dest, which reaches the node, to dests. A prefix +// made only of node addresses, which no route of the node overlaps, becomes +// one entry per node address inside it; anything else stays whole. +func appendOwnDests( + dests []tailcfg.NetPortRange, + dest tailcfg.NetPortRange, + nodeIPs []netip.Addr, + subnetRoutes, exitRoutes []netip.Prefix, + nodeAddrs *netipx.IPSet, +) []tailcfg.NetPortRange { + pfx, err := netip.ParsePrefix(dest.IP) + if err != nil || pfx.IsSingleIP() || nodeAddrs == nil || !nodeAddrs.ContainsPrefix(pfx) || + slices.ContainsFunc(subnetRoutes, pfx.Overlaps) || + slices.ContainsFunc(exitRoutes, pfx.Overlaps) { + return append(dests, dest) + } + + for _, ip := range nodeIPs { + if pfx.Contains(ip) { + own := dest + own.IP = ip.String() + dests = append(dests, own) + } + } + + return dests +} + // reduceCapGrantRule filters a [tailcfg.CapGrant] rule to only include // [tailcfg.CapGrant] entries whose Dsts match the given node's IPs. When a // broad prefix (e.g. 100.64.0.0/10 from dst:*) contains a node's IP, it is diff --git a/hscontrol/policy/policyutil/reduce_test.go b/hscontrol/policy/policyutil/reduce_test.go index 9039d75f1..06bca135e 100644 --- a/hscontrol/policy/policyutil/reduce_test.go +++ b/hscontrol/policy/policyutil/reduce_test.go @@ -13,6 +13,7 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "github.com/stretchr/testify/require" + "go4.org/netipx" "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/util/must" @@ -778,7 +779,7 @@ func TestReduceFilterRules(t *testing.T) { got, _ := pm.Filter() t.Logf("full filter:\n%s", must.Get(json.MarshalIndent(got, "", " "))) - got = policyutil.ReduceFilterRules(tt.node.View(), got) + got = policyutil.ReduceFilterRules(tt.node.View(), got, nil) if diff := cmp.Diff(tt.want, got); diff != "" { log.Trace().Interface("got", got).Msg("result") @@ -951,7 +952,7 @@ func TestReduceFilterRulesPartialApproval(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := policyutil.ReduceFilterRules( - tt.node.View(), tt.rules, + tt.node.View(), tt.rules, nil, ) require.Len(t, got, tt.wantCount, "rule count mismatch") @@ -1315,7 +1316,7 @@ func TestReduceFilterRulesCapGrant(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := policyutil.ReduceFilterRules(tt.node.View(), tt.rules) + got := policyutil.ReduceFilterRules(tt.node.View(), tt.rules, nil) require.Len(t, got, len(tt.want), "rule count mismatch") @@ -1377,7 +1378,7 @@ func TestReduceFilterRulesDestForms(t *testing.T) { rule.DstPorts = append(rule.DstPorts, tailcfg.NetPortRange{IP: dst, Ports: tailcfg.PortRangeAny}) } - got := policyutil.ReduceFilterRules(node.View(), []tailcfg.FilterRule{rule}) + got := policyutil.ReduceFilterRules(node.View(), []tailcfg.FilterRule{rule}, nil) require.Len(t, got, 1) kept := make(map[string]bool) @@ -1397,6 +1398,13 @@ func TestReduceFilterRulesDestForms(t *testing.T) { func TestReduceFilterRulesAllocsIndependentOfDests(t *testing.T) { node := (&types.Node{IPv4: ap("100.64.0.1"), IPv6: ap("fd7a:115c:a1e0::1")}).View() + var b netipx.IPSetBuilder + + node.AppendToIPSet(&b) + + nodeAddrs, err := b.IPSet() + require.NoError(t, err) + allocs := func(n int) int { rule := tailcfg.FilterRule{SrcIPs: []string{"*"}} for i := range n { @@ -1408,9 +1416,48 @@ func TestReduceFilterRulesAllocsIndependentOfDests(t *testing.T) { rules := []tailcfg.FilterRule{rule} - return int(testing.AllocsPerRun(10, func() { policyutil.ReduceFilterRules(node, rules) })) + return int(testing.AllocsPerRun(10, func() { policyutil.ReduceFilterRules(node, rules, nodeAddrs) })) } require.Equal(t, allocs(10), allocs(1000), "reducing a filter must not allocate per DstPorts entry") } + +// TestReduceFilterRulesMergedNodeDests checks that a destination merged from +// node addresses shrinks to the node's own addresses, while a written prefix, +// and any destination a router serves, stays whole. +func TestReduceFilterRulesMergedNodeDests(t *testing.T) { + node := &types.Node{IPv4: ap("100.64.0.6"), IPv6: ap("fd7a:115c:a1e0::6")} + neighbour := &types.Node{IPv4: ap("100.64.0.7"), IPv6: ap("fd7a:115c:a1e0::7")} + + var b netipx.IPSetBuilder + + node.AppendToIPSet(&b) + neighbour.AppendToIPSet(&b) + + nodeAddrs, err := b.IPSet() + require.NoError(t, err) + + rules := []tailcfg.FilterRule{{ + SrcIPs: []string{"100.64.0.1"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "100.64.0.6/31", Ports: tailcfg.PortRange{First: 22, Last: 22}}, + {IP: "fd7a:115c:a1e0::6/127", Ports: tailcfg.PortRange{First: 22, Last: 22}}, + {IP: "100.64.0.0/12", Ports: tailcfg.PortRangeAny}, + }, + }} + + got := policyutil.ReduceFilterRules(node.View(), rules, nodeAddrs) + require.Equal(t, []tailcfg.NetPortRange{ + {IP: "100.64.0.6", Ports: tailcfg.PortRange{First: 22, Last: 22}}, + {IP: "fd7a:115c:a1e0::6", Ports: tailcfg.PortRange{First: 22, Last: 22}}, + {IP: "100.64.0.0/12", Ports: tailcfg.PortRangeAny}, + }, got[0].DstPorts) + + // An exit node forwards to every destination, so it keeps them whole. + node.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: tsaddr.ExitRoutes()} + node.ApprovedRoutes = tsaddr.ExitRoutes() + + got = policyutil.ReduceFilterRules(node.View(), rules, nodeAddrs) + require.Equal(t, rules[0].DstPorts, got[0].DstPorts) +} diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index 4f6d322c5..4b475d60d 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -139,6 +139,20 @@ func buildUserNodeIndex( return idx } +// nodeAddrSet is the addresses of every node. +func nodeAddrSet(nodes views.Slice[types.NodeView]) *netipx.IPSet { + var b netipx.IPSetBuilder + + for _, n := range nodes.All() { + n.AppendToIPSet(&b) + } + + // The builder only fails on invalid addresses, which nodes never hold. + set, _ := b.IPSet() + + return set +} + // compileNodeAttrs returns the per-node CapMap derived from policy // nodeAttrs plus the tailnet-wide [Policy.RandomizeClientPort] flag. // diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index fe62e1b4c..dde6a605e 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -1497,7 +1497,7 @@ func TestTagUserMutualExclusivity(t *testing.T) { compiled := pol.compileFilterRulesForNode(users, userNode, nodes.ViewSlice()) - userRules := policyutil.ReduceFilterRules(userNode, compiled) + userRules := policyutil.ReduceFilterRules(userNode, compiled, nodeAddrSet(nodes.ViewSlice())) for _, rule := range userRules { for _, dst := range rule.DstPorts { @@ -1519,7 +1519,7 @@ func TestTagUserMutualExclusivity(t *testing.T) { compiled = pol.compileFilterRulesForNode(users, dbNode, nodes.ViewSlice()) - dbRules := policyutil.ReduceFilterRules(dbNode, compiled) + dbRules := policyutil.ReduceFilterRules(dbNode, compiled, nodeAddrSet(nodes.ViewSlice())) foundServerSrc := false @@ -1590,7 +1590,7 @@ func TestUserToTagCrossIdentityGrant(t *testing.T) { compiled := pol.compileFilterRulesForNode(users, taggedNode, nodes.ViewSlice()) - rules := policyutil.ReduceFilterRules(taggedNode, compiled) + rules := policyutil.ReduceFilterRules(taggedNode, compiled, nodeAddrSet(nodes.ViewSlice())) // user1's IP should appear as a source that can reach tag:server. foundUser1Src := false diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index ab08b3a4f..667d908ee 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -42,6 +42,9 @@ type PolicyManager struct { nodes views.Slice[types.NodeView] // nodesByID indexes nodes; see [PolicyManager.cacheableLocked]. nodesByID map[types.NodeID]types.NodeView + // nodeAddrs is every node's addresses, for + // [policyutil.ReduceFilterRules]. + nodeAddrs *netipx.IPSet // resolvedPol is pol with its tags indexed and its groups resolved // against users and nodes, see [Policy.withNodesByTag] and @@ -313,6 +316,7 @@ func (pm *PolicyManager) updateLocked() (bool, error) { pm.resolvedPol = pol pm.compiledGrants = grants pm.userNodeIdx = buildUserNodeIndex(pm.nodes) + pm.nodeAddrs = nodeAddrSet(pm.nodes) pm.needsPerNodeFilter = hasPerNodeGrants(pm.compiledGrants) pm.viaTargetTags = collectViaTargetTags(pm.compiledGrants) pm.relayTargetIPs = relayTargetIPs @@ -973,7 +977,7 @@ func (pm *PolicyManager) filterForNodeLocked( ) } - reduced := policyutil.ReduceFilterRules(node, unreduced) + reduced := policyutil.ReduceFilterRules(node, unreduced, pm.nodeAddrs) if pm.cacheableLocked(node) { pm.filterRulesMap.Store(node.ID(), reduced) } diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index 9e8e6bdc2..2395d310c 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -372,6 +372,7 @@ func testACLSuccess( gotRules := policyutil.ReduceFilterRules( node.View(), pol.compileFilterRulesForNode(users, node.View(), nodes.ViewSlice()), + nodeAddrSet(nodes.ViewSlice()), ) if diff := cmp.Diff(wantRules, gotRules, opts...); diff != "" { diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index 760a43585..54da13bd1 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -281,6 +281,7 @@ func testGrantSuccess( gotRules := policyutil.ReduceFilterRules( node.View(), pol.compileFilterRulesForNode(users, node.View(), nodes.ViewSlice()), + nodeAddrSet(nodes.ViewSlice()), ) pmRules, err := pm.FilterForNode(node.View()) diff --git a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go index 1e7bbe765..a07591939 100644 --- a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go @@ -366,6 +366,7 @@ func TestRoutesCompat(t *testing.T) { gotRules := policyutil.ReduceFilterRules( node.View(), compiledRules, + nodeAddrSet(nodes.ViewSlice()), ) wantRules := capture.PacketFilterRules diff --git a/hscontrol/servertest/mapstream_compat_test.go b/hscontrol/servertest/mapstream_compat_test.go index 4eb58738c..41faedeb0 100644 --- a/hscontrol/servertest/mapstream_compat_test.go +++ b/hscontrol/servertest/mapstream_compat_test.go @@ -140,15 +140,6 @@ var mapStreamSkipReasons = map[string]string{ "policy-generation/G4/thor-1/peer-owners": sameUserPeers, "policy-generation/G4/thor-2/peer-owners": sameUserPeers, - "policy-generation/G2/freya-1/filter": filterDstMerged, - "policy-generation/G2/thor-2/filter": filterDstMerged, - "policy-generation/G3/freya-1/filter": filterDstMerged, - "policy-generation/G3/thor-2/filter": filterDstMerged, - "policy-generation/G4/freya-1/filter": filterDstMerged, - "policy-generation/G4/thor-2/filter": filterDstMerged, - "policy-generation/G5/freya-1/filter": filterDstMerged, - "policy-generation/G5/thor-2/filter": filterDstMerged, - "policy-generation/G4/rt/enabled": withdrawnRouteEnabled, "policy-generation/G5/rt/enabled": withdrawnRouteEnabled, "policy-generation/G7/rt/enabled": withdrawnRouteEnabled, diff --git a/hscontrol/servertest/via_compat_test.go b/hscontrol/servertest/via_compat_test.go index 127eb4bb6..fe774ab5f 100644 --- a/hscontrol/servertest/via_compat_test.go +++ b/hscontrol/servertest/via_compat_test.go @@ -107,9 +107,6 @@ var routesMapCompatTests = []struct { var viaCompatSkipReasons = map[string]string{ "tagged-self": "TAGGED_SELF_PROFILE: SaaS gives a tagged node a profile of its own, headscale the shared tagged-devices one", - "routes-f2-subnet-as-acl-source/ivysaur/packet-filter": filterDstMerged, - "routes-f2-subnet-as-acl-source/venusaur/packet-filter": filterDstMerged, - // SaaS sends a router every approved route in its own AllowedIPs and // every route it is primary for in PrimaryRoutes. "grant-v11/blastoise/self-routes": selfRoutesReduced, @@ -258,10 +255,6 @@ var viaCompatSkipReasons = map[string]string{ const exitRoutesOnOverlap = "EXIT_ROUTES_ON_OVERLAP: headscale offers a peer's exit routes to a viewer whose rules reach only nodes or subnets; SaaS leaves them out" -// Adjacent member addresses merge into one prefix, and the reduced filter -// keeps all of it; SaaS lists only the node's own address. -const filterDstMerged = "FILTER_DST_MERGED: own filter names a neighbour's address" - const sameUserPeers = "SAME_USER_PEERS: SaaS lists a user's own devices as each other's peers with no rule linking them; headscale peers only what the policy links" const selfRoutesReduced = "SELF_ROUTES_REDUCED: headscale sends a router only the primary routes its policy lets it reach"