state: apply requested tags on a key login that owns them

A requested tag the key's user owns directly, or the key's tags own one
hop away, replaces the key's tags instead of refusing the login.

hscontrol/servertest/reauth_compat_test.go

@@ -39,35 +39,6 @@ import (
 var reauthSkipReasons = map[string]string{
 	"owner-routes/r-expired-retag/expire/online": expiredPeerOffline,
 
-	"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,
-	"tag-owner-chain/u-base-join/u-base/outcome":      keyLoginTagsRefused,
-	"tag-owner-chain/u-base-join/u-base/expiry":       keyLoginTagsRefused,
-	"tag-owner-chain/u-base-join/u-base/ephemeral":    keyLoginTagsRefused,
-	"tag-owner-chain/u-base-join/u-base/peer-obs":     keyLoginTagsRefused,
-	"tag-owner-chain/u-base-join/u-base/peer-key-obs": keyLoginTagsRefused,
-
-	"tag-owner-chain/t-mid-join/t-mid/devices":      taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/owner":        taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/outcome":      taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/expiry":       taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/ephemeral":    taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/peer-obs":     taggedKeyOwnedTagRefused,
-	"tag-owner-chain/t-mid-join/t-mid/peer-key-obs": taggedKeyOwnedTagRefused,
-
 	"reauth/late-pak-spent-key/late-pak/text":        refusalText,
 	"reauth/late-pak-spent-key/late-pak/legacy-text": refusalText,
 
@@ -78,9 +49,7 @@ var reauthSkipReasons = map[string]string{
 }
 
 const (
-	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"
+	refusalText = "REFUSAL_TEXT: SaaS refuses a key by naming its ID; headscale says why the key is refused"
 )
 
 // reauthNotCompared lists, by key prefix, checks left out: SaaS's answer

hscontrol/state/auth_tagged_expiry_test.go

@@ -1853,7 +1853,7 @@ func (c pakReregCase) reregister(
 	regReq tailcfg.RegisterRequest,
 ) (types.NodeView, error) {
 	hi := regReq.Hostinfo.Clone()
-	node, _, err := c.s.reregisterNodeWithPAK(view, pak, regReq, c.machineKey, hi.Hostname, hi)
+	node, _, err := c.s.reregisterNodeWithPAK(view, pak, nil, regReq, c.machineKey, hi.Hostname, hi)
 
 	return node, err
 }
@@ -3044,10 +3044,70 @@ func TestPAKWakeAcceptsRequestedTags(t *testing.T) {
 	assert.Equal(t, "h1", ns.Hostname)
 
 	req.NodeKey = key.NewNode().Public()
+	req.Hostinfo = &tailcfg.Hostinfo{Hostname: "h1", RequestTags: []string{"tag:other"}}
 	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
 	require.ErrorIs(t, err, ErrRequestedTagsInvalidOrNotPermitted)
 }
 
+// TestPAKLoginAppliesOwnedRequestTags: a key login gets the tags it requests
+// when a user's key's user owns each directly, or a tagged key's tags own it
+// one hop away; the requested tags replace the key's. Anything else is
+// refused.
+func TestPAKLoginAppliesOwnedRequestTags(t *testing.T) {
+	s := newRetagTestState(t)
+	user, _, err := s.CreateUser(types.User{Name: "tag-req-user"})
+	require.NoError(t, err)
+
+	_, err = s.SetPolicy([]byte(`{"tagOwners": {
+		"tag:base": ["tag-req-user@"],
+		"tag:mid":  ["tag:base"],
+		"tag:top":  ["tag:mid"]
+	}}`))
+	require.NoError(t, err)
+
+	userKey, err := s.CreatePreAuthKey(user.TypedID(), true, false, nil, nil)
+	require.NoError(t, err)
+	baseKey, err := s.CreatePreAuthKey(nil, true, false, nil, []string{"tag:base"})
+	require.NoError(t, err)
+
+	join := func(k *types.PreAuthKeyNew, tags ...string) (types.NodeView, error) {
+		node, _, err := s.HandleNodeFromPreAuthKey(tailcfg.RegisterRequest{
+			Auth:     &tailcfg.RegisterResponseAuth{AuthKey: k.Key},
+			NodeKey:  key.NewNode().Public(),
+			Hostinfo: &tailcfg.Hostinfo{Hostname: "h", RequestTags: tags},
+		}, key.NewMachine().Public())
+
+		return node, err
+	}
+
+	node, err := join(userKey, "tag:base")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"tag:base"}, node.Tags().AsSlice())
+	assert.False(t, node.UserID().Valid())
+	assert.False(t, node.Expiry().Valid())
+
+	node, err = join(baseKey, "tag:mid")
+	require.NoError(t, err)
+	assert.Equal(t, []string{"tag:mid"}, node.Tags().AsSlice())
+
+	_, err = join(userKey, "tag:mid")
+	require.ErrorIs(t, err, ErrRequestedTagsInvalidOrNotPermitted, "a user owns only its tags")
+	_, err = join(baseKey, "tag:top")
+	require.ErrorIs(t, err, ErrRequestedTagsInvalidOrNotPermitted, "two hops")
+
+	c := registerForPAKRereg(t, s, userKey)
+	req := c.regReq
+	req.NodeKey = key.NewNode().Public()
+	req.Hostinfo = &tailcfg.Hostinfo{Hostname: "h1", RequestTags: []string{"tag:base"}}
+	_, _, err = s.HandleNodeFromPreAuthKey(req, c.machineKey)
+	require.NoError(t, err)
+
+	ns, row := c.fields(t)
+	assert.Equal(t, []string{"tag:base"}, ns.Tags, "converted in place")
+	assert.Equal(t, []string{"tag:base"}, row.Tags)
+	assert.Nil(t, ns.UserID)
+}
+
 // 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 or gets a device of its own beside it

hscontrol/state/autoapprove_test.go

@@ -199,7 +199,7 @@ func TestRegistrationApprovalComparesReplacedNode(t *testing.T) {
 
 	req := c.regReq
 	req.Hostinfo = hi.Clone()
-	node, replaced, err := s.reregisterNodeWithPAK(stale, stalePAK, req, c.machineKey, hi.Hostname, req.Hostinfo)
+	node, replaced, err := s.reregisterNodeWithPAK(stale, stalePAK, nil, req, c.machineKey, hi.Hostname, req.Hostinfo)
 	require.NoError(t, err)
 
 	_, _, err = s.approveRoutesAfterRegistration(replaced, node, change.Change{})

hscontrol/state/state.go

@@ -2174,6 +2174,8 @@ type newNodeParams struct {
 
 	// Optional: Pre-auth key specific fields
 	PreAuthKey *types.PreAuthKey
+	// Tags the key login was allowed to request; they replace the key's.
+	Tags []string
 
 	// Optional: Existing node for netinfo preservation
 	ExistingNodeForNetinfo types.NodeView
@@ -2455,10 +2457,15 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 
 	// Assign ownership based on PreAuthKey
 	if params.PreAuthKey != nil {
-		if params.PreAuthKey.IsTagged() {
+		tags := params.PreAuthKey.Tags
+		if params.Tags != nil {
+			tags = params.Tags
+		}
+
+		if len(tags) > 0 {
 			// Tagged nodes are owned by their tags, not a user.
 			// UserID is intentionally left nil.
-			nodeToRegister.Tags = params.PreAuthKey.Tags
+			nodeToRegister.Tags = tags
 
 			// Tagged nodes have key expiry disabled.
 			nodeToRegister.Expiry = nil
@@ -3099,24 +3106,35 @@ 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, ahead of the key's own error, so new nodes and
+	// and the tailscale client's OAuth authkey flow always does it. Other
+	// requested tags replace the key's when a tagged key's tags own each one,
+	// or a user's key's user owns it directly, as on Tailscale SaaS; otherwise
+	// they are refused, ahead of the key's own error, so new nodes and
 	// re-registrations are held to the same rule.
-	var tagsErr error
+	var (
+		requestedTags []string
+		tagsErr       error
+	)
 
-	if regReq.Hostinfo != nil {
-		var extraTags []string
+	if regReq.Hostinfo != nil && slices.ContainsFunc(regReq.Hostinfo.RequestTags, func(tag string) bool {
+		return !slices.Contains(pak.Tags, tag)
+	}) {
+		var refused []string
 
 		for _, tag := range regReq.Hostinfo.RequestTags {
-			if !slices.Contains(pak.Tags, tag) {
-				extraTags = append(extraTags, tag)
+			owned := s.polMan.TagOwnedByTags(tag, pak.Tags) ||
+				(!pak.IsTagged() && pak.User != nil && s.polMan.UserCanHaveTag(pak.User.View(), tag))
+			if !owned {
+				refused = append(refused, tag)
 			}
 		}
 
-		if len(extraTags) > 0 {
-			tagsErr = fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, extraTags)
+		if len(refused) > 0 {
+			tagsErr = fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, refused)
+		} else {
+			// Sort a copy: RequestTags is the node's reported Hostinfo.
+			requestedTags = slices.Compact(slices.Sorted(slices.Values(regReq.Hostinfo.RequestTags)))
 		}
 	}
 
@@ -3175,7 +3193,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 	// a tagged node (findExistingNodeForPAK matches it under UserID 0); for a
 	// user-owned key it is true when the same user already has the node.
 	if existsSameUser && existingNodeSameUser.Valid() {
-		finalNode, before, err = s.reregisterNodeWithPAK(existingNodeSameUser, pak, regReq, machineKey, hostname, validHostinfo)
+		finalNode, before, err = s.reregisterNodeWithPAK(existingNodeSameUser, pak, requestedTags, regReq, machineKey, hostname, validHostinfo)
 		if err != nil {
 			return types.NodeView{}, s.policyChangeSince(genBefore), err
 		}
@@ -3254,6 +3272,7 @@ func (s *State) HandleNodeFromPreAuthKey(
 			Expiry:                 reqExpiry,
 			RegisterMethod:         util.RegisterMethodAuthKey,
 			PreAuthKey:             pak,
+			Tags:                   requestedTags,
 			ExistingNodeForNetinfo: differentUserNode,
 		})
 		if err != nil {
@@ -3316,6 +3335,7 @@ func (s *State) approveRoutesAfterRegistration(
 func (s *State) reregisterNodeWithPAK(
 	existingNodeSameUser types.NodeView,
 	pak *types.PreAuthKey,
+	requestedTags []string,
 	regReq tailcfg.RegisterRequest,
 	machineKey key.MachinePublic,
 	hostname string,
@@ -3397,10 +3417,19 @@ func (s *State) reregisterNodeWithPAK(
 		// node.AuthKeyID still holds the prior key's ID here (it is reassigned
 		// below), and SetNodeTags leaves AuthKeyID intact, so an admin retag
 		// cannot masquerade as a new key.
-		if pak.ConvertsNodeToTagged(node.View()) || pak.RetagsNode(node.View()) {
+		//
+		// Tags the login was allowed to request replace the node's instead.
+		tags := pak.Tags
+		retag := pak.ConvertsNodeToTagged(node.View()) || pak.RetagsNode(node.View())
+
+		if requestedTags != nil {
+			tags, retag = requestedTags, !slices.Equal(node.Tags, requestedTags)
+		}
+
+		if retag {
 			wasUserOwned := !node.IsTagged()
 
-			node.Tags = pak.Tags
+			node.Tags = tags
 			node.UserID = nil
 			node.User = nil
 
@@ -3414,7 +3443,8 @@ func (s *State) reregisterNodeWithPAK(
 			if wasUserOwned || node.IsExpiredAt(now) {
 				node.Expiry = nil
 			}
-		} else if node.IsTagged() && !pak.IsTagged() && prior.NodeKey != regReq.NodeKey {
+		} else if requestedTags == nil && node.IsTagged() && !pak.IsTagged() &&
+			prior.NodeKey != regReq.NodeKey {
 			// A user key that speaks for a tagged node, with a new node key,
 			// takes it back as that user's device, as on Tailscale SaaS. A
 			// relogin under the same node key keeps it tagged.

integration/tags_test.go

@@ -913,13 +913,15 @@ func TestTagsAuthKeyWithTagCLICannotModifyAdminTags(t *testing.T) {
 // Test Suite 3: Auth Key WITHOUT Tags
 // =============================================================================
 
-// TestTagsAuthKeyWithoutTagCannotRequestTags tests that nodes cannot request tags
-// when using an auth key that has no tags.
+// TestTagsAuthKeyWithoutTagCanRequestOwnedTag tests that a node registering
+// with a user's auth key that has no tags gets the tags it requests when the
+// user owns them, as on Tailscale SaaS. Tags the user does not own are still
+// refused (Test 3.8).
 //
-// Test 3.1: Cannot request tags with tagless key
+// Test 3.1: Request an owned tag with a tagless key
 // Setup: Run `tailscale up --advertise-tags="tag:valid-owned" --auth-key AUTH_KEY_WITHOUT_TAG`
-// Expected: Registration fails with error containing "requested tags [tag:valid-owned] are invalid or not permitted".
-func TestTagsAuthKeyWithoutTagCannotRequestTags(t *testing.T) {
+// Expected: Registration succeeds and the node is tagged tag:valid-owned.
+func TestTagsAuthKeyWithoutTagCanRequestOwnedTag(t *testing.T) {
 	IntegrationSkip(t)
 
 	policy := tagsTestPolicy()
@@ -962,26 +964,18 @@ func TestTagsAuthKeyWithoutTagCannotRequestTags(t *testing.T) {
 	)
 	require.NoError(t, err)
 
-	// Login should fail because the auth key has no tags
 	err = client.Login(headscale.GetEndpoint(), authKey.Key)
-	if err != nil {
-		t.Logf("Test 3.1 PASS: Registration correctly rejected: %v", err)
-		assert.ErrorContains(t, err, "requested tags")
-	} else {
-		// If it succeeded, document this unexpected behavior
-		t.Logf("Test 3.1 UNEXPECTED: Registration succeeded when it should have failed")
+	require.NoError(t, err)
 
-		assert.EventuallyWithT(t, func(c *assert.CollectT) {
-			nodes, err := headscale.ListNodes()
-			assert.NoError(c, err)
+	assert.EventuallyWithT(t, func(c *assert.CollectT) {
+		nodes, err := headscale.ListNodes()
+		assert.NoError(c, err)
+		assert.Len(c, nodes, 1)
 
-			if len(nodes) == 1 {
-				t.Logf("Node registered with tags: %v (expected rejection)", nodes[0].Tags)
-			}
-		}, integrationutil.ScaledTimeout(10*time.Second), integrationutil.SlowPoll, "checking node state")
-
-		t.Fail()
-	}
+		if len(nodes) == 1 {
+			assertNodeHasTagsWithCollect(c, nodes[0], []string{"tag:valid-owned"})
+		}
+	}, integrationutil.ScaledTimeout(10*time.Second), integrationutil.SlowPoll, "node should carry the owned tag it requested")
 }
 
 // TestTagsAuthKeyWithoutTagRegisterNoTags tests that registering with a tagless auth key