mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-19 14:30:44 +09:00
Compare commits
14 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| b0c221f3a1 | |||
| 68a6d3cf17 | |||
| 1689478485 | |||
| a1d3e98255 | |||
| b83bf3f993 | |||
| 96d2e6ed60 | |||
| 9b8949727d | |||
| bff216a184 | |||
| fd08b8fa8c | |||
| a73d38bb3f | |||
| e759d9fc90 | |||
| 0961e79e16 | |||
| a5ef3aff15 | |||
| 4da06925d0 |
+5
-1
@@ -1,6 +1,10 @@
|
||||
# CHANGELOG
|
||||
|
||||
## 0.29.0 (202x-xx-xx)
|
||||
## 0.30.0 (202x-xx-xx)
|
||||
|
||||
**Minimum supported Tailscale client version: v1.xx.0**
|
||||
|
||||
## 0.29.0 (2026-06-17)
|
||||
|
||||
**Minimum supported Tailscale client version: v1.80.0**
|
||||
|
||||
|
||||
+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")
|
||||
|
||||
+10
-27
@@ -143,27 +143,6 @@ func GetNodeByID(tx *gorm.DB, id types.NodeID) (*types.Node, error) {
|
||||
return &mach, nil
|
||||
}
|
||||
|
||||
func (hsdb *HSDatabase) GetNodeByMachineKey(machineKey key.MachinePublic) (*types.Node, error) {
|
||||
return GetNodeByMachineKey(hsdb.DB, machineKey)
|
||||
}
|
||||
|
||||
// GetNodeByMachineKey finds a [types.Node] by its [key.MachinePublic] and returns the [types.Node] struct.
|
||||
func GetNodeByMachineKey(
|
||||
tx *gorm.DB,
|
||||
machineKey key.MachinePublic,
|
||||
) (*types.Node, error) {
|
||||
mach := types.Node{}
|
||||
if result := tx.
|
||||
Preload("AuthKey").
|
||||
Preload("AuthKey.User").
|
||||
Preload("User").
|
||||
First(&mach, "machine_key = ?", machineKey.String()); result.Error != nil {
|
||||
return nil, result.Error
|
||||
}
|
||||
|
||||
return &mach, nil
|
||||
}
|
||||
|
||||
func (hsdb *HSDatabase) GetNodeByNodeKey(nodeKey key.NodePublic) (*types.Node, error) {
|
||||
return GetNodeByNodeKey(hsdb.DB, nodeKey)
|
||||
}
|
||||
@@ -297,12 +276,16 @@ func RegisterNodeForTest(tx *gorm.DB, node types.Node, ipv4 *netip.Addr, ipv6 *n
|
||||
|
||||
logEvent.Msg("registering test node")
|
||||
|
||||
// If the a new node is registered with the same machine key, to the same user,
|
||||
// update the existing node.
|
||||
// If the same node is registered again, but to a new user, then that is considered
|
||||
// a new node.
|
||||
oldNode, _ := GetNodeByMachineKey(tx, node.MachineKey)
|
||||
if oldNode != nil && oldNode.UserID == node.UserID {
|
||||
// Reuse the existing node's identity only when the same machine
|
||||
// re-registers for the same user; a different user is a new node. Match on
|
||||
// (machine_key, user_id) precisely - a machine key can map to several nodes
|
||||
// (one per user), so a machine-key-only lookup would be ambiguous.
|
||||
var oldNode types.Node
|
||||
|
||||
err := tx.
|
||||
Where("machine_key = ? AND user_id = ?", node.MachineKey.String(), node.UserID).
|
||||
First(&oldNode).Error
|
||||
if err == nil {
|
||||
node.ID = oldNode.ID
|
||||
node.GivenName = oldNode.GivenName
|
||||
node.ApprovedRoutes = oldNode.ApprovedRoutes
|
||||
|
||||
@@ -15,7 +15,10 @@ import (
|
||||
)
|
||||
|
||||
var (
|
||||
ErrPreAuthKeyNotFound = errors.New("auth-key not found")
|
||||
// ErrPreAuthKeyNotFound wraps gorm.ErrRecordNotFound so an unknown or
|
||||
// deleted key is treated as a missing record by callers, which the
|
||||
// registration handler maps to a 401 rather than a raw server error.
|
||||
ErrPreAuthKeyNotFound = fmt.Errorf("auth-key not found: %w", gorm.ErrRecordNotFound)
|
||||
ErrPreAuthKeyExpired = errors.New("auth-key expired")
|
||||
ErrSingleUseAuthKeyHasBeenUsed = errors.New("auth-key has already been used")
|
||||
ErrUserMismatch = errors.New("user mismatch")
|
||||
|
||||
@@ -487,3 +487,16 @@ func TestUsePreAuthKeyAtomicCAS(t *testing.T) {
|
||||
"second UsePreAuthKey error must be a PAKError, got: %v", err)
|
||||
assert.Equal(t, "authkey already used", pakErr.Error())
|
||||
}
|
||||
|
||||
// TestGetPreAuthKeyUnknownMapsToRecordNotFound ensures an unknown (or deleted)
|
||||
// pre-auth key resolves to a record-not-found error, which the registration
|
||||
// handler maps to a 401 rather than a raw server error.
|
||||
func TestGetPreAuthKeyUnknownMapsToRecordNotFound(t *testing.T) {
|
||||
db, err := newSQLiteTestDB()
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = db.GetPreAuthKey("nonexistent-key")
|
||||
require.Error(t, err)
|
||||
require.ErrorIs(t, err, gorm.ErrRecordNotFound,
|
||||
"unknown pre-auth key must map to record-not-found (handled as 401)")
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/stretchr/testify/require"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
)
|
||||
|
||||
// TestTaggedReauthKeepsNilExpiry ensures that when an existing tagged node
|
||||
@@ -92,3 +93,388 @@ func TestTaggedReauthKeepsNilExpiry(t *testing.T) {
|
||||
require.Nil(t, finalNode.AsStruct().Expiry,
|
||||
"tagged node must keep nil key expiry (tagged nodes never expire)")
|
||||
}
|
||||
|
||||
// TestTaggedReauthWithReusedUserPAK reproduces issue #3312: a containerized
|
||||
// node registered with a user-owned one-shot pre-auth key, then converted to a
|
||||
// tagged node (UserID cleared to NULL), is logged out when the container
|
||||
// restarts and re-registers with the SAME, now-used TS_AUTHKEY.
|
||||
//
|
||||
// Root cause: findExistingNodeForPAK (state.go) looks the node up by the PAK's
|
||||
// owning user (alice). After tagging, the node is indexed under UserID(0), so
|
||||
// the same-user machine-key lookup misses, the re-registration fast-path is
|
||||
// skipped, and the already-used one-shot PAK is re-validated and rejected with
|
||||
// "authkey already used" — logging the node out.
|
||||
//
|
||||
// https://github.com/juanfont/headscale/issues/3312
|
||||
func TestTaggedReauthWithReusedUserPAK(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("authkey-user")
|
||||
|
||||
policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name)
|
||||
_, err = s.SetPolicy([]byte(policy))
|
||||
require.NoError(t, err)
|
||||
|
||||
// One-shot, user-owned PAK: `headscale preauthkeys create -u 1`.
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "authkey-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
// First registration: node joins as alice, the one-shot PAK is consumed.
|
||||
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, first.Valid())
|
||||
nodeID := first.ID()
|
||||
|
||||
// `headscale nodes tag -t tag:foo`: convert to a tagged node. This clears
|
||||
// both UserID and User (state.SetNodeTags), diverging the node's ownership
|
||||
// from the still-user-owned PAK.
|
||||
tagged, _, err := s.SetNodeTags(nodeID, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
require.True(t, tagged.IsTagged(), "precondition: node must be tagged")
|
||||
|
||||
// Container restart: the same node re-registers with the SAME, now-used
|
||||
// one-shot TS_AUTHKEY. The machine key proves identity, so this must
|
||||
// succeed. It currently fails with "authkey already used".
|
||||
second, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err,
|
||||
"re-registration with a reused user PAK on a tagged node must not be rejected")
|
||||
require.True(t, second.Valid())
|
||||
require.True(t, second.IsTagged(), "node must remain tagged after re-registration")
|
||||
require.Equal(t, nodeID, second.ID(),
|
||||
"must update the existing node, not create a new one")
|
||||
}
|
||||
|
||||
// reregisterExpiredUserNodeWithSpentKey registers a user-owned node with a
|
||||
// one-shot key, forces it into the expired state, and re-registers with the
|
||||
// same spent key. sameNodeKey distinguishes the two re-auth shapes:
|
||||
// - false: the node rotates its node key (normal tailscale client on re-auth)
|
||||
// - true: the node reuses its node key
|
||||
//
|
||||
// In both cases an expired node is genuinely re-authenticating and must present
|
||||
// a valid key; a spent one-shot key must be rejected.
|
||||
func reregisterExpiredUserNodeWithSpentKey(t *testing.T, sameNodeKey bool) (types.NodeView, error) {
|
||||
t.Helper()
|
||||
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("expired-user")
|
||||
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "expired-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, first.Valid())
|
||||
require.False(t, first.IsTagged(), "precondition: node must be user-owned")
|
||||
|
||||
// Force the node into the expired state.
|
||||
past := time.Now().Add(-1 * time.Hour)
|
||||
_, ok := s.nodeStore.UpdateNode(first.ID(), func(n *types.Node) {
|
||||
n.Expiry = &past
|
||||
})
|
||||
require.True(t, ok)
|
||||
|
||||
reReg := regReq
|
||||
if !sameNodeKey {
|
||||
reReg.NodeKey = key.NewNode().Public()
|
||||
}
|
||||
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
|
||||
|
||||
return node, err
|
||||
}
|
||||
|
||||
// TestExpiredUserNodeReusedOneShotKey_RotatedNodeKey: a node rotating its node
|
||||
// key on re-auth is already a key rotation, so the key is re-validated.
|
||||
func TestExpiredUserNodeReusedOneShotKey_RotatedNodeKey(t *testing.T) {
|
||||
_, err := reregisterExpiredUserNodeWithSpentKey(t, false)
|
||||
require.Error(t, err,
|
||||
"expired node re-authenticating with a rotated node key must present a valid key")
|
||||
require.Contains(t, err.Error(), "authkey already used")
|
||||
}
|
||||
|
||||
// TestExpiredUserNodeReusedOneShotKey_SameNodeKey: the security boundary must
|
||||
// not depend on the client rotating its node key. An expired node re-using its
|
||||
// node key must still re-validate the key, otherwise a spent one-shot key
|
||||
// silently re-authorises it.
|
||||
func TestExpiredUserNodeReusedOneShotKey_SameNodeKey(t *testing.T) {
|
||||
_, err := reregisterExpiredUserNodeWithSpentKey(t, true)
|
||||
require.Error(t, err,
|
||||
"expired node re-registering with the same node key must re-validate its key")
|
||||
require.Contains(t, err.Error(), "authkey already used")
|
||||
}
|
||||
|
||||
// TestReusableUserPAKReauthOnTaggedNodeNoDuplicate guards against a reusable
|
||||
// user pre-auth key creating a second node when it is re-presented for a node
|
||||
// that has since been converted to tagged. The node must be updated in place.
|
||||
func TestReusableUserPAKReauthOnTaggedNodeNoDuplicate(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("reusable-user")
|
||||
|
||||
policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name)
|
||||
_, err = s.SetPolicy([]byte(policy))
|
||||
require.NoError(t, err)
|
||||
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
_, _, err = s.SetNodeTags(first.ID(), []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
|
||||
second, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, second.IsTagged())
|
||||
require.Equal(t, first.ID(), second.ID(), "must update in place, not duplicate")
|
||||
require.Equal(t, 1, s.ListNodes().Len(), "machine must map to exactly one node")
|
||||
}
|
||||
|
||||
// TestTaggedPAKReauthConvertsUserOwnedNode ensures presenting a tagged pre-auth
|
||||
// key for a machine that already has a user-owned node converts that node in
|
||||
// place (same machine, new ownership) rather than creating a duplicate.
|
||||
func TestTaggedPAKReauthConvertsUserOwnedNode(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("owner")
|
||||
|
||||
userPak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
nodeKey := key.NewNode()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: userPak.Key},
|
||||
NodeKey: nodeKey.Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "owned-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
owned, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.False(t, owned.IsTagged(), "precondition: node is user-owned")
|
||||
|
||||
// A tags-only key re-registers the same machine (same node key).
|
||||
taggedPak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
|
||||
convReq := regReq
|
||||
convReq.Auth = &tailcfg.RegisterResponseAuth{AuthKey: taggedPak.Key}
|
||||
|
||||
converted, _, err := s.HandleNodeFromPreAuthKey(convReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, owned.ID(), converted.ID(), "must convert in place, not duplicate")
|
||||
require.True(t, converted.IsTagged(), "node must become tagged")
|
||||
require.Equal(t, []string{"tag:foo"}, converted.Tags().AsSlice())
|
||||
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`).
|
||||
func TestTaggedNodeCanHaveKeyExpiry(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:foo":["tagger@"]}}`))
|
||||
require.NoError(t, err)
|
||||
|
||||
pak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "tagged-node"},
|
||||
}
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(regReq, key.NewMachine().Public())
|
||||
require.NoError(t, err)
|
||||
require.True(t, node.IsTagged())
|
||||
require.Nil(t, node.AsStruct().Expiry, "key expiry is disabled by default for tagged nodes")
|
||||
|
||||
expiry := time.Now().Add(24 * time.Hour)
|
||||
after, _, err := s.SetNodeExpiry(node.ID(), &expiry)
|
||||
require.NoError(t, err)
|
||||
require.True(t, after.IsTagged(), "node stays tagged")
|
||||
require.NotNil(t, after.AsStruct().Expiry, "expiry can be set on a tagged node")
|
||||
require.Equal(t, expiry.Unix(), after.AsStruct().Expiry.Unix())
|
||||
}
|
||||
|
||||
// TestTaggingPreservesNodeExpiry matches Tailscale: changing a node's tags does
|
||||
// not alter its key expiry (expiry only changes on re-authentication).
|
||||
func TestTaggingPreservesNodeExpiry(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("owner")
|
||||
|
||||
_, err = s.SetPolicy(fmt.Appendf(nil, `{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name))
|
||||
require.NoError(t, err)
|
||||
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
expiry := time.Now().Add(24 * time.Hour)
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "owned-node"},
|
||||
Expiry: expiry,
|
||||
}
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(regReq, key.NewMachine().Public())
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, node.AsStruct().Expiry, "precondition: user node has an expiry")
|
||||
|
||||
tagged, _, err := s.SetNodeTags(node.ID(), []string{"tag:foo"})
|
||||
require.NoError(t, err)
|
||||
require.True(t, tagged.IsTagged())
|
||||
require.NotNil(t, tagged.AsStruct().Expiry, "tag change must not clear expiry")
|
||||
require.Equal(t, expiry.Unix(), tagged.AsStruct().Expiry.Unix())
|
||||
}
|
||||
|
||||
@@ -111,3 +111,88 @@ func TestEndpointStorageInNodeStore(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestEndpointBroadcastWorthy verifies the gate that decides whether an
|
||||
// endpoint-only delta is worth fanning out to peers as an incremental
|
||||
// PeersChangedPatch. A delta that only adds STUN-derived endpoints (or only
|
||||
// removes endpoints) is suppressed: it is churny and unlikely to be useful,
|
||||
// and disco's callMeMaybe re-derives STUN paths anyway. Only deltas that
|
||||
// introduce a genuinely useful (non-STUN) endpoint are broadcast-worthy.
|
||||
func TestEndpointBroadcastWorthy(t *testing.T) {
|
||||
local := netip.MustParseAddrPort("192.168.1.5:41641")
|
||||
local2 := netip.MustParseAddrPort("192.168.1.6:41641")
|
||||
stun := netip.MustParseAddrPort("203.0.113.7:41641")
|
||||
stun2 := netip.MustParseAddrPort("203.0.113.8:41641")
|
||||
portmap := netip.MustParseAddrPort("198.51.100.9:41641")
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
stored []netip.AddrPort
|
||||
newEPs []netip.AddrPort
|
||||
newType []tailcfg.EndpointType
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "adds only a STUN endpoint - suppress",
|
||||
stored: []netip.AddrPort{local},
|
||||
newEPs: []netip.AddrPort{local, stun},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "adds only STUN4LocalPort - suppress",
|
||||
stored: []netip.AddrPort{local},
|
||||
newEPs: []netip.AddrPort{local, stun},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN4LocalPort},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "adds a useful local endpoint - broadcast",
|
||||
stored: []netip.AddrPort{stun},
|
||||
newEPs: []netip.AddrPort{stun, local},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "adds a useful portmapped endpoint - broadcast",
|
||||
stored: []netip.AddrPort{local},
|
||||
newEPs: []netip.AddrPort{local, portmap},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointPortmapped},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "pure shrink, no additions - suppress",
|
||||
stored: []netip.AddrPort{local, local2},
|
||||
newEPs: []netip.AddrPort{local},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "nil types (older client) adding endpoint - broadcast",
|
||||
stored: []netip.AddrPort{local},
|
||||
newEPs: []netip.AddrPort{local, local2},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "only STUN endpoints churn (replace one STUN with another) - suppress",
|
||||
stored: []netip.AddrPort{local, stun},
|
||||
newEPs: []netip.AddrPort{local, stun2},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "mixed add: one STUN and one useful - broadcast",
|
||||
stored: []netip.AddrPort{local},
|
||||
newEPs: []netip.AddrPort{local, stun, local2},
|
||||
newType: []tailcfg.EndpointType{tailcfg.EndpointLocal, tailcfg.EndpointSTUN, tailcfg.EndpointLocal},
|
||||
want: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := endpointBroadcastWorthy(tt.stored, tt.newEPs, tt.newType)
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -787,44 +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"))
|
||||
// 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.
|
||||
//
|
||||
// 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").Inc()
|
||||
nodeStoreOperations.WithLabelValues("get_nodes_by_machine_key_all_users").Inc()
|
||||
|
||||
snapshot := s.data.Load()
|
||||
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
|
||||
if node, exists := userMap[userID]; exists {
|
||||
return node, true
|
||||
}
|
||||
}
|
||||
userMap := s.data.Load().nodesByMachineKey[machineKey]
|
||||
|
||||
return types.NodeView{}, false
|
||||
}
|
||||
out := make(map[types.UserID]types.NodeView, len(userMap))
|
||||
maps.Copy(out, userMap)
|
||||
|
||||
// GetNodeByMachineKeyAnyUser returns the first 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.
|
||||
// If multiple nodes exist with the same machine key (different users), the
|
||||
// first one found is returned (order is not guaranteed).
|
||||
func (s *NodeStore) GetNodeByMachineKeyAnyUser(machineKey key.MachinePublic) (types.NodeView, bool) {
|
||||
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("get_by_machine_key_any_user"))
|
||||
defer timer.ObserveDuration()
|
||||
|
||||
nodeStoreOperations.WithLabelValues("get_by_machine_key_any_user").Inc()
|
||||
|
||||
snapshot := s.data.Load()
|
||||
if userMap, exists := snapshot.nodesByMachineKey[machineKey]; exists {
|
||||
// Return the first node found (order not guaranteed due to map iteration)
|
||||
for _, node := range userMap {
|
||||
return node, true
|
||||
}
|
||||
}
|
||||
|
||||
return types.NodeView{}, false
|
||||
return out
|
||||
}
|
||||
|
||||
// DebugString returns debug information about the [NodeStore].
|
||||
|
||||
@@ -1321,3 +1321,63 @@ func TestRebuildPeerMapsWithChangedPeersFunc(t *testing.T) {
|
||||
assert.Equal(t, 1, peers1.Len(), "ListPeers for node1 should return 1")
|
||||
assert.Equal(t, 1, peers2.Len(), "ListPeers for node2 should return 1")
|
||||
}
|
||||
|
||||
// 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()
|
||||
|
||||
t.Run("empty when absent", func(t *testing.T) {
|
||||
store := NewNodeStore(nil, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
|
||||
|
||||
store.Start()
|
||||
defer store.Stop()
|
||||
|
||||
require.Empty(t, store.GetNodesByMachineKeyAllUsers(mk))
|
||||
})
|
||||
|
||||
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()
|
||||
defer store.Stop()
|
||||
|
||||
owned := createTestNode(1, 1, "user1", "node1")
|
||||
owned.MachineKey = mk
|
||||
tagged := createTestNode(3, 3, "user3", "node3")
|
||||
tagged.MachineKey = mk
|
||||
tagged.UserID = nil
|
||||
tagged.User = nil
|
||||
tagged.Tags = []string{"tag:foo"}
|
||||
|
||||
store.PutNode(owned)
|
||||
store.PutNode(tagged)
|
||||
|
||||
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())
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1,14 +1,18 @@
|
||||
package state
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"net/netip"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"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"
|
||||
"gorm.io/gorm"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
)
|
||||
@@ -325,3 +329,256 @@ func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
|
||||
require.Error(t, err,
|
||||
"re-auth claiming a NodeKey bound to another machine must be rejected")
|
||||
}
|
||||
|
||||
// TestReauthPreservesEndpointsWhenClientOmitsThem proves the re-auth/update
|
||||
// path keeps a node's live WireGuard endpoints when the originating
|
||||
// RegisterRequest carried none. Web/OIDC relogins report endpoints via
|
||||
// MapRequest, not register, so RegData.Endpoints is empty; wiping the stored
|
||||
// endpoints would advertise the re-keyed node to peers endpoint-less, which
|
||||
// drives head/unstable tailscale clients into one-way disco-deafness.
|
||||
func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
database, err := db.NewHeadscaleDatabase(cfg)
|
||||
require.NoError(t, err)
|
||||
|
||||
user := database.CreateUserForTest("user")
|
||||
require.NoError(t, database.Close())
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
machine := key.NewMachine()
|
||||
endpoints := []netip.AddrPort{
|
||||
netip.MustParseAddrPort("192.168.1.5:41641"),
|
||||
netip.MustParseAddrPort("10.0.0.5:41641"),
|
||||
}
|
||||
|
||||
// Node is registered and has reported live endpoints (as after its first
|
||||
// MapRequest).
|
||||
node, err := s.createAndSaveNewNode(newNodeParams{
|
||||
User: *user,
|
||||
MachineKey: machine.Public(),
|
||||
NodeKey: key.NewNode().Public(),
|
||||
DiscoKey: key.NewDisco().Public(),
|
||||
Hostname: "node",
|
||||
Endpoints: endpoints,
|
||||
RegisterMethod: util.RegisterMethodCLI,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, endpoints, node.Endpoints().AsSlice(),
|
||||
"precondition: node has live endpoints")
|
||||
|
||||
// Node re-authenticates, rotating its NodeKey. The RegisterRequest carries
|
||||
// no endpoints.
|
||||
updated, err := s.applyAuthNodeUpdate(authNodeUpdateParams{
|
||||
ExistingNode: node,
|
||||
RegData: &types.RegistrationData{
|
||||
MachineKey: machine.Public(),
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostname: "node",
|
||||
Hostinfo: &tailcfg.Hostinfo{},
|
||||
Endpoints: nil,
|
||||
},
|
||||
ValidHostinfo: &tailcfg.Hostinfo{},
|
||||
Hostname: "node",
|
||||
User: user,
|
||||
RegisterMethod: util.RegisterMethodCLI,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, endpoints, updated.Endpoints().AsSlice(),
|
||||
"re-auth without reported endpoints must preserve the node's live endpoints")
|
||||
}
|
||||
|
||||
// TestReauthChange covers the decision both re-auth paths share: a same-user
|
||||
// relogin must be an incremental peer patch (so the tailscale client takes its
|
||||
// fast patch path), never a whole-node add (which strands a re-keyed,
|
||||
// momentarily-endpoint-less peer disco-deaf); a policy change forces a full
|
||||
// recompute; a new node is a whole-node add.
|
||||
func TestReauthChange(t *testing.T) {
|
||||
n := types.Node{
|
||||
ID: 7,
|
||||
NodeKey: key.NewNode().Public(),
|
||||
DiscoKey: key.NewDisco().Public(),
|
||||
}
|
||||
node := n.View()
|
||||
|
||||
relogin := reauthChange(node, true, false)
|
||||
assert.Len(t, relogin.PeerPatches, 1, "relogin must be a peer patch")
|
||||
assert.Empty(t, relogin.PeersChanged, "relogin must not be a whole-node add")
|
||||
|
||||
added := reauthChange(node, false, false)
|
||||
assert.Empty(t, added.PeerPatches)
|
||||
assert.Len(t, added.PeersChanged, 1, "a new node must be a whole-node add")
|
||||
|
||||
pol := reauthChange(node, true, true)
|
||||
assert.Empty(t, pol.PeerPatches, "a policy change must not be a peer patch")
|
||||
assert.Empty(t, pol.PeersChanged)
|
||||
assert.False(t, pol.IsEmpty(), "a policy change must be non-empty")
|
||||
}
|
||||
|
||||
// TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine is the pre-auth-key
|
||||
// analogue of TestReauthRejectsNodeKeyClaimedByAnotherMachine: re-registering
|
||||
// via a pre-auth key must enforce the same 1:1 NodeKey<->MachineKey binding the
|
||||
// auth path and poll-time validation enforce, so a node cannot rotate its key
|
||||
// to a victim's and poison the NodeStore NodeKey index.
|
||||
func TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
attacker := s.CreateUserForTest("attacker")
|
||||
victim := s.CreateUserForTest("victim")
|
||||
|
||||
victimMachine := key.NewMachine()
|
||||
victimNodeKey := key.NewNode()
|
||||
_, err = s.createAndSaveNewNode(newNodeParams{
|
||||
User: *victim,
|
||||
MachineKey: victimMachine.Public(),
|
||||
NodeKey: victimNodeKey.Public(),
|
||||
DiscoKey: key.NewDisco().Public(),
|
||||
Hostname: "victim",
|
||||
RegisterMethod: util.RegisterMethodCLI,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Attacker registers its own node with a reusable pre-auth key.
|
||||
pak, err := s.CreatePreAuthKey(attacker.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
attackerMachine := key.NewMachine()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "attacker"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
_, _, err = s.HandleNodeFromPreAuthKey(regReq, attackerMachine.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
// Attacker re-registers its own node but supplies the victim's NodeKey.
|
||||
attack := regReq
|
||||
attack.NodeKey = victimNodeKey.Public()
|
||||
_, _, err = s.HandleNodeFromPreAuthKey(attack, attackerMachine.Public())
|
||||
require.ErrorIs(t, err, ErrNodeKeyInUse,
|
||||
"pre-auth-key re-registration claiming another machine's NodeKey must be rejected")
|
||||
|
||||
// The victim still owns its NodeKey.
|
||||
owner, ok := s.GetNodeByNodeKey(victimNodeKey.Public())
|
||||
require.True(t, ok)
|
||||
require.Equal(t, victimMachine.Public(), owner.MachineKey(),
|
||||
"victim's NodeKey index entry must be untouched")
|
||||
}
|
||||
|
||||
var errInjectedNodeUpdate = errors.New("injected node update failure")
|
||||
|
||||
// TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure ensures a failed database
|
||||
// write during pre-auth-key re-registration does not leave the NodeStore
|
||||
// holding a node key that was never persisted: a restart would reload the old
|
||||
// row and the client's current key would no longer resolve, locking it out.
|
||||
func TestPreAuthKeyReauthRevertsNodeStoreOnDBFailure(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("reauth-user")
|
||||
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "reauth-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
require.NoError(t, err)
|
||||
|
||||
origNodeKey := node.NodeKey()
|
||||
|
||||
// Fail the node row update so the re-registration's database write errors
|
||||
// after the NodeStore has already been mutated.
|
||||
require.NoError(t, s.db.DB.Callback().Update().Before("gorm:update").
|
||||
Register("fail_node_update", func(tx *gorm.DB) {
|
||||
if tx.Statement.Table == "nodes" {
|
||||
_ = tx.AddError(errInjectedNodeUpdate)
|
||||
}
|
||||
}))
|
||||
|
||||
reReg := regReq
|
||||
reReg.NodeKey = key.NewNode().Public() // rotate -> NodeStore mutation, then DB write fails
|
||||
_, _, err = s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
|
||||
require.NoError(t, s.db.DB.Callback().Update().Remove("fail_node_update"))
|
||||
require.Error(t, err, "re-registration must fail when the database write fails")
|
||||
|
||||
got, ok := s.nodeStore.GetNode(node.ID())
|
||||
require.True(t, ok)
|
||||
require.Equal(t, origNodeKey, got.NodeKey(),
|
||||
"NodeStore must revert to the persisted node key when the write fails")
|
||||
}
|
||||
|
||||
// TestConcurrentPreAuthKeyRegistrationSameMachineKey ensures concurrent
|
||||
// registrations of the same machine key resolve to a single node. Without
|
||||
// serialising the find-then-create section, each request sees "no existing
|
||||
// node" and creates its own, leaving duplicate nodes and IP allocations for
|
||||
// one machine.
|
||||
func TestConcurrentPreAuthKeyRegistrationSameMachineKey(t *testing.T) {
|
||||
dbPath := t.TempDir() + "/headscale.db"
|
||||
cfg := persistTestConfig(dbPath)
|
||||
|
||||
s, err := NewState(cfg)
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = s.Close() })
|
||||
|
||||
user := s.CreateUserForTest("concurrent-user")
|
||||
|
||||
pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
|
||||
require.NoError(t, err)
|
||||
|
||||
machineKey := key.NewMachine()
|
||||
|
||||
const n = 12
|
||||
|
||||
var wg sync.WaitGroup
|
||||
|
||||
start := make(chan struct{})
|
||||
errs := make(chan error, n)
|
||||
|
||||
for range n {
|
||||
wg.Go(func() {
|
||||
regReq := tailcfg.RegisterRequest{
|
||||
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
|
||||
NodeKey: key.NewNode().Public(),
|
||||
Hostinfo: &tailcfg.Hostinfo{Hostname: "concurrent-node"},
|
||||
Expiry: time.Now().Add(24 * time.Hour),
|
||||
}
|
||||
|
||||
<-start
|
||||
|
||||
_, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
|
||||
errs <- err
|
||||
})
|
||||
}
|
||||
|
||||
close(start)
|
||||
wg.Wait()
|
||||
close(errs)
|
||||
|
||||
for err := range errs {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
require.Equal(t, 1, s.ListNodes().Len(),
|
||||
"concurrent registrations of one machine key must yield a single node")
|
||||
}
|
||||
|
||||
+295
-72
@@ -32,6 +32,7 @@ import (
|
||||
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/juanfont/headscale/hscontrol/util/zlog/zf"
|
||||
"github.com/puzpuzpuz/xsync/v4"
|
||||
"github.com/rs/zerolog"
|
||||
"github.com/rs/zerolog/log"
|
||||
"gorm.io/gorm"
|
||||
@@ -117,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 {
|
||||
@@ -179,6 +187,22 @@ type State struct {
|
||||
// persistNodeToDB so the database row always converges on [NodeStore]
|
||||
// rather than being clobbered by a stale caller snapshot.
|
||||
persistMu sync.Mutex
|
||||
|
||||
// registerLocks serialises registration per machine key so concurrent
|
||||
// registrations of the same machine resolve to a single node instead of
|
||||
// racing the find-then-create section and each creating their own.
|
||||
// ponytail: entries are never pruned; bounded by distinct machine keys
|
||||
// seen, add cleanup on node delete only if it ever matters.
|
||||
registerLocks *xsync.Map[key.MachinePublic, *sync.Mutex]
|
||||
}
|
||||
|
||||
// lockRegistration serialises registration for a single machine key and
|
||||
// returns the unlock function.
|
||||
func (s *State) lockRegistration(machineKey key.MachinePublic) func() {
|
||||
mu, _ := s.registerLocks.LoadOrStore(machineKey, &sync.Mutex{})
|
||||
mu.Lock()
|
||||
|
||||
return mu.Unlock
|
||||
}
|
||||
|
||||
// NewState creates and initializes a new [State] instance, setting up the database,
|
||||
@@ -272,7 +296,8 @@ func NewState(cfg *types.Config) (*State, error) {
|
||||
nodeStore: nodeStore,
|
||||
pings: newPingTracker(),
|
||||
|
||||
sshCheckAuth: make(map[sshCheckPair]time.Time),
|
||||
sshCheckAuth: make(map[sshCheckPair]time.Time),
|
||||
registerLocks: xsync.NewMap[key.MachinePublic, *sync.Mutex](),
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -727,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
|
||||
@@ -1615,7 +1639,14 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
|
||||
params.ValidHostinfo,
|
||||
)
|
||||
|
||||
node.Endpoints = regData.Endpoints
|
||||
// Preserve the node's live endpoints when the register request carried
|
||||
// none. Web/OIDC relogins report endpoints via MapRequest, not register,
|
||||
// so RegData.Endpoints is empty; clearing the stored set would advertise
|
||||
// the re-keyed node with no way for peers to reach it. The first
|
||||
// MapRequest restores the live set.
|
||||
if len(regData.Endpoints) > 0 {
|
||||
node.Endpoints = regData.Endpoints
|
||||
}
|
||||
// Do NOT reset IsOnline here. Online status is managed exclusively by
|
||||
// [State.Connect]/[State.Disconnect] in the poll session lifecycle.
|
||||
// Resetting it during re-registration causes a false offline blip: the
|
||||
@@ -2021,16 +2052,37 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
|
||||
// Lookup existing nodes
|
||||
machineKey := regData.MachineKey
|
||||
existingNodeSameUser, _ := s.nodeStore.GetNodeByMachineKey(machineKey, types.UserID(user.ID))
|
||||
existingNodeAnyUser, _ := s.nodeStore.GetNodeByMachineKeyAnyUser(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
|
||||
// Serialise registration for this machine so concurrent auth callbacks
|
||||
// resolve to a single node rather than racing the find-then-create section.
|
||||
defer s.lockRegistration(machineKey)()
|
||||
|
||||
all := s.nodeStore.GetNodesByMachineKeyAllUsers(machineKey)
|
||||
|
||||
// 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().
|
||||
@@ -2060,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)
|
||||
@@ -2068,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() {
|
||||
@@ -2076,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
|
||||
@@ -2115,14 +2167,12 @@ func (s *State) HandleNodeFromAuthPath(
|
||||
return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
|
||||
}
|
||||
|
||||
var c change.Change
|
||||
if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
|
||||
c = change.PolicyChange()
|
||||
} else {
|
||||
c = change.NodeAdded(finalNode.ID())
|
||||
}
|
||||
policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
|
||||
|
||||
return finalNode, c, nil
|
||||
// nodeExistsForSameUser is true only for a same-user relogin; a tag->user
|
||||
// conversion is excluded, as it changes the peer's User — a structural
|
||||
// change peers must see in full, not a key-rotation patch.
|
||||
return finalNode, reauthChange(finalNode, nodeExistsForSameUser, policyChanged), nil
|
||||
}
|
||||
|
||||
// createNewNodeFromAuth creates a new node during auth callback.
|
||||
@@ -2164,21 +2214,60 @@ 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
|
||||
// registered (SetNodeTags clears UserID, re-indexing it under UserID(0)).
|
||||
// It is still the same machine, proven by the machine key, so recognise
|
||||
// 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, 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
|
||||
}
|
||||
}
|
||||
|
||||
// Tagged nodes have nil UserID, so they are indexed under UserID(0)
|
||||
// in nodesByMachineKey. Check there for tagged PAK re-registration.
|
||||
if pak.IsTagged() {
|
||||
return s.nodeStore.GetNodeByMachineKey(machineKey, 0)
|
||||
}
|
||||
|
||||
return types.NodeView{}, false
|
||||
return types.NodeView{}, false, nil
|
||||
}
|
||||
|
||||
//nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
|
||||
@@ -2186,6 +2275,10 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
regReq tailcfg.RegisterRequest,
|
||||
machineKey key.MachinePublic,
|
||||
) (types.NodeView, change.Change, error) {
|
||||
// Serialise registration for this machine so concurrent restarts resolve
|
||||
// to a single node rather than racing the find-then-create section.
|
||||
defer s.lockRegistration(machineKey)()
|
||||
|
||||
pak, err := s.GetPreAuthKey(regReq.Auth.AuthKey)
|
||||
if err != nil {
|
||||
return types.NodeView{}, change.Change{}, err
|
||||
@@ -2200,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)
|
||||
@@ -2219,10 +2315,24 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
isNodeKeyRotation := existsSameUser && existingNodeSameUser.Valid() &&
|
||||
existingNodeSameUser.NodeKey() != regReq.NodeKey
|
||||
|
||||
if isExistingNodeReregistering && !isNodeKeyRotation {
|
||||
// Existing node re-registering with same NodeKey: skip validation.
|
||||
// Pre-auth keys are only needed for initial authentication. Critical for
|
||||
// containers that run "tailscale up --authkey=KEY" on every restart.
|
||||
// An expired node is genuinely re-authenticating, not just waking up, so it
|
||||
// must present a valid key. Without this a node that re-uses its NodeKey
|
||||
// after expiry would skip validation and be re-authorised with a spent or
|
||||
// expired key; the boundary must not depend on the client rotating its key.
|
||||
isExpired := existsSameUser && existingNodeSameUser.Valid() &&
|
||||
existingNodeSameUser.IsExpired()
|
||||
|
||||
// A tagged key presented for a currently user-owned node converts that node
|
||||
// to tagged. That is an ownership change, not a plain refresh, so it must
|
||||
// present a valid key rather than ride the skip-validation fast-path.
|
||||
isOwnershipConversion := existsSameUser && existingNodeSameUser.Valid() &&
|
||||
pak.IsTagged() && !existingNodeSameUser.IsTagged()
|
||||
|
||||
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion {
|
||||
// Existing, still-valid node re-registering with same NodeKey: skip
|
||||
// validation. Pre-auth keys are only needed for initial authentication.
|
||||
// Critical for containers that run "tailscale up --authkey=KEY" on every
|
||||
// restart.
|
||||
log.Debug().
|
||||
Caller().
|
||||
Uint64(zf.NodeID, existingNodeSameUser.ID().Uint64()).
|
||||
@@ -2278,6 +2388,23 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
Str(zf.UserName, pakUsername()).
|
||||
Msg("Node re-registering with existing machine key and user, updating in place")
|
||||
|
||||
// Re-registration rotates the NodeKey to the client-supplied value.
|
||||
// Enforce the same 1:1 NodeKey<->MachineKey binding the auth path
|
||||
// (applyAuthNodeUpdate) and poll-time validation enforce: a NodeKey
|
||||
// already bound to a different machine must not be claimed here, or a
|
||||
// re-registering 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(regReq.NodeKey); ok &&
|
||||
existing.MachineKey() != machineKey {
|
||||
return types.NodeView{}, change.Change{}, ErrNodeKeyInUse
|
||||
}
|
||||
|
||||
// Snapshot the pre-update node so the NodeStore can be rolled back if
|
||||
// the database write below fails. The view points at the immutable
|
||||
// pre-update snapshot (UpdateNode swaps in a new one), so this stays
|
||||
// valid after the mutation.
|
||||
priorNode := existingNodeSameUser.AsStruct()
|
||||
|
||||
// Update existing node - NodeStore first, then database
|
||||
updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
|
||||
node.NodeKey = regReq.NodeKey
|
||||
@@ -2293,8 +2420,17 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
node.RegisterMethod = util.RegisterMethodAuthKey
|
||||
|
||||
// Tags from PreAuthKey are only applied during initial registration.
|
||||
// On re-registration the node keeps its existing tags and ownership.
|
||||
// Only update AuthKey reference.
|
||||
// On re-registration the node keeps its existing tags and ownership,
|
||||
// except when a tagged key converts a user-owned node: that adopts
|
||||
// the key's tags and drops user ownership (tagged nodes are
|
||||
// user-less and never expire). Only update AuthKey reference
|
||||
// otherwise.
|
||||
if pak.IsTagged() && !node.IsTagged() {
|
||||
node.Tags = pak.Proto().GetAclTags()
|
||||
node.UserID = nil
|
||||
node.User = nil
|
||||
node.Expiry = nil
|
||||
}
|
||||
node.AuthKey = pak
|
||||
node.AuthKeyID = &pak.ID
|
||||
// Do NOT reset IsOnline here. Online status is managed exclusively by
|
||||
@@ -2349,6 +2485,13 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
return nil, nil //nolint:nilnil // intentional: transaction success
|
||||
})
|
||||
if err != nil {
|
||||
// The NodeStore was updated before the database write. Roll it back
|
||||
// so it does not advertise a registration the database rejected
|
||||
// (e.g. a node key that a restart would not reload).
|
||||
if priorNode != nil {
|
||||
s.nodeStore.PutNode(*priorNode)
|
||||
}
|
||||
|
||||
return types.NodeView{}, change.Change{}, fmt.Errorf("writing node to database: %w", err)
|
||||
}
|
||||
|
||||
@@ -2363,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() {
|
||||
@@ -2389,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()).
|
||||
@@ -2430,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)
|
||||
@@ -2448,14 +2596,27 @@ func (s *State) HandleNodeFromPreAuthKey(
|
||||
return finalNode, change.NodeAdded(finalNode.ID()), fmt.Errorf("updating policy manager nodes: %w", err)
|
||||
}
|
||||
|
||||
var c change.Change
|
||||
if !usersChange.IsEmpty() || !nodesChange.IsEmpty() {
|
||||
c = change.PolicyChange()
|
||||
} else {
|
||||
c = change.NodeAdded(finalNode.ID())
|
||||
}
|
||||
policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
|
||||
|
||||
return finalNode, c, nil
|
||||
return finalNode, reauthChange(finalNode, existsSameUser, policyChanged), nil
|
||||
}
|
||||
|
||||
// reauthChange returns the [change.Change] to broadcast after an authentication
|
||||
// that updated or created a node.
|
||||
//
|
||||
// A pure relogin (isRelogin: an existing node, same user, with only its NodeKey
|
||||
// rotated) is sent as a minimal incremental peer patch via [change.NodeKeyRotated]
|
||||
// rather than re-advertising the whole node. A policy change forces a full
|
||||
// recompute; any other (new) node is a whole-node add.
|
||||
func reauthChange(node types.NodeView, isRelogin, policyChanged bool) change.Change {
|
||||
switch {
|
||||
case policyChanged:
|
||||
return change.PolicyChange()
|
||||
case isRelogin:
|
||||
return change.NodeKeyRotated(node)
|
||||
default:
|
||||
return change.NodeAdded(node.ID())
|
||||
}
|
||||
}
|
||||
|
||||
// updatePolicyManagerUsers updates the policy manager with current users.
|
||||
@@ -2656,8 +2817,13 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
|
||||
updatedNode, ok := s.nodeStore.UpdateNode(id, func(currentNode *types.Node) {
|
||||
peerChange := currentNode.PeerChangeFromMapRequest(req)
|
||||
|
||||
// Track what specifically changed
|
||||
endpointChanged = peerChange.Endpoints != nil
|
||||
// Track what specifically changed. An endpoint delta is only
|
||||
// broadcast-worthy when it adds a useful (non-STUN) endpoint;
|
||||
// STUN-only churn and pure shrinks are suppressed to reduce peer
|
||||
// churn (see endpointBroadcastWorthy). The new set is still stored
|
||||
// via ApplyPeerChange below regardless of this decision.
|
||||
endpointChanged = peerChange.Endpoints != nil &&
|
||||
endpointBroadcastWorthy(currentNode.Endpoints, req.Endpoints, req.EndpointTypes)
|
||||
derpChanged = peerChange.DERPRegion != 0
|
||||
hostinfoChanged = !hostinfoEqual(currentNode.View(), req.Hostinfo)
|
||||
|
||||
@@ -2841,6 +3007,63 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
|
||||
return buildMapRequestChangeResponse(id, updatedNode, hostinfoChanged, endpointChanged, derpChanged)
|
||||
}
|
||||
|
||||
// endpointBroadcastWorthy reports whether an endpoint-only delta is worth
|
||||
// fanning out to peers as an incremental PeersChangedPatch. A delta that only
|
||||
// adds STUN-derived endpoints — or only removes endpoints — is suppressed:
|
||||
// bare STUN endpoints are unlikely to be open and churn a lot (the client
|
||||
// re-derives those paths over disco anyway), and a pure shrink is not worth
|
||||
// telling peers about. Suppressing this churn keeps peers' views stable.
|
||||
//
|
||||
// The decision is intentionally conservative: it gates the broadcast only,
|
||||
// not storage. The node's full endpoint set (STUN included) is still stored
|
||||
// and rides along the next substantive change or full MapResponse, so no
|
||||
// reachable path is permanently hidden from peers.
|
||||
//
|
||||
// Limitation: headscale stores bare []netip.AddrPort with no per-endpoint
|
||||
// type, so we can only classify the *new* request's endpoints (via the
|
||||
// parallel newTypes slice). We therefore gate on whether any newly-added
|
||||
// endpoint (present in new, absent from stored) is useful (non-STUN). When
|
||||
// newTypes is absent or shorter than newEPs (older clients), the unknown
|
||||
// endpoints are treated as useful, preserving the pre-existing always-broadcast
|
||||
// behaviour and never hiding a genuinely new endpoint.
|
||||
func endpointBroadcastWorthy(
|
||||
stored, newEPs []netip.AddrPort,
|
||||
newTypes []tailcfg.EndpointType,
|
||||
) bool {
|
||||
storedSet := make(map[netip.AddrPort]struct{}, len(stored))
|
||||
for _, ep := range stored {
|
||||
storedSet[ep] = struct{}{}
|
||||
}
|
||||
|
||||
for i, ep := range newEPs {
|
||||
if _, ok := storedSet[ep]; ok {
|
||||
// Already known to peers; not a newly-added endpoint.
|
||||
continue
|
||||
}
|
||||
|
||||
// A newly-added endpoint with no type information (older client)
|
||||
// is treated as useful so we never hide a genuinely new endpoint.
|
||||
t := tailcfg.EndpointUnknownType
|
||||
if i < len(newTypes) {
|
||||
t = newTypes[i]
|
||||
}
|
||||
|
||||
if isUsefulEndpointType(t) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
// isUsefulEndpointType reports whether an endpoint type is worth eagerly
|
||||
// broadcasting to peers. STUN-derived endpoints are excluded because they are
|
||||
// churny and unlikely to be directly reachable; magicsock's disco handles
|
||||
// establishing those paths.
|
||||
func isUsefulEndpointType(t tailcfg.EndpointType) bool {
|
||||
return t != tailcfg.EndpointSTUN && t != tailcfg.EndpointSTUN4LocalPort
|
||||
}
|
||||
|
||||
// buildMapRequestChangeResponse determines the appropriate response type for a [tailcfg.MapRequest] update.
|
||||
// Hostinfo changes require a full update, while endpoint/DERP changes can use lightweight patches.
|
||||
func buildMapRequestChangeResponse(
|
||||
|
||||
@@ -490,6 +490,35 @@ func EndpointOrDERPUpdate(id types.NodeID, patch *tailcfg.PeerChange) Change {
|
||||
return c
|
||||
}
|
||||
|
||||
// NodeKeyRotated returns a [Change] for a node re-logging in: its NodeKey (and
|
||||
// possibly DiscoKey, key expiry, or endpoints) changed, but nothing structural
|
||||
// did. Peers only need those changed fields, so it is sent as the minimal
|
||||
// incremental [tailcfg.PeerChange] patch rather than re-advertising the whole
|
||||
// node — the smallest update that conveys the rotation, and the least
|
||||
// disruptive for peers reconciling it.
|
||||
func NodeKeyRotated(node types.NodeView) Change {
|
||||
nk := node.NodeKey()
|
||||
dk := node.DiscoKey()
|
||||
|
||||
// KeyExpiry is always set: the zero value clears any prior expiry on the
|
||||
// peer (un-expire), and a non-zero value carries the new expiry.
|
||||
var expiry time.Time
|
||||
if e, ok := node.Expiry().GetOk(); ok {
|
||||
expiry = e
|
||||
}
|
||||
|
||||
c := PeerPatched("node key rotated (relogin)", &tailcfg.PeerChange{
|
||||
NodeID: tailcfg.NodeID(node.ID()), //nolint:gosec // NodeID is bounded
|
||||
Key: &nk,
|
||||
DiscoKey: &dk,
|
||||
KeyExpiry: &expiry,
|
||||
Endpoints: node.Endpoints().AsSlice(),
|
||||
})
|
||||
c.OriginNode = node.ID()
|
||||
|
||||
return c
|
||||
}
|
||||
|
||||
// UserAdded returns a [Change] for when a user is added or updated.
|
||||
// A full update is sent to refresh user profiles on all nodes.
|
||||
func UserAdded() Change {
|
||||
|
||||
@@ -4,11 +4,13 @@ import (
|
||||
"net/netip"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
)
|
||||
|
||||
func TestChange_FieldSync(t *testing.T) {
|
||||
@@ -653,3 +655,34 @@ func TestNodeOnlineOfflineForSubnetRouter(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestNodeKeyRotatedEmitsPatchNotWholeNode proves a relogin is delivered to
|
||||
// peers as an incremental peer patch, not a whole-node add. A whole-node add is
|
||||
// non-patchifiable on the tailscale client whenever Hostinfo changed (which it
|
||||
// does on relogin), forcing the broken NodeMutationAdd path that strands a
|
||||
// re-keyed, momentarily-endpoint-less peer.
|
||||
func TestNodeKeyRotatedEmitsPatchNotWholeNode(t *testing.T) {
|
||||
expiry := time.Now().Add(24 * time.Hour).UTC()
|
||||
node := types.Node{
|
||||
ID: 7,
|
||||
NodeKey: key.NewNode().Public(),
|
||||
DiscoKey: key.NewDisco().Public(),
|
||||
Endpoints: []netip.AddrPort{netip.MustParseAddrPort("192.168.1.9:41641")},
|
||||
Expiry: &expiry,
|
||||
}
|
||||
view := node.View()
|
||||
|
||||
c := NodeKeyRotated(view)
|
||||
|
||||
assert.False(t, c.IsFull(), "relogin must be a peer patch, not a full update")
|
||||
assert.Empty(t, c.PeersChanged, "relogin must not emit a whole-node PeersChanged")
|
||||
require.Len(t, c.PeerPatches, 1, "relogin must emit exactly one peer patch")
|
||||
|
||||
patch := c.PeerPatches[0]
|
||||
assert.Equal(t, view.ID().NodeID(), patch.NodeID)
|
||||
require.NotNil(t, patch.Key, "patch must carry the rotated NodeKey")
|
||||
assert.Equal(t, node.NodeKey, *patch.Key)
|
||||
require.NotNil(t, patch.KeyExpiry, "patch must carry KeyExpiry to (un)expire the peer")
|
||||
assert.Equal(t, expiry, *patch.KeyExpiry)
|
||||
assert.Equal(t, []netip.AddrPort(node.Endpoints), patch.Endpoints, "patch must carry endpoints")
|
||||
}
|
||||
|
||||
+1
-1
@@ -111,7 +111,7 @@ extra:
|
||||
- icon: fontawesome/brands/discord
|
||||
link: https://discord.gg/c84AZQhmpx
|
||||
headscale:
|
||||
version: 0.28.0
|
||||
version: 0.29.0
|
||||
|
||||
# Extensions
|
||||
markdown_extensions:
|
||||
|
||||
Reference in New Issue
Block a user