policy/v2: resolve unregistered users to an empty set

Updates #3513, #3516

(cherry picked from commit 2dcd5c876e)
This commit is contained in:
Kristoffer Dalby committed 2026-10-13 14:35:52 +02:00
1 parent bd8b8ce757
commit f079cf7a59
10 files changed
+466 -6

No files matched your search

+19
View File
@@ -2809,6 +2809,25 @@ func TestSSHCheckParams(t *testing.T) {
wantPeriod: 2 * time.Hour,
wantOK: true,
},
{
// One unregistered member must not void the group (#3513).
name: "group src with unregistered member",
policy: []byte(`{
"groups": {"group:ops": ["user2@", "ghost@"]},
"tagOwners": {"tag:server": ["user1@"]},
"ssh": [{
"action": "check",
"checkPeriod": "2h",
"src": ["group:ops"],
"dst": ["tag:server"],
"users": ["autogroup:nonroot"]
}]
}`),
srcID: types.NodeID(2),
dstID: types.NodeID(3),
wantPeriod: 2 * time.Hour,
wantOK: true,
},
{
name: "default period when checkPeriod omitted",
policy: []byte(`{
+9
View File
@@ -92,6 +92,15 @@ func TestNodeAttrsCompile(t *testing.T) {
extra string
want map[types.NodeID]tailcfg.NodeCapMap
}{
{
// One unregistered member must not reject the policy (#3513).
name: "group target with unregistered member hits registered members",
extra: `"groups": {"group:g": ["alice@example.com", "ghost@example.com"]},
"nodeAttrs": [{"target": ["group:g"], "attr": ["randomize-client-port"]}]`,
want: map[types.NodeID]tailcfg.NodeCapMap{
1: capMap("randomize-client-port"),
},
},
{
name: "wildcard target hits every node",
extra: `"nodeAttrs": [{"target": ["*"], "attr": ["randomize-client-port"]}]`,
+337 -5
View File
@@ -87,14 +87,14 @@ func TestPolicyManager(t *testing.T) {
}
// TestSetPolicyRejectedKeepsLiveFilter pins that a policy SetPolicy rejects
// does not take effect. A nodeAttrs target naming no user passes validation
// but fails to compile after the filter is already compiled; keeping that
// does not take effect. A nodeAttrs target naming an ambiguous user passes
// validation but fails to compile after the filter is already compiled; keeping that
// filter would run the rejected policy while the stored one is unchanged.
func TestSetPolicyRejectedKeepsLiveFilter(t *testing.T) {
// IDs are assigned after construction so the test also builds where
// types.User embeds gorm.Model.
users := types.Users{{Name: "user1"}, {Name: "user2"}}
users[0].ID, users[1].ID = 1, 2
users := types.Users{{Name: "user1"}, {Name: "user2"}, {Name: "dup"}, {Name: "dup"}}
users[0].ID, users[1].ID, users[2].ID, users[3].ID = 1, 2, 3, 4
nodes := types.Nodes{
node("n1", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]),
node("n2", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
@@ -112,7 +112,7 @@ func TestSetPolicyRejectedKeepsLiveFilter(t *testing.T) {
_, err = pm.SetPolicy([]byte(`{
"acls": [{"action": "accept", "src": ["*"], "dst": ["*:*"]}],
"nodeAttrs": [{"target": ["ghost@"], "attr": ["randomize-client-port"]}]
"nodeAttrs": [{"target": ["dup@"], "attr": ["randomize-client-port"]}]
}`))
require.Error(t, err)
@@ -2351,6 +2351,75 @@ func TestViaRoutesForPeer(t *testing.T) {
"disjoint dst must produce nothing — the via gate requires advertised-route overlap")
require.Empty(t, result.Exclude)
})
// juanfont/headscale#3513: a group member must get via-steered exit
// routes even when another member of the group has not registered.
// The group resolves to the registered members' IPs plus an error
// for the missing one; the error must not drop the resolved IPs.
t.Run("group_src_with_unregistered_member_autogroup_internet", 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: "exit-node",
IPv4: ap("100.64.0.2"),
User: new(users[0]),
UserID: new(users[0].ID),
Tags: []string{"tag:exit"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
mp("0.0.0.0/0"),
mp("::/0"),
},
},
ApprovedRoutes: []netip.Prefix{
mp("0.0.0.0/0"),
mp("::/0"),
},
},
}
for _, members := range []string{
`["user1@"]`,
`["user1@", "unregistered@"]`,
} {
pol := `{
"groups": {"group:develop": ` + members + `},
"tagOwners": {"tag:exit": ["user1@"]},
"grants": [{
"src": ["group:develop"],
"dst": ["autogroup:internet"],
"ip": ["*"],
"via": ["tag:exit"]
}]
}`
pm, err := NewPolicyManager([]byte(pol), users, nodes.ViewSlice())
require.NoError(t, err)
// The exit node's filter already admits the viewer: the
// compile path keeps a group's partial resolution.
rules, err := pm.FilterForNode(nodes[1].View())
require.NoError(t, err)
require.NotEmpty(t, rules, "members=%s: exit node must accept viewer traffic", members)
result := pm.ViaRoutesForPeer(nodes[0].View(), nodes[1].View())
require.Containsf(t, result.Include, mp("0.0.0.0/0"),
"members=%s: viewer in group must get exit routes from via-tagged exit node", members)
require.Containsf(t, result.Include, mp("::/0"),
"members=%s: viewer in group must get exit routes from via-tagged exit node", members)
require.Empty(t, result.Exclude)
}
})
}
// TestBuildPeerMap_AutogroupInternetMakesExitNodeVisible reproduces
@@ -2435,6 +2504,244 @@ func TestNewPolicyManager_UnknownUsernameTolerant(t *testing.T) {
require.NoError(t, err, "missing-user references must not block policy load (#2863)")
}
// TestUnregisteredUsersAreNoOp pins, at every policy site that takes users,
// that naming a user who has not registered changes nothing (#3513).
// Tailscale accepts such policies; the user gains access once they join and
// loses it when deleted. Swapping in a registered user proves each site has
// an observable effect, so an equal result is not vacuous.
func TestUnregisteredUsersAreNoOp(t *testing.T) {
t.Parallel()
// IDs are assigned after construction so the test also builds where
// types.User embeds gorm.Model.
all := types.Users{{Name: "alice"}, {Name: "bob"}, {Name: "ghost"}}
all[0].ID, all[1].ID, all[2].ID = 1, 2, 3
registered := all[:2:2]
exitRoutes := []netip.Prefix{mp("0.0.0.0/0"), mp("::/0")}
userRoutes := []netip.Prefix{mp("10.44.0.0/16"), mp("0.0.0.0/0"), mp("::/0")}
// mkNodes adds ghost's node, advertising routes for autoApprovers to
// act on, when users includes ghost.
mkNodes := func(users types.Users) types.Nodes {
withHostinfo := func(n *types.Node, tags []string, routes, approved []netip.Prefix) *types.Node {
n.Tags = tags
n.Hostinfo = &tailcfg.Hostinfo{RoutableIPs: routes}
n.ApprovedRoutes = approved
return n
}
nodes := types.Nodes{
withHostinfo(node("alice", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]), nil, nil, nil),
withHostinfo(node("alice-router", "100.64.0.2", "fd7a:115c:a1e0::2", users[0]), nil, userRoutes, nil),
withHostinfo(node("bob", "100.64.0.3", "fd7a:115c:a1e0::3", users[1]), nil, nil, nil),
withHostinfo(node("exit", "100.64.0.4", "fd7a:115c:a1e0::4", users[1]),
[]string{"tag:exit"}, exitRoutes, exitRoutes),
withHostinfo(node("router", "100.64.0.5", "fd7a:115c:a1e0::5", users[1]),
[]string{"tag:router"}, []netip.Prefix{mp("10.33.0.0/16")}, []netip.Prefix{mp("10.33.0.0/16")}),
withHostinfo(node("server", "100.64.0.6", "fd7a:115c:a1e0::6", users[1]),
[]string{"tag:server"}, nil, nil),
}
if len(users) > 2 {
nodes = append(nodes, withHostinfo(
node("ghost", "100.64.0.7", "fd7a:115c:a1e0::7", users[2]), nil, userRoutes, nil))
}
for i, n := range nodes {
n.ID = types.NodeID(i + 1) //nolint:gosec
}
return nodes
}
// Each body receives the member list; group cases put it in group:g,
// list cases name the users directly.
tests := []struct {
name string
devOwners string
body func(members string) string
}{
{name: "acl src group", body: func(string) string {
return `"acls": [{"action": "accept", "src": ["group:g"], "dst": ["tag:server:22"]}]`
}},
{name: "acl src list", body: func(m string) string {
return `"acls": [{"action": "accept", "src": [` + m + `], "dst": ["tag:server:22"]}]`
}},
{name: "acl dst group", body: func(string) string {
return `"acls": [{"action": "accept", "src": ["tag:server"], "dst": ["group:g:*"]}]`
}},
{name: "acl autogroup:self", body: func(string) string {
return `"acls": [{"action": "accept", "src": ["group:g"], "dst": ["autogroup:self:*"]}]`
}},
{name: "grant ip group", body: func(string) string {
return `"grants": [{"src": ["group:g"], "dst": ["tag:server"], "ip": ["tcp:443"]}]`
}},
{name: "grant via exit group", body: func(string) string {
return `"grants": [{"src": ["group:g"], "dst": ["autogroup:internet"], "ip": ["*"], "via": ["tag:exit"]}]`
}},
{name: "grant via exit list", body: func(m string) string {
return `"grants": [{"src": [` + m + `], "dst": ["autogroup:internet"], "ip": ["*"], "via": ["tag:exit"]}]`
}},
{name: "grant via subnet group", body: func(string) string {
return `"grants": [{"src": ["group:g"], "dst": ["10.33.0.0/16"], "ip": ["*"], "via": ["tag:router"]}]`
}},
{name: "grant app dst group", body: func(string) string {
return `"grants": [{"src": ["tag:server"], "dst": ["group:g"], "app": {"tailscale.com/cap/relay": [{}]}}]`
}},
{name: "ssh accept group", body: func(string) string {
return `"ssh": [{"action": "accept", "src": ["group:g"], "dst": ["tag:server"], "users": ["root"]}]`
}},
{name: "ssh accept list", body: func(m string) string {
return `"ssh": [{"action": "accept", "src": [` + m + `], "dst": ["tag:server"], "users": ["root"]}]`
}},
{name: "ssh autogroup:self", body: func(string) string {
return `"ssh": [{"action": "accept", "src": ["group:g"], "dst": ["autogroup:self"], "users": ["root"]}]`
}},
{name: "ssh check group", body: func(string) string {
return `"ssh": [{"action": "check", "checkPeriod": "2h", "src": ["group:g"], "dst": ["tag:server"], "users": ["root"]}]`
}},
{name: "tagOwners group", devOwners: `"group:g"`, body: func(string) string { return "" }},
{name: "tagOwners list", devOwners: "LIST", body: func(string) string { return "" }},
{name: "autoApprovers routes group", body: func(string) string {
return `"autoApprovers": {"routes": {"10.44.0.0/16": ["group:g"]}}`
}},
{name: "autoApprovers exitNode group", body: func(string) string {
return `"autoApprovers": {"exitNode": ["group:g"]}`
}},
{name: "nodeAttrs group", body: func(string) string {
return `"nodeAttrs": [{"target": ["group:g"], "attr": ["randomize-client-port"]}]`
}},
{name: "nodeAttrs list", body: func(m string) string {
return `"nodeAttrs": [{"target": [` + m + `], "attr": ["randomize-client-port"]}]`
}},
}
type checkParams struct {
Period time.Duration
OK bool
}
// snapshot gathers every per-node and per-pair result a policy drives.
snapshot := func(t *testing.T, pm *PolicyManager, nodes types.Nodes) map[string]any {
t.Helper()
got := map[string]any{}
got["filter"], _ = pm.Filter()
peers := pm.BuildPeerMap(nodes.ViewSlice())
for _, n := range nodes {
nv := n.View()
rules, err := pm.FilterForNode(nv)
require.NoError(t, err)
ssh, err := pm.SSHPolicy("", nv)
require.NoError(t, err)
ids := slices.Clone(peers[n.ID])
slices.Sort(ids)
got[n.Hostname+" filter"] = rules
got[n.Hostname+" ssh"] = ssh
got[n.Hostname+" caps"] = pm.NodeCapMap(n.ID)
got[n.Hostname+" peers"] = ids
got[n.Hostname+" tag:dev"] = pm.NodeCanHaveTag(nv, "tag:dev")
for _, r := range n.Hostinfo.RoutableIPs {
got[n.Hostname+" approves "+r.String()] = pm.NodeCanApproveRoute(nv, r)
}
for _, p := range nodes {
if p.ID == n.ID {
continue
}
period, ok := pm.SSHCheckParams(n.ID, p.ID)
got[n.Hostname+"->"+p.Hostname+" via"] = pm.ViaRoutesForPeer(nv, p.View())
got[n.Hostname+"->"+p.Hostname+" check"] = checkParams{period, ok}
}
}
return got
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
render := func(members string) []byte {
devOwners := tt.devOwners
switch devOwners {
case "":
devOwners = `"bob@"`
case "LIST":
devOwners = members
}
return []byte(`{
"groups": {"group:g": [` + members + `]},
"tagOwners": {
"tag:exit": ["bob@"], "tag:router": ["bob@"], "tag:server": ["bob@"],
"tag:dev": [` + devOwners + `],
},
` + tt.body(members) + `
}`)
}
load := func(members string, users types.Users, nodes types.Nodes) *PolicyManager {
t.Helper()
pm, err := NewPolicyManager(render(members), users, nodes.ViewSlice())
require.NoError(t, err, "a policy naming an unregistered user must load")
return pm
}
diff := func(want, got map[string]any) string {
return cmp.Diff(want, got, cmpOptions()...)
}
const (
withGhost = `"alice@", "ghost@"`
alice = `"alice@"`
)
nodes := mkNodes(registered)
want := snapshot(t, load(alice, registered, nodes), nodes)
pm := load(withGhost, registered, nodes)
require.Empty(t, diff(want, snapshot(t, pm, nodes)),
"an unregistered user must change nothing")
require.NotEmpty(t, diff(want, snapshot(t, load(`"bob@"`, registered, nodes), nodes)),
"the site must react to its members")
// ghost registers, then their node joins.
grown := mkNodes(all)
_, _, err := pm.SetUsers(all)
require.NoError(t, err)
_, err = pm.SetNodes(grown.ViewSlice())
require.NoError(t, err)
joined := snapshot(t, pm, grown)
require.Empty(t, diff(snapshot(t, load(withGhost, all, grown), grown), joined),
"after joining, ghost must match a fresh load")
require.NotEmpty(t, diff(snapshot(t, load(alice, all, grown), grown), joined),
"after joining, ghost must gain access")
// ghost's node leaves, then ghost is deleted.
_, err = pm.SetNodes(nodes.ViewSlice())
require.NoError(t, err)
_, _, err = pm.SetUsers(registered)
require.NoError(t, err)
require.Empty(t, diff(want, snapshot(t, pm, nodes)),
"after deletion, ghost must leave no trace")
})
}
}
// Rejected SetPolicy must keep the previous policy intact.
func TestSetPolicy_DuplicateUsername(t *testing.T) {
users := types.Users{
@@ -2715,6 +3022,31 @@ func TestPeerRelayGrantMakesRelayVisible(t *testing.T) {
srcIDs: []types.NodeID{1},
relayID: 3,
},
{
// One unregistered member must not drop the cap grant (#3513).
name: "tag src, group dst with unregistered member",
nodes: types.Nodes{
taggedNode(1, "client-a", "100.64.0.1", "fd7a:115c:a1e0::1", "tag:client"),
userNode(3, "peer-relay", "100.64.0.3", "fd7a:115c:a1e0::3"),
},
policy: `{
"groups": {
"group:relays": ["alice@headscale.net", "ghost@headscale.net"]
},
"tagOwners": {
"tag:client": ["tagowner@headscale.net"]
},
"grants": [
{
"src": ["tag:client"],
"dst": ["group:relays"],
"app": {"tailscale.com/cap/relay": []}
}
]
}`,
srcIDs: []types.NodeID{1},
relayID: 3,
},
}
for _, tt := range tests {
+8
View File
@@ -400,6 +400,14 @@ func resolveSSHTestSource(
return nil, 0, nil
}
// Mirrors resolveTestSource: an unregistered user fails the test.
if u, ok := src.(*Username); ok {
_, err := u.resolveUser(users)
if err != nil {
return nil, 0, fmt.Errorf("resolving: %w", err)
}
}
addrs, err := src.Resolve(pol, users, nodes)
if err != nil {
return nil, 0, fmt.Errorf("resolving: %w", err)
+40
View File
@@ -134,6 +134,46 @@ func TestRunSSHTests(t *testing.T) {
}`,
wantPass: true,
},
{
// Rules tolerate unregistered users; a test naming one fails.
name: "unknown-src-user",
policy: `{
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["alice@headscale.net"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [{
"src": "ghost@headscale.net",
"dst": ["tag:server"],
"accept": ["root"]
}]
}`,
wantPass: false,
wantErrSub: []string{"ghost@headscale.net", "failed to resolve source"},
},
{
// A group tolerates unregistered members like any rule (#3513).
name: "group-src-with-unregistered-member",
policy: `{
"groups": {"group:eng": ["alice@headscale.net", "ghost@headscale.net"]},
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"ssh": [{
"action": "accept",
"src": ["group:eng"],
"dst": ["tag:server"],
"users": ["root"]
}],
"sshTests": [{
"src": "group:eng",
"dst": ["tag:server"],
"accept": ["root"]
}]
}`,
wantPass: true,
},
{
name: "accept-fail-no-rule",
policy: `{
+9
View File
@@ -331,6 +331,15 @@ func resolveTestSource(src string, pol *Policy, users []types.User, nodes views.
return nil, fmt.Errorf("invalid alias: %w", err)
}
// Rules tolerate unregistered users; a test naming one must still
// fail (SaaS: policytest-src-unknown-user-email).
if u, ok := alias.(*Username); ok {
_, err := u.resolveUser(users)
if err != nil {
return nil, fmt.Errorf("resolving: %w", err)
}
}
addrs, err := alias.Resolve(pol, users, nodes)
if err != nil {
return nil, fmt.Errorf("resolving: %w", err)
+18
View File
@@ -138,6 +138,24 @@ func TestRunTests(t *testing.T) {
wantErrSub: []string{"ghost@headscale.net", "failed to resolve source"},
wantNoErrIs: errPolicyTestsFailed,
},
{
// A group tolerates unregistered members like any rule (#3513).
name: "group-src-with-unregistered-member",
policy: `{
"groups": {"group:eng": ["alice@headscale.net", "ghost@headscale.net"]},
"tagOwners": { "tag:server": ["alice@headscale.net"] },
"acls": [{
"action": "accept",
"src": ["group:eng"],
"dst": ["tag:server:22"]
}],
"tests": [{
"src": "group:eng",
"accept": ["tag:server:22"]
}]
}`,
wantPass: true,
},
// "malformed-dst-missing-port" used to live here; structural
// shape errors are now caught at parse by validateTests, so
// RunTests no longer sees them. The parse-side behaviour is
+7
View File
@@ -450,6 +450,13 @@ func (u *Username) resolve(_ *Policy, users types.Users, nodes views.Slice[types
)
user, err := u.resolveUser(users)
if errors.Is(err, ErrUserNotFound) {
// A policy may name users before they register (#2863). Report it
// as an empty set, not an error: callers that bail on error would
// otherwise drop a whole group for one missing member (#3513).
return &netipx.IPSet{}, nil
}
if err != nil {
errs = append(errs, err)
}
+18 -1
View File
@@ -2585,6 +2585,7 @@ func TestResolvePolicy(t *testing.T) {
want: util.TheInternet().Prefixes(),
},
{
// Unregistered users resolve to nothing, without an error (#3513).
name: "invalid-username",
toResolve: new(Username("invaliduser@")),
nodes: types.Nodes{
@@ -2593,7 +2594,23 @@ func TestResolvePolicy(t *testing.T) {
IPv4: ap("100.100.101.103"),
},
},
wantErr: `user not found: token "invaliduser@"`,
},
{
// One unregistered member must not void the group (#3513).
name: "group-with-unregistered-member",
toResolve: new(Group("group:testgroup")),
nodes: types.Nodes{
{
User: new(users["groupuser"]),
IPv4: ap("100.100.101.203"),
},
},
pol: &Policy{
Groups: Groups{
"group:testgroup": Usernames{"groupuser@", "invaliduser@"},
},
},
want: []netip.Prefix{mp("100.100.101.203/32")},
},
{
name: "invalid-tag",
+1
View File
@@ -32,6 +32,7 @@ var viaCompatTests = []struct {
id string
desc string
}{
{"grant-v13", "group src steers subnet via router to members only"},
{"via-grant-v29", "crossed subnet steering: group-a via router-a, group-b via router-b"},
{"via-grant-v30", "crossed mixed: subnet via router-a/b, exit via exit-b/a"},
{"via-grant-v31", "peer connectivity + via exit A/B steering"},