From c48767eed6efafd920d6c86d8ebc3a6982f341ba Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 09:22:04 +0000 Subject: [PATCH] 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 | 13 -- hscontrol/state/auth_tagged_expiry_test.go | 178 +++++++++++++++++++++ hscontrol/state/state.go | 110 +++++++------ hscontrol/util/zlog/zf/fields.go | 23 ++- 4 files changed, 248 insertions(+), 76 deletions(-) diff --git a/hscontrol/servertest/reauth_compat_test.go b/hscontrol/servertest/reauth_compat_test.go index 5964c0db0..3458ffe44 100644 --- a/hscontrol/servertest/reauth_compat_test.go +++ b/hscontrol/servertest/reauth_compat_test.go @@ -57,10 +57,6 @@ 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-reauth-foreign-relogin/t-reauth-foreign/devices": otherUserKeyReauthsTagged, "reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/owner": otherUserKeyReauthsTagged, "reauth-foreign/t-reauth-foreign-relogin/t-reauth-foreign/node-key": otherUserKeyReauthsTagged, @@ -80,13 +76,6 @@ var reauthSkipReasons = map[string]string{ "reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/login-devices": loginTakesTaggedDevice, "reauth-foreign/t-exp-foreign-relogin/t-exp-foreign/login-owner": loginTakesTaggedDevice, - "reauth-foreign/t-eph-spent-foreign-key/t-eph/ephemeral": spentKeyRebindsNode, - "reauth-foreign/t-eph-logout/t-eph/devices": spentKeyRebindsNode, - "reauth-foreign/t-eph-logout/t-eph/owner": spentKeyRebindsNode, - "reauth-foreign/t-eph-logout/t-eph/expiry": spentKeyRebindsNode, - "reauth-foreign/t-eph-logout/t-eph/ephemeral": spentKeyRebindsNode, - "reauth-foreign/t-eph-logout/t-eph/peer-obs": spentKeyRebindsNode, - "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, @@ -143,10 +132,8 @@ 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" otherUserKeyReauthsTagged = "OTHER_USER_KEY_REAUTHS_TAGGED: headscale re-registers a tagged device in place when another user's key logs in anew on its machine; SaaS keeps the tagged device and adds one for that user" loginTakesTaggedDevice = "LOGIN_TAKES_TAGGED_DEVICE: headscale's interactive login gives another user the tagged device on the machine; SaaS's key login keeps it and adds a device for that user" - spentKeyRebindsNode = "SPENT_KEY_REBINDS_NODE: headscale binds a live node to the spent key it logs in with, so an ephemeral node stops being ephemeral and survives logout; SaaS keeps the node as it was" 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" diff --git a/hscontrol/state/auth_tagged_expiry_test.go b/hscontrol/state/auth_tagged_expiry_test.go index 5185992dd..1ac355b32 100644 --- a/hscontrol/state/auth_tagged_expiry_test.go +++ b/hscontrol/state/auth_tagged_expiry_test.go @@ -2779,3 +2779,181 @@ func TestSingleUseKeyPublishesOnlyOneRegistration(t *testing.T) { require.NoError(t, err) 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() + 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) +} diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index ca5883d00..244c5151d 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2808,14 +2808,19 @@ func pakSkipsValidation(n types.NodeView, pak *types.PreAuthKey, nodeKey key.Nod !isOwnershipConversion && !isRetag } -// pakReregisterErr is the authorisation decision for re-registering n with -// pak at now: nil when validation may be skipped, else the key's validity. -func pakReregisterErr(n types.NodeView, pak *types.PreAuthKey, nodeKey key.NodePublic, now time.Time) error { - if pakSkipsValidation(n, pak, nodeKey, now) { - return nil +// 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) { + err := pak.ValidAt(now) + if err == nil { + return true, nil } - return pak.ValidAt(now) + if pakSkipsValidation(n, pak, nodeKey, now) { + return false, nil + } + + return false, err } //nolint:gocyclo // sequential validation/update/create paths with security-sensitive ordering @@ -2850,8 +2855,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 @@ -2862,7 +2869,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) } } @@ -2874,24 +2881,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 @@ -3068,6 +3071,7 @@ func (s *State) reregisterNodeWithPAK( prior *types.Node written *types.Node authErr error + reauth bool ) // Update existing node - NodeStore first, then database @@ -3078,12 +3082,11 @@ func (s *State) reregisterNodeWithPAK( prior = node.Clone() defer func() { written = node.Clone() }() - authErr = pakReregisterErr(node.View(), pak, regReq.NodeKey, now) + reauth, authErr = pakReregister(node.View(), pak, regReq.NodeKey, now) if authErr != nil { return } - node.NodeKey = regReq.NodeKey node.Hostname = hostname // TODO(kradalby): We should ensure we use the same hostinfo and node merge semantics @@ -3093,6 +3096,16 @@ func (s *State) reregisterNodeWithPAK( node.Hostinfo = validHostinfo node.Hostinfo.NetInfo = preserveNetInfo(prior.View(), prior.ID, validHostinfo) + // Preserve online state during re-registration so a live node does + // not appear offline before the client restarts its map stream. + node.LastSeen = new(now) + + if !reauth { + return + } + + node.NodeKey = regReq.NodeKey + node.RegisterMethod = util.RegisterMethodAuthKey // Tags from a PreAuthKey are applied on initial registration and @@ -3134,28 +3147,28 @@ func (s *State) reregisterNodeWithPAK( if !pak.Reusable && !pak.Used { node.AuthKey.Used = true } - // Preserve online state during re-registration so a live node does - // not appear offline before the client restarts its map stream. - node.LastSeen = new(now) - // 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 = ®Req.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 = ®Req.Expiry + case s.cfg.Node.Expiry > 0: + exp := now.Add(s.cfg.Node.Expiry) + node.Expiry = &exp + default: node.Expiry = nil } }) @@ -3178,13 +3191,12 @@ 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. - reregColumns := append(slices.Clone(nodeUpdateColumns), "AuthKeyID") + // 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") + } _, err := s.persistNodeWith(updatedNodeView, reregColumns, func(tx *gorm.DB) error { // Only mark the key used on the *first* registration. On @@ -3192,7 +3204,7 @@ func (s *State) reregisterNodeWithPAK( // atomic compare-and-set in [hsdb.UsePreAuthKey] would otherwise // reject it as "authkey already used", locking out containers // that restart with the same one-shot key. - if !pak.Reusable && !pak.Used { + if reauth && !pak.Reusable && !pak.Used { err := hsdb.UsePreAuthKey(tx, pak) if err != nil { return fmt.Errorf("using pre auth key: %w", err) diff --git a/hscontrol/util/zlog/zf/fields.go b/hscontrol/util/zlog/zf/fields.go index 89da7399e..1f67fc1e1 100644 --- a/hscontrol/util/zlog/zf/fields.go +++ b/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.