Skip to content

Commit 466ee87

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]>
1 parent fa4ae46 commit 466ee87

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

247247
if lurl.Host != lhost.Host || lhost.Scheme != lurl.Scheme {
248-
249248
lhost.Scheme = lurl.Scheme
250249
lhost.Host = lurl.Host
251-
log.G(ctx).WithField("host", lhost.Host).WithField("scheme", lhost.Scheme).Debug("upload changed destination")
252250

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