Mirrors/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2026-01-22 18:18:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 7

hscontrol: handle tags-only PreAuthKeys in registration

Browse source 
HandleNodeFromPreAuthKey assumed pak.User was always set, but
tags-only PreAuthKeys have nil User. This caused nil pointer
dereference when registering nodes with tags-only keys.

Also updates integration tests to use GetTags() instead of the
removed GetValidTags() method.

Updates #2977
This commit is contained in:
Kristoffer Dalby 2026-01-14 08:55:58 +00:00
parent 165c5f0491
commit 4ab06930a2
2 changed files with 42 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

50
hscontrol/state/state.go
View file

@ -1557,11 +1557,29 @@ func (s *State) HandleNodeFromPreAuthKey(
return types.NodeView{}, change.Change{}, err
}
// Helper to get username for logging (handles nil User for tags-only keys)
pakUsername := func() string {
if pak.User != nil {
return pak.User.Username()
}
return types.TaggedDevices.Name
}
// Check if node exists with same machine key before validating the key.
// For #2830: container restarts send the same pre-auth key which may be used/expired.
// Skip validation for existing nodes re-registering with the same NodeKey, as the
// key was only needed for initial authentication. NodeKey rotation requires validation.
existingNodeSameUser, existsSameUser := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
//
// For tags-only keys (pak.User == nil), we skip the user-based lookup since there's
// no user to match against. These keys create tagged nodes without user ownership.
var existingNodeSameUser types.NodeView
var existsSameUser bool
if pak.User != nil {
existingNodeSameUser, existsSameUser = s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
}
// For existing nodes, skip validation if:
// 1. MachineKey matches (cryptographic proof of machine identity)
@ -1574,6 +1592,8 @@ func (s *State) HandleNodeFromPreAuthKey(
// - Container restarts may use different PAKs (e.g., env var changed)
// - Original PAK may be deleted
// - MachineKey + User is sufficient to prove this is the same node
//
// Note: For tags-only keys, existsSameUser is always false, so we always validate.
isExistingNodeReregistering := existsSameUser && existingNodeSameUser.Valid()
// Check if this is a NodeKey rotation (different NodeKey)
@ -1619,7 +1639,7 @@ func (s *State) HandleNodeFromPreAuthKey(
logHostinfoValidation(
machineKey.ShortString(),
regReq.NodeKey.ShortString(),
pak.User.Username(),
pakUsername(),
hostname,
regReq.Hostinfo,
)
@ -1629,12 +1649,13 @@ func (s *State) HandleNodeFromPreAuthKey(
Str("node.name", hostname).
Str("machine.key", machineKey.ShortString()).
Str("node.key", regReq.NodeKey.ShortString()).
Str("user.name", pak.User.Username()).
Str("user.name", pakUsername()).
Msg("Registering node with pre-auth key")
var finalNode types.NodeView
// If this node exists for this user, update the node in place.
// Note: For tags-only keys (pak.User == nil), existsSameUser is always false.
if existsSameUser && existingNodeSameUser.Valid() {
log.Trace().
Caller().
@ -1642,7 +1663,7 @@ func (s *State) HandleNodeFromPreAuthKey(
Uint64("node.id", existingNodeSameUser.ID().Uint64()).
Str("machine.key", machineKey.ShortString()).
Str("node.key", existingNodeSameUser.NodeKey().ShortString()).
Str("user.name", pak.User.Username()).
Str("user.name", pakUsername()).
Msg("Node re-registering with existing machine key and user, updating in place")
// Update existing node - NodeStore first, then database
@ -1706,7 +1727,7 @@ func (s *State) HandleNodeFromPreAuthKey(
Uint64("node.id", updatedNodeView.ID().Uint64()).
Str("machine.key", machineKey.ShortString()).
Str("node.key", updatedNodeView.NodeKey().ShortString()).
Str("user.name", pak.User.Username()).
Str("user.name", pakUsername()).
Msg("Node re-authorized")
finalNode = updatedNodeView
@ -1715,7 +1736,9 @@ func (s *State) HandleNodeFromPreAuthKey(
// Check if node exists with this machine key for a different user
existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
if existsAnyUser && existingNodeAnyUser.Valid() && existingNodeAnyUser.UserID().Get() != pak.User.ID {
// For user-owned keys, check if node exists for a different user
// For tags-only keys (pak.User == nil), this check is skipped
if pak.User != nil && existsAnyUser && existingNodeAnyUser.Valid() && existingNodeAnyUser.UserID().Get() != pak.User.ID {
// Node exists but belongs to a different user
// Create a NEW node for the new user (do not transfer)
// This allows the same machine to have separate node identities per user
@ -1726,17 +1749,24 @@ func (s *State) HandleNodeFromPreAuthKey(
Uint64("existing.node.id", existingNodeAnyUser.ID().Uint64()).
Str("machine.key", machineKey.ShortString()).
Str("old.user", oldUser.Name()).
Str("new.user", pak.User.Username()).
Str("new.user", pakUsername()).
Msg("Creating new node for different user (same machine key exists for another user)")
}
// This is a new node for this user - create it
// (Either completely new, or new for this user while existing for another user)
// This is a new node - create it
// For user-owned keys: create for the user
// For tags-only keys: create as tagged node (createAndSaveNewNode handles this via PreAuthKey)
// Create and save new node
// Note: For tags-only keys, User is empty but createAndSaveNewNode uses PreAuthKey for ownership
var pakUser types.User
if pak.User != nil {
pakUser = *pak.User
}
var err error
finalNode, err = s.createAndSaveNewNode(newNodeParams{
User: *pak.User,
User: pakUser,
MachineKey: machineKey,
NodeKey: regReq.NodeKey,
DiscoKey: key.DiscoPublic{}, // DiscoKey not available in RegisterRequest

4
integration/tags_test.go
View file

@ -3046,7 +3046,7 @@ func TestTagsAuthKeyWithoutUserInheritsTags(t *testing.T) {
if len(nodes) == 1 {
node := nodes[0]
t.Logf("Node registered with tags: %v", node.GetValidTags())
t.Logf("Node registered with tags: %v", node.GetTags())
assertNodeHasTagsWithCollect(c, node, []string{"tag:valid-owned"})
}
}, 30*time.Second, 500*time.Millisecond, "verifying node inherited tags from auth key")
@ -3117,7 +3117,7 @@ func TestTagsAuthKeyWithoutUserIgnoresAdvertisedTags(t *testing.T) {
if len(nodes) == 1 {
node := nodes[0]
t.Logf("Node registered with tags: %v (advertised: tag:second)", node.GetValidTags())
t.Logf("Node registered with tags: %v (advertised: tag:second)", node.GetTags())
// Should have auth key's tags, NOT the advertised tags
assertNodeHasTagsWithCollect(c, node, []string{"tag:valid-owned"})
}