mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-08 00:50:20 +09:00
state: return all nodes for a machine key, reject ambiguous ownership
Collapse the single-pick machine-key lookups onto GetNodesByMachineKeyAllUsers so callers see every node sharing a machine key and reject the ambiguous or impossible cases (tagged and user-owned coexistence; a tagged key with several user-owned candidates) instead of mutating an arbitrarily-picked node. Updates #3312
This commit is contained in:
committed by
Kristoffer Dalby
parent
a1d3e98255
commit
1689478485
+13
-13
@@ -1699,7 +1699,7 @@ func TestAuthenticationFlows(t *testing.T) {
|
||||
assert.False(t, resp.NodeKeyExpired)
|
||||
|
||||
// Verify NEW node was created for user2
|
||||
node2, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
|
||||
node2, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
|
||||
require.True(t, found, "new node should exist for user2")
|
||||
assert.Equal(t, uint(2), node2.UserID().Get(), "new node should belong to user2")
|
||||
|
||||
@@ -1707,7 +1707,7 @@ func TestAuthenticationFlows(t *testing.T) {
|
||||
assert.Equal(t, "user2-context", user.Name(), "new node should show user2 username")
|
||||
|
||||
// Verify original node still exists for user1
|
||||
node1, found := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
|
||||
node1, found := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
|
||||
require.True(t, found, "original node should still exist for user1")
|
||||
assert.Equal(t, uint(1), node1.UserID().Get(), "original node should still belong to user1")
|
||||
|
||||
@@ -1775,13 +1775,13 @@ func TestAuthenticationFlows(t *testing.T) {
|
||||
validateCompleteResponse: true,
|
||||
validate: func(t *testing.T, resp *tailcfg.RegisterResponse, app *Headscale) { //nolint:thelper
|
||||
// User1's original node should STILL exist (not transferred)
|
||||
node1, found1 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(1))
|
||||
node1, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(1)]
|
||||
require.True(t, found1, "user1's original node should still exist")
|
||||
assert.Equal(t, uint(1), node1.UserID().Get(), "user1's node should still belong to user1")
|
||||
assert.Equal(t, nodeKey1.Public(), node1.NodeKey(), "user1's node should have original node key")
|
||||
|
||||
// User2 should have a NEW node created
|
||||
node2, found2 := app.state.GetNodeByMachineKey(machineKey1.Public(), types.UserID(2))
|
||||
node2, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey1.Public())[types.UserID(2)]
|
||||
require.True(t, found2, "user2 should have new node created")
|
||||
assert.Equal(t, uint(2), node2.UserID().Get(), "user2's node should belong to user2")
|
||||
|
||||
@@ -2914,7 +2914,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
|
||||
for i := range 2 {
|
||||
node := nodes[i]
|
||||
// User1's original nodes should still be owned by user1
|
||||
registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
|
||||
registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
|
||||
require.True(t, found, "User1's original node %s should still exist", node.hostname)
|
||||
require.Equal(t, user1.ID, registeredNode.UserID().Get(), "Node %s should still belong to user1", node.hostname)
|
||||
t.Logf("✓ User1's original node %s (ID=%d) still owned by user1", node.hostname, registeredNode.ID().Uint64())
|
||||
@@ -2923,7 +2923,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
|
||||
for i := 2; i < 4; i++ {
|
||||
node := nodes[i]
|
||||
// User2's original nodes should still be owned by user2
|
||||
registeredNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user2.ID))
|
||||
registeredNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user2.ID)]
|
||||
require.True(t, found, "User2's original node %s should still exist", node.hostname)
|
||||
require.Equal(t, user2.ID, registeredNode.UserID().Get(), "Node %s should still belong to user2", node.hostname)
|
||||
t.Logf("✓ User2's original node %s (ID=%d) still owned by user2", node.hostname, registeredNode.ID().Uint64())
|
||||
@@ -2935,7 +2935,7 @@ func TestPreAuthKeyLogoutAndReloginDifferentUser(t *testing.T) {
|
||||
for i := 2; i < 4; i++ {
|
||||
node := nodes[i]
|
||||
// Should be able to find a node with user1 and this machine key (the new one)
|
||||
newNode, found := app.state.GetNodeByMachineKey(node.machineKey.Public(), types.UserID(user1.ID))
|
||||
newNode, found := app.state.GetNodesByMachineKeyAllUsers(node.machineKey.Public())[types.UserID(user1.ID)]
|
||||
require.True(t, found, "Should have created new node for user1 with machine key from %s", node.hostname)
|
||||
require.Equal(t, user1.ID, newNode.UserID().Get(), "New node should belong to user1")
|
||||
t.Logf("✓ New node created for user1 with machine key from %s (ID=%d)", node.hostname, newNode.ID().Uint64())
|
||||
@@ -2984,7 +2984,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
|
||||
require.True(t, resp1.MachineAuthorized, "Should be authorized via pre-auth key")
|
||||
|
||||
// Verify node exists for user1
|
||||
user1Node, found := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
|
||||
user1Node, found := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
|
||||
require.True(t, found, "Node should exist for user1")
|
||||
require.Equal(t, user1.ID, user1Node.UserID().Get(), "Node should belong to user1")
|
||||
user1NodeID := user1Node.ID()
|
||||
@@ -3000,7 +3000,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify node is expired
|
||||
user1Node, found = app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
|
||||
user1Node, found = app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
|
||||
require.True(t, found, "Node should still exist after logout")
|
||||
require.True(t, user1Node.IsExpired(), "Node should be expired after logout")
|
||||
t.Logf("✓ User1 node expired (logged out)")
|
||||
@@ -3041,7 +3041,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
|
||||
|
||||
t.Run("user1_original_node_still_exists", func(t *testing.T) {
|
||||
// User1's original node should STILL exist (not transferred to user2)
|
||||
user1NodeAfter, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
|
||||
user1NodeAfter, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
|
||||
assert.True(t, found1, "User1's original node should still exist (not transferred)")
|
||||
|
||||
if !found1 {
|
||||
@@ -3056,7 +3056,7 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
|
||||
|
||||
t.Run("user2_has_new_node_created", func(t *testing.T) {
|
||||
// User2 should have a NEW node created (not transfer from user1)
|
||||
user2Node, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
|
||||
user2Node, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
|
||||
assert.True(t, found2, "User2 should have a new node created")
|
||||
|
||||
if !found2 {
|
||||
@@ -3080,8 +3080,8 @@ func TestWebFlowReauthDifferentUser(t *testing.T) {
|
||||
|
||||
t.Run("both_nodes_share_machine_key", func(t *testing.T) {
|
||||
// Both nodes should have the same machine key (same physical device)
|
||||
user1NodeFinal, found1 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user1.ID))
|
||||
user2NodeFinal, found2 := app.state.GetNodeByMachineKey(machineKey.Public(), types.UserID(user2.ID))
|
||||
user1NodeFinal, found1 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user1.ID)]
|
||||
user2NodeFinal, found2 := app.state.GetNodesByMachineKeyAllUsers(machineKey.Public())[types.UserID(user2.ID)]
|
||||
|
||||
require.True(t, found1, "User1 node should exist")
|
||||
require.True(t, found2, "User2 node should exist")
|
||||
|
||||
@@ -319,6 +319,95 @@ func TestTaggedPAKReauthConvertsUserOwnedNode(t *testing.T) {
|
||||
require.Equal(t, 1, s.ListNodes().Len(), "machine must map to exactly one node")
|
||||
}
|
||||
|
||||
// registerTwoUsersOnOneMachine registers two user-owned nodes that share a
|
||||
// machine key (the "create new, do not transfer" multi-user device state) and
|
||||
// returns the State and the shared machine key.
|
||||
func registerTwoUsersOnOneMachine(t *testing.T) (*State, key.MachinePublic, types.NodeID) {
|
||||
t.Helper()
|
||||
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
u1 := s.CreateUserForTest("u1")
|
||||
u2 := s.CreateUserForTest("u2")
|
||||
mk := key.NewMachine()
|
||||
|
||||
reg := func(pakUser *types.User) types.NodeView {
|
||||
pak, err := s.CreatePreAuthKey(pakUser.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
n, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}, mk.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
return n
|
||||
}
|
||||
|
||||
n1 := reg(u1)
|
||||
n2 := reg(u2)
|
||||
require.NotEqual(t, n1.ID(), n2.ID(), "precondition: two distinct nodes share the machine key")
|
||||
require.Equal(t, 2, s.ListNodes().Len())
|
||||
|
||||
return s, mk.Public(), n1.ID()
|
||||
}
|
||||
|
||||
// TestTaggedPAKReauthRejectsAmbiguousMultiUserNode: a tagged pre-auth key on a
|
||||
// machine that has more than one user-owned node cannot know which to convert,
|
||||
// so the registration is rejected rather than converting an arbitrary one and
|
||||
// orphaning the rest.
|
||||
func TestTaggedPAKReauthRejectsAmbiguousMultiUserNode(t *testing.T) {
|
||||
s, mk, _ := registerTwoUsersOnOneMachine(t)
|
||||
|
||||
taggedPak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
|
||||
_, _, err = s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: taggedPak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}, mk)
|
||||
require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
|
||||
require.Equal(t, 2, s.ListNodes().Len(), "no node created or converted on rejection")
|
||||
}
|
||||
|
||||
// TestAuthPathRejectsTaggedAndUserCoexistence: if a machine key ends up with
|
||||
// both a tagged node and a user-owned node (impossible per validateNodeOwnership,
|
||||
// but reachable by tagging one node of a multi-user device via the admin path),
|
||||
// an OIDC re-auth must reject rather than silently converting the tagged node
|
||||
// and orphaning the user-owned one.
|
||||
func TestAuthPathRejectsTaggedAndUserCoexistence(t *testing.T) {
|
||||
s, mk, n1 := registerTwoUsersOnOneMachine(t)
|
||||
|
||||
// Tag one of the two user-owned nodes -> {0: tagged, u2: user-owned} coexist.
|
||||
_, err := s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["u1@"]}}`))
|
||||
require.NoError(t, err)
|
||||
tagged, _, err := s.SetNodeTags(n1, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
require.True(t, tagged.IsTagged())
|
||||
|
||||
// A third user authenticates the same machine via OIDC.
|
||||
u3 := s.CreateUserForTest("u3")
|
||||
regData := &types.RegistrationData{
|
||||
MachineKey: mk,
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostname: "multi",
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "multi"},
|
||||
}
|
||||
authID := types.MustAuthID()
|
||||
s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData))
|
||||
|
||||
_, _, err = s.HandleNodeFromAuthPath(authID, types.UserID(u3.ID), nil, util.RegisterMethodOIDC)
|
||||
require.ErrorIs(t, err, ErrAmbiguousNodeOwnership)
|
||||
}
|
||||
|
||||
// TestTaggedNodeCanHaveKeyExpiry matches Tailscale: a tagged node has key
|
||||
// expiry disabled by default, but it can still be set explicitly (e.g. via
|
||||
// `headscale nodes expire`).
|
||||
|
||||
@@ -787,53 +787,27 @@ func (s *NodeStore) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bo
|
||||
return nodeView, exists
|
||||
}
|
||||
|
||||
// GetNodeByMachineKey returns a node by its machine key and user ID. The bool indicates if the node exists.
|
||||
func (s *NodeStore) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
|
||||
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key"))
|
||||
defer timer.ObserveDuration()
|
||||
|
||||
nodeStoreOperations.WithLabelValues("get_by_machine_key").Inc()
|
||||
|
||||
snapshot := s.data.Load()
|
||||
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
|
||||
if node, exists := userMap[userID]; exists {
|
||||
return node, true
|
||||
}
|
||||
}
|
||||
|
||||
return types.NodeView{}, false
|
||||
}
|
||||
|
||||
// GetNodeByMachineKeyAnyUser returns a node with the given machine key,
|
||||
// regardless of which user it belongs to. This is useful for scenarios like
|
||||
// transferring a node to a different user when re-authenticating with a
|
||||
// different user's auth key.
|
||||
// GetNodesByMachineKeyAllUsers returns every node sharing machineKey, keyed by
|
||||
// owning UserID. Tagged nodes are indexed under UserID(0) (the tagged sentinel);
|
||||
// user-owned nodes under their owning UserID. Returns an empty map if none.
|
||||
//
|
||||
// When more than one node shares the machine key (e.g. a tagged node and a
|
||||
// user-owned node), the choice is deterministic: the tagged node is preferred,
|
||||
// otherwise the node with the lowest ID. A stable pick keeps re-auth branch
|
||||
// selection (convert vs create) from depending on map iteration order.
|
||||
func (s *NodeStore) GetNodeByMachineKeyAnyUser(machineKey key.MachinePublic) (types.NodeView, bool) {
|
||||
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key_any_user"))
|
||||
// One machine key can map to several nodes (the same device registered by
|
||||
// different users via the "create new, do not transfer" path). Exposing the
|
||||
// whole set lets callers decide with full context — index [userID] for an exact
|
||||
// match, [0] for a tagged node, or reject when the set is ambiguous — rather
|
||||
// than guessing from a single arbitrary pick.
|
||||
func (s *NodeStore) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
|
||||
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_nodes_by_machine_key_all_users"))
|
||||
defer timer.ObserveDuration()
|
||||
|
||||
nodeStoreOperations.WithLabelValues("get_by_machine_key_any_user").Inc()
|
||||
nodeStoreOperations.WithLabelValues("get_nodes_by_machine_key_all_users").Inc()
|
||||
|
||||
snapshot := s.data.Load()
|
||||
userMap := s.data.Load().nodesByMachineKey[machineKey]
|
||||
|
||||
var best types.NodeView
|
||||
out := make(map[types.UserID]types.NodeView, len(userMap))
|
||||
maps.Copy(out, userMap)
|
||||
|
||||
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
|
||||
for _, node := range userMap {
|
||||
if !best.Valid() ||
|
||||
(node.IsTagged() && !best.IsTagged()) ||
|
||||
(node.IsTagged() == best.IsTagged() && node.ID() < best.ID()) {
|
||||
best = node
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return best, best.Valid()
|
||||
return out
|
||||
}
|
||||
|
||||
// DebugString returns debug information about the [NodeStore].
|
||||
|
||||
@@ -1322,42 +1322,42 @@ func TestRebuildPeerMapsWithChangedPeersFunc(t *testing.T) {
|
||||
assert.Equal(t, 1, peers2.Len(), "ListPeers for node2 should return 1")
|
||||
}
|
||||
|
||||
// TestGetNodeByMachineKeyAnyUserDeterministic ensures the any-user machine-key
|
||||
// lookup returns a stable, well-defined node when more than one node shares a
|
||||
// machine key: the tagged node if present, otherwise the lowest node ID. A
|
||||
// nondeterministic pick makes re-auth branch choice (convert vs create) depend
|
||||
// on map iteration order.
|
||||
func TestGetNodeByMachineKeyAnyUserDeterministic(t *testing.T) {
|
||||
// TestGetNodesByMachineKeyAllUsers ensures the lookup returns every node sharing
|
||||
// a machine key keyed by owning UserID (tagged nodes under UserID(0)), so callers
|
||||
// see the full set instead of a single arbitrary pick.
|
||||
func TestGetNodesByMachineKeyAllUsers(t *testing.T) {
|
||||
mk := key.NewMachine().Public()
|
||||
|
||||
assertStablePick := func(t *testing.T, store *NodeStore, want types.NodeID) {
|
||||
t.Helper()
|
||||
|
||||
for range 50 {
|
||||
got, ok := store.GetNodeByMachineKeyAnyUser(mk)
|
||||
require.True(t, ok)
|
||||
require.Equal(t, want, got.ID(), "pick must be stable and well-defined")
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("prefers lowest node ID", func(t *testing.T) {
|
||||
t.Run("empty when absent", func(t *testing.T) {
|
||||
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
|
||||
|
||||
store.Start()
|
||||
defer store.Stop()
|
||||
|
||||
n2 := createTestNode(2, 2, "user2", "node2")
|
||||
n2.MachineKey = mk
|
||||
n1 := createTestNode(1, 1, "user1", "node1")
|
||||
n1.MachineKey = mk
|
||||
|
||||
store.PutNode(n2)
|
||||
store.PutNode(n1)
|
||||
|
||||
assertStablePick(t, store, 1)
|
||||
require.Empty(t, store.GetNodesByMachineKeyAllUsers(mk))
|
||||
})
|
||||
|
||||
t.Run("prefers tagged node", func(t *testing.T) {
|
||||
t.Run("returns all user-owned nodes keyed by user", func(t *testing.T) {
|
||||
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
|
||||
|
||||
store.Start()
|
||||
defer store.Stop()
|
||||
|
||||
n1 := createTestNode(1, 1, "user1", "node1")
|
||||
n1.MachineKey = mk
|
||||
n2 := createTestNode(2, 2, "user2", "node2")
|
||||
n2.MachineKey = mk
|
||||
|
||||
store.PutNode(n1)
|
||||
store.PutNode(n2)
|
||||
|
||||
all := store.GetNodesByMachineKeyAllUsers(mk)
|
||||
require.Len(t, all, 2)
|
||||
require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
|
||||
require.Equal(t, types.NodeID(2), all[types.UserID(2)].ID())
|
||||
})
|
||||
|
||||
t.Run("tagged node indexed under UserID(0)", func(t *testing.T) {
|
||||
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
|
||||
|
||||
store.Start()
|
||||
@@ -1374,6 +1374,10 @@ func TestGetNodeByMachineKeyAnyUserDeterministic(t *testing.T) {
|
||||
store.PutNode(owned)
|
||||
store.PutNode(tagged)
|
||||
|
||||
assertStablePick(t, store, 3)
|
||||
all := store.GetNodesByMachineKeyAllUsers(mk)
|
||||
require.Len(t, all, 2)
|
||||
require.Equal(t, types.NodeID(1), all[types.UserID(1)].ID())
|
||||
require.True(t, all[types.UserID(0)].IsTagged())
|
||||
require.Equal(t, types.NodeID(3), all[types.UserID(0)].ID())
|
||||
})
|
||||
}
|
||||
|
||||
+109
-53
@@ -118,6 +118,13 @@ var ErrRegistrationExpired = errors.New("registration expired")
|
||||
// binding.
|
||||
var ErrNodeKeyInUse = errors.New("node key already in use by another machine")
|
||||
|
||||
// ErrAmbiguousNodeOwnership is returned when a machine key maps to a set of
|
||||
// nodes from which the correct one to update or convert cannot be determined:
|
||||
// multiple user-owned candidates for a tagged conversion, or a tagged node and
|
||||
// a user-owned node coexisting (impossible per validateNodeOwnership). The
|
||||
// registration is rejected rather than mutating an arbitrarily-picked node.
|
||||
var ErrAmbiguousNodeOwnership = errors.New("machine key maps to ambiguous node ownership")
|
||||
|
||||
// sshCheckPair identifies a (source, destination) node pair for
|
||||
// SSH check auth tracking.
|
||||
type sshCheckPair struct {
|
||||
@@ -745,12 +752,11 @@ func (s *State) GetNodeByNodeKey(nodeKey key.NodePublic) (types.NodeView, bool)
|
||||
return s.nodeStore.GetNodeByNodeKey(nodeKey)
|
||||
}
|
||||
|
||||
// GetNodeByMachineKey retrieves a node by its machine key and user ID.
|
||||
// The bool indicates if the node exists or is available (like "err not found").
|
||||
// The NodeView might be invalid, so it must be checked with .Valid(), which must be used to ensure
|
||||
// it isn't an invalid node (this is more of a node error or node is broken).
|
||||
func (s *State) GetNodeByMachineKey(machineKey key.MachinePublic, userID types.UserID) (types.NodeView, bool) {
|
||||
return s.nodeStore.GetNodeByMachineKey(machineKey, userID)
|
||||
// GetNodesByMachineKeyAllUsers returns every node sharing the machine key,
|
||||
// keyed by owning UserID (tagged nodes under UserID(0)). See
|
||||
// [NodeStore.GetNodesByMachineKeyAllUsers].
|
||||
func (s *State) GetNodesByMachineKeyAllUsers(machineKey key.MachinePublic) map[types.UserID]types.NodeView {
|
||||
return s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
|
||||
}
|
||||
|
||||
// ResolveNode looks up a node by numeric ID, IPv4/IPv6 address, given
|
||||
@@ -2051,16 +2057,32 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
// resolve to a single node rather than racing the find-then-create section.
|
||||
defer s.lockRegistration(machineKey)()
|
||||
|
||||
existingNodeSameUser, _ := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(user.ID))
|
||||
existingNodeAnyUser, _ := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
|
||||
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
|
||||
|
||||
// Named conditions - describe WHAT we found, not HOW we check it
|
||||
nodeExistsForSameUser := existingNodeSameUser.Valid()
|
||||
nodeExistsForAnyUser := existingNodeAnyUser.Valid()
|
||||
existingNodeIsTagged := nodeExistsForAnyUser && existingNodeAnyUser.IsTagged()
|
||||
existingNodeOwnedByOtherUser := nodeExistsForAnyUser &&
|
||||
!existingNodeIsTagged &&
|
||||
existingNodeAnyUser.UserID().Get() != user.ID
|
||||
// Named conditions - describe WHAT we found, not HOW we check it.
|
||||
existingNodeSameUser, nodeExistsForSameUser := all[types.UserID(user.ID)]
|
||||
|
||||
taggedNode, hasTagged := all[0]
|
||||
existingNodeIsTagged := hasTagged && taggedNode.IsTagged()
|
||||
|
||||
var existingNodeOtherUser types.NodeView
|
||||
|
||||
existingNodeOwnedByOtherUser := false
|
||||
|
||||
for uid, n := range all {
|
||||
if uid != 0 && uid != types.UserID(user.ID) && !n.IsTagged() {
|
||||
existingNodeOtherUser = n
|
||||
existingNodeOwnedByOtherUser = true
|
||||
}
|
||||
}
|
||||
|
||||
// A tagged node and a user-owned node cannot legitimately share a machine
|
||||
// key (validateNodeOwnership enforces tags XOR user ownership). If both are
|
||||
// present the machine key is in a corrupt/ambiguous state; reject rather
|
||||
// than converting an arbitrary node and orphaning the other.
|
||||
if existingNodeIsTagged && (nodeExistsForSameUser || existingNodeOwnedByOtherUser) {
|
||||
return types.NodeView{}, change.Change{}, ErrAmbiguousNodeOwnership
|
||||
}
|
||||
|
||||
// Create logger with common fields for all auth operations
|
||||
logger := log.With().
|
||||
@@ -2090,7 +2112,7 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
}
|
||||
} else if existingNodeIsTagged {
|
||||
updateParams.ExistingNode = existingNodeAnyUser
|
||||
updateParams.ExistingNode = taggedNode
|
||||
updateParams.IsConvertFromTag = true
|
||||
|
||||
finalNode, err = s.applyAuthNodeUpdate(updateParams)
|
||||
@@ -2098,7 +2120,7 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
}
|
||||
} else if existingNodeOwnedByOtherUser {
|
||||
oldUser := existingNodeAnyUser.User()
|
||||
oldUser := existingNodeOtherUser.User()
|
||||
|
||||
oldUserName := ""
|
||||
if oldUser.Valid() {
|
||||
@@ -2106,14 +2128,14 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
}
|
||||
|
||||
logger.Info().
|
||||
Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
|
||||
Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
|
||||
Str(zf.ExistingNodeName, existingNodeOtherUser.Hostname()).
|
||||
Uint64(zf.ExistingNodeID, existingNodeOtherUser.ID().Uint64()).
|
||||
Str(zf.OldUser, oldUserName).
|
||||
Msg("Creating new node for different user (same machine key exists for another user)")
|
||||
|
||||
finalNode, err = s.createNewNodeFromAuth(
|
||||
logger, user, regData, hostname, hostinfo,
|
||||
expiry, registrationMethod, existingNodeAnyUser,
|
||||
expiry, registrationMethod, existingNodeOtherUser,
|
||||
)
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
@@ -2192,11 +2214,12 @@ func (s *State) createNewNodeFromAuth(
|
||||
func (s *State) findExistingNodeForPAK(
|
||||
machineKey key.MachinePublic,
|
||||
pak *types.PreAuthKey,
|
||||
) (types.NodeView, bool) {
|
||||
) (types.NodeView, bool, error) {
|
||||
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
|
||||
|
||||
if pak.User != nil {
|
||||
node, exists := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(pak.User.ID))
|
||||
if exists {
|
||||
return node, true
|
||||
if node, ok := all[types.UserID(pak.User.ID)]; ok {
|
||||
return node, true, nil
|
||||
}
|
||||
|
||||
// The node may have been converted to a tagged node since it first
|
||||
@@ -2205,21 +2228,46 @@ func (s *State) findExistingNodeForPAK(
|
||||
// it for re-registration instead of re-validating the spent key or
|
||||
// creating a duplicate node. Re-registration preserves the node's tagged
|
||||
// ownership. See https://github.com/juanfont/headscale/issues/3312.
|
||||
if node, exists := s.nodeStore.GetNodeByMachineKey(machineKey, 0); exists && node.IsTagged() {
|
||||
return node, true
|
||||
if node, ok := all[0]; ok && node.IsTagged() {
|
||||
return node, true, nil
|
||||
}
|
||||
|
||||
return types.NodeView{}, false, nil
|
||||
}
|
||||
|
||||
// A tagged key re-registers the same machine regardless of how it is
|
||||
// currently owned. An existing tagged node is a plain re-registration. A
|
||||
// single user-owned node is converted to tagged in place (handled by the
|
||||
// caller). More than one user-owned node is ambiguous - we cannot know
|
||||
// which to convert - so reject rather than convert an arbitrary one and
|
||||
// orphan the rest.
|
||||
if pak.IsTagged() {
|
||||
if node, ok := all[0]; ok && node.IsTagged() {
|
||||
return node, true, nil
|
||||
}
|
||||
|
||||
var userOwned types.NodeView
|
||||
|
||||
count := 0
|
||||
|
||||
for uid, node := range all {
|
||||
if uid != 0 && !node.IsTagged() {
|
||||
userOwned = node
|
||||
count++
|
||||
}
|
||||
}
|
||||
|
||||
switch count {
|
||||
case 0:
|
||||
return types.NodeView{}, false, nil
|
||||
case 1:
|
||||
return userOwned, true, nil
|
||||
default:
|
||||
return types.NodeView{}, false, ErrAmbiguousNodeOwnership
|
||||
}
|
||||
}
|
||||
|
||||
// A tagged key re-registers the same machine regardless of how that machine
|
||||
// is currently owned: a tagged node (indexed under UserID(0)) is a plain
|
||||
// re-registration, while a user-owned node is converted to tagged in place
|
||||
// (handled by the caller). Match on the machine key alone so neither case
|
||||
// creates a duplicate node.
|
||||
if pak.IsTagged() {
|
||||
return s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
|
||||
}
|
||||
|
||||
return types.NodeView{}, false
|
||||
return types.NodeView{}, false, nil
|
||||
}
|
||||
|
||||
//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
|
||||
@@ -2245,7 +2293,10 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
return types.TaggedDevices.Name
|
||||
}
|
||||
|
||||
existingNodeSameUser, existsSameUser := s.findExistingNodeForPAK(machineKey, pak)
|
||||
existingNodeSameUser, existsSameUser, err := s.findExistingNodeForPAK(machineKey, pak)
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
}
|
||||
|
||||
// For existing nodes, skip validation if:
|
||||
// 1. MachineKey matches (cryptographic proof of machine identity)
|
||||
@@ -2455,24 +2506,29 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
|
||||
finalNode = updatedNodeView
|
||||
} else {
|
||||
// Node does not exist for this user with this machine key
|
||||
// Check if node exists with this machine key for a different user
|
||||
existingNodeAnyUser, existsAnyUser := s.nodeStore.GetNodeByMachineKeyAnyUser(machineKey)
|
||||
// Node does not exist for this user with this machine key.
|
||||
// For a user-owned key, check whether the machine key is already held
|
||||
// by a node belonging to a different user (tags-only keys skip this;
|
||||
// tagged nodes have no owning user). Any such node yields the same
|
||||
// outcome - create a new node for the new user, do not transfer - so a
|
||||
// single representative is enough.
|
||||
var differentUserNode types.NodeView
|
||||
|
||||
// For user-owned keys, check if node exists for a different user.
|
||||
// Tags-only keys (pak.User == nil) skip this check.
|
||||
// Tagged nodes are also skipped since they have no owning user.
|
||||
existingIsUserOwned := existsAnyUser &&
|
||||
existingNodeAnyUser.Valid() &&
|
||||
!existingNodeAnyUser.IsTagged()
|
||||
belongsToDifferentUser := pak.User != nil &&
|
||||
existingIsUserOwned &&
|
||||
existingNodeAnyUser.UserID().Get() != pak.User.ID
|
||||
belongsToDifferentUser := false
|
||||
|
||||
if pak.User != nil {
|
||||
for uid, node := range s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey) {
|
||||
if uid != 0 && !node.IsTagged() && uid != types.UserID(pak.User.ID) {
|
||||
differentUserNode = node
|
||||
belongsToDifferentUser = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if belongsToDifferentUser {
|
||||
// Node exists but belongs to a different user.
|
||||
// Create a new node for the new user (do not transfer).
|
||||
oldUser := existingNodeAnyUser.User()
|
||||
oldUser := differentUserNode.User()
|
||||
|
||||
oldUserName := ""
|
||||
if oldUser.Valid() {
|
||||
@@ -2481,8 +2537,8 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
|
||||
log.Info().
|
||||
Caller().
|
||||
Str(zf.ExistingNodeName, existingNodeAnyUser.Hostname()).
|
||||
Uint64(zf.ExistingNodeID, existingNodeAnyUser.ID().Uint64()).
|
||||
Str(zf.ExistingNodeName, differentUserNode.Hostname()).
|
||||
Uint64(zf.ExistingNodeID, differentUserNode.ID().Uint64()).
|
||||
Str(zf.MachineKey, machineKey.ShortString()).
|
||||
Str(zf.OldUser, oldUserName).
|
||||
Str(zf.NewUser, pakUsername()).
|
||||
@@ -2522,7 +2578,7 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
Expiry: reqExpiry,
|
||||
RegisterMethod: util.RegisterMethodAuthKey,
|
||||
PreAuthKey: pak,
|
||||
ExistingNodeForNetinfo: cmp.Or(existingNodeAnyUser, types.NodeView{}),
|
||||
ExistingNodeForNetinfo: cmp.Or(differentUserNode, types.NodeView{}),
|
||||
})
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, fmt.Errorf("creating new node: %w", err)
|
||||
|
||||
Reference in New Issue
Block a user