Skip to content

Commit 5dd6430

Browse files
dmcgowandmcgowan
committed
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 <[email protected]> (cherry picked from commit 466ee87) Signed-off-by: Derek McGowan <[email protected]>
1 parent 51df21d commit 5dd6430

2 files changed

Lines changed: 57 additions & 6 deletions

File tree

‎remotes/docker/pusher.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,13 +246,16 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
246246
}
247247

248248
if lurl.Host != lhost.Host || lhost.Scheme != lurl.Scheme {
249-
250249
lhost.Scheme = lurl.Scheme
251250
lhost.Host = lurl.Host
252-
log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination")
253251

254-
// Strip authorizer if change to host or scheme
255-
lhost.Authorizer = nil
252+
// Check if different than what was requested, accounting for fallback in the transport layer
253+
requested := resp.Request.URL
254+
if requested.Host != lhost.Host || requested.Scheme != lhost.Scheme {
255+
// Strip authorizer if change to host or scheme
256+
lhost.Authorizer = nil
257+
log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination, authorizer removed")
258+
}
256259
}
257260
}
258261
q := lurl.Query()

‎remotes/docker/pusher_test.go‎

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/containerd/containerd/errdefs"
3434
"github.com/containerd/containerd/reference"
3535
"github.com/containerd/containerd/remotes"
36+
"github.com/containerd/log/logtest"
3637
"github.com/opencontainers/go-digest"
3738
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
3839
"github.com/stretchr/testify/assert"
@@ -86,6 +87,42 @@ func TestPusherErrClosedRetry(t *testing.T) {
8687
}
8788
}
8889

90+
func TestPusherHTTPFallback(t *testing.T) {
91+
ctx := logtest.WithT(context.Background(), t)
92+
93+
p, reg, _, done := samplePusher(t)
94+
defer done()
95+
96+
reg.uploadable = true
97+
reg.username = "testuser"
98+
reg.secret = "testsecret"
99+
reg.locationPrefix = p.hosts[0].Scheme + "://" + p.hosts[0].Host
100+
101+
p.hosts[0].Scheme = "https"
102+
client := p.hosts[0].Client
103+
if client == nil {
104+
clientC := *http.DefaultClient
105+
client = &clientC
106+
}
107+
if client.Transport == nil {
108+
client.Transport = http.DefaultTransport
109+
}
110+
client.Transport = HTTPFallback{client.Transport}
111+
p.hosts[0].Client = client
112+
phost := p.hosts[0].Host
113+
p.hosts[0].Authorizer = NewDockerAuthorizer(WithAuthCreds(func(host string) (string, string, error) {
114+
if host == phost {
115+
return "testuser", "testsecret", nil
116+
}
117+
return "", "", nil
118+
}))
119+
120+
layerContent := []byte("test")
121+
if err := tryUpload(ctx, t, p, layerContent); err != nil {
122+
t.Errorf("upload failed: %v", err)
123+
}
124+
}
125+
89126
// TestPusherErrReset tests the push method if the request needs to be retried
90127
// i.e when ErrReset occurs
91128
func TestPusherErrReset(t *testing.T) {
@@ -189,9 +226,20 @@ type uploadableMockRegistry struct {
189226
availableContents []string
190227
uploadable bool
191228
putHandlerFunc func(w http.ResponseWriter, r *http.Request) bool
229+
locationPrefix string
230+
username string
231+
secret string
192232
}
193233

194234
func (u *uploadableMockRegistry) ServeHTTP(w http.ResponseWriter, r *http.Request) {
235+
if u.secret != "" {
236+
user, pass, ok := r.BasicAuth()
237+
if !ok || user != u.username || pass != u.secret {
238+
w.Header().Add("WWW-Authenticate", "basic realm=test")
239+
w.WriteHeader(http.StatusUnauthorized)
240+
return
241+
}
242+
}
195243
if r.Method == http.MethodPut && u.putHandlerFunc != nil {
196244
// if true return the response witout calling default handler
197245
if u.putHandlerFunc(w, r) {
@@ -205,9 +253,9 @@ func (u *uploadableMockRegistry) defaultHandler(w http.ResponseWriter, r *http.R
205253
if r.Method == http.MethodPost {
206254
if matches := blobUploadRegexp.FindStringSubmatch(r.URL.Path); len(matches) != 0 {
207255
if u.uploadable {
208-
w.Header().Set("Location", "/upload")
256+
w.Header().Set("Location", u.locationPrefix+"/upload")
209257
} else {
210-
w.Header().Set("Location", "/cannotupload")
258+
w.Header().Set("Location", u.locationPrefix+"/cannotupload")
211259
}
212260

213261
dgstr := digest.Canonical.Digester()

0 commit comments

Comments
 (0)