From 5ad45e546e502dae3ca8ab972e01a3b2946a417e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Tue, 11 Aug 2026 12:30:20 +0200 Subject: [PATCH 1/2] fix(distribution): validate token realm on pull and re-challenge paths CVE-2026-33990 was fixed only in the hand-rolled Exchange() used by the push flow. The pull path (remote.Image -> createResolver) and the push re-challenge authorizer build containerd's default authorizer, which follows the realm URL from a 401 WWW-Authenticate challenge without validating it, so a malicious registry can drive a token fetch at an internal address and turn Model Runner into an SSRF proxy. Guard the HTTP client containerd uses for token fetches via docker.WithAuthClient. Its dialer validates the resolved IP against the private/loopback/link-local blocklist and dials that exact address, covering the pull, push, and fallback resolvers uniformly instead of only the hand-rolled Exchange(). --- .../oci/remote/guard_internal_test.go | 52 ++++++++++++++ pkg/distribution/oci/remote/remote.go | 6 +- pkg/distribution/oci/remote/ssrf_pull_test.go | 50 +++++++++++++ pkg/distribution/oci/remote/transport.go | 70 +++++++++++++++---- 4 files changed, 161 insertions(+), 17 deletions(-) create mode 100644 pkg/distribution/oci/remote/guard_internal_test.go create mode 100644 pkg/distribution/oci/remote/ssrf_pull_test.go diff --git a/pkg/distribution/oci/remote/guard_internal_test.go b/pkg/distribution/oci/remote/guard_internal_test.go new file mode 100644 index 000000000..6e3ca5276 --- /dev/null +++ b/pkg/distribution/oci/remote/guard_internal_test.go @@ -0,0 +1,52 @@ +package remote + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +func TestNewGuardedAuthClientBlocksLoopback(t *testing.T) { + var hits atomic.Int32 + internalService := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.WriteHeader(http.StatusOK) + })) + defer internalService.Close() + + client := newGuardedAuthClient(nil) + resp, err := client.Get(internalService.URL) //nolint:noctx + if err == nil { + resp.Body.Close() + t.Fatal("guarded auth client should refuse to connect to a loopback token endpoint") + } + if got := hits.Load(); got != 0 { + t.Errorf("guarded auth client contacted the loopback service %d time(s); the dialer must reject it before connecting", got) + } +} + +func TestResolveAndValidateHost(t *testing.T) { + disallowed := []string{ + "127.0.0.1", + "10.1.2.3", + "172.16.0.1", + "192.168.1.1", + "169.254.169.254", + "::1", + "localhost", + "host.docker.internal", + "model-runner.docker.internal", + "gateway.docker.internal", + } + for _, host := range disallowed { + if _, err := resolveAndValidateHost(host, "443"); err == nil { + t.Errorf("resolveAndValidateHost(%q) = nil error; want rejection", host) + } + } + + // A public literal IP carries no DNS to rebind and must be accepted. + if _, err := resolveAndValidateHost("8.8.8.8", "443"); err != nil { + t.Errorf("resolveAndValidateHost(%q) = %v; want nil error", "8.8.8.8", err) + } +} diff --git a/pkg/distribution/oci/remote/remote.go b/pkg/distribution/oci/remote/remote.go index 7aa45a991..14f33491a 100644 --- a/pkg/distribution/oci/remote/remote.go +++ b/pkg/distribution/oci/remote/remote.go @@ -424,7 +424,8 @@ type resolverComponents struct { // createResolver creates a docker resolver with the given options. func createResolver(o *options, ref reference.Reference) resolverComponents { authorizer := docker.NewDockerAuthorizer( - docker.WithAuthCreds(credentialsFunc(o, ref))) + docker.WithAuthCreds(credentialsFunc(o, ref)), + docker.WithAuthClient(newGuardedAuthClient(o.transport))) // Wrap transport with Range header support for resumable downloads // and User-Agent header for registry compatibility (required by HuggingFace) @@ -528,7 +529,8 @@ func createResolverWithPushScope(o *options, ref reference.Reference) (resolverC return "", cfg.RegistryToken, nil } return cfg.Username, cfg.Password, nil - })) + }), + docker.WithAuthClient(newGuardedAuthClient(o.transport))) resolver := docker.NewResolver(docker.ResolverOptions{ Hosts: docker.ConfigureDefaultRegistries( diff --git a/pkg/distribution/oci/remote/ssrf_pull_test.go b/pkg/distribution/oci/remote/ssrf_pull_test.go new file mode 100644 index 000000000..1ac8ca6f4 --- /dev/null +++ b/pkg/distribution/oci/remote/ssrf_pull_test.go @@ -0,0 +1,50 @@ +package remote_test + +import ( + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/docker/model-runner/pkg/distribution/oci/reference" + "github.com/docker/model-runner/pkg/distribution/oci/remote" +) + +// TestPullSSRF_RealmNotFollowedToInternalService exercises the pull path end to +// end: a malicious registry answers every request with a 401 Bearer challenge +// whose realm points at a loopback "internal service". The token fetch that +// containerd's authorizer performs against that realm must be blocked, so the +// internal service is never contacted. This is the code path (remote.Image -> +// createResolver) that the original CVE-2026-33990 fix left unguarded. +func TestPullSSRF_RealmNotFollowedToInternalService(t *testing.T) { + var internalHits atomic.Int32 + internalService := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + internalHits.Add(1) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintln(w, `{"token":"leaked-via-ssrf"}`) + })) + defer internalService.Close() + + maliciousRegistry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", + fmt.Sprintf(`Bearer realm="%s/token",service="evil-registry"`, internalService.URL)) + w.WriteHeader(http.StatusUnauthorized) + })) + defer maliciousRegistry.Close() + + registryHost := strings.TrimPrefix(maliciousRegistry.URL, "http://") + ref, err := reference.ParseReference(registryHost + "/evil/model:latest") + if err != nil { + t.Fatalf("parsing reference: %v", err) + } + + _, err = remote.Image(ref, remote.WithContext(t.Context()), remote.WithPlainHTTP(true)) + if err == nil { + t.Fatal("remote.Image should have failed: the token realm resolves to a loopback address and must be rejected") + } + if hits := internalHits.Load(); hits != 0 { + t.Errorf("SSRF not blocked on the pull path: the internal service at %s was contacted %d time(s) via the token realm", internalService.URL, hits) + } +} diff --git a/pkg/distribution/oci/remote/transport.go b/pkg/distribution/oci/remote/transport.go index c63769c5e..805791e9d 100644 --- a/pkg/distribution/oci/remote/transport.go +++ b/pkg/distribution/oci/remote/transport.go @@ -117,31 +117,39 @@ func resolveAndValidateRealm(rawURL string) (dialAddr, hostname string, err erro } } - // Block well-known internal hostnames regardless of DNS resolution. + dialAddr, err = resolveAndValidateHost(hostname, port) + if err != nil { + return "", "", err + } + return dialAddr, hostname, nil +} + +// resolveAndValidateHost validates hostname against the internal-hostname +// blocklist and the private/loopback/link-local IP ranges, returning a dial +// address (ip:port) that is safe to connect to. Returning the resolved IP lets +// callers dial that exact address, closing the DNS-rebinding (TOCTOU) window +// between validation and connection. A literal IP hostname is validated +// directly without a DNS lookup. +func resolveAndValidateHost(hostname, port string) (dialAddr string, err error) { for _, internal := range internalHostnames { if strings.EqualFold(hostname, internal) { - return "", "", fmt.Errorf("realm URL hostname %q is not allowed", hostname) + return "", fmt.Errorf("realm URL hostname %q is not allowed", hostname) } } - // If the hostname is a literal IP address, validate it directly without a - // DNS lookup — there is no DNS to rebind. if ip := net.ParseIP(hostname); ip != nil { if isDisallowedIP(ip) { - return "", "", fmt.Errorf("realm URL contains a disallowed IP address %s", hostname) + return "", fmt.Errorf("realm URL contains a disallowed IP address %s", hostname) } - return net.JoinHostPort(hostname, port), hostname, nil + return net.JoinHostPort(hostname, port), nil } - // Resolve the hostname and validate every returned address. Using the - // resolved IP as the dial address prevents DNS rebinding: the same IP that - // passed validation is the one that will be used for the connection. ips, err := net.LookupHost(hostname) if err != nil { - return "", "", fmt.Errorf("resolving realm hostname %q: %w", hostname, err) + return "", fmt.Errorf("resolving realm hostname %q: %w", hostname, err) } if len(ips) == 0 { - return "", "", fmt.Errorf("realm hostname %q resolved to no addresses", hostname) + return "", fmt.Errorf("realm hostname %q resolved to no addresses", hostname) } for _, ipStr := range ips { ip := net.ParseIP(ipStr) @@ -149,13 +157,45 @@ func resolveAndValidateRealm(rawURL string) (dialAddr, hostname string, err erro continue } if isDisallowedIP(ip) { - return "", "", fmt.Errorf("realm URL resolves to a disallowed address %s", ipStr) + return "", fmt.Errorf("realm URL resolves to a disallowed address %s", ipStr) + } + } + + return net.JoinHostPort(ips[0], port), nil +} + +// newGuardedAuthClient returns the HTTP client that containerd's authorizer uses +// to fetch bearer tokens. containerd contacts the realm URL from a registry's +// WWW-Authenticate challenge with this client only (see the auth package's +// FetchToken/FetchTokenWithOAuth), so guarding its dialer blocks token-exchange +// SSRF on every path that builds an authorizer — the pull path, the push +// re-challenge path, and their fallbacks — rather than only the hand-rolled +// Exchange(). The dialer validates the resolved IP just before connecting and +// dials that exact address, so DNS rebinding cannot slip an internal address +// past the check. +func newGuardedAuthClient(base http.RoundTripper) *http.Client { + var cloned *http.Transport + if t, ok := base.(*http.Transport); ok { + cloned = t.Clone() + } else if dt, ok := http.DefaultTransport.(*http.Transport); ok { + cloned = dt.Clone() + } else { + cloned = &http.Transport{} + } + + cloned.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, fmt.Errorf("invalid token endpoint address %q: %w", addr, err) + } + dialAddr, err := resolveAndValidateHost(host, port) + if err != nil { + return nil, err } + return (&net.Dialer{}).DialContext(ctx, network, dialAddr) } - // All resolved IPs passed validation. Use the first one as the dial - // address so the HTTP client never performs a second DNS lookup. - return net.JoinHostPort(ips[0], port), hostname, nil + return &http.Client{Transport: cloned} } // buildSafeTransport wraps base with a custom DialContext that connects From bde712dc2ac0e14d104b879024cef2ab1abf5df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ignacio=20L=C3=B3pez=20Luna?= Date: Tue, 11 Aug 2026 15:14:24 +0200 Subject: [PATCH 2/2] test(distribution): add endpoint-level SSRF regression for model pull Drive the pull from POST /models/create through the manager, distribution client, and containerd resolver against a malicious registry that advertises a loopback token realm. Asserts the registry is contacted but the realm is never followed, so the internal service receives nothing. Fails without the WithAuthClient guard. --- pkg/inference/models/ssrf_e2e_test.go | 65 +++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 pkg/inference/models/ssrf_e2e_test.go diff --git a/pkg/inference/models/ssrf_e2e_test.go b/pkg/inference/models/ssrf_e2e_test.go new file mode 100644 index 000000000..909980a62 --- /dev/null +++ b/pkg/inference/models/ssrf_e2e_test.go @@ -0,0 +1,65 @@ +package models_test + +import ( + "fmt" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "github.com/docker/model-runner/pkg/inference/models" + "github.com/docker/model-runner/pkg/logging" +) + +// TestCreateModelSSRF_RealmNotFollowedToInternalService drives the pull from the +// unauthenticated HTTP surface a caller actually reaches — POST /models/create — +// all the way down through the manager, distribution client, and containerd +// resolver. A malicious registry answers every request with a 401 Bearer +// challenge whose realm points at a loopback "internal service". The registry +// itself must be contacted (proving the request reached the pull path), but the +// realm must never be followed, so the internal service receives nothing. +func TestCreateModelSSRF_RealmNotFollowedToInternalService(t *testing.T) { + var internalHits, registryHits atomic.Int32 + + internalService := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + internalHits.Add(1) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintln(w, `{"token":"leaked-via-ssrf"}`) + })) + defer internalService.Close() + + maliciousRegistry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + registryHits.Add(1) + w.Header().Set("WWW-Authenticate", + fmt.Sprintf(`Bearer realm="%s/token",service="evil-registry"`, internalService.URL)) + w.WriteHeader(http.StatusUnauthorized) + })) + defer maliciousRegistry.Close() + + log := logging.NewLogger(slog.LevelError) + manager := models.NewManager(log, models.ClientConfig{ + StoreRootPath: t.TempDir(), + Logger: log, + UserAgent: "model-runner-test", + PlainHTTP: true, + }) + apiServer := httptest.NewServer(models.NewHTTPHandler(log, manager, nil)) + defer apiServer.Close() + + registryHost := strings.TrimPrefix(maliciousRegistry.URL, "http://") + body := fmt.Sprintf(`{"from":%q}`, registryHost+"/evil/model:latest") + resp, err := http.Post(apiServer.URL+"/models/create", "application/json", strings.NewReader(body)) + if err != nil { + t.Fatalf("POST /models/create: %v", err) + } + resp.Body.Close() + + if got := registryHits.Load(); got == 0 { + t.Fatalf("test is inconclusive: the malicious registry was never contacted, so the pull path was not exercised") + } + if got := internalHits.Load(); got != 0 { + t.Errorf("SSRF not blocked end to end: the internal service at %s was contacted %d time(s) via the token realm advertised by the registry", internalService.URL, got) + } +}