state: send the whole node when a relogin removes a key expiry

A patch omits a zero expiry, so peers kept the old one and would cut the
node off when it passed.

hscontrol/servertest/lifecycle_test.go

@@ -679,6 +679,47 @@ func TestNodeExpiryRouteFailover(t *testing.T) {
 	}
 }
 
+// TestReloginWithoutExpiryClearsPeerExpiry: a relogin can drop a node's expiry on
+// the default config, and peers must not keep the old one and cut the node off.
+func TestReloginWithoutExpiryClearsPeerExpiry(t *testing.T) {
+	t.Parallel()
+
+	h := servertest.NewHarness(t, 2)
+	client, observer := h.Client(0), h.Client(1)
+	id := findNodeID(t, h.Server, client.Name)
+
+	future := time.Now().Add(time.Hour).UTC()
+	_, c, err := h.Server.State().SetNodeExpiry(id, &future)
+	require.NoError(t, err)
+	h.Server.App.Change(c)
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		p, ok := observer.PeerByName(client.Name)
+		if assert.True(c, ok) {
+			assert.True(c, p.KeyExpiry().Equal(future))
+		}
+	}, 10*time.Second, 50*time.Millisecond, "observer holds the expiry")
+
+	nv, ok := h.Server.State().GetNodeByID(id)
+	require.True(t, ok)
+
+	key := h.Server.CreatePreAuthKey(t, nv.TypedUserID())
+	hostinfo := &tailcfg.Hostinfo{BackendLogID: "servertest-" + client.Name, Hostname: client.Name}
+	require.NoError(t, client.Relogin(t.Context(), key, true, hostinfo))
+
+	require.EventuallyWithT(t, func(c *assert.CollectT) {
+		cur, ok := h.Server.State().GetNodeByID(id)
+		if assert.True(c, ok) {
+			assert.False(c, cur.Expiry().Valid(), "the server holds no expiry")
+		}
+
+		p, ok := observer.PeerByName(client.Name)
+		if assert.True(c, ok) {
+			assert.True(c, p.KeyExpiry().IsZero(), "observer holds %v", p.KeyExpiry())
+		}
+	}, 10*time.Second, 50*time.Millisecond, "observer drops the expiry")
+}
+
 // TestReloginFollowsNewHostname: a node's name follows a new hostname at a login
 // as at a map request, and later hostname changes still rename it. An admin-set
 // name stays.

hscontrol/state/state.go

@@ -3957,7 +3957,9 @@ func reauthChange(node types.NodeView, r relogin, policyChanged bool) change.Cha
 // reloginChanged compares what peers read before and after a relogin.
 // [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.
+// relogin stored the Hostinfo, so no later MapRequest shows the change. It
+// cannot remove a key expiry either, as it omits a zero one, and peers would
+// cut the node off when the old one passes.
 func reloginChanged(before, after types.NodeView, now time.Time) relogin {
 	expiry := func(n types.NodeView) time.Time {
 		e, _ := n.Expiry().GetOk()
@@ -3965,13 +3967,20 @@ func reloginChanged(before, after types.NodeView, now time.Time) relogin {
 		return e
 	}
 
+	// What a peer patch cannot carry.
+	expiredChanged := before.IsExpiredAt(now) != after.IsExpiredAt(now)
+	hostinfoChanged := !peerHostinfoEqual(before.Hostinfo(), after.Hostinfo())
+	expiryRemoved := before.Expiry().Valid() && !after.Expiry().Valid()
+
+	// What a peer patch carries.
+	keysSame := before.NodeKey() == after.NodeKey() && before.DiscoKey() == after.DiscoKey()
+	expirySame := expiry(before).Equal(expiry(after))
+	endpointsSame := views.SliceEqual(before.Endpoints(), after.Endpoints())
+
 	switch {
-	case before.IsExpiredAt(now) != after.IsExpiredAt(now) ||
-		!peerHostinfoEqual(before.Hostinfo(), after.Hostinfo()):
+	case expiredChanged || hostinfoChanged || expiryRemoved:
 		return reloginWhole
-	case before.NodeKey() == after.NodeKey() && before.DiscoKey() == after.DiscoKey() &&
-		expiry(before).Equal(expiry(after)) &&
-		views.SliceEqual(before.Endpoints(), after.Endpoints()):
+	case keysSame && expirySame && endpointsSame:
 		return reloginNone
 	default:
 		return reloginKeys

hscontrol/types/change/change.go

@@ -625,9 +625,9 @@ func NodeKeyRotated(node types.NodeView) Change {
 	nk := node.NodeKey()
 	dk := node.DiscoKey()
 
-	// 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]. It is sent in UTC, as
+	// KeyExpiry carries the new expiry. The zero value does not reach the wire
+	// ([tailcfg.PeerChange] omits it), so a removed expiry needs the whole node.
+	// It does not clear [tailcfg.Node.Expired]. It is sent in UTC, as
 	// [types.NodeView.TailNode] sends it: the stored zone depends on the
 	// database.
 	var expiry time.Time