policy/v2: match via-grant destinations by prefix overlap

slices.Contains required exact equality between grant dst and the
advertised subnet route. Any non-identical pair was rejected, so a
via grant with broader (or narrower) dst emitted no filter rule and
added no route to the viewer's AllowedIPs. Tailscale SaaS uses
containment in either direction.

Switch to slices.ContainsFunc(routes, dst.Overlaps) for filter rule
emission (keep dst literal in DstPorts), and append overlapping
advertised routes to ViaRoutesForPeer.Include / Exclude. Rewrite the
multi-router HA election and regular-grant overlap detection to key
off the matched routes rather than the dst. Resolve *Host aliases to
*Prefix once in compileOneViaGrant and at the top of ViaRoutesForPeer
so the switch arms reach them.

Fixes #3267
This commit is contained in:
Kristoffer Dalby
2026-05-18 09:42:28 +00:00
parent af7e7a4560
commit e5fcd01ee6
4 changed files with 402 additions and 96 deletions

View File

@@ -66,12 +66,52 @@ type selfGrantData struct {
}
// viaGrantData holds data needed for per-node via-grant compilation.
// Sources are already resolved into srcIPStrings.
// Sources are already resolved into srcIPStrings; destinations are
// pre-resolved into prefixes plus a flag for autogroup:internet, which
// the consumers handle separately because its gate is per-node
// (IsExitNode()) rather than per-prefix overlap.
type viaGrantData struct {
viaTags []Tag
destinations Aliases
internetProtocols []ProtocolPort
srcIPStrings []string
viaTags []Tag
resolvedDsts []netip.Prefix
hasAutoGroupInternet bool
internetProtocols []ProtocolPort
srcIPStrings []string
}
// resolveViaDestinations splits a via grant's destinations into the
// flat list of IP prefixes they resolve to plus a flag for
// autogroup:internet. Every alias kind goes through Alias.Resolve so
// adding a new alias type to the policy parser does not silently
// disappear from the via path. Non-IP alias kinds (tag, user, group,
// wildcard) resolve to /32 host IPs that never overlap with subnet
// route advertisements and therefore contribute nothing here.
func resolveViaDestinations(
pol *Policy,
users types.Users,
nodes views.Slice[types.NodeView],
dsts Aliases,
) ([]netip.Prefix, bool) {
var (
prefixes []netip.Prefix
hasAutoGroupInternet bool
)
for _, d := range dsts {
if ag, ok := d.(*AutoGroup); ok && ag.Is(AutoGroupInternet) {
hasAutoGroupInternet = true
continue
}
ips, err := d.Resolve(pol, users, nodes)
if err != nil || ips == nil {
continue
}
prefixes = append(prefixes, ips.Prefixes()...)
}
return prefixes, hasAutoGroupInternet
}
// userNodeIndex maps user IDs to their untagged nodes. Built once per
@@ -343,12 +383,17 @@ func (pol *Policy) compileOneViaGrant(
hasWildcard := sourcesHaveWildcard(grant.Sources)
hasDangerAll := sourcesHaveDangerAll(grant.Sources)
resolvedDsts, hasAutoGroupInternet := resolveViaDestinations(
pol, users, nodes, grant.Destinations,
)
return &compiledGrant{
category: grantCategoryVia,
via: &viaGrantData{
viaTags: grant.Via,
destinations: grant.Destinations,
internetProtocols: grant.InternetProtocols,
viaTags: grant.Via,
resolvedDsts: resolvedDsts,
hasAutoGroupInternet: hasAutoGroupInternet,
internetProtocols: grant.InternetProtocols,
srcIPStrings: srcIPsWithRoutes(
srcResolved, hasWildcard, hasDangerAll, nodes,
),
@@ -759,39 +804,40 @@ func compileViaForNode(
return nil
}
// Find matching destination prefixes. SubnetRoutes() excludes exit
// routes, so the *Prefix check below sees only subnet advertisements;
// the *AutoGroup AutoGroupInternet branch checks IsExitNode() instead.
// SubnetRoutes excludes exit routes, so the overlap gate below sees
// only subnet advertisements. autogroup:internet on a via-tagged
// exit advertiser is handled separately because its eligibility is
// per-node (IsExitNode) rather than per-prefix overlap.
nodeSubnetRoutes := node.SubnetRoutes()
var viaDstPrefixes []netip.Prefix
for _, dst := range cg.via.destinations {
switch d := dst.(type) {
case *Prefix:
dstPrefix := netip.Prefix(*d)
if slices.Contains(nodeSubnetRoutes, dstPrefix) {
viaDstPrefixes = append(
viaDstPrefixes, dstPrefix,
)
}
case *AutoGroup:
// 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()...,
)
}
for _, dstPrefix := range cg.via.resolvedDsts {
// Equality would reject any broader or narrower dst relative
// to the advertised route. Containment in either direction
// matches the operator's authorisation: a broader dst restricts
// traffic to the subset the router serves; a narrower dst rides
// on a router covering more than the operator asked for. The
// rule emits the literal dst either way because that is what
// the policy authorised.
if slices.ContainsFunc(nodeSubnetRoutes, dstPrefix.Overlaps) {
viaDstPrefixes = append(viaDstPrefixes, dstPrefix)
}
}
// 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 cg.via.hasAutoGroupInternet && node.IsExitNode() {
viaDstPrefixes = append(
viaDstPrefixes,
util.TheInternet().Prefixes()...,
)
}
if len(viaDstPrefixes) == 0 {
return nil
}

View File

@@ -0,0 +1,120 @@
package v2
import (
"net/netip"
"slices"
"testing"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"tailscale.com/tailcfg"
)
const issue3267AliceEmail = "alice@headscale.net"
// TestIssue3267ViaGrantBroaderDestination locks the SaaS contract for
// a via grant whose destination is a host alias broader than (or
// narrower than) the router's advertised subnet route. Alice's only
// path into the subnet is via tag:subnet-router. The grant destination
// resolves to a /64 (IPv6) or /16 (IPv4), and the router advertises a
// contained /120 / /24. Pre-fix the policy compiler emitted no rule
// and ViaRoutesForPeer left Include empty because the prefix relation
// was checked by slices.Contains (exact equality). SaaS behaviour is
// the authority — see testdata/grant_results/via-grant-v47..v51 for
// the equivalent compatibility regression.
func TestIssue3267ViaGrantBroaderDestination(t *testing.T) {
t.Parallel()
users := types.Users{
{Model: gorm.Model{ID: 1}, Name: "alice", Email: issue3267AliceEmail}, //nolint:goconst
}
cases := []struct {
name string
hostAlias string
dst string // value the hosts alias resolves to
advertised string // narrower prefix the router actually serves
}{
{
name: "ipv6_4via6_64_dst_with_120_advertised",
hostAlias: "example-4via6",
dst: "fd7a:115c:a1e0:b1a::/64",
advertised: "fd7a:115c:a1e0:b1a:0:13:ad2:7300/120",
},
{
name: "ipv4_16_dst_with_24_advertised",
hostAlias: "subnet",
dst: "10.33.0.0/16",
advertised: "10.33.5.0/24",
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
aliceLaptop := node("alice-laptop", "100.64.0.10", "fd7a:115c:a1e0::a", users[0])
aliceLaptop.ID = 1
router := node("subnet-router", "100.64.0.11", "fd7a:115c:a1e0::b", users[0])
router.ID = 2
router.Tags = []string{"tag:subnet-router"}
route := netip.MustParsePrefix(tc.advertised)
router.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: []netip.Prefix{route}}
router.ApprovedRoutes = []netip.Prefix{route}
nodes := types.Nodes{aliceLaptop, router}
policy := `{
"tagOwners": {
"tag:subnet-router": ["` + issue3267AliceEmail + `"]
},
"hosts": {
"` + tc.hostAlias + `": "` + tc.dst + `"
},
"grants": [
{
"src": ["` + issue3267AliceEmail + `"],
"dst": ["` + tc.hostAlias + `"],
"via": ["tag:subnet-router"],
"ip": ["icmp:*"]
}
]
}`
pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice())
require.NoError(t, err)
pol, err := unmarshalPolicy([]byte(policy))
require.NoError(t, err)
require.NoError(t, pol.validate())
t.Run("compileFilterRulesForNode_emits_rule_with_grant_dst", func(t *testing.T) {
t.Parallel()
rules, err := pol.compileFilterRulesForNode(users, router.View(), nodes.ViewSlice())
require.NoError(t, err)
found := slices.ContainsFunc(rules, func(r tailcfg.FilterRule) bool {
return slices.ContainsFunc(r.DstPorts, func(d tailcfg.NetPortRange) bool {
return d.IP == tc.dst
})
})
require.Truef(t, found,
"router %s must receive a via filter rule whose DstPorts.IP equals the grant dst %q; got rules=%+v",
router.Hostname, tc.dst, rules)
})
t.Run("ViaRoutesForPeer_includes_advertised_prefix", func(t *testing.T) {
t.Parallel()
result := pm.ViaRoutesForPeer(aliceLaptop.View(), router.View())
require.Contains(t, result.Include, route,
"alice viewing tag:subnet-router must Include advertised prefix %s — drives AllowedIPs in state.RoutesForPeer", route)
require.Empty(t, result.Exclude,
"alice viewing tag:subnet-router must not Exclude any prefix — there is no competing via tag")
})
})
}
}

View File

@@ -1009,11 +1009,13 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
grants = append(grants, aclToGrants(acl)...)
}
// Resolve each grant's sources against the viewer once. The three
// passes below reuse this result instead of calling src.Resolve
// per grant per pass.
// Resolve each grant's sources against the viewer once, and each
// grant's destinations into a flat prefix list. The three passes
// below reuse both results instead of re-resolving per pass.
viewerIPs := viewer.IPs()
viewerMatchesGrant := make([]bool, len(grants))
resolvedDstPrefixes := make([][]netip.Prefix, len(grants))
grantHasAutoGroupInternet := make([]bool, len(grants))
for i, grant := range grants {
for _, src := range grant.Sources {
@@ -1028,6 +1030,10 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
break
}
}
resolvedDstPrefixes[i], grantHasAutoGroupInternet[i] = resolveViaDestinations(
pm.pol, pm.users, pm.nodes, grant.Destinations,
)
}
for i, grant := range grants {
@@ -1039,32 +1045,32 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
continue
}
// Collect destination prefixes that the peer actually advertises.
// Filter rules and AllowedIPs are different layers. The filter
// rule carries the dst (the authorisation surface). AllowedIPs
// carries the advertised route (the routing fact the viewer
// needs to pick this peer). This loop builds the AllowedIPs
// side, so it emits routes — not dst prefixes.
peerSubnetRoutes := peer.SubnetRoutes()
var matchedPrefixes []netip.Prefix
for _, dst := range grant.Destinations {
switch d := dst.(type) {
case *Prefix:
dstPrefix := netip.Prefix(*d)
if slices.Contains(peerSubnetRoutes, dstPrefix) {
matchedPrefixes = append(matchedPrefixes, dstPrefix)
}
case *AutoGroup:
// 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()...,
)
for _, dstPrefix := range resolvedDstPrefixes[i] {
for _, route := range peerSubnetRoutes {
if dstPrefix.Overlaps(route) {
matchedPrefixes = append(matchedPrefixes, route)
}
}
}
// 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 the viewer to exit nodes carrying the
// via tag.
if grantHasAutoGroupInternet[i] && peer.IsExitNode() {
matchedPrefixes = append(matchedPrefixes, peer.ExitRoutes()...)
}
if len(matchedPrefixes) == 0 {
continue
}
@@ -1121,40 +1127,35 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
continue
}
for _, dst := range grant.Destinations {
d, ok := dst.(*Prefix)
if !ok {
continue
}
// Elect per matched route, not per dst — a peer can only
// be primary for a prefix it actually advertises, and one
// dst may cover multiple distinct routes.
for _, dstPrefix := range resolvedDstPrefixes[i] {
for _, included := range slices.Clone(result.Include) {
if !dstPrefix.Overlaps(included) {
continue
}
dstPrefix := netip.Prefix(*d)
if !slices.Contains(result.Include, dstPrefix) {
continue
}
var viaPrimaryID types.NodeID
// Find the lowest-ID peer with this via tag that
// advertises this prefix — the via-group primary.
var viaPrimaryID types.NodeID
for _, viaTag := range grant.Via {
for _, node := range pm.nodes.All() {
if node.HasTag(string(viaTag)) &&
slices.Contains(node.SubnetRoutes(), dstPrefix) {
if viaPrimaryID == 0 || node.ID() < viaPrimaryID {
viaPrimaryID = node.ID()
for _, viaTag := range grant.Via {
for _, node := range pm.nodes.All() {
if node.HasTag(string(viaTag)) &&
slices.Contains(node.SubnetRoutes(), included) {
if viaPrimaryID == 0 || node.ID() < viaPrimaryID {
viaPrimaryID = node.ID()
}
}
}
}
}
// If the current peer is not the via-group primary,
// demote the prefix from Include to Exclude.
if viaPrimaryID != 0 && peer.ID() != viaPrimaryID {
result.Include = slices.DeleteFunc(result.Include, func(p netip.Prefix) bool {
return p == dstPrefix
})
if !slices.Contains(result.Exclude, dstPrefix) {
result.Exclude = append(result.Exclude, dstPrefix)
if viaPrimaryID != 0 && peer.ID() != viaPrimaryID {
result.Include = slices.DeleteFunc(result.Include, func(p netip.Prefix) bool {
return p == included
})
if !slices.Contains(result.Exclude, included) {
result.Exclude = append(result.Exclude, included)
}
}
}
}
@@ -1175,21 +1176,19 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
continue
}
for _, dst := range grant.Destinations {
if d, ok := dst.(*Prefix); ok {
dstPrefix := netip.Prefix(*d)
if slices.Contains(result.Include, dstPrefix) &&
!slices.Contains(result.UsePrimary, dstPrefix) {
result.UsePrimary = append(result.UsePrimary, dstPrefix)
// A non-via grant covering routes that a via grant included
// defers to global HA primary election. Match by overlap so
// a broader or narrower regular dst still catches the
// routes the via grant added to Include.
for _, dstPrefix := range resolvedDstPrefixes[i] {
for _, p := range result.Include {
if dstPrefix.Overlaps(p) &&
!slices.Contains(result.UsePrimary, p) {
result.UsePrimary = append(result.UsePrimary, p)
}
// A regular grant overrides a via exclusion: the
// peer doesn't need the via tag if the viewer has
// direct (non-via) access to the prefix.
result.Exclude = slices.DeleteFunc(result.Exclude, func(p netip.Prefix) bool {
return p == dstPrefix
})
}
result.Exclude = slices.DeleteFunc(result.Exclude, dstPrefix.Overlaps)
}
}
}

View File

@@ -1813,6 +1813,147 @@ func TestViaRoutesForPeer(t *testing.T) {
"client should NOT be able to access 10.0.0.0/24 via matchers alone; "+
"state.RoutesForPeer adds via routes after ReduceRoutes to fix this")
})
t.Run("broader_dst_includes_narrower_advertised_route", func(t *testing.T) {
t.Parallel()
nodes := types.Nodes{
{
ID: 1,
Hostname: "viewer",
IPv4: ap("100.64.0.1"),
User: new(users[0]),
UserID: new(users[0].ID),
Hostinfo: &tailcfg.Hostinfo{},
},
{
ID: 2,
Hostname: "router",
IPv4: ap("100.64.0.2"),
User: new(users[0]),
UserID: new(users[0].ID),
Tags: []string{"tag:router"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{mp("10.33.5.0/24")},
},
ApprovedRoutes: []netip.Prefix{mp("10.33.5.0/24")},
},
}
pol := `{
"tagOwners": {
"tag:router": ["user1@"]
},
"grants": [{
"src": ["user1@"],
"dst": ["10.0.0.0/8"],
"ip": ["*"],
"via": ["tag:router"]
}]
}`
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
require.NoError(t, err)
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
require.Equal(t, []netip.Prefix{mp("10.33.5.0/24")}, result.Include,
"Include must hold the advertised route /24, not the broader grant dst /8")
require.Empty(t, result.Exclude)
})
t.Run("narrower_dst_includes_advertised_route", func(t *testing.T) {
t.Parallel()
nodes := types.Nodes{
{
ID: 1,
Hostname: "viewer",
IPv4: ap("100.64.0.1"),
User: new(users[0]),
UserID: new(users[0].ID),
Hostinfo: &tailcfg.Hostinfo{},
},
{
ID: 2,
Hostname: "router",
IPv4: ap("100.64.0.2"),
User: new(users[0]),
UserID: new(users[0].ID),
Tags: []string{"tag:router"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{mp("10.33.0.0/16")},
},
ApprovedRoutes: []netip.Prefix{mp("10.33.0.0/16")},
},
}
pol := `{
"tagOwners": {
"tag:router": ["user1@"]
},
"grants": [{
"src": ["user1@"],
"dst": ["10.33.5.0/24"],
"ip": ["*"],
"via": ["tag:router"]
}]
}`
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
require.NoError(t, err)
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
require.Equal(t, []netip.Prefix{mp("10.33.0.0/16")}, result.Include,
"Include must hold the advertised route /16 that covers the narrower grant dst /24")
require.Empty(t, result.Exclude)
})
t.Run("disjoint_dst_emits_nothing", func(t *testing.T) {
t.Parallel()
nodes := types.Nodes{
{
ID: 1,
Hostname: "viewer",
IPv4: ap("100.64.0.1"),
User: new(users[0]),
UserID: new(users[0].ID),
Hostinfo: &tailcfg.Hostinfo{},
},
{
ID: 2,
Hostname: "router",
IPv4: ap("100.64.0.2"),
User: new(users[0]),
UserID: new(users[0].ID),
Tags: []string{"tag:router"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{mp("10.33.0.0/16")},
},
ApprovedRoutes: []netip.Prefix{mp("10.33.0.0/16")},
},
}
pol := `{
"tagOwners": {
"tag:router": ["user1@"]
},
"grants": [{
"src": ["user1@"],
"dst": ["192.168.0.0/16"],
"ip": ["*"],
"via": ["tag:router"]
}]
}`
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
require.NoError(t, err)
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
require.Empty(t, result.Include,
"disjoint dst must produce nothing — the via gate requires advertised-route overlap")
require.Empty(t, result.Exclude)
})
}
// TestBuildPeerMap_AutogroupInternetMakesExitNodeVisible reproduces