mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-01 03:04:07 +09:00
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
(cherry picked from commit e1de83cbdf, adapted: inline PAK re-register path, no servertest compat table)
This commit is contained in:
1 parent
e6219cd106
commit
c8fb25d451
8 files changed
+457
-98
No files matched your search
@@ -365,7 +365,7 @@ jobs:
|
||||
- TestTagsUserLoginReauthWithEmptyTagsRemovesAllTags
|
||||
- TestTagsAuthKeyWithoutUserInheritsTags
|
||||
- TestTagsAuthKeyWithoutUserRejectsAdvertisedTags
|
||||
- TestTagsAuthKeyConvertToUserViaCLIRegister
|
||||
- TestTagsAuthKeyWithoutUserCLIRegisterAddsUserDevice
|
||||
- TestTaggedNodeLogoutReloginSingleUseKeyOnline
|
||||
- TestTaggedNodeLogoutReloginReusableKeyOnline
|
||||
- TestTagsOIDCReauthAddOwnedTag
|
||||
|
||||
+12
-9
@@ -3900,9 +3900,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()
|
||||
|
||||
@@ -3975,19 +3976,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",
|
||||
|
||||
@@ -8,6 +8,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"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
@@ -378,34 +379,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
|
||||
@@ -461,12 +539,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")
|
||||
@@ -702,8 +784,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.
|
||||
@@ -716,12 +798,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.GetPreAuthKey(ephKey.Key)
|
||||
require.NoError(t, err)
|
||||
|
||||
seeded, ok := n.s.nodeStore.UpdateNode(n.id, func(nd *types.Node) {
|
||||
nd.RegisterMethod = util.RegisterMethodAuthKey
|
||||
nd.AuthKey = pak
|
||||
nd.AuthKeyID = &pak.ID
|
||||
})
|
||||
@@ -1622,3 +1705,155 @@ func TestTaggingPreservesNodeExpiry(t *testing.T) {
|
||||
require.NotNil(t, tagged.AsStruct().Expiry, "tag change must not clear expiry")
|
||||
require.Equal(t, expiry.Unix(), tagged.AsStruct().Expiry.Unix())
|
||||
}
|
||||
|
||||
// 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) {
|
||||
type nodeFields struct {
|
||||
NodeKey key.NodePublic
|
||||
Hostname string
|
||||
AuthKeyID *uint64
|
||||
Expiry int64
|
||||
Tags []string
|
||||
UserID *uint
|
||||
}
|
||||
|
||||
fieldsOf := func(n *types.Node) nodeFields {
|
||||
f := nodeFields{
|
||||
NodeKey: n.NodeKey,
|
||||
Hostname: n.Hostname,
|
||||
AuthKeyID: n.AuthKeyID,
|
||||
Tags: n.Tags,
|
||||
UserID: n.UserID,
|
||||
}
|
||||
if n.Expiry != nil {
|
||||
f.Expiry = n.Expiry.UnixNano()
|
||||
}
|
||||
|
||||
return f
|
||||
}
|
||||
|
||||
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)
|
||||
|
||||
machineKey := key.NewMachine().Public()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: original.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "h0"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey)
|
||||
require.NoError(t, err)
|
||||
|
||||
fields := func() (nodeFields, nodeFields) {
|
||||
t.Helper()
|
||||
|
||||
ns, ok := s.GetNodeByID(node.ID())
|
||||
require.True(t, ok)
|
||||
|
||||
dbNode, err := s.db.GetNodeByID(node.ID())
|
||||
require.NoError(t, err)
|
||||
|
||||
return fieldsOf(ns.AsStruct()), fieldsOf(dbNode)
|
||||
}
|
||||
|
||||
if adminTagged {
|
||||
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:prod":["former-owner@"]}}`))
|
||||
require.NoError(t, err)
|
||||
_, _, err = s.SetNodeTags(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 := regReq
|
||||
req.Hostinfo = &tailcfg.Hostinfo{Hostname: "h1"}
|
||||
req.Auth = &tailcfg.RegisterResponseAuth{AuthKey: presented.Key}
|
||||
|
||||
switch mode {
|
||||
case "expired":
|
||||
past := time.Now().Add(-time.Hour)
|
||||
_, _, err = s.SetNodeExpiry(node.ID(), &past)
|
||||
require.NoError(t, err)
|
||||
case "rotated":
|
||||
req.NodeKey = key.NewNode().Public()
|
||||
}
|
||||
|
||||
beforeCache, beforeDB := fields()
|
||||
|
||||
got, _, err := s.HandleNodeFromPreAuthKey(req, machineKey)
|
||||
|
||||
afterCache, afterDB := fields()
|
||||
assert.Equal(t, afterCache, afterDB, "NodeStore and database agree")
|
||||
|
||||
storedKey, keyErr := s.GetPreAuthKey(presented.Key)
|
||||
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, 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, 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)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
+70
-39
@@ -2252,10 +2252,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
|
||||
|
||||
@@ -2268,14 +2265,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{}, change.Change{}, ErrAmbiguousNodeOwnership
|
||||
}
|
||||
|
||||
// Create logger with common fields for all auth operations
|
||||
logger := log.With().
|
||||
Str(zf.RegistrationID, authID.String()).
|
||||
@@ -2299,15 +2288,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{}, change.Change{}, err
|
||||
}
|
||||
} else if existingNodeIsTagged {
|
||||
updateParams.ExistingNode = taggedNode
|
||||
} else if nodeExists {
|
||||
updateParams.ExistingNode = existingNode
|
||||
updateParams.IsConvertFromTag = true
|
||||
|
||||
finalNode, _, err = s.applyAuthNodeUpdate(updateParams)
|
||||
@@ -2403,31 +2392,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);
|
||||
// HandleNodeFromPreAuthKey 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
|
||||
@@ -2465,6 +2450,25 @@ func (s *State) findExistingNodeForPAK(
|
||||
return types.NodeView{}, false, nil
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
|
||||
func (s *State) HandleNodeFromPreAuthKey(
|
||||
regReq tailcfg.RegisterRequest,
|
||||
@@ -2488,7 +2492,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
return types.TaggedDevices.Name
|
||||
}
|
||||
|
||||
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak)
|
||||
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak, regReq.NodeKey)
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
}
|
||||
@@ -2537,6 +2541,11 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
pak.IsTagged() && existingNodeSameUser.IsTagged() &&
|
||||
(!existingNodeSameUser.AuthKeyID().Valid() || existingNodeSameUser.AuthKeyID().Get() != pak.ID)
|
||||
|
||||
// Only a key that speaks for the node re-authenticates it. Any other key
|
||||
// (another user's key on a tagged machine, under the node's own node key)
|
||||
// only wakes it.
|
||||
reauth := isExistingNodeReregistering && pak.CanReauthenticate(existingNodeSameUser)
|
||||
|
||||
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion && !isRetag {
|
||||
// Existing, still-valid node re-registering with same NodeKey: skip
|
||||
// validation. Pre-auth keys are only needed for initial authentication.
|
||||
@@ -2561,6 +2570,10 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
}
|
||||
|
||||
if isExistingNodeReregistering && !reauth {
|
||||
return types.NodeView{}, change.Change{}, types.PAKError("authkey cannot re-authenticate this node")
|
||||
}
|
||||
}
|
||||
|
||||
// Preserve the raw hostname as reported by the client. Sanitisation
|
||||
@@ -2621,7 +2634,6 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
|
||||
// Update existing node - NodeStore first, then database
|
||||
before, updatedNodeView, ok := s.nodeStore.UpdateNodeDiff(existingNodeSameUser.ID(), func(node *types.Node) {
|
||||
node.NodeKey = regReq.NodeKey
|
||||
node.Hostname = hostname
|
||||
|
||||
// TODO(kradalby): We should ensure we use the same hostinfo and node merge semantics
|
||||
@@ -2631,6 +2643,16 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
node.Hostinfo = validHostinfo
|
||||
node.Hostinfo.NetInfo = preserveNetInfo(existingNodeSameUser, existingNodeSameUser.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(time.Now())
|
||||
|
||||
if !reauth {
|
||||
return
|
||||
}
|
||||
|
||||
node.NodeKey = regReq.NodeKey
|
||||
|
||||
node.RegisterMethod = util.RegisterMethodAuthKey
|
||||
|
||||
// Tags from a PreAuthKey are applied on initial registration and
|
||||
@@ -2664,12 +2686,17 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
if wasUserOwned || node.IsExpired() {
|
||||
node.Expiry = nil
|
||||
}
|
||||
} else if node.IsTagged() && !pak.IsTagged() && isNodeKeyRotation {
|
||||
// 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
|
||||
node.AuthKeyID = &pak.ID
|
||||
// Preserve online state during re-registration so a live node does
|
||||
// not appear offline before the client restarts its map stream.
|
||||
node.LastSeen = new(time.Now())
|
||||
|
||||
// Tagged nodes keep their existing expiry (disabled).
|
||||
// User-owned nodes update expiry from the client request,
|
||||
@@ -2704,7 +2731,11 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
// (#2862), but re-registration presents a freshly-validated key, 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 {
|
||||
@@ -2717,7 +2748,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
// reject it as "authkey already used". This is the path
|
||||
// behind issue #2830 where containers 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)
|
||||
|
||||
@@ -1012,6 +1012,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() {
|
||||
|
||||
@@ -150,6 +150,29 @@ func (pak *PreAuthKey) IsTagged() bool {
|
||||
return len(pak.Tags) > 0
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -6,6 +6,8 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/google/go-cmp/cmp"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestCanUsePreAuthKey(t *testing.T) {
|
||||
@@ -127,3 +129,50 @@ func TestCanUsePreAuthKey(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
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: &PreAuthKey{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: &PreAuthKey{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
@@ -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 uint64
|
||||
|
||||
// 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].GetId()
|
||||
t.Logf("Initial state - Node ID: %d, Tags: %v", nodes[0].GetId(), nodes[0].GetTags())
|
||||
}
|
||||
}, 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].GetUser().GetName(),
|
||||
"Node ownership should be returned to user after untagging")
|
||||
t.Logf("After conversion - Node ID: %d, Tags: %v, User: %s",
|
||||
nodes[0].GetId(), nodes[0].GetTags(), nodes[0].GetUser().GetName())
|
||||
for _, node := range nodes {
|
||||
if node.GetId() == taggedID {
|
||||
assertNodeHasTagsWithCollect(c, node, []string{"tag:valid-owned"})
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
assertNodeHasNoTagsWithCollect(c, node)
|
||||
assert.Equal(c, tagTestUser, node.GetUser().GetName(), "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
|
||||
|
||||
Reference in new issue
Block a user