diff --git a/hscontrol/servertest/reauth_compat_test.go b/hscontrol/servertest/reauth_compat_test.go index 03c26d2c7..c863cda27 100644 --- a/hscontrol/servertest/reauth_compat_test.go +++ b/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" diff --git a/hscontrol/state/auth_tagged_expiry_test.go b/hscontrol/state/auth_tagged_expiry_test.go index 90f012aa5..1aa1abc8b 100644 --- a/hscontrol/state/auth_tagged_expiry_test.go +++ b/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() + 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 diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 76a3afac6..df459b389 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -2892,22 +2892,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 @@ -2942,8 +2943,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 @@ -2954,7 +2957,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) } } @@ -2966,24 +2969,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 @@ -3244,24 +3243,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 = ®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 } }) @@ -3284,13 +3286,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") 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.