From 1ed5693fa4c4ae459e607ab652ac7d25c35aef51 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 22 Jul 2026 06:27:34 +0000 Subject: [PATCH] state: do not expire tagged nodes on logout Tagged nodes never expire, but handleLogout stamped a past expiry on them, leaving them stuck expired and unable to re-authenticate. Fixes #3371 --- .github/workflows/test-integration.yaml | 2 + CHANGELOG.md | 8 + hscontrol/auth.go | 14 + hscontrol/auth_tags_test.go | 389 ++++++++++++++++++ hscontrol/db/db.go | 27 ++ hscontrol/db/db_test.go | 58 +++ ...lear_tagged_node_expiry_migration_test.sql | 85 ++++ hscontrol/state/auth_tagged_expiry_test.go | 145 +++++++ hscontrol/state/state.go | 22 +- integration/tags_test.go | 199 +++++++++ 10 files changed, 948 insertions(+), 1 deletion(-) create mode 100644 hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 2f03497e..2c9913dc 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -379,6 +379,8 @@ jobs: - TestTagsAuthKeyWithoutUserInheritsTags - TestTagsAuthKeyWithoutUserRejectsAdvertisedTags - TestTagsAuthKeyConvertToUserViaCLIRegister + - TestTaggedNodeLogoutReloginSingleUseKeyOnline + - TestTaggedNodeLogoutReloginReusableKeyOnline - TestTS2021WebSocketGET - TestTS2021WASMClientUnderNode - TestTailscaleRustAxum diff --git a/CHANGELOG.md b/CHANGELOG.md index 785d7d85..180f6772 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,14 @@ keys remain all-access. - Expiring or deleting a non-existent pre-auth key now returns an error instead of silently succeeding [#3324](https://github.com/juanfont/headscale/pull/3324) - Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341) +## 0.29.3 (202x-xx-xx) + +**Minimum supported Tailscale client version: v1.80.0** + +### Changes + +- Fix tagged node stuck expired after `tailscale logout`, unable to re-authenticate [#3394](https://github.com/juanfont/headscale/pull/3394) + ## 0.29.2 (2026-07-01) **Minimum supported Tailscale client version: v1.80.0** diff --git a/hscontrol/auth.go b/hscontrol/auth.go index 7fc54b7c..727fc210 100644 --- a/hscontrol/auth.go +++ b/hscontrol/auth.go @@ -233,6 +233,20 @@ func (h *Headscale) handleLogout( Msg("Node is not ephemeral, setting expiry instead of deleting") } + // Tagged nodes have key expiry permanently disabled (they are owned by + // their tags, not a user, and never expire - KB 1068). Logging one out has + // no expiry semantics, so do not stamp an expiry on it: doing so leaves the + // node IsExpired() forever and it can never re-authenticate (#3371). The + // admin path `headscale nodes expire` remains free to set a deliberate + // expiry via SetNodeExpiry; only the logout path is guarded here. + if node.IsTagged() { + log.Debug(). + EmbedObject(node). + Msg("Tagged node logout: not stamping expiry (tagged nodes never expire)") + + return nodeToRegisterResponse(node), nil + } + // Update the internal state with the nodes new expiry, meaning it is // logged out. // diff --git a/hscontrol/auth_tags_test.go b/hscontrol/auth_tags_test.go index 3cea0cb0..da6ccf28 100644 --- a/hscontrol/auth_tags_test.go +++ b/hscontrol/auth_tags_test.go @@ -1337,3 +1337,392 @@ func TestReregistrationZeroExpiryStaysNil(t *testing.T) { assert.False(t, node2.Expiry().Valid(), "re-registration with zero client expiry and no default should leave expiry nil, not pointer to zero time") } + +// tsLogoutSentinelExpiry is the past expiry a real tailscale client sends on +// `tailscale logout`: time.Unix(123, 0) (controlclient/direct.go). The issue +// #3371 trace shows it verbatim as `expiry=123`. Using it here (rather than a +// generic time.Now().Add(-1h)) keeps the reproduction faithful to the wire +// behaviour: handleRegister must classify this as a logout, and handleLogout +// must clamp it to now. +func tsLogoutSentinelExpiry() time.Time { + return time.Unix(123, 0) +} + +// TestIssue3371_TaggedNodeLogoutReloginSingleUseKey reproduces +// https://github.com/juanfont/headscale/issues/3371 through the real +// register/logout HTTP-handler path (handleRegister -> handleLogout -> +// handleRegister), not by poking SetNodeExpiry directly. +// +// Root cause (a): `tailscale logout` sends a past expiry; handleLogout stamps +// it on the node via SetNodeExpiry with no IsTagged guard, so a tagged node — +// which must have key-expiry disabled — becomes Expired. +// +// Root cause (b): on the next `tailscale up --auth-key `, +// HandleNodeFromPreAuthKey sees an expired node, takes the expired-node +// validation path, consumes the fresh single-use key on the in-place +// re-registration, yet leaves the node expired (the expiry-refresh block is +// gated `!node.IsTagged()`). The response carries NodeKeyExpired=true, so the +// client rotates its node key and retries with the now-spent key, which is +// rejected with "authkey already used" forever. +// +// Faithful to the artifacts: the client rotates its NodeKey on relogin, the +// logout carries BOTH a past expiry AND an auth key, and a BRAND NEW key is +// presented for the relogin (the trace shows tag:tag2 keys burned while the +// node kept tag:tag1). +func TestIssue3371_TaggedNodeLogoutReloginSingleUseKey(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-logout-user") + tags := []string{"tag:tag1"} + + // `headscale preauthkeys create --tags tag:tag1` (single-use). + pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags) + require.NoError(t, err) + + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // `tailscale up --auth-key $KEY1`: initial join. + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"}, + } + + resp, err := app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + require.False(t, resp.NodeKeyExpired) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, node.IsTagged(), "precondition: node is tagged") + require.False(t, node.Expiry().Valid(), "precondition: tagged node has expiry disabled") + + // `tailscale logout`: client sends a past-expiry register with the auth key + // still attached (handleRegister must treat past expiry as logout regardless + // of Auth). Reuse the same node key: logout does not rotate it. + logoutReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Expiry: tsLogoutSentinelExpiry(), + } + + _, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public()) + require.NoError(t, err) + + // A tagged node must NOT be expired by logout — tagged nodes never expire. + // This is root cause (a); it fails before the fix. + nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + assert.False(t, nodeAfterLogout.IsExpired(), + "issue #3371 root cause (a): logout must not expire a tagged node") + assert.False(t, nodeAfterLogout.Expiry().Valid(), + "issue #3371 root cause (a): tagged node must keep key-expiry disabled after logout") + + // `tailscale up --auth-key $KEY2`: a BRAND NEW single-use key, and the client + // rotates its node key (as the real client does on relogin). + pak2, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, tags) + require.NoError(t, err) + + nodeKey2 := key.NewNode() + reloginReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak2.Key}, + NodeKey: nodeKey2.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "headscale-debug"}, + } + + reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public()) + require.NoError(t, err, + "issue #3371: a fresh valid key must re-authenticate the tagged node after logout") + require.NotNil(t, reloginResp) + + // The whole point: the node comes back online, not stuck expired. + assert.False(t, reloginResp.NodeKeyExpired, + "issue #3371: relogin response must not report the node key as expired") + assert.True(t, reloginResp.MachineAuthorized) + + relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public()) + require.True(t, found) + assert.True(t, relogged.IsTagged(), "node stays tagged after relogin") + assert.False(t, relogged.IsExpired(), + "issue #3371: tagged node must be online (not expired) after relogin") + assert.False(t, relogged.Expiry().Valid(), + "issue #3371: tagged node must have key-expiry disabled after relogin") + assert.Equal(t, node.ID(), relogged.ID(), "must re-use the same node, not duplicate") + assert.Equal(t, 1, app.state.ListNodes().Len(), "machine maps to exactly one node") +} + +// TestIssue3371_TaggedNodeLogoutReloginReusableKey is the reusable-key variant +// from the issue ("tailscale up then hangs indefinitely instead of erroring"). +// With a reusable key the relogin does not hit "authkey already used", but the +// node still stays expired without the fix — so the client never observes a +// non-expired node and hangs. The observable failure here is the persisted +// expired state after relogin. +func TestIssue3371_TaggedNodeLogoutReloginReusableKey(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-logout-reusable") + tags := []string{"tag:tag1"} + + // `headscale preauthkeys create --reusable --tags tag:tag1`. + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags) + 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: "reusable-tagged"}, + } + + _, err = app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, node.IsTagged()) + require.False(t, node.Expiry().Valid()) + + // Logout. + logoutReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Expiry: tsLogoutSentinelExpiry(), + } + _, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public()) + require.NoError(t, err) + + // Relogin with the same reusable key, rotating the node key. + nodeKey2 := key.NewNode() + reloginReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey2.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "reusable-tagged"}, + } + reloginResp, err := app.handleRegister(context.Background(), reloginReq, machineKey.Public()) + require.NoError(t, err) + require.NotNil(t, reloginResp) + + assert.False(t, reloginResp.NodeKeyExpired, + "issue #3371: reusable-key relogin must not report node key expired") + + relogged, found := app.state.GetNodeByNodeKey(nodeKey2.Public()) + require.True(t, found) + assert.True(t, relogged.IsTagged()) + assert.False(t, relogged.IsExpired(), + "issue #3371: tagged node must be online (not expired) after reusable-key relogin") + assert.False(t, relogged.Expiry().Valid(), + "issue #3371: tagged node must have key-expiry disabled after reusable-key relogin") +} + +// TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry pins the deepest root cause +// (a) in isolation: `tailscale logout` on a tagged node must not stamp an +// expiry at all. This is the assertion PR #3372 does not make — it leaves +// handleLogout expiring tagged nodes and only unwinds the damage on the next +// registration. Keeping this separate from the relogin tests means a +// regression that re-introduces logout-sets-expiry is caught even if the +// re-registration cleanup masks it. +func TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-logout-noexpiry") + tags := []string{"tag:tag1"} + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags) + 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: "noexpiry-tagged"}, + } + _, err = app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + + logoutReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Expiry: tsLogoutSentinelExpiry(), + } + _, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public()) + require.NoError(t, err) + + nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + assert.True(t, nodeAfterLogout.IsTagged(), "node stays tagged through logout") + assert.False(t, nodeAfterLogout.Expiry().Valid(), + "issue #3371 root cause (a): logout must not set an expiry on a tagged node") + assert.False(t, nodeAfterLogout.IsExpired(), + "issue #3371 root cause (a): tagged node must not be expired by logout") + + // The database column must be NULL, not a clamped 'now' timestamp — a + // persisted expiry survives restart and re-triggers the lockout. + var dbNode types.Node + require.NoError(t, + app.state.DB().DB.First(&dbNode, nodeAfterLogout.ID().Uint64()).Error) + assert.Nil(t, dbNode.Expiry, + "issue #3371 root cause (a): tagged node's DB expiry must remain NULL after logout") +} + +// TestIssue3371_UserOwnedNodeLogoutStillExpires is the guard rail: the fix for +// tagged nodes must not change logout for ordinary user-owned nodes. A +// user-owned node that logs out MUST still be expired (that is what logout +// means for it). +func TestIssue3371_UserOwnedNodeLogoutStillExpires(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("user-logout") + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, 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: "user-node"}, + Expiry: time.Now().Add(24 * time.Hour), + } + _, err = app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.False(t, node.IsTagged(), "precondition: user-owned node") + + logoutReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Expiry: tsLogoutSentinelExpiry(), + } + logoutResp, err := app.handleRegister(context.Background(), logoutReq, machineKey.Public()) + require.NoError(t, err) + require.NotNil(t, logoutResp) + + nodeAfterLogout, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + assert.True(t, nodeAfterLogout.IsExpired(), + "user-owned node must still be expired by logout (fix must not regress this)") + assert.True(t, logoutResp.NodeKeyExpired, + "logout response for a user-owned node must report the key expired") +} + +// TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin is the discriminator +// guard rail for the fix. A tagged node may carry a DELIBERATE future expiry +// set by an admin (`headscale nodes expire`); TestTaggedNodeCanHaveKeyExpiry +// establishes that is legal. The #3371 fix clears only a STALE PAST expiry (the +// logout stamp) on re-registration — it must NOT wipe a future expiry. This +// test locks that boundary: without care, a "tagged => clear expiry" fix would +// silently destroy the admin's setting. +// +// Passes before the fix (re-registration currently never touches a tagged +// node's expiry) and must keep passing after. +func TestIssue3371_TaggedNodeFutureExpirySurvivesRelogin(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-future-expiry") + tags := []string{"tag:tag1"} + + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags) + 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: "future-expiry-tagged"}, + } + _, err = app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, node.IsTagged()) + + // Admin sets a deliberate future expiry (`headscale nodes expire`). + future := time.Now().Add(24 * time.Hour) + _, _, err = app.state.SetNodeExpiry(node.ID(), &future) + require.NoError(t, err) + + withFuture, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, withFuture.Expiry().Valid(), "precondition: future expiry set") + require.False(t, withFuture.IsExpired(), "precondition: future expiry is not expired") + + // Node re-registers (rotating its node key). The future expiry must survive. + nodeKey2 := key.NewNode() + reregReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey2.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "future-expiry-tagged"}, + } + _, err = app.handleRegister(context.Background(), reregReq, machineKey.Public()) + require.NoError(t, err) + + after, found := app.state.GetNodeByNodeKey(nodeKey2.Public()) + require.True(t, found) + require.True(t, after.IsTagged(), "node stays tagged") + assert.True(t, after.Expiry().Valid(), + "deliberate future expiry must survive re-registration (not cleared by #3371 fix)") + assert.WithinDuration(t, future, after.Expiry().Get(), 5*time.Second, + "the surviving expiry must be the admin-set future value, unchanged") +} + +// TestIssue3371_EphemeralTaggedNodeLogoutDeletes is a regression guard for the +// ephemeral+tagged combination. A tagged pre-auth key can also be ephemeral. +// handleLogout deletes ephemeral nodes (before any expiry stamp), so the #3371 +// fix (which suppresses the expiry stamp for tagged nodes) must not divert an +// ephemeral tagged node away from deletion. +// +// Passes before the fix and must keep passing after. +func TestIssue3371_EphemeralTaggedNodeLogoutDeletes(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-ephemeral") + tags := []string{"tag:tag1"} + + // Ephemeral + tagged key. + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, true, nil, tags) + 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: "ephemeral-tagged"}, + } + _, err = app.handleRegister(context.Background(), regReq, machineKey.Public()) + require.NoError(t, err) + + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, node.IsTagged(), "precondition: node is tagged") + require.True(t, node.IsEphemeral(), "precondition: node is ephemeral") + + // Logout: an ephemeral node is deleted, not expired. + logoutReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Expiry: tsLogoutSentinelExpiry(), + } + _, err = app.handleRegister(context.Background(), logoutReq, machineKey.Public()) + require.NoError(t, err) + + require.EventuallyWithT(t, func(c *assert.CollectT) { + _, stillThere := app.state.GetNodeByNodeKey(nodeKey.Public()) + assert.False(c, stillThere, + "ephemeral tagged node must be deleted on logout, not expired") + }, 2*time.Second, 50*time.Millisecond, "waiting for ephemeral node deletion") +} diff --git a/hscontrol/db/db.go b/hscontrol/db/db.go index c9b94ef6..a2c76faf 100644 --- a/hscontrol/db/db.go +++ b/hscontrol/db/db.go @@ -900,6 +900,33 @@ WHERE user_id IS NULL }, Rollback: func(db *gorm.DB) error { return nil }, }, + { + // Clear stale key expiry on tagged nodes. A tagged node is + // owned by its tags and never expires (KB 1068), but a buggy + // handleLogout stamped a past expiry on it, leaving it + // permanently Expired and unable to re-authenticate. The + // buggy writer is fixed, so this only repairs rows written + // before the upgrade; a fixed server cannot recreate them. + // Match the tagged-node predicate the earlier + // clear-tagged-node-user-id migration uses (a nil tags slice + // marshals to 'null', so exclude it). + // Fixes: https://github.com/juanfont/headscale/issues/3371 + ID: "202607241200-clear-tagged-node-expiry", + Migrate: func(tx *gorm.DB) error { + err := tx.Exec(` +UPDATE nodes +SET expiry = NULL +WHERE tags IS NOT NULL AND tags != '[]' AND tags != '' AND tags != 'null' + AND expiry IS NOT NULL; + `).Error + if err != nil { + return fmt.Errorf("clearing expiry on tagged nodes: %w", err) + } + + return nil + }, + Rollback: func(db *gorm.DB) error { return nil }, + }, }, ) diff --git a/hscontrol/db/db_test.go b/hscontrol/db/db_test.go index 88d282c4..7d654061 100644 --- a/hscontrol/db/db_test.go +++ b/hscontrol/db/db_test.go @@ -295,6 +295,64 @@ func TestSQLiteMigrationAndDataValidation(t *testing.T) { assert.Equal(t, uint(1), *node4.UserID, "node4 should still belong to user1") }, }, + // Test for the clear-tagged-node-expiry migration + // (202607241200-clear-tagged-node-expiry). A buggy handleLogout stamped + // a key expiry on tagged nodes, which never expire (KB 1068), leaving + // them permanently Expired. The migration clears expiry on tagged rows + // only, preserving user-owned nodes' expiry. + // Fixes: https://github.com/juanfont/headscale/issues/3371 + { + dbPath: "testdata/sqlite/clear_tagged_node_expiry_migration_test.sql", + wantFunc: func(t *testing.T, hsdb *HSDatabase) { + t.Helper() + + nodes, err := Read(hsdb.DB, func(rx *gorm.DB) (types.Nodes, error) { + return ListNodes(rx) + }) + require.NoError(t, err) + require.Len(t, nodes, 5, "should have all 5 nodes") + + byHostname := make(map[string]*types.Node, len(nodes)) + for _, n := range nodes { + byHostname[n.Hostname] = n + } + + // Node 1: tagged with a stale PAST expiry (the bug). Cleared. + node1 := byHostname["node1"] + require.NotNil(t, node1, "node1 should exist") + assert.True(t, node1.IsTagged(), "node1 should be tagged") + assert.Nil(t, node1.Expiry, "node1 (tagged) stale expiry should be cleared") + assert.False(t, node1.IsExpired(), "node1 must not be reported expired") + + // Node 2: tagged with a FUTURE expiry. Tagged nodes never expire, + // so this is cleared too. + node2 := byHostname["node2"] + require.NotNil(t, node2, "node2 should exist") + assert.True(t, node2.IsTagged(), "node2 should be tagged") + assert.Nil(t, node2.Expiry, "node2 (tagged) expiry should be cleared") + + // Node 3: tagged, expiry already NULL. Stays NULL. + node3 := byHostname["node3"] + require.NotNil(t, node3, "node3 should exist") + assert.True(t, node3.IsTagged(), "node3 should be tagged") + assert.Nil(t, node3.Expiry, "node3 (tagged) NULL expiry should be preserved") + + // Node 4: untagged (tags='null') with a PAST expiry. PRESERVED — + // the migration must not touch user-owned nodes. + node4 := byHostname["node4"] + require.NotNil(t, node4, "node4 should exist") + assert.False(t, node4.IsTagged(), "node4 (tags='null') should be untagged") + require.NotNil(t, node4.Expiry, "node4 (user-owned) expiry must be preserved") + assert.Equal(t, 2020, node4.Expiry.UTC().Year(), "node4 past expiry preserved") + + // Node 5: untagged (tags='[]') with a FUTURE expiry. PRESERVED. + node5 := byHostname["node5"] + require.NotNil(t, node5, "node5 should exist") + assert.False(t, node5.IsTagged(), "node5 (tags='[]') should be untagged") + require.NotNil(t, node5.Expiry, "node5 (user-owned) expiry must be preserved") + assert.Equal(t, 2099, node5.Expiry.UTC().Year(), "node5 future expiry preserved") + }, + }, } for _, tt := range tests { diff --git a/hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql b/hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql new file mode 100644 index 00000000..22ac1eed --- /dev/null +++ b/hscontrol/db/testdata/sqlite/clear_tagged_node_expiry_migration_test.sql @@ -0,0 +1,85 @@ +-- Test SQL dump for the clear-tagged-node-expiry migration +-- (202607241200-clear-tagged-node-expiry) +-- +-- A buggy handleLogout stamped a past key expiry on tagged nodes. Tagged +-- nodes are owned by their tags and never expire (KB 1068), so such a row is +-- reported as permanently Expired and can never re-authenticate. The migration +-- clears expiry on tagged rows while leaving user-owned nodes untouched. +-- Fixes: https://github.com/juanfont/headscale/issues/3371 + +PRAGMA foreign_keys=OFF; +BEGIN TRANSACTION; + +-- Migrations table: entries applied up to (but not including) the fix. The +-- intervening expiry migrations (clear-zero-time) also run against this dump; +-- their predicates (expiry < 1900) do not match the post-2000 dates below, so +-- they leave these rows for the new migration to handle. +CREATE TABLE `migrations` (`id` text,PRIMARY KEY (`id`)); +INSERT INTO migrations VALUES('202312101416'); +INSERT INTO migrations VALUES('202312101430'); +INSERT INTO migrations VALUES('202402151347'); +INSERT INTO migrations VALUES('2024041121742'); +INSERT INTO migrations VALUES('202406021630'); +INSERT INTO migrations VALUES('202409271400'); +INSERT INTO migrations VALUES('202407191627'); +INSERT INTO migrations VALUES('202408181235'); +INSERT INTO migrations VALUES('202501221827'); +INSERT INTO migrations VALUES('202501311657'); +INSERT INTO migrations VALUES('202502070949'); +INSERT INTO migrations VALUES('202502131714'); +INSERT INTO migrations VALUES('202502171819'); +INSERT INTO migrations VALUES('202505091439'); +INSERT INTO migrations VALUES('202505141324'); +INSERT INTO migrations VALUES('202507021200'); +INSERT INTO migrations VALUES('202510311551'); +INSERT INTO migrations VALUES('202511101554-drop-old-idx'); +INSERT INTO migrations VALUES('202511011637-preauthkey-bcrypt'); +INSERT INTO migrations VALUES('202511122344-remove-newline-index'); +INSERT INTO migrations VALUES('202511131445-node-forced-tags-to-tags'); +INSERT INTO migrations VALUES('202601121700-migrate-hostinfo-request-tags'); +INSERT INTO migrations VALUES('202602201200-clear-tagged-node-user-id'); + +-- Users table +CREATE TABLE `users` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`name` text,`display_name` text,`email` text,`provider_identifier` text,`provider` text,`profile_pic_url` text); +INSERT INTO users VALUES(1,'2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL,'user1','User One','user1@example.com',NULL,NULL,NULL); + +-- Pre-auth keys table +CREATE TABLE `pre_auth_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`user_id` integer,`reusable` numeric,`ephemeral` numeric DEFAULT false,`used` numeric DEFAULT false,`tags` text,`created_at` datetime,`expiration` datetime,`prefix` text,`hash` blob,CONSTRAINT `fk_pre_auth_keys_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE SET NULL); + +-- API keys table +CREATE TABLE `api_keys` (`id` integer PRIMARY KEY AUTOINCREMENT,`prefix` text,`hash` blob,`created_at` datetime,`expiration` datetime,`last_seen` datetime); + +-- Nodes table - current schema (after the tags rename + last_seen/expiry reordering) +CREATE TABLE IF NOT EXISTS "nodes" (`id` integer PRIMARY KEY AUTOINCREMENT,`machine_key` text,`node_key` text,`disco_key` text,`endpoints` text,`host_info` text,`ipv4` text,`ipv6` text,`hostname` text,`given_name` varchar(63),`user_id` integer,`register_method` text,`tags` text,`auth_key_id` integer,`last_seen` datetime,`expiry` datetime,`approved_routes` text,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,CONSTRAINT `fk_nodes_user` FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_nodes_auth_key` FOREIGN KEY (`auth_key_id`) REFERENCES `pre_auth_keys`(`id`)); + +-- Node 1: TAGGED, user_id NULL, stale PAST expiry (the #3371 bug). After migration: expiry NULL. +INSERT INTO nodes VALUES(1,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e01','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605501','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57701','[]','{}','100.64.0.1','fd7a:115c:a1e0::1','node1','node1',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL); + +-- Node 2: TAGGED, user_id NULL, FUTURE expiry. Tagged nodes never expire, so cleared to NULL too. +INSERT INTO nodes VALUES(2,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e02','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605502','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57702','[]','{}','100.64.0.2','fd7a:115c:a1e0::2','node2','node2',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL); + +-- Node 3: TAGGED, user_id NULL, expiry already NULL. After migration: still NULL. +INSERT INTO nodes VALUES(3,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e03','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605503','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57703','[]','{}','100.64.0.3','fd7a:115c:a1e0::3','node3','node3',NULL,'authkey','["tag:foo"]',NULL,'2024-01-01 00:00:00+00:00',NULL,'[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL); + +-- Node 4: UNTAGGED user-owned (tags='null'), PAST expiry. Must be PRESERVED (guard against +-- the #3323-class over-match: a nil tags slice marshals to the literal 'null'). +INSERT INTO nodes VALUES(4,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e04','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605504','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57704','[]','{}','100.64.0.4','fd7a:115c:a1e0::4','node4','node4',1,'cli','null',NULL,'2024-01-01 00:00:00+00:00','2020-06-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL); + +-- Node 5: UNTAGGED user-owned (tags='[]'), FUTURE expiry. Must be PRESERVED. +INSERT INTO nodes VALUES(5,'mkey:a0ab77456320823945ae0331823e3c0d516fae9585bd42698dfa1ac3d7679e05','nodekey:7c84167ab68f494942de14deb83587fd841843de2bac105b6c670048c1605505','discokey:53075b3c6cad3b62a2a29caea61beeb93f66b8c75cb89dac465236a5bbf57705','[]','{}','100.64.0.5','fd7a:115c:a1e0::5','node5','node5',1,'cli','[]',NULL,'2024-01-01 00:00:00+00:00','2099-01-01 00:00:00+00:00','[]','2024-01-01 00:00:00+00:00','2024-01-01 00:00:00+00:00',NULL); + +-- Policies table (empty) +CREATE TABLE `policies` (`id` integer PRIMARY KEY AUTOINCREMENT,`created_at` datetime,`updated_at` datetime,`deleted_at` datetime,`data` text); + +DELETE FROM sqlite_sequence; +INSERT INTO sqlite_sequence VALUES('users',1); +INSERT INTO sqlite_sequence VALUES('nodes',5); +CREATE INDEX idx_users_deleted_at ON users(deleted_at); +CREATE UNIQUE INDEX idx_api_keys_prefix ON api_keys(prefix); +CREATE INDEX idx_policies_deleted_at ON policies(deleted_at); +CREATE UNIQUE INDEX idx_provider_identifier ON users(provider_identifier) WHERE provider_identifier IS NOT NULL; +CREATE UNIQUE INDEX idx_name_provider_identifier ON users(name, provider_identifier); +CREATE UNIQUE INDEX idx_name_no_provider_identifier ON users(name) WHERE provider_identifier IS NULL; +CREATE UNIQUE INDEX IF NOT EXISTS idx_pre_auth_keys_prefix ON pre_auth_keys(prefix) WHERE prefix IS NOT NULL AND prefix != ''; + +COMMIT; diff --git a/hscontrol/state/auth_tagged_expiry_test.go b/hscontrol/state/auth_tagged_expiry_test.go index 4eef8fee..51e1a6dd 100644 --- a/hscontrol/state/auth_tagged_expiry_test.go +++ b/hscontrol/state/auth_tagged_expiry_test.go @@ -408,6 +408,151 @@ func TestAuthPathRejectsTaggedAndUserCoexistence(t *testing.T) { require.ErrorIs(t, err, ErrAmbiguousNodeOwnership) } +// TestIssue3371_TaggedNodeInteractiveReloginAfterLogout reproduces the +// interactive/OIDC arm of https://github.com/juanfont/headscale/issues/3371 +// ("With no key (interactive): the register URL is printed and the login never +// completes"). +// +// A logout stamps a stale PAST expiry on a tagged node. When the node +// re-authenticates through the auth path (HandleNodeFromAuthPath -> +// applyAuthNodeUpdate), the tagged->tagged branch keeps the existing expiry +// ("Tagged → Tagged: keep existing expiry (nil) - no action needed", +// state.go). That comment assumes the existing expiry is nil; after a logout it +// is a past timestamp, so the node stays expired. The fix must clear a stale +// past expiry on a node that remains tagged (scoped to IsExpired(), so a +// deliberate future expiry is preserved). +func TestIssue3371_TaggedNodeInteractiveReloginAfterLogout(t *testing.T) { + dbPath := t.TempDir() + "/headscale.db" + cfg := persistTestConfig(dbPath) + + database, err := db.NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + user := database.CreateUserForTest("interactive-user") + node := database.CreateRegisteredNodeForTest(user, "interactive-tagged") + machineKey := node.MachineKey + nodeID := node.ID + discoKey := node.DiscoKey + + require.NoError(t, database.Close()) + + s, err := NewState(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = s.Close() }) + + // Make the node tagged with a stale PAST expiry (the state a `tailscale + // logout` leaves behind). Leaving UserID nil but retaining User routes the + // re-auth through the convert-from-tag branch and lets the tag + // re-advertisement be permitted (mirrors TestTaggedReauthKeepsNilExpiry, + // which seeds Expiry=nil; here the only change is a past expiry). + past := time.Now().Add(-1 * time.Hour) + seeded, ok := s.nodeStore.UpdateNode(nodeID, func(n *types.Node) { + n.Tags = []string{"tag:foo"} + n.UserID = nil + n.User = user + n.Expiry = &past + }) + require.True(t, ok) + require.True(t, seeded.IsTagged(), "precondition: node is tagged") + require.True(t, seeded.IsExpired(), "precondition: logout left the tagged node expired") + + policy := fmt.Sprintf(`{"tagOwners":{"tag:foo":["%s@"]}}`, user.Name) + _, err = s.SetPolicy([]byte(policy)) + require.NoError(t, err) + require.True(t, s.NodeCanHaveTag(seeded, "tag:foo"), + "precondition: tagged node is permitted to re-advertise tag:foo") + + // Interactive/OIDC relogin: the client re-advertises the same tag (rotating + // its node key, as a real client does). The node must come back not-expired. + regData := &types.RegistrationData{ + MachineKey: machineKey, + NodeKey: key.NewNode().Public(), + DiscoKey: discoKey, + Hostname: "interactive-tagged", + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "interactive-tagged", + RequestTags: []string{"tag:foo"}, + }, + } + authID := types.MustAuthID() + s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(regData)) + + relogged, _, err := s.HandleNodeFromAuthPath( + authID, types.UserID(user.ID), nil, util.RegisterMethodOIDC, + ) + require.NoError(t, err) + require.True(t, relogged.Valid()) + + require.True(t, relogged.IsTagged(), "node stays tagged after interactive relogin") + require.False(t, relogged.IsExpired(), + "issue #3371: interactive relogin must clear the stale logout expiry") + require.Nil(t, relogged.AsStruct().Expiry, + "issue #3371: tagged node must have key-expiry disabled after interactive relogin") +} + +// TestIssue3371_TaggedNodePastExpirySelfHealsOnReregister covers the 0.29.x +// upgrade path: a tagged node broken by an OLDER headscale carries a past +// expiry persisted in its DB row. After a restart (State reloads the row) it +// comes back expired, and its next auth-key re-registration must self-heal it +// by clearing the stale past expiry. This is the "part b" defensive clear. +func TestIssue3371_TaggedNodePastExpirySelfHealsOnReregister(t *testing.T) { + dbPath := t.TempDir() + "/headscale.db" + cfg := persistTestConfig(dbPath) + + s, err := NewState(cfg) + require.NoError(t, err) + + _, 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) + + machineKey := key.NewMachine() + nodeKey := key.NewNode() + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "broken-tagged"}, + } + node, _, err := s.HandleNodeFromPreAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + + nodeID := node.ID() + + // A prior (buggy) version persisted a past expiry on this tagged node. + past := time.Now().Add(-1 * time.Hour) + err = s.DB().NodeSetExpiry(nodeID, &past) + require.NoError(t, err) + + // Restart: reload State from the same database file. + 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) + require.True(t, reloaded.IsExpired(), + "precondition: a persisted past expiry survives restart and re-triggers the lockout") + + // Re-register (rotating the node key). The stale past expiry must be cleared. + reregReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{AuthKey: pak.Key}, + NodeKey: key.NewNode().Public(), + Hostinfo: &tailcfg.Hostinfo{Hostname: "broken-tagged"}, + } + healed, _, err := s2.HandleNodeFromPreAuthKey(reregReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, healed.IsTagged(), "node stays tagged") + require.False(t, healed.IsExpired(), + "issue #3371: re-registration must self-heal a tagged node broken by an older version") + require.Nil(t, healed.AsStruct().Expiry, + "issue #3371: self-healed tagged node must have key-expiry disabled (DB NULL)") + require.Equal(t, nodeID, healed.ID(), "must be the same node") +} + // 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`). diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index ddb410eb..e8784551 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -1801,8 +1801,14 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView } else { node.Expiry = regData.Expiry } + case isTagged && node.IsExpired(): + // Tagged → Tagged, but carrying a stale PAST expiry from an older + // headscale's logout stamp (#3371). Tagged nodes never expire, so + // clear it; a deliberate future expiry has IsExpired() == false and + // falls through to the no-op below. + node.Expiry = nil } - // Tagged → Tagged: keep existing expiry (nil) - no action needed + // Tagged → Tagged with no stale expiry: keep existing expiry - no action. // Apply default node expiry for non-tagged nodes when the // resolved expiry is still nil or zero (e.g., CLI registration @@ -2421,7 +2427,14 @@ func (s *State) HandleNodeFromPreAuthKey( // 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. + // + // Tagged nodes are excluded: they never expire (KB 1068), so an + // IsExpired() tagged node only reflects a stale logout stamp left by an + // older headscale (#3371). Forcing it down the re-validation path burns its + // fresh key and blocks re-auth forever; treat it as a plain re-registration + // and clear the stale expiry in the update below. isExpired := existsSameUser && existingNodeSameUser.Valid() && + !existingNodeSameUser.IsTagged() && existingNodeSameUser.IsExpired() // A tagged key presented for a currently user-owned node converts that node @@ -2557,6 +2570,13 @@ func (s *State) HandleNodeFromPreAuthKey( } else { node.Expiry = nil } + } else if node.IsExpired() { + // #3371: a tagged node must never carry key expiry. Clear a + // stale PAST expiry left by a logout (older headscale) so + // re-auth is not permanently blocked. A deliberate future + // expiry (headscale nodes expire) has IsExpired() == false and + // is left untouched. + node.Expiry = nil } }) diff --git a/integration/tags_test.go b/integration/tags_test.go index 27cf1c73..81b5154b 100644 --- a/integration/tags_test.go +++ b/integration/tags_test.go @@ -3203,3 +3203,202 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(t *testing.T) { } }, integrationutil.HAConvergeTimeout, 1*time.Second, "node should be user-owned after conversion via CLI register") } + +// TestTaggedNodeLogoutReloginSingleUseKeyOnline reproduces issue #3371 +// end-to-end with a real tailscale client: a tagged node registered with a +// single-use key logs out (`tailscale logout`) and re-authenticates with a +// FRESH single-use tagged key. Tagged nodes never expire (KB 1068), so logout +// must not stamp an expiry; before the fix the node was left permanently +// expired and the fresh key was consumed on a re-registration that still +// reported NodeKeyExpired, locking the node out forever. +// +// The observable proof at the integration level is that after relogin the node +// is back online with a NULL expiry and the same node ID — not stuck expired. +// +// https://github.com/juanfont/headscale/issues/3371 +func TestTaggedNodeLogoutReloginSingleUseKeyOnline(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-logout-single"), + ) + requireNoErrHeadscaleEnv(t, err) + + headscale, err := scenario.Headscale() + requireNoErrGetHeadscale(t, err) + + userMap, err := headscale.MapUsers() + require.NoError(t, err) + + userID := mustParseID(userMap[tagTestUser].Id) + + // KEY1: single-use tag:valid-owned. Initial join. + 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.Key) + 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 = mustParseID(nodes[0].Id) + assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"}) + assert.Nil(c, nodes[0].Expiry, "tagged node must have no expiry") + } + }, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration") + + // `tailscale logout`. A tagged node must not be expired by this. + err = client.Logout() + require.NoError(t, err) + + err = client.WaitForNeedsLogin(integrationutil.ScaledTimeout(60 * time.Second)) + require.NoError(t, err) + + // The node must remain in the DB, tagged, and crucially NOT carry a + // stale expiry. This is the #3371 root cause (a) surface. + assert.EventuallyWithT(t, func(c *assert.CollectT) { + nodes, err := headscale.ListNodes() + assert.NoError(c, err) + assert.Len(c, nodes, 1, "node must persist through logout") + + if len(nodes) == 1 { + assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"}) + assert.Nil(c, nodes[0].Expiry, "#3371: logout must not stamp expiry on a tagged node") + } + }, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "tagged node must survive logout without expiry") + + // KEY2: a FRESH single-use tagged key. Relogin. + key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"}) + require.NoError(t, err) + + err = client.Login(headscale.GetEndpoint(), key2.Key) + require.NoError(t, err, + "#3371: a fresh key must re-authenticate the tagged node after logout") + + // Back online, same node, still tagged, still no expiry. + 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 { + assert.Equal(c, initialNodeID, mustParseID(nodes[0].Id), "node ID must be unchanged") + assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"}) + assert.Nil(c, nodes[0].Expiry, "#3371: tagged node must have no expiry after relogin") + assert.True(c, nodes[0].Online, "#3371: tagged node must be online after relogin, not stuck expired") + } + }, integrationutil.ScaledTimeout(60*time.Second), integrationutil.SlowPoll, "tagged node must come back online after relogin") + + t.Logf("Test #3371 PASS: tagged node logged out and re-authenticated online with a fresh single-use key") +} + +// TestTaggedNodeLogoutReloginReusableKeyOnline is the reusable-key variant of +// issue #3371 (the "tailscale up hangs indefinitely" report). With a reusable +// key the relogin does not hit "authkey already used", but before the fix the +// node still stayed expired, so the client never observed a non-expired node. +// The observable proof is the same: online with NULL expiry after relogin. +// +// https://github.com/juanfont/headscale/issues/3371 +func TestTaggedNodeLogoutReloginReusableKeyOnline(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-logout-reuse"), + ) + requireNoErrHeadscaleEnv(t, err) + + headscale, err := scenario.Headscale() + requireNoErrGetHeadscale(t, err) + + userMap, err := headscale.MapUsers() + require.NoError(t, err) + + userID := mustParseID(userMap[tagTestUser].Id) + + // A single REUSABLE tag:valid-owned key used for both login and relogin. + key, err := scenario.CreatePreAuthKeyWithTags(userID, true, 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(), key.Key) + 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 = mustParseID(nodes[0].Id) + assert.Nil(c, nodes[0].Expiry, "tagged node must have no expiry") + } + }, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration") + + err = client.Logout() + require.NoError(t, err) + + err = client.WaitForNeedsLogin(integrationutil.ScaledTimeout(60 * time.Second)) + require.NoError(t, err) + + // Relogin with the SAME reusable key. + err = client.Login(headscale.GetEndpoint(), key.Key) + require.NoError(t, err) + + 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 { + assert.Equal(c, initialNodeID, mustParseID(nodes[0].Id), "node ID must be unchanged") + assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"}) + assert.Nil(c, nodes[0].Expiry, "#3371: tagged node must have no expiry after reusable-key relogin") + assert.True(c, nodes[0].Online, "#3371: tagged node must be online after reusable-key relogin") + } + }, integrationutil.ScaledTimeout(60*time.Second), integrationutil.SlowPoll, "tagged node must come back online after reusable-key relogin") + + t.Logf("Test #3371 PASS: tagged node logged out and re-authenticated online with a reusable key") +}