x/transfer, server: tighten redirect handling for registry requests (#18512)

Redirects for registry and blob transfers now validate the target scheme
and resolved addresses before following, re-check DNS on each redirect,
and do not follow redirects that switch an https session to plain http.
The --insecure option continues to relax address checks for private
registries but not scheme checks.
This commit is contained in:
Patrick Devine
2026-09-17 14:21:28 -07:00
committed by GitHub
parent 35bdad61df
commit dfabde4539
8 changed files with 722 additions and 68 deletions
+45 -21
View File
@@ -1190,15 +1190,16 @@ func pullWithTransfer(ctx context.Context, n model.Name, layers []manifest.Layer
}
if err := transfer.Download(ctx, transfer.DownloadOptions{
Blobs: blobs,
BaseURL: baseURL,
DestDir: destDir,
Repository: n.DisplayNamespaceModel(),
BodyConcurrency: max(1, int(envconfig.MaxTransferStreams())),
Progress: progress,
Token: regOpts.Token,
GetToken: getToken,
Logger: slog.Default(),
Blobs: blobs,
BaseURL: baseURL,
DestDir: destDir,
Repository: n.DisplayNamespaceModel(),
BodyConcurrency: max(1, int(envconfig.MaxTransferStreams())),
Progress: progress,
Token: regOpts.Token,
GetToken: getToken,
Logger: slog.Default(),
AllowPrivateHosts: regOpts != nil && regOpts.Insecure,
}); err != nil {
return err
}
@@ -1267,17 +1268,18 @@ func pushWithTransfer(ctx context.Context, n model.Name, layers []manifest.Layer
}
return transfer.Upload(ctx, transfer.UploadOptions{
Blobs: blobs,
BaseURL: baseURL,
SrcDir: srcDir,
BodyConcurrency: max(1, int(envconfig.MaxTransferStreams())),
Progress: progress,
Token: regOpts.Token,
GetToken: getToken,
Logger: slog.Default(),
Manifest: manifestJSON,
ManifestRef: n.Tag,
Repository: n.DisplayNamespaceModel(),
Blobs: blobs,
BaseURL: baseURL,
SrcDir: srcDir,
BodyConcurrency: max(1, int(envconfig.MaxTransferStreams())),
Progress: progress,
Token: regOpts.Token,
GetToken: getToken,
Logger: slog.Default(),
Manifest: manifestJSON,
ManifestRef: n.Tag,
Repository: n.DisplayNamespaceModel(),
AllowPrivateHosts: regOpts != nil && regOpts.Insecure,
})
}
@@ -1379,6 +1381,8 @@ func makeRequestWithRetry(ctx context.Context, method string, requestURL *url.UR
// structured in a way that makes this easy, so this will have to do for now.
var testMakeRequestDialContext func(ctx context.Context, network, addr string) (net.Conn, error)
var errBlockedRedirect = errors.New("blocked redirect to a different host")
func makeRequest(ctx context.Context, method string, requestURL *url.URL, headers http.Header, body io.Reader, regOpts *registryOptions) (*http.Response, error) {
if requestURL.Scheme != "http" && regOpts != nil && regOpts.Insecure {
requestURL.Scheme = "http"
@@ -1412,8 +1416,28 @@ func makeRequest(ctx context.Context, method string, requestURL *url.URL, header
req.ContentLength = contentLength
}
var checkRedirect func(req *http.Request, via []*http.Request) error
if regOpts != nil {
checkRedirect = regOpts.CheckRedirect
}
if checkRedirect == nil {
insecure := regOpts != nil && regOpts.Insecure
// Default redirect policy: same-host only, so a registry can't steer
// manifest or blob requests at internal addresses. --insecure opts out
// for trusted LAN/local registries.
checkRedirect = func(req *http.Request, via []*http.Request) error {
if len(via) > 10 {
return errMaxRedirectsExceeded
}
if !insecure && req.URL.Host != via[0].URL.Host {
return errBlockedRedirect
}
return nil
}
}
c := &http.Client{
CheckRedirect: regOpts.CheckRedirect,
CheckRedirect: checkRedirect,
}
if testMakeRequestDialContext != nil {
tr := http.DefaultTransport.(*http.Transport).Clone()
+52
View File
@@ -9,6 +9,7 @@ import (
"net/url"
"os"
"strings"
"sync/atomic"
"testing"
"time"
@@ -901,3 +902,54 @@ func TestPullModelDuplicateDigestVerifiesBlob(t *testing.T) {
t.Fatalf("PullModel = %v, want errDigestMismatch (unverified blob would persist)", err)
}
}
// TestPullManifestRejectsCrossHostRedirect: a manifest GET that the registry
// redirects to a different host must be refused by default, so a malicious
// registry can't turn a pull into a request to an internal address.
// --insecure opts out for trusted registries.
func TestPullManifestRejectsCrossHostRedirect(t *testing.T) {
t.Setenv("OLLAMA_MODELS", t.TempDir())
var internalHit atomic.Bool
internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
internalHit.Store(true)
}))
defer internal.Close()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, internal.URL+r.URL.Path, http.StatusFound)
}))
defer ts.Close()
requestURL, err := url.Parse(ts.URL + "/v2/test/attack/manifests/latest")
if err != nil {
t.Fatal(err)
}
// Default policy: cross-host redirect is refused before any request
// leaves for the internal host. (regOpts nil exercises the makeRequest
// default; the insecure protocol check doesn't apply at this level.)
blockedResp, err := makeRequest(t.Context(), http.MethodGet, requestURL, nil, nil, &registryOptions{})
// On a CheckRedirect failure the client returns the pre-redirect
// response with its body already closed; close again defensively to
// satisfy bodyclose (double close is a no-op).
if blockedResp != nil && blockedResp.Body != nil {
blockedResp.Body.Close()
}
if !errors.Is(err, errBlockedRedirect) {
t.Fatalf("makeRequest = %v, want errBlockedRedirect", err)
}
if internalHit.Load() {
t.Fatal("internal host received a request despite the blocked redirect")
}
// Insecure opts out: the cross-host redirect is followed.
resp, err := makeRequest(t.Context(), http.MethodGet, requestURL, nil, nil, &registryOptions{Insecure: true})
if err != nil {
t.Fatalf("makeRequest with Insecure = %v, want redirect followed", err)
}
resp.Body.Close()
if !internalHit.Load() {
t.Fatal("redirect target was not reached with Insecure set")
}
}
+10 -2
View File
@@ -39,6 +39,7 @@ type downloader struct {
progress *progressTracker
speeds *speedTracker
logger *slog.Logger
allowPrivate bool
// bodySem caps the number of simultaneous body-bearing transfers so a
// modest home downlink isn't saturated. Always set by download(); nil
// only when tests build downloader directly (in which case holdBody is
@@ -120,7 +121,7 @@ func download(ctx context.Context, opts DownloadOptions) error {
progress.add(alreadyCompleted) // Report already-downloaded bytes upfront
d := &downloader{
client: cmp.Or(opts.Client, defaultClient),
client: cmp.Or(opts.Client, checkedClient(opts.BaseURL, opts.AllowPrivateHosts)),
baseURL: opts.BaseURL,
destDir: opts.DestDir,
repository: cmp.Or(opts.Repository, "library/_"),
@@ -131,6 +132,7 @@ func download(ctx context.Context, opts DownloadOptions) error {
progress: progress,
speeds: &speedTracker{},
logger: opts.Logger,
allowPrivate: opts.AllowPrivateHosts,
}
// 0 or negative serializes; never unbounded.
d.bodySem = semaphore.NewWeighted(int64(max(1, opts.BodyConcurrency)))
@@ -211,7 +213,7 @@ func (d *downloader) download(ctx context.Context, blob Blob) error {
}
lastErr = err
}
return fmt.Errorf("%w: %v", errMaxRetriesExceeded, lastErr)
return fmt.Errorf("%w: %w", errMaxRetriesExceeded, lastErr)
}
func (d *downloader) downloadOnce(ctx context.Context, blob Blob) (int64, error) {
@@ -428,7 +430,13 @@ func (d *downloader) resolve(ctx context.Context, rawURL string) (*url.URL, erro
}
case http.StatusTemporaryRedirect, http.StatusFound, http.StatusMovedPermanently:
loc, _ := resp.Location()
if err := validateRedirectScheme(loc, d.baseURL); err != nil {
return nil, err
}
if loc.Host != u.Host {
if err := validateRedirectTarget(ctx, loc, d.baseURL, d.allowPrivate); err != nil {
return nil, err
}
return loc, nil
}
u = loc
+174
View File
@@ -0,0 +1,174 @@
package transfer
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"net/url"
"strings"
"time"
)
var errRedirectNotAllowed = errors.New("redirect target not allowed")
// blockedIPv4Nets are globally-unicast-but-not-public IPv4 ranges that
// net.IP's helpers don't classify: CGNAT (RFC 6598), which carries real
// internal services like Alibaba Cloud metadata (100.100.2.148), and the
// benchmarking block (RFC 2544).
var blockedIPv4Nets = []*net.IPNet{parseIPv4Net("100.64.0.0/10"), parseIPv4Net("198.18.0.0/15")}
func parseIPv4Net(s string) *net.IPNet {
_, n, err := net.ParseCIDR(s)
if err != nil {
panic(err)
}
return n
}
// isPublicIP reports whether ip is a globally routed address.
func isPublicIP(ip net.IP) bool {
if !ip.IsGlobalUnicast() ||
ip.IsLoopback() ||
ip.IsPrivate() ||
ip.IsLinkLocalUnicast() ||
ip.IsLinkLocalMulticast() ||
ip.IsUnspecified() {
return false
}
// To4() unwraps 4-in-6 mapped addresses so mapped CGNAT is blocked too.
if v4 := ip.To4(); v4 != nil {
for _, n := range blockedIPv4Nets {
if n.Contains(v4) {
return false
}
}
}
return true
}
// validateRedirectScheme rejects redirects that downgrade an https session
// to cleartext http, even when the target host is unchanged — a hostile or
// compromised registry must not be able to strip TLS off follow-up requests.
// It applies even under allowPrivate (the --insecure opt-in), which relaxes
// address checks but never scheme checks.
func validateRedirectScheme(loc *url.URL, baseURL string) error {
if loc == nil {
return fmt.Errorf("%w: missing Location", errRedirectNotAllowed)
}
if loc.Scheme != "https" && loc.Scheme != "http" {
return fmt.Errorf("%w: scheme %q", errRedirectNotAllowed, loc.Scheme)
}
// http is only acceptable when the registry itself was already reached
// over plain http (i.e. the caller opted into an insecure registry).
base, _ := url.Parse(baseURL)
if loc.Scheme == "http" && base != nil && base.Scheme == "https" {
return fmt.Errorf("%w: https registry redirecting to http", errRedirectNotAllowed)
}
return nil
}
// validateRedirectTarget rejects redirect targets that aren't public HTTPS
// endpoints, unless allowPrivate is set.
func validateRedirectTarget(ctx context.Context, loc *url.URL, baseURL string, allowPrivate bool) error {
if loc == nil {
return fmt.Errorf("%w: missing Location", errRedirectNotAllowed)
}
if allowPrivate {
return nil
}
if err := validateRedirectScheme(loc, baseURL); err != nil {
return err
}
host := loc.Hostname()
if ip := net.ParseIP(host); ip != nil {
if !isPublicIP(ip) {
return fmt.Errorf("%w: %s is not a public address", errRedirectNotAllowed, ip)
}
return nil
}
ips, err := net.DefaultResolver.LookupIP(ctx, "ip", host)
if err != nil {
return fmt.Errorf("%w: resolving %s: %w", errRedirectNotAllowed, host, err)
}
if len(ips) == 0 {
return fmt.Errorf("%w: %s has no addresses", errRedirectNotAllowed, host)
}
for _, ip := range ips {
if !isPublicIP(ip) {
return fmt.Errorf("%w: %s resolves to non-public %s", errRedirectNotAllowed, host, ip)
}
}
return nil
}
// checkedDialer resolves, validates, and dials a pinned IP so DNS rebinding
// can't swap a private address in after validation.
func checkedDialer(d *net.Dialer, allowPrivate bool) func(ctx context.Context, network, addr string) (net.Conn, error) {
return func(ctx context.Context, network, addr string) (net.Conn, error) {
host, port, err := net.SplitHostPort(addr)
if err != nil {
return nil, err
}
dialHost := host
if ip := net.ParseIP(host); ip == nil {
ips, err := d.Resolver.LookupIP(ctx, "ip", host)
if err != nil {
return nil, err
}
if len(ips) == 0 {
return nil, fmt.Errorf("no addresses for %s", host)
}
for _, ip := range ips {
if !allowPrivate && !isPublicIP(ip) {
return nil, fmt.Errorf("%w: %s resolves to non-public %s", errRedirectNotAllowed, host, ip)
}
}
dialHost = ips[0].String()
} else if !allowPrivate && !isPublicIP(ip) {
return nil, fmt.Errorf("%w: %s is not a public address", errRedirectNotAllowed, ip)
}
conn, err := d.DialContext(ctx, network, net.JoinHostPort(dialHost, port))
if err != nil {
return nil, err
}
if tc, ok := conn.(*net.TCPConn); ok {
tc.SetKeepAlive(true)
tc.SetKeepAlivePeriod(3 * time.Minute)
}
return conn, nil
}
}
// checkedClient returns an HTTP client using checkedDialer; the registry
// base host is exempt since the caller explicitly directed traffic at it.
func checkedClient(baseURL string, allowPrivate bool) *http.Client {
var baseHostname string
if b, err := url.Parse(baseURL); err == nil {
baseHostname = b.Hostname()
}
return &http.Client{
Transport: &http.Transport{
MaxIdleConns: 100,
MaxIdleConnsPerHost: 100,
IdleConnTimeout: 90 * time.Second,
// Custom DialContext disables HTTP/2 auto-configuration;
// ForceAttemptHTTP2 opts back in.
ForceAttemptHTTP2: true,
DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
host, _, err := net.SplitHostPort(addr)
if err == nil && baseHostname != "" && strings.EqualFold(host, baseHostname) {
return new(net.Dialer).DialContext(ctx, network, addr)
}
return checkedDialer(&net.Dialer{Timeout: 30 * time.Second, KeepAlive: 3 * time.Minute}, allowPrivate)(ctx, network, addr)
},
},
CheckRedirect: func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
},
}
}
+370
View File
@@ -0,0 +1,370 @@
package transfer
import (
"context"
"errors"
"fmt"
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path/filepath"
"strings"
"sync/atomic"
"testing"
"time"
)
// SSRF regression tests for CVE-2026-85180: a registry can answer a blob
// request with a redirect to an internal address. By default the transfer
// package must refuse to fetch it; AllowPrivateHosts opts out for trusted
// LAN/local registries.
func TestDownloadRedirectToPrivateHostDenied(t *testing.T) {
blob, _ := createTestBlob(t, t.TempDir(), 1024)
var internalHit atomic.Bool
internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
internalHit.Store(true)
}))
defer internal.Close()
registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, internal.URL+r.URL.Path, http.StatusTemporaryRedirect)
}))
defer registry.Close()
err := Download(context.Background(), DownloadOptions{
Blobs: []Blob{blob},
BaseURL: registry.URL,
DestDir: t.TempDir(),
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
if internalHit.Load() {
t.Error("internal host received a request despite redirect rejection")
}
}
func TestDownloadRedirectToPrivateHostAllowedWithOptIn(t *testing.T) {
cdnDir := t.TempDir()
blob, data := createTestBlob(t, cdnDir, 1024)
cdn := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
path := filepath.Join(cdnDir, digestToPath(filepath.Base(r.URL.Path)))
data, err := os.ReadFile(path)
if err != nil {
http.NotFound(w, r)
return
}
w.Write(data)
}))
defer cdn.Close()
registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, cdn.URL+r.URL.Path, http.StatusTemporaryRedirect)
}))
defer registry.Close()
clientDir := t.TempDir()
err := Download(context.Background(), DownloadOptions{
Blobs: []Blob{blob},
BaseURL: registry.URL,
DestDir: clientDir,
AllowPrivateHosts: true,
})
if err != nil {
t.Fatalf("download with AllowPrivateHosts failed: %v", err)
}
verifyBlob(t, clientDir, blob, data)
}
func TestDownloadRedirectMetadataEndpoint(t *testing.T) {
blob, _ := createTestBlob(t, t.TempDir(), 1024)
registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "http://169.254.169.254/latest/meta-data/", http.StatusFound)
}))
defer registry.Close()
err := Download(context.Background(), DownloadOptions{
Blobs: []Blob{blob},
BaseURL: registry.URL,
DestDir: t.TempDir(),
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
}
func TestUploadRedirectToPrivateHostDenied(t *testing.T) {
clientDir := t.TempDir()
blob, _ := createTestBlob(t, clientDir, 1024)
var internalHit atomic.Bool
internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
internalHit.Store(true)
}))
defer internal.Close()
// Minimal registry: HEAD 404, POST hands an absolute session URL on the
// internal host — simulating a hostile Docker-Upload-Location.
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodHead:
http.NotFound(w, r)
case http.MethodPost:
w.Header().Set("Location", internal.URL+"/v2/library/_/blobs/uploads/1")
w.WriteHeader(http.StatusAccepted)
default:
http.NotFound(w, r)
}
}))
defer server.Close()
err := Upload(context.Background(), UploadOptions{
Blobs: []Blob{blob},
BaseURL: server.URL,
SrcDir: clientDir,
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
if internalHit.Load() {
t.Error("internal host received a request despite rejection")
}
}
func TestUploadPatchRedirectToPrivateHostDenied(t *testing.T) {
clientDir := t.TempDir()
blob, _ := createTestBlob(t, clientDir, 1024)
var internalHit atomic.Bool
internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
internalHit.Store(true)
}))
defer internal.Close()
var serverURL string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodHead:
http.NotFound(w, r)
case http.MethodPost:
w.Header().Set("Location", serverURL+"/v2/library/_/blobs/uploads/1")
w.WriteHeader(http.StatusAccepted)
case http.MethodPatch:
// 307 the part body at the internal host
w.Header().Set("Docker-Upload-Location", r.URL.Path)
http.Redirect(w, r, internal.URL+r.URL.Path, http.StatusTemporaryRedirect)
default:
http.NotFound(w, r)
}
}))
defer server.Close()
serverURL = server.URL
err := Upload(context.Background(), UploadOptions{
Blobs: []Blob{blob},
BaseURL: server.URL,
SrcDir: clientDir,
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
if internalHit.Load() {
t.Error("internal host received a request despite rejection")
}
}
func TestCheckedDialerRejectsRebind(t *testing.T) {
// A hostname resolving to a private address must be refused at dial
// time even when the URL-level validation step is bypassed or raced
// (DNS rebinding). localhost deterministically resolves to loopback.
dial := checkedDialer(&net.Dialer{Timeout: 2 * time.Second}, false)
conn, err := dial(context.Background(), "tcp", "localhost:1")
if !strings.Contains(fmt.Sprint(err), "not allowed") {
t.Errorf("expected dns rejection, got conn=%v err=%v", conn, err)
}
// With the escape hatch, the same dial is permitted (then fails
// normally on connection refused for port 1).
dial = checkedDialer(&net.Dialer{Timeout: 2 * time.Second}, true)
conn, err = dial(context.Background(), "tcp", "localhost:1")
if err != nil && strings.Contains(fmt.Sprint(err), "not allowed") {
t.Errorf("allowPrivate dial was still rejected: %v", err)
}
if conn != nil {
conn.Close()
}
// IP literals are checked without DNS.
dial = checkedDialer(&net.Dialer{Timeout: 2 * time.Second}, false)
if _, err := dial(context.Background(), "tcp", "169.254.169.254:80"); !strings.Contains(fmt.Sprint(err), "not allowed") {
t.Errorf("metadata IP dial not rejected: %v", err)
}
}
func TestCheckedClientExemptsBaseHost(t *testing.T) {
// The registry base host is caller-directed, so it must be exempt from
// the public-IP dial check even when the base URL has no explicit port —
// otherwise a private registry without AllowPrivateHosts breaks on
// default ports (regression: dial addr carries ":443", baseHost didn't).
for _, addr := range []string{"127.0.0.1:443", "localhost:443"} {
tr := checkedClient("https://127.0.0.1", false).Transport.(*http.Transport)
if addr == "localhost:443" {
tr = checkedClient("https://localhost", false).Transport.(*http.Transport)
}
_, err := tr.DialContext(t.Context(), "tcp", addr)
if strings.Contains(fmt.Sprint(err), "not allowed") {
t.Errorf("base host %s was validated at dial time: %v", addr, err)
}
// A real dial is expected to fail with connection refused here (or
// succeed if something listens) — only the policy rejection is a bug.
}
}
func TestValidateRedirectTarget(t *testing.T) {
ctx := context.Background()
okPublic, _ := url.Parse("https://203.0.113.10/blob")
httpsBase := "https://registry.example.com"
httpBase := "http://192.168.1.10:5000"
cases := []struct {
name string
raw string
base string
allowPrivate bool
wantErr bool
}{
{"public https allowed", "https://203.0.113.10/blob", httpsBase, false, false},
{"metadata denied", "http://169.254.169.254/latest/meta-data", httpsBase, false, true},
{"loopback denied", "http://127.0.0.1:11434/api/tags", httpsBase, false, true},
{"rfc1918 denied", "http://10.0.0.5/internal", httpsBase, false, true},
{"cgnat denied", "http://100.64.1.1/internal", httpsBase, false, true},
{"alibaba metadata denied", "http://100.100.2.148/latest/meta-data", httpsBase, false, true},
{"cgnat 4-in-6 denied", "http://[::ffff:100.64.1.1]/internal", httpsBase, false, true},
{"benchmarking net denied", "http://198.18.0.1/bench", httpsBase, false, true},
{"ipv6 link-local denied", "http://[fe80::1]/x", httpsBase, false, true},
{"localhost hostname denied", "http://localhost/x", httpsBase, false, true},
{"bad scheme denied", "file:///etc/passwd", httpsBase, false, true},
{"https registry downgrade to http denied", "http://203.0.113.10/blob", httpsBase, false, true},
{"unresolvable name fails closed", "https://no-such-host.invalid/blob", httpsBase, false, true},
{"http registry may redirect to public http", "http://203.0.113.10/blob", httpBase, false, false},
{"private allowed with opt-in", "http://192.168.1.20/cdn/blob", httpsBase, true, false},
{"metadata allowed with opt-in", "http://169.254.169.254/latest", httpsBase, true, false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
u, _ := url.Parse(tc.raw)
err := validateRedirectTarget(ctx, u, tc.base, tc.allowPrivate)
if tc.wantErr && err == nil {
t.Error("expected error, got nil")
}
if !tc.wantErr && err != nil {
t.Errorf("unexpected error: %v", err)
}
})
}
// The public-IP literal case must pass too (no DNS involved).
if err := validateRedirectTarget(ctx, okPublic, httpsBase, false); err != nil {
t.Errorf("public https target rejected: %v", err)
}
}
// Same-host HTTPS→HTTP redirects must be rejected even though the host is
// unchanged — otherwise a registry can strip TLS and rebinding DNS steers
// the cleartext request at an internal address.
func TestDownloadSameHostDowngradeRedirectDenied(t *testing.T) {
blob, _ := createTestBlob(t, t.TempDir(), 1024)
var serverURL string
registry := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Redirect to the same host:port, but cleartext.
u, _ := url.Parse(serverURL)
u.Scheme = "http"
http.Redirect(w, r, u.String()+r.URL.Path, http.StatusTemporaryRedirect)
}))
defer registry.Close()
serverURL = registry.URL
// The checked dialer wouldn't trust the test cert, so use the server's
// own TLS config; but mirror checkedClient's no-auto-follow policy so
// resolve() sees the 307 and the redirect policy is under test.
client := registry.Client()
client.CheckRedirect = func(*http.Request, []*http.Request) error {
return http.ErrUseLastResponse
}
err := Download(context.Background(), DownloadOptions{
Blobs: []Blob{blob},
BaseURL: registry.URL,
DestDir: t.TempDir(),
Client: client,
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
}
func TestUploadSameHostDowngradeSessionURLDenied(t *testing.T) {
clientDir := t.TempDir()
blob, _ := createTestBlob(t, clientDir, 1024)
var serverURL string
registry := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case http.MethodHead:
http.NotFound(w, r)
case http.MethodPost:
// Hand back a same-host session URL, but cleartext.
u, _ := url.Parse(serverURL)
u.Scheme = "http"
w.Header().Set("Location", u.String()+"/v2/library/_/blobs/uploads/1")
w.WriteHeader(http.StatusAccepted)
default:
http.NotFound(w, r)
}
}))
defer registry.Close()
serverURL = registry.URL
err := Upload(context.Background(), UploadOptions{
Blobs: []Blob{blob},
BaseURL: registry.URL,
SrcDir: clientDir,
Client: registry.Client(),
})
if !errors.Is(err, errRedirectNotAllowed) {
t.Fatalf("expected errRedirectNotAllowed, got %v", err)
}
}
func TestValidateRedirectScheme(t *testing.T) {
cases := []struct {
name string
raw string
base string
wantErr bool
}{
{"https to same-host https ok", "https://registry.example.com/v2/x", "https://registry.example.com", false},
{"https to same-host http denied", "http://registry.example.com/v2/x", "https://registry.example.com", true},
{"http registry to same-host http ok", "http://192.168.1.10:5000/v2/x", "http://192.168.1.10:5000", false},
{"bad scheme denied", "file:///etc/passwd", "https://registry.example.com", true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
u, _ := url.Parse(tc.raw)
err := validateRedirectScheme(u, tc.base)
if tc.wantErr && err == nil {
t.Error("expected error, got nil")
}
if !tc.wantErr && err != nil {
t.Errorf("unexpected error: %v", err)
}
})
}
}
+26 -24
View File
@@ -58,34 +58,36 @@ type Blob struct {
// DownloadOptions configures a parallel download operation.
type DownloadOptions struct {
Blobs []Blob // Blobs to download
BaseURL string // Registry base URL
DestDir string // Destination directory for blobs
Repository string // Repository path for blob URLs (e.g., "library/model")
Concurrency int // Max parallel downloads (default DefaultDownloadConcurrency)
BodyConcurrency int // Max simultaneous body-bearing transfers; 0 or negative serializes (capacity 1)
Progress func(completed, total int64) // Progress callback (optional)
Client *http.Client // HTTP client (optional, uses default)
Token string // Auth token (optional)
GetToken func(ctx context.Context, challenge AuthChallenge) (string, error) // Token refresh callback
Logger *slog.Logger // Optional structured logger
UserAgent string // User-Agent header (optional, has default)
StallTimeout time.Duration // Timeout for stall detection (default 10s)
Blobs []Blob // Blobs to download
BaseURL string // Registry base URL
DestDir string // Destination directory for blobs
Repository string // Repository path for blob URLs (e.g., "library/model")
Concurrency int // Max parallel downloads (default DefaultDownloadConcurrency)
BodyConcurrency int // Max simultaneous body-bearing transfers; 0 or negative serializes (capacity 1)
Progress func(completed, total int64) // Progress callback (optional)
Client *http.Client // HTTP client (optional, uses default)
Token string // Auth token (optional)
GetToken func(ctx context.Context, challenge AuthChallenge) (string, error) // Token refresh callback
Logger *slog.Logger // Optional structured logger
UserAgent string // User-Agent header (optional, has default)
StallTimeout time.Duration // Timeout for stall detection (default 10s)
AllowPrivateHosts bool // Permits cross-host redirects to local addresses
}
// UploadOptions configures a parallel upload operation.
type UploadOptions struct {
Blobs []Blob // Blobs to upload
BaseURL string // Registry base URL
SrcDir string // Source directory containing blobs
Concurrency int // Max parallel uploads (default DefaultUploadConcurrency)
BodyConcurrency int // Max simultaneous body-bearing transfers; 0 or negative serializes (capacity 1)
Progress func(completed, total int64) // Progress callback (optional)
Client *http.Client // HTTP client (optional, uses default)
Token string // Auth token (optional)
GetToken func(ctx context.Context, challenge AuthChallenge) (string, error) // Token refresh callback
Logger *slog.Logger // Optional structured logger
UserAgent string // User-Agent header (optional, has default)
Blobs []Blob // Blobs to upload
BaseURL string // Registry base URL
SrcDir string // Source directory containing blobs
Concurrency int // Max parallel uploads (default DefaultUploadConcurrency)
BodyConcurrency int // Max simultaneous body-bearing transfers; 0 or negative serializes (capacity 1)
Progress func(completed, total int64) // Progress callback (optional)
Client *http.Client // HTTP client (optional, uses default)
Token string // Auth token (optional)
GetToken func(ctx context.Context, challenge AuthChallenge) (string, error) // Token refresh callback
Logger *slog.Logger // Optional structured logger
UserAgent string // User-Agent header (optional, has default)
AllowPrivateHosts bool // Permits cross-host redirects to local addresses
// Manifest fields (optional) - if set, manifest is pushed after all blobs complete
Manifest []byte // Raw manifest JSON to push
+8
View File
@@ -178,6 +178,8 @@ func TestDownloadWithRedirect(t *testing.T) {
Blobs: []Blob{blob},
BaseURL: registry.URL,
DestDir: clientDir,
// httptest servers are loopback; opt in as a trusted local registry.
AllowPrivateHosts: true,
})
if err != nil {
t.Fatalf("Download with redirect failed: %v", err)
@@ -545,6 +547,8 @@ func TestUploadWithRedirect(t *testing.T) {
Blobs: []Blob{blob},
BaseURL: server.URL,
SrcDir: clientDir,
// httptest servers are loopback; opt in as a trusted local registry.
AllowPrivateHosts: true,
})
if err != nil {
t.Fatalf("Upload with redirect failed: %v", err)
@@ -2376,6 +2380,8 @@ func TestChunkedUploadCDNRedirect(t *testing.T) {
Blobs: []Blob{blob},
BaseURL: server.URL,
SrcDir: clientDir,
// CDN redirect target is a loopback httptest server; opt in.
AllowPrivateHosts: true,
})
if err != nil {
t.Fatalf("Upload with CDN redirect failed: %v", err)
@@ -2788,6 +2794,8 @@ func TestV2DirectUpload(t *testing.T) {
Blobs: []Blob{blob},
BaseURL: server.URL,
SrcDir: clientDir,
// Direct-upload URL is a loopback httptest server; opt in.
AllowPrivateHosts: true,
})
if err != nil {
t.Fatalf("Upload failed: %v", err)
+37 -21
View File
@@ -27,16 +27,17 @@ import (
)
type uploader struct {
client *http.Client
baseURL string
srcDir string
repository string // Repository path for blob URLs (e.g., "library/model")
tokenMu sync.RWMutex
token string
getToken func(context.Context, AuthChallenge) (string, error)
userAgent string
progress *progressTracker
logger *slog.Logger
client *http.Client
baseURL string
srcDir string
repository string // Repository path for blob URLs (e.g., "library/model")
tokenMu sync.RWMutex
token string
getToken func(context.Context, AuthChallenge) (string, error)
userAgent string
progress *progressTracker
logger *slog.Logger
allowPrivate bool
// bodySem caps the number of simultaneous body-bearing transfers so a
// modest home uplink isn't saturated. Always set by upload(); nil only
// when tests build uploader directly (in which case holdBody is a no-op).
@@ -92,14 +93,15 @@ func upload(ctx context.Context, opts UploadOptions) error {
}
u := &uploader{
client: cmp.Or(opts.Client, defaultClient),
baseURL: opts.BaseURL,
srcDir: opts.SrcDir,
repository: cmp.Or(opts.Repository, "library/_"),
token: opts.Token,
getToken: opts.GetToken,
userAgent: cmp.Or(opts.UserAgent, defaultUserAgent),
logger: opts.Logger,
client: cmp.Or(opts.Client, checkedClient(opts.BaseURL, opts.AllowPrivateHosts)),
baseURL: opts.BaseURL,
srcDir: opts.SrcDir,
repository: cmp.Or(opts.Repository, "library/_"),
token: opts.Token,
getToken: opts.GetToken,
userAgent: cmp.Or(opts.UserAgent, defaultUserAgent),
logger: opts.Logger,
allowPrivate: opts.AllowPrivateHosts,
}
// 0 or negative serializes; never unbounded.
u.bodySem = semaphore.NewWeighted(int64(max(1, opts.BodyConcurrency)))
@@ -224,7 +226,7 @@ func (u *uploader) upload(ctx context.Context, blob Blob) error {
u.progress.add(-n)
lastErr = err
}
return fmt.Errorf("%w: %v", errMaxRetriesExceeded, lastErr)
return fmt.Errorf("%w: %w", errMaxRetriesExceeded, lastErr)
}
func (u *uploader) uploadOnce(ctx context.Context, blob Blob) (int64, error) {
@@ -370,10 +372,18 @@ func (u *uploader) initUpload(ctx context.Context, blob Blob) (uploadEndpoint, e
}
sessionURL, _ := url.Parse(loc)
base, _ := url.Parse(u.baseURL)
if !sessionURL.IsAbs() {
base, _ := url.Parse(u.baseURL)
sessionURL = base.ResolveReference(sessionURL)
}
if err := validateRedirectScheme(sessionURL, u.baseURL); err != nil {
return uploadEndpoint{}, err
}
if base != nil && sessionURL.Host != base.Host {
if err := validateRedirectTarget(ctx, sessionURL, u.baseURL, u.allowPrivate); err != nil {
return uploadEndpoint{}, err
}
}
ep := uploadEndpoint{sessionURL: sessionURL.String()}
@@ -387,6 +397,9 @@ func (u *uploader) initUpload(ctx context.Context, blob Blob) (uploadEndpoint, e
// (percent-encoding case, query ordering) which can change the
// canonical form a signed URL was computed over.
if d, err := url.Parse(directURL); err == nil && d.IsAbs() {
if err := validateRedirectTarget(ctx, d, u.baseURL, u.allowPrivate); err != nil {
return uploadEndpoint{}, err
}
ep.directUploadURL = directURL
ep.signedHeaders = make(http.Header)
const signedPrefix = "X-Signed-Header-"
@@ -519,7 +532,7 @@ func (u *uploader) bodylessRegistryPUT(ctx context.Context, url string, op strin
lastErr = fmt.Errorf("%s: status %d: %s", op, resp.StatusCode, body)
}
}
return fmt.Errorf("%w: %v", errMaxRetriesExceeded, lastErr)
return fmt.Errorf("%w: %w", errMaxRetriesExceeded, lastErr)
}
// putChunked is the fallback used when the server doesn't return a
@@ -657,6 +670,9 @@ func (u *uploader) uploadOnePart(ctx context.Context, sessionURL *url.URL, part
if redirectURL == nil {
return nil, nil, pr.bytes(), fmt.Errorf("patch part %d: 307 without Location", part.n)
}
if err := validateRedirectTarget(ctx, redirectURL, u.baseURL, u.allowPrivate); err != nil {
return nil, nil, pr.bytes(), err
}
// The PATCH attempt's progress is wasted — we re-upload to CDN.
// We can't safely Reset partHash here: the http transport's
// writeLoop may still be feeding TeeReader bytes into it, so