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..a007c8d9 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 ( @@ -24,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. @@ -37,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 { @@ -85,11 +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{pullScope(repository)} if !ok || !canReplay(req) || !t.trustedWithToken(req.URL, challenge) { return resp, nil } @@ -115,37 +126,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 + } + if registry.Scheme != "https" || realm.Scheme != "https" || registry.Port() != "" || realm.Port() != "" { + return false } - return strings.EqualFold(a.Host, b.Host) + want, ok := knownTokenServices[strings.ToLower(registry.Hostname())] + return ok && strings.EqualFold(realm.Hostname(), want) } func (t *Transport) applyAuthentication(req *http.Request, hasExplicitAuthorization bool) { @@ -193,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 } @@ -272,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 } @@ -334,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()) } @@ -364,21 +397,38 @@ 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 { +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 +// 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 +443,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) { 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) + } +}