Skip to content

Commit 68abc54

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 35c7634 commit 68abc54

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
@@ -249,13 +249,16 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
249249
}
250250

251251
if lurl.Host != lhost.Host || lhost.Scheme != lurl.Scheme {
252-
253252
lhost.Scheme = lurl.Scheme
254253
lhost.Host = lurl.Host
255-
log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination")
256254

257-
// Strip authorizer if change to host or scheme
258-
lhost.Authorizer = nil
255+
// Check if different than what was requested, accounting for fallback in the transport layer
256+
requested := resp.Request.URL
257+
if requested.Host != lhost.Host || requested.Scheme != lhost.Scheme {
258+
// Strip authorizer if change to host or scheme
259+
lhost.Authorizer = nil
260+
log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination, authorizer removed")
261+
}
259262
}
260263
}
261264
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)