Skip to content

fix(distribution): honor proxies in the guarded token-exchange client - #1040

Merged
ilopezluna merged 2 commits into
mainfrom
fix/guarded-auth-client-proxy
Aug 12, 2026
Merged

fix(distribution): honor proxies in the guarded token-exchange client#1040
ilopezluna merged 2 commits into
mainfrom
fix/guarded-auth-client-proxy

Conversation

@ilopezluna

Copy link
Copy Markdown
Contributor

Problem

#1038 guarded the token-exchange HTTP client by validating the address handed to DialContext. With a proxy configured — which production transports always carry via http.ProxyFromEnvironment (pkg/server/server.go) — that address is the proxy's, not the token realm's. Proxies commonly live on private or loopback addresses (corporate proxies, Docker Desktop's embedded proxy at 192.168.65.x), so the guard rejected the proxy itself and every token fetch failed:

failed to authorize: failed to fetch anonymous token:
Get "https://auth.docker.io/token?...": proxyconnect tcp:
realm URL contains a disallowed IP address 127.0.0.1

Model pulls from any authenticated registry — Docker Hub included — broke in proxied deployments. CI stayed green because CI runs without a proxy.

Fix

newGuardedAuthClient now returns a client backed by a guardedAuthTransport that picks the enforcement point per request:

  • Direct connections keep the validating dialer that resolves the realm host, validates every IP, and dials exactly the validated address — the DNS-rebinding protection is unchanged.
  • Proxied connections validate the realm host at the request level (internal-hostname blocklist + private/loopback/link-local ranges) and let the proxy make the connection: the proxy is the one dialing, so pinning the dial address is neither possible nor meaningful there.

Exchange() (the hand-rolled token exchange used by the push flow) now uses the same guarded client, replacing buildSafeTransport/resolveAndValidateRealm. This also fixes a latent bug on that path: it dialed the realm's IP directly, silently bypassing any configured proxy, so push broke in proxy-only networks too. The early realm URL rejected error contract is preserved.

Verification

  • Reproduced against Docker Hub: a pull through a CONNECT proxy on 127.0.0.1 failed before this change with realm URL contains a disallowed IP address 127.0.0.1 and succeeds after it (token fetch goes through the proxy). A direct pull (manifest + both layers, 91 MB) keeps working.
  • New hermetic regression test TestGuardedAuthClientHonorsProxyOnPrivateAddress: with a loopback proxy configured, a public realm is fetched through the proxy while a link-local realm is still rejected before reaching it.
  • All existing SSRF tests pass unchanged: TestPullSSRF_RealmNotFollowedToInternalService, TestExchangeSSRF_*, TestNewGuardedAuthClientBlocksLoopback, and the endpoint-level e2e in pkg/inference/models.
  • go test -race ./pkg/distribution/... ./pkg/inference/models green, golangci-lint clean.

🤖 Generated with Claude Code

The SSRF guard added for the pull and re-challenge paths validated the
address handed to DialContext. With a proxy configured — which production
transports always carry via http.ProxyFromEnvironment — that address is
the proxy's, not the token realm's. Proxies commonly live on private or
loopback addresses (corporate proxies, Docker Desktop's embedded proxy),
so the guard rejected the proxy itself and every token fetch failed:

    failed to fetch anonymous token: Get "https://auth.docker.io/token?...":
    proxyconnect tcp: realm URL contains a disallowed IP address 127.0.0.1

Model pulls from any authenticated registry, Docker Hub included, broke
in proxied deployments.

Split the guarded client per request: direct connections keep the
validating dialer pinned to the resolved IP (DNS-rebinding safe), while
proxied connections validate the realm host at the request level and let
the proxy connect. Exchange() now uses the same guarded client, which
also fixes the hand-rolled push path silently bypassing the proxy by
dialing the realm directly.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ilopezluna
ilopezluna marked this pull request as ready for review August 12, 2026 09:34

@sourcery-ai sourcery-ai Bot 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.

Hey - I've found 1 issue, and left some high level feedback:

  • In validateTokenEndpointURL you call resolveAndValidateHost and discard the returned dial address, which forces an extra DNS lookup even when a proxy will perform resolution; consider separating host/IP validation from resolution so proxied requests don’t incur unnecessary lookups or leak DNS queries.
  • The new guardedAuthTransport silently falls back to a zero-value http.Transport when base and http.DefaultTransport are not *http.Transport, which may change behavior (timeouts, TLS config, etc.); you might want to either restrict newGuardedAuthClient to *http.Transport inputs with an explicit error or preserve more of the original RoundTripper semantics.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `validateTokenEndpointURL` you call `resolveAndValidateHost` and discard the returned dial address, which forces an extra DNS lookup even when a proxy will perform resolution; consider separating host/IP validation from resolution so proxied requests don’t incur unnecessary lookups or leak DNS queries.
- The new `guardedAuthTransport` silently falls back to a zero-value `http.Transport` when `base` and `http.DefaultTransport` are not `*http.Transport`, which may change behavior (timeouts, TLS config, etc.); you might want to either restrict `newGuardedAuthClient` to `*http.Transport` inputs with an explicit error or preserve more of the original `RoundTripper` semantics.

## Individual Comments

### Comment 1
<location path="pkg/distribution/oci/remote/transport.go" line_range="209-211" />
<code_context>
+//     intact and a stock dialer: the proxy is the one connecting to the realm,
+//     so pinning the dial address is neither possible nor meaningful. The
+//     realm host is validated here at the request level instead.
+type guardedAuthTransport struct {
+	proxied *http.Transport // proxy settings intact, stock dialer
+	direct  *http.Transport // no proxy, validating dialer pinned to the resolved IP
+}

-	cloned := t.Clone()
-	cloned.DialContext = dial
-	if cloned.TLSClientConfig == nil {
-		cloned.TLSClientConfig = &tls.Config{MinVersion: tls.VersionTLS12} //nolint:gosec
-	} else {
-		cloned.TLSClientConfig = cloned.TLSClientConfig.Clone()
+func (g *guardedAuthTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	if g.proxied.Proxy != nil {
+		proxyURL, err := g.proxied.Proxy(req)
</code_context>
<issue_to_address>
**suggestion:** Make error messages from request-level validation more contextual and consistent.

In the proxied path, `validateTokenEndpointURL(req.URL)` returns its error directly, while `Exchange` wraps the same validator error with `fmt.Errorf("realm URL rejected: %w", err)` for better context. For proxied requests, this bare error makes it harder to understand why a request was rejected. Consider wrapping the error here (e.g. `fmt.Errorf("realm URL %q rejected: %w", req.URL, err)`) to match the other call site and improve diagnosability without changing behavior.

```suggestion
			if err := validateTokenEndpointURL(req.URL); err != nil {
				return nil, fmt.Errorf("realm URL %q rejected: %w", req.URL, err)
			}
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread pkg/distribution/oci/remote/transport.go
Wrap the proxied-path validation error as "realm URL rejected" to match
Exchange(), and document that the local DNS resolution during proxied
validation is a deliberate fail-closed choice.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ilopezluna
ilopezluna requested a review from a team August 12, 2026 10:11
@ilopezluna
ilopezluna merged commit 5a3112f into main Aug 12, 2026
14 checks passed
@ilopezluna
ilopezluna deleted the fix/guarded-auth-client-proxy branch August 12, 2026 10:13
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.

2 participants