policy/v2: evaluate sshTests sources through the nodes they cover

A src was expanded to every address it names, so "*" or ::/0 never
finished. Principals are node addresses: keep those plus one stand-in.
This commit is contained in:
Kristoffer Dalby committed 2026-11-02 19:15:40 +01:00
1 parent 2c7a9a22f6
commit 6fd3dbd0a2
2 files changed
+141 -1

No files matched your search

+36 -1
View File
@@ -385,6 +385,12 @@ func appendUserDst(m map[string][]string, user, dst string) map[string][]string
// resolveSSHTestSource returns the src's principal addresses and, for
// user-shaped sources, the user ID (so autogroup:self can scope to it).
// [Tag], [Host], and IP sources return userID 0.
//
// SSH principals are always node addresses, so a src is represented by
// the node addresses it covers plus one address standing in for all
// the others, which no rule can match. The result is bounded by the
// node count; a CIDR, `*`, or autogroup:internet src is never walked
// address by address.
func resolveSSHTestSource(
src Alias,
pol *Policy,
@@ -412,7 +418,36 @@ func resolveSSHTestSource(
return nil, 0, nil
}
out := slices.Collect(addrs.Iter())
var (
out []netip.Addr
rest netipx.IPSetBuilder
)
for _, p := range addrs.Prefixes() {
rest.AddPrefix(p)
}
for _, n := range nodes.All() {
for _, ip := range n.IPs() {
if addrs.Contains(ip) {
out = append(out, ip)
rest.Remove(ip)
}
}
}
restSet, err := rest.IPSet()
if err != nil {
return nil, 0, fmt.Errorf("resolving: %w", err)
}
if ranges := restSet.Ranges(); len(ranges) > 0 {
out = append(out, ranges[0].From())
}
// Address order, as a full walk has: the first failing address
// decides how a check failure is rendered.
slices.SortFunc(out, netip.Addr.Compare)
var userID uint
+105
View File
@@ -2,6 +2,7 @@ package v2
import (
"encoding/json"
"net/netip"
"strings"
"testing"
@@ -747,6 +748,88 @@ func TestRunSSHTests(t *testing.T) {
wantPass: false,
wantErrSub: []string{"autogroup:self", "resolved to no nodes"},
},
{
// A src naming address space is evaluated through the node
// addresses inside it, never address by address: ::/0
// holds alice's IPv6, which the rule allows.
name: "src-cidr-deny-fails-on-covered-node",
policy: `{
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["alice@headscale.net"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [{
"src": "::/0",
"dst": ["tag:server"],
"deny": ["root"]
}]
}`,
wantPass: false,
wantErrSub: []string{"::/0", "expected DENIED, got ALLOWED"},
},
{
name: "src-address-space-deny-pass",
policy: `{
"hosts": { "lan": "10.0.0.0/8" },
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["alice@headscale.net"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [
{ "src": "*", "dst": ["tag:server"], "deny": ["ubuntu"] },
{ "src": "autogroup:danger-all", "dst": ["tag:server"], "deny": ["ubuntu"] },
{ "src": "autogroup:internet", "dst": ["tag:server"], "deny": ["root"] },
{ "src": "lan", "dst": ["tag:server"], "deny": ["root"] }
]
}`,
wantPass: true,
},
{
// Every node is allowed, but `*` also covers addresses no
// node holds, and all src addresses must agree.
name: "src-wildcard-accept-fails-on-unassigned-address",
policy: `{
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["autogroup:member", "autogroup:tagged"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [{
"src": "*",
"dst": ["tag:server"],
"accept": ["root"]
}]
}`,
wantPass: false,
wantErrSub: []string{"expected ALLOWED, got DENIED"},
},
{
// 100.64.0.4/31 is exactly alice-tablet and thor-laptop.
name: "src-cidr-fully-assigned-accept-pass",
policy: `{
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["autogroup:member"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [{
"src": "100.64.0.4/31",
"dst": ["tag:server"],
"accept": ["root"]
}]
}`,
wantPass: true,
},
}
for _, tt := range tests {
@@ -772,6 +855,28 @@ func TestRunSSHTests(t *testing.T) {
}
}
// TestResolveSSHTestSourceIsBoundedByNodes pins that a src naming address
// space resolves to the node addresses it covers plus one address standing
// in for the rest, not to every address in it.
func TestResolveSSHTestSourceIsBoundedByNodes(t *testing.T) {
users := sshTestUsers()
nodes := sshTestNodes(users)
src := Prefix(netip.MustParsePrefix("100.64.0.0/16"))
got, _, err := resolveSSHTestSource(&src, nil, users, nodes.ViewSlice())
require.NoError(t, err)
want := make([]netip.Addr, 0, 1+len(nodes))
want = append(want, netip.MustParseAddr("100.64.0.0"))
for _, n := range nodes {
want = append(want, *n.IPv4)
}
require.Equal(t, want, got)
}
// TestRunSSHTestsBothTestsPassSSHTestsFail captures the distinction the
// caller cares about: a passing ACL `tests` block plus a failing
// `sshTests` block returns errSSHPolicyTestsFailed and NOT