state: apply a new pre-auth key's tags on re-registration

Re-registering a tagged node with a different key discarded the new key's
tags and left a stale auth-key reference; retag on key change and persist it.

Fixes #3370

(cherry picked from commit fc16cc6905)
This commit is contained in:
Kristoffer Dalby
2026-07-22 06:32:08 +00:00
committed by Kristoffer Dalby
parent d202883200
commit 1fccdb18bd
5 changed files with 814 additions and 14 deletions
+2
View File
@@ -340,6 +340,8 @@ jobs:
- TestTagsAuthKeyWithTagCannotAddViaCLI
- TestTagsAuthKeyWithTagCannotChangeViaCLI
- TestTagsAuthKeyWithTagAdminOverrideReauthPreserves
- TestTagsReauthDifferentKeyRetagsNode
- TestTagsReauthDifferentKeyRemovesTag
- TestTagsAuthKeyWithTagCLICannotModifyAdminTags
- TestTagsAuthKeyWithoutTagCannotRequestTags
- TestTagsAuthKeyWithoutTagRegisterNoTags
+1
View File
@@ -7,6 +7,7 @@
### Changes
- Fix tagged node stuck expired after `tailscale logout`, unable to re-authenticate [#3394](https://github.com/juanfont/headscale/pull/3394)
- Re-registering a tagged node with a different pre-auth key now applies the new key's tags instead of silently keeping the old ones [#3394](https://github.com/juanfont/headscale/pull/3394)
## 0.29.2 (2026-07-01)
+552
View File
@@ -553,6 +553,558 @@ func TestIssue3371_TaggedNodePastExpirySelfHealsOnReregister(t *testing.T) {
require.Equal(t, nodeID, healed.ID(), "must be the same node")
}
// TestIssue3371_ExpiredTaggedNodeSameSpentKeyNotRevalidated is the gating test
// for the isExpired-gate exclusion (state.go: `&& !existingNodeSameUser.IsTagged()`).
// A tagged node carrying a stale PAST expiry, re-registering with the SAME
// single-use key and the SAME node key (no rotation), must take the
// skip-validation fast path — otherwise the spent key is re-validated and
// rejected with "authkey already used", the exact lockout #3371 fixes. Without
// the tagged exclusion this fails; with it the node self-heals. The neighbours
// all rotate the node key or use a reusable key, so validation runs regardless
// and none probes this fast-path exclusion.
func TestIssue3371_ExpiredTaggedNodeSameSpentKeyNotRevalidated(t *testing.T) {
s := newRetagTestState(t)
// Single-use tagged key.
pak, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:foo"})
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: "spent-tagged"},
}
node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
require.True(t, node.IsTagged())
// Stale past (logout) expiry, and the single-use key is now spent.
past := time.Now().Add(-1 * time.Hour)
_, ok := s.nodeStore.UpdateNode(node.ID(), func(n *types.Node) {
n.Expiry = &past
})
require.True(t, ok)
// Re-register with the SAME key and the SAME node key (no rotation). The
// tagged exclusion from the isExpired gate must let this skip validation, so
// the spent single-use key is not rejected.
healed, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err,
"a tagged node with a stale past expiry must skip re-validation of its spent key")
require.False(t, healed.IsExpired(), "stale expiry cleared")
require.Nil(t, healed.AsStruct().Expiry, "tagged node key-expiry disabled")
require.Equal(t, node.ID(), healed.ID())
}
// TestTaggedPAKReauthRetagsExistingTaggedNode reproduces issue #3370: an
// already-tagged node re-authenticating with a *fresh, valid* tagged pre-auth
// key carrying *different* tags has that key validated and consumed, but its
// tags are silently discarded — the node keeps its old tags.
//
// Root cause: the in-place re-registration path in HandleNodeFromPreAuthKey
// only applies a key's tags when a tagged key converts a user-owned node
// (`pak.IsTagged() && !node.IsTagged()`). An already-tagged node fails
// `!node.IsTagged()`, so a differently-tagged key leaves the tags unchanged —
// while the single-use key is still marked used below (hsdb.UsePreAuthKey).
//
// Tagged-PAK tags are authorised by possession of the key (only syntactic
// `tag:` validation at creation; no tagOwners policy is required — the bug
// reproduces with an empty policy). Re-keying is Tailscale's documented method
// for changing an auth-key device's tags, so a fresh key's tags must be
// applied on re-registration.
//
// https://github.com/juanfont/headscale/issues/3370
func TestTaggedPAKReauthRetagsExistingTaggedNode(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
// Empty policy: tagged-PAK tags need no tagOwners entry (issue reproduces
// with a completely empty policy).
// KEY1: single-use tags-only key carrying tag:tag1.
key1, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag1"})
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key1.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "retag-node"},
}
// Initial registration: node comes up tagged tag:tag1.
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
require.True(t, first.IsTagged(), "precondition: node registered tagged")
require.Equal(t, []string{"tag:tag1"}, first.Tags().AsSlice())
nodeID := first.ID()
firstIPv4 := first.IPv4()
firstIPv6 := first.IPv6()
// KEY2: fresh single-use tags-only key carrying tag:tag2.
key2, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag2"})
require.NoError(t, err)
// `tailscale up --force-reauth --auth-key KEY2`: same machine, rotated node
// key, fresh valid key. This validates and consumes KEY2.
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: key2.Key}
reReg.NodeKey = key.NewNode().Public()
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, nodeID, second.ID(), "must update in place, not duplicate")
require.Equal(t, 1, s.ListNodes().Len(), "machine must map to exactly one node")
require.True(t, second.IsTagged(), "node must remain tagged")
// KEY2 was validated and consumed regardless of the outcome.
consumed, err := s.GetPreAuthKey(key2.Key)
require.NoError(t, err)
require.True(t, consumed.Used, "single-use key was consumed by the re-auth")
// The consumed key's tags must be applied: re-keying retags the device
// (Tailscale's documented behaviour). This is the assertion that fails
// before the fix — the node keeps ["tag:tag1"].
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice(),
"re-authenticating with a differently-tagged key must retag the node, "+
"not silently discard the tags of the consumed key")
// Identity continuity: the reporter stresses "same node, same IP". The
// re-auth updates in place, so the machine key and both Tailscale IPs are
// preserved while the node key rotates. A retag must not re-allocate IPs or
// duplicate the node.
require.Equal(t, machineKey.Public(), second.MachineKey(),
"machine key is the stable identity across re-auth")
require.Equal(t, firstIPv4, second.IPv4(), "IPv4 preserved across retag")
require.Equal(t, firstIPv6, second.IPv6(), "IPv6 preserved across retag")
require.NotEqual(t, first.NodeKey(), second.NodeKey(),
"--force-reauth rotates the node key")
// A retagged node stays a tagged node: user-less and key-expiry disabled.
require.Nil(t, second.AsStruct().Expiry, "tagged node keeps nil key expiry")
}
// TestTaggedPAKReauthSameKeyPreservesTags is the counterpart constraint to
// #3370: re-authenticating with the *same* tagged key must NOT clobber the
// node's current tags, even after an admin retagged it via
// `headscale nodes tag`. This is the unit-level guard for the integration
// test TestTagsAuthKeyWithTagAdminOverrideReauthPreserves (admin decisions are
// authoritative), and it is why the retag discriminator must key on the
// pre-auth key's *identity* (a different key) rather than on "validation ran"
// (a --force-reauth with the same key also runs validation). This test passes
// today and must keep passing after the #3370 fix.
func TestTaggedPAKReauthSameKeyPreservesTags(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
// tag:admin must be permitted for the admin SetNodeTags call.
_, err = s.SetPolicy([]byte(`{"tagOwners":{"tag:admin":["admin@"]}}`))
require.NoError(t, err)
// Reusable tagged key (the shape used by the admin-override integration
// test) so the same key can be presented twice.
pak, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:orig"})
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "sticky-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
require.Equal(t, []string{"tag:orig"}, first.Tags().AsSlice())
nodeID := first.ID()
// Admin retags the node out-of-band.
tagged, _, err := s.SetNodeTags(nodeID, []string{"tag:admin"})
require.NoError(t, err)
require.Equal(t, []string{"tag:admin"}, tagged.Tags().AsSlice())
// `--force-reauth` with the SAME key: rotate the node key so validation
// runs, exactly like the client does.
reReg := regReq
reReg.NodeKey = key.NewNode().Public()
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, nodeID, second.ID())
require.Equal(t, []string{"tag:admin"}, second.Tags().AsSlice(),
"re-authenticating with the same key must preserve the admin-assigned tags")
}
// TestTaggedPAKReauthSpentKeySameNodeKeyRejected pins the authorization boundary
// the retag must respect: presenting a *spent* single-use tagged key to retag an
// already-tagged node must be rejected, even when the client reuses its node key
// (the skip-validation fast path). Without forcing validation on a retag, a
// used/revoked/expired tagged credential could still apply its tags — replaying
// a dead key to escalate a machine onto a tag, and defeating key revocation. The
// existing isExpired comment already declares the boundary "must not depend on
// the client rotating its key"; retag must honour the same rule.
func TestTaggedPAKReauthSpentKeySameNodeKeyRejected(t *testing.T) {
s := newRetagTestState(t)
// KEY1: single-use tag:tag1, used to register.
key1, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag1"})
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key1.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "retag-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
require.Equal(t, []string{"tag:tag1"}, first.Tags().AsSlice())
// KEY2: single-use tag:tag2, spent elsewhere so it is already Used.
key2, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag2"})
require.NoError(t, err)
_, _, err = s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key2.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "other-node"},
}, key.NewMachine().Public())
require.NoError(t, err)
// Re-register the FIRST node with the now-spent KEY2, REUSING its node key
// (no rotation) so it targets the skip-validation fast path. A spent key must
// be rejected, and the node's tags must be unchanged.
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: key2.Key}
// reReg.NodeKey stays nodeKey (same) -> fast path.
_, _, err = s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.Error(t, err, "a spent tagged key must not be able to retag on the fast path")
require.Contains(t, err.Error(), "authkey already used")
after, ok := s.GetNodeByID(first.ID())
require.True(t, ok)
require.Equal(t, []string{"tag:tag1"}, after.Tags().AsSlice(),
"a rejected spent key must not have applied its tags")
}
// retagReauthCase drives the #3370 retag scenario with configurable key shapes
// so the sibling cases below stay a few lines each. It registers a node with
// key1, then re-registers the same machine with key2, and returns the
// re-registered node. rotateNodeKey mirrors --force-reauth (the client rotates
// its node key); when false the client reuses its node key.
func retagReauthCase(
t *testing.T,
s *State,
key1Tags, key2Tags []string,
key1User, key2User *types.UserID,
reusable, rotateNodeKey bool,
) (types.NodeView, types.NodeView) {
t.Helper()
key1, err := s.CreatePreAuthKey(key1User, reusable, false, nil, key1Tags)
require.NoError(t, err)
machineKey := key.NewMachine()
nodeKey := key.NewNode()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key1.Key},
NodeKey: nodeKey.Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "retag-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
key2, err := s.CreatePreAuthKey(key2User, reusable, false, nil, key2Tags)
require.NoError(t, err)
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: key2.Key}
if rotateNodeKey {
reReg.NodeKey = key.NewNode().Public()
}
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, first.ID(), second.ID(), "must update in place, not duplicate")
return first, second
}
// TestTaggedPAKReauthReusableKeyRetags: a *reusable* differently-tagged key
// must retag too. The retag must key on key identity, not on the single-use
// `Used` flag, so it cannot be coupled to single-use semantics.
func TestTaggedPAKReauthReusableKeyRetags(t *testing.T) {
s := newRetagTestState(t)
_, second := retagReauthCase(t, s,
[]string{"tag:tag1"}, []string{"tag:tag2"}, nil, nil, true /*reusable*/, true)
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice())
}
// TestTaggedPAKReauthDifferentKeySameNodeKey: retag must fire even when the
// client reuses its node key (no rotation). The trigger is key identity,
// decoupled from NodeKey rotation.
func TestTaggedPAKReauthDifferentKeySameNodeKey(t *testing.T) {
s := newRetagTestState(t)
_, second := retagReauthCase(t, s,
[]string{"tag:tag1"}, []string{"tag:tag2"}, nil, nil, false, false /*same node key*/)
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice())
}
// TestTaggedPAKReauthSameSingleUseKeySameNodeKeyPreservesTags guards the #2830
// container-restart permutation for a tags-only single-use key: presenting the
// SAME (already-used) single-use key with the SAME node key must take the
// skip-validation fast path, succeed, and keep the node's tags — not reject
// with "authkey already used" and not spuriously retag. This confirms the
// keyChanged/isRetag additions never fire for a same-key restart, the exact
// property #2830/#3312 depend on. (The existing #3312 test uses a user-owned
// one-shot key; this covers the tags-only single-use key.)
func TestTaggedPAKReauthSameSingleUseKeySameNodeKeyPreservesTags(t *testing.T) {
s := newRetagTestState(t)
pak, err := s.CreatePreAuthKey(nil, false /*single-use*/, false, nil, []string{"tag:tag1"})
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "restart-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
require.Equal(t, []string{"tag:tag1"}, first.Tags().AsSlice())
// Container restart: same key, same node key, same machine. The single-use
// key is already consumed, so re-validation would reject it.
second, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err,
"same-key restart of a tagged node must skip validation, not reject a spent single-use key")
require.Equal(t, first.ID(), second.ID(), "must update in place")
require.Equal(t, []string{"tag:tag1"}, second.Tags().AsSlice(),
"same-key restart must preserve tags (no spurious retag)")
require.Equal(t, 1, s.ListNodes().Len(), "no duplicate node")
}
// TestTaggedPAKReauthUserScopedKeyRetags: a user-scoped tagged key
// (User != nil, the `headscale preauthkeys create -u <user> --tags` shape)
// exercises the pak.User != nil branch of findExistingNodeForPAK, a different
// lookup path from tags-only keys. It must still retag.
func TestTaggedPAKReauthUserScopedKeyRetags(t *testing.T) {
s := newRetagTestState(t)
user := s.CreateUserForTest("owner")
uid := user.TypedID()
_, second := retagReauthCase(t, s,
[]string{"tag:tag1"}, []string{"tag:tag2"}, uid, uid, false, true)
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice())
require.True(t, second.IsTagged())
}
// TestTaggedPAKReauthReplacesNotMerges pins the Tailscale KB 1068 rule that
// re-keying *replaces* the tag set (it does not union). Re-keying a
// {tag1,tag2} node with a {tag1} key must drop tag2.
func TestTaggedPAKReauthReplacesNotMerges(t *testing.T) {
s := newRetagTestState(t)
_, second := retagReauthCase(t, s,
[]string{"tag:tag1", "tag:tag2"}, []string{"tag:tag1"}, nil, nil, false, true)
require.Equal(t, []string{"tag:tag1"}, second.Tags().AsSlice(),
"re-keying replaces the tag set; tag:tag2 must be removed, not merged")
}
// TestExpiredTaggedNodeReauthRetags: a tagged node that looks expired (a stale
// logout stamp — see #3371) re-authenticating with a fresh differently-tagged
// key must still retag. Combines the isExpired validation path with the retag.
func TestExpiredTaggedNodeReauthRetags(t *testing.T) {
s := newRetagTestState(t)
key1, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag1"})
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key1.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "retag-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
// Force a stale past expiry onto the tagged node.
past := time.Now().Add(-1 * time.Hour)
_, ok := s.nodeStore.UpdateNode(first.ID(), func(n *types.Node) {
n.Expiry = &past
})
require.True(t, ok)
key2, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag2"})
require.NoError(t, err)
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: key2.Key}
reReg.NodeKey = key.NewNode().Public()
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice(),
"an expired (stale logout stamp) tagged node must retag on fresh-key re-auth")
require.Nil(t, second.AsStruct().Expiry,
"retag must clear a stale expiry: the node is tagged and tagged nodes never expire")
require.False(t, second.IsExpired(),
"a retagged node must not remain expired")
}
// TestTaggedPAKReauthRetagPreservesFutureAdminExpiry pins the composition
// decision between #3370 (retag) and #3371 (never wrongly expire a tagged
// node): a deliberate FUTURE expiry set by an admin via `headscale nodes expire`
// is a node property, not tied to the auth key, so re-keying an already-tagged
// node with a different key must retag it WITHOUT wiping that future expiry.
// Only a stale PAST expiry is cleared (see TestExpiredTaggedNodeReauthRetags).
// Symmetric with the same-key relogin path.
func TestTaggedPAKReauthRetagPreservesFutureAdminExpiry(t *testing.T) {
s := newRetagTestState(t)
key1, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag1"})
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: key1.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "retag-node"},
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
// Admin sets a deliberate future expiry on the tagged node
// (`headscale nodes expire`), which SetNodeExpiry permits.
future := time.Now().Add(30 * 24 * time.Hour)
_, _, err = s.SetNodeExpiry(first.ID(), &future)
require.NoError(t, err)
// Re-key with a different tagged key.
key2, err := s.CreatePreAuthKey(nil, false, false, nil, []string{"tag:tag2"})
require.NoError(t, err)
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: key2.Key}
reReg.NodeKey = key.NewNode().Public()
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, []string{"tag:tag2"}, second.Tags().AsSlice(), "node retags")
require.NotNil(t, second.AsStruct().Expiry,
"a deliberate future admin expiry must survive a different-key retag")
require.Equal(t, future.Unix(), second.AsStruct().Expiry.Unix(),
"the admin expiry value must be unchanged")
}
// newRetagTestState builds a State with an empty policy (tagged-PAK tags need
// no tagOwners entry).
func newRetagTestState(t *testing.T) *State {
t.Helper()
cfg := persistTestConfig(t.TempDir() + "/headscale.db")
s, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s.Close() })
return s
}
// TestPreAuthKeyReauthPersistsAuthKeyID pins the root cause behind the
// ephemeral-flag revert bug found while auditing #3370: on re-registration
// with a *different* key, HandleNodeFromPreAuthKey updates node.AuthKeyID in
// the NodeStore (state.go, in the in-place closure) but AuthKeyID is excluded
// from nodeUpdateColumns (added for #2862 to avoid persisting a *deleted* key's
// stale reference on MapRequest). The exclusion is correct for MapRequest, but
// on the re-registration path the presented key is freshly loaded and valid, so
// the new auth_key_id must be persisted. Otherwise a control-plane restart
// reloads the OLD key, and any key-scoped property (notably Ephemeral) silently
// reverts — an ephemeral->non-ephemeral re-auth leaves a node that looks
// persistent in memory but is ephemeral again after restart and can be GC'd.
//
// Drives the real handler twice, then reopens the DB to prove the association
// survives the reload.
func TestPreAuthKeyReauthPersistsAuthKeyID(t *testing.T) {
dbPath := t.TempDir() + "/headscale.db"
cfg := persistTestConfig(dbPath)
s, err := NewState(cfg)
require.NoError(t, err)
user := s.CreateUserForTest("rekey-user")
// KEY A: ephemeral, single-use.
keyA, err := s.CreatePreAuthKey(user.TypedID(), false, true /*ephemeral*/, nil, nil)
require.NoError(t, err)
machineKey := key.NewMachine()
regReq := tailcfg.RegisterRequest{
Auth: &tailcfg.RegisterResponseAuth{AuthKey: keyA.Key},
NodeKey: key.NewNode().Public(),
Hostinfo: &tailcfg.Hostinfo{Hostname: "rekey-node"},
Expiry: time.Now().Add(24 * time.Hour),
}
first, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public())
require.NoError(t, err)
nodeID := first.ID()
require.True(t, first.IsEphemeral(), "precondition: registered with an ephemeral key")
// KEY B: non-ephemeral, single-use, same user. `--force-reauth` rotates the
// node key so this is a genuine re-registration that consumes KEY B.
keyB, err := s.CreatePreAuthKey(user.TypedID(), false, false /*non-ephemeral*/, nil, nil)
require.NoError(t, err)
reReg := regReq
reReg.Auth = &tailcfg.RegisterResponseAuth{AuthKey: keyB.Key}
reReg.NodeKey = key.NewNode().Public()
second, _, err := s.HandleNodeFromPreAuthKey(reReg, machineKey.Public())
require.NoError(t, err)
require.Equal(t, nodeID, second.ID(), "must update in place")
// In-memory, the node now tracks KEY B (non-ephemeral).
require.NotNil(t, second.AuthKeyID().Get())
require.Equal(t, keyB.ID, second.AuthKeyID().Get(),
"NodeStore must reference the key just used to re-authenticate")
require.False(t, second.IsEphemeral(),
"re-auth with a non-ephemeral key must make the node non-ephemeral")
// Restart the control plane: reload state from the database only.
require.NoError(t, s.Close())
s2, err := NewState(cfg)
require.NoError(t, err)
t.Cleanup(func() { _ = s2.Close() })
reloaded, ok := s2.GetNodeByID(nodeID)
require.True(t, ok, "node must reload from DB after restart")
// This is the assertion that fails today: auth_key_id was never persisted on
// re-registration, so the reload resurrects KEY A and the node is ephemeral
// again — at risk of GC on the next disconnect.
require.NotNil(t, reloaded.AuthKeyID().Get())
require.Equal(t, keyB.ID, reloaded.AuthKeyID().Get(),
"auth_key_id must persist across restart, not revert to the old key")
require.False(t, reloaded.IsEphemeral(),
"ephemerality must not silently revert after a control-plane restart")
}
// 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`).
+52 -14
View File
@@ -2351,7 +2351,17 @@ func (s *State) HandleNodeFromPreAuthKey(
isOwnershipConversion := existsSameUser && existingNodeSameUser.Valid() &&
pak.IsTagged() && !existingNodeSameUser.IsTagged()
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion {
// A tagged key that differs from the one the node last authed with retags
// the node (see the in-place update below). Applying a key's tags is an
// authorisation decision, so the key must be validated rather than ride the
// skip-validation fast-path; otherwise a spent, revoked or expired tagged
// key could still retag a node that reuses its node key. Like isExpired,
// this boundary must not depend on the client rotating its key.
isRetag := existsSameUser && existingNodeSameUser.Valid() &&
pak.IsTagged() && existingNodeSameUser.IsTagged() &&
(!existingNodeSameUser.AuthKeyID().Valid() || existingNodeSameUser.AuthKeyID().Get() != pak.ID)
if isExistingNodeReregistering && !isNodeKeyRotation && !isExpired && !isOwnershipConversion && !isRetag {
// 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
@@ -2399,8 +2409,10 @@ func (s *State) HandleNodeFromPreAuthKey(
var finalNode types.NodeView
// If this node exists for this user, update the node in place.
// Note: For tags-only keys (pak.User == nil), existsSameUser is always false.
// If this node exists for this user, update the node in place. For a
// tags-only key (pak.User == nil) this is true when the machine already has
// a tagged node (findExistingNodeForPAK matches it under UserID 0); for a
// user-owned key it is true when the same user already has the node.
if existsSameUser && existingNodeSameUser.Valid() {
log.Trace().
Caller().
@@ -2442,17 +2454,37 @@ 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,
// 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()
// Tags from a PreAuthKey are applied on initial registration and
// re-applied whenever a *different* key is presented on
// re-registration: re-keying is Tailscale's documented way to change
// an auth-key device's tags (KB 1068 - "generate a new auth key with
// the new set of tags ... doing so replaces the device's existing
// tags"). Presenting the SAME key again (container restart,
// #2830/#3312) preserves the node's current tags and any admin
// override. A tagged key presented for a user-owned node also converts
// it, dropping user ownership.
//
// node.AuthKeyID still holds the prior key's ID here (it is reassigned
// below), and SetNodeTags leaves AuthKeyID intact, so an admin retag
// cannot masquerade as a new key.
keyChanged := node.AuthKeyID == nil || *node.AuthKeyID != pak.ID
if pak.IsTagged() && (!node.IsTagged() || keyChanged) {
wasUserOwned := !node.IsTagged()
node.Tags = pak.Tags
node.UserID = nil
node.User = nil
node.Expiry = nil
// Converting a user-owned node to tagged drops the user's key
// expiry (tagged nodes never expire). But retagging an
// already-tagged node must preserve a deliberate FUTURE expiry
// set via `headscale nodes expire` - that is a node property, not
// tied to the auth key - and only clear a stale PAST expiry. This
// keeps the retag path symmetric with the same-key relogin path
// (#3371) rather than silently overriding an admin decision.
if wasUserOwned || node.IsExpired() {
node.Expiry = nil
}
}
node.AuthKey = pak
node.AuthKeyID = &pak.ID
@@ -2493,8 +2525,14 @@ func (s *State) HandleNodeFromPreAuthKey(
_, err = hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
// Explicitly select all node columns so GORM includes nil/zero-value fields
// (see nodeUpdateColumns comment).
err := tx.Select(nodeUpdateColumns).Updates(updatedNodeView.AsStruct()).Error
// (see nodeUpdateColumns comment). AuthKeyID is normally excluded to
// avoid persisting a deleted key's stale reference on MapRequest
// (#2862), but re-registration presents a freshly-validated key, so
// its ID must be persisted here — otherwise a restart reloads the old
// key and any key-scoped property (e.g. Ephemeral) silently reverts.
reregColumns := append(slices.Clone(nodeUpdateColumns), "AuthKeyID")
err := tx.Select(reregColumns).Updates(updatedNodeView.AsStruct()).Error
if err != nil {
return nil, fmt.Errorf("saving node: %w", err)
}
+207
View File
@@ -579,6 +579,213 @@ func TestTagsAuthKeyWithTagAdminOverrideReauthPreserves(t *testing.T) {
t.Logf("Test 2.5 PASS: Admin tags preserved through reauth (admin decisions are authoritative)")
}
// TestTagsReauthDifferentKeyRetagsNode reproduces issue #3370 end-to-end with a
// real tailscale client: a node registered with a single-use tag:valid-owned
// key is re-authenticated via `tailscale up --force-reauth` with a *fresh*
// single-use tag:second key. Tailscale's documented behaviour (KB 1068) is that
// re-keying replaces the device's tags, verified by the reporter against SaaS on
// the same node/IP. Before the fix headscale consumes the new key but keeps the
// old tag; after the fix the node retags in place.
//
// Unlike Test 2.5 (same reusable key + admin override -> tags preserved), this
// presents a *different* key, so it exercises the opposite arm of the retag
// discriminator. It also asserts what a state-unit test cannot: the new tag
// propagates to the node's own Self view and netmap, and the node ID and IPs are
// unchanged.
//
// https://github.com/juanfont/headscale/issues/3370
func TestTagsReauthDifferentKeyRetagsNode(t *testing.T) {
IntegrationSkip(t)
spec := ScenarioSpec{
NodesPerUser: 0,
Users: []string{tagTestUser},
}
scenario, err := NewScenario(spec)
require.NoError(t, err)
defer scenario.ShutdownAssertNoPanics(t)
err = scenario.CreateHeadscaleEnv(
[]tsic.Option{},
hsic.WithACLPolicy(tagsTestPolicy()),
hsic.WithTestName("tags-rekey-retag"),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
userMap, err := headscale.MapUsers()
require.NoError(t, err)
userID := userMap[tagTestUser].GetId()
// KEY1: single-use tag:valid-owned.
key1, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})
require.NoError(t, err)
client, err := scenario.CreateTailscaleNode(
"head",
tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
)
require.NoError(t, err)
err = client.Login(headscale.GetEndpoint(), key1.GetKey())
require.NoError(t, err)
var (
initialNodeID uint64
initialIPs []string
)
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1)
if len(nodes) == 1 {
initialNodeID = nodes[0].GetId()
initialIPs = nodes[0].GetIpAddresses()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
t.Logf("Step 1: node %d registered with tag:valid-owned, IPs %v", initialNodeID, initialIPs)
// KEY2: fresh single-use tag:second. Re-key via --force-reauth.
key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:second"})
require.NoError(t, err)
//nolint:errcheck // result is verified via EventuallyWithT below
client.Execute([]string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--authkey=" + key2.GetKey(),
"--force-reauth",
})
// Server-side: node retagged in place, same node ID and IPs, no duplicate.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1, "must not duplicate the node")
if len(nodes) == 1 {
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:second"})
assert.Equal(c, initialNodeID, nodes[0].GetId(), "node ID must be unchanged")
assert.ElementsMatch(c, initialIPs, nodes[0].GetIpAddresses(), "IPs must be preserved")
}
}, integrationutil.ScaledTimeout(20*time.Second), integrationutil.SlowPoll, "server must reflect the retag")
// Node self view: the new tag propagates to the client (issue #2978 surface).
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNodeSelfHasTagsWithCollect(c, client, []string{"tag:second"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "node self view must reflect the retag")
// Netmap: independent serialization surface.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNetmapSelfHasTagsWithCollect(c, client, []string{"tag:second"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "netmap self must reflect the retag")
t.Logf("Test #3370 PASS: re-keying with a different tagged key retagged the node in place")
}
// TestTagsReauthDifferentKeyRemovesTag is the sharpest proof of the KB 1068
// "replaces, not merges" rule at the integration level: a node registered with
// a two-tag key is re-keyed with a single-tag key, and the second tag must be
// *removed*, not retained. Tag removal is the highest-risk propagation path
// (peers must stop seeing the removed tag), so it is worth a real-client test.
//
// https://github.com/juanfont/headscale/issues/3370
func TestTagsReauthDifferentKeyRemovesTag(t *testing.T) {
IntegrationSkip(t)
spec := ScenarioSpec{
NodesPerUser: 0,
Users: []string{tagTestUser},
}
scenario, err := NewScenario(spec)
require.NoError(t, err)
defer scenario.ShutdownAssertNoPanics(t)
err = scenario.CreateHeadscaleEnv(
[]tsic.Option{},
hsic.WithACLPolicy(tagsTestPolicy()),
hsic.WithTestName("tags-rekey-remove"),
)
requireNoErrHeadscaleEnv(t, err)
headscale, err := scenario.Headscale()
requireNoErrGetHeadscale(t, err)
userMap, err := headscale.MapUsers()
require.NoError(t, err)
userID := userMap[tagTestUser].GetId()
// KEY1: single-use with BOTH tags.
key1, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned", "tag:second"})
require.NoError(t, err)
client, err := scenario.CreateTailscaleNode(
"head",
tsic.WithNetwork(scenario.networks[scenario.testDefaultNetwork]),
)
require.NoError(t, err)
err = client.Login(headscale.GetEndpoint(), key1.GetKey())
require.NoError(t, err)
var initialNodeID uint64
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1)
if len(nodes) == 1 {
initialNodeID = nodes[0].GetId()
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:second", "tag:valid-owned"})
}
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
// KEY2: fresh single-use with ONLY tag:valid-owned. Re-key.
key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})
require.NoError(t, err)
//nolint:errcheck // result is verified via EventuallyWithT below
client.Execute([]string{
"tailscale", "up",
"--login-server=" + headscale.GetEndpoint(),
"--hostname=" + client.Hostname(),
"--authkey=" + key2.GetKey(),
"--force-reauth",
})
// tag:second must be gone on every surface.
assert.EventuallyWithT(t, func(c *assert.CollectT) {
nodes, err := headscale.ListNodes()
assert.NoError(c, err)
assert.Len(c, nodes, 1)
if len(nodes) == 1 {
assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
assert.Equal(c, initialNodeID, nodes[0].GetId())
}
}, integrationutil.ScaledTimeout(20*time.Second), integrationutil.SlowPoll, "re-keying must replace (remove tag:second), not merge")
assert.EventuallyWithT(t, func(c *assert.CollectT) {
assertNodeSelfHasTagsWithCollect(c, client, []string{"tag:valid-owned"})
}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "removed tag must clear from node self view")
t.Logf("Test #3370 PASS: re-keying replaced the tag set (tag:second removed)")
}
// TestTagsAuthKeyWithTagCLICannotModifyAdminTags tests that the client CLI
// cannot modify admin-assigned tags.
//