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.
This commit is contained in:
Kristoffer Dalby
2026-06-07 07:35:17 +00:00
committed by Kristoffer Dalby
parent 8237ac662a
commit 4914f9f2fd
2 changed files with 77 additions and 0 deletions
+64
View File
@@ -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")
}
+13
View File
@@ -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