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

@@ -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")
+}

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