auth, state: preserve client expiry and require expired node key rotation

Return NodeKeyExpired for same-key login so the client rotates its key;
the machine stays authorized. Store earlier client expiry and expire
tagged nodes at logout.

hscontrol/auth.go

@@ -203,9 +203,11 @@ func (h *Headscale) handleLogout(
 			Bool("unexpected", true).
 			Msg("Node key expired, forcing re-authentication")
 
+		// The machine stays authorised, as on Tailscale SaaS; only its node
+		// key needs replacing.
 		return &tailcfg.RegisterResponse{
 			NodeKeyExpired:    true,
-			MachineAuthorized: false,
+			MachineAuthorized: true,
 			AuthURL:           "", // Client will need to re-authenticate
 		}, nil
 	}
@@ -216,9 +218,22 @@ func (h *Headscale) handleLogout(
 	// that the node is not attempting to tamper/extend their expiry.
 	// If it is not, we will expire the node or in the case of an ephemeral node, delete it.
 
-	// The client is trying to extend their key, this is not allowed.
+	// A client may bring its expiry forward, as `tailscale debug set-expire`
+	// does and Tailscale SaaS stores, which turns expiry on for a node without
+	// one. It may not push the expiry back.
 	if req.Expiry.After(time.Now()) {
-		return nil, NewHTTPError(http.StatusBadRequest, "extending key is not allowed", nil)
+		updatedNode, c, err := h.state.BringNodeExpiryForward(node.ID(), req.NodeKey, req.Expiry)
+		h.Change(c)
+
+		if errors.Is(err, state.ErrExpiryExtension) {
+			return nil, NewHTTPError(http.StatusBadRequest, "extending key is not allowed", nil)
+		}
+
+		if err != nil {
+			return nil, fmt.Errorf("setting node expiry: %w", err)
+		}
+
+		return nodeToRegisterResponse(updatedNode), nil
 	}
 
 	// If the request expiry is in the past, we consider it a logout.
@@ -254,22 +269,9 @@ 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.
+	// logged out. A tagged node is expired too, as on Tailscale SaaS; a valid
+	// key under a new node key brings it back (#3371).
 	//
 	// Clamp the client-supplied value to now: Tailscale sends the
 	// sentinel time.Unix(123, 0) on logout (controlclient/direct.go),
@@ -476,6 +478,10 @@ func (h *Headscale) handleRegisterWithAuthKey(
 	if err != nil {
 		h.Change(changed)
 
+		if errors.Is(err, state.ErrNodeKeyExpired) {
+			return &tailcfg.RegisterResponse{NodeKeyExpired: true, MachineAuthorized: true}, nil
+		}
+
 		if errors.Is(err, gorm.ErrRecordNotFound) {
 			return nil, NewHTTPError(http.StatusUnauthorized, "invalid pre auth key", nil)
 		}

hscontrol/auth_compat_test.go

@@ -30,12 +30,7 @@ import (
 // pendingLoginSkipReasons lists checks where headscale differs from SaaS,
 // keyed <test_id>/<step>/<node>/<check>. A listed check that matches fails,
 // so a fix must delete its entry.
-var pendingLoginSkipReasons = map[string]string{
-	"pending-login/PL-E2-expire-restart/pl-laptop/exchange-0": expiredRestartUnauthorized,
-	"pending-login/PL-E3-restart/pl-laptop/exchange-1":        expiredRestartUnauthorized,
-}
-
-const expiredRestartUnauthorized = "EXPIRED_RESTART_UNAUTHORIZED: headscale answers an expired node's restart with MachineAuthorized false; SaaS answers true, and both say the node key expired"
+var pendingLoginSkipReasons = map[string]string{}
 
 // pendingLoginNotCompared lists, by key prefix, checks left out: SaaS's
 // answer there is in doubt, so it is no reference.

hscontrol/auth_tags_test.go

@@ -1353,11 +1353,10 @@ func tsLogoutSentinelExpiry() time.Time {
 // 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.
+// `tailscale logout` sends a past expiry, which expires the tagged node as on
+// Tailscale SaaS. The relogin must bring it back.
 //
-// Root cause (b): on the next `tailscale up --auth-key <fresh key>`,
+// Root cause: on the next `tailscale up --auth-key <fresh 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
@@ -1411,14 +1410,9 @@ func TestIssue3371_TaggedNodeLogoutReloginSingleUseKey(t *testing.T) {
 	_, 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")
+	assert.True(t, nodeAfterLogout.IsExpired(), "logout expires a tagged node")
 
 	// `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).
@@ -1518,14 +1512,9 @@ func TestIssue3371_TaggedNodeLogoutReloginReusableKey(t *testing.T) {
 		"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) {
+// TestTaggedNodeLogoutExpiresIt: `tailscale logout` expires a tagged node, as
+// on Tailscale SaaS, and the expiry reaches the database.
+func TestTaggedNodeLogoutExpiresIt(t *testing.T) {
 	app := createTestApp(t)
 
 	user := app.state.CreateUserForTest("tag-logout-noexpiry")
@@ -1556,18 +1545,51 @@ func TestIssue3371_TaggedNodeLogoutDoesNotSetExpiry(t *testing.T) {
 	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")
+	assert.True(t, nodeAfterLogout.IsExpired(), "logout expires a tagged node")
 
-	// 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")
+	require.NotNil(t, dbNode.Expiry)
+	assert.True(t, dbNode.IsExpired(), "the logout expiry is stored")
+}
+
+// TestClientBringsExpiryForward: a client may ask for an earlier expiry, which
+// turns expiry on for a node without one, but may not push it back.
+func TestClientBringsExpiryForward(t *testing.T) {
+	app := createTestApp(t)
+
+	pak, err := app.state.CreatePreAuthKey(nil, true, false, nil, []string{"tag:tag1"})
+	require.NoError(t, err)
+
+	machineKey := key.NewMachine()
+	nodeKey := key.NewNode()
+
+	_, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  nodeKey.Public(),
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "set-expire"},
+	}, machineKey.Public())
+	require.NoError(t, err)
+
+	soon := time.Now().Add(time.Hour).Truncate(time.Second)
+	resp, err := app.handleRegister(context.Background(), tailcfg.RegisterRequest{
+		NodeKey: nodeKey.Public(),
+		Expiry:  soon,
+	}, machineKey.Public())
+	require.NoError(t, err)
+	assert.False(t, resp.NodeKeyExpired)
+
+	node, found := app.state.GetNodeByNodeKey(nodeKey.Public())
+	require.True(t, found)
+	require.True(t, node.Expiry().Valid(), "the earlier expiry turns expiry on")
+	assert.True(t, soon.Equal(node.Expiry().Get()))
+
+	_, err = app.handleRegister(context.Background(), tailcfg.RegisterRequest{
+		NodeKey: nodeKey.Public(),
+		Expiry:  soon.Add(time.Hour),
+	}, machineKey.Public())
+	require.ErrorContains(t, err, "extending key is not allowed")
 }
 
 // TestIssue3371_UserOwnedNodeLogoutStillExpires is the guard rail: the fix for

hscontrol/auth_test.go

@@ -605,10 +605,11 @@ func TestAuthenticationFlows(t *testing.T) {
 				}
 			},
 			machineKey:  machineKey1.Public,
+			wantAuth:    true,
 			wantExpired: true,
 			validate: func(t *testing.T, resp *tailcfg.RegisterResponse, app *Headscale) { //nolint:thelper //nolint:thelper
 				assert.True(t, resp.NodeKeyExpired)
-				assert.False(t, resp.MachineAuthorized)
+				assert.True(t, resp.MachineAuthorized, "the machine stays authorised; only its node key expired")
 			},
 		},
 		// TEST: Ephemeral node is deleted on logout

hscontrol/servertest/expiry_compat_test.go

@@ -42,16 +42,11 @@ var expirySkipReasons = map[string]string{
 	"expiry/C4-expire-r2/r2/paths-r1":        expiredPeerPaths,
 	"expiry/C4-expire-r2/r2/self":            selfExpiredFlags,
 
-	"expiry/D2-set-expire/subject/key-expiry": clientExpiryRefused,
-	"expiry/D2-set-expire/subject/outcome":    clientExpiryRefused,
-
-	"expiry/D3-past-expiry/subject/expired-observer": clientExpiryRefused,
-	"expiry/D3-past-expiry/subject/key-expiry":       clientExpiryRefused,
-	"expiry/D3-past-expiry/subject/paths-observer":   clientExpiryRefused,
-	"expiry/D3-past-expiry/subject/self":             clientExpiryRefused,
-
-	"expiry/D5-logout-tagged/tagged/key-expiry": taggedLogoutNotExpired,
-	"expiry/D5-logout-tagged/tagged/outcome":    taggedLogoutNotExpired,
+	// A client asked for this expiry; once it passes the node is expired
+	// like any other.
+	"expiry/D3-past-expiry/subject/online-observer": expiredPeerOffline,
+	"expiry/D3-past-expiry/subject/paths-observer":  expiredPeerPaths,
+	"expiry/D3-past-expiry/subject/self":            selfExpiredFlags,
 
 	"expiry/A2-expire/laptop/online-observer": expiredPeerOffline,
 	"expiry/A2-expire/laptop/online-peer1":    expiredPeerOffline,
@@ -111,7 +106,6 @@ var expirySkipReasons = map[string]string{
 const (
 	selfExpiredFlags      = "SELF_EXPIRED_FLAGS: headscale sends an expired node its own entry with MachineAuthorized false and Expired set; SaaS keeps MachineAuthorized true and leaves Expired out"
 	expiredPeerPaths      = "EXPIRED_PEER_PATHS: headscale sends peers an expired node's endpoints and home DERP; SaaS leaves both out"
-	clientExpiryRefused   = "CLIENT_EXPIRY_REFUSED: headscale refuses a client's request for an earlier key expiry; SaaS stores it, and the node expires then"
 	nameserversFromConfig = "NAMESERVERS_FROM_CONFIG: headscale reads nameservers from its config and cannot change them while running; SaaS pushes a change to every node, an expired one too"
 )
 
@@ -153,6 +147,27 @@ func runNodeExpiryCompat(t *testing.T, id, observer string) {
 	r := newStepReplay(t, c, opts...)
 	r.paths = true
 
+	// The test server runs no scheduled tasks. Run headscale's expiry pass on
+	// a short timer, so peers learn when an expiry a client asked for passes.
+	go func() {
+		tick := time.NewTicker(250 * time.Millisecond)
+		defer tick.Stop()
+
+		last := time.Unix(0, 0)
+
+		for {
+			select {
+			case <-t.Context().Done():
+				return
+			case <-tick.C:
+				next, changes, _ := r.srv.State().ExpireExpiredNodes(last)
+				last = next
+
+				r.srv.App.Change(changes...)
+			}
+		}
+	}()
+
 	// A change is judged against the step before only, so both sides
 	// compare the same span.
 	prevCaps := c.Captures

hscontrol/servertest/lifecycle_compat_test.go

@@ -56,10 +56,6 @@ var lifecycleSkipReasons = map[string]string{
 	"ephemeral-lifecycle/collection/perm-to-eph/left":             ephemeralFollowsKey,
 	"ephemeral-lifecycle/collection/perm-to-eph/removed-observer": ephemeralFollowsKey,
 	"ephemeral-lifecycle/collection/perm-to-eph/present-observer": ephemeralFollowsKey,
-
-	"ephemeral-lifecycle/E5-mem-shutdown/mem/expiry":        taggedLogoutNotExpired,
-	"ephemeral-lifecycle/E5-mem-shutdown/mem/outcome":       taggedLogoutNotExpired,
-	"ephemeral-lifecycle/E5-mem-shutdown/mem/peer-observer": taggedLogoutNotExpired,
 }
 
 // SaaS runs auto-approval when a router's announced routes change and on a

hscontrol/servertest/reauth_compat_test.go

@@ -39,8 +39,6 @@ import (
 var reauthSkipReasons = map[string]string{
 	"owner-routes/r-expired-retag/expire/online": expiredPeerOffline,
 
-	"owner-routes/r-expired-retag/owner/node-key": expiredKeyAccepted,
-
 	"owner-routes/r-admin/owner/enabled":     adminRetagNoApproval,
 	"owner-routes/r-admin/owner/served":      adminRetagNoApproval,
 	"owner-routes/r-admin/owner/self-routes": adminRetagNoApproval,
@@ -49,19 +47,6 @@ var reauthSkipReasons = map[string]string{
 	"owner-routes/r-expired-retag/withdraw/enabled": withdrawKeepsAutoApproval,
 	"owner-routes/u-convert/withdraw/enabled":       withdrawKeepsAutoApproval,
 
-	"reauth/t-spent-relogin/t-spent/outcome":            expiredKeyNotRotated,
-	"reauth/t-revoked-relogin/t-revoked/outcome":        expiredKeyNotRotated,
-	"reauth/t-valid-relogin/t-valid/outcome":            expiredKeyNotRotated,
-	"reauth/t-valid-relogin/t-valid/node-key":           expiredKeyAccepted,
-	"reauth-user/a4-expired-spent-key/u-spent/outcome":  expiredKeyNotRotated,
-	"reauth-user/a5-expired-valid-key/u-valid/outcome":  expiredKeyNotRotated,
-	"reauth-user/a5-expired-valid-key/u-valid/node-key": expiredKeyAccepted,
-
-	"reauth-foreign/t-logout-logout/t-logout/expiry":    taggedLogoutNotExpired,
-	"reauth-foreign/t-logout-logout/t-logout/peer-obs":  taggedLogoutNotExpired,
-	"reauth-foreign/t-logout-relogin/t-logout/expiry":   taggedLogoutNotExpired,
-	"reauth-foreign/t-logout-relogin/t-logout/peer-obs": taggedLogoutNotExpired,
-
 	"identity/w-tagreq-relogin-tags/w-tagreq/owner":        keyLoginTagsRefused,
 	"identity/w-tagreq-relogin-tags/w-tagreq/outcome":      keyLoginTagsRefused,
 	"identity/w-tagreq-relogin-tags/w-tagreq/node-key":     keyLoginTagsRefused,
@@ -101,14 +86,11 @@ var reauthSkipReasons = map[string]string{
 }
 
 const (
-	expiredKeyAccepted        = "EXPIRED_KEY_ACCEPTED: headscale re-registers an expired node under its old node key; SaaS answers NodeKeyExpired, so the client rotates it"
 	adminRetagNoApproval      = "ADMIN_RETAG_NO_APPROVAL: an admin tag change does not auto-approve the routes the new tags may approve; SaaS approves them at once"
 	withdrawKeepsAutoApproval = "WITHDRAW_KEEPS_AUTO_APPROVAL: headscale keeps an auto-approved route once it is withdrawn; SaaS drops it and keeps only admin-set approvals"
 )
 
 const (
-	expiredKeyNotRotated     = "EXPIRED_KEY_NOT_ROTATED: headscale answers an expired node's key login without asking for a new node key; SaaS answers NodeKeyExpired first, so the client rotates it"
-	taggedLogoutNotExpired   = "TAGGED_LOGOUT_NOT_EXPIRED: headscale leaves a tagged node unexpired when it logs out; SaaS expires it"
 	keyLoginTagsRefused      = "KEY_LOGIN_TAGS_REFUSED: headscale refuses a user's key login that requests tags the user owns; SaaS tags the device, converting it in place or creating it tagged"
 	taggedKeyOwnedTagRefused = "TAGGED_KEY_OWNED_TAG_REFUSED: headscale refuses a tagged key's login that requests a tag the key's tags own; SaaS gives the device the requested tags"
 	refusalText              = "REFUSAL_TEXT: SaaS refuses a key by naming its ID; headscale says why the key is refused"

hscontrol/state/auth_tagged_expiry_test.go

@@ -235,13 +235,11 @@ func TestExpiredUserNodeReusedOneShotKey_RotatedNodeKey(t *testing.T) {
 
 // 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.
+// node key is asked for a new one, so a spent one-shot key never 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")
+	require.ErrorIs(t, err, ErrNodeKeyExpired)
 }
 
 // TestReusableUserPAKReauthOnTaggedNodeNoDuplicate guards against a reusable
@@ -1098,7 +1096,13 @@ func TestExpiredTaggedNodeRequiresValidPreAuthKey(t *testing.T) {
 			require.Equal(t, past.UnixNano(), nsBefore.Expiry)
 			require.Equal(t, nsBefore, rowBefore)
 
-			node, _, err := s.HandleNodeFromPreAuthKey(c.regReq, c.machineKey)
+			_, _, err = s.HandleNodeFromPreAuthKey(c.regReq, c.machineKey)
+			require.ErrorIs(t, err, ErrNodeKeyExpired, "the old node key is answered first")
+
+			req := c.regReq
+			req.NodeKey = key.NewNode().Public()
+
+			node, _, err := s.HandleNodeFromPreAuthKey(req, c.machineKey)
 			if !reusable {
 				require.ErrorIs(t, err, types.PAKError("authkey already used"))
 				nsAfter, rowAfter := c.fields(t)
@@ -1937,7 +1941,7 @@ func TestPAKReregisterRevalidatesAtMutation(t *testing.T) {
 		nsBefore, dbBefore := c.fields(t)
 
 		_, err = c.reregister(view, pak, c.regReq)
-		require.ErrorIs(t, err, types.PAKError("authkey already used"))
+		require.ErrorIs(t, err, ErrNodeKeyExpired)
 
 		nsAfter, dbAfter := c.fields(t)
 		require.Equal(t, nsBefore, nsAfter, "NodeStore node must be untouched")
@@ -1965,13 +1969,14 @@ func TestPAKReregisterRevalidatesAtMutation(t *testing.T) {
 		_, _, err = s.SetNodeExpiry(c.node.ID(), &tA)
 		require.NoError(t, err)
 
-		got, err := c.reregister(view, pak, c.regReq)
-		require.NoError(t, err, "a valid key re-authorises whatever raced")
-		require.False(t, got.IsExpired())
+		nsBefore, dbBefore := c.fields(t)
 
-		ns, dbNode := c.fields(t)
-		require.Equal(t, c.regReq.Expiry.UnixNano(), ns.Expiry, "expiry is extended")
-		require.Equal(t, ns, dbNode, "database must equal the NodeStore")
+		_, err = c.reregister(view, pak, c.regReq)
+		require.ErrorIs(t, err, ErrNodeKeyExpired, "even a valid key needs a new node key")
+
+		nsAfter, dbAfter := c.fields(t)
+		require.Equal(t, nsBefore, nsAfter)
+		require.Equal(t, dbBefore, dbAfter)
 	})
 
 	t.Run("nodekey_changed_spent_key", func(t *testing.T) {
@@ -2110,9 +2115,9 @@ func TestPAKReregisterRevalidatesAtMutation(t *testing.T) {
 	})
 
 	// The node is expired after the lookup, then the key expires, then the
-	// writer runs. The decision must be taken at the writer's own clock: any
-	// validity or clock read taken before the NodeStore write lets the expired
-	// key revive the node.
+	// writer runs a login under a new node key. The decision must be taken at
+	// the writer's own clock: any validity or clock read taken before the
+	// NodeStore write lets the expired key revive the node.
 	t.Run("admin_expire_then_key_expires", func(t *testing.T) {
 		s := newRetagTestState(t)
 		user := s.CreateUserForTest("pakuser")
@@ -2130,6 +2135,7 @@ func TestPAKReregisterRevalidatesAtMutation(t *testing.T) {
 
 		regReq := c.regReq
 		regReq.Auth = &tailcfg.RegisterResponseAuth{AuthKey: k.Key}
+		regReq.NodeKey = key.NewNode().Public()
 
 		view, pak := c.lookup(t, k.Key)
 		require.NoError(t, pak.ValidAt(time.Now()))
@@ -2213,11 +2219,12 @@ func TestPAKReregisterRollbackKeepsPriorWrite(t *testing.T) {
 
 	regReq := c.regReq
 	regReq.Auth = &tailcfg.RegisterResponseAuth{AuthKey: k.Key}
+	regReq.NodeKey = key.NewNode().Public()
 
 	view, pak := c.lookup(t, k.Key)
 	require.False(t, pak.Used, "precondition: the lookup read a fresh single-use key")
 
-	tA := time.Now()
+	tA := time.Now().Add(time.Hour)
 	_, _, err = s.SetNodeExpiry(c.node.ID(), &tA)
 	require.NoError(t, err)
 
@@ -2235,6 +2242,7 @@ func TestPAKReregisterRollbackKeepsPriorWrite(t *testing.T) {
 	require.Equal(t, tA.UnixNano(), ns.Expiry, "rollback must keep the admin expiry")
 	require.Equal(t, &k0.ID, ns.AuthKeyID)
 	require.Equal(t, "h0", ns.Hostname)
+	require.Equal(t, c.regReq.NodeKey, ns.NodeKey)
 }
 
 // TestPAKReregisterConcurrentAdminExpire races a same-key restart with a spent
@@ -3042,8 +3050,8 @@ func TestPAKWakeAcceptsRequestedTags(t *testing.T) {
 
 // TestUserKeyOnTaggedMachine: a user key on a tagged machine re-authenticates
 // the tagged node only when it speaks for it (the user an admin tagged it
-// from); another user's key wakes it, gets a device of its own beside it with
-// a new node key, and cannot lift its expiry.
+// from); another user's key wakes it or gets a device of its own beside it
+// with a new node key. An expired node is asked for a new node key first.
 func TestUserKeyOnTaggedMachine(t *testing.T) {
 	for _, adminTagged := range []bool{false, true} {
 		for _, mode := range []string{"expired", "rotated", "wake"} {
@@ -3099,8 +3107,8 @@ func TestUserKeyOnTaggedMachine(t *testing.T) {
 				require.NoError(t, keyErr)
 
 				switch {
-				case !adminTagged && mode == "expired":
-					require.EqualError(t, err, "authkey cannot re-authenticate this node")
+				case mode == "expired":
+					require.ErrorIs(t, err, ErrNodeKeyExpired)
 					assert.Equal(t, beforeCache, afterCache)
 					assert.Equal(t, beforeDB, afterDB)
 					assert.False(t, storedKey.Used)
@@ -3133,13 +3141,8 @@ func TestUserKeyOnTaggedMachine(t *testing.T) {
 					require.NoError(t, err)
 					assert.Equal(t, []string{"tag:prod"}, afterCache.Tags, "a same-key relogin keeps it tagged")
 					assert.Equal(t, presented.ID, *afterCache.AuthKeyID)
+					assert.Equal(t, beforeCache.Expiry, afterCache.Expiry)
 					assert.Equal(t, 1, s.ListNodes().Len())
-
-					if mode == "expired" {
-						assert.Zero(t, afterCache.Expiry, "a valid key lifts the past expiry")
-					} else {
-						assert.Equal(t, beforeCache.Expiry, afterCache.Expiry)
-					}
 				}
 			})
 		}
@@ -3147,7 +3150,8 @@ func TestUserKeyOnTaggedMachine(t *testing.T) {
 }
 
 // TestPAKWakeRechecksAuthorityAtMutation: another user's key that wakes a
-// live tagged node is refused when an expiry lands before the writer runs.
+// live tagged node is asked for a new node key when an expiry lands before
+// the writer runs.
 func TestPAKWakeRechecksAuthorityAtMutation(t *testing.T) {
 	s := newRetagTestState(t)
 	original, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:prod"})
@@ -3166,9 +3170,55 @@ func TestPAKWakeRechecksAuthorityAtMutation(t *testing.T) {
 
 	beforeCache, beforeDB := c.fields(t)
 	_, err = c.reregister(view, pak, c.regReq)
-	require.EqualError(t, err, "authkey cannot re-authenticate this node")
+	require.ErrorIs(t, err, ErrNodeKeyExpired)
 
 	afterCache, afterDB := c.fields(t)
 	assert.Equal(t, beforeCache, afterCache)
 	assert.Equal(t, beforeDB, afterDB)
 }
+
+// TestExpiredNodeSameKeyLoginAsksForNewKey: an expired node that logs in
+// under its old node key is asked for a new one and changes nothing, even
+// with a valid key; the login with the new key then renews it.
+func TestExpiredNodeSameKeyLoginAsksForNewKey(t *testing.T) {
+	for _, tagged := range []bool{false, true} {
+		t.Run(fmt.Sprintf("tagged=%t", tagged), func(t *testing.T) {
+			s := newRetagTestState(t)
+
+			var (
+				owner *types.UserID
+				tags  []string
+			)
+
+			if tagged {
+				tags = []string{"tag:prod"}
+			} else {
+				owner = s.CreateUserForTest("expired-user").TypedID()
+			}
+
+			pak, err := s.CreatePreAuthKey(owner, true, false, nil, tags)
+			require.NoError(t, err)
+
+			c := registerForPAKRereg(t, s, pak)
+			past := time.Now().Add(-time.Hour)
+			_, _, err = s.SetNodeExpiry(c.node.ID(), &past)
+			require.NoError(t, err)
+
+			beforeCache, beforeDB := c.fields(t)
+
+			_, _, err = s.HandleNodeFromPreAuthKey(c.regReq, c.machineKey)
+			require.ErrorIs(t, err, ErrNodeKeyExpired)
+
+			afterCache, afterDB := c.fields(t)
+			assert.Equal(t, beforeCache, afterCache)
+			assert.Equal(t, beforeDB, afterDB)
+
+			req := c.regReq
+			req.NodeKey = key.NewNode().Public()
+			node, _, err := s.HandleNodeFromPreAuthKey(req, c.machineKey)
+			require.NoError(t, err)
+			assert.Equal(t, c.node.ID(), node.ID())
+			assert.False(t, node.IsExpired())
+		})
+	}
+}

hscontrol/state/persist_test.go

@@ -1896,6 +1896,49 @@ func reopenLabels(t *testing.T, s *State) map[types.NodeID]string {
 	return out
 }
 
+// TestBringNodeExpiryForwardDecidesInWriter: a client's earlier expiry is
+// checked against the node as the writer holds it, so an admin expiry that
+// lands after the caller read the node is not pushed back, and a node key
+// the node no longer holds changes nothing.
+func TestBringNodeExpiryForwardDecidesInWriter(t *testing.T) {
+	_, s, nodeID := persistTestSetup(t)
+	t.Cleanup(func() { _ = s.Close() })
+
+	read, ok := s.GetNodeByID(nodeID)
+	require.True(t, ok)
+
+	_, _, err := s.SetNodeExpiry(nodeID, nil)
+	require.NoError(t, err)
+
+	soon := time.Now().Add(time.Hour).Truncate(time.Second)
+	node, _, err := s.BringNodeExpiryForward(nodeID, read.NodeKey(), soon)
+	require.NoError(t, err)
+	require.True(t, node.Expiry().Valid(), "an earlier expiry turns expiry on")
+	assert.True(t, soon.Equal(node.Expiry().Get()))
+
+	_, _, err = s.BringNodeExpiryForward(nodeID, read.NodeKey(), soon.Add(time.Minute))
+	require.ErrorIs(t, err, ErrExpiryExtension, "a later expiry")
+
+	_, _, err = s.BringNodeExpiryForward(nodeID, key.NewNode().Public(), soon.Add(-time.Minute))
+	require.ErrorIs(t, err, ErrExpiryExtension, "another node key")
+
+	past := time.Now().Add(-time.Minute).Truncate(time.Second)
+	_, _, err = s.SetNodeExpiry(nodeID, &past)
+	require.NoError(t, err)
+
+	_, _, err = s.BringNodeExpiryForward(nodeID, read.NodeKey(), soon.Add(-time.Minute))
+	require.ErrorIs(t, err, ErrExpiryExtension, "the admin expiry is earlier")
+
+	current, ok := s.GetNodeByID(nodeID)
+	require.True(t, ok)
+	assert.True(t, current.IsExpired(), "the admin expiry stands")
+
+	stored, err := s.db.GetNodeByID(nodeID)
+	require.NoError(t, err)
+	require.NotNil(t, stored.Expiry)
+	assert.True(t, past.Equal(*stored.Expiry))
+}
+
 // TestSetNodeExpiryRejectsUnencodableExpiry proves an admin cannot store a key
 // expiry that every map carrying the node would fail to encode.
 func TestSetNodeExpiryRejectsUnencodableExpiry(t *testing.T) {

hscontrol/state/state.go

@@ -140,11 +140,19 @@ var ErrAuthCacheClosed = errors.New("authentication cache closed")
 // ErrAuthRequestFinished prevents reusing an already resolved verdict channel.
 var ErrAuthRequestFinished = errors.New("authentication request already finished")
 
+// ErrNodeKeyExpired is returned when an expired node logs in again under its
+// old node key; the client answers it by rotating the key.
+var ErrNodeKeyExpired = errors.New("node key expired")
+
 // ErrNodeKeyInUse is returned when a registration or re-auth claims a NodeKey
 // already bound to a different machine, enforcing the 1:1 NodeKey<->MachineKey
 // binding.
 var ErrNodeKeyInUse = errors.New("node key already in use by another machine")
 
+// ErrExpiryExtension is returned when a client asks for an expiry no earlier
+// than its node's, or under a node key the node no longer holds.
+var ErrExpiryExtension = errors.New("extending key is not allowed")
+
 // ErrNodeKeyNotCurrent is returned when a map request uses a replaced node key.
 var ErrNodeKeyNotCurrent = errors.New("node key is no longer current")
 
@@ -1079,6 +1087,31 @@ func (s *State) ListEphemeralNodes() views.Slice[types.NodeView] {
 // SetNodeExpiry updates the expiration time for a node.
 // If expiry is nil, the node's expiry is disabled (node will never expire).
 func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.NodeView, change.Change, error) {
+	return s.setNodeExpiry(nodeID, expiry, nil)
+}
+
+// BringNodeExpiryForward stores the expiry a client asks for when the node
+// still holds nodeKey and has no expiry or a later one. The NodeStore writer
+// decides, so an expiry that lands meanwhile, such as an admin's, is never
+// pushed back.
+func (s *State) BringNodeExpiryForward(
+	nodeID types.NodeID,
+	nodeKey key.NodePublic,
+	expiry time.Time,
+) (types.NodeView, change.Change, error) {
+	return s.setNodeExpiry(nodeID, &expiry, func(node *types.Node) bool {
+		return node.NodeKey == nodeKey &&
+			(node.Expiry == nil || node.Expiry.IsZero() || expiry.Before(*node.Expiry))
+	})
+}
+
+// setNodeExpiry sets the node's expiry. A non-nil allow decides on the node
+// as the writer holds it; a refusal writes nothing.
+func (s *State) setNodeExpiry(
+	nodeID types.NodeID,
+	expiry *time.Time,
+	allow func(*types.Node) bool,
+) (types.NodeView, change.Change, error) {
 	if expiry != nil {
 		utc, err := types.ValidateExpiry(*expiry)
 		if err != nil {
@@ -1088,7 +1121,7 @@ func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.Nod
 		expiry = &utc
 	}
 
-	var onlineChanged, expired bool
+	var onlineChanged, expired, refused bool
 
 	genBefore := s.polMan.NodesGeneration()
 
@@ -1097,6 +1130,12 @@ func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.Nod
 	// must make the exact same change. If the database update fails, the [NodeStore]
 	// change will remain, and the change describing it is returned with the error.
 	n, ok := s.nodeStore.UpdateNode(nodeID, func(node *types.Node) {
+		if allow != nil && !allow(node) {
+			refused = true
+
+			return
+		}
+
 		wasOnline := node.Online()
 		node.Expiry = expiry
 		// Control stays connected in NeedsLogin so an expiry extension can
@@ -1111,6 +1150,10 @@ func (s *State) SetNodeExpiry(nodeID types.NodeID, expiry *time.Time) (types.Nod
 		return types.NodeView{}, change.Change{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, nodeID)
 	}
 
+	if refused {
+		return types.NodeView{}, change.Change{}, ErrExpiryExtension
+	}
+
 	if expired {
 		// Expiry revokes the human check even if a pre-auth key renews the node.
 		s.sshCheckMu.Lock()
@@ -3012,9 +3055,8 @@ func loginNode(all map[types.UserID]types.NodeView, userID types.UserID) (types.
 // --authkey=KEY") re-presents a spent key legitimately. Anything that
 // authorises something new must present a valid key:
 //   - a node key rotation;
-//   - an expired node. Tagged nodes can carry an administrative expiry or
-//     retain their expiry after an admin tags them. Legacy logout stamps
-//     were cleared by migration; logout no longer expires tagged nodes;
+//   - an expired node. Tagged nodes expire at logout, by an administrative
+//     expiry, or keep the expiry they had when an admin tagged them;
 //   - a tagged key converting a user-owned node to tagged;
 //   - a tagged key other than the one the node last authed with, which retags
 //     it.
@@ -3040,6 +3082,12 @@ func pakSkipsValidation(n types.NodeView, pak *types.PreAuthKey, nodeKey key.Nod
 // pakReregister distinguishes re-authentication with a valid key from a
 // wake-up that only refreshes a live node's reported metadata.
 func pakReregister(n types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePublic, now time.Time) (bool, error) {
+	// As on Tailscale SaaS, an expired node is asked for a new node key before
+	// any key is judged.
+	if n.NodeKey() == nodeKey && n.IsExpiredAt(now) {
+		return false, ErrNodeKeyExpired
+	}
+
 	err := pak.ValidAt(now)
 	if err == nil {
 		if pak.CanReauthenticate(n) {
@@ -3122,7 +3170,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 	}
 
 	// A wake-up writes no tags, so the tags it asks for are not refused.
-	if tagsErr != nil && (reauth || err != nil) {
+	if tagsErr != nil && !errors.Is(err, ErrNodeKeyExpired) && (reauth || err != nil) {
 		return types.NodeView{}, s.policyChangeSince(genBefore), tagsErr
 	}
 
@@ -3400,9 +3448,9 @@ func (s *State) reregisterNodeWithPAK(
 			if node.IsExpiredAt(now) {
 				node.Expiry = nil
 			}
-		case prior.NodeKey == regReq.NodeKey && !prior.IsExpiredAt(now):
-			// A live node that keeps its node key keeps its expiry, as on
-			// Tailscale SaaS: renewal comes with a new node key.
+		case prior.NodeKey == regReq.NodeKey:
+			// A node that keeps its node key keeps its expiry, as on Tailscale
+			// SaaS: renewal comes with a new node key.
 		case !regReq.Expiry.IsZero():
 			node.Expiry = &regReq.Expiry
 		case s.cfg.Node.Expiry > 0:

integration/tags_test.go

@@ -3419,10 +3419,9 @@ func TestTagsAuthKeyWithoutUserCLIRegisterAddsUserDevice(t *testing.T) {
 // 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.
+// FRESH single-use tagged key. Logout expires the tagged node, as on Tailscale
+// SaaS; before the fix 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.
@@ -3483,15 +3482,13 @@ func TestTaggedNodeLogoutReloginSingleUseKeyOnline(t *testing.T) {
 		}
 	}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "waiting for initial registration")
 
-	// `tailscale logout`. A tagged node must not be expired by this.
+	// `tailscale logout` expires the tagged node.
 	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)
@@ -3499,9 +3496,9 @@ func TestTaggedNodeLogoutReloginSingleUseKeyOnline(t *testing.T) {
 
 		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")
+			assert.NotNil(c, nodes[0].Expiry, "logout expires a tagged node")
 		}
-	}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "tagged node must survive logout without expiry")
+	}, integrationutil.StatusReadyTimeout, integrationutil.SlowPoll, "tagged node must survive logout, expired")
 
 	// KEY2: a FRESH single-use tagged key. Relogin.
 	key2, err := scenario.CreatePreAuthKeyWithTags(userID, false, false, []string{"tag:valid-owned"})