fix: plain-http enforces use of http on bearer token fetch (#677) (#678)

Co-authored-by: Adam Martin <adam.martin@ranchergovernment.com>

cmd/hauler/cli/store/copy.go

@@ -181,7 +181,7 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef
 			Insecure:  o.Insecure,
 		}
 		// Shared across every per-artifact RegistryTarget below to keep connections pooled.
-		registryClient := content.NewRegistryHTTPClient(registryOpts)
+		registryClient := content.NewRegistryHTTPClient(components[1], registryOpts)
 
 		// Pre-build a map from base ref → image manifest digest so that sig/att/sbom
 		// descriptors (which store the base image ref, not the cosign tag) can be routed

pkg/content/registry.go

@@ -23,14 +23,50 @@ type RegistryTarget struct {
 	resolver remotes.Resolver
 }
 
+// plainHTTPRoundTripper rewrites outgoing https:// requests to http:// for a
+// single registry authority (host:port). This is required when PlainHTTP is
+// set: the Docker authorizer follows the Bearer realm URL from the
+// WWW-Authenticate header literally, and registries like Harbor always
+// advertise an https:// realm regardless of the incoming transport, so the
+// token fetch fails with "server gave HTTP response to HTTPS client" unless
+// the scheme is rewritten before the request leaves the client.
+//
+// The rewrite is scoped to the registry's exact authority on purpose: a
+// plain-http registry may legitimately 301-redirect blob fetches to a real
+// HTTPS object store or CDN on a different host (or even a different port on
+// the same host), and those must NOT be downgraded. host must already be a
+// bare authority (no path) -- NewRegistryHTTPClient strips any path before
+// building this struct, since req.URL.Host is never anything but the authority.
+type plainHTTPRoundTripper struct {
+	inner http.RoundTripper
+	host  string // registry authority (host:port), the only host we downgrade
+}
+
+func (r plainHTTPRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
+	if req.URL.Scheme == "https" && req.URL.Host == r.host {
+		reqCopy := req.Clone(req.Context())
+		reqCopy.URL.Scheme = "http"
+		req = reqCopy
+	}
+	return r.inner.RoundTrip(req)
+}
+
 // NewRegistryHTTPClient builds an *http.Client configured for opts, cloning
 // http.DefaultTransport rather than mutating it in place, which would leak
 // InsecureSkipVerify into every other HTTP client in the process.
 //
+// host is the registry this client talks to (e.g. "localhost:5000"). Callers
+// such as cmd/hauler/cli/store/copy.go derive it from a target reference's
+// remainder after "://", so it may arrive as "host:port/repo/path"; any path
+// is stripped to the bare authority before use. When opts.PlainHTTP is set,
+// that authority scopes a targeted https->http rewrite (see
+// plainHTTPRoundTripper) so a legitimate cross-host https redirect (e.g. to a
+// CDN or object store) is not downgraded.
+//
 // Build this once and share it across all RegistryTargets for a copy: a
 // transport per target defeats connection pooling and can exhaust file
 // descriptors on large copies.
-func NewRegistryHTTPClient(opts RegistryOptions) *http.Client {
+func NewRegistryHTTPClient(host string, opts RegistryOptions) *http.Client {
 	var transport *http.Transport
 	if dt, ok := http.DefaultTransport.(*http.Transport); ok {
 		transport = dt.Clone()
@@ -41,7 +77,14 @@ func NewRegistryHTTPClient(opts RegistryOptions) *http.Client {
 	if opts.Insecure {
 		transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
 	}
-	return &http.Client{Transport: transport}
+	var rt http.RoundTripper = transport
+	if opts.PlainHTTP {
+		// host may arrive as "registry:port/repo/path" (copy.go passes
+		// components[1]); req.URL.Host is only ever the authority, so match on that.
+		authority, _, _ := strings.Cut(host, "/")
+		rt = plainHTTPRoundTripper{inner: transport, host: authority}
+	}
+	return &http.Client{Transport: rt}
 }
 
 // NewRegistryTarget returns a RegistryTarget that pushes to host (e.g. "localhost:5000").

pkg/content/registry_test.go

@@ -13,6 +13,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync/atomic"
 	"testing"
 )
 
@@ -30,7 +31,7 @@ func TestNewRegistryTarget_InsecureSkipsTLSVerification(t *testing.T) {
 	host := strings.TrimPrefix(srv.URL, "https://")
 
 	opts := RegistryOptions{Insecure: true}
-	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(opts))
+	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(host, opts))
 
 	_, err := target.Resolve(context.Background(), host+"/library/test:latest")
 	if err == nil {
@@ -53,8 +54,17 @@ func TestNewRegistryTarget_InsecureSkipsTLSVerification(t *testing.T) {
 // HTTPS endpoint signed by a private CA. With --insecure --plain-http,
 // hauler should dial http, follow the redirect to https, and skip cert
 // verification on the redirected request.
+//
+// The two httptest servers both listen on 127.0.0.1, on different ports, so
+// this also proves that the PlainHTTP https->http rewrite is scoped
+// precisely enough to leave the redirect target's scheme alone: a blanket
+// rewrite of every outgoing https request (as opposed to one scoped to the
+// registry's own host:port) would downgrade this redirect to http and the
+// TLS-only redirect target would never be reached.
 func TestNewRegistryTarget_InsecurePlainHTTPFollowsHTTPSRedirect(t *testing.T) {
+	var tlsReached atomic.Bool
 	tlsSrv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		tlsReached.Store(true)
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer tlsSrv.Close()
@@ -67,7 +77,7 @@ func TestNewRegistryTarget_InsecurePlainHTTPFollowsHTTPSRedirect(t *testing.T) {
 	host := strings.TrimPrefix(httpSrv.URL, "http://")
 
 	opts := RegistryOptions{Insecure: true, PlainHTTP: true}
-	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(opts))
+	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(host, opts))
 
 	_, err := target.Resolve(context.Background(), host+"/library/test:latest")
 	if err == nil {
@@ -77,6 +87,90 @@ func TestNewRegistryTarget_InsecurePlainHTTPFollowsHTTPSRedirect(t *testing.T) {
 	if strings.Contains(lower, "certificate signed by unknown authority") || strings.Contains(lower, "certificate") {
 		t.Fatalf("expected no certificate verification error after following http->https redirect with Insecure: true, got: %v", err)
 	}
+	if !tlsReached.Load() {
+		t.Fatal("plain-http downgraded a cross-host https redirect: TLS endpoint was never reached")
+	}
+}
+
+// TestNewRegistryTarget_PlainHTTPRewritesBearerTokenFetchScheme reproduces
+// issue #677: a plain-http Bearer-auth registry (e.g. Harbor) that always
+// advertises an https:// realm in its WWW-Authenticate challenge, even
+// though the registry itself is only reachable over plain http. The token
+// realm is on the SAME host:port as the registry (Harbor's own token
+// service is co-located behind the same reverse proxy), which is what makes
+// the host-scoped rewrite in plainHTTPRoundTripper apply here. Before the
+// fix, the containerd Docker authorizer dialed the https realm literally
+// and failed with "server gave HTTP response to HTTPS client".
+func TestNewRegistryTarget_PlainHTTPRewritesBearerTokenFetchScheme(t *testing.T) {
+	var registrySrv *httptest.Server
+	registrySrv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/service/token" {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{"token":"fake-token"}`))
+			return
+		}
+		realm := "https://" + strings.TrimPrefix(registrySrv.URL, "http://") + "/service/token"
+		w.Header().Set("WWW-Authenticate", `Bearer realm="`+realm+`",service="registry"`)
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	defer registrySrv.Close()
+
+	host := strings.TrimPrefix(registrySrv.URL, "http://")
+
+	opts := RegistryOptions{PlainHTTP: true}
+	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(host, opts))
+
+	_, err := target.Resolve(context.Background(), host+"/library/test:latest")
+	if err == nil {
+		t.Fatalf("expected an error resolving against a 401-only fake registry, got nil")
+	}
+	lower := strings.ToLower(err.Error())
+	if strings.Contains(lower, "server gave http response to https client") {
+		t.Fatalf("plain-http Bearer token fetch dialed the https realm literally instead of being rewritten to http, got: %v", err)
+	}
+}
+
+// TestNewRegistryTarget_PlainHTTPRewritesBearerTokenFetchScheme_PathBearingHost
+// reproduces the real call path used by `hauler store copy registry://`:
+// cmd/hauler/cli/store/copy.go derives its host argument from
+// strings.SplitN(targetRef, "://", 2)[1], which for a target reference like
+// "oci://harbor:80/library" is "harbor:80/library" -- host:port WITH the
+// repo path still attached, not a clean authority. NewRegistryHTTPClient
+// must normalize that down to just the authority before comparing against
+// req.URL.Host (which is never anything but the authority), or the
+// plainHTTPRoundTripper rewrite silently never fires and #677 recurs in
+// production even though the "clean host" tests above pass.
+func TestNewRegistryTarget_PlainHTTPRewritesBearerTokenFetchScheme_PathBearingHost(t *testing.T) {
+	var registrySrv *httptest.Server
+	registrySrv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/service/token" {
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(`{"token":"fake-token"}`))
+			return
+		}
+		realm := "https://" + strings.TrimPrefix(registrySrv.URL, "http://") + "/service/token"
+		w.Header().Set("WWW-Authenticate", `Bearer realm="`+realm+`",service="registry"`)
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	defer registrySrv.Close()
+
+	hostPort := strings.TrimPrefix(registrySrv.URL, "http://")
+	// Mirrors copy.go's components[1]: host:port with a repo path attached.
+	componentsOne := hostPort + "/library"
+
+	opts := RegistryOptions{PlainHTTP: true}
+	target := NewRegistryTarget(componentsOne, opts, NewRegistryHTTPClient(componentsOne, opts))
+
+	_, err := target.Resolve(context.Background(), hostPort+"/library/test:latest")
+	if err == nil {
+		t.Fatalf("expected an error resolving against a 401-only fake registry, got nil")
+	}
+	lower := strings.ToLower(err.Error())
+	if strings.Contains(lower, "server gave http response to https client") {
+		t.Fatalf("plain-http Bearer token fetch dialed the https realm literally instead of being rewritten to http (path-bearing host arg like copy.go passes), got: %v", err)
+	}
 }
 
 // TestNewRegistryTarget_InsecureAppliesToBearerTokenFetch reproduces the case
@@ -106,7 +200,7 @@ func TestNewRegistryTarget_InsecureAppliesToBearerTokenFetch(t *testing.T) {
 	host := strings.TrimPrefix(registrySrv.URL, "https://")
 
 	opts := RegistryOptions{Insecure: true}
-	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(opts))
+	target := NewRegistryTarget(host, opts, NewRegistryHTTPClient(host, opts))
 
 	_, err := target.Resolve(context.Background(), host+"/library/test:latest")
 	if err == nil {
@@ -164,7 +258,7 @@ func TestNewRegistryTarget_SchemeSelection(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			target := NewRegistryTarget(host, tt.opts, NewRegistryHTTPClient(tt.opts))
+			target := NewRegistryTarget(host, tt.opts, NewRegistryHTTPClient(host, tt.opts))
 
 			_, err := target.Resolve(context.Background(), host+"/library/test:latest")
 
@@ -192,7 +286,7 @@ func TestNewRegistryHTTPClient_DoesNotLeakGlobalTLSConfig(t *testing.T) {
 	}
 	before := dt.TLSClientConfig
 
-	_ = NewRegistryHTTPClient(RegistryOptions{Insecure: true})
+	_ = NewRegistryHTTPClient("registry.example.com", RegistryOptions{Insecure: true})
 
 	if dt.TLSClientConfig != before {
 		t.Fatalf("NewRegistryHTTPClient mutated the global http.DefaultTransport.TLSClientConfig: before=%+v after=%+v", before, dt.TLSClientConfig)
@@ -226,7 +320,7 @@ func TestNewRegistryHTTPClient_FallsBackWhenDefaultTransportIsNotHTTPTransport(t
 				t.Fatalf("NewRegistryHTTPClient panicked with a non-*http.Transport DefaultTransport: %v", r)
 			}
 		}()
-		client = NewRegistryHTTPClient(RegistryOptions{Insecure: true})
+		client = NewRegistryHTTPClient("registry.example.com", RegistryOptions{Insecure: true})
 	}()
 
 	if client == nil {