From 4914f9f2fd054b6d55c291d80ce08c847a9437bd Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sun, 7 Jun 2026 07:35:17 +0000 Subject: [PATCH] state: reject re-auth claiming another machine's NodeKey applyAuthNodeUpdate rotated the node's NodeKey to the client-supplied value without the 1:1 NodeKey/MachineKey check createAndSaveNewNode (f8f08cf7) and getAndValidateNode enforce. A re-authenticating node could thus rotate its key to a victim's and poison the NodeStore NodeKey index, denying the victim service. Apply the same uniqueness check on the re-auth path. --- hscontrol/state/persist_test.go | 64 +++++++++++++++++++++++++++++++++ hscontrol/state/state.go | 13 +++++++ 2 files changed, 77 insertions(+) diff --git a/hscontrol/state/persist_test.go b/hscontrol/state/persist_test.go index 10f5080a..cecc17dc 100644 --- a/hscontrol/state/persist_test.go +++ b/hscontrol/state/persist_test.go @@ -9,6 +9,7 @@ import ( "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/tailcfg" "tailscale.com/types/key" ) @@ -259,3 +260,66 @@ func TestRegistrationRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) { require.Error(t, err, "registering a NodeKey already bound to another machine must be rejected") } + +// TestReauthRejectsNodeKeyClaimedByAnotherMachine proves the re-auth/update +// path enforces the same 1:1 NodeKey<->MachineKey binding as the create path +// (TestRegistrationRejectsNodeKeyClaimedByAnotherMachine) and the poll path +// (getAndValidateNode). Without it, a node re-authenticating could rotate its +// NodeKey to a victim's, poisoning the NodeStore NodeKey index so the victim's +// MapRequest resolves to the attacker's node and is rejected — a DoS. +func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) { + dbPath := t.TempDir() + "/headscale.db" + cfg := persistTestConfig(dbPath) + + database, err := db.NewHeadscaleDatabase(cfg) + require.NoError(t, err) + attacker := database.CreateUserForTest("attacker") + victim := database.CreateUserForTest("victim") + require.NoError(t, database.Close()) + + s, err := NewState(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = s.Close() }) + + victimNodeKey := key.NewNode() + attackerMachine := key.NewMachine() + + // Victim's node holds victimNodeKey. + _, err = s.createAndSaveNewNode(newNodeParams{ + User: *victim, + MachineKey: key.NewMachine().Public(), + NodeKey: victimNodeKey.Public(), + DiscoKey: key.NewDisco().Public(), + Hostname: "victim", + RegisterMethod: util.RegisterMethodCLI, + }) + require.NoError(t, err) + + // Attacker registers its own node. + attackerNode, err := s.createAndSaveNewNode(newNodeParams{ + User: *attacker, + MachineKey: attackerMachine.Public(), + NodeKey: key.NewNode().Public(), + DiscoKey: key.NewDisco().Public(), + Hostname: "attacker", + RegisterMethod: util.RegisterMethodCLI, + }) + require.NoError(t, err) + + // Attacker re-authenticates its own node but supplies the victim's NodeKey. + _, err = s.applyAuthNodeUpdate(authNodeUpdateParams{ + ExistingNode: attackerNode, + RegData: &types.RegistrationData{ + MachineKey: attackerMachine.Public(), + NodeKey: victimNodeKey.Public(), + Hostname: "attacker", + Hostinfo: &tailcfg.Hostinfo{}, + }, + ValidHostinfo: &tailcfg.Hostinfo{}, + Hostname: "attacker", + User: attacker, + RegisterMethod: util.RegisterMethodCLI, + }) + require.Error(t, err, + "re-auth claiming a NodeKey bound to another machine must be rejected") +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 39089cd7..421da1b2 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -1563,6 +1563,19 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView ) } + // Re-auth rotates the NodeKey to the client-supplied value. Enforce the + // same 1:1 NodeKey<->MachineKey binding createAndSaveNewNode applies at + // registration and getAndValidateNode enforces at poll time: a NodeKey + // already bound to a different machine must not be claimed here, or a + // re-authenticating 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(regData.NodeKey); ok && + existing.MachineKey() != regData.MachineKey { + return types.NodeView{}, fmt.Errorf( + "node key already in use by another machine", + ) + } + // Update existing node in [NodeStore] - validation passed, safe to mutate updatedNodeView, ok := s.nodeStore.UpdateNode(params.ExistingNode.ID(), func(node *types.Node) { node.NodeKey = regData.NodeKey