state: send a relogin as a whole node unless only its keys changed

A PeerChange patch cannot clear a peer's Expired flag or carry the
Hostinfo the relogin stored.

Fixes #3531

.github/workflows/test-integration.yaml

@@ -259,6 +259,7 @@ jobs:
           - TestOIDCReloginSameUserRoutesPreserved
           - TestAuthWebFlowAuthenticationPingAll
           - TestAuthWebFlowLogoutAndReloginSameUser
+          - TestAuthWebFlowReloginExpiredNode
           - TestAuthWebFlowLogoutAndReloginNewUser
           - TestApiKeyCommand
           - TestApiKeyCommandValidation

CHANGELOG.md

@@ -152,6 +152,7 @@ clients, and how to run the same setup without Nix.
 - Fix policy `tests` and `sshTests` failing for a group that names an unknown user [#3516](https://github.com/juanfont/headscale/pull/3516)
 - Fix an unknown user in `nodeAttrs` rejecting the policy [#3516](https://github.com/juanfont/headscale/pull/3516)
 - Fix an ephemeral node being deleted while online, when it reconnected while its previous session was being marked offline [#3539](https://github.com/juanfont/headscale/pull/3539)
+- Fix peers dropping traffic from a node that re-authenticates after its key expired [#3541](https://github.com/juanfont/headscale/pull/3541)
 
 - Fix an exit node or subnet router not seeing its own approved routes until it reconnected, so `tailscale status` did not show it offering an exit node [#3518](https://github.com/juanfont/headscale/pull/3518)
 

hscontrol/auth_test.go

@@ -4328,6 +4328,73 @@ func TestHandleNodeFromAuthPath_OldUserNil_NoPanic(t *testing.T) {
 	assert.Equal(t, userB.ID, node.UserID().Get(), "new node belongs to userB")
 }
 
+// TestHandleNodeFromAuthPath_ReloginWholeNode covers an interactive (web or
+// OIDC) relogin that rotates the node key. A [tailcfg.PeerChange] patch can
+// neither clear a peer's Expired flag nor carry Hostinfo, so a relogin that
+// un-expires the node or changes Hostinfo peers read must be sent as a whole
+// node. A relogin that only rotates keys keeps the key-rotation patch.
+func TestHandleNodeFromAuthPath_ReloginWholeNode(t *testing.T) {
+	tests := []struct {
+		name      string
+		expired   bool
+		services  []tailcfg.Service
+		wantPatch bool
+	}{
+		{name: "keys only", wantPatch: true},
+		{name: "expired", expired: true},
+		{
+			name:     "peer-visible hostinfo",
+			services: []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 4242}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			app := createTestApp(t)
+
+			user := app.state.CreateUserForTest("authpath-relogin")
+			node := app.state.CreateRegisteredNodeForTest(user, "authpath-relogin")
+			node.Hostinfo = &tailcfg.Hostinfo{Hostname: node.Hostname}
+
+			if tt.expired {
+				node.Expiry = new(time.Now().Add(-time.Minute))
+			}
+
+			app.state.PutNodeInStoreForTest(*node)
+
+			newNodeKey := key.NewNode()
+			authID := types.MustAuthID()
+			app.state.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{
+				MachineKey: node.MachineKey,
+				NodeKey:    newNodeKey.Public(),
+				Hostname:   node.Hostname,
+				Hostinfo: &tailcfg.Hostinfo{
+					Hostname: node.Hostname,
+					Services: tt.services,
+				},
+			}))
+
+			relogged, c, err := app.state.HandleNodeFromAuthPath(
+				authID,
+				types.UserID(user.ID),
+				nil,
+				"oidc",
+			)
+			require.NoError(t, err)
+			require.Equal(t, node.ID, relogged.ID(), "relogin must update the existing node")
+			require.Equal(t, newNodeKey.Public(), relogged.NodeKey())
+
+			if tt.wantPatch {
+				assert.Empty(t, c.PeersChanged, "relogin must not be a whole-node add")
+				assert.Len(t, c.PeerPatches, 1, "relogin must be a peer patch")
+			} else {
+				assert.Empty(t, c.PeerPatches, "relogin must not be a peer patch")
+				assert.Contains(t, c.PeersChanged, node.ID, "relogin must be a whole-node add")
+			}
+		})
+	}
+}
+
 // TestWaitForFollowupMachineKeyMismatch covers the followup poll in
 // [Headscale.waitForFollowup]. That poll is authenticated only by the auth ID
 // embedded in the followup URL, so without a machine-key check anyone who

hscontrol/servertest/lifecycle_test.go

@@ -14,6 +14,8 @@ import (
 	"github.com/juanfont/headscale/hscontrol/types/change"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"tailscale.com/control/controlclient"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/netmap"
 )
 
@@ -270,6 +272,95 @@ func TestRestoredExpirySurvivesQueuedChanges(t *testing.T) {
 	}
 }
 
+// TestReloginOfExpiredNodeClearsPeerExpiry covers #3531. Peers hold an expired
+// node with Expired=true, which only a whole node from control can clear:
+// [tailcfg.PeerChange] has no Expired field. Hostinfo is identical across the
+// relogin, so no Hostinfo-driven whole-node update can mask a missing one.
+func TestReloginOfExpiredNodeClearsPeerExpiry(t *testing.T) {
+	t.Parallel()
+
+	h := servertest.NewHarness(t, 2,
+		servertest.WithServerOptions(servertest.WithBatchDelay(10*time.Millisecond)),
+	)
+	client, observer := h.Client(0), h.Client(1)
+	id := findNodeID(t, h.Server, client.Name)
+	node, ok := h.Server.State().GetNodeByID(id)
+	require.True(t, ok)
+
+	oldKey := node.NodeKey()
+
+	expiry := time.Now()
+	_, c, err := h.Server.State().SetNodeExpiry(id, &expiry)
+	require.NoError(t, err)
+	h.Server.App.Change(c)
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		peer, found := observer.PeerByName(client.Name)
+		if assert.True(c, found) {
+			assert.True(c, peer.Expired())
+		}
+	}, 5*time.Second, 10*time.Millisecond, "observer must see the node expired")
+
+	// The server answers the expired key with NodeKeyExpired, so the client
+	// generates a new key, the way tailscaled re-authenticates.
+	client.Reconnect(t)
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		current, found := h.Server.State().GetNodeByID(id)
+		if !assert.True(c, found) {
+			return
+		}
+
+		assert.NotEqual(c, oldKey, current.NodeKey())
+		assert.False(c, current.IsExpired())
+
+		peer, found := observer.PeerByName(client.Name)
+		if assert.True(c, found) {
+			assert.Equal(c, current.NodeKey(), peer.Key())
+			assert.False(c, peer.Expired(), "relogin must clear the peer's expired flag")
+		}
+	}, 5*time.Second, 10*time.Millisecond, "observer must see the relogged node with its new key and not expired")
+}
+
+// TestReloginPeerVisibleHostinfoReachesPeers covers the rest of the #3531
+// class: a relogin stores the RegisterRequest's Hostinfo, so the following
+// MapRequest carries no Hostinfo delta, and a peer-visible change made at
+// relogin reaches peers only if the relogin itself sends the whole node.
+func TestReloginPeerVisibleHostinfoReachesPeers(t *testing.T) {
+	t.Parallel()
+
+	h := servertest.NewHarness(t, 2,
+		servertest.WithServerOptions(servertest.WithBatchDelay(10*time.Millisecond)),
+	)
+	client, observer := h.Client(0), h.Client(1)
+
+	svc := tailcfg.Service{Proto: tailcfg.PeerAPI4, Port: 4242}
+	// Same Hostinfo as servertest.NewClient, plus a service.
+	hi := &tailcfg.Hostinfo{
+		BackendLogID: "servertest-" + client.Name,
+		Hostname:     client.Name,
+		Services:     []tailcfg.Service{svc},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	// `tailscale up --force-reauth`: an interactive login rotates the key.
+	client.Disconnect(t)
+	client.Direct().SetHostinfo(hi)
+	_, err := client.Direct().TryLogin(ctx, controlclient.LoginInteractive)
+	require.NoError(t, err)
+	require.NoError(t, client.RestartPoll(ctx))
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		peer, found := observer.PeerByName(client.Name)
+		if assert.True(c, found) {
+			assert.Equal(c, []tailcfg.Service{svc}, peer.Hostinfo().Services().AsSlice(),
+				"peer must see the Hostinfo the node relogged in with")
+		}
+	}, 5*time.Second, 10*time.Millisecond, "observer must see the relogin's peer-visible Hostinfo")
+}
+
 func TestNodeExpiryRouteFailover(t *testing.T) {
 	t.Parallel()
 

hscontrol/state/auth_tagged_expiry_test.go

@@ -1767,8 +1767,9 @@ func (c pakReregCase) reregister(
 	regReq tailcfg.RegisterRequest,
 ) (types.NodeView, error) {
 	hi := regReq.Hostinfo.Clone()
+	node, _, err := c.s.reregisterNodeWithPAK(view, pak, regReq, c.machineKey, hi.Hostname, hi)
 
-	return c.s.reregisterNodeWithPAK(view, pak, regReq, c.machineKey, hi.Hostname, hi)
+	return node, err
 }
 
 // pakNodeFields are the node fields a re-registration writes. Expiry is in

hscontrol/state/persist_test.go

@@ -354,7 +354,7 @@ func TestReauthRejectsNodeKeyClaimedByAnotherMachine(t *testing.T) {
 	require.NoError(t, err)
 
 	// Attacker re-authenticates its own node but supplies the victim's NodeKey.
-	_, err = s.applyAuthNodeUpdate(authNodeUpdateParams{
+	_, _, err = s.applyAuthNodeUpdate(authNodeUpdateParams{
 		ExistingNode: attackerNode,
 		RegData: &types.RegistrationData{
 			MachineKey: attackerMachine.Public(),
@@ -414,7 +414,7 @@ func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) {
 
 	// Node re-authenticates, rotating its NodeKey. The RegisterRequest carries
 	// no endpoints.
-	updated, err := s.applyAuthNodeUpdate(authNodeUpdateParams{
+	updated, _, err := s.applyAuthNodeUpdate(authNodeUpdateParams{
 		ExistingNode: node,
 		RegData: &types.RegistrationData{
 			MachineKey: machine.Public(),
@@ -434,7 +434,7 @@ func TestReauthPreservesEndpointsWhenClientOmitsThem(t *testing.T) {
 		"re-auth without reported endpoints must preserve the node's live endpoints")
 }
 
-// TestReauthChange covers the decision both re-auth paths share: a same-user
+// TestReauthChange covers the decision both re-auth paths share: a keys-only
 // relogin must be an incremental peer patch (so the tailscale client takes its
 // fast patch path), never a whole-node add (which strands a re-keyed,
 // momentarily-endpoint-less peer disco-deaf); a policy change forces a full
@@ -461,6 +461,47 @@ func TestReauthChange(t *testing.T) {
 	assert.False(t, pol.IsEmpty(), "a policy change must be non-empty")
 }
 
+// TestOnlyKeysChanged covers which relogins may ride the key-rotation patch:
+// one that flips Expired or changes Hostinfo peers read must not, as
+// [tailcfg.PeerChange] carries neither (#3531).
+func TestOnlyKeysChanged(t *testing.T) {
+	now := time.Now()
+	past, future := now.Add(-time.Minute), now.Add(time.Hour)
+
+	node := func(expiry time.Time, hi tailcfg.Hostinfo) types.NodeView {
+		n := types.Node{
+			NodeKey:  key.NewNode().Public(),
+			DiscoKey: key.NewDisco().Public(),
+			Expiry:   &expiry,
+			Hostinfo: &hi,
+		}
+
+		return n.View()
+	}
+
+	hi := tailcfg.Hostinfo{Hostname: "node", OS: "linux"}
+	withOSVersion, withService := hi, hi
+	withOSVersion.OSVersion = "6.1"
+	withService.Services = []tailcfg.Service{{Proto: tailcfg.PeerAPI4, Port: 4242}}
+
+	tests := []struct {
+		name          string
+		before, after types.NodeView
+		want          bool
+	}{
+		{name: "keys", before: node(future, hi), after: node(future, hi), want: true},
+		{name: "hostinfo peers do not read", before: node(future, hi), after: node(future, withOSVersion), want: true},
+		{name: "un-expired", before: node(past, hi), after: node(future, hi)},
+		{name: "hostinfo peers read", before: node(future, hi), after: node(future, withService)},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, onlyKeysChanged(tt.before, tt.after, now))
+		})
+	}
+}
+
 // TestPreAuthKeyReauthRejectsNodeKeyClaimedByAnotherMachine is the pre-auth-key
 // analogue of TestReauthRejectsNodeKeyClaimedByAnotherMachine: re-registering
 // via a pre-auth key must enforce the same 1:1 NodeKey<->MachineKey binding the

hscontrol/state/state.go

@@ -1859,8 +1859,10 @@ type authNodeUpdateParams struct {
 
 // applyAuthNodeUpdate applies common update logic for re-authenticating or converting
 // an existing node. It updates the node in [NodeStore], processes RequestTags, and
-// persists changes to the database.
-func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView, error) {
+// persists changes to the database. The bool reports whether peers read the
+// node it replaced and the update the same, apart from its keys; see
+// [onlyKeysChanged].
+func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView, bool, error) {
 	regData := params.RegData
 	// Log the operation type
 	if params.IsConvertFromTag {
@@ -1901,7 +1903,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 
 	rejectedTags := s.validateRequestTagsForReauth(params.ExistingNode, authUser, requestTags)
 	if len(rejectedTags) > 0 {
-		return types.NodeView{}, fmt.Errorf(
+		return types.NodeView{}, false, fmt.Errorf(
 			"%w %v are invalid or not permitted",
 			ErrRequestedTagsInvalidOrNotPermitted,
 			rejectedTags,
@@ -1916,11 +1918,11 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 	// the NodeStore NodeKey index (denying the victim service).
 	if existing, ok := s.nodeStore.GetNodeByNodeKey(regData.NodeKey); ok &&
 		existing.MachineKey() != regData.MachineKey {
-		return types.NodeView{}, ErrNodeKeyInUse
+		return types.NodeView{}, false, ErrNodeKeyInUse
 	}
 
 	// Update existing node in [NodeStore] - validation passed, safe to mutate
-	updatedNodeView, ok := s.nodeStore.UpdateNode(params.ExistingNode.ID(), func(node *types.Node) {
+	before, updatedNodeView, ok := s.nodeStore.UpdateNodeDiff(params.ExistingNode.ID(), func(node *types.Node) {
 		node.NodeKey = regData.NodeKey
 		node.DiscoKey = regData.DiscoKey
 		node.Hostname = params.Hostname
@@ -2011,9 +2013,11 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 	})
 
 	if !ok {
-		return types.NodeView{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, params.ExistingNode.ID())
+		return types.NodeView{}, false, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, params.ExistingNode.ID())
 	}
 
+	keysOnly := onlyKeysChanged(before, updatedNodeView, time.Now())
+
 	// Persist to database.
 	// Explicitly select all node columns so GORM includes nil/zero-value fields
 	// (see nodeUpdateColumns comment).
@@ -2039,7 +2043,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 		return nil, nil //nolint:nilnil // side-effect only write
 	})
 	if err != nil {
-		return types.NodeView{}, err
+		return types.NodeView{}, false, err
 	}
 
 	// Log completion
@@ -2053,7 +2057,7 @@ func (s *State) applyAuthNodeUpdate(params authNodeUpdateParams) (types.NodeView
 			Msg("Node re-authorized")
 	}
 
-	return updatedNodeView, nil
+	return updatedNodeView, keysOnly, nil
 }
 
 // createAndSaveNewNode creates a new node, allocates IPs, saves to DB, and adds to [NodeStore].
@@ -2460,12 +2464,15 @@ func (s *State) HandleNodeFromAuthPath(
 		RegisterMethod: registrationMethod,
 	}
 
-	var finalNode types.NodeView
+	var (
+		finalNode types.NodeView
+		keysOnly  bool
+	)
 
 	if nodeExistsForSameUser {
 		updateParams.ExistingNode = existingNodeSameUser
 
-		finalNode, err = s.applyAuthNodeUpdate(updateParams)
+		finalNode, keysOnly, err = s.applyAuthNodeUpdate(updateParams)
 		if err != nil {
 			return types.NodeView{}, s.policyChangeSince(genBefore), err
 		}
@@ -2473,7 +2480,7 @@ func (s *State) HandleNodeFromAuthPath(
 		updateParams.ExistingNode = taggedNode
 		updateParams.IsConvertFromTag = true
 
-		finalNode, err = s.applyAuthNodeUpdate(updateParams)
+		finalNode, _, err = s.applyAuthNodeUpdate(updateParams)
 		if err != nil {
 			return types.NodeView{}, s.policyChangeSince(genBefore), err
 		}
@@ -2527,10 +2534,10 @@ func (s *State) HandleNodeFromAuthPath(
 
 	policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
 
-	// nodeExistsForSameUser is true only for a same-user relogin; a tag->user
-	// conversion is excluded, as it changes the peer's User — a structural
-	// change peers must see in full, not a key-rotation patch.
-	return finalNode, reauthChange(finalNode, nodeExistsForSameUser, policyChanged), nil
+	// keysOnly is set only for a same-user relogin; a tag->user conversion is
+	// excluded, as it changes the peer's User — a structural change peers must
+	// see in full, not a key-rotation patch.
+	return finalNode, reauthChange(finalNode, keysOnly, policyChanged), nil
 }
 
 // createNewNodeFromAuth creates a new node during auth callback.
@@ -2753,14 +2760,17 @@ func (s *State) HandleNodeFromPreAuthKey(
 		Str(zf.UserName, pak.Username()).
 		Msg("Registering node with pre-auth key")
 
-	var finalNode types.NodeView
+	var (
+		finalNode types.NodeView
+		keysOnly  bool
+	)
 
 	// If this node exists for this user, update the node in place. For a
 	// tags-only key (pak.User == nil) this is true when the machine already has
 	// a tagged node (findExistingNodeForPAK matches it under UserID 0); for a
 	// user-owned key it is true when the same user already has the node.
 	if existsSameUser && existingNodeSameUser.Valid() {
-		finalNode, err = s.reregisterNodeWithPAK(existingNodeSameUser, pak, regReq, machineKey, hostname, validHostinfo)
+		finalNode, keysOnly, err = s.reregisterNodeWithPAK(existingNodeSameUser, pak, regReq, machineKey, hostname, validHostinfo)
 		if err != nil {
 			return types.NodeView{}, s.policyChangeSince(genBefore), err
 		}
@@ -2857,7 +2867,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 
 	policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
 
-	return finalNode, reauthChange(finalNode, existsSameUser, policyChanged), nil
+	return finalNode, reauthChange(finalNode, keysOnly, policyChanged), nil
 }
 
 // reregisterNodeWithPAK updates an existing node in place for a pre-auth key
@@ -2868,7 +2878,8 @@ func (s *State) HandleNodeFromPreAuthKey(
 // in between, and the key or the node can expire. So the writer decides
 // whether the key must be valid on the node it replaces, at its own clock, and
 // leaves the node untouched when it is not. existingNodeSameUser is used only
-// for its ID and for logging.
+// for its ID and for logging. The bool reports whether peers read the node it
+// replaced and the update the same, apart from its keys; see [onlyKeysChanged].
 func (s *State) reregisterNodeWithPAK(
 	existingNodeSameUser types.NodeView,
 	pak *types.PreAuthKey,
@@ -2876,7 +2887,7 @@ func (s *State) reregisterNodeWithPAK(
 	machineKey key.MachinePublic,
 	hostname string,
 	validHostinfo *tailcfg.Hostinfo,
-) (types.NodeView, error) {
+) (types.NodeView, bool, error) {
 	log.Trace().
 		Caller().
 		Str(zf.NodeName, existingNodeSameUser.Hostname()).
@@ -2894,7 +2905,7 @@ func (s *State) reregisterNodeWithPAK(
 	// NodeStore NodeKey index, denying the victim service.
 	if existing, ok := s.nodeStore.GetNodeByNodeKey(regReq.NodeKey); ok &&
 		existing.MachineKey() != machineKey {
-		return types.NodeView{}, ErrNodeKeyInUse
+		return types.NodeView{}, false, ErrNodeKeyInUse
 	}
 
 	// prior is the node the writer replaced: the NetInfo source and the
@@ -2905,7 +2916,7 @@ func (s *State) reregisterNodeWithPAK(
 	)
 
 	// Update existing node - NodeStore first, then database
-	updatedNodeView, ok := s.nodeStore.UpdateNode(existingNodeSameUser.ID(), func(node *types.Node) {
+	before, updatedNodeView, ok := s.nodeStore.UpdateNodeDiff(existingNodeSameUser.ID(), func(node *types.Node) {
 		// One clock read, so the whole mutation is decided at one instant.
 		now := time.Now()
 
@@ -2998,7 +3009,7 @@ func (s *State) reregisterNodeWithPAK(
 	// A nil prior means the writer never saw the node (deleted, or the store
 	// stopped), so nothing was decided and nothing may be persisted.
 	if !ok || prior == nil {
-		return types.NodeView{}, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, existingNodeSameUser.ID())
+		return types.NodeView{}, false, fmt.Errorf("%w: %d", ErrNodeNotInNodeStore, existingNodeSameUser.ID())
 	}
 
 	if authErr != nil {
@@ -3010,7 +3021,7 @@ func (s *State) reregisterNodeWithPAK(
 			Err(authErr).
 			Msg("Re-registration needs a valid auth key when it applies, rejecting")
 
-		return types.NodeView{}, authErr
+		return types.NodeView{}, false, authErr
 	}
 
 	_, err := hsdb.Write(s.db.DB, func(tx *gorm.DB) (*types.Node, error) {
@@ -3058,7 +3069,7 @@ func (s *State) reregisterNodeWithPAK(
 			n.AuthKeyID = prior.AuthKeyID
 		})
 
-		return types.NodeView{}, fmt.Errorf("writing node to database: %w", err)
+		return types.NodeView{}, false, fmt.Errorf("writing node to database: %w", err)
 	}
 
 	log.Trace().
@@ -3070,27 +3081,37 @@ func (s *State) reregisterNodeWithPAK(
 		Str(zf.UserName, pak.Username()).
 		Msg("Node re-authorized")
 
-	return updatedNodeView, nil
+	return updatedNodeView, onlyKeysChanged(before, updatedNodeView, time.Now()), nil
 }
 
 // reauthChange returns the [change.Change] to broadcast after an authentication
-// that updated or created a node.
+// that updated or created a node. keysOnly marks a same-user relogin that
+// changed nothing peers read besides its keys; see [onlyKeysChanged].
 //
-// A pure relogin (isRelogin: an existing node, same user, with only its NodeKey
-// rotated) is sent as a minimal incremental peer patch via [change.NodeKeyRotated]
-// rather than re-advertising the whole node. A policy change forces a full
-// recompute; any other (new) node is a whole-node add.
-func reauthChange(node types.NodeView, isRelogin, policyChanged bool) change.Change {
+// Such a relogin is sent as the minimal peer patch [change.NodeKeyRotated].
+// Anything else is a whole-node add, and a policy change forces a full
+// recompute.
+func reauthChange(node types.NodeView, keysOnly, policyChanged bool) change.Change {
 	switch {
 	case policyChanged:
 		return change.PolicyChange()
-	case isRelogin:
+	case keysOnly:
 		return change.NodeKeyRotated(node)
 	default:
 		return change.NodeAdded(node.ID())
 	}
 }
 
+// onlyKeysChanged reports whether peers read before and after the same, apart
+// from what [change.NodeKeyRotated] patches. [tailcfg.PeerChange] carries
+// neither [tailcfg.Node.Expired] nor Hostinfo: peers left holding an expired
+// node drop its WireGuard handshakes, and the relogin stored the Hostinfo, so
+// no later MapRequest shows the change.
+func onlyKeysChanged(before, after types.NodeView, now time.Time) bool {
+	return before.IsExpiredAt(now) == after.IsExpiredAt(now) &&
+		peerHostinfoEqual(before.Hostinfo(), after.Hostinfo())
+}
+
 // updatePolicyManagerUsers pushes the current user list into the policy
 // manager, rebuilds peer adjacency when user identity changed, and returns
 // a PolicyChange when clients need a refresh.

hscontrol/types/change/change.go

@@ -504,12 +504,17 @@ func EndpointOrDERPUpdate(id types.NodeID, patch *tailcfg.PeerChange) Change {
 // incremental [tailcfg.PeerChange] patch rather than re-advertising the whole
 // node — the smallest update that conveys the rotation, and the least
 // disruptive for peers reconciling it.
+//
+// Use it only when nothing else peers read changed: [tailcfg.PeerChange] has
+// no field for Hostinfo or for [tailcfg.Node.Expired], which only control can
+// clear, and only by sending the whole node.
 func NodeKeyRotated(node types.NodeView) Change {
 	nk := node.NodeKey()
 	dk := node.DiscoKey()
 
-	// KeyExpiry is always set: the zero value clears any prior expiry on the
-	// peer (un-expire), and a non-zero value carries the new expiry.
+	// KeyExpiry is always set: the zero value clears any prior expiry
+	// timestamp on the peer, and a non-zero value carries the new expiry. It
+	// does not clear [tailcfg.Node.Expired].
 	var expiry time.Time
 	if e, ok := node.Expiry().GetOk(); ok {
 		expiry = e

integration/auth_web_flow_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"tailscale.com/types/key"
 )
 
 func TestAuthWebFlowAuthenticationPingAll(t *testing.T) {
@@ -197,6 +198,181 @@ func TestAuthWebFlowLogoutAndReloginSameUser(t *testing.T) {
 	t.Logf("all clients IPs are the same")
 }
 
+// peerAPISettleTimeout bounds the wait for an expired client to serve
+// peerapi again; clients that do take about a second.
+const peerAPISettleTimeout = 10 * time.Second
+
+// TestAuthWebFlowReloginExpiredNode expires one node at a time and logs it
+// back in through the web flow, which rotates its node key. Peers hold the
+// expired node with Expired=true, so the relogin must clear that flag on every
+// peer that stayed connected. Otherwise the peers keep dropping the node's
+// WireGuard handshakes while disco pings still succeed, which is why
+// reachability is checked with TSMP pings rather than disco pings.
+func TestAuthWebFlowReloginExpiredNode(t *testing.T) {
+	IntegrationSkip(t)
+
+	spec := ScenarioSpec{
+		NodesPerUser: len(MustTestVersions),
+		Users:        []string{"user1"},
+	}
+
+	scenario, err := NewScenario(spec)
+
+	require.NoError(t, err)
+	defer scenario.ShutdownAssertNoPanics(t)
+
+	err = scenario.CreateHeadscaleEnvWithLoginURL(
+		nil,
+		hsic.WithTestName("webexpiredrelogin"),
+	)
+	requireNoErrHeadscaleEnv(t, err)
+
+	allClients, err := scenario.ListTailscaleClients()
+	requireNoErrListClients(t, err)
+
+	allIps, err := scenario.ListTailscaleClientsIPs()
+	requireNoErrListClientIPs(t, err)
+
+	err = scenario.WaitForTailscaleSync()
+	requireNoErrSync(t, err)
+
+	allAddrs := lo.Map(allIps, func(x netip.Addr, index int) string {
+		return x.String()
+	})
+
+	assertPingAll(t, allClients, allAddrs)
+
+	headscale, err := scenario.Headscale()
+	requireNoErrGetHeadscale(t, err)
+
+	for _, target := range allClients {
+		t.Run(target.Hostname(), func(t *testing.T) {
+			var (
+				selfID string
+				oldKey key.NodePublic
+			)
+
+			require.EventuallyWithT(t, func(c *assert.CollectT) {
+				status, err := target.Status()
+				if !assert.NoError(c, err) {
+					return
+				}
+
+				selfID, oldKey = string(status.Self.ID), status.Self.PublicKey
+			}, integrationutil.StatusReadyTimeout, integrationutil.FastPoll, "client must report its own status before expiry")
+
+			targetIP := target.MustIPv4().String()
+
+			_, err := headscale.Execute([]string{
+				"headscale", "nodes", "expire", "--identifier", selfID,
+			})
+			require.NoError(t, err)
+
+			for _, peer := range allClients {
+				if peer.Hostname() == target.Hostname() {
+					continue
+				}
+
+				require.EventuallyWithT(t, func(c *assert.CollectT) {
+					status, err := peer.Status()
+					if !assert.NoError(c, err) {
+						return
+					}
+
+					expired, found := status.Peer[oldKey]
+					if assert.True(c, found, "expired node must remain visible") {
+						assert.True(c, expired.Expired)
+					}
+				}, integrationutil.StatusReadyTimeout, integrationutil.FastPoll, "peer must see the node expired")
+			}
+
+			require.EventuallyWithT(t, func(c *assert.CollectT) {
+				status, err := target.Status()
+				if !assert.NoError(c, err) {
+					return
+				}
+
+				assert.Equal(c, "NeedsLogin", status.BackendState)
+			}, integrationutil.StatusReadyTimeout, integrationutil.FastPoll, "expired client must wait for login")
+
+			// Some clients serve peerapi again while they wait for login.
+			// Relogging once they do makes them register with the Hostinfo
+			// they later report as running, so no Hostinfo change can carry
+			// the whole node and the relogin itself must clear the expired
+			// flag. Which clients do is up to the client, so it is observed
+			// rather than assumed; the others relog with changed Hostinfo.
+			settled := false
+			tick := time.NewTicker(integrationutil.FastPoll)
+			timeout := time.After(peerAPISettleTimeout)
+
+		settle:
+			for {
+				status, err := target.Status()
+				if err == nil && len(status.Self.PeerAPIURL) > 0 {
+					settled = true
+
+					break
+				}
+
+				select {
+				case <-tick.C:
+				case <-timeout:
+					break settle
+				}
+			}
+
+			tick.Stop()
+
+			t.Logf("%s served peerapi while expired: %v", target.Version(), settled)
+
+			loginURL, err := target.LoginWithURL(headscale.GetEndpoint())
+			require.NoError(t, err)
+			body, err := doLoginURL(target.Hostname(), loginURL)
+			require.NoError(t, err)
+			require.NoError(t, scenario.runHeadscaleRegister("user1", body))
+			require.NoError(t, target.WaitForRunning(integrationutil.PeerSyncTimeout()))
+
+			var newKey key.NodePublic
+
+			require.EventuallyWithT(t, func(c *assert.CollectT) {
+				status, err := target.Status()
+				if !assert.NoError(c, err) {
+					return
+				}
+
+				newKey = status.Self.PublicKey
+				assert.NotEqual(c, oldKey, newKey, "relogin of an expired node must rotate its node key")
+			}, integrationutil.StatusReadyTimeout, integrationutil.FastPoll, "client must report its new node key")
+
+			for _, peer := range allClients {
+				if peer.Hostname() == target.Hostname() {
+					continue
+				}
+
+				require.EventuallyWithT(t, func(c *assert.CollectT) {
+					status, err := peer.Status()
+					if !assert.NoError(c, err) {
+						return
+					}
+
+					relogged, found := status.Peer[newKey]
+					if assert.True(c, found, "peer must know the new node key") {
+						assert.False(c, relogged.Expired, "peer must clear the expired flag")
+					}
+
+					stdout, _, err := peer.Execute([]string{
+						"tailscale", "ping", "--tsmp", "--c=1", "--timeout=2s", targetIP,
+					})
+					assert.NoError(c, err)
+					assert.Contains(c, stdout, "pong")
+				}, integrationutil.StatusReadyTimeout, integrationutil.FastPoll, "peer must reach the relogged node over WireGuard")
+			}
+		})
+	}
+
+	assertPingAll(t, allClients, allAddrs)
+}
+
 // TestAuthWebFlowLogoutAndReloginNewUser tests the scenario where multiple Tailscale clients
 // initially authenticate using the web-based authentication flow (where users visit a URL
 // in their browser to authenticate), then all clients log out and log back in as a different user.