mirror of
https://github.com/juanfont/headscale.git
synced 2026-10-10 08:40:07 +09:00
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
This commit is contained in:
committed by
Kristoffer Dalby
parent
545322aec7
commit
85c754a1ac
@@ -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(`{
|
||||
|
||||
@@ -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"]}]`,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user