github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Check scheme and host of request on push redirect

Browse Source 
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>
This commit is contained in:
Derek McGowan 2023-10-24 14:45:07 -07:00
parent fa4ae46b15
commit 466ee870d5
No known key found for this signature in database
GPG Key ID: F58C5D0A4405ACDB
2 changed files with 57 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
remotes/docker/pusher.go
View File

@ -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()

52
remotes/docker/pusher_test.go
View File

@ -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()