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
This commit is contained in:
parent
ecaf56e0a0
commit
2b7f15abaa
@ -5,6 +5,7 @@ import (
|
|||||||
"slices"
|
"slices"
|
||||||
|
|
||||||
"github.com/juanfont/headscale/hscontrol/types"
|
"github.com/juanfont/headscale/hscontrol/types"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/util"
|
||||||
"github.com/rs/zerolog/log"
|
"github.com/rs/zerolog/log"
|
||||||
"go4.org/netipx"
|
"go4.org/netipx"
|
||||||
"tailscale.com/tailcfg"
|
"tailscale.com/tailcfg"
|
||||||
@ -671,11 +672,10 @@ func compileViaForNode(
|
|||||||
return nil
|
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()
|
nodeSubnetRoutes := node.SubnetRoutes()
|
||||||
if len(nodeSubnetRoutes) == 0 {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
|
|
||||||
var viaDstPrefixes []netip.Prefix
|
var viaDstPrefixes []netip.Prefix
|
||||||
|
|
||||||
@ -689,8 +689,19 @@ func compileViaForNode(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
case *AutoGroup:
|
case *AutoGroup:
|
||||||
// autogroup:internet via grants do not produce
|
// autogroup:internet on a via-tagged exit advertiser
|
||||||
// PacketFilter rules on exit nodes.
|
// 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()...,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -3668,6 +3668,27 @@ func TestCompileViaGrant(t *testing.T) {
|
|||||||
Hostinfo: &tailcfg.Hostinfo{},
|
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 {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
grant Grant
|
grant Grant
|
||||||
@ -3724,11 +3745,12 @@ func TestCompileViaGrant(t *testing.T) {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
// autogroup:internet via grants do NOT produce PacketFilter rules
|
// autogroup:internet on a via-tagged exit advertiser
|
||||||
// on exit nodes. Tailscale SaaS handles exit traffic forwarding
|
// produces a rule with DstPorts enumerating
|
||||||
// through the client's exit node mechanism, not PacketFilter.
|
// util.TheInternet(). The matchers derived from this
|
||||||
// Verified by golden captures GRANT-V14 through GRANT-V36.
|
// rule let Node.CanAccess surface the exit node to
|
||||||
name: "autogroup:internet with exit routes produces no rules",
|
// grant sources via DestsIsTheInternet.
|
||||||
|
name: "autogroup:internet with exit routes produces TheInternet rule",
|
||||||
grant: Grant{
|
grant: Grant{
|
||||||
Sources: Aliases{up("testuser@")},
|
Sources: Aliases{up("testuser@")},
|
||||||
Destinations: Aliases{agp(string(AutoGroupInternet))},
|
Destinations: Aliases{agp(string(AutoGroupInternet))},
|
||||||
@ -3738,7 +3760,7 @@ func TestCompileViaGrant(t *testing.T) {
|
|||||||
node: exitNode,
|
node: exitNode,
|
||||||
nodes: types.Nodes{exitNode, srcNode},
|
nodes: types.Nodes{exitNode, srcNode},
|
||||||
pol: &Policy{},
|
pol: &Policy{},
|
||||||
want: nil,
|
want: internetWant,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "autogroup:internet without exit routes returns nil",
|
name: "autogroup:internet without exit routes returns nil",
|
||||||
|
|||||||
106
hscontrol/policy/v2/issue_3233_test.go
Normal file
106
hscontrol/policy/v2/issue_3233_test.go
Normal file
@ -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")
|
||||||
|
})
|
||||||
|
}
|
||||||
@ -903,10 +903,16 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
|
|||||||
matchedPrefixes = append(matchedPrefixes, dstPrefix)
|
matchedPrefixes = append(matchedPrefixes, dstPrefix)
|
||||||
}
|
}
|
||||||
case *AutoGroup:
|
case *AutoGroup:
|
||||||
// autogroup:internet via grants do NOT affect AllowedIPs or
|
// Per-viewer steering for autogroup:internet: a peer
|
||||||
// route steering for exit nodes. Tailscale SaaS handles exit
|
// advertising approved exit routes is the via-tagged
|
||||||
// traffic forwarding through the client's exit node selection
|
// node's analogue of "advertises the destination".
|
||||||
// mechanism, not through AllowedIPs.
|
// 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()...,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -1640,10 +1640,14 @@ func TestViaRoutesForPeer(t *testing.T) {
|
|||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
|
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
|
||||||
// Include should have only the subnet route.
|
// Include contains the subnet route plus the peer's approved
|
||||||
// autogroup:internet does not produce via route effects.
|
// 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.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)
|
require.Empty(t, result.Exclude)
|
||||||
})
|
})
|
||||||
|
|
||||||
@ -1713,17 +1717,20 @@ func TestViaRoutesForPeer(t *testing.T) {
|
|||||||
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
|
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
|
||||||
require.NoError(t, err)
|
require.NoError(t, err)
|
||||||
|
|
||||||
// autogroup:internet via grants do NOT affect AllowedIPs or
|
// autogroup:internet via grants surface the peer's approved
|
||||||
// route steering. Tailscale SaaS handles exit traffic through
|
// exit routes when the peer carries the via tag, and exclude
|
||||||
// the client's exit node mechanism, not ViaRoutesForPeer.
|
// them when it does not — restricting which exit nodes the
|
||||||
// Verified by golden captures GRANT-V14 through GRANT-V36.
|
// viewer may use, per Tailscale's grants-via spec for
|
||||||
|
// autogroup:internet.
|
||||||
resultExit := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
|
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)
|
require.Empty(t, resultExit.Exclude)
|
||||||
|
|
||||||
resultOther := pm.ViaRoutesForPeer(nodes[0].View(), nodes[2].View())
|
resultOther := pm.ViaRoutesForPeer(nodes[0].View(), nodes[2].View())
|
||||||
require.Empty(t, resultOther.Include)
|
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) {
|
t.Run("via_routes_survive_reduce_routes", func(t *testing.T) {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user