policy/v2: stop ViaRoutesForPeer appending into pm.pol.Grants

Updates #3515

(cherry picked from commit 5cadfa16eb)
This commit is contained in:
Kristoffer Dalby committed 2026-10-13 14:35:52 +02:00
1 parent d03d3a0231
commit 6ed6dd8db5
2 files changed
+53 -1

No files matched your search

+3 -1
View File
@@ -1241,7 +1241,9 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
return result
}
grants := pm.pol.Grants
// 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)
for _, acl := range pm.pol.ACLs {
grants = append(grants, aclToGrants(acl)...)
}
@@ -2,6 +2,7 @@ package v2
import (
"fmt"
"slices"
"sync"
"testing"
@@ -101,3 +102,52 @@ func TestPolicyManagerConcurrentReads(t *testing.T) {
wg.Wait()
}
// TestViaRoutesForPeerDoesNotWritePolicyGrants guards against
// ViaRoutesForPeer appending ACL-derived grants into the backing array of
// pm.pol.Grants. Readers hold only the shared RLock, so any spare capacity
// in that slice becomes a write/write race between concurrent callers.
func TestViaRoutesForPeerDoesNotWritePolicyGrants(t *testing.T) {
users := types.Users{{Name: "user1", Email: "user1@headscale.net"}}
a := node("a", "100.64.0.1", "fd7a:115c:a1e0::1", users[0])
a.ID = 1
b := node("b", "100.64.0.2", "fd7a:115c:a1e0::2", users[0])
b.ID = 2
nodes := types.Nodes{a, b}
// The via grant keeps ViaRoutesForPeer past its no-via early return.
policy := `{
"tagOwners": {"tag:router": ["user1@headscale.net"]},
"grants": [
{"src": ["*"], "dst": ["*"], "ip": ["tcp:22"]},
{"src": ["*"], "dst": ["10.0.0.0/8"], "ip": ["*"], "via": ["tag:router"]}
],
"acls": [
{"action": "accept", "src": ["*"], "dst": ["*:*"]}
]
}`
pm, err := NewPolicyManager([]byte(policy), users, nodes.ViewSlice())
require.NoError(t, err)
// Guarantee spare capacity instead of relying on the JSON decoder's
// growth policy.
pm.pol.Grants = slices.Grow(pm.pol.Grants, 4)
spare := pm.pol.Grants[len(pm.pol.Grants):cap(pm.pol.Grants)]
var wg sync.WaitGroup
for range 8 {
wg.Go(func() {
for range 50 {
pm.ViaRoutesForPeer(a.View(), b.View())
}
})
}
wg.Wait()
for i, g := range spare {
assert.Zero(t, g, "spare slot %d of pm.pol.Grants was written", i)
}
}