diff --git a/pkg/distribution/oci/remote/guard_internal_test.go b/pkg/distribution/oci/remote/guard_internal_test.go index 6e3ca5276..c28783593 100644 --- a/pkg/distribution/oci/remote/guard_internal_test.go +++ b/pkg/distribution/oci/remote/guard_internal_test.go @@ -1,8 +1,10 @@ package remote import ( + "fmt" "net/http" "net/http/httptest" + "net/url" "sync/atomic" "testing" ) @@ -26,6 +28,52 @@ func TestNewGuardedAuthClientBlocksLoopback(t *testing.T) { } } +// TestGuardedAuthClientHonorsProxyOnPrivateAddress reproduces the proxied +// deployment regression: production transports carry Proxy settings +// (server.go sets http.ProxyFromEnvironment), and proxies commonly live on +// private or loopback addresses. The guard must validate the realm host, not +// the proxy's address — otherwise every token fetch behind a proxy fails and +// model pulls break. +func TestGuardedAuthClientHonorsProxyOnPrivateAddress(t *testing.T) { + var proxiedURLs []string + proxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + proxiedURLs = append(proxiedURLs, r.RequestURI) + w.Header().Set("Content-Type", "application/json") + fmt.Fprintln(w, `{"token":"via-proxy"}`) + })) + defer proxy.Close() + + proxyURL, err := url.Parse(proxy.URL) + if err != nil { + t.Fatalf("parsing proxy URL: %v", err) + } + base := http.DefaultTransport.(*http.Transport).Clone() + base.Proxy = http.ProxyURL(proxyURL) + + client := newGuardedAuthClient(base) + + // A public realm must be reachable through the loopback proxy. 203.0.113.0/24 + // is TEST-NET-3: never routable, so a hit proves the request went via the proxy. + resp, err := client.Get("http://203.0.113.10/token") //nolint:noctx + if err != nil { + t.Fatalf("token fetch through a loopback proxy should succeed, got: %v", err) + } + resp.Body.Close() + if len(proxiedURLs) != 1 || proxiedURLs[0] != "http://203.0.113.10/token" { + t.Errorf("expected exactly the realm request to go through the proxy, got %v", proxiedURLs) + } + + // A disallowed realm must still be rejected before reaching the proxy. + resp, err = client.Get("http://169.254.169.254/token") //nolint:noctx + if err == nil { + resp.Body.Close() + t.Fatal("a link-local realm must be rejected even when a proxy is configured") + } + if len(proxiedURLs) != 1 { + t.Errorf("the disallowed realm request must not reach the proxy, got %v", proxiedURLs) + } +} + func TestResolveAndValidateHost(t *testing.T) { disallowed := []string{ "127.0.0.1", diff --git a/pkg/distribution/oci/remote/transport.go b/pkg/distribution/oci/remote/transport.go index 805791e9d..40b0b75ed 100644 --- a/pkg/distribution/oci/remote/transport.go +++ b/pkg/distribution/oci/remote/transport.go @@ -2,7 +2,6 @@ package remote import ( "context" - "crypto/tls" "encoding/json" "fmt" "io" @@ -91,37 +90,23 @@ func isDisallowedIP(ip net.IP) bool { return false } -// resolveAndValidateRealm parses the realm URL, validates the hostname against -// a static blocklist, and resolves it to a dial address (ip:port) that is safe -// to connect to. By returning the resolved IP, callers can use a custom -// DialContext to connect to that exact address — preventing DNS-rebinding -// attacks where a malicious DNS server could return different IPs for -// successive lookups (TOCTOU). -// -// If hostname is a literal IP address it is validated directly without -// triggering a DNS lookup. -func resolveAndValidateRealm(rawURL string) (dialAddr, hostname string, err error) { - u, err := url.Parse(rawURL) - if err != nil { - return "", "", fmt.Errorf("invalid realm URL: %w", err) - } - - hostname = u.Hostname() +// validateTokenEndpointURL validates the host of a token-endpoint URL against +// the internal-hostname blocklist and the private/loopback/link-local ranges. +// The local DNS resolution this performs is deliberate even when a proxy will +// resolve the name itself: checking the resolved IPs is the validation, and a +// name that cannot be resolved locally is rejected (fail closed) rather than +// forwarded unchecked. +func validateTokenEndpointURL(u *url.URL) error { port := u.Port() if port == "" { - switch u.Scheme { - case "https": + if u.Scheme == "https" { port = "443" - default: + } else { port = "80" } } - - dialAddr, err = resolveAndValidateHost(hostname, port) - if err != nil { - return "", "", err - } - return dialAddr, hostname, nil + _, err := resolveAndValidateHost(u.Hostname(), port) + return err } // resolveAndValidateHost validates hostname against the internal-hostname @@ -164,26 +149,31 @@ func resolveAndValidateHost(hostname, port string) (dialAddr string, err error) 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. +// newGuardedAuthClient returns the HTTP client used to fetch bearer tokens, +// both by containerd's authorizer (via docker.WithAuthClient) and by the +// hand-rolled Exchange(). The realm URL in a registry's WWW-Authenticate +// challenge is attacker-controlled, so every request this client makes is +// validated against the internal-hostname blocklist and the private/loopback/ +// link-local IP ranges before a connection is established. +// +// How the connection is guarded depends on whether a proxy applies to the +// request (see guardedAuthTransport). A dial-time-only guard would break every +// proxied deployment: with a proxy configured, the dialer sees the proxy's +// 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 { - var cloned *http.Transport + var proxied *http.Transport if t, ok := base.(*http.Transport); ok { - cloned = t.Clone() + proxied = t.Clone() } else if dt, ok := http.DefaultTransport.(*http.Transport); ok { - cloned = dt.Clone() + proxied = dt.Clone() } else { - cloned = &http.Transport{} + proxied = &http.Transport{} } - cloned.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { + direct := proxied.Clone() + direct.Proxy = nil + direct.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) @@ -195,37 +185,38 @@ func newGuardedAuthClient(base http.RoundTripper) *http.Client { return (&net.Dialer{}).DialContext(ctx, network, dialAddr) } - return &http.Client{Transport: cloned} + return &http.Client{Transport: &guardedAuthTransport{proxied: proxied, direct: direct}} } -// buildSafeTransport wraps base with a custom DialContext that connects -// directly to dialAddr (a pre-validated "ip:port") instead of relying on DNS -// resolution. This closes the TOCTOU window between realm URL validation and -// the actual connection. For TLS connections, serverName is set as the SNI -// value so that certificate validation still uses the original hostname. -func buildSafeTransport(base http.RoundTripper, dialAddr, serverName string) (http.RoundTripper, error) { - if base == nil { - base = http.DefaultTransport - } - - t, ok := base.(*http.Transport) - if !ok { - return nil, fmt.Errorf("cannot build safe transport from base of type %T; only *http.Transport is supported", base) - } - - dial := func(ctx context.Context, network, _ string) (net.Conn, error) { - return (&net.Dialer{}).DialContext(ctx, network, dialAddr) - } +// guardedAuthTransport validates every token-endpoint request against the SSRF +// blocklist, choosing the enforcement point per request: +// +// - Direct connections use a transport whose dialer validates the resolved +// IP just before connecting and dials that exact address, so DNS rebinding +// cannot slip an internal address past the check. +// - Proxied connections go through a transport with the proxy configuration +// 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) + if err != nil { + return nil, fmt.Errorf("determining proxy for token endpoint: %w", err) + } + if proxyURL != nil { + if err := validateTokenEndpointURL(req.URL); err != nil { + return nil, fmt.Errorf("realm URL rejected: %w", err) + } + return g.proxied.RoundTrip(req) + } } - cloned.TLSClientConfig.ServerName = serverName - return cloned, nil + return g.direct.RoundTrip(req) } // Ping pings a registry and returns authentication information. @@ -293,22 +284,12 @@ func parseWWWAuthenticate(header string) WWWAuthenticate { return result } -// Exchange exchanges credentials for a bearer token. +// Exchange exchanges credentials for a bearer token. The realm URL comes from +// the registry's WWW-Authenticate challenge and is therefore untrusted; the +// guarded client rejects realms on internal hostnames or private/loopback +// addresses and honors any configured proxy. func Exchange(ctx context.Context, _ reference.Registry, auth authn.Authenticator, transport http.RoundTripper, scopes []string, pr *PingResponse) (*Token, error) { - if transport == nil { - transport = http.DefaultTransport - } - - dialAddr, hostname, err := resolveAndValidateRealm(pr.WWWAuthenticate.Realm) - if err != nil { - return nil, fmt.Errorf("realm URL rejected: %w", err) - } - - safeTransport, err := buildSafeTransport(transport, dialAddr, hostname) - if err != nil { - return nil, fmt.Errorf("failed to build safe transport: %w", err) - } - client := &http.Client{Transport: safeTransport} + client := newGuardedAuthClient(transport) // Build token request URL tokenURL, err := url.Parse(pr.WWWAuthenticate.Realm) @@ -316,6 +297,13 @@ func Exchange(ctx context.Context, _ reference.Registry, auth authn.Authenticato return nil, fmt.Errorf("parsing realm URL: %w", err) } + // Validate the realm before any request is made so a blocked realm fails + // fast with a clear error. The guarded client re-validates at connection + // time (or per request when proxied), closing the TOCTOU window. + if err := validateTokenEndpointURL(tokenURL); err != nil { + return nil, fmt.Errorf("realm URL rejected: %w", err) + } + q := tokenURL.Query() if pr.WWWAuthenticate.Service != "" { q.Set("service", pr.WWWAuthenticate.Service)