From 7f5fdbe5d2ff656d78c0cdd44a78edda285ebe98 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 25 Sep 2026 12:49:56 +0000 Subject: [PATCH] policy/v2: send exit nodes every user's autogroup:self rules Same as other rules: exit routes contain every self destination. Updates #3493 --- hscontrol/policy/v2/compiled.go | 46 ++++++++++++++-- hscontrol/policy/v2/filter.go | 5 +- hscontrol/policy/v2/policy.go | 5 +- hscontrol/policy/v2/policy_test.go | 86 ++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 5 deletions(-) diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index 7deafa6cd..43e3065cc 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -2,6 +2,7 @@ package v2 import ( "fmt" + "maps" "net/netip" "slices" @@ -704,15 +705,54 @@ func compileAutogroupSelf( node types.NodeView, userIdx userNodeIndex, ) []tailcfg.FilterRule { - if node.IsTagged() || cg.self == nil { + if node.IsTagged() || cg.self == nil || !node.User().Valid() { return nil } - if !node.User().Valid() { + return compileSelfForUser(cg, userIdx[node.User().ID()]) +} + +// exitNodeSelfRules returns the autogroup:self rules of every user +// other than the node's own for an exit node, whose exit routes contain +// every self destination. Only the packet filter needs them: they never +// make the exit node a peer, and expanding every user for peer matching +// would cost O(users) per exit node on each peer map build. +func exitNodeSelfRules( + grants []compiledGrant, + node types.NodeView, + userIdx userNodeIndex, +) []tailcfg.FilterRule { + if !node.IsExitNode() { return nil } - sameUserNodes := userIdx[node.User().ID()] + var rules []tailcfg.FilterRule + + for i := range grants { + cg := &grants[i] + if cg.self == nil { + continue + } + + for _, uid := range slices.Sorted(maps.Keys(userIdx)) { + // compileAutogroupSelf already covers an untagged node's own user. + if !node.IsTagged() && node.User().Valid() && node.User().ID() == uid { + continue + } + + rules = append(rules, compileSelfForUser(cg, userIdx[uid])...) + } + } + + return rules +} + +// compileSelfForUser produces the autogroup:self rules for one user's +// untagged devices. +func compileSelfForUser( + cg *compiledGrant, + sameUserNodes []types.NodeView, +) []tailcfg.FilterRule { if len(sameUserNodes) == 0 { return nil } diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 8ed85f97a..283d3106b 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -203,7 +203,10 @@ func (pol *Policy) compileFilterRulesForNode( grants := pol.compileGrants(users, nodes) userIdx := buildUserNodeIndex(nodes) - return filterRulesForNode(grants, node, userIdx) + return append( + filterRulesForNode(grants, node, userIdx), + exitNodeSelfRules(grants, node, userIdx)..., + ) } var sshAccept = tailcfg.SSHAction{ diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index 8aa228dc4..12d56d965 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -738,7 +738,10 @@ func (pm *PolicyManager) filterForNodeLocked( if !pm.needsPerNodeFilter { unreduced = pm.filter } else { - unreduced = pm.filterRulesForNodeLocked(node) + unreduced = append( + pm.filterRulesForNodeLocked(node), + exitNodeSelfRules(pm.compiledGrants, node, pm.userNodeIdx)..., + ) } reduced := policyutil.ReduceFilterRules(node, unreduced) diff --git a/hscontrol/policy/v2/policy_test.go b/hscontrol/policy/v2/policy_test.go index 07187e2f8..ee1c02e77 100644 --- a/hscontrol/policy/v2/policy_test.go +++ b/hscontrol/policy/v2/policy_test.go @@ -1445,6 +1445,92 @@ func TestAutogroupSelfCombinedWithTags(t *testing.T) { "web server should see admin phone (symmetric)") } +// TestAutogroupSelfRulesReachExitNodes checks that an approved exit +// node receives every user's autogroup:self rules: its exit routes +// contain every destination, and Tailscale SaaS delivers them. +func TestAutogroupSelfRulesReachExitNodes(t *testing.T) { + users := types.Users{ + {ID: 1, Name: "alice", Email: "alice@example.com"}, + {ID: 2, Name: "bob", Email: "bob@example.com"}, + } + + alice := &types.Node{ + ID: 1, + Hostname: "alice", + User: new(users[0]), + UserID: new(users[0].ID), + IPv4: ap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{}, + } + + bob := &types.Node{ + ID: 2, + Hostname: "bob", + User: new(users[1]), + UserID: new(users[1].ID), + IPv4: ap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{}, + } + + exit := &types.Node{ + ID: 3, + Hostname: "exit", + User: new(users[0]), + UserID: new(users[0].ID), + IPv4: ap("100.64.0.3"), + Tags: []string{"tag:exit"}, + Hostinfo: &tailcfg.Hostinfo{RoutableIPs: tsaddr.ExitRoutes()}, + ApprovedRoutes: tsaddr.ExitRoutes(), + } + + server := &types.Node{ + ID: 4, + Hostname: "server", + User: new(users[0]), + UserID: new(users[0].ID), + IPv4: ap("100.64.0.4"), + Tags: []string{"tag:server"}, + Hostinfo: &tailcfg.Hostinfo{}, + } + + nodes := types.Nodes{alice, bob, exit, server} + + policy := `{ + "tagOwners": { + "tag:exit": ["alice@example.com"], + "tag:server": ["alice@example.com"] + }, + "acls": [ + {"action": "accept", "src": ["autogroup:member"], "dst": ["autogroup:self:*"]} + ] + }` + + pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice()) + require.NoError(t, err) + + dsts := func(n *types.Node) []string { + rules, err := pm.FilterForNode(n.View()) + require.NoError(t, err) + + var out []string + + for _, r := range rules { + for _, dp := range r.DstPorts { + out = append(out, dp.IP) + } + } + + return out + } + + require.ElementsMatch(t, []string{"100.64.0.1", "100.64.0.2"}, dsts(exit), + "exit node must get every user's self rule") + require.Empty(t, dsts(server), + "tagged node without exit routes gets no self rules") + require.ElementsMatch(t, []string{"100.64.0.1"}, dsts(alice), + "user device gets only its own user's self rule") +} + // TestIssue2990SameUserTaggedDevice reproduces the exact scenario from issue #2990: // - One user (user1) who is in group:admin // - node1: user device (not tagged), belongs to user1