From 351fe7c0b55c0a1f0e32e31ddfd73d9712ec0167 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 24 Oct 2026 08:00:48 +0200 Subject: [PATCH 1/2] Send registry tokens only to the token service's own registry After a 401 the transport fetches a token from the challenge's realm, using credentials configured for that realm, and sends it to the host that issued the challenge. It trusted that host if it had any configured credentials, even unrelated ones, or shared the realm's registrable domain. The second rule makes every tenant on a shared domain such as jfrog.io, azurecr.io or pkg.dev the same site, and the first hands a token minted from one registry's credentials to another registry the proxy happens to have a key for. A credentialed token now goes only to a registry on the realm's own host and port, to one configured with the same credentials as the realm, or to Docker Hub and GitLab.com, whose registries use a token service on another host. Anonymous tokens are unaffected. The scope in the token request also came from the challenge, so a registry could ask for push access or for other repositories. The proxy only reads, so it now always asks for pull access to the repository in the request path. --- docs/configuration.md | 17 ++++- internal/httpclient/token_scope_test.go | 94 +++++++++++++++++++++---- internal/httpclient/transport.go | 69 +++++++++++------- 3 files changed, 141 insertions(+), 39 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index fa77ec69..e2a0513b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -839,9 +839,22 @@ This field has no environment variable, as with the other named upstream maps. Configure authentication for private upstream registries. The same authentication-aware client is used for metadata and artifact downloads, and credentials can reference environment variables using `${VAR_NAME}` syntax. -OCI registries that return a Bearer challenge from a `/v2/{repository}/…` endpoint are handled automatically. The proxy discovers the token realm from `WWW-Authenticate`, applies any configured credentials for the token URL, and reuses the scoped token until shortly before it expires. +OCI registries that return a Bearer challenge from a `/v2/{repository}/…` endpoint are handled automatically. The proxy discovers the token realm from `WWW-Authenticate`, applies any configured credentials for the token URL, and reuses the token until shortly before it expires. The token is always requested for pull access to the repository being fetched (`repository:{repository}:pull`), whatever scope the challenge names. -A token fetched with credentials configured for the token URL is only fetched for, and sent to, a registry that is trusted with it: one with configured credentials of its own, or one on the same site (registrable domain) as the token service, as `registry-1.docker.io` is with `auth.docker.io`. Any other host that answers with a challenge naming that token service gets its 401 back, so it cannot obtain a token minted from your credentials. Tokens from a token service with no configured credentials are anonymous and are fetched for any registry, as before. If your registry and its token service are on different sites, configure credentials for the registry URL as well. +A token fetched with credentials configured for the token URL is only fetched for, and sent to, a registry that is trusted with it: one on the same host and port as the token service, one configured with the same credentials as the token URL, or Docker Hub (`registry-1.docker.io` and `index.docker.io` with `auth.docker.io`) and GitLab.com (`registry.gitlab.com` with `gitlab.com`). Sharing a parent domain is not enough, so `team-a.jfrog.io` is not trusted with tokens from `team-b.jfrog.io`. Any other host that answers with a challenge naming that token service gets its 401 back, so it cannot obtain a token minted from your credentials. Tokens from a token service with no configured credentials are anonymous and are fetched for any registry, as before. If your registry and its token service are on different hosts, such as a self-hosted GitLab with its registry on its own hostname, configure the same credentials for both URLs: + +```yaml +upstream: + auth: + "https://registry.gitlab.example.com": + type: basic + username: "deploy-token" + password: "${GITLAB_DEPLOY_TOKEN}" + "https://gitlab.example.com/jwt/auth": + type: basic + username: "deploy-token" + password: "${GITLAB_DEPLOY_TOKEN}" +``` ### Bearer Token diff --git a/internal/httpclient/token_scope_test.go b/internal/httpclient/token_scope_test.go index 97d8154c..563d9392 100644 --- a/internal/httpclient/token_scope_test.go +++ b/internal/httpclient/token_scope_test.go @@ -141,24 +141,92 @@ func TestTransportFollowsAnonymousTokenServiceOnAnotherHost(t *testing.T) { } } -func TestSameSite(t *testing.T) { +// TestTransportDoesNotSendCredentialedTokenToHostWithOtherCredentials checks +// that a registry with credentials of its own, but not the realm's, is not +// handed a token minted from the realm's credentials. +func TestTransportDoesNotSendCredentialedTokenToHostWithOtherCredentials(t *testing.T) { + var issued atomic.Int32 + realm := tokenService(t, &issued) + var received []string + vendor := challengingRegistry(t, realm.URL, &received) + + auth := func(rawURL string) (string, string) { + switch { + case strings.HasPrefix(rawURL, realm.URL): + return "Authorization", "Basic Y29ycDpwYXNz" + case strings.HasPrefix(rawURL, vendor.URL): + return "Authorization", "Basic dmVuZG9yOmtleQ==" + } + return "", "" + } + client := &http.Client{Transport: NewTransport(http.DefaultTransport, auth)} + if status := get(t, client, vendor.URL+"/v2/corp/private/blobs/sha256:abc"); status != http.StatusUnauthorized { + t.Errorf("status = %d, want the challenge passed back as 401", status) + } + for _, got := range received { + if got != "Basic dmVuZG9yOmtleQ==" { + t.Errorf("the vendor registry received Authorization %q, want only its own credentials", got) + } + } + if n := issued.Load(); n != 0 { + t.Errorf("token service was asked %d times on the vendor registry's behalf, want 0", n) + } +} + +// TestTransportRequestsPullScopeForRequestedRepository checks that the token +// is requested for pull access to the repository in the request path, not +// whatever scope the challenge names. +func TestTransportRequestsPullScopeForRequestedRepository(t *testing.T) { + var scopes [][]string + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/token" { + scopes = append(scopes, r.URL.Query()["scope"]) + _, _ = io.WriteString(w, `{"token":"t","expires_in":3600}`) + return + } + if r.Header.Get("Authorization") != "Bearer t" { + w.Header().Set("WWW-Authenticate", `Bearer realm="`+srv.URL+`/token",service="registry.corp",scope="repository:corp/other:pull,push"`) + http.Error(w, "authentication required", http.StatusUnauthorized) + return + } + _, _ = io.WriteString(w, "blob") + })) + t.Cleanup(srv.Close) + + client := &http.Client{Transport: NewTransport(http.DefaultTransport, credentialsFor(srv.URL))} + if status := get(t, client, srv.URL+"/v2/corp/app/manifests/latest"); status != http.StatusOK { + t.Fatalf("status = %d, want 200", status) + } + if len(scopes) != 1 || len(scopes[0]) != 1 || scopes[0][0] != "repository:corp/app:pull" { + t.Errorf("token requested with scopes %q, want [[repository:corp/app:pull]]", scopes) + } +} + +func TestSameTokenService(t *testing.T) { for _, tt := range []struct { registry, realm string want bool }{ - {"https://registry-1.docker.io", "https://auth.docker.io", true}, - {"https://registry.gitlab.com", "https://gitlab.com", true}, - {"https://ghcr.io", "https://ghcr.io", true}, - {"https://registry.corp.example", "https://auth.corp.example:8443", true}, - {"https://evil.example", "https://auth.docker.io", false}, - {"https://docker.io.evil.example", "https://auth.docker.io", false}, - {"https://a.github.io", "https://b.github.io", false}, // public suffix: separate sites - {"http://127.0.0.1:5000", "http://127.0.0.1:5000", true}, - {"http://127.0.0.1:5000", "http://127.0.0.1:5001", false}, - {"http://localhost:5000", "http://localhost:5001", false}, + {"https://registry-1.docker.io", "https://auth.docker.io/token", true}, + {"https://index.docker.io", "https://auth.docker.io/token", true}, + {"https://registry.gitlab.com", "https://gitlab.com/jwt/auth", true}, + {"https://ghcr.io", "https://ghcr.io/token", true}, + {"https://harbor.corp.example", "https://harbor.corp.example/service/token", true}, + {"https://registry.corp.example", "https://auth.corp.example", false}, + {"https://evil.example", "https://auth.docker.io/token", false}, + {"https://docker.io.evil.example", "https://auth.docker.io/token", false}, + {"https://evil.jfrog.io", "https://myco.jfrog.io/artifactory/api/docker/token", false}, + {"https://evil.azurecr.io", "https://myco.azurecr.io/oauth2/token", false}, + {"https://gitlab.com", "https://registry.gitlab.com", false}, + {"http://registry-1.docker.io", "https://auth.docker.io/token", false}, + {"https://registry-1.docker.io", "https://auth.docker.io:8443/token", false}, + {"http://127.0.0.1:5000", "http://127.0.0.1:5000/token", true}, + {"http://127.0.0.1:5000", "http://127.0.0.1:5001/token", false}, + {"http://localhost:5000", "https://localhost:5000/token", false}, } { - if got := sameSite(mustParse(t, tt.registry), mustParse(t, tt.realm)); got != tt.want { - t.Errorf("sameSite(%s, %s) = %v, want %v", tt.registry, tt.realm, got, tt.want) + if got := sameTokenService(mustParse(t, tt.registry), mustParse(t, tt.realm)); got != tt.want { + t.Errorf("sameTokenService(%s, %s) = %v, want %v", tt.registry, tt.realm, got, tt.want) } } } diff --git a/internal/httpclient/transport.go b/internal/httpclient/transport.go index 56c0f48c..ba78ee5a 100644 --- a/internal/httpclient/transport.go +++ b/internal/httpclient/transport.go @@ -13,8 +13,6 @@ import ( "strings" "sync" "time" - - "golang.org/x/net/publicsuffix" ) const ( @@ -90,6 +88,9 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { } challenge, ok := parseBearerChallenge(resp.Header.Values("WWW-Authenticate")) + // Ask only for pull access to the repository being fetched, whatever + // scope the challenge names: the proxy only reads. + challenge.scopes = []string{"repository:" + registryRepository(req.URL) + ":pull"} if !ok || !canReplay(req) || !t.trustedWithToken(req.URL, challenge) { return resp, nil } @@ -115,37 +116,46 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { // from that registry, which can be any host the proxy fetches from, such as // one named in a Composer dist URL or a Helm index. Credentials configured for // the realm would otherwise be turned into a token handed to that host. A -// registry is trusted with such a token when it has configured credentials of -// its own, or is on the same site as the realm, as Docker Hub's -// registry-1.docker.io is with auth.docker.io. A token from a realm with no -// configured credentials is anonymous, and any registry may have it. +// token from a realm with no configured credentials is anonymous, and any +// registry may have it. A credentialed token only goes to the realm's own +// host, to a registry configured with the same credentials as the realm, or +// to a registry in knownTokenServices. func (t *Transport) trustedWithToken(requestURL *url.URL, challenge bearerChallenge) bool { if t.authForURL == nil { return true } - if name, value := t.authForURL(challenge.realm); name == "" || value == "" { + realmName, realmValue := t.authForURL(challenge.realm) + if realmName == "" || realmValue == "" { return true } - if name, value := t.authForURL(requestURL.String()); name != "" && value != "" { + if name, value := t.authForURL(requestURL.String()); name == realmName && value == realmValue { return true } realm, err := url.Parse(challenge.realm) - return err == nil && sameSite(requestURL, realm) -} - -// sameSite reports whether two URLs are on the same site: the same registrable -// domain (public suffix plus one label), or, for IP addresses and names -// without one, such as localhost, the same host and port. -func sameSite(a, b *url.URL) bool { - hostA, hostB := strings.ToLower(a.Hostname()), strings.ToLower(b.Hostname()) - if net.ParseIP(hostA) == nil && net.ParseIP(hostB) == nil { - siteA, errA := publicsuffix.EffectiveTLDPlusOne(hostA) - siteB, errB := publicsuffix.EffectiveTLDPlusOne(hostB) - if errA == nil && errB == nil { - return siteA == siteB - } + return err == nil && sameTokenService(requestURL, realm) +} + +// knownTokenServices maps registry hosts to the token service host they +// use where the two differ and the registry cannot be configured with the +// token service's credentials under the same upstream.auth key. +var knownTokenServices = map[string]string{ + "registry-1.docker.io": "auth.docker.io", + "index.docker.io": "auth.docker.io", + "registry.gitlab.com": "gitlab.com", +} + +// sameTokenService reports whether a token service at realm belongs to the +// registry at registry: both on the same scheme, host and port, or a known +// pair of registry and token service hosts. +func sameTokenService(registry, realm *url.URL) bool { + if strings.EqualFold(registry.Scheme, realm.Scheme) && strings.EqualFold(registry.Host, realm.Host) { + return true } - return strings.EqualFold(a.Host, b.Host) + if registry.Scheme != "https" || realm.Scheme != "https" || registry.Port() != "" || realm.Port() != "" { + return false + } + want, ok := knownTokenServices[strings.ToLower(registry.Hostname())] + return ok && strings.EqualFold(realm.Hostname(), want) } func (t *Transport) applyAuthentication(req *http.Request, hasExplicitAuthorization bool) { @@ -379,6 +389,17 @@ func (c bearerChallenge) key() string { } func registryProtectionSpace(u *url.URL) string { + repository := registryRepository(u) + if repository == "" { + return "" + } + return u.Scheme + "://" + u.Host + "/v2/" + repository +} + +// registryRepository returns the repository name from an OCI distribution +// API path, such as library/alpine from /v2/library/alpine/manifests/latest, +// or "" if the path is not one. +func registryRepository(u *url.URL) string { const registryPrefix = "/v2/" if u == nil || !strings.HasPrefix(u.Path, registryPrefix) { return "" @@ -393,7 +414,7 @@ func registryProtectionSpace(u *url.URL) string { if end == len(rest) || end == 0 { return "" } - return u.Scheme + "://" + u.Host + registryPrefix + rest[:end] + return rest[:end] } func parseBearerChallenge(values []string) (bearerChallenge, bool) { From aa3adbfa63ced63cd2e416689b0a0c769aea7e5d Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 24 Oct 2026 08:02:10 +0200 Subject: [PATCH 2/2] Bound the registry token and challenge caches The transport remembered a challenge for every /v2/ it was ever challenged on and never forgot one, so a long-running proxy, or a metadata document naming many registries, grew the map without limit. Tokens were only pruned of expired entries, and the expiry came from the token service. Challenges are now kept per registry host, holding only the realm and service, and the pull scope for a request is built from its path, so pulling many repositories from one registry keeps one entry. Both caches have a size limit, evicting an arbitrary challenge or the token that expires soonest, and a token is reused for at most an hour, whatever lifetime the token service claims. --- internal/httpclient/transport.go | 65 +++++++++++++++++++-------- internal/httpclient/transport_test.go | 49 ++++++++++++++++++++ 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/internal/httpclient/transport.go b/internal/httpclient/transport.go index ba78ee5a..a007c8d9 100644 --- a/internal/httpclient/transport.go +++ b/internal/httpclient/transport.go @@ -22,6 +22,15 @@ const ( shortTokenSkewDivisor = 10 tokenMaxRetries = 3 tokenRetryBaseDelay = 500 * time.Millisecond + + // maxTokenLifetime caps how long a token is reused, whatever lifetime + // the token service claims. + maxTokenLifetime = time.Hour + // maxCachedTokens and maxCachedChallenges bound the token and + // challenge caches, which grow with every repository and registry + // host the proxy pulls from. + maxCachedTokens = 1024 + maxCachedChallenges = 256 ) // AuthFunc returns a configured authentication header for a URL. @@ -35,7 +44,7 @@ type Transport struct { mu sync.Mutex tokens map[string]cachedToken - challenges map[string]bearerChallenge + challenges map[string]bearerChallenge // by registry origin, without scopes } type cachedToken struct { @@ -83,14 +92,15 @@ func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { if hasExplicitAuthorization { return resp, nil } - if registryProtectionSpace(req.URL) == "" { + repository := registryRepository(req.URL) + if repository == "" { return resp, nil } challenge, ok := parseBearerChallenge(resp.Header.Values("WWW-Authenticate")) // Ask only for pull access to the repository being fetched, whatever // scope the challenge names: the proxy only reads. - challenge.scopes = []string{"repository:" + registryRepository(req.URL) + ":pull"} + challenge.scopes = []string{pullScope(repository)} if !ok || !canReplay(req) || !t.trustedWithToken(req.URL, challenge) { return resp, nil } @@ -203,6 +213,17 @@ func (t *Transport) cacheToken(key string, token cachedToken) { delete(t.tokens, cachedKey) } } + if _, ok := t.tokens[key]; !ok && len(t.tokens) >= maxCachedTokens { + // Drop the token that expires soonest. + var oldestKey string + var oldest time.Time + for cachedKey, cached := range t.tokens { + if oldestKey == "" || cached.expiresAt.Before(oldest) { + oldestKey, oldest = cachedKey, cached.expiresAt + } + } + delete(t.tokens, oldestKey) + } t.tokens[key] = token } @@ -282,6 +303,7 @@ func decodeTokenResponse(resp *http.Response) (string, time.Time, error) { if lifetime <= 0 { lifetime = defaultTokenLifetime } + lifetime = min(lifetime, maxTokenLifetime) expiresAt := issuedAt.Add(lifetime).Add(-expirySkew(lifetime)) return token, expiresAt, nil } @@ -344,17 +366,18 @@ func (t configuredTransport) RoundTrip(req *http.Request) (*http.Response, error } func (t *Transport) cachedTokenForRequest(requestURL *url.URL) string { - space := registryProtectionSpace(requestURL) - if space == "" { + repository := registryRepository(requestURL) + if repository == "" { return "" } t.mu.Lock() - challenge, ok := t.challenges[space] + challenge, ok := t.challenges[registryOrigin(requestURL)] t.mu.Unlock() if !ok { return "" } + challenge.scopes = []string{pullScope(repository)} return t.cachedToken(challenge.key()) } @@ -374,26 +397,32 @@ func (t *Transport) cachedToken(key string) string { return token.value } +// rememberChallenge records the token service a registry named, so later +// requests to any of its repositories can send a cached token up front. func (t *Transport) rememberChallenge(requestURL *url.URL, challenge bearerChallenge) { - space := registryProtectionSpace(requestURL) - if space == "" { - return - } + origin := registryOrigin(requestURL) + challenge.scopes = nil t.mu.Lock() - t.challenges[space] = challenge - t.mu.Unlock() + defer t.mu.Unlock() + if _, ok := t.challenges[origin]; !ok && len(t.challenges) >= maxCachedChallenges { + for evict := range t.challenges { + delete(t.challenges, evict) + break + } + } + t.challenges[origin] = challenge } func (c bearerChallenge) key() string { return c.realm + "\x00" + c.service + "\x00" + strings.Join(c.scopes, "\x00") } -func registryProtectionSpace(u *url.URL) string { - repository := registryRepository(u) - if repository == "" { - return "" - } - return u.Scheme + "://" + u.Host + "/v2/" + repository +func registryOrigin(u *url.URL) string { + return strings.ToLower(u.Scheme + "://" + u.Host) +} + +func pullScope(repository string) string { + return "repository:" + repository + ":pull" } // registryRepository returns the repository name from an OCI distribution diff --git a/internal/httpclient/transport_test.go b/internal/httpclient/transport_test.go index 01f3e8b6..46a31b62 100644 --- a/internal/httpclient/transport_test.go +++ b/internal/httpclient/transport_test.go @@ -3,6 +3,7 @@ package httpclient import ( "context" "errors" + "fmt" "io" "net" "net/http" @@ -412,3 +413,51 @@ func TestTransportDoesNotFollowBearerChallengeOutsideOCIRegistry(t *testing.T) { t.Errorf("token requests = %d, want 0", tokenRequests) } } + +func TestTransportBoundsTokenCache(t *testing.T) { + transport := NewTransport(http.DefaultTransport, nil) + expiry := time.Now().Add(time.Hour) + for i := range maxCachedTokens + 10 { + transport.cacheToken(fmt.Sprintf("key-%d", i), cachedToken{ + value: "token", + expiresAt: expiry.Add(time.Duration(i) * time.Second), + }) + } + if got := len(transport.tokens); got != maxCachedTokens { + t.Errorf("cached tokens = %d, want %d", got, maxCachedTokens) + } + if got := transport.cachedToken(fmt.Sprintf("key-%d", maxCachedTokens+9)); got != "token" { + t.Error("the newest token was evicted") + } + if got := transport.cachedToken("key-0"); got != "" { + t.Error("the token expiring soonest was kept") + } +} + +func TestTransportBoundsChallengeCache(t *testing.T) { + transport := NewTransport(http.DefaultTransport, nil) + challenge := bearerChallenge{realm: "https://auth.example.test/token", service: "registry"} + for i := range 50 { + transport.rememberChallenge(mustParse(t, fmt.Sprintf("https://registry.example.test/v2/repo-%d/blobs/sha256:abc", i)), challenge) + } + if got := len(transport.challenges); got != 1 { + t.Errorf("challenges for one registry host = %d, want 1", got) + } + for i := range maxCachedChallenges + 10 { + transport.rememberChallenge(mustParse(t, fmt.Sprintf("https://registry-%d.example.test/v2/repo/blobs/sha256:abc", i)), challenge) + } + if got := len(transport.challenges); got != maxCachedChallenges { + t.Errorf("challenges = %d, want %d", got, maxCachedChallenges) + } +} + +func TestTransportCapsTokenLifetime(t *testing.T) { + resp := &http.Response{Body: io.NopCloser(strings.NewReader(`{"token":"t","expires_in":31536000}`))} + _, expiresAt, err := decodeTokenResponse(resp) + if err != nil { + t.Fatal(err) + } + if limit := time.Now().Add(maxTokenLifetime); expiresAt.After(limit) { + t.Errorf("token expires at %v, want no later than %v", expiresAt, limit) + } +}