From 2b7f15abaa079039660a1415ef357344372bc1fb Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 30 Apr 2026 11:40:29 +0000 Subject: [PATCH] policy/v2: surface autogroup:internet via grants on exit nodes A grant of the form `{src: alice, dst: autogroup:internet, via: tag:exit1}` was loading without error but stripping every exit node from alice's view: `tailscale exit-node list` returned "no exit nodes found". Two sites skipped autogroup:internet at the compile / steering layer: compileViaForNode's *AutoGroup arm produced no FilterRule for the via-tagged exit node, and ViaRoutesForPeer's *AutoGroup arm produced no Include/Exclude. With pm.needsPerNodeFilter true, the exit node's matchers were empty, BuildPeerMap could not link source to exit, and RoutesForPeer's ReduceRoutes stripped 0.0.0.0/0 and ::/0 from AllowedIPs. The skip belongs at the wire-format layer (ReduceFilterRules), not at the compile layer that also feeds internal matchers. Lift autogroup:internet handling into both *AutoGroup arms with the same shape used for *Prefix destinations: emit a TheInternet rule on via-tagged exit advertisers; surface peer.ExitRoutes() in Include when the peer carries the via tag, Exclude otherwise. ReduceFilterRules continues to keep the rule on exit-route advertisers' wire output and strip it elsewhere, preserving SaaS PacketFilter encoding. Also drop compileViaForNode's early len(SubnetRoutes)==0 return: SubnetRoutes excludes exit routes, so the early return pre-empted the autogroup:internet branch on nodes that only advertise exit routes. Existing tests pinning the buggy behaviour (TestViaRoutesForPeer subtests, TestCompileViaGrant case) flipped to the new contract. Fixes #3233 --- hscontrol/policy/v2/compiled.go | 23 ++++-- hscontrol/policy/v2/filter_test.go | 34 ++++++-- hscontrol/policy/v2/issue_3233_test.go | 106 +++++++++++++++++++++++++ hscontrol/policy/v2/policy.go | 14 +++- hscontrol/policy/v2/policy_test.go | 25 +++--- 5 files changed, 177 insertions(+), 25 deletions(-) create mode 100644 hscontrol/policy/v2/issue_3233_test.go diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index 935ccf03..ebc32c7b 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -5,6 +5,7 @@ import ( "slices" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" "go4.org/netipx" "tailscale.com/tailcfg" @@ -671,11 +672,10 @@ func compileViaForNode( return nil } - // Find matching destination prefixes. + // Find matching destination prefixes. SubnetRoutes() excludes exit + // routes, so the *Prefix check below sees only subnet advertisements; + // the *AutoGroup AutoGroupInternet branch checks IsExitNode() instead. nodeSubnetRoutes := node.SubnetRoutes() - if len(nodeSubnetRoutes) == 0 { - return nil - } var viaDstPrefixes []netip.Prefix @@ -689,8 +689,19 @@ func compileViaForNode( ) } case *AutoGroup: - // autogroup:internet via grants do not produce - // PacketFilter rules on exit nodes. + // autogroup:internet on a via-tagged exit advertiser + // becomes a rule whose DstPorts enumerate + // util.TheInternet(). The matchers derived from this + // rule let Node.CanAccess surface the exit node to the + // grant source via DestsIsTheInternet. ReduceFilterRules + // strips the rule from the wire format on non-exit + // advertisers, preserving SaaS PacketFilter encoding. + if d.Is(AutoGroupInternet) && node.IsExitNode() { + viaDstPrefixes = append( + viaDstPrefixes, + util.TheInternet().Prefixes()..., + ) + } } } diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index af874b8c..81f78f41 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -3668,6 +3668,27 @@ func TestCompileViaGrant(t *testing.T) { Hostinfo: &tailcfg.Hostinfo{}, } + // Expected rule for autogroup:internet on a via-tagged exit + // advertiser: SrcIPs scoped to the grant source, DstPorts + // enumerating util.TheInternet() prefixes. + internetDstPorts := make( + []tailcfg.NetPortRange, 0, len(util.TheInternet().Prefixes()), + ) + + for _, p := range util.TheInternet().Prefixes() { + internetDstPorts = append(internetDstPorts, tailcfg.NetPortRange{ + IP: p.String(), + Ports: tailcfg.PortRangeAny, + }) + } + + internetWant := []tailcfg.FilterRule{ + { + SrcIPs: []string{"100.64.0.10"}, + DstPorts: internetDstPorts, + }, + } + tests := []struct { name string grant Grant @@ -3724,11 +3745,12 @@ func TestCompileViaGrant(t *testing.T) { }, }, { - // autogroup:internet via grants do NOT produce PacketFilter rules - // on exit nodes. Tailscale SaaS handles exit traffic forwarding - // through the client's exit node mechanism, not PacketFilter. - // Verified by golden captures GRANT-V14 through GRANT-V36. - name: "autogroup:internet with exit routes produces no rules", + // autogroup:internet on a via-tagged exit advertiser + // produces a rule with DstPorts enumerating + // util.TheInternet(). The matchers derived from this + // rule let Node.CanAccess surface the exit node to + // grant sources via DestsIsTheInternet. + name: "autogroup:internet with exit routes produces TheInternet rule", grant: Grant{ Sources: Aliases{up("testuser@")}, Destinations: Aliases{agp(string(AutoGroupInternet))}, @@ -3738,7 +3760,7 @@ func TestCompileViaGrant(t *testing.T) { node: exitNode, nodes: types.Nodes{exitNode, srcNode}, pol: &Policy{}, - want: nil, + want: internetWant, }, { name: "autogroup:internet without exit routes returns nil", diff --git a/hscontrol/policy/v2/issue_3233_test.go b/hscontrol/policy/v2/issue_3233_test.go new file mode 100644 index 00000000..42c78891 --- /dev/null +++ b/hscontrol/policy/v2/issue_3233_test.go @@ -0,0 +1,106 @@ +// A via grant scoping autogroup:internet to a tag must surface only +// the matching exit node to the source — not strip every exit node +// from the source's view. +// +// Spec: https://tailscale.com/docs/features/access-control/grants/grants-via#route-users-through-exit-nodes-based-on-location +package v2 + +import ( + "net/netip" + "slices" + "testing" + + "github.com/juanfont/headscale/hscontrol/types" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + "tailscale.com/net/tsaddr" + "tailscale.com/tailcfg" +) + +// TestIssue3233ViaInternetExitVisibility loads a policy where alice's +// only access to autogroup:internet is via tag:exit1. Alice sees her +// tag:exit1 exit node as a peer with 0.0.0.0/0 + ::/0 in AllowedIPs, +// and does not see bob's tag:exit2 exit node. +func TestIssue3233ViaInternetExitVisibility(t *testing.T) { + t.Parallel() + + users := types.Users{ + {Model: gorm.Model{ID: 1}, Name: "alice", Email: "alice@headscale.net"}, + {Model: gorm.Model{ID: 2}, Name: "bob", Email: "bob@headscale.net"}, + } + + exitRoutes := []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()} + + aliceLaptop := node("alice-laptop", "100.64.0.10", "fd7a:115c:a1e0::a", users[0]) + aliceLaptop.ID = 1 + + aliceExit := node("alice-exit", "100.64.0.11", "fd7a:115c:a1e0::b", users[0]) + aliceExit.ID = 2 + aliceExit.Tags = []string{"tag:exit1"} + aliceExit.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: exitRoutes} + aliceExit.ApprovedRoutes = exitRoutes + + bobExit := node("bob-exit", "100.64.0.21", "fd7a:115c:a1e0::15", users[1]) + bobExit.ID = 3 + bobExit.Tags = []string{"tag:exit2"} + bobExit.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: exitRoutes} + bobExit.ApprovedRoutes = exitRoutes + + nodes := types.Nodes{aliceLaptop, aliceExit, bobExit} + + policy := `{ + "tagOwners": { + "tag:exit1": ["alice@headscale.net"], + "tag:exit2": ["bob@headscale.net"] + }, + "grants": [ + { + "src": ["alice@headscale.net"], + "dst": ["autogroup:internet"], + "via": ["tag:exit1"], + "ip": ["*"] + } + ] + }` + + pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice()) + require.NoError(t, err) + + t.Run("BuildPeerMap_includes_via_tagged_exit", func(t *testing.T) { + t.Parallel() + + peerMap := pm.BuildPeerMap(nodes.ViewSlice()) + + require.True(t, + slices.ContainsFunc(peerMap[aliceLaptop.ID], func(n types.NodeView) bool { + return n.ID() == aliceExit.ID + }), + "alice must see her tag:exit1 exit node as a peer") + + require.False(t, + slices.ContainsFunc(peerMap[aliceLaptop.ID], func(n types.NodeView) bool { + return n.ID() == bobExit.ID + }), + "alice must not see bob's tag:exit2 exit node — via grant scopes to tag:exit1") + }) + + t.Run("ViaRoutesForPeer_includes_exit_for_matching_tag", func(t *testing.T) { + t.Parallel() + + result := pm.ViaRoutesForPeer(aliceLaptop.View(), aliceExit.View()) + require.Contains(t, result.Include, tsaddr.AllIPv4(), + "alice viewing tag:exit1 exit must Include 0.0.0.0/0 — drives AllowedIPs in state.RoutesForPeer") + require.Contains(t, result.Include, tsaddr.AllIPv6(), + "alice viewing tag:exit1 exit must Include ::/0 — drives AllowedIPs in state.RoutesForPeer") + }) + + t.Run("ViaRoutesForPeer_excludes_exit_for_other_tag", func(t *testing.T) { + t.Parallel() + + result := pm.ViaRoutesForPeer(aliceLaptop.View(), bobExit.View()) + require.Contains(t, result.Exclude, tsaddr.AllIPv4(), + "alice viewing tag:exit2 exit must Exclude 0.0.0.0/0 — strips it from AllowedIPs") + require.Contains(t, result.Exclude, tsaddr.AllIPv6(), + "alice viewing tag:exit2 exit must Exclude ::/0 — strips it from AllowedIPs") + }) +} diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index 892082c2..355c8158 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -903,10 +903,16 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via matchedPrefixes = append(matchedPrefixes, dstPrefix) } case *AutoGroup: - // autogroup:internet via grants do NOT affect AllowedIPs or - // route steering for exit nodes. Tailscale SaaS handles exit - // traffic forwarding through the client's exit node selection - // mechanism, not through AllowedIPs. + // Per-viewer steering for autogroup:internet: a peer + // advertising approved exit routes is the via-tagged + // node's analogue of "advertises the destination". + // The downstream Include/Exclude split below restricts + // alice to exit nodes carrying the via tag. + if d.Is(AutoGroupInternet) && peer.IsExitNode() { + matchedPrefixes = append( + matchedPrefixes, peer.ExitRoutes()..., + ) + } } } diff --git a/hscontrol/policy/v2/policy_test.go b/hscontrol/policy/v2/policy_test.go index 10c55391..722799db 100644 --- a/hscontrol/policy/v2/policy_test.go +++ b/hscontrol/policy/v2/policy_test.go @@ -1640,10 +1640,14 @@ func TestViaRoutesForPeer(t *testing.T) { require.NoError(t, err) result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View()) - // Include should have only the subnet route. - // autogroup:internet does not produce via route effects. + // Include contains the subnet route plus the peer's approved + // exit routes — the peer holds tag:router and advertises exit + // routes, so autogroup:internet steering applies alongside the + // explicit prefix. require.Contains(t, result.Include, mp("10.0.0.0/24")) - require.Len(t, result.Include, 1) + require.Contains(t, result.Include, mp("0.0.0.0/0")) + require.Contains(t, result.Include, mp("::/0")) + require.Len(t, result.Include, 3) require.Empty(t, result.Exclude) }) @@ -1713,17 +1717,20 @@ func TestViaRoutesForPeer(t *testing.T) { pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice()) require.NoError(t, err) - // autogroup:internet via grants do NOT affect AllowedIPs or - // route steering. Tailscale SaaS handles exit traffic through - // the client's exit node mechanism, not ViaRoutesForPeer. - // Verified by golden captures GRANT-V14 through GRANT-V36. + // autogroup:internet via grants surface the peer's approved + // exit routes when the peer carries the via tag, and exclude + // them when it does not — restricting which exit nodes the + // viewer may use, per Tailscale's grants-via spec for + // autogroup:internet. resultExit := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View()) - require.Empty(t, resultExit.Include) + require.Contains(t, resultExit.Include, mp("0.0.0.0/0")) + require.Contains(t, resultExit.Include, mp("::/0")) require.Empty(t, resultExit.Exclude) resultOther := pm.ViaRoutesForPeer(nodes[0].View(), nodes[2].View()) require.Empty(t, resultOther.Include) - require.Empty(t, resultOther.Exclude) + require.Contains(t, resultOther.Exclude, mp("0.0.0.0/0")) + require.Contains(t, resultOther.Exclude, mp("::/0")) }) t.Run("via_routes_survive_reduce_routes", func(t *testing.T) {