mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-08 17:10:21 +09:00
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:
@@ -6,8 +6,10 @@ import (
|
|||||||
|
|
||||||
"github.com/juanfont/headscale/hscontrol/db"
|
"github.com/juanfont/headscale/hscontrol/db"
|
||||||
"github.com/juanfont/headscale/hscontrol/types"
|
"github.com/juanfont/headscale/hscontrol/types"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/util"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
"tailscale.com/types/key"
|
||||||
)
|
)
|
||||||
|
|
||||||
// persistTestSetup pre-creates a sqlite database on disk with a single
|
// 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,
|
assert.Empty(t, nv.AsStruct().Endpoints,
|
||||||
"after restart, NodeStore should reflect the cleared 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")
|
||||||
|
}
|
||||||
|
|||||||
@@ -1664,6 +1664,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
|
// Prepare the node for registration
|
||||||
nodeToRegister := types.Node{
|
nodeToRegister := types.Node{
|
||||||
Hostname: params.Hostname,
|
Hostname: params.Hostname,
|
||||||
|
|||||||
Reference in New Issue
Block a user