From 3fdedefbf95cf2ea8ca6286b2015158827fcd910 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 31 Oct 2026 14:05:42 +0000 Subject: [PATCH] policy/v2: link a via grant's sources to its router only A rule naming the route also linked a router that is a source to all others serving it. SaaS does not (via-grant-v81). Updates #3626 --- hscontrol/policy/v2/canaccess_equiv_test.go | 2 +- hscontrol/policy/v2/compiled.go | 72 +++++++++++++++++++-- hscontrol/policy/v2/filter_test.go | 64 ++++++++++++++++++ hscontrol/policy/v2/peermap_scan_test.go | 2 +- hscontrol/policy/v2/policy.go | 13 +++- 5 files changed, 142 insertions(+), 11 deletions(-) diff --git a/hscontrol/policy/v2/canaccess_equiv_test.go b/hscontrol/policy/v2/canaccess_equiv_test.go index 24b4fb9c5..0bd2eab22 100644 --- a/hscontrol/policy/v2/canaccess_equiv_test.go +++ b/hscontrol/policy/v2/canaccess_equiv_test.go @@ -177,7 +177,7 @@ func peerMapPrefixReference( for _, n := range nodes { if pm.needsPerNodeFilter { nodeMatchers[n.ID] = matcher.MatchesFromFilterRules( - pm.filterRulesForNodeLocked(n.View())) + pm.peerRulesForNodeLocked(n.View())) } else { nodeMatchers[n.ID] = pm.matchers } diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index fddc11cae..d7ad7472d 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -442,7 +442,7 @@ func (s *grantSources) strings(hasWildcard, hasDangerAll bool, nodes views.Slice // compileOneViaGrant resolves sources for a via grant and stores the // deferred per-node data. The actual via-node matching and route -// intersection happens in [compileViaForNode]. +// intersection happens in [viaDstsForNode]. func (pol *Policy) compileOneViaGrant( grant Grant, users types.Users, @@ -787,6 +787,25 @@ func filterRulesForNode( grants []compiledGrant, node types.NodeView, userIdx userNodeIndex, +) []tailcfg.FilterRule { + return rulesForNode(grants, node, userIdx, compileViaForNode) +} + +// peerRulesForNode returns the rules that decide a node's peers: its +// unreduced filter rules, with [viaPeerRule] in place of each via rule. +func peerRulesForNode( + grants []compiledGrant, + node types.NodeView, + userIdx userNodeIndex, +) []tailcfg.FilterRule { + return rulesForNode(grants, node, userIdx, viaPeerRule) +} + +func rulesForNode( + grants []compiledGrant, + node types.NodeView, + userIdx userNodeIndex, + viaRules func(*compiledGrant, types.NodeView) []tailcfg.FilterRule, ) []tailcfg.FilterRule { var rules []tailcfg.FilterRule @@ -810,7 +829,7 @@ func filterRulesForNode( case grantCategoryVia: rules = append( rules, - compileViaForNode(cg, node)..., + viaRules(cg, node)..., ) } } @@ -985,13 +1004,13 @@ func filterSourcesToSameUser( return srcResolved } -// compileViaForNode produces via-grant filter rules for a specific -// node. Only produces rules when the node matches one of the via -// tags and advertises routes that match the grant destinations. -func compileViaForNode( +// viaDstsForNode returns the destinations node serves for a via grant: none +// unless it matches a via tag and has approved routes that match them, or is an +// exit node under autogroup:internet. +func viaDstsForNode( cg *compiledGrant, node types.NodeView, -) []tailcfg.FilterRule { +) []netip.Prefix { if cg.via == nil { return nil } @@ -1046,6 +1065,17 @@ func compileViaForNode( ) } + return viaDstPrefixes +} + +// compileViaForNode produces via-grant filter rules for a specific +// node. Only produces rules when the node matches one of the via +// tags and advertises routes that match the grant destinations. +func compileViaForNode( + cg *compiledGrant, + node types.NodeView, +) []tailcfg.FilterRule { + viaDstPrefixes := viaDstsForNode(cg, node) if len(viaDstPrefixes) == 0 { return nil } @@ -1079,3 +1109,31 @@ func compileViaForNode( return rules } + +// viaPeerRule returns a rule that links node, as a via router, to the grant's +// sources. It names the router's own addresses: the filter rule names the route, +// which would also link a router that is a source to everything else serving +// that route, and via-grant-v81 shows SaaS does not. +// +// shortcut: a node with an approved route over the router's address also matches, +// as for any rule naming a node. Bind the link to the router if that shape matters. +func viaPeerRule( + cg *compiledGrant, + node types.NodeView, +) []tailcfg.FilterRule { + if len(viaDstsForNode(cg, node)) == 0 { + return nil + } + + ips := node.IPs() + destPorts := make([]tailcfg.NetPortRange, 0, len(ips)) + + for _, ip := range ips { + destPorts = append(destPorts, tailcfg.NetPortRange{ + IP: ip.String(), + Ports: tailcfg.PortRangeAny, + }) + } + + return []tailcfg.FilterRule{{SrcIPs: cg.via.srcIPStrings, DstPorts: destPorts}} +} diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index 02b9e5872..eace8ff57 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -3990,6 +3990,70 @@ func TestCompileViaGrant(t *testing.T) { } } +// TestViaGrantPeers pins who a via grant links, as via-grant-v81 records: with +// ports, sources peer via routers only, also when a router is itself a source. +func TestViaGrantPeers(t *testing.T) { + t.Parallel() + + route := netip.MustParsePrefix("10.1.0.0/24") + tagged := func(id types.NodeID, advertised, approved bool, tags ...string) *types.Node { + nd := &types.Node{ + ID: id, + Hostname: fmt.Sprintf("n%d", id), + IPv4: ap(fmt.Sprintf("100.64.0.%d", id)), + Tags: tags, + Hostinfo: &tailcfg.Hostinfo{}, + } + + if advertised { + nd.Hostinfo.RoutableIPs = []netip.Prefix{route} + } + + if approved { + nd.ApprovedRoutes = []netip.Prefix{route} + } + + return nd + } + + nodes := types.Nodes{ + tagged(1, false, false, "tag:client"), + tagged(2, true, true, "tag:client", "tag:router"), // via router and a source + tagged(3, true, false, "tag:router"), // route not approved + tagged(4, false, false, "tag:router"), + tagged(5, true, true, "tag:other"), // serves the route, no via tag + tagged(6, false, false, "tag:other"), + } + + tests := []struct { + name string + grant string + want map[types.NodeID][]types.NodeID + }{ + { + name: "ports", + grant: `"ip": ["tcp:80"]`, + want: map[types.NodeID][]types.NodeID{1: {2}, 2: {1}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + policy := fmt.Sprintf(`{ + "tagOwners": {"tag:client": [], "tag:router": [], "tag:other": []}, + "grants": [{"src": ["tag:client"], "dst": ["10.1.0.0/24"], "via": ["tag:router"], %s}] + }`, tt.grant) + + pm, err := NewPolicyManager([]byte(policy), nil, nodes.ViewSlice()) + require.NoError(t, err) + + assert.Equal(t, tt.want, pm.BuildPeerMap(nodes.ViewSlice())) + }) + } +} + func TestCompileGrantWithAutogroupSelf_GrantPaths(t *testing.T) { t.Parallel() diff --git a/hscontrol/policy/v2/peermap_scan_test.go b/hscontrol/policy/v2/peermap_scan_test.go index 19bdd12d3..f78fefdb3 100644 --- a/hscontrol/policy/v2/peermap_scan_test.go +++ b/hscontrol/policy/v2/peermap_scan_test.go @@ -57,7 +57,7 @@ func buildPeerMapPairScan(pm *PolicyManager, nodes views.Slice[types.NodeView]) nodeMatchers := make(map[types.NodeID][]matcher.Match, nodes.Len()) for _, node := range nodes.All() { - nodeMatchers[node.ID()] = matcher.MatchesFromFilterRules(pm.filterRulesForNodeLocked(node)) + nodeMatchers[node.ID()] = matcher.MatchesFromFilterRules(pm.peerRulesForNodeLocked(node)) } for i := range nodes.Len() { diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index 6a87abc94..613e8dfaa 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -925,7 +925,7 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ // but peer relationships require the full bidirectional access rules. nodeMatchers := make(map[types.NodeID][]matcher.Match, nodes.Len()) for _, node := range nodes.All() { - unreduced := pm.filterRulesForNodeLocked(node) + unreduced := pm.peerRulesForNodeLocked(node) nodeMatchers[node.ID()] = matcher.MatchesFromFilterRules(unreduced) } @@ -946,7 +946,7 @@ func (pm *PolicyManager) BuildPeerMap(nodes views.Slice[types.NodeView]) map[typ riJ := routeInfo[nodeJ.ID()] // Check all access directions for symmetric peer visibility. - // For via grants, filter rules exist on the via-designated node + // For via grants, rules exist on the via-designated node // (e.g., router-a) with sources being the client (group-a). // We need to check BOTH: // 1. nodeI.CanAccess(matchersI, nodeJ) — can nodeI reach nodeJ? @@ -994,6 +994,14 @@ func (pm *PolicyManager) filterRulesForNodeLocked( ) } +// peerRulesForNodeLocked returns the rules that decide a node's peers, see +// [peerRulesForNode]. +func (pm *PolicyManager) peerRulesForNodeLocked( + node types.NodeView, +) []tailcfg.FilterRule { + return peerRulesForNode(pm.compiledGrants, node, pm.userNodeIdx) +} + // filterForNodeLocked returns the filter rules for a specific node, // already reduced to only include rules relevant to that node. // @@ -1055,6 +1063,7 @@ func (pm *PolicyManager) FilterForNode(node types.NodeView) ([]tailcfg.FilterRul // MatchersForNode returns the matchers that filter the routes a node sees. // These are UNREDUCED matchers - they include all rules where the node could be either source or destination. // This is different from [PolicyManager.FilterForNode] which returns REDUCED rules for packet filtering. +// Peers come from [PolicyManager.BuildPeerMap], which uses [viaPeerRule] where these matchers use the via rule. // // For global policies: returns the global matchers (same for all nodes) // For autogroup:self: returns node-specific matchers from unreduced compiled rules.