From 85c754a1acb10a2bd6f2dea3c2a03f463e7b14e0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 30 Sep 2026 15:01:57 +0000 Subject: [PATCH] policy/v2: resolve unregistered users to an empty set Callers bailing on the error dropped a whole group for one missing member: via routes, SSH check, app grants, nodeAttrs. Fixes #3513 --- hscontrol/policy/v2/filter_test.go | 19 ++++++ hscontrol/policy/v2/nodeattrs_test.go | 9 +++ hscontrol/policy/v2/policy_test.go | 94 +++++++++++++++++++++++++++ hscontrol/policy/v2/sshtest.go | 8 +++ hscontrol/policy/v2/test.go | 9 +++ hscontrol/policy/v2/types.go | 7 ++ hscontrol/policy/v2/types_test.go | 19 +++++- 7 files changed, 164 insertions(+), 1 deletion(-) diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index 15a757f38..83f89b991 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -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(`{ diff --git a/hscontrol/policy/v2/nodeattrs_test.go b/hscontrol/policy/v2/nodeattrs_test.go index e6d3a50aa..4a70164a5 100644 --- a/hscontrol/policy/v2/nodeattrs_test.go +++ b/hscontrol/policy/v2/nodeattrs_test.go @@ -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"]}]`, diff --git a/hscontrol/policy/v2/policy_test.go b/hscontrol/policy/v2/policy_test.go index f9c82807d..5bf367f5f 100644 --- a/hscontrol/policy/v2/policy_test.go +++ b/hscontrol/policy/v2/policy_test.go @@ -2352,6 +2352,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 @@ -2716,6 +2785,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 { diff --git a/hscontrol/policy/v2/sshtest.go b/hscontrol/policy/v2/sshtest.go index 89d339c76..a8b1421db 100644 --- a/hscontrol/policy/v2/sshtest.go +++ b/hscontrol/policy/v2/sshtest.go @@ -379,6 +379,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) diff --git a/hscontrol/policy/v2/test.go b/hscontrol/policy/v2/test.go index da79f7c72..3a1231094 100644 --- a/hscontrol/policy/v2/test.go +++ b/hscontrol/policy/v2/test.go @@ -325,6 +325,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) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index a672e7540..7d877b1be 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -429,6 +429,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) } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 25c5e635e..eeac6f8cc 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -2584,6 +2584,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{ @@ -2592,7 +2593,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",