state: give another user's key on a tagged machine a new device

Only a key that speaks for a tagged node re-authenticates it: a tagged
key, or one of the user it was tagged from. Another user's key or login
adds that user's own device, or only wakes the node under its node key.

Fixes ANT-2026-AWGRB7O1
This commit is contained in:
Kristoffer Dalby committed 2026-10-29 14:00:02 +01:00
1 parent e67ef8619a
commit e1de83cbdf
9 files changed
+455 -144

No files matched your search

+1
View File
@@ -155,6 +155,7 @@ the ACME DNS-01 challenge through a built-in DNS provider: `acmedns`,
- Add the v2 `acl/validate` endpoint, which Terraform provider 0.29.3 and newer call to check `tailscale_acl` at plan time; before it, those releases failed with `invalid character 'p' after top-level value`
- A route that is a single address inside the node ranges, in either spelling, is approved and listed but never served; wider routes and `::ffff:` prefixes are still served
- Require a valid pre-auth key to re-register an expired tagged node
- Another user's key or login on a tagged machine now creates a new device for that user instead of re-authenticating the tagged node
## 0.29.7 (202x-xx-xx)
+12 -9
View File
@@ -4120,9 +4120,10 @@ func TestDeletedPreAuthKeyNotRecreatedOnNodeUpdate(t *testing.T) {
t.Log("SUCCESS: PreAuthKey remained deleted after node update")
}
// TestTaggedNodeWithoutUserToDifferentUser tests that a node registered with a
// tags-only PreAuthKey (no user) can be re-registered to a different user
// without panicking. This reproduces the issue reported in #3038.
// TestTaggedNodeWithoutUserToDifferentUser tests that a user's login on the
// machine of a node registered with a tags-only PreAuthKey (no user) works
// without panicking (#3038). No user made the key, so the login gets a device
// of its own and the tagged node stays.
func TestTaggedNodeWithoutUserToDifferentUser(t *testing.T) {
t.Parallel()
@@ -4195,19 +4196,21 @@ func TestTaggedNodeWithoutUserToDifferentUser(t *testing.T) {
)
require.NoError(t, err, "Re-registration to alice should succeed without panic")
// Verify the existing tagged node was converted to be owned by alice (same node ID)
require.True(t, nodeAfterReauth.Valid(), "Node should be valid")
require.True(t, nodeAfterReauth.UserID().Valid(), "Node should have a UserID")
require.Equal(t, alice.ID, nodeAfterReauth.UserID().Get(), "Node should be owned by alice")
require.Equal(t, node.ID(), nodeAfterReauth.ID(), "Should be the same node (converted, not new)")
require.False(t, nodeAfterReauth.IsTagged(), "Node should no longer be tagged")
require.Empty(t, nodeAfterReauth.Tags().AsSlice(), "Node should have no tags")
require.NotEqual(t, node.ID(), nodeAfterReauth.ID(), "alice gets a device of her own")
require.False(t, nodeAfterReauth.IsTagged(), "Node should not be tagged")
tagged, ok := app.state.GetNodeByID(node.ID())
require.True(t, ok, "the tagged node stays")
require.ElementsMatch(t, tags, tagged.Tags().AsSlice())
// Verify [types.NodeView.Owner] works without panicking - this is what the mapper's
// [generateUserProfiles] calls, and it would panic with a nil pointer
// dereference if node.User was not set during the tag→user conversion.
// dereference if node.User was not set.
owner := nodeAfterReauth.Owner()
require.True(t, owner.Valid(), "Owner should be valid after conversion (mapper would panic if nil)")
require.True(t, owner.Valid(), "Owner should be valid (mapper would panic if nil)")
require.Equal(t, alice.ID, owner.Model().ID, "Owner should be alice")
t.Logf("Re-registration complete - Node ID: %d, Tags: %v, IsTagged: %t, UserID: %d",
+6 -40
View File
@@ -60,32 +60,6 @@ var reauthSkipReasons = map[string]string{
"reauth-user/a1-spent-key/u-spent/expiry": sameKeyRenewsExpiry,
"reauth-user/a2-valid-key/u-valid/expiry": sameKeyRenewsExpiry,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/devices": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/owner": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/node-key": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/expiry": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/ephemeral": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/peer-obs": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/obs/filter": otherUserKeyReauthsTagged,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/login-devices": loginTakesTaggedDevice,
"reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/login-owner": loginTakesTaggedDevice,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/devices": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/owner": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/node-key": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/expiry": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/ephemeral": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/peer-obs": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/obs/filter": otherUserKeyReauthsTagged,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/login-devices": loginTakesTaggedDevice,
"reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/login-owner": loginTakesTaggedDevice,
"reauth-foreign/t-eph-spent-foreign-key/t-eph/ephemeral": spentKeyRebindsNode,
"reauth-foreign/t-eph-logout/t-eph/devices": spentKeyRebindsNode,
"reauth-foreign/t-eph-logout/t-eph/owner": spentKeyRebindsNode,
"reauth-foreign/t-eph-logout/t-eph/expiry": spentKeyRebindsNode,
"reauth-foreign/t-eph-logout/t-eph/ephemeral": spentKeyRebindsNode,
"reauth-foreign/t-eph-logout/t-eph/peer-obs": spentKeyRebindsNode,
"reauth-foreign/t-logout-logout/t-logout/expiry": taggedLogoutNotExpired,
"reauth-foreign/t-logout-logout/t-logout/peer-obs": taggedLogoutNotExpired,
"reauth-foreign/t-logout-relogin/t-logout/expiry": taggedLogoutNotExpired,
@@ -105,10 +79,6 @@ var reauthSkipReasons = map[string]string{
"identity/w-tagreq-new-join/w-tagreq-new/peer-obs": keyLoginTagsRefused,
"identity/w-tagreq-new-join/w-tagreq-new/peer-key-obs": keyLoginTagsRefused,
"reauth-owner/u-tagged-reauth-relogin/u-tagged-reauth/owner": adminTagsKeptOnUserLogin,
"reauth-owner/u-tagged-reauth-relogin/u-tagged-reauth/expiry": adminTagsKeptOnUserLogin,
"reauth-owner/u-tagged-reauth-relogin/obs/filter": adminTagsKeptOnUserLogin,
"reauth/late-pak-spent-key/late-pak/text": refusalText,
"reauth/late-pak-spent-key/late-pak/legacy-text": refusalText,
}
@@ -120,16 +90,12 @@ const (
)
const (
expiredKeyNotRotated = "EXPIRED_KEY_NOT_ROTATED: headscale answers an expired node's key login without asking for a new node key; SaaS answers NodeKeyExpired first, so the client rotates it"
sameKeyRenewsExpiry = "SAME_KEY_LOGIN_RENEWS_EXPIRY: headscale renews a user node's expiry on a key login that keeps its node key; SaaS renews it only with a new node key"
otherUserKeyReauthsTagged = "OTHER_USER_KEY_REAUTHS_TAGGED: headscale re-registers a tagged device in place when another user's key logs in anew on its machine; SaaS keeps the tagged device and adds one for that user"
loginTakesTaggedDevice = "LOGIN_TAKES_TAGGED_DEVICE: headscale's interactive login gives another user the tagged device on the machine; SaaS's key login keeps it and adds a device for that user"
spentKeyRebindsNode = "SPENT_KEY_REBINDS_NODE: headscale binds a live node to the spent key it logs in with, so an ephemeral node stops being ephemeral and survives logout; SaaS keeps the node as it was"
taggedLogoutNotExpired = "TAGGED_LOGOUT_NOT_EXPIRED: headscale leaves a tagged node unexpired when it logs out; SaaS expires it"
wakeWithTagsRefused = "WAKE_WITH_TAGS_REFUSED: headscale refuses a key login that requests tags the key does not carry even when the node keeps its node key and tags; SaaS accepts it and changes nothing"
keyLoginTagsRefused = "KEY_LOGIN_TAGS_REFUSED: headscale refuses a user's key login that requests tags the user owns; SaaS tags the device, converting it in place or creating it tagged"
adminTagsKeptOnUserLogin = "ADMIN_TAGS_KEPT_ON_USER_LOGIN: headscale keeps an admin-tagged device tagged when its user's key logs in anew; SaaS turns it back into that user's device"
refusalText = "REFUSAL_TEXT: SaaS refuses a key by naming its ID; headscale says why the key is refused"
expiredKeyNotRotated = "EXPIRED_KEY_NOT_ROTATED: headscale answers an expired node's key login without asking for a new node key; SaaS answers NodeKeyExpired first, so the client rotates it"
sameKeyRenewsExpiry = "SAME_KEY_LOGIN_RENEWS_EXPIRY: headscale renews a user node's expiry on a key login that keeps its node key; SaaS renews it only with a new node key"
taggedLogoutNotExpired = "TAGGED_LOGOUT_NOT_EXPIRED: headscale leaves a tagged node unexpired when it logs out; SaaS expires it"
wakeWithTagsRefused = "WAKE_WITH_TAGS_REFUSED: headscale refuses a key login that requests tags the key does not carry even when the node keeps its node key and tags; SaaS accepts it and changes nothing"
keyLoginTagsRefused = "KEY_LOGIN_TAGS_REFUSED: headscale refuses a user's key login that requests tags the user owns; SaaS tags the device, converting it in place or creating it tagged"
refusalText = "REFUSAL_TEXT: SaaS refuses a key by naming its ID; headscale says why the key is refused"
)
// reauthNotCompared lists, by key prefix, checks left out: SaaS's answer
+240 -24
View File
@@ -11,6 +11,7 @@ import (
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"tailscale.com/tailcfg"
@@ -382,34 +383,111 @@ func TestTaggedPAKReauthRejectsAmbiguousMultiUserNode(t *testing.T) {
require.Equal(t, 2, s.ListNodes().Len(), "no node created or converted on rejection")
}
// TestAuthPathRejectsTaggedAndUserCoexistence: if a machine key ends up with
// both a tagged node and a user-owned node (impossible per validateNodeOwnership,
// but reachable by tagging one node of a multi-user device via the admin path),
// an OIDC re-auth must reject rather than silently converting the tagged node
// and orphaning the user-owned one.
func TestAuthPathRejectsTaggedAndUserCoexistence(t *testing.T) {
// TestAuthPathOnTaggedMachine: a login on a machine with a tagged node and
// another user's node re-authenticates the tagged node only for the user it
// was tagged from; anyone else gets a device of their own, even under the
// tagged node's node key.
func TestAuthPathOnTaggedMachine(t *testing.T) {
s, mk, n1 := registerTwoUsersOnOneMachine(t)
// Tag one of the two user-owned nodes -> {0: tagged, u2: user-owned} coexist.
_, err := s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["u1@"]}}`))
require.NoError(t, err)
tagged, _, err := s.SetNodeTags(n1, []string{"tag:foo"})
require.NoError(t, err)
require.True(t, tagged.IsTagged())
// A third user authenticates the same machine via OIDC.
u3 := s.CreateUserForTest("u3")
regData := &types.RegistrationData{
MachineKey: mk,
NodeKey: key.NewNode().Public(),
Hostname: "multi",
Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
}
authID := types.MustAuthID()
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData))
login := func(user *types.User, nodeKey key.NodePublic) (types.NodeView, error) {
t.Helper()
_, _, err = s.HandleNodeFromAuthPath(authID, types.UserID(u3.ID), nil, util.RegisterMethodOIDC)
require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
authID := types.MustAuthID()
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{
MachineKey: mk,
NodeKey: nodeKey,
Hostname: "multi",
Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
}))
node, _, err := s.HandleNodeFromAuthPath(authID, types.UserID(user.ID), nil, util.RegisterMethodOIDC)
return node, err
}
u3 := s.CreateUserForTest("u3")
other, err := login(u3, tagged.NodeKey())
require.NoError(t, err)
assert.NotEqual(t, n1, other.ID(), "another user gets a device of their own")
assert.Equal(t, u3.ID, other.UserID().Get())
current, ok := s.GetNodeByID(n1)
require.True(t, ok)
assert.True(t, current.IsTagged(), "the tagged node is untouched")
u1, err := s.GetUserByName("u1")
require.NoError(t, err)
claimed, err := login(u1, key.NewNode().Public())
require.NoError(t, err)
assert.Equal(t, n1, claimed.ID(), "the user it was tagged from takes it back")
assert.False(t, claimed.IsTagged())
assert.Equal(t, 3, s.ListNodes().Len())
}
// TestUserKeyOnLoginJoinedTaggedNode: an admin-tagged node that joined by
// login records no former owner, so no user key speaks for it. Under its own
// node key a key only wakes it; under a new one each user gets a device of
// their own.
func TestUserKeyOnLoginJoinedTaggedNode(t *testing.T) {
s := newRetagTestState(t)
owner := s.CreateUserForTest("login-owner")
machineKey := key.NewMachine().Public()
nodeKey := key.NewNode().Public()
authID := types.MustAuthID()
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{
MachineKey: machineKey,
NodeKey: nodeKey,
Hostname: "login-node",
Hostinfo: &tailcfg.Hostinfo{Hostname: "login-node"},
}))
joined, _, err := s.HandleNodeFromAuthPath(authID, types.UserID(owner.ID), nil, util.RegisterMethodOIDC)
require.NoError(t, err)
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:prod":["login-owner@"]}}`))
require.NoError(t, err)
_, _, err = s.SetNodeTags(joined.ID(), []string{"tag:prod"})
require.NoError(t, err)
for _, user := range []*types.User{owner, s.CreateUserForTest("login-other")} {
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
require.NoError(t, err)
register := func(nk key.NodePublic) types.NodeView {
t.Helper()
node, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: nk,
Hostinfo: &tailcfg.Hostinfo{Hostname: "login-node"},
}, machineKey)
require.NoError(t, err)
return node
}
woken := register(nodeKey)
assert.Equal(t, joined.ID(), woken.ID(), "%s: its own node key wakes it", user.Name)
assert.True(t, woken.IsTagged())
own := register(key.NewNode().Public())
assert.NotEqual(t, joined.ID(), own.ID(), "%s: a device of its own", user.Name)
assert.Equal(t, user.ID, own.UserID().Get())
}
current, ok := s.GetNodeByID(joined.ID())
require.True(t, ok)
assert.True(t, current.IsTagged(), "the tagged node is untouched")
assert.Equal(t, nodeKey, current.NodeKey())
assert.Equal(t, 3, s.ListNodes().Len())
}
// seededTaggedNode holds the identity of a node seeded into the genuinely
@@ -465,12 +543,16 @@ func seedTagOwnedNode(t *testing.T, tags []string, createdByName string) seededT
// Genuinely tag-owned: tags set, UserID nil. UserID nil indexes the node
// under userID 0, so HandleNodeFromAuthPath takes the convert-from-tag
// branch — the path the issue exercises. User mirrors createdBy.
// branch — the path the issue exercises. User mirrors createdBy. It joined
// by login, so it carries no auth key.
seeded, ok := s.nodeStore.UpdateNode(node.ID, func(n *types.Node) {
n.Tags = tags
n.UserID = nil
n.User = createdBy
n.Expiry = nil
n.RegisterMethod = util.RegisterMethodOIDC
n.AuthKeyID = nil
n.AuthKey = nil
})
require.True(t, ok)
require.True(t, seeded.IsTagged(), "precondition: node must be tagged")
@@ -706,8 +788,8 @@ func TestTaggedReauthUntagReturnsToAuthUser(t *testing.T) {
}
// TestTaggedReauthUntagClearsEphemeralAuthKey guards the interactive-path mirror
// of the #3370 ephemeral bug: a node created by a tagged *ephemeral* pre-auth
// key, then untagged via an interactive re-auth, must not keep the ephemeral key
// of the #3370 ephemeral bug: a node joined with a user's tagged *ephemeral*
// pre-auth key, then untagged via their interactive re-auth, must not keep the ephemeral key
// reference. Otherwise it stays IsEphemeral() and is garbage-collected on its
// next disconnect, silently deleting the user's just-claimed device — and the
// stale reference survives a control-plane restart via the reloaded AuthKeyID.
@@ -720,12 +802,13 @@ func TestTaggedReauthUntagClearsEphemeralAuthKey(t *testing.T) {
// Attach a tagged, ephemeral auth key to the seeded node (the shape a
// `tailscale up --authkey <ephemeral tagged key>` node has).
ephKey, err := n.s.CreatePreAuthKey(nil, false, true /*ephemeral*/, nil, []string{"tag:foo"})
ephKey, err := n.s.CreatePreAuthKey(alice.TypedID(), false, true /*ephemeral*/, nil, []string{"tag:foo"})
require.NoError(t, err)
pak, err := n.s.GetPreAuthKeyByID(ephKey.ID)
require.NoError(t, err)
seeded, ok := n.s.nodeStore.UpdateNode(n.id, func(nd *types.Node) {
nd.RegisterMethod = util.RegisterMethodAuthKey
nd.AuthKey = pak.AsCredential()
nd.AuthKeyID = &pak.ID
})
@@ -1745,7 +1828,7 @@ func (c pakReregCase) lookup(t *testing.T, keyStr string) (types.NodeView, *type
pak, err := c.s.GetPreAuthKey(keyStr)
require.NoError(t, err)
view, ok, err := c.s.findExistingNodeForPAK(c.machineKey, pak)
view, ok, err := c.s.findExistingNodeForPAK(c.machineKey, pak, c.regReq.NodeKey)
require.NoError(t, err)
require.True(t, ok)
require.True(t, pakSkipsValidation(view, pak, c.regReq.NodeKey, time.Now()),
@@ -2244,3 +2327,136 @@ func TestPreAuthKeyRegistrationOwnsHostinfo(t *testing.T) {
})
}
}
// TestUserKeyOnTaggedMachine: a user key on a tagged machine re-authenticates
// the tagged node only when it speaks for it (the user an admin tagged it
// from); another user's key wakes it, gets a device of its own beside it with
// a new node key, and cannot lift its expiry.
func TestUserKeyOnTaggedMachine(t *testing.T) {
for _, adminTagged := range []bool{false, true} {
for _, mode := range []string{"expired", "rotated", "wake"} {
t.Run(fmt.Sprintf("admin=%t/%s", adminTagged, mode), func(t *testing.T) {
s := newRetagTestState(t)
user := s.CreateUserForTest("former-owner")
var owner *types.UserID
tags := []string{"tag:prod"}
if adminTagged {
owner = user.TypedID()
tags = nil
}
original, err := s.CreatePreAuthKey(owner, true, true, nil, tags)
require.NoError(t, err)
c := registerForPAKRereg(t, s, original)
if adminTagged {
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:prod":["former-owner@"]}}`))
require.NoError(t, err)
_, _, err = s.SetNodeTags(c.node.ID(), []string{"tag:prod"})
require.NoError(t, err)
} else {
user = s.CreateUserForTest("guest")
}
presented, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
require.NoError(t, err)
req := c.regReq
req.Auth = &tailcfg.RegisterResponseAuth{AuthKey: presented.Key}
switch mode {
case "expired":
past := time.Now().Add(-time.Hour)
_, _, err = s.SetNodeExpiry(c.node.ID(), &past)
require.NoError(t, err)
case "rotated":
req.NodeKey = key.NewNode().Public()
}
beforeCache, beforeDB := c.fields(t)
got, _, err := s.HandleNodeFromPreAuthKey(req, c.machineKey)
afterCache, afterDB := c.fields(t)
assert.Equal(t, afterCache, afterDB, "NodeStore and database agree")
storedKey, keyErr := s.GetPreAuthKeyByID(presented.ID)
require.NoError(t, keyErr)
switch {
case !adminTagged && mode == "expired":
require.EqualError(t, err, "authkey cannot re-authenticate this node")
assert.Equal(t, beforeCache, afterCache)
assert.Equal(t, beforeDB, afterDB)
assert.False(t, storedKey.Used)
assert.Equal(t, 1, s.ListNodes().Len())
case !adminTagged && mode == "rotated":
require.NoError(t, err)
assert.NotEqual(t, c.node.ID(), got.ID(), "a device of its own")
assert.False(t, got.IsTagged())
assert.Equal(t, user.ID, got.UserID().Get())
assert.Equal(t, beforeCache.NodeKey, afterCache.NodeKey, "the tagged node is untouched")
assert.Equal(t, beforeCache.Tags, afterCache.Tags)
assert.Equal(t, beforeCache.AuthKeyID, afterCache.AuthKeyID)
assert.True(t, storedKey.Used)
assert.Equal(t, 2, s.ListNodes().Len())
case !adminTagged:
require.NoError(t, err)
assert.Equal(t, original.ID, *afterCache.AuthKeyID)
assert.Equal(t, beforeCache.Tags, afterCache.Tags)
assert.False(t, storedKey.Used)
assert.Equal(t, 1, s.ListNodes().Len())
case mode == "rotated":
require.NoError(t, err)
assert.Equal(t, c.node.ID(), got.ID(), "converted in place")
assert.Empty(t, afterCache.Tags)
require.NotNil(t, afterCache.UserID)
assert.Equal(t, user.ID, *afterCache.UserID)
assert.Equal(t, req.NodeKey, afterCache.NodeKey)
assert.Equal(t, 1, s.ListNodes().Len())
default:
require.NoError(t, err)
assert.Equal(t, []string{"tag:prod"}, afterCache.Tags, "a same-key relogin keeps it tagged")
assert.Equal(t, presented.ID, *afterCache.AuthKeyID)
assert.Equal(t, 1, s.ListNodes().Len())
if mode == "expired" {
assert.Zero(t, afterCache.Expiry, "a valid key lifts the past expiry")
} else {
assert.Equal(t, beforeCache.Expiry, afterCache.Expiry)
}
}
})
}
}
}
// TestPAKWakeRechecksAuthorityAtMutation: another user's key that wakes a
// live tagged node is refused when an expiry lands before the writer runs.
func TestPAKWakeRechecksAuthorityAtMutation(t *testing.T) {
s := newRetagTestState(t)
original, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:prod"})
require.NoError(t, err)
c := registerForPAKRereg(t, s, original)
guest := s.CreateUserForTest("guest")
guestKey, err := s.CreatePreAuthKey(guest.TypedID(), true, false, nil, nil)
require.NoError(t, err)
view, pak := c.lookup(t, guestKey.Key)
past := time.Now().Add(-time.Hour)
_, _, err = s.SetNodeExpiry(c.node.ID(), &past)
require.NoError(t, err)
beforeCache, beforeDB := c.fields(t)
_, err = c.reregister(view, pak, c.regReq)
require.EqualError(t, err, "authkey cannot re-authenticate this node")
afterCache, afterDB := c.fields(t)
assert.Equal(t, beforeCache, afterCache)
assert.Equal(t, beforeDB, afterDB)
}
+81 -45
View File
@@ -2501,10 +2501,7 @@ func (s *State) HandleNodeFromAuthPath(
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
// Named conditions - describe WHAT we found, not HOW we check it.
existingNodeSameUser, nodeExistsForSameUser := all[types.UserID(user.ID)]
taggedNode, hasTagged := all[0]
existingNodeIsTagged := hasTagged && taggedNode.IsTagged()
existingNode, nodeExists := loginNode(all, types.UserID(user.ID))
var existingNodeOtherUser types.NodeView
@@ -2517,14 +2514,6 @@ func (s *State) HandleNodeFromAuthPath(
}
}
// A tagged node and a user-owned node cannot legitimately share a machine
// key (validateNodeOwnership enforces tags XOR user ownership). If both are
// present the machine key is in a corrupt/ambiguous state; reject rather
// than converting an arbitrary node and orphaning the other.
if existingNodeIsTagged && (nodeExistsForSameUser || existingNodeOwnedByOtherUser) {
return types.NodeView{}, s.policyChangeSince(genBefore), ErrAmbiguousNodeOwnership
}
// Create logger with common fields for all auth operations
logger := log.With().
Str(zf.RegistrationID, authID.String()).
@@ -2548,15 +2537,15 @@ func (s *State) HandleNodeFromAuthPath(
keysOnly bool
)
if nodeExistsForSameUser {
updateParams.ExistingNode = existingNodeSameUser
if nodeExists && !existingNode.IsTagged() {
updateParams.ExistingNode = existingNode
finalNode, keysOnly, err = s.applyAuthNodeUpdate(updateParams)
if err != nil {
return types.NodeView{}, s.policyChangeSince(genBefore), err
}
} else if existingNodeIsTagged {
updateParams.ExistingNode = taggedNode
} else if nodeExists {
updateParams.ExistingNode = existingNode
updateParams.IsConvertFromTag = true
finalNode, _, err = s.applyAuthNodeUpdate(updateParams)
@@ -2652,31 +2641,27 @@ func (s *State) createNewNodeFromAuth(
// HandleNodeFromPreAuthKey handles node registration using a pre-authentication key.
// findExistingNodeForPAK looks up an existing node by machine key,
// matching the PAK's ownership. For user-owned keys it checks the
// user's ID; for tagged keys it checks UserID(0) since tagged nodes
// have no owning user.
// matching the PAK's ownership. A key with a user finds that user's node,
// else the machine's tagged node; a tags-only key checks UserID(0) since
// tagged nodes have no owning user.
func (s *State) findExistingNodeForPAK(
machineKey key.MachinePublic,
pak *types.PreAuthKey,
nodeKey key.NodePublic,
) (types.NodeView, bool, error) {
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
if pak.User != nil {
if node, ok := all[types.UserID(pak.User.ID)]; ok {
return node, true, nil
node, ok := all[types.UserID(pak.User.ID)]
if !ok {
// The machine's tagged node, when the key speaks for it. Any other
// key under its own node key is the device restarting (#3312);
// pakReregister lets that key only wake it.
node, ok = all[0]
ok = ok && node.IsTagged() && (pak.CanReauthenticate(node) || node.NodeKey() == nodeKey)
}
// The node may have been converted to a tagged node since it first
// registered (SetNodeTags clears UserID, re-indexing it under UserID(0)).
// It is still the same machine, proven by the machine key, so recognise
// it for re-registration instead of re-validating the spent key or
// creating a duplicate node. Re-registration preserves the node's tagged
// ownership. See https://github.com/juanfont/headscale/issues/3312.
if node, ok := all[0]; ok && node.IsTagged() {
return node, true, nil
}
return types.NodeView{}, false, nil
return node, ok, nil
}
// A tagged key re-registers the same machine regardless of how it is
@@ -2714,6 +2699,31 @@ func (s *State) findExistingNodeForPAK(
return types.NodeView{}, false, nil
}
// LoginNode returns the node that a login by userID on machineKey
// re-authenticates. False means the login adds a new device.
func (s *State) LoginNode(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
return loginNode(s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey), userID)
}
// loginNode picks, from a machine's nodes, the one a login by userID
// re-authenticates: the user's own node, else the machine's tagged node when
// the user can claim it. A tagged node that joined by login records no former
// owner, so any login re-authenticates it, as before; the tags the login asks
// for decide whether it stays tagged. Otherwise another user's login on a
// tagged machine adds a device for that user beside it, as on Tailscale SaaS.
func loginNode(all map[types.UserID]types.NodeView, userID types.UserID) (types.NodeView, bool) {
if node, ok := all[userID]; ok {
return node, true
}
if node, ok := all[0]; ok && node.IsTagged() &&
(node.ClaimableBy(userID) || node.RegisterMethod() != util.RegisterMethodAuthKey) {
return node, true
}
return types.NodeView{}, false
}
// pakSkipsValidation reports whether re-registering n with pak may skip key
// validation at now. The machine key proves identity and a pre-auth key only
// authorises the initial join, so a plain restart (container "tailscale up
@@ -2745,14 +2755,22 @@ func pakSkipsValidation(n types.NodeView, pak *types.PreAuthKey, nodeKey key.Nod
!isOwnershipConversion && !isRetag
}
// pakReregisterErr is the authorisation decision for re-registering n with
// pak at now: nil when validation may be skipped, else the key's validity.
func pakReregisterErr(n types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePublic, now time.Time) error {
// pakReregister is the authorisation decision for re-registering n with pak
// at now: whether the key re-authenticates n, and nil when validation may be
// skipped, else the key's validity. A key that does not speak for n only
// wakes it under its own node key.
func pakReregister(n types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePublic, now time.Time) (bool, error) {
reauth := pak.CanReauthenticate(n)
if pakSkipsValidation(n, pak, nodeKey, now) {
return nil
return reauth, nil
}
return pak.ValidAt(now)
err := pak.ValidAt(now)
if err == nil && !reauth {
err = types.PAKError("authkey cannot re-authenticate this node")
}
return reauth, err
}
//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
@@ -2791,7 +2809,7 @@ func (s *State) HandleNodeFromPreAuthKey(
}
}
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak)
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak, regReq.NodeKey)
if err != nil {
return types.NodeView{}, s.policyChangeSince(genBefore), err
}
@@ -2991,6 +3009,7 @@ func (s *State) reregisterNodeWithPAK(
var (
prior *types.Node
authErr error
reauth bool
)
// Update existing node - NodeStore first, then database
@@ -3000,12 +3019,11 @@ func (s *State) reregisterNodeWithPAK(
prior = node.Clone()
authErr = pakReregisterErr(node.View(), pak, regReq.NodeKey, now)
reauth, authErr = pakReregister(node.View(), pak, regReq.NodeKey, now)
if authErr != nil {
return
}
node.NodeKey = regReq.NodeKey
node.Hostname = hostname
// TODO(kradalby): We should ensure we use the same hostinfo and node merge semantics
@@ -3015,6 +3033,16 @@ func (s *State) reregisterNodeWithPAK(
node.Hostinfo = validHostinfo
node.Hostinfo.NetInfo = preserveNetInfo(prior.View(), prior.ID, validHostinfo)
// Preserve online state during re-registration so a live node does
// not appear offline before the client restarts its map stream.
node.LastSeen = new(now)
if !reauth {
return
}
node.NodeKey = regReq.NodeKey
node.RegisterMethod = util.RegisterMethodAuthKey
// Tags from a PreAuthKey are applied on initial registration and
@@ -3046,6 +3074,13 @@ func (s *State) reregisterNodeWithPAK(
if wasUserOwned || node.IsExpiredAt(now) {
node.Expiry = nil
}
} else if node.IsTagged() && !pak.IsTagged() && prior.NodeKey != regReq.NodeKey {
// A user key that speaks for a tagged node, with a new node key,
// takes it back as that user's device, as on Tailscale SaaS. A
// relogin under the same node key keeps it tagged.
node.Tags = nil
node.UserID = &pak.User.ID
node.User = pak.User
}
node.AuthKey = pak.AsCredential()
@@ -3056,9 +3091,6 @@ func (s *State) reregisterNodeWithPAK(
if !pak.Reusable && !pak.Used {
node.AuthKey.Used = true
}
// Preserve online state during re-registration so a live node does
// not appear offline before the client restarts its map stream.
node.LastSeen = new(now)
// Tagged nodes keep their existing expiry (disabled).
// User-owned nodes update expiry from the client request,
@@ -3107,7 +3139,11 @@ func (s *State) reregisterNodeWithPAK(
// re-registration presents a key just loaded from the database, so
// its ID must be persisted here — otherwise a restart reloads the old
// key and any key-scoped property (e.g. Ephemeral) silently reverts.
reregColumns := append(slices.Clone(nodeUpdateColumns), "AuthKeyID")
// A wake-up keeps the prior key, which may have been deleted (#2862).
reregColumns := nodeUpdateColumns
if reauth {
reregColumns = append(slices.Clone(nodeUpdateColumns), "AuthKeyID")
}
err := tx.Select(reregColumns).Updates(updatedNodeView.AsStruct()).Error
if err != nil {
@@ -3119,7 +3155,7 @@ func (s *State) reregisterNodeWithPAK(
// atomic compare-and-set in [hsdb.UsePreAuthKey] would otherwise
// reject it as "authkey already used", locking out containers
// that restart with the same one-shot key.
if !pak.Reusable && !pak.Used {
if reauth && !pak.Reusable && !pak.Used {
err = hsdb.UsePreAuthKey(tx, pak)
if err != nil {
return nil, fmt.Errorf("using pre auth key: %w", err)
+14
View File
@@ -963,6 +963,20 @@ func (nv NodeView) IsTagged() bool {
return nv.ж.IsTagged()
}
// ClaimableBy reports whether a login by userID takes this tagged node back
// as that user's device, rather than adding a device beside it: userID owns
// or created the key it joined with. Tagging clears a node's user, so a node
// that joined by login records no former owner and nobody claims it.
func (nv NodeView) ClaimableBy(userID UserID) bool {
if !nv.IsTagged() || nv.RegisterMethod() != util.RegisterMethodAuthKey {
return false
}
k := nv.AuthKey()
return k.Valid() && k.UserID().Valid() && UserID(k.UserID().Get()) == userID
}
// IsExpired returns whether the node registration has expired.
func (nv NodeView) IsExpired() bool {
if !nv.Valid() {
+23
View File
@@ -157,6 +157,29 @@ func (pak *PreAuthKey) RetagsNode(node NodeView) bool {
(!node.AuthKeyID().Valid() || node.AuthKeyID().Get() != pak.ID)
}
// CanReauthenticate reports whether this key speaks for node: a tagged key
// (it brings its own tags), a key of the node's user, or a key of the user a
// tagged node can be claimed by (see [NodeView.ClaimableBy]).
func (pak *PreAuthKey) CanReauthenticate(node NodeView) bool {
if pak == nil || !node.Valid() {
return false
}
if pak.IsTagged() {
return true
}
if pak.User == nil {
return false
}
if node.IsTagged() {
return node.ClaimableBy(UserID(pak.User.ID))
}
return node.UserID().Valid() && node.UserID().Get() == pak.User.ID
}
// maskedPrefix returns the key prefix in masked format for safe logging.
// SECURITY: Never log the full key or hash, only the masked prefix.
func (pak *PreAuthKey) maskedPrefix() string {
+48
View File
@@ -6,6 +6,7 @@ import (
"time"
"github.com/google/go-cmp/cmp"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/require"
)
@@ -199,3 +200,50 @@ func TestPreAuthKeyTagChanges(t *testing.T) {
require.False(t, pak.ConvertsNodeToTagged(NodeView{}))
require.False(t, pak.RetagsNode(NodeView{}))
}
func TestPreAuthKeyCanReauthenticate(t *testing.T) {
owner, other := uint(1), uint(2)
userNode := (&Node{ID: 1, UserID: &owner}).View()
// An admin tagged a node its user joined with a user key.
taggedByUserKey := (&Node{
ID: 1, Tags: []string{"tag:prod"}, RegisterMethod: util.RegisterMethodAuthKey,
AuthKey: &Credential{ID: 10, UserID: &owner},
}).View()
// A user made the tagged key the node joined with.
taggedByTaggedKey := (&Node{
ID: 1, Tags: []string{"tag:prod"}, RegisterMethod: util.RegisterMethodAuthKey,
AuthKey: &Credential{ID: 10, UserID: &owner, Tags: []string{"tag:prod"}},
}).View()
// A node registered by login records no user.
taggedByLogin := (&Node{
ID: 1, Tags: []string{"tag:prod"}, RegisterMethod: util.RegisterMethodOIDC,
}).View()
userKey := func(id uint) *PreAuthKey { return &PreAuthKey{ID: 11, User: &User{ID: id}} }
taggedKey := &PreAuthKey{Tags: []string{"tag:ci"}}
for _, tt := range []struct {
name string
pak *PreAuthKey
node NodeView
want bool
}{
{"tagged key converts", taggedKey, userNode, true},
{"tagged key retags", taggedKey, taggedByTaggedKey, true},
{"same user", userKey(owner), userNode, true},
{"other user", userKey(other), userNode, false},
{"former owner after admin tagging", userKey(owner), taggedByUserKey, true},
{"other user on admin-tagged node", userKey(other), taggedByUserKey, false},
{"tagged key creator", userKey(owner), taggedByTaggedKey, true},
{"other user on tagged key node", userKey(other), taggedByTaggedKey, false},
{"node registered by login", userKey(owner), taggedByLogin, false},
{"no owner", &PreAuthKey{ID: 11}, userNode, false},
{"invalid node", taggedKey, NodeView{}, false},
{"nil key", nil, userNode, false},
} {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.want, tt.pak.CanReauthenticate(tt.node))
})
}
}
+30 -26
View File
@@ -3295,18 +3295,19 @@ func TestTagsAuthKeyWithoutUserRejectsAdvertisedTags(t *testing.T) {
}
// =============================================================================
// Test Suite 6: Tagged→User Conversion via CLI Register (#3038)
// Test Suite 6: User Login on a Tagged Machine via CLI Register (#3038)
// =============================================================================
// TestTagsAuthKeyConvertToUserViaCLIRegister reproduces the panic from
// issue #3038: register a node with a tags-only preauthkey (no user), then
// convert it to a user-owned node via "headscale auth register --auth-id <id> --user <user>".
// The crash happens in the mapper's generateUserProfiles when [types.Node.User] is nil
// after the tag→user conversion in [State.processReauthTags].
// TestTagsAuthKeyWithoutUserCLIRegisterAddsUserDevice covers issue #3038:
// register a node with a tags-only preauthkey (no user), then log the machine
// in as a user via "headscale auth register --auth-id <id> --user <user>".
// The crash was in the mapper's generateUserProfiles when [types.Node.User]
// was nil.
//
// The key detail is using a tags-only PreAuthKey (User: nil). When created under
// a user, the node inherits User from the PreAuthKey and the bug is masked.
func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
// No user made the key, so no user can take the tagged node back. As for
// another user's login on a tagged machine on Tailscale SaaS, the login adds a
// device of the user's own beside it.
func TestTagsAuthKeyWithoutUserCLIRegisterAddsUserDevice(t *testing.T) {
IntegrationSkip(t)
policy := tagsTestPolicy()
@@ -3331,10 +3332,8 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
// Step 1: Create a tags-only preauthkey WITHOUT a user.
// This is the critical detail: when [types.PreAuthKey.UserID] is nil, the node
// enters the [state.NodeStore] with [types.Node.User] == nil. The [state.State.processReauthTags]
// conversion then sets UserID but not User, leaving it nil for the mapper.
// Step 1: Create a tags-only preauthkey WITHOUT a user, so the node enters
// the [state.NodeStore] with [types.Node.User] == nil.
authKey, err := scenario.CreatePreAuthKeyWithOptions(hsic.AuthKeyOptions{
User: nil,
Reusable: false,
@@ -3356,6 +3355,8 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
err = client.WaitForRunning(integrationutil.PeerSyncTimeout())
require.NoError(t, err)
var taggedID string
// Verify initial state: node is tagged
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
@@ -3364,6 +3365,7 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
if len(nodes) == 1 {
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
taggedID = nodes[0].Id
t.Logf("Initial state - Node ID: %s, Tags: %v", nodes[0].Id, nodes[0].Tags)
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "node should be tagged initially")
@@ -3386,30 +3388,32 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) {
body, err := doLoginURL(client.Hostname(), loginURL)
require.NoError(t, err)
// Step 3: Register via CLI with user (this is the exact step that triggers the panic)
// Step 3: Register via CLI with user (the step that triggered the panic)
err = scenario.runHeadscaleRegister(tagTestUser, body)
require.NoError(t, err)
err = client.WaitForRunning(integrationutil.PeerSyncTimeout())
require.NoError(t, err)
// Step 4: Verify node is now user-owned and the mapper didn't panic.
// The panic would occur when the mapper builds the [tailcfg.MapResponse] and calls
// [types.Node.Owner].Model().ID with a nil User pointer.
// ShutdownAssertNoPanics in the defer catches any panics in headscale logs.
// Step 4: The tagged node stays and the user gets a device of its own. The
// mapper builds both into the map; ShutdownAssertNoPanics in the defer
// catches a panic in the headscale logs.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1)
assert.Len(c, nodes, 2)
if len(nodes) == 1 {
assertNodeHasNoTagsWithCollect(c, nodes[0])
assert.Equal(c, tagTestUser, nodes[0].User.Name,
"Node ownership should be returned to user after untagging")
t.Logf("After conversion - Node ID: %s, Tags: %v, User: %s",
nodes[0].Id, nodes[0].Tags, nodes[0].User.Name)
for _, node := range nodes {
if node.Id == taggedID {
assertNodeHasTagsWithCollect(c, node, []string{"tag:valid-owned"})
continue
}
assertNodeHasNoTagsWithCollect(c, node)
assert.Equal(c, tagTestUser, node.User.Name, "the login's device belongs to the user")
}
}, integrationutil.HAConvergeTimeout, 1*time.Second, "node should be user-owned after conversion via CLI register")
}, integrationutil.HAConvergeTimeout, 1*time.Second, "login should add a user device beside the tagged node")
}
// TestTaggedNodeLogoutReloginSingleUseKeyOnline reproduces issue #3371