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

Readers share pm.pol under RLock; appending ACL grants into its spare
capacity raced between concurrent callers.
This commit is contained in:
Kristoffer Dalby
2026-09-30 15:13:18 +00:00
committed by Kristoffer Dalby
parent a9cc142ebe
commit a99c8a030c
2 changed files with 50 additions and 1 deletions
+3 -1
View File
@@ -1321,7 +1321,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"
@@ -100,3 +101,49 @@ 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}
policy := `{
"grants": [
{"src": ["*"], "dst": ["*"], "ip": ["tcp:22"]}
],
"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)
}
}