state: accept advertise-tags subset of a pre-auth key's tags

tailscale client's OAuth authkey flow re-advertises the key's tags. Reject only tags the key lacks, for new nodes and re-registrations.
2026-09-26 02:04:53 +09:00 · 2026-06-27 10:12:29 +00:00
parent 69e84c356d
commit 61a626eab7
3 changed files with 249 additions and 12 deletions
@@ -50,6 +50,7 @@ func TestAuthenticationFlows(t *testing.T) {
 		machineKey  func() key.MachinePublic
 		wantAuth    bool
 		wantError   bool
+		wantErrMsg  string
 		wantAuthURL bool
 		wantExpired bool
 		validate    func(*testing.T, *tailcfg.RegisterResponse, *Headscale)
@@ -958,11 +959,12 @@ func TestAuthenticationFlows(t *testing.T) {
 		// === ADVERTISE-TAGS (RequestTags) SCENARIOS ===
 		// Tests for client-provided tags via --advertise-tags flag

-		// TEST: PreAuthKey registration rejects client-provided RequestTags
-		// WHAT: Tests that PreAuthKey registrations cannot use client-provided tags
-		// INPUT: PreAuthKey registration with [tailcfg.Hostinfo.RequestTags] set
+		// TEST: Untagged PreAuthKey registration rejects client-provided RequestTags
+		// WHAT: Tests that PreAuthKey registrations cannot claim tags the key lacks
+		// INPUT: Untagged PreAuthKey registration with [tailcfg.Hostinfo.RequestTags] set
 		// EXPECTED: Registration fails with "requested tags [...] are invalid or not permitted" error
-		// WHY: PreAuthKey nodes get their tags from the key itself, not from client requests
+		// WHY: PreAuthKey nodes get their tags from the key; only a subset of the
+		// key's own tags may be advertised
 		{
 			name: "preauth_key_rejects_request_tags",
 			setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper
@@ -1031,6 +1033,222 @@ func TestAuthenticationFlows(t *testing.T) {
 			wantError:  true, // RequestTags rejected for PreAuthKey registrations
 		},

+		// TEST: Tagged PreAuthKey accepts RequestTags that are a subset of the key's tags
+		// WHAT: The tailscale client's OAuth authkey flow re-advertises the key's
+		// own tags via --advertise-tags.
+		// INPUT: Tagged PreAuthKey ([tag:authorized, tag:other]) with RequestTags [tag:authorized]
+		// EXPECTED: Registration succeeds; node carries all of the key's tags.
+		// WHY: Advertising tags the key already grants is not an escalation; tags
+		// still come from the key, not the request.
+		{
+			name: "tagged_preauth_key_accepts_subset_request_tags",
+			setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper
+				t.Helper()
+
+				user := app.state.CreateUserForTest("tagged-pak-subsettags-user")
+
+				pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, []string{"tag:authorized", "tag:other"})
+				if err != nil {
+					return "", err
+				}
+
+				return pak.Key, nil
+			},
+			request: func(authKey string) tailcfg.RegisterRequest {
+				return tailcfg.RegisterRequest{
+					Auth: &tailcfg.RegisterResponseAuth{
+						AuthKey: authKey,
+					},
+					NodeKey: nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{
+						Hostname:    "tagged-pak-subsettags-node",
+						RequestTags: []string{"tag:authorized"},
+					},
+					Expiry: time.Now().Add(24 * time.Hour),
+				}
+			},
+			machineKey: machineKey1.Public,
+			wantAuth:   true,
+			validate: func(t *testing.T, _ *tailcfg.RegisterResponse, app *Headscale) {
+				t.Helper()
+
+				node, found := app.state.GetNodeByNodeKey(nodeKey1.Public())
+				require.True(t, found)
+				assert.True(t, node.IsTagged())
+				assert.ElementsMatch(t, []string{"tag:authorized", "tag:other"}, node.Tags().AsSlice())
+				assert.False(t, node.UserID().Valid(), "tagged node must not be user-owned")
+			},
+		},
+
+		// TEST: Tagged PreAuthKey rejects RequestTags that go beyond the key's tags
+		// WHAT: A request mixing a key tag with a foreign tag
+		// INPUT: Tagged PreAuthKey ([tag:authorized]) with RequestTags [tag:authorized, tag:client-wants-this]
+		// EXPECTED: Registration fails naming only the foreign tag
+		// WHY: One matching tag must not smuggle in others
+		{
+			name: "tagged_preauth_key_rejects_partially_foreign_request_tags",
+			setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper
+				t.Helper()
+
+				user := app.state.CreateUserForTest("tagged-pak-mixedtags-user")
+
+				pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, []string{"tag:authorized"})
+				if err != nil {
+					return "", err
+				}
+
+				return pak.Key, nil
+			},
+			request: func(authKey string) tailcfg.RegisterRequest {
+				return tailcfg.RegisterRequest{
+					Auth: &tailcfg.RegisterResponseAuth{
+						AuthKey: authKey,
+					},
+					NodeKey: nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{
+						Hostname:    "tagged-pak-mixedtags-node",
+						RequestTags: []string{"tag:authorized", "tag:client-wants-this"},
+					},
+					Expiry: time.Now().Add(24 * time.Hour),
+				}
+			},
+			machineKey: machineKey1.Public,
+			wantError:  true,
+			wantErrMsg: "[tag:client-wants-this] are invalid",
+		},
+
+		// TEST: Existing tagged node re-registering with foreign RequestTags is rejected
+		// WHAT: Same machine and node key re-run `tailscale up --authkey` with a tag
+		// the key does not carry, hitting the skip-validation re-registration path
+		// INPUT: Node registered with tagged PreAuthKey ([tag:authorized]), then the
+		// same key with RequestTags [tag:client-wants-this]
+		// EXPECTED: Re-registration fails
+		// WHY: New and existing nodes are held to the same advertise-tags rule
+		{
+			name: "existing_tagged_node_reregister_rejects_foreign_request_tags",
+			setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper
+				t.Helper()
+
+				user := app.state.CreateUserForTest("tagged-pak-rereg-user")
+
+				pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, []string{"tag:authorized"})
+				if err != nil {
+					return "", err
+				}
+
+				_, err = app.handleRegisterWithAuthKey(tailcfg.RegisterRequest{
+					Auth:     &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+					NodeKey:  nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{Hostname: "tagged-pak-rereg-node"},
+					Expiry:   time.Now().Add(24 * time.Hour),
+				}, machineKey1.Public())
+				if err != nil {
+					return "", err
+				}
+
+				require.EventuallyWithT(t, func(c *assert.CollectT) {
+					_, found := app.state.GetNodeByNodeKey(nodeKey1.Public())
+					assert.True(c, found)
+				}, 1*time.Second, 50*time.Millisecond)
+
+				return pak.Key, nil
+			},
+			request: func(authKey string) tailcfg.RegisterRequest {
+				return tailcfg.RegisterRequest{
+					Auth: &tailcfg.RegisterResponseAuth{
+						AuthKey: authKey,
+					},
+					NodeKey: nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{
+						Hostname:    "tagged-pak-rereg-node",
+						RequestTags: []string{"tag:client-wants-this"},
+					},
+					Expiry: time.Now().Add(24 * time.Hour),
+				}
+			},
+			machineKey: machineKey1.Public,
+			wantError:  true,
+			wantErrMsg: "[tag:client-wants-this] are invalid",
+		},
+
+		// TEST: Existing tagged node re-registers with a spent single-use key advertising a subset
+		// WHAT: Container restart re-running `tailscale up --authkey` with the key's
+		// own tag after an admin retagged the node
+		// INPUT: Single-use PreAuthKey ([tag:authorized, tag:other]) registers, admin
+		// sets tags [tag:admin], same key and node key re-register with RequestTags [tag:authorized]
+		// EXPECTED: Re-registration succeeds; admin's tags preserved
+		// WHY: The subset check must not break the same-key restart path, and the
+		// same key must not undo an admin override
+		{
+			name: "existing_tagged_node_reregister_spent_key_subset_tags_keeps_admin_tags",
+			setupFunc: func(t *testing.T, app *Headscale) (string, error) { //nolint:thelper
+				t.Helper()
+
+				_, err := app.state.SetPolicy([]byte(`{"tagOwners":{"tag:admin":[],"tag:authorized":[],"tag:other":[]}}`))
+				require.NoError(t, err)
+
+				user := app.state.CreateUserForTest("tagged-pak-spent-user")
+
+				pak, err := app.state.CreatePreAuthKey(user.TypedID(), false, false, nil, []string{"tag:authorized", "tag:other"})
+				if err != nil {
+					return "", err
+				}
+
+				_, err = app.handleRegisterWithAuthKey(tailcfg.RegisterRequest{
+					Auth:    &tailcfg.RegisterResponseAuth{AuthKey: pak.Key},
+					NodeKey: nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{
+						Hostname:    "tagged-pak-spent-node",
+						RequestTags: []string{"tag:authorized"},
+					},
+					Expiry: time.Now().Add(24 * time.Hour),
+				}, machineKey1.Public())
+				if err != nil {
+					return "", err
+				}
+
+				var node types.NodeView
+
+				require.EventuallyWithT(t, func(c *assert.CollectT) {
+					var found bool
+
+					node, found = app.state.GetNodeByNodeKey(nodeKey1.Public())
+					assert.True(c, found)
+				}, 1*time.Second, 50*time.Millisecond)
+
+				_, _, err = app.state.SetNodeTags(node.ID(), []string{"tag:admin"})
+				require.NoError(t, err)
+
+				spent, err := app.state.GetPreAuthKey(pak.Key)
+				require.NoError(t, err)
+				require.True(t, spent.Used, "precondition: single-use key must be spent")
+
+				return pak.Key, nil
+			},
+			request: func(authKey string) tailcfg.RegisterRequest {
+				return tailcfg.RegisterRequest{
+					Auth: &tailcfg.RegisterResponseAuth{
+						AuthKey: authKey,
+					},
+					NodeKey: nodeKey1.Public(),
+					Hostinfo: &tailcfg.Hostinfo{
+						Hostname:    "tagged-pak-spent-node",
+						RequestTags: []string{"tag:authorized"},
+					},
+					Expiry: time.Now().Add(24 * time.Hour),
+				}
+			},
+			machineKey: machineKey1.Public,
+			wantAuth:   true,
+			validate: func(t *testing.T, _ *tailcfg.RegisterResponse, app *Headscale) {
+				t.Helper()
+
+				node, found := app.state.GetNodeByNodeKey(nodeKey1.Public())
+				require.True(t, found)
+				assert.Equal(t, []string{"tag:admin"}, node.Tags().AsSlice())
+			},
+		},
+
 		// === RE-AUTHENTICATION SCENARIOS ===
 		// TEST: Existing node re-authenticates with new pre-auth key
 		// WHAT: Tests that existing node can re-authenticate using new pre-auth key
@@ -2534,6 +2752,11 @@ func TestAuthenticationFlows(t *testing.T) {
 			// Validate error expectations
 			if tt.wantError {
 				assert.Error(t, err, "expected error but got none")
+
+				if tt.wantErrMsg != "" {
+					require.ErrorContains(t, err, tt.wantErrMsg)
+				}
+
 				return
 			}

@@ -2574,6 +2797,7 @@ func runInteractiveWorkflowTest(t *testing.T, tt struct {
 	machineKey                func() key.MachinePublic
 	wantAuth                  bool
 	wantError                 bool
+	wantErrMsg                string
 	wantAuthURL               bool
 	wantExpired               bool
 	validate                  func(*testing.T, *tailcfg.RegisterResponse, *Headscale)
@@ -2024,12 +2024,6 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
 		nodeToRegister.Tags = nil
 	}

-	// Reject advertise-tags for PreAuthKey registrations early, before any resource allocation.
-	// PreAuthKey nodes get their tags from the key itself, not from client requests.
-	if params.PreAuthKey != nil && params.Hostinfo != nil && len(params.Hostinfo.RequestTags) > 0 {
-		return types.NodeView{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, params.Hostinfo.RequestTags)
-	}
-
 	// Process RequestTags (from tailscale up --advertise-tags) ONLY for non-PreAuthKey registrations.
 	// Validate early before IP allocation to avoid resource leaks on failure.
 	if params.PreAuthKey == nil && params.Hostinfo != nil && len(params.Hostinfo.RequestTags) > 0 {
@@ -2542,6 +2536,25 @@ func (s *State) HandleNodeFromPreAuthKey(
 		return types.NodeView{}, change.Change{}, err
 	}

+	// 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.
+	if regReq.Hostinfo != nil {
+		var extraTags []string
+
+		for _, tag := range regReq.Hostinfo.RequestTags {
+			if !slices.Contains(pak.Tags, tag) {
+				extraTags = append(extraTags, tag)
+			}
+		}
+
+		if len(extraTags) > 0 {
+			return types.NodeView{}, change.Change{}, fmt.Errorf("%w %v are invalid or not permitted", ErrRequestedTagsInvalidOrNotPermitted, extraTags)
+		}
+	}
+
 	// Helper to get username for logging (handles nil User for tags-only keys)
 	pakUsername := func() string {
 		if pak.User != nil {
@@ -3234,8 +3234,8 @@ func TestTagsAuthKeyWithoutUserInheritsTags(t *testing.T) {
 }

 // TestTagsAuthKeyWithoutUserRejectsAdvertisedTags tests that when an auth key without
-// a user (tags-only) is used WITH --advertise-tags, the registration is rejected.
-// PreAuthKey registrations do not allow client-requested tags.
+// a user (tags-only) is used WITH --advertise-tags naming a tag the key lacks, the
+// registration is rejected. Only a subset of the key's own tags may be advertised.
 //
 // Test 5.2: Auth key without user, with --advertise-tags (should be rejected)
 // Setup: Run `tailscale up --advertise-tags="tag:second" --auth-key AUTH_KEY_WITH_TAGS_NO_USER`