state: send relogins as whole nodes when peers need them

Updates #3541, #3531

Adapted: #3525 is not backported, so the inline pre-auth key
re-registration path reports keysOnly, and NodeView.IsExpiredAt comes
along from it.

(cherry picked from commit a8d6f5be81)
This commit is contained in:
Kristoffer Dalby committed 2026-10-13 14:35:52 +02:00
1 parent 5fb2d4e089
commit 5c25a66983
12 files changed
+639 -55

No files matched your search

+70
View File
@@ -4108,6 +4108,76 @@ 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)
// Direct fixture writes bypass registration's policy-cache updates.
require.NoError(t, app.state.UpdatePolicyManagerUsersForTest())
require.NoError(t, app.state.UpdatePolicyManagerNodesForTest())
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
+91
View File
@@ -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()
+47 -20
View File
@@ -10,6 +10,7 @@ import (
"github.com/rs/zerolog"
"github.com/rs/zerolog/log"
"tailscale.com/tailcfg"
"tailscale.com/types/views"
)
// mapRequestDelta carries the classified facts extracted from one MapRequest
@@ -154,30 +155,56 @@ func hostinfoEqual(oldHI, newHI *tailcfg.Hostinfo) bool {
return oldCopy.Equal(&newCopy)
}
// peerHostinfo keeps the Hostinfo fields another node's client reads from
// a peer: what tailscale status shows, PeerAPI services, SSH known hosts,
// exit-node location, app-connector eligibility, and the routes this server
// feeds into policy. Everything else is stored but never fanned out, and a
// peer's NetInfo is never read at all.
func peerHostinfo(hi *tailcfg.Hostinfo) *tailcfg.Hostinfo {
if hi == nil {
return nil
// peerHostinfoEqual reports whether a and b agree on the Hostinfo fields
// another node's client reads from a peer: what tailscale status shows, PeerAPI
// services, SSH known hosts, exit-node location, app-connector eligibility, and
// the routes this server feeds into policy. Everything else is stored but never
// fanned out, and a peer's NetInfo is never read at all.
func peerHostinfoEqual(a, b tailcfg.HostinfoView) bool {
if !a.Valid() || !b.Valid() {
return a.Valid() == b.Valid()
}
return &tailcfg.Hostinfo{
Hostname: hi.Hostname,
OS: hi.OS,
Services: hi.Services,
SSH_HostKeys: hi.SSH_HostKeys,
Location: hi.Location,
AppConnector: hi.AppConnector,
RoutableIPs: hi.RoutableIPs,
}
return a.Hostname() == b.Hostname() &&
a.OS() == b.OS() &&
servicesEqual(a.Services(), b.Services()) &&
views.SliceEqual(a.SSH_HostKeys(), b.SSH_HostKeys()) &&
locationEqual(a.Location(), b.Location()) &&
a.AppConnector() == b.AppConnector() &&
views.SliceEqual(a.RoutableIPs(), b.RoutableIPs())
}
// peerHostinfoEqual reports whether the fields peers read are unchanged.
func peerHostinfoEqual(oldHI, newHI *tailcfg.Hostinfo) bool {
return peerHostinfo(oldHI).Equal(peerHostinfo(newHI))
// servicesEqual compares field by field, as [tailcfg.Service] is
// incomparable.
func servicesEqual(a, b views.Slice[tailcfg.Service]) bool {
if a.Len() != b.Len() {
return false
}
for i := range a.Len() {
x, y := a.At(i), b.At(i)
if x.Proto != y.Proto || x.Port != y.Port || x.Description != y.Description {
return false
}
}
return true
}
// locationEqual compares field by field, as [tailcfg.LocationView] has no
// Equal.
func locationEqual(a, b tailcfg.LocationView) bool {
if !a.Valid() || !b.Valid() {
return a.Valid() == b.Valid()
}
return a.Country() == b.Country() &&
a.CountryCode() == b.CountryCode() &&
a.City() == b.City() &&
a.CityCode() == b.CityCode() &&
a.Latitude() == b.Latitude() &&
a.Longitude() == b.Longitude() &&
a.Priority() == b.Priority()
}
// netInfoEqualIgnoringDERP compares two NetInfo values via
+52 -3
View File
@@ -2,6 +2,7 @@ package state
import (
"net/netip"
"reflect"
"strings"
"sync"
"sync/atomic"
@@ -858,12 +859,60 @@ func TestPeerHostinfoEqual(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
changed := base()
tt.mutate(changed)
require.Equal(t, tt.want, peerHostinfoEqual(base(), changed))
require.Equal(t, tt.want, peerHostinfoEqual(base().View(), changed.View()))
})
}
require.True(t, peerHostinfoEqual(nil, nil))
require.False(t, peerHostinfoEqual(nil, base()))
require.True(t, peerHostinfoEqual(tailcfg.HostinfoView{}, tailcfg.HostinfoView{}))
require.False(t, peerHostinfoEqual(tailcfg.HostinfoView{}, base().View()))
}
// TestPeerHostinfoEqualSeesEveryField fails when tailscale adds a field to
// [tailcfg.Service] or [tailcfg.Location] that servicesEqual or locationEqual
// does not compare.
func TestPeerHostinfoEqualSeesEveryField(t *testing.T) {
set := func(t *testing.T, f reflect.Value) {
t.Helper()
switch {
case f.Kind() == reflect.String:
f.SetString("x")
case f.CanInt():
f.SetInt(1)
case f.CanUint():
f.SetUint(1)
case f.CanFloat():
f.SetFloat(1)
default:
t.Fatalf("field kind %s not handled; compare it", f.Kind())
}
}
for _, field := range reflect.VisibleFields(reflect.TypeFor[tailcfg.Service]()) {
if !field.IsExported() {
continue
}
t.Run("Service."+field.Name, func(t *testing.T) {
var svc tailcfg.Service
set(t, reflect.ValueOf(&svc).Elem().FieldByIndex(field.Index))
a := &tailcfg.Hostinfo{Services: []tailcfg.Service{{}}}
b := &tailcfg.Hostinfo{Services: []tailcfg.Service{svc}}
assert.False(t, peerHostinfoEqual(a.View(), b.View()))
})
}
for _, field := range reflect.VisibleFields(reflect.TypeFor[tailcfg.Location]()) {
t.Run("Location."+field.Name, func(t *testing.T) {
var loc tailcfg.Location
set(t, reflect.ValueOf(&loc).Elem().FieldByIndex(field.Index))
a := &tailcfg.Hostinfo{Location: &tailcfg.Location{}}
b := &tailcfg.Hostinfo{Location: &loc}
assert.False(t, peerHostinfoEqual(a.View(), b.View()))
})
}
}
func TestNetInfoEqualIgnoringDERP(t *testing.T) {
+38
View File
@@ -198,6 +198,9 @@ type work struct {
// prober applying multiple probe results at once) cannot have a
// partial snapshot published between the updates.
multiUpdates map[types.NodeID]UpdateNodeFunc
// pre, when non-nil, receives each updated node as it was before its
// update function ran.
pre map[types.NodeID]types.NodeView
}
// updateChanges reports whether an in-place update moved a peer-visibility
@@ -277,6 +280,27 @@ func (s *NodeStore) UpdateNode(nodeID types.NodeID, updateFn UpdateNodeFunc) (ty
return s.GetNode(nodeID)
}
// UpdateNodeDiff is [NodeStore.UpdateNode] that also returns, first, the node
// as it was before updateFn ran, so callers can tell what the update changed
// without cloning the node themselves. That view is invalid when the node does
// not exist.
func (s *NodeStore) UpdateNodeDiff(
nodeID types.NodeID,
updateFn UpdateNodeFunc,
) (types.NodeView, types.NodeView, bool) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("update"))
defer timer.ObserveDuration()
pre := make(map[types.NodeID]types.NodeView, 1)
s.updateNodes(map[types.NodeID]UpdateNodeFunc{nodeID: updateFn}, pre)
nodeStoreOperations.WithLabelValues("update").Inc()
after, ok := s.GetNode(nodeID)
return pre[nodeID], after, ok
}
// UpdateNodes applies per-node update functions in a single atomic
// batch. The election that recomputes primary routes runs once, after
// every update has landed, so callers cannot observe an intermediate
@@ -285,6 +309,15 @@ func (s *NodeStore) UpdateNode(nodeID types.NodeID, updateFn UpdateNodeFunc) (ty
// would change the election outcome — e.g. the HA prober applying
// concurrent probe-timeout results.
func (s *NodeStore) UpdateNodes(updates map[types.NodeID]UpdateNodeFunc) {
s.updateNodes(updates, nil)
}
// updateNodes queues updates as one batch entry and waits for it, filling pre
// when it is non-nil.
func (s *NodeStore) updateNodes(
updates map[types.NodeID]UpdateNodeFunc,
pre map[types.NodeID]types.NodeView,
) {
timer := prometheus.NewTimer(nodeStoreOperationDuration.WithLabelValues("update_multi"))
defer timer.ObserveDuration()
@@ -295,6 +328,7 @@ func (s *NodeStore) UpdateNodes(updates map[types.NodeID]UpdateNodeFunc) {
w := work{
op: updateMulti,
multiUpdates: updates,
pre: pre,
result: make(chan struct{}),
}
@@ -510,6 +544,10 @@ func (s *NodeStore) applyBatch(batch []work) {
nodes[id] = n
if w.pre != nil {
w.pre[id] = pre.View()
}
relation, election := updateChanges(pre, &n)
relationChanged = relationChanged || relation
electionChanged = electionChanged || election
+22
View File
@@ -1667,6 +1667,28 @@ func TestUpdateNodeRecomputesPeersOnlyForRelationInputs(t *testing.T) {
}
}
// TestUpdateNodeDiff proves the update returns the node the writer replaced
// alongside the result, and an invalid before for a missing node.
func TestUpdateNodeDiff(t *testing.T) {
node := createTestNode(1, 1, "user1", "node1")
oldKey := node.NodeKey
store := NewNodeStore(types.Nodes{&node}, allowAllPeersFunc, TestBatchSize, TestBatchTimeout)
store.Start()
defer store.Stop()
newKey := key.NewNode().Public()
before, after, ok := store.UpdateNodeDiff(1, func(n *types.Node) { n.NodeKey = newKey })
require.True(t, ok)
assert.Equal(t, oldKey, before.NodeKey())
assert.Equal(t, newKey, after.NodeKey())
before, _, ok = store.UpdateNodeDiff(99, func(*types.Node) {})
assert.False(t, ok)
assert.False(t, before.Valid())
}
// TestListPeersExcludesSelf proves a node is never returned among its own
// peers, on both the snapshot path and the explicit peer-ID path.
//
+44 -3
View File
@@ -315,7 +315,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(),
@@ -375,7 +375,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(),
@@ -395,7 +395,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
@@ -422,6 +422,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
+47 -26
View File
@@ -1679,8 +1679,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 {
@@ -1721,7 +1723,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,
@@ -1736,11 +1738,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
@@ -1831,9 +1833,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).
@@ -1859,7 +1863,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
@@ -1873,7 +1877,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].
@@ -2277,12 +2281,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{}, change.Change{}, err
}
@@ -2290,7 +2297,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{}, change.Change{}, err
}
@@ -2344,10 +2351,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.
@@ -2566,7 +2573,10 @@ func (s *State) HandleNodeFromPreAuthKey(
Str(zf.UserName, pakUsername()).
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
@@ -2600,7 +2610,7 @@ func (s *State) HandleNodeFromPreAuthKey(
priorNode := existingNodeSameUser.AsStruct()
// 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) {
node.NodeKey = regReq.NodeKey
node.Hostname = hostname
@@ -2730,6 +2740,7 @@ func (s *State) HandleNodeFromPreAuthKey(
Msg("Node re-authorized")
finalNode = updatedNodeView
keysOnly = onlyKeysChanged(before, updatedNodeView, time.Now())
} else {
// Node does not exist for this user with this machine key.
// For a user-owned key, check whether the machine key is already held
@@ -2823,27 +2834,37 @@ func (s *State) HandleNodeFromPreAuthKey(
policyChanged := !usersChange.IsEmpty() || !nodesChange.IsEmpty()
return finalNode, reauthChange(finalNode, existsSameUser, policyChanged), nil
return finalNode, reauthChange(finalNode, keysOnly, policyChanged), 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.
@@ -3120,7 +3141,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
delta.hostinfoChanged = newHostinfo != nil &&
!hostinfoEqual(currentNode.Hostinfo, newHostinfo)
delta.peerHostinfoChanged = newHostinfo != nil &&
!peerHostinfoEqual(currentNode.Hostinfo, newHostinfo)
!peerHostinfoEqual(currentNode.Hostinfo.View(), newHostinfo.View())
delta.dnsMetadataChanged = newHostinfo != nil &&
(currentNode.Hostinfo == nil ||
currentNode.Hostinfo.Hostname != newHostinfo.Hostname ||
+7 -2
View File
@@ -508,12 +508,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
+15 -1
View File
@@ -214,6 +214,11 @@ func (ns Nodes) ViewSlice() views.Slice[NodeView] {
// IsExpired returns whether the node registration has expired.
func (node *Node) IsExpired() bool {
return node.IsExpiredAt(time.Now())
}
// IsExpiredAt reports whether the node registration has expired at now.
func (node *Node) IsExpiredAt(now time.Time) bool {
// If Expiry is not set, the client has not indicated that
// it wants an expiry time, it is therefore considered
// to mean "not expired"
@@ -221,7 +226,7 @@ func (node *Node) IsExpired() bool {
return false
}
return time.Since(*node.Expiry) > 0
return now.After(*node.Expiry)
}
// Online reports the node's last known connectivity. Unknown counts as
@@ -1016,6 +1021,15 @@ func (nv NodeView) IsExpired() bool {
return nv.ж.IsExpired()
}
// IsExpiredAt reports whether the node registration has expired at now.
func (nv NodeView) IsExpiredAt(now time.Time) bool {
if !nv.Valid() {
return true
}
return nv.ж.IsExpiredAt(now)
}
// Online reports the node's last known connectivity.
func (nv NodeView) Online() bool {
if !nv.Valid() {
+30
View File
@@ -1095,3 +1095,33 @@ func TestHasPolicyChangeFields(t *testing.T) {
})
}
}
func TestNodeIsExpiredAt(t *testing.T) {
now := time.Now()
zero := time.Time{}
justBefore := now.Add(-time.Nanosecond)
tests := []struct {
name string
expiry *time.Time
want bool
}{
{name: "nil expiry", expiry: nil, want: false},
{name: "zero expiry", expiry: &zero, want: false},
{name: "expiry equals now", expiry: &now, want: false},
{name: "now one nanosecond after expiry", expiry: &justBefore, want: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
n := &Node{Expiry: tt.expiry}
require.Equal(t, tt.want, n.IsExpiredAt(now))
require.Equal(t, tt.want, n.View().IsExpiredAt(now))
})
}
t.Run("invalid view", func(t *testing.T) {
require.True(t, NodeView{}.IsExpiredAt(now),
"an invalid view counts as expired, like IsExpired")
})
}
+176
View File
@@ -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.