state: reject registration claiming another machine's NodeKey

createAndSaveNewNode trusted the client NodeKey without checking it was already bound to a different machine, so an authenticated party could register a node carrying a victim's public NodeKey, poison the NodeStore NodeKey index, and make the victim's MapRequest resolve to the wrong node (rejected by getAndValidateNode = DoS). Enforce the 1:1 NodeKey/MachineKey binding at registration, as poll time already does.
This commit is contained in:
Kristoffer Dalby
2026-06-06 20:42:02 +00:00
committed by Kristoffer Dalby
parent 56cd3eb24d
commit eb57a3a62b
2 changed files with 66 additions and 0 deletions
+50
View File
@@ -6,8 +6,10 @@ import (
"github.com/juanfont/headscale/hscontrol/db"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"tailscale.com/types/key"
)
// persistTestSetup pre-creates a sqlite database on disk with a single
@@ -209,3 +211,51 @@ func TestPersistEmptyEndpoints(t *testing.T) {
assert.Empty(t, nv.AsStruct().Endpoints,
"after restart, NodeStore should reflect the cleared endpoints")
}
// TestRegistrationRejectsNodeKeyClaimedByAnotherMachine proves a new
// registration cannot claim a NodeKey already bound to a different machine.
// NodeKeys are public (peers learn them from the netmap), so without this
// check an authenticated party can register a node carrying a victim's
// NodeKey. That poisons the NodeStore NodeKey index (a map keyed on NodeKey,
// last writer wins), so the victim's MapRequest resolves to the attacker's
// node and is rejected by getAndValidateNode's MachineKey check (noise.go) —
// a denial of service against the victim. getAndValidateNode already enforces
// a 1:1 NodeKey<->MachineKey binding at poll time; this enforces the same
// invariant at registration time.
func TestRegistrationRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
database, err := db.NewHeadscaleDatabase(cfg)
require.NoError(t, err)
user := database.CreateUserForTest("nk-user")
require.NoError(t, database.Close())
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
sharedNodeKey := key.NewNode()
_, err = s.createAndSaveNewNode(newNodeParams{
User: *user,
MachineKey: key.NewMachine().Public(),
NodeKey: sharedNodeKey.Public(),
DiscoKey: key.NewDisco().Public(),
Hostname: "victim",
RegisterMethod: util.RegisterMethodCLI,
})
require.NoError(t, err)
// A different machine tries to register carrying the victim's NodeKey.
_, err = s.createAndSaveNewNode(newNodeParams{
User: *user,
MachineKey: key.NewMachine().Public(),
NodeKey: sharedNodeKey.Public(),
DiscoKey: key.NewDisco().Public(),
Hostname: "attacker",
RegisterMethod: util.RegisterMethodCLI,
})
require.Error(t, err,
"registering a NodeKey already bound to another machine must be rejected")
}
+16
View File
@@ -1690,6 +1690,22 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
)
}
// Enforce NodeKey uniqueness across machines. NodeKeys are public
// (peers learn them from the netmap), so an authenticated party could
// otherwise register a node carrying a victim's NodeKey, poisoning the
// NodeStore NodeKey index so the victim's MapRequest resolves to the
// wrong node and is rejected by getAndValidateNode's MachineKey check
// (a DoS). createAndSaveNewNode only runs for a machine that has no
// existing node, so any current holder of this NodeKey is a different
// machine; mirror the 1:1 binding getAndValidateNode enforces at poll
// time and reject before allocating any resources.
if existing, ok := s.nodeStore.GetNodeByNodeKey(params.NodeKey); ok &&
existing.MachineKey() != params.MachineKey {
return types.NodeView{}, fmt.Errorf(
"node key already in use by another machine",
)
}
// Prepare the node for registration
nodeToRegister := types.Node{
Hostname: params.Hostname,