From 04ce4b20a87eda12de5b4dddd1833d77da3526ce Mon Sep 17 00:00:00 2001 From: Dipti Pai Date: Tue, 30 Jun 2026 14:42:15 -0700 Subject: [PATCH] Cache registry token during Notation verification The ORAS auth.Client used for Notation signature verification was constructed without a Cache, which ORAS treats as no caching. As a result every request in a single verification (manifest resolve, signature listing, signature manifest and blob fetches) performed its own token exchange against the registry, multiplying token-endpoint traffic by roughly five for each verified artifact. Set Cache to auth.NewCache() so the registry authorization token is fetched once and reused across the requests of a verification. This matches the ORAS and Notation reference clients and does not change verification behaviour or credential resolution; token staleness is still handled by ORAS on a 401. Add unit tests asserting the cache is wired and that the token endpoint is hit once instead of per request. Signed-off-by: Dipti Pai Assisted-by: GitHub Copilot/Claude Opus 4.8 (cherry picked from commit 69305ed84a0790439b6c254873d94938e80412ad) --- internal/oci/notation/notation.go | 5 + internal/oci/notation/notation_test.go | 153 +++++++++++++++++++++++++ 2 files changed, 158 insertions(+) diff --git a/internal/oci/notation/notation.go b/internal/oci/notation/notation.go index 0158ffd03..e78dd8caf 100644 --- a/internal/oci/notation/notation.go +++ b/internal/oci/notation/notation.go @@ -366,6 +366,11 @@ func (v *NotationVerifier) remoteRepo(repoUrl string) (*oras.Repository, error) "User-Agent": {"flux"}, }, Credential: credentialProvider, + // Cache the registry authorization token so that the manifest + // resolution, signature listing and signature blob fetches performed + // during a single verification reuse the same token instead of + // re-authenticating on every request. + Cache: oauth.NewCache(), } remoteRepo.Client = repoClient diff --git a/internal/oci/notation/notation_test.go b/internal/oci/notation/notation_test.go index cdd8a3872..1a547617d 100644 --- a/internal/oci/notation/notation_test.go +++ b/internal/oci/notation/notation_test.go @@ -18,11 +18,15 @@ package notation import ( "context" + "encoding/json" "fmt" "net/http" + "net/http/httptest" "net/url" "path" "reflect" + "strings" + "sync/atomic" "testing" "github.com/go-logr/logr" @@ -32,6 +36,7 @@ import ( "github.com/notaryproject/notation-go" "github.com/notaryproject/notation-go/verifier/trustpolicy" . "github.com/onsi/gomega" + oauth "oras.land/oras-go/v2/registry/remote/auth" "github.com/fluxcd/source-controller/internal/oci" testproxy "github.com/fluxcd/source-controller/tests/proxy" @@ -542,6 +547,154 @@ func TestRepoUrlWithDigest(t *testing.T) { } } +func TestRemoteRepoAuthCache(t *testing.T) { + testCases := []struct { + name string + auth authn.Authenticator + keychain authn.Keychain + transport bool + }{ + { + name: "anonymous", + }, + { + name: "with authenticator", + auth: &authn.Basic{Username: "foo", Password: "bar"}, + }, + { + name: "with keychain", + keychain: authn.DefaultKeychain, + }, + { + name: "with custom transport", + auth: &authn.Basic{Username: "foo", Password: "bar"}, + transport: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + v := &NotationVerifier{ + auth: tc.auth, + keychain: tc.keychain, + } + if tc.transport { + v.transport = http.DefaultTransport.(*http.Transport).Clone() + } + + repo, err := v.remoteRepo("ghcr.io/stefanprodan/charts/podinfo") + g.Expect(err).NotTo(HaveOccurred()) + + client, ok := repo.Client.(*oauth.Client) + g.Expect(ok).To(BeTrue(), "expected repository client to be *oauth.Client") + + // The auth cache must be set so that the manifest resolution, + // signature listing and signature blob fetches performed during a + // single verification reuse the same registry token instead of + // re-authenticating on every request. + g.Expect(client.Cache).NotTo(BeNil()) + }) + } +} + +// TestRemoteRepoAuthTokenReuse exercises the registry authorization flow against +// a fake bearer-auth registry and counts how many times the token endpoint is +// hit. It demonstrates the effect of the auth cache fix: without a cache every +// registry request performs its own token exchange (modelling the pre-fix +// behaviour), whereas the client built by remoteRepo fetches the token once and +// reuses it for all subsequent requests of the same scope. +func TestRemoteRepoAuthTokenReuse(t *testing.T) { + g := NewWithT(t) + + const ( + repository = "test/podinfo" + bearerToken = "secret-token" + numRequests = 5 + ) + scope := oauth.ScopeRepository(repository, "pull") + + var tokenRequests atomic.Int32 + + var baseURL string + mux := http.NewServeMux() + // Token endpoint: counts every issuance and returns a static token. + mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) { + tokenRequests.Add(1) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "token": bearerToken, + "access_token": bearerToken, + "expires_in": 3600, + }) + }) + // Registry endpoint: challenges unauthenticated requests with a bearer + // challenge pointing at the token endpoint, and accepts the issued token. + mux.HandleFunc("/v2/", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer "+bearerToken { + w.Header().Set("Www-Authenticate", + fmt.Sprintf(`Bearer realm="%s/token",service="registry",scope=%q`, baseURL, scope)) + w.WriteHeader(http.StatusUnauthorized) + return + } + w.WriteHeader(http.StatusOK) + }) + + server := httptest.NewServer(mux) + defer server.Close() + baseURL = server.URL + host := strings.TrimPrefix(baseURL, "http://") + + anonymousCredential := func(context.Context, string) (oauth.Credential, error) { + return oauth.EmptyCredential, nil + } + + // doRequests issues n authenticated GET requests through the given client, + // setting the scope hint on the context exactly like the ORAS repository + // methods do during verification. + doRequests := func(client *oauth.Client, n int) { + for i := 0; i < n; i++ { + ctx := oauth.WithScopesForHost(context.Background(), host, scope) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, + fmt.Sprintf("%s/v2/%s/manifests/latest", baseURL, repository), nil) + g.Expect(err).NotTo(HaveOccurred()) + resp, err := client.Do(req) + g.Expect(err).NotTo(HaveOccurred()) + resp.Body.Close() + g.Expect(resp.StatusCode).To(Equal(http.StatusOK)) + } + } + + // Before the fix: an ORAS client without a cache re-authenticates on every + // request, so the token endpoint is hit once per request. + tokenRequests.Store(0) + noCacheClient := &oauth.Client{ + Client: server.Client(), + Header: http.Header{"User-Agent": {"flux"}}, + Credential: anonymousCredential, + // Cache is deliberately unset to model the pre-fix behaviour. + } + doRequests(noCacheClient, numRequests) + g.Expect(tokenRequests.Load()).To(Equal(int32(numRequests)), + "without an auth cache each request performs its own token exchange") + + // After the fix: remoteRepo wires an auth cache, so the token is fetched + // once and reused for all subsequent requests of the same scope. + tokenRequests.Store(0) + v := &NotationVerifier{insecure: true} + repo, err := v.remoteRepo(fmt.Sprintf("%s/%s", host, repository)) + g.Expect(err).NotTo(HaveOccurred()) + cachedClient, ok := repo.Client.(*oauth.Client) + g.Expect(ok).To(BeTrue(), "expected repository client to be *oauth.Client") + g.Expect(cachedClient.Cache).NotTo(BeNil()) + // Route the cached client through the test server's HTTP client. + cachedClient.Client = server.Client() + doRequests(cachedClient, numRequests) + g.Expect(tokenRequests.Load()).To(Equal(int32(1)), + "with an auth cache the token is fetched once and reused across requests") +} + func TestVerificationWithProxy(t *testing.T) { g := NewWithT(t)