state: let a pre-auth key that is no longer valid only wake its node

A login that keeps its node key leaves a live node's expiry alone, even
with a valid key, and a wake-up is not refused for the tags it requests.

hscontrol/servertest/reauth_compat_test.go

@@ -57,28 +57,23 @@ var reauthSkipReasons = map[string]string{
 	"reauth-user/a5-expired-valid-key/u-valid/outcome":  expiredKeyNotRotated,
 	"reauth-user/a5-expired-valid-key/u-valid/node-key": expiredKeyAccepted,
 
-	"reauth-owner/u-renew-relogin/u-renew/expiry": sameKeyRenewsExpiry,
-	"reauth-user/a1-spent-key/u-spent/expiry":     sameKeyRenewsExpiry,
-	"reauth-user/a2-valid-key/u-valid/expiry":     sameKeyRenewsExpiry,
-
 	"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,
 
-	"reauth-foreign/t-wake-foreign-key-tags/t-wake/outcome": wakeWithTagsRefused,
-	"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,
-	"identity/w-tagreq-relogin-tags/w-tagreq/expiry":        keyLoginTagsRefused,
-	"identity/w-tagreq-relogin-tags/obs/filter":             keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/devices":       keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/owner":         keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/outcome":       keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/expiry":        keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/ephemeral":     keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/peer-obs":      keyLoginTagsRefused,
-	"identity/w-tagreq-new-join/w-tagreq-new/peer-key-obs":  keyLoginTagsRefused,
+	"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,
+	"identity/w-tagreq-relogin-tags/w-tagreq/expiry":       keyLoginTagsRefused,
+	"identity/w-tagreq-relogin-tags/obs/filter":            keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/devices":      keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/owner":        keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/outcome":      keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/expiry":       keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/ephemeral":    keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/peer-obs":     keyLoginTagsRefused,
+	"identity/w-tagreq-new-join/w-tagreq-new/peer-key-obs": keyLoginTagsRefused,
 
 	"tag-owner-chain/u-base-join/u-base/devices":      keyLoginTagsRefused,
 	"tag-owner-chain/u-base-join/u-base/owner":        keyLoginTagsRefused,
@@ -113,9 +108,7 @@ const (
 
 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"
-	sameKeyRenewsExpiry      = "SAME_KEY_LOGIN_RENEWS_EXPIRY: headscale renews a user node's expiry on a key login that keeps its node key; SaaS renews it only with a new node key"
 	taggedLogoutNotExpired   = "TAGGED_LOGOUT_NOT_EXPIRED: headscale leaves a tagged node unexpired when it logs out; SaaS expires it"
-	wakeWithTagsRefused      = "WAKE_WITH_TAGS_REFUSED: headscale refuses a key login that requests tags the key does not carry even when the node keeps its node key and tags; SaaS accepts it and changes nothing"
 	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

@@ -2862,6 +2862,184 @@ func TestSingleUseKeyPublishesOnlyOneRegistration(t *testing.T) {
 	assert.True(t, storedKey.Used)
 }
 
+func TestPAKWakeupKeepsEphemeralKey(t *testing.T) {
+	s := newRetagTestState(t)
+	original, err := s.CreatePreAuthKey(nil, false, true, nil, []string{"tag:prod"})
+	require.NoError(t, err)
+	c := registerForPAKRereg(t, s, original)
+	_, err = s.UpdateNodeFromMapRequest(c.node.ID(), tailcfg.MapRequest{
+		NodeKey:  c.regReq.NodeKey,
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "h0", NetInfo: &tailcfg.NetInfo{PreferredDERP: 8}},
+	})
+	require.NoError(t, err)
+
+	user := s.CreateUserForTest("wake-guest")
+	spent, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+	registerForPAKRereg(t, s, spent)
+	before, ok := s.GetNodeByID(c.node.ID())
+	require.True(t, ok)
+
+	req := c.regReq
+	req.Auth = &tailcfg.RegisterResponseAuth{AuthKey: spent.Key}
+	req.Hostinfo = &tailcfg.Hostinfo{Hostname: "h1", OS: "linux"}
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.NoError(t, err)
+
+	current, ok := s.GetNodeByID(c.node.ID())
+	require.True(t, ok)
+	assert.Equal(t, original.ID, current.AuthKeyID().Get())
+	assert.True(t, current.IsEphemeral())
+	assert.Equal(t, before.Tags().AsSlice(), current.Tags().AsSlice())
+	assert.Equal(t, "h1", current.Hostname())
+	assert.Equal(t, "linux", current.Hostinfo().OS())
+	assert.Equal(t, tailcfg.DERPRegionID(8), current.Hostinfo().NetInfo().PreferredDERP())
+	assert.True(t, current.LastSeen().Get().After(before.LastSeen().Get()))
+
+	stored, err := s.db.GetNodeByID(c.node.ID())
+	require.NoError(t, err)
+	require.NotNil(t, stored.AuthKeyID)
+	assert.Equal(t, original.ID, *stored.AuthKeyID)
+	assert.True(t, stored.IsEphemeral())
+	assert.Equal(t, "h1", stored.Hostname)
+	assert.Equal(t, "linux", stored.Hostinfo.OS)
+	assert.Equal(t, tailcfg.DERPRegionID(8), stored.Hostinfo.NetInfo.PreferredDERP)
+}
+
+func TestPAKWakeupKeepsScheduledExpiry(t *testing.T) {
+	for _, defaultExpiry := range []time.Duration{0, 24 * time.Hour} {
+		for _, requestExpiry := range []bool{false, true} {
+			t.Run(fmt.Sprintf("default=%s/request=%t", defaultExpiry, requestExpiry), func(t *testing.T) {
+				s := newRetagTestState(t)
+				s.cfg.Node.Expiry = defaultExpiry
+				user := s.CreateUserForTest("wake-user")
+				pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+				require.NoError(t, err)
+				c := registerForPAKRereg(t, s, pak)
+				scheduled := time.Now().Add(2 * time.Hour)
+				_, _, err = s.SetNodeExpiry(c.node.ID(), &scheduled)
+				require.NoError(t, err)
+
+				req := c.regReq
+
+				req.Expiry = time.Time{}
+				if requestExpiry {
+					req.Expiry = time.Now().AddDate(100, 0, 0)
+				}
+
+				_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+				require.NoError(t, err)
+				ns, row := c.fields(t)
+				assert.Equal(t, scheduled.UnixNano(), ns.Expiry)
+				assert.Equal(t, scheduled.UnixNano(), row.Expiry)
+			})
+		}
+	}
+}
+
+func TestPAKWakeupKeepsOIDCRegistration(t *testing.T) {
+	s := newRetagTestState(t)
+	user := s.CreateUserForTest("wake-oidc-user")
+	pak, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+	registerForPAKRereg(t, s, pak)
+
+	machineKey := key.NewMachine().Public()
+	nodeKey := key.NewNode().Public()
+	authID := types.MustAuthID()
+	require.NoError(t, s.SetAuthCacheEntry(authID, types.NewRegisterAuthRequest(&types.RegistrationData{
+		MachineKey: machineKey,
+		NodeKey:    nodeKey,
+		Hostname:   "oidc-node",
+		Hostinfo:   &tailcfg.Hostinfo{Hostname: "oidc-node"},
+	})))
+	node, _, err := s.HandleNodeFromAuthPath(authID, types.UserID(user.ID), nil, util.RegisterMethodOIDC)
+	require.NoError(t, err)
+	require.Equal(t, util.RegisterMethodOIDC, node.RegisterMethod())
+	require.False(t, node.AuthKeyID().Valid())
+
+	_, _, err = s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
+		Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+		NodeKey:  nodeKey,
+		Hostinfo: &tailcfg.Hostinfo{Hostname: "oidc-node"},
+	}, machineKey)
+	require.NoError(t, err)
+
+	current, ok := s.GetNodeByID(node.ID())
+	require.True(t, ok)
+	assert.Equal(t, util.RegisterMethodOIDC, current.RegisterMethod())
+	assert.False(t, current.AuthKeyID().Valid())
+
+	stored, err := s.db.GetNodeByID(node.ID())
+	require.NoError(t, err)
+	assert.Equal(t, util.RegisterMethodOIDC, stored.RegisterMethod)
+	assert.Nil(t, stored.AuthKeyID)
+}
+
+// TestPAKSameNodeKeyLoginKeepsExpiry: a valid key renews a live node's expiry
+// only with a new node key.
+func TestPAKSameNodeKeyLoginKeepsExpiry(t *testing.T) {
+	s := newRetagTestState(t)
+	user := s.CreateUserForTest("same-key-user")
+	pak, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+
+	c := registerForPAKRereg(t, s, pak)
+	scheduled := time.Now().Add(2 * time.Hour)
+	_, _, err = s.SetNodeExpiry(c.node.ID(), &scheduled)
+	require.NoError(t, err)
+
+	req := c.regReq
+	req.Expiry = time.Now().AddDate(1, 0, 0)
+
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.NoError(t, err)
+
+	ns, row := c.fields(t)
+	assert.Equal(t, scheduled.UnixNano(), ns.Expiry, "same node key")
+	assert.Equal(t, scheduled.UnixNano(), row.Expiry, "same node key")
+
+	req.NodeKey = key.NewNode().Public()
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.NoError(t, err)
+
+	ns, row = c.fields(t)
+	assert.Equal(t, req.Expiry.UnixNano(), ns.Expiry, "new node key")
+	assert.Equal(t, req.Expiry.UnixNano(), row.Expiry, "new node key")
+}
+
+// TestPAKWakeAcceptsRequestedTags: a wake-up writes no tags, so tags it
+// requests beyond the key's are not refused; a key that re-authenticates the
+// node still is.
+func TestPAKWakeAcceptsRequestedTags(t *testing.T) {
+	s := newRetagTestState(t)
+	user := s.CreateUserForTest("wake-tags-user")
+	spent, err := s.CreatePreAuthKey(user.TypedID(), false, false, nil, nil)
+	require.NoError(t, err)
+
+	_, err = s.SetPolicy([]byte(`{"tagOwners": {"tag:prod": ["wake-tags-user@"]}}`))
+	require.NoError(t, err)
+
+	c := registerForPAKRereg(t, s, spent)
+	_, _, err = s.SetNodeTags(c.node.ID(), []string{"tag:prod"})
+	require.NoError(t, err)
+
+	req := c.regReq
+	req.Hostinfo = &tailcfg.Hostinfo{Hostname: "h1", RequestTags: []string{"tag:prod"}}
+
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.NoError(t, err)
+
+	ns, row := c.fields(t)
+	assert.Equal(t, []string{"tag:prod"}, ns.Tags)
+	assert.Equal(t, []string{"tag:prod"}, row.Tags)
+	assert.Equal(t, "h1", ns.Hostname)
+
+	req.NodeKey = key.NewNode().Public()
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.ErrorIs(t, err, ErrRequestedTagsInvalidOrNotPermitted)
+}
+
 // 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

hscontrol/state/state.go

@@ -2987,22 +2987,23 @@ func pakSkipsValidation(n types.NodeView, pak *types.PreAuthKey, nodeKey key.Nod
 		!isOwnershipConversion && !isRetag
 }
 
-// pakReregister is the authorisation decision for re-registering n with pak
-// at now: whether the key re-authenticates n, and nil when validation may be
-// skipped, else the key's validity. A key that does not speak for n only
-// wakes it under its own node key.
+// 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) {
-	reauth := pak.CanReauthenticate(n)
-	if pakSkipsValidation(n, pak, nodeKey, now) {
-		return reauth, nil
-	}
-
 	err := pak.ValidAt(now)
-	if err == nil && !reauth {
+	if err == nil {
+		if pak.CanReauthenticate(n) {
+			return true, nil
+		}
+
 		err = types.PAKError("authkey cannot re-authenticate this node")
 	}
 
-	return reauth, err
+	if pakSkipsValidation(n, pak, nodeKey, now) {
+		return false, nil
+	}
+
+	return false, err
 }
 
 //nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering
@@ -3037,8 +3038,10 @@ func (s *State) HandleNodeFromPreAuthKey(
 	// A pre-auth key node's tags come from the key, never from RequestTags.
 	// Advertising a subset of the key's tags is redundant, not an escalation,
 	// and the tailscale client's OAuth authkey flow always does it; any other
-	// tag is rejected. Checked before key validation or resource allocation so
-	// new nodes and re-registrations are held to the same rule.
+	// tag is rejected, ahead of the key's own error, so new nodes and
+	// re-registrations are held to the same rule.
+	var tagsErr error
+
 	if regReq.Hostinfo != nil {
 		var extraTags []string
 
@@ -3049,7 +3052,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 		}
 
 		if len(extraTags) > 0 {
-			return types.NodeView{}, s.policyChangeSince(genBefore), fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, extraTags)
+			tagsErr = fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, extraTags)
 		}
 	}
 
@@ -3061,24 +3064,20 @@ func (s *State) HandleNodeFromPreAuthKey(
 	// This decision is only an early reject that keeps a key error ahead of
 	// ErrNodeKeyInUse. An in-place re-registration decides again in the
 	// NodeStore writer, on the node it replaces; see reregisterNodeWithPAK.
-	if existsSameUser && pakSkipsValidation(existingNodeSameUser, pak, regReq.NodeKey, time.Now()) {
-		log.Debug().
-			Caller().
-			Uint64(zf.NodeID, existingNodeSameUser.ID().Uint64()).
-			Str(zf.NodeName, existingNodeSameUser.Hostname()).
-			Str(zf.MachineKey, machineKey.ShortString()).
-			Str(zf.NodeKeyExisting, existingNodeSameUser.NodeKey().ShortString()).
-			Str(zf.NodeKeyRequest, regReq.NodeKey.ShortString()).
-			Uint64(zf.AuthKeyID, pak.ID).
-			Bool(zf.AuthKeyUsed, pak.Used).
-			Bool(zf.AuthKeyExpired, pak.Expiration != nil && pak.Expiration.Before(time.Now())).
-			Bool(zf.AuthKeyReusable, pak.Reusable).
-			Msg("Existing node re-registering with same NodeKey; lookup permits skipping key validation, writer re-decides")
+	reauth := true
+	if existsSameUser {
+		reauth, err = pakReregister(existingNodeSameUser, pak, regReq.NodeKey, time.Now())
 	} else {
 		err = pak.Validate()
-		if err != nil {
-			return types.NodeView{}, s.policyChangeSince(genBefore), err
-		}
+	}
+
+	// A wake-up writes no tags, so the tags it asks for are not refused.
+	if tagsErr != nil && (reauth || err != nil) {
+		return types.NodeView{}, s.policyChangeSince(genBefore), tagsErr
+	}
+
+	if err != nil {
+		return types.NodeView{}, s.policyChangeSince(genBefore), err
 	}
 
 	// Preserve the raw hostname as reported by the client. Sanitisation
@@ -3339,24 +3338,27 @@ func (s *State) reregisterNodeWithPAK(
 			node.AuthKey.Used = true
 		}
 
-		// Tagged nodes keep their existing expiry (disabled).
 		// User-owned nodes update expiry from the client request,
 		// falling back to the configured default if the client
 		// did not request a specific expiry. If neither is set,
 		// clear the expiry so the database holds NULL instead of
 		// a pointer to zero time.
-		if !node.IsTagged() {
-			if !regReq.Expiry.IsZero() {
-				node.Expiry = &regReq.Expiry
-			} else if s.cfg.Node.Expiry > 0 {
-				exp := now.Add(s.cfg.Node.Expiry)
-				node.Expiry = &exp
-			} else {
+		switch {
+		case node.IsTagged():
+			// Tagged nodes keep their expiry; a valid key lifts a past one. A
+			// future administrative expiry has not taken effect and stays.
+			if node.IsExpiredAt(now) {
 				node.Expiry = nil
 			}
-		} else if node.IsExpiredAt(now) {
-			// A valid key re-authenticated this expired tagged node. A future
-			// administrative expiry has not taken effect and stays untouched.
+		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 !regReq.Expiry.IsZero():
+			node.Expiry = &regReq.Expiry
+		case s.cfg.Node.Expiry > 0:
+			exp := now.Add(s.cfg.Node.Expiry)
+			node.Expiry = &exp
+		default:
 			node.Expiry = nil
 		}
 	})
@@ -3379,13 +3381,8 @@ func (s *State) reregisterNodeWithPAK(
 		return types.NodeView{}, false, authErr
 	}
 
-	// Explicitly select all node columns so GORM includes nil/zero-value fields
-	// (see nodeUpdateColumns comment). AuthKeyID is excluded on MapRequest
-	// so a deleted key's stale reference is not persisted, but
-	// re-registration presents a key just loaded from the database, so
-	// its ID must be persisted here — otherwise a restart reloads the old
-	// key and any key-scoped property (e.g. Ephemeral) silently reverts.
-	// A wake-up keeps the prior key, which may have been deleted (#2862).
+	// Only re-authentication writes a newly loaded key's reference. A wake-up
+	// keeps the prior key, which may have been deleted (#2862).
 	reregColumns := nodeUpdateColumns
 	if reauth {
 		reregColumns = append(slices.Clone(nodeUpdateColumns), "AuthKeyID")

hscontrol/util/zlog/zf/fields.go

@@ -15,8 +15,6 @@ const (
 	NodeID             = "node.id"
 	NodeName           = "node.name"
 	NodeKey            = "node.key"
-	NodeKeyExisting    = "node.key.existing"
-	NodeKeyRequest     = "node.key.request"
 	NodeTags           = "node.tags"
 	NodeIsTagged       = "node.is_tagged"
 	NodeOnline         = "node.online"
@@ -66,18 +64,15 @@ const (
 
 // PreAuthKey fields.
 const (
-	PAKID           = "pak.id"
-	PAKPrefix       = "pak.prefix"
-	PAKTags         = "pak.tags"
-	PAKReusable     = "pak.reusable"
-	PAKEphemeral    = "pak.ephemeral"
-	PAKUsed         = "pak.used"
-	PAKIsTagged     = "pak.is_tagged"
-	PAKExpiration   = "pak.expiration"
-	AuthKeyID       = "authkey.id"
-	AuthKeyUsed     = "authkey.used"
-	AuthKeyExpired  = "authkey.expired"
-	AuthKeyReusable = "authkey.reusable"
+	PAKID         = "pak.id"
+	PAKPrefix     = "pak.prefix"
+	PAKTags       = "pak.tags"
+	PAKReusable   = "pak.reusable"
+	PAKEphemeral  = "pak.ephemeral"
+	PAKUsed       = "pak.used"
+	PAKIsTagged   = "pak.is_tagged"
+	PAKExpiration = "pak.expiration"
+	AuthKeyID     = "authkey.id"
 )
 
 // APIKey fields.