From 4e946edb08f8a6f7c96294e171f15ff28136922e Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Thu, 22 Oct 2026 01:44:43 +0000 Subject: [PATCH] api/v2: enforce and document OAuth tag permissions Retagging requires all current and requested tags; disallowed tags return 400. Token responses list tags only when the token may assign them. --- docs/ref/api.md | 11 ++++++ hscontrol/api/v2/devices.go | 15 +++++--- hscontrol/api/v2/errors.go | 21 +++++++++++ hscontrol/api/v2/keys.go | 16 +++----- hscontrol/api/v2/oauth.go | 15 +++++++- hscontrol/apiv2_compat_test.go | 37 ++----------------- hscontrol/apiv2_oauth_test.go | 34 ++++++++++++++--- .../servertest/apiv2_oauth_scopes_test.go | 4 +- 8 files changed, 95 insertions(+), 58 deletions(-) diff --git a/docs/ref/api.md b/docs/ref/api.md index 858aa1d1b..0c4741baf 100644 --- a/docs/ref/api.md +++ b/docs/ref/api.md @@ -78,6 +78,17 @@ secret is shown once: headscale oauth-clients create --scope auth_keys --tag tag:ci ``` +Scopes apply to the whole tailnet, as in Tailscale: an `auth_keys` token can read +and revoke auth keys of any tag, an `oauth_keys` token every OAuth client, and a +`devices:core:read` token reads every device, routes included with `fields=all`. +Keep the secret as safe as any credential that can revoke keys. Without +`all=true`, a token lists only the auth keys made through a token, whatever their +tags; with it, every valid auth key. No listing holds a spent, expired or revoked +key. Tags limit only the tags a token puts on keys, clients and devices, and +which tagged devices it may retag. Deleting an OAuth client revokes its secret +and its access tokens; auth keys, clients and nodes created with it stay until +you remove them. + Build the auth key from the secret: 1. Swap the prefix: `hskey-client-…` becomes `tskey-client-…`. The client only diff --git a/hscontrol/api/v2/devices.go b/hscontrol/api/v2/devices.go index a5fbe4547..2eba40215 100644 --- a/hscontrol/api/v2/devices.go +++ b/hscontrol/api/v2/devices.go @@ -285,12 +285,17 @@ func registerDevices(api huma.API, b Backend) { // owned by a held tag per policy); an admin API key is unrestricted. The // devices:core scope alone must not let a token stamp an arbitrary policy // tag (e.g. tag:prod) onto any node. SetNodeTags still enforces that each - // tag exists in policy. + // tag exists in policy. SetNodeTags replaces the set, so the token must + // also own every tag the device holds; as on Tailscale SaaS, the refusal + // then names them all. if tokenTags, isOAuth := principalTags(ctx); isOAuth { - for _, tag := range in.Body.Tags { - if !b.State.TagOwnedByTags(tag, tokenTags) { - return nil, huma.Error403Forbidden("token may not assign tag " + tag) - } + current := node.Tags().AsSlice() + if len(unownedTags(b, current, tokenTags)) > 0 { + return nil, tagsRefused("cannot remove current tags: ", current) + } + + if refused := unownedTags(b, in.Body.Tags, tokenTags); len(refused) > 0 { + return nil, tagsRefused("", refused) } } diff --git a/hscontrol/api/v2/errors.go b/hscontrol/api/v2/errors.go index 00e28f71a..3bfec76ce 100644 --- a/hscontrol/api/v2/errors.go +++ b/hscontrol/api/v2/errors.go @@ -2,6 +2,7 @@ package apiv2 import ( "errors" + "fmt" "net/http" "strconv" "strings" @@ -120,6 +121,26 @@ func policyRefusal(err error) (string, []apiErrorData) { return "test(s) failed", data } +// unownedTags returns the tags a token holding tokenTags may not apply. +func unownedTags(b Backend, tags, tokenTags []string) []string { + var out []string + + for _, tag := range tags { + if !b.State.TagOwnedByTags(tag, tokenTags) { + out = append(out, tag) + } + } + + return out +} + +// tagsRefused is Tailscale's 400 for tags a token may not apply. +func tagsRefused(prefix string, tags []string) error { + return huma.Error400BadRequest( + fmt.Sprintf("%srequested tags %v are invalid or not permitted", prefix, tags), + ) +} + // mapError translates a state/db-layer error into a Huma HTTP error // (not-found→404, invalid input→400, everything else→500). The transformer // then reshapes it into the Tailscale body. diff --git a/hscontrol/api/v2/keys.go b/hscontrol/api/v2/keys.go index cf5fe953f..bd4a9ec6a 100644 --- a/hscontrol/api/v2/keys.go +++ b/hscontrol/api/v2/keys.go @@ -393,12 +393,10 @@ func createAuthKey(ctx context.Context, b Backend, body CreateKeyRequest) (*keyO if !b.State.TagExists(tag) { return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy") } + } - if !b.State.TagOwnedByTags(tag, tokenTags) { - return nil, huma.Error403Forbidden( - "token may not assign tag " + tag, - ) - } + if refused := unownedTags(b, create.Tags, tokenTags); len(refused) > 0 { + return nil, tagsRefused("", refused) } } @@ -496,12 +494,10 @@ func createOAuthClient(ctx context.Context, b Backend, body CreateKeyRequest) (* if !b.State.TagExists(tag) { return nil, huma.Error400BadRequest("tag " + tag + " is not defined in policy") } + } - if !b.State.TagOwnedByTags(tag, tokenTags) { - return nil, huma.Error403Forbidden( - "client may not be granted tag " + tag + " beyond the creating token", - ) - } + if refused := unownedTags(b, body.Tags, tokenTags); len(refused) > 0 { + return nil, tagsRefused("", refused) } } diff --git a/hscontrol/api/v2/oauth.go b/hscontrol/api/v2/oauth.go index fb6085a51..738d5c32c 100644 --- a/hscontrol/api/v2/oauth.go +++ b/hscontrol/api/v2/oauth.go @@ -35,6 +35,9 @@ type tokenResponse struct { TokenType string `json:"token_type"` ExpiresIn int `json:"expires_in"` Scope string `json:"scope,omitempty"` + // Tags names the tags of a token that can assign them, as Tailscale's + // answer does; a reader token's answer has none. + Tags string `json:"tags,omitempty"` } // oauthTokenHandler implements the client-credentials grant (RFC 6749 §4.4): @@ -110,12 +113,20 @@ func oauthTokenHandler(b Backend) http.HandlerFunc { return } - writeJSON(w, http.StatusOK, tokenResponse{ + resp := tokenResponse{ AccessToken: tokenStr, TokenType: "Bearer", ExpiresIn: int(accessTokenTTL.Seconds()), Scope: strings.Join(token.Scopes, " "), - }) + } + + granted := scope.Parse(token.Scopes) + if scope.Grants(granted, scope.AuthKeys) || scope.Grants(granted, scope.DevicesCore) || + scope.Grants(granted, scope.OAuthKeys) { + resp.Tags = strings.Join(token.Tags, " ") + } + + writeJSON(w, http.StatusOK, resp) } } diff --git a/hscontrol/apiv2_compat_test.go b/hscontrol/apiv2_compat_test.go index 7d8aaa807..42c5da41c 100644 --- a/hscontrol/apiv2_compat_test.go +++ b/hscontrol/apiv2_compat_test.go @@ -56,37 +56,12 @@ var apiv2CompatSkipReasons = map[string]string{ "oauth/O3/0/status": mintScopeRefusalStatus, "oauth/O4/0/scope": unknownScopeGranted, + "oauth/O4/0/tags": unknownScopeGranted, - "oauth/O4/0/tags": mintTagsNotEchoed, - "oauth/E1/0/tags": mintTagsNotEchoed, - "oauth/E9a/0/tags": mintTagsNotEchoed, - "oauth/T1/0/tags": mintTagsNotEchoed, - "oauth/D0/6/tags": mintTagsNotEchoed, - "oauth/B1/0/tags": mintTagsNotEchoed, - "oauth/B3/0/tags": mintTagsNotEchoed, - "oauth/B11/1/tags": mintTagsNotEchoed, - - "key-list/setup-token-keys/0/tags": mintTagsNotEchoed, - "key-list/setup-token-keys/2/tags": mintTagsNotEchoed, - "key-list/setup-token-keys/4/tags": mintTagsNotEchoed, - "key-list/setup-token-keys/6/tags": mintTagsNotEchoed, + "key-list/setup-token-keys/4/tags": mintTagsSorted, "oauth/K5/0/status": unknownKeyType, - "oauth/E4/0/status": tagRefusalStatus, - "oauth/E5/0/status": tagRefusalStatus, - "oauth/T2/1/status": tagRefusalStatus, - "oauth/T3/1/status": tagRefusalStatus, - - "oauth/T4/1/status": retagRemovesUnownedTag, - "oauth/T4/1/outcome": retagRemovesUnownedTag, - "oauth/T4/2/tags": retagRemovesUnownedTag, - "oauth/T4/2/owner": retagRemovesUnownedTag, - "oauth/T5/1/status": retagRemovesUnownedTag, - "oauth/T5/1/outcome": retagRemovesUnownedTag, - "oauth/T5/2/tags": retagRemovesUnownedTag, - "oauth/T5/2/owner": retagRemovesUnownedTag, - "oauth/D0/1/listed": emptyKeyListNull, "oauth/D7/2/status": deletedClientGone, @@ -131,17 +106,11 @@ func notCompared(prefixes []string, key string) bool { }) } -// Differences a planned change removes. -const ( - 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" -) - // Differences no planned change removes yet. const ( mintScopeRefusalStatus = "TOKEN_SCOPE_REFUSAL_STATUS: SaaS refuses a token scope the client lacks with 403 and a message; headscale with 400 invalid_scope" - mintTagsNotEchoed = "TOKEN_TAGS_NOT_ECHOED: SaaS's token answer names the token's tags; headscale's has no tags" + mintTagsSorted = "TOKEN_TAGS_SORTED: SaaS's token answer names the tags in the order its client was made with; headscale stores a client's tags sorted" unknownKeyType = "UNKNOWN_KEY_TYPE: SaaS refuses an unknown keyType with 400 before it checks scopes; headscale checks the scope first and answers 403" - tagRefusalStatus = "TAG_REFUSAL_STATUS: SaaS refuses a tag outside the token's grant with 400 requested tags are invalid or not permitted; headscale with 403" emptyKeyListNull = "EMPTY_KEY_LIST_NULL: SaaS answers a listing that holds no key with keys null; headscale with []" deletedClientGone = "DELETED_CLIENT_GONE: SaaS keeps a deleted OAuth client readable as invalid and revoked; headscale removes it" deletedClientCredentialText = "DELETED_CLIENT_CREDENTIAL_TEXT: SaaS refuses a deleted client's secret and tokens with API token invalid; headscale says invalid client credentials and unauthorized" diff --git a/hscontrol/apiv2_oauth_test.go b/hscontrol/apiv2_oauth_test.go index d7046084f..88fe58d06 100644 --- a/hscontrol/apiv2_oauth_test.go +++ b/hscontrol/apiv2_oauth_test.go @@ -215,6 +215,15 @@ func TestAPIv2OAuth_TokenEndpoint(t *testing.T) { require.Equalf(t, http.StatusOK, statusDup, "%v", mDup) assert.Equal(t, "auth_keys", mDup["scope"]) + // It names the tags of a token that can assign them; a reader token's + // answer has no tags field, whatever its client's tags. + assert.Equal(t, "tag:ci", m["tags"]) + + _, readerSecret := createClient(t, baseURL, admin, []string{"auth_keys:read"}, []string{"tag:ci"}) + statusReader, mReader := mintToken(t, baseURL, "", readerSecret, "", false) + require.Equalf(t, http.StatusOK, statusReader, "%v", mReader) + assert.NotContains(t, mReader, "tags") + // Bad credentials → RFC 6749 invalid_client. statusBad, mBad := mintToken(t, baseURL, clientID, "hskey-client-deadbeefdead-"+stringOf("0", 64), "", false) assert.Equal(t, http.StatusUnauthorized, statusBad) @@ -533,7 +542,7 @@ func TestAPIv2OAuth_ScopeEnforcement(t *testing.T) { "write scope + in-grant tag allowed") // ...but not with a tag outside its grant. - assert.Equal(t, http.StatusForbidden, createTaggedKey(t, baseURL, writeTok, []string{"tag:other"}), + assert.Equal(t, http.StatusBadRequest, createTaggedKey(t, baseURL, writeTok, []string{"tag:other"}), "tag outside grant denied") // A read-only token is denied the write. @@ -603,7 +612,7 @@ func TestAPIv2OAuth_TaggedAllClientStaysInTags(t *testing.T) { 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, + assert.Equal(t, http.StatusBadRequest, createTaggedKey(t, baseURL, tok, []string{"tag:b"}), "the token is held to its tags") @@ -652,7 +661,7 @@ func TestAPIv2OAuth_TagOwnedBy(t *testing.T) { "a token may use a tag owned by its tag") // Unrelated tag: denied. - assert.Equal(t, http.StatusForbidden, createTaggedKey(t, baseURL, tok, []string{"tag:other"}), + assert.Equal(t, http.StatusBadRequest, createTaggedKey(t, baseURL, tok, []string{"tag:other"}), "a token may not use an unowned tag") } @@ -704,13 +713,28 @@ func TestAPIv2OAuth_SetDeviceTagsGrant(t *testing.T) { st, body := apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci"}}) assert.Equalf(t, http.StatusOK, st, "in-grant tag: %s", body) + // The device now carries only tags within the grant. + st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci"}}) + assert.Equalf(t, http.StatusOK, st, "in-grant device: %s", body) + // An existing policy tag outside the token's grant is denied, not silently set. - st, _ = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:other"}}) - assert.Equal(t, http.StatusForbidden, st, "out-of-grant tag must be denied") + st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:other"}}) + assert.Equal(t, http.StatusBadRequest, st, "out-of-grant tag must be denied") + assert.Contains(t, string(body), "requested tags [tag:other] are invalid or not permitted") // An admin API key is unrestricted. st, body = apiPost(t, devURL, admin, map[string]any{"tags": []string{"tag:other"}}) assert.Equalf(t, http.StatusOK, st, "admin may set any policy tag: %s", body) + + // SetNodeTags replaces the set, so retagging would remove tag:other. + st, body = apiPost(t, devURL, tok, map[string]any{"tags": []string{"tag:ci", "tag:other"}}) + assert.Equal(t, http.StatusBadRequest, st, "token must not retag an out-of-grant device") + assert.Contains(t, string(body), + "cannot remove current tags: requested tags [tag:other] are invalid or not permitted") + + got, ok := app.state.GetNodeByID(view.ID()) + require.True(t, ok) + assert.Equal(t, []string{"tag:other"}, got.Tags().AsSlice()) } // TestAPIv2OAuth_UndefinedTagRejected asserts an OAuth token cannot create a diff --git a/hscontrol/servertest/apiv2_oauth_scopes_test.go b/hscontrol/servertest/apiv2_oauth_scopes_test.go index fb2fdf8c7..015882a05 100644 --- a/hscontrol/servertest/apiv2_oauth_scopes_test.go +++ b/hscontrol/servertest/apiv2_oauth_scopes_test.go @@ -291,7 +291,7 @@ resource "tailscale_tailnet_key" "k" { tags: []string{"tag:k8s-operator"}, config: k8sAuthKeyConfig("tag:other"), deny: true, - denyContains: []string{"403", "Forbidden", "is not owned"}, + denyContains: []string{"requested tags [tag:other]", "(400)"}, }, } @@ -494,7 +494,7 @@ func TestAPIv2OAuthTailscaleClientAuthKey(t *testing.T) { name: "tag_not_granted", authKey: tsSecret + base, tags: []string{"tag:other"}, - wantErr: "may not assign tag tag:other", + wantErr: "requested tags [tag:other] are invalid or not permitted", }, { name: "wrong_secret",