auth: store a client's earlier expiry and expire a tagged node at logout

A client may bring its expiry forward, which turns expiry on for a node
without one, but still cannot push it back. Logout expires tagged nodes
too.

hscontrol/auth.go

@@ -198,9 +198,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.
@@ -236,22 +249,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),

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/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

@@ -47,11 +47,6 @@ var reauthSkipReasons = map[string]string{
 	"owner-routes/r-expired-retag/withdraw/enabled": withdrawKeepsAutoApproval,
 	"owner-routes/u-convert/withdraw/enabled":       withdrawKeepsAutoApproval,
 
-	"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,
@@ -96,7 +91,6 @@ const (
 )
 
 const (
-	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/persist_test.go

@@ -1801,3 +1801,46 @@ 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))
+}

hscontrol/state/state.go

@@ -135,6 +135,10 @@ var ErrNodeKeyExpired = errors.New("node key expired")
 // 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")
 
@@ -978,7 +982,32 @@ 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) {
-	var onlineChanged, expired bool
+	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) {
+	var onlineChanged, expired, refused bool
 
 	genBefore := s.polMan.NodesGeneration()
 
@@ -987,6 +1016,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
@@ -1001,6 +1036,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()
@@ -2847,9 +2886,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.

integration/tags_test.go

@@ -3415,10 +3415,9 @@ func TestTagsAuthKeyConvertToUserViaCLIRegister(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.
@@ -3479,15 +3478,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)
@@ -3495,9 +3492,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"})