From a73d38bb3f3b0f9d9a1a090574c3e1774d88fc36 Mon Sep 17 00:00:00 2001
From: Kristoffer Dalby <kristoffer@tailscale.com>
Date: Mon, 15 Jun 2026 10:58:42 +0000
Subject: [PATCH] state: reject re-registration claiming another node's key

The pre-auth-key path wrote the client node key without the collision check
the auth path applies, so a re-registration could claim another node's key
and poison the node-key index. Reject keys bound to a different machine.

Updates #3312
---
 hscontrol/state/persist_test.go | 57 +++++++++++++++++++++++++++++++++
 hscontrol/state/state.go        | 11 +++++++
 2 files changed, 68 insertions(+)

diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go
index ab35d9a4..e29de839 100644
--- a/hscontrol/state/persist_test.go
+++ b/hscontrol/state/persist_test.go
@@ -3,6 +3,7 @@ package state
 import (
 	"net/netip"
 	"testing"
+	"time"
 
 	"github.com/juanfont/headscale/hscontrol/db"
 	"github.com/juanfont/headscale/hscontrol/types"
@@ -415,3 +416,59 @@ func TestReauthChange(t *testing.T) {
 	assert.Empty(t, pol.PeersChanged)
 	assert.False(t, pol.IsEmpty(), "a policy change must be non-empty")
 }
+
+// TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine is the pre-auth-key
+// analogue of TestReauthRejectsNodeKeyClaimedByAnotherMachine: re-registering
+// via a pre-auth key must enforce the same 1:1 NodeKey<->MachineKey binding the
+// auth path and poll-time validation enforce, so a node cannot rotate its key
+// to a victim's and poison the NodeStore NodeKey index.
+func TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
+	dbPath := t.TempDir() + "/headscale.db"
+	cfg := persistTestConfig(dbPath)
+
+	s, err := NewState(cfg)
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = s.Close() })
+
+	attacker := s.CreateUserForTest("attacker")
+	victim := s.CreateUserForTest("victim")
+
+	victimMachine := key.NewMachine()
+	victimNodeKey := key.NewNode()
+	_, err = s.createAndSaveNewNode(newNodeParams{
+		User:           *victim,
+		MachineKey:     victimMachine.Public(),
+		NodeKey:        victimNodeKey.Public(),
+		DiscoKey:       key.NewDisco().Public(),
+		Hostname:       "victim",
+		RegisterMethod: util.RegisterMethodCLI,
+	})
+	require.NoError(t, err)
+
+	// Attacker registers its own node with a reusable pre-auth key.
+	pak, err := s.CreatePreAuthKey(attacker.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	attackerMachine := key.NewMachine()
+	regReq := tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  key.NewNode().Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "attacker"},
+		Expiry:   time.Now().Add(24 * time.Hour),
+	}
+	_, _, err = s.HandleNodeFromPreAuthKey(regReq, attackerMachine.Public())
+	require.NoError(t, err)
+
+	// Attacker re-registers its own node but supplies the victim's NodeKey.
+	attack := regReq
+	attack.NodeKey = victimNodeKey.Public()
+	_, _, err = s.HandleNodeFromPreAuthKey(attack, attackerMachine.Public())
+	require.ErrorIs(t, err, ErrNodeKeyInUse,
+		"pre-auth-key re-registration claiming another machine's NodeKey must be rejected")
+
+	// The victim still owns its NodeKey.
+	owner, ok := s.GetNodeByNodeKey(victimNodeKey.Public())
+	require.True(t, ok)
+	require.Equal(t, victimMachine.Public(), owner.MachineKey(),
+		"victim's NodeKey index entry must be untouched")
+}
diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go
index c8b1c20f..f5427184 100644
--- a/hscontrol/state/state.go
+++ b/hscontrol/state/state.go
@@ -2301,6 +2301,17 @@ func (s *State) HandleNodeFromPreAuthKey(
 			Str(zf.UserName, pakUsername()).
 			Msg("Node re-registering with existing machine key and user, updating in place")
 
+		// Re-registration rotates the NodeKey to the client-supplied value.
+		// Enforce the same 1:1 NodeKey<->MachineKey binding the auth path
+		// (applyAuthNodeUpdate) and poll-time validation enforce: a NodeKey
+		// already bound to a different machine must not be claimed here, or a
+		// re-registering node could rotate its key to a victim's and poison the
+		// NodeStore NodeKey index, denying the victim service.
+		if existing, ok := s.nodeStore.GetNodeByNodeKey(regReq.NodeKey); ok &&
+			existing.MachineKey() != machineKey {
+			return types.NodeView{}, change.Change{}, ErrNodeKeyInUse
+		}
+
 		// Update existing node - NodeStore first, then database
 		updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
 			node.NodeKey = regReq.NodeKey