From fab5833a6a2b8518b5cf5be2fc1f412325243507 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 15:36:43 +0000 Subject: [PATCH] api/v2: keep a tagged all-scope client's tokens inside its tags Only an untagged all-scope client may mint tokens for any tag, so a tagged token may not create one. --- hscontrol/api/v2/keys.go | 10 ++++++++ hscontrol/api/v2/oauth.go | 11 +++++---- hscontrol/apiv2_compat_test.go | 6 ----- hscontrol/apiv2_oauth_matrix_test.go | 11 ++++++--- hscontrol/apiv2_oauth_test.go | 36 ++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/hscontrol/api/v2/keys.go b/hscontrol/api/v2/keys.go index 552109136..64b53ab18 100644 --- a/hscontrol/api/v2/keys.go +++ b/hscontrol/api/v2/keys.go @@ -3,6 +3,7 @@ package apiv2 import ( "context" "net/http" + "slices" "strconv" "time" @@ -430,6 +431,15 @@ func createOAuthClient(ctx context.Context, b Backend, body CreateKeyRequest) (* tokenTags, _ := principalTags(ctx) + // An untagged "all" client may mint tokens for any tag (narrowTags), so a + // tag-limited token would escape its tags one client away. + if len(tokenTags) > 0 && len(body.Tags) == 0 && + slices.Contains(body.Scopes, string(scope.All)) { + return nil, huma.Error403Forbidden( + "client may not be granted scope all without tags by a tagged token", + ) + } + for _, tag := range body.Tags { if !b.State.TagExists(tag) { return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy") diff --git a/hscontrol/api/v2/oauth.go b/hscontrol/api/v2/oauth.go index 330c94f96..897e53dad 100644 --- a/hscontrol/api/v2/oauth.go +++ b/hscontrol/api/v2/oauth.go @@ -90,7 +90,7 @@ func oauthTokenHandler(b Backend) http.HandlerFunc { tags, badTag, ok := narrowTags(client, strings.Fields(r.PostForm.Get("tags"))) if !ok { writeOAuthError(w, http.StatusBadRequest, "invalid_target", - "tag "+badTag+" is not granted to this client") + "requested tags ["+badTag+"] are invalid or not permitted") return } @@ -133,24 +133,25 @@ func narrowScopes(granted, requested []string) ([]string, string, bool) { // narrowTags returns the requested tags if each is within the client's grant (an // empty request means "the client's full tag set"), otherwise the offending tag -// and false. A client with the "all" scope may assign any tag, matching Tailscale. +// and false. Only an untagged client with the "all" scope may assign any tag; +// a tagged one stays inside its tags, matching Tailscale. func narrowTags(client *types.OAuthClient, requested []string) ([]string, string, bool) { if len(requested) == 0 { return client.Tags, "", true } - allScope := slices.Contains(client.Scopes, string(scope.All)) + anyTag := len(client.Tags) == 0 && slices.Contains(client.Scopes, string(scope.All)) for _, req := range requested { // Reject malformed tags at the trust boundary. A client's own tags are - // validated at creation, so this only matters for an "all"-scope client, + // validated at creation, so this only matters for an any-tag client, // whose tags would otherwise skip the membership check below and flow // unvalidated into auth-key creation. if !strings.HasPrefix(req, "tag:") { return nil, req, false } - if !allScope && !slices.Contains(client.Tags, req) { + if !anyTag && !slices.Contains(client.Tags, req) { return nil, req, false } } diff --git a/hscontrol/apiv2_compat_test.go b/hscontrol/apiv2_compat_test.go index 7f230c9ae..0d2d0e6c5 100644 --- a/hscontrol/apiv2_compat_test.go +++ b/hscontrol/apiv2_compat_test.go @@ -83,11 +83,6 @@ var apiv2CompatSkipReasons = map[string]string{ "oauth/J3/1/detail": refusalDetail, "oauth/J4/0/detail": refusalDetail, - "oauth/E2/0/status": taggedAllClientAnyTag, - "oauth/E2/0/outcome": taggedAllClientAnyTag, - "oauth/E9a/1/status": taggedAllClientAnyTag, - "oauth/E9a/1/outcome": taggedAllClientAnyTag, - "oauth/E4/0/status": tagRefusalStatus, "oauth/E5/0/status": tagRefusalStatus, "oauth/T2/1/status": tagRefusalStatus, @@ -225,7 +220,6 @@ func notCompared(prefixes []string, key string) bool { const ( scopeEchoAsSent = "SCOPE_ECHO_AS_SENT: SaaS echoes a token's scopes sorted and without repeats; headscale echoes the request as sent" bodyBeforeScope = "BODY_BEFORE_SCOPE: headscale validates a createKey body before the token's scope and answers 422; SaaS answers 404 for the missing scope" - taggedAllClientAnyTag = "TAGGED_ALL_CLIENT_ANY_TAG: SaaS lets a tagged all-scope client's token ask only for the client's tags; headscale lets it ask for any tag" retagRemovesUnownedTag = "RETAG_REMOVES_UNOWNED_TAG: SaaS refuses a token's retag of a device holding a tag outside the token's grant with 400 cannot remove current tags; headscale replaces the tags" ) diff --git a/hscontrol/apiv2_oauth_matrix_test.go b/hscontrol/apiv2_oauth_matrix_test.go index 3e71d730f..49df9a683 100644 --- a/hscontrol/apiv2_oauth_matrix_test.go +++ b/hscontrol/apiv2_oauth_matrix_test.go @@ -183,7 +183,7 @@ func TestAPIv2OAuthMatrix_ScopeNarrowing(t *testing.T) { // TestAPIv2OAuthMatrix_TagNarrowing proves P1 for tags at mint time: a client may // only mint a token for tags within its grant (closing the /oauth/token tags-param -// path). A client with the "all" scope may request any tag. +// path). Only an untagged client with the "all" scope may request any tag. func TestAPIv2OAuthMatrix_TagNarrowing(t *testing.T) { _, baseURL, admin := newOAuthTestServer(t) @@ -202,10 +202,15 @@ func TestAPIv2OAuthMatrix_TagNarrowing(t *testing.T) { assert.Equal(t, http.StatusBadRequest, st, mm) assert.Equal(t, "invalid_target", mm["error"]) - // An all-scope client may request any tag. + // A tagged all-scope client stays inside its tags; an untagged one may + // request any tag. _, allSecret := createClient(t, baseURL, admin, []string{"all"}, []string{"tag:a"}) stAll, mAll := mintTokenWithTags(t, baseURL, allSecret, "tag:anything") - assert.Equalf(t, http.StatusOK, stAll, "all-scope client may assign any tag: %v", mAll) + assert.Equalf(t, http.StatusBadRequest, stAll, "tagged all-scope client: %v", mAll) + + _, anySecret := createClient(t, baseURL, admin, []string{"all"}, nil) + stAny, mAny := mintTokenWithTags(t, baseURL, anySecret, "tag:anything") + assert.Equalf(t, http.StatusOK, stAny, "untagged all-scope client may assign any tag: %v", mAny) } // TestAPIv2OAuthMatrix_Lifecycle proves expired and revoked credentials are denied diff --git a/hscontrol/apiv2_oauth_test.go b/hscontrol/apiv2_oauth_test.go index 70601fc3f..82063c8fe 100644 --- a/hscontrol/apiv2_oauth_test.go +++ b/hscontrol/apiv2_oauth_test.go @@ -288,6 +288,42 @@ func TestAPIv2OAuth_ClientManagementScopes(t *testing.T) { assert.Equal(t, http.StatusForbidden, statusDeny, "auth_keys (no oauth_keys) cannot create a client") } +// TestAPIv2OAuth_TaggedAllClientStaysInTags asserts an all-scope client with +// tags mints tokens only inside its tags, and that a tagged token cannot +// escape them through an untagged all-scope child, whose tokens may take any +// tag. +func TestAPIv2OAuth_TaggedAllClientStaysInTags(t *testing.T) { + _, baseURL, admin := newOAuthTestServer(t) + + _, secret := createClient(t, baseURL, admin, []string{"all"}, []string{"tag:a"}) + + st, m := mintTokenWithTags(t, baseURL, secret, "tag:b") + assert.Equalf(t, http.StatusBadRequest, st, "%v", m) + assert.Equal(t, "requested tags [tag:b] are invalid or not permitted", m["error_description"]) + + tok := tokenFor(t, baseURL, secret) + assert.Equal(t, http.StatusForbidden, + createTaggedKey(t, baseURL, tok, []string{"tag:b"}), + "the token is held to its tags") + + // A tagged child stays inside its tags. + _, childSecret := createClient(t, baseURL, tok, []string{"all"}, []string{"tag:a"}) + st, m = mintTokenWithTags(t, baseURL, childSecret, "tag:b") + assert.Equalf(t, http.StatusBadRequest, st, "tagged child: %v", m) + + createClient(t, baseURL, tok, []string{"auth_keys", "oauth_keys"}, []string{"tag:a"}) + + st, body := apiPost(t, baseURL+"/api/v2/tailnet/-/keys", tok, + apiv2.CreateKeyRequest{KeyType: "client", Scopes: []string{"all"}}) + assert.Equalf(t, http.StatusForbidden, st, "untagged all-scope child: %s", body) + + // An admin API key still creates an untagged all-scope client, whose + // tokens may take any tag. + _, anySecret := createClient(t, baseURL, admin, []string{"all"}, nil) + st, m = mintTokenWithTags(t, baseURL, anySecret, "tag:b") + assert.Equalf(t, http.StatusOK, st, "%v", m) +} + func TestAPIv2OAuth_TagOwnedBy(t *testing.T) { app, baseURL, admin := newOAuthTestServer(t)