policy/v2: bound via route steering by the peer's routes

Include and Exclude held one entry per overlapping dst, and the election
walked them per dst. Keep one entry per route; a peer without routes
returns at once.
This commit is contained in:
Kristoffer Dalby committed 2026-10-28 18:28:21 +00:00
1 parent c49217fcd3
commit cbd2eef3de
2 files changed
+113 -30

No files matched your search

+44 -30
View File
@@ -1440,6 +1440,11 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
return result
}
// A peer without routes has nothing to steer.
if len(peer.SubnetRoutes()) == 0 && !peer.IsExitNode() {
return result
}
// Clip so the appends below allocate: callers share pm.pol under RLock,
// and writing into its spare capacity races between them.
grants := slices.Clip(pm.pol.Grants)
@@ -1504,11 +1509,13 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
var matchedPrefixes []netip.Prefix
for _, dstPrefix := range resolvedDstPrefixes[i] {
for _, route := range peerSubnetRoutes {
if dstPrefix.Overlaps(route) {
matchedPrefixes = append(matchedPrefixes, route)
}
// Emit each route once, however many dst prefixes overlap it:
// the passes below walk Include and Exclude per grant, so their
// size must be bounded by the peer's routes, not by the grant's
// destinations.
for _, route := range peerSubnetRoutes {
if slices.ContainsFunc(resolvedDstPrefixes[i], route.Overlaps) {
matchedPrefixes = append(matchedPrefixes, route)
}
}
@@ -1536,10 +1543,16 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
}
}
if peerHasVia {
result.Include = append(result.Include, matchedPrefixes...)
} else {
result.Exclude = append(result.Exclude, matchedPrefixes...)
// Several grants, or repeated announcements, can match the same
// route; keep one entry per route.
for _, p := range matchedPrefixes {
if peerHasVia {
if !slices.Contains(result.Include, p) {
result.Include = append(result.Include, p)
}
} else if !slices.Contains(result.Exclude, p) {
result.Exclude = append(result.Exclude, p)
}
}
}
@@ -1579,33 +1592,34 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
// 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
}
// dst may cover multiple distinct routes. The election
// does not depend on which dst matched, so each included
// route is looked at once per grant.
for _, included := range slices.Clone(result.Include) {
if !slices.ContainsFunc(resolvedDstPrefixes[i], included.Overlaps) {
continue
}
var viaPrimaryID types.NodeID
var viaPrimaryID types.NodeID
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()
}
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 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)
}
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)
}
}
}
+69
View File
@@ -2571,6 +2571,75 @@ func TestViaRoutesForPeer(t *testing.T) {
require.Empty(t, result.Exclude)
}
})
// A route is steered once, however many dst prefixes or grants overlap
// it and however often the peer announces it. Otherwise Include grows
// with the policy instead of the peer's routes, and the election pass,
// which walks Include per dst, goes quadratic under the policy lock.
t.Run("route_steered_once_per_peer", 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.0.0.0/24"), mp("10.0.0.0/24")},
},
ApprovedRoutes: []netip.Prefix{mp("10.0.0.0/24")},
},
{
ID: 3,
Hostname: "other",
IPv4: ap("100.64.0.3"),
User: new(users[0]),
UserID: new(users[0].ID),
Tags: []string{"tag:other"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{mp("10.0.0.0/24"), mp("10.0.0.0/24")},
},
ApprovedRoutes: []netip.Prefix{mp("10.0.0.0/24")},
},
}
grant := `{
"src": ["user1@"],
"dst": ["10.0.0.0/24", "10.0.0.0/24", "10.0.0.0/25", "10.0.0.7/32"],
"ip": ["*"],
"via": ["tag:router"]
}`
pol := `{
"tagOwners": {
"tag:router": ["user1@"],
"tag:other": ["user1@"]
},
"grants": [` + grant + `, ` + grant + `]
}`
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.0.0.0/24")}, result.Include)
require.Empty(t, result.Exclude)
result = pm.ViaRoutesForPeer(nodes[0].View(), nodes[2].View())
require.Empty(t, result.Include)
require.Equal(t, []netip.Prefix{mp("10.0.0.0/24")}, result.Exclude)
})
}
// TestBuildPeerMap_AutogroupInternetMakesExitNodeVisible reproduces