From 466ee870d59d01119a60d9df83cedff96de07c6c Mon Sep 17 00:00:00 2001
From: Derek McGowan <derek@mcg.dev>
Date: Tue, 24 Oct 2023 14:45:07 -0700
Subject: [PATCH] Check scheme and host of request on push redirect

When the HTTP fallback is used, the scheme changes from HTTPS to HTTP
which can cause a mismatch on redirect, causing the authorizer to get
stripped out. Since the redirect host must match the redirect host in
this case, credentials are only sent to the same origin host that
returned the redirect.

This fixes an issue for a push getting a 401 unauthorized on the PUT
request even though credentials are available.

Signed-off-by: Derek McGowan <derek@mcg.dev>
---
 remotes/docker/pusher.go      | 11 +++++---
 remotes/docker/pusher_test.go | 52 +++++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/remotes/docker/pusher.go b/remotes/docker/pusher.go
index c2ae3a147..720e5e198 100644
--- a/remotes/docker/pusher.go
+++ b/remotes/docker/pusher.go
@@ -245,13 +245,16 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
 			}
 
 			if lurl.Host != lhost.Host || lhost.Scheme != lurl.Scheme {
-
 				lhost.Scheme = lurl.Scheme
 				lhost.Host = lurl.Host
-				log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination")
 
-				// Strip authorizer if change to host or scheme
-				lhost.Authorizer = nil
+				// Check if different than what was requested, accounting for fallback in the transport layer
+				requested := resp.Request.URL
+				if requested.Host != lhost.Host || requested.Scheme != lhost.Scheme {
+					// Strip authorizer if change to host or scheme
+					lhost.Authorizer = nil
+					log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination, authorizer removed")
+				}
 			}
 		}
 		q := lurl.Query()
diff --git a/remotes/docker/pusher_test.go b/remotes/docker/pusher_test.go
index a2ab9f4d3..e17410d86 100644
--- a/remotes/docker/pusher_test.go
+++ b/remotes/docker/pusher_test.go
@@ -33,6 +33,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/reference"
 	"github.com/containerd/containerd/remotes"
+	"github.com/containerd/log/logtest"
 	"github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
@@ -86,6 +87,42 @@ func TestPusherErrClosedRetry(t *testing.T) {
 	}
 }
 
+func TestPusherHTTPFallback(t *testing.T) {
+	ctx := logtest.WithT(context.Background(), t)
+
+	p, reg, _, done := samplePusher(t)
+	defer done()
+
+	reg.uploadable = true
+	reg.username = "testuser"
+	reg.secret = "testsecret"
+	reg.locationPrefix = p.hosts[0].Scheme + "://" + p.hosts[0].Host
+
+	p.hosts[0].Scheme = "https"
+	client := p.hosts[0].Client
+	if client == nil {
+		clientC := *http.DefaultClient
+		client = &clientC
+	}
+	if client.Transport == nil {
+		client.Transport = http.DefaultTransport
+	}
+	client.Transport = HTTPFallback{client.Transport}
+	p.hosts[0].Client = client
+	phost := p.hosts[0].Host
+	p.hosts[0].Authorizer = NewDockerAuthorizer(WithAuthCreds(func(host string) (string, string, error) {
+		if host == phost {
+			return "testuser", "testsecret", nil
+		}
+		return "", "", nil
+	}))
+
+	layerContent := []byte("test")
+	if err := tryUpload(ctx, t, p, layerContent); err != nil {
+		t.Errorf("upload failed: %v", err)
+	}
+}
+
 // TestPusherErrReset tests the push method if the request needs to be retried
 // i.e when ErrReset occurs
 func TestPusherErrReset(t *testing.T) {
@@ -189,9 +226,20 @@ type uploadableMockRegistry struct {
 	availableContents []string
 	uploadable        bool
 	putHandlerFunc    func(w http.ResponseWriter, r *http.Request) bool
+	locationPrefix    string
+	username          string
+	secret            string
 }
 
 func (u *uploadableMockRegistry) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if u.secret != "" {
+		user, pass, ok := r.BasicAuth()
+		if !ok || user != u.username || pass != u.secret {
+			w.Header().Add("WWW-Authenticate", "basic realm=test")
+			w.WriteHeader(http.StatusUnauthorized)
+			return
+		}
+	}
 	if r.Method == http.MethodPut && u.putHandlerFunc != nil {
 		// if true return the response witout calling default handler
 		if u.putHandlerFunc(w, r) {
@@ -205,9 +253,9 @@ func (u *uploadableMockRegistry) defaultHandler(w http.ResponseWriter, r *http.R
 	if r.Method == http.MethodPost {
 		if matches := blobUploadRegexp.FindStringSubmatch(r.URL.Path); len(matches) != 0 {
 			if u.uploadable {
-				w.Header().Set("Location", "/upload")
+				w.Header().Set("Location", u.locationPrefix+"/upload")
 			} else {
-				w.Header().Set("Location", "/cannotupload")
+				w.Header().Set("Location", u.locationPrefix+"/cannotupload")
 			}
 
 			dgstr := digest.Canonical.Digester()