Skip to content

transport: block redirects from token server to private/link-local addresses (SSRF fix)#2292

Merged
Subserial merged 4 commits into
google:mainfrom
evilgensec:fix/token-refresh-redirect-ssrf
May 13, 2026
Merged

transport: block redirects from token server to private/link-local addresses (SSRF fix)#2292
Subserial merged 4 commits into
google:mainfrom
evilgensec:fix/token-refresh-redirect-ssrf

Conversation

@evilgensec

@evilgensec evilgensec commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

refreshOauth and refreshBasic in bearer.go construct an http.Client without a CheckRedirect handler. Go's default redirect policy follows up to 10 hops unconditionally, forwarding the request to any URL returned by the server including private and link-local addresses.

A malicious or compromised token server (or a registry that controls the realm parameter in WWW-Authenticate) can therefore bypass the existing validateRealmURL check by first responding with a valid-looking realm URL and then issuing an HTTP 302 redirect to an internal service:

WWW-Authenticate: Bearer realm="https://legit-token-server.example.com/token",...
→ GET https://legit-token-server.example.com/token   ← passes validateRealmURL
← HTTP/1.1 302 Found
   Location: http://169.254.169.254/latest/meta-data/iam/security-credentials/ROLE
→ GET http://169.254.169.254/...                      ← never checked
← {"Token": "AQo...", ...}                            ← AWS session token
→ stored as bt.bearer.RegistryToken
→ sent as Authorization: Bearer <AWS_SESSION_TOKEN> to attacker's registry

This is mechanically distinct from the DNS-based SSRF explicitly noted as out-of-scope in the validateRealmURL comment redirect-based SSRF is fully preventable at the HTTP client layer.

Fix

Add realmRedirectCheck, which applies the same scheme and private-IP restrictions as validateRealmURL to every redirect hop. Wire it as CheckRedirect on both http.Client instances used for token fetches.

Test

TestTokenServerRedirectSSRF spins up a loopback token server that redirects to a second loopback "internal service" and confirms that bt.refresh() returns an error rather than following the redirect.

Relation to prior work

PR #2243 added validateRealmURL to block direct private-IP realm values. This PR closes the remaining redirect-bypass path that #2243 left open.

refreshOauth and refreshBasic constructed http.Client without a
CheckRedirect handler, allowing a malicious token server to issue an
HTTP redirect to an internal address (e.g. 169.254.169.254) that would
pass the initial validateRealmURL check but still reach private services.

Add realmRedirectCheck, which applies the same scheme and private-IP
restrictions as validateRealmURL to every redirect hop during a
token-fetch request. Wire it as CheckRedirect on both clients.

Add TestTokenServerRedirectSSRF to confirm the redirect is rejected.

@Subserial Subserial left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has some AI assistant hallmarks, so I will be pedantic about comments.

Looks good, just mostly needs to adjust overly-descriptive or inaccurate language.

Comment thread pkg/v1/remote/transport/bearer.go Outdated
Comment thread pkg/v1/remote/transport/bearer.go Outdated
Comment thread pkg/v1/remote/transport/bearer.go Outdated
Comment thread pkg/v1/remote/transport/bearer_test.go Outdated
@codecov-commenter

codecov-commenter commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.74%. Comparing base (a4da0ed) to head (f877bb9).

Files with missing lines Patch % Lines
pkg/v1/remote/transport/bearer.go 72.72% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2292      +/-   ##
==========================================
+ Coverage   56.70%   56.74%   +0.04%     
==========================================
  Files         165      165              
  Lines       11239    11248       +9     
==========================================
+ Hits         6373     6383      +10     
  Misses       4106     4106              
+ Partials      760      759       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Shorten realmRedirectCheck doc comment.
- Extract allowInsecure local in refreshOauth and refreshBasic.
- Drop redundant inline comment in TestTokenServerRedirectSSRF.
@evilgensec evilgensec requested a review from Subserial May 13, 2026 04:04
@Subserial

Copy link
Copy Markdown
Contributor

LGTM, just need to make the linter happy and I'll merge the PR. Thanks!

@Subserial Subserial merged commit a70d75a into google:main May 13, 2026
17 checks passed
Subserial pushed a commit to Subserial/go-containerregistry that referenced this pull request May 15, 2026
…dresses (SSRF fix) (google#2292)

* transport: block redirects from token server to private addresses

refreshOauth and refreshBasic constructed http.Client without a
CheckRedirect handler, allowing a malicious token server to issue an
HTTP redirect to an internal address (e.g. 169.254.169.254) that would
pass the initial validateRealmURL check but still reach private services.

Add realmRedirectCheck, which applies the same scheme and private-IP
restrictions as validateRealmURL to every redirect hop during a
token-fetch request. Wire it as CheckRedirect on both clients.

Add TestTokenServerRedirectSSRF to confirm the redirect is rejected.

* transport: address review feedback

- Shorten realmRedirectCheck doc comment.
- Extract allowInsecure local in refreshOauth and refreshBasic.
- Drop redundant inline comment in TestTokenServerRedirectSSRF.

* transport: rename unused handler param to satisfy revive linter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants