Fix token realm SSRF guard to allow registries on the same host - #1053
Fix token realm SSRF guard to allow registries on the same host#1053okhowang wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="pkg/distribution/oci/remote/remote.go" line_range="428" />
<code_context>
func createResolver(o *options, ref reference.Reference) resolverComponents {
authorizer := docker.NewDockerAuthorizer(
docker.WithAuthCreds(credentialsFunc(o, ref)),
- docker.WithAuthClient(newGuardedAuthClient(o.transport)))
+ docker.WithAuthClient(newGuardedAuthClient(o.transport, ref.Context().Registry.RegistryStr())))
</code_context>
<issue_to_address>
**issue (broader_impact):** The resolver's guarded auth client compares a token realm against the original reference registry, even when the resolver is contacting a configured mirror. A mirror at `localhost`, `127.0.0.1`, or an RFC1918 host therefore has its same-host realm rejected because it is compared with `docker.io`/`registry-1.docker.io`, so authentication through internal Docker Hub mirrors fails.
**Triggers:** When `WithRegistryMirrors` is configured and the mirror's token endpoint is on the mirror host.
**Suggested fix:** Pass the actual `RegistryHost.Host` being contacted into the guarded auth client, or construct the auth client/guard with the host per resolver host rather than once from `ref.Context().Registry`.
</issue_to_address>
### Comment 2
<location path="pkg/distribution/oci/remote/transport.go" line_range="199-202" />
<code_context>
// address — commonly a private or loopback IP — rather than the realm's, and
// would reject the proxy itself.
-func newGuardedAuthClient(base http.RoundTripper) *http.Client {
+func newGuardedAuthClient(base http.RoundTripper, registryHost string) *http.Client {
var proxied *http.Transport
if t, ok := base.(*http.Transport); ok {
</code_context>
<issue_to_address>
**nitpick:** The function comment still says every token request is validated against the internal-hostname and private-address blocklists, but the new same-host path deliberately bypasses both checks. The documentation is false for realms matching `registryHost`, obscuring that `localhost` and private addresses are now explicitly permitted.
**Triggers:** When callers or security reviewers rely on the function comment to understand the SSRF guarantees.
**Suggested fix:** Update the comment to state that validation is skipped for realms whose host matches `registryHost`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| func createResolver(o *options, ref reference.Reference) resolverComponents { | ||
| authorizer := docker.NewDockerAuthorizer( | ||
| docker.WithAuthCreds(credentialsFunc(o, ref)), | ||
| docker.WithAuthClient(newGuardedAuthClient(o.transport))) |
There was a problem hiding this comment.
issue (broader_impact): The resolver's guarded auth client compares a token realm against the original reference registry, even when the resolver is contacting a configured mirror. A mirror at localhost, 127.0.0.1, or an RFC1918 host therefore has its same-host realm rejected because it is compared with docker.io/registry-1.docker.io, so authentication through internal Docker Hub mirrors fails.
Triggers: When WithRegistryMirrors is configured and the mirror's token endpoint is on the mirror host.
Suggested fix: Pass the actual RegistryHost.Host being contacted into the guarded auth client, or construct the auth client/guard with the host per resolver host rather than once from ref.Context().Registry.
| func newGuardedAuthClient(base http.RoundTripper, registryHost string) *http.Client { | ||
| var proxied *http.Transport | ||
| if t, ok := base.(*http.Transport); ok { | ||
| proxied = t.Clone() |
There was a problem hiding this comment.
nitpick: The function comment still says every token request is validated against the internal-hostname and private-address blocklists, but the new same-host path deliberately bypasses both checks. The documentation is false for realms matching registryHost, obscuring that localhost and private addresses are now explicitly permitted.
Triggers: When callers or security reviewers rely on the function comment to understand the SSRF guarantees.
Suggested fix: Update the comment to state that validation is skipped for realms whose host matches registryHost.
Close #1050
Description
PR #1038 added an SSRF guard that blocks token-exchange realms
(
WWW-Authenticate: Bearer realm=...) resolving to private/loopback/link-localaddresses. That blocklist is too broad: it also rejects internal/corporate
registries whose token endpoint lives on an RFC1918 network (or on
localhost), because their realm resolves to a private IP. Such registries area common enterprise deployment (internal Harbor/Artifactory/HuggingFace
mirrors), and after #1038 pulls from them fail with
realm URL rejected.This PR narrows the guard so a realm on the same host as the registry being
pulled is permitted, while still blocking a registry from pivoting the client
to a different internal host (e.g. the cloud metadata service
169.254.169.254).Why this is safe
The SSRF threat is a cross-trust-domain pivot: a registry at host A returns a
realmpointing at host B (metadata / internal admin) that the client canreach but the registry cannot. When the realm host equals the registry host, the
token request stays within the same trust domain the user explicitly chose to
pull from, so there is no pivot. Metadata and other internal hosts remain
blocked because they differ from the registry host.
Changes
validateTokenEndpointURL(u, registryHost): returns nil (skip the blocklist)when the realm host equals
registryHost.sameHost(realmHost, registryHost): case-insensitive, port-agnostic hostcomparison (
url.URL.Hostname()already strips ports/brackets).newGuardedAuthClient(base, registryHost): threads the registry host into theguarded transport; the direct dialer also skips the blocklist for same-host
realms (DNS-rebinding protection is preserved for cross-host cases).
Exchangeand the resolver (createResolver/createResolverWithPushScope)pass
ref.Context().Registry.RegistryStr()as the registry host.Tests
TestExchangeAllowsInternalRealmOnSameHost: same-host loopback realm allowed.TestPullSSRF_RealmNotFollowedToInternalService: reworked to assert thecross-host pivot to
169.254.169.254is still blocked on the pull path.""registryhost → full validation).
How to test
private IP /
localhost; pulling now succeeds (previouslyrealm URL rejected).realm=http://169.254.169.254/...is still rejected.Related
Created with: CodeBuddy