Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions core/remotes/docker/pusher.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ func (p dockerPusher) push(ctx context.Context, desc ocispec.Descriptor, ref str
}
} else if resp.StatusCode != http.StatusNotFound {
err := unexpectedResponseErr(resp)
// A HEAD 403 carries no body, so issue a follow-up GET to the
// same URL to surface the registry's error details for diagnostics.
if resp.StatusCode == http.StatusForbidden && req.method == http.MethodHead {
err = withGETErrorBody(ctx, err, resp, func() (*http.Response, error) {
getReq := p.request(host, http.MethodGet, existCheck...)
getReq.header.Set("Accept", strings.Join([]string{desc.MediaType, `*/*`}, ", "))
if addErr := getReq.addNamespace(p.refspec.Hostname()); addErr != nil {
return nil, addErr
}
return getReq.doWithRetries(ctx, false)
})
}
log.G(ctx).WithError(err).Debug("unexpected response")
resp.Body.Close()
return nil, err
Expand Down Expand Up @@ -574,6 +586,41 @@ func (pw *pushWriter) Truncate(size int64) error {
return errors.New("cannot truncate remote upload")
}

// withGETErrorBody enriches originalErr, produced from a bodyless HEAD
// response, with the error body from a follow-up GET to the same URL. HEAD
// responses carry no body, so a 403 only surfaces its status code; a GET
// returns the registry's error details (e.g. "key vault access denied", IP
// restrictions) that explain the failure.
//
// The GET body is only used when the GET also returns 403, so the enriched
// error's status and body stay consistent. In that case the original HEAD
// request's method and status are preserved and only the body is borrowed from
// the GET; any other outcome (GET failed, or returned a different status)
// leaves originalErr untouched.
func withGETErrorBody(ctx context.Context, originalErr error, headResp *http.Response, doGET func() (*http.Response, error)) error {
getResp, err := doGET()
if err != nil {
log.G(ctx).WithError(err).Debug("failed to retrieve error body with GET fallback")
return originalErr
}
defer getResp.Body.Close()

if getResp.StatusCode != http.StatusForbidden {
log.G(ctx).WithFields(log.Fields{
"head_status": headResp.Status,
"get_status": getResp.Status,
}).Debug("ignoring GET fallback response with different status")
return originalErr
}

// Preserve the original HEAD request's method and status and borrow only
// the body from the GET, so the error still reflects the request the caller
// actually made.
enriched := *headResp
enriched.Body = getResp.Body
return unexpectedResponseErr(&enriched)
}

func requestWithMountFrom(req *request, mount, from string) *request {
creq := *req

Expand Down
75 changes: 75 additions & 0 deletions core/remotes/docker/pusher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,3 +739,78 @@ func Test_dockerPusher_push(t *testing.T) {
})
}
}

func TestPusherForbiddenGETFallbackForErrorBody(t *testing.T) {
// When the existence-check HEAD returns 403, the pusher should issue
// a follow-up GET to retrieve the registry's error body for diagnostics.
const errorMessage = "encryption key is disabled"

p, reg, _, done := samplePusher(t)
defer done()

reg.defaultHandlerFunc = func(w http.ResponseWriter, r *http.Request) bool {
if strings.Contains(r.URL.Path, "/manifests/") || strings.Contains(r.URL.Path, "/blobs/sha256:") {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusForbidden)
if r.Method == http.MethodGet {
w.Write([]byte(fmt.Sprintf(`{"errors":[{"code":"DENIED","message":"%s"}]}`, errorMessage)))
}
// HEAD: no body
return true
}
return false
}

ctx := context.Background()
desc := ocispec.Descriptor{
MediaType: ocispec.MediaTypeImageManifest,
Digest: digest.FromString("test-manifest"),
Size: 100,
}

_, err := p.push(ctx, desc, "test-ref", true)
require.Error(t, err)
assert.Contains(t, err.Error(), errorMessage,
"expected registry error body from GET fallback")
assert.Contains(t, err.Error(), "403")
}

func TestPusherForbiddenGETFallbackProxy(t *testing.T) {
// When the pusher is configured as a proxy (Host != image hostname),
// the GET fallback must include the ?ns= query parameter so the proxy
// can route the request to the correct upstream registry.
const errorMessage = "encryption key is disabled"

var gotNsParam string

p, reg, _, done := samplePusher(t)
defer done()

reg.defaultHandlerFunc = func(w http.ResponseWriter, r *http.Request) bool {
if strings.Contains(r.URL.Path, "/manifests/") || strings.Contains(r.URL.Path, "/blobs/sha256:") {
if r.Method == http.MethodGet {
gotNsParam = r.URL.Query().Get("ns")
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusForbidden)
if r.Method == http.MethodGet {
w.Write([]byte(fmt.Sprintf(`{"errors":[{"code":"DENIED","message":"%s"}]}`, errorMessage)))
}
return true
}
return false
}

ctx := context.Background()
desc := ocispec.Descriptor{
MediaType: ocispec.MediaTypeImageManifest,
Digest: digest.FromString("test-manifest"),
Size: 100,
}

_, err := p.push(ctx, desc, "test-ref", true)
require.Error(t, err)
assert.Contains(t, err.Error(), errorMessage)
assert.Equal(t, samplePusherHostname, gotNsParam,
"GET fallback on proxy must include ?ns= query parameter")
}
19 changes: 18 additions & 1 deletion core/remotes/docker/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,25 @@ func (r *dockerResolver) Resolve(ctx context.Context, ref string) (string, ocisp
continue
}
if resp.StatusCode > 399 {
err := unexpectedResponseErr(resp)
// A HEAD 403 carries no body, so issue a follow-up GET to
// the same URL to surface the registry's error details
// (e.g. "key vault access denied", IP restrictions) for
// diagnostics.
if resp.StatusCode == http.StatusForbidden && req.method == http.MethodHead {
err = withGETErrorBody(ctx, err, resp, func() (*http.Response, error) {
getReq := base.request(host, http.MethodGet, u...)
if addErr := getReq.addNamespace(base.refspec.Hostname()); addErr != nil {
return nil, addErr
}
for key, value := range r.resolveHeader {
getReq.header[key] = append(getReq.header[key], value...)
}
return getReq.doWithRetries(ctx, false)
})
}
if firstErrPriority < 3 {
firstErr = unexpectedResponseErr(resp)
firstErr = err
firstErrPriority = 3
}
log.G(ctx).Infof("%s after status: %s", nextHostOrFail(i), resp.Status)
Expand Down
199 changes: 199 additions & 0 deletions core/remotes/docker/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1602,3 +1602,202 @@ func TestResolverErrorStatusCodeOnFetch(t *testing.T) {
})
}
}

func TestResolveForbiddenGETFallbackForErrorBody(t *testing.T) {
// When HEAD returns 403 with no body, the resolver should issue a
// follow-up GET to retrieve the registry's error details.
const (
name = "test/repo"
tag = "latest"
errorBody = `{"errors":[{"code":"DENIED","message":"encryption key is disabled"}]}`
)

handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "/token") {
rw.Header().Set("Content-Type", "application/json")
rw.Write([]byte(`{"access_token":"test"}`))
return
}
if strings.Contains(r.URL.Path, "/manifests/") {
rw.Header().Set("Content-Type", "application/json")
rw.WriteHeader(http.StatusForbidden)
if r.Method == http.MethodGet {
rw.Write([]byte(errorBody))
}
return
}
if r.URL.Path == "/v2/" {
rw.WriteHeader(http.StatusOK)
return
}
rw.WriteHeader(http.StatusNotFound)
})

base, options, close := tlsServer(handler)
defer close()

options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client))
resolver := NewResolver(options)
ref := fmt.Sprintf("%s/%s:%s", base, name, tag)

_, _, err := resolver.Resolve(context.Background(), ref)
if err == nil {
t.Fatal("expected error from resolve, got nil")
}

errMsg := err.Error()
if !strings.Contains(errMsg, "encryption key is disabled") {
t.Errorf("expected error to contain registry error body, got: %s", errMsg)
}
if !strings.Contains(errMsg, "403") {
t.Errorf("expected error to contain status code 403, got: %s", errMsg)
}
}

func TestResolveForbiddenNoFallbackOnGET(t *testing.T) {
const (
name = "test/repo"
tag = "latest"
)

var getCount int
handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "/token") {
rw.Header().Set("Content-Type", "application/json")
rw.Write([]byte(`{"access_token":"test"}`))
return
}
if strings.Contains(r.URL.Path, "/manifests/") {
if r.Method == http.MethodGet {
getCount++
}
rw.Header().Set("Content-Type", "application/json")
rw.WriteHeader(http.StatusForbidden)
rw.Write([]byte(`{"errors":[{"code":"DENIED","message":"forbidden"}]}`))
return
}
if r.URL.Path == "/v2/" {
rw.WriteHeader(http.StatusOK)
return
}
rw.WriteHeader(http.StatusNotFound)
})

base, options, close := tlsServer(handler)
defer close()

options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client))
resolver := NewResolver(options)
ref := fmt.Sprintf("%s/%s:%s", base, name, tag)

_, _, err := resolver.Resolve(context.Background(), ref)
if err == nil {
t.Fatal("expected error from resolve, got nil")
}

// The resolver should issue exactly 1 GET (the fallback from HEAD 403).
// It must NOT issue a second fallback GET when the first GET also returns 403.
if getCount != 1 {
t.Errorf("expected exactly 1 GET fallback request for manifests, got %d", getCount)
}
}

func TestResolve404NoFallbackGET(t *testing.T) {
const (
name = "test/repo"
tag = "latest"
)

var getCount int
handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "/token") {
rw.Header().Set("Content-Type", "application/json")
rw.Write([]byte(`{"access_token":"test"}`))
return
}
if strings.Contains(r.URL.Path, "/manifests/") {
if r.Method == http.MethodGet {
getCount++
}
rw.WriteHeader(http.StatusNotFound)
return
}
if r.URL.Path == "/v2/" {
rw.WriteHeader(http.StatusOK)
return
}
rw.WriteHeader(http.StatusNotFound)
})

base, options, close := tlsServer(handler)
defer close()

options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client))
resolver := NewResolver(options)
ref := fmt.Sprintf("%s/%s:%s", base, name, tag)

_, _, err := resolver.Resolve(context.Background(), ref)
if err == nil {
t.Fatal("expected error from resolve, got nil")
}

if getCount != 0 {
t.Errorf("expected 0 GET requests for 404, got %d", getCount)
}
}

func TestResolveForbiddenGETFallbackNetworkError(t *testing.T) {
const (
name = "test/repo"
tag = "latest"
)

var requestCount int
handler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
if strings.Contains(r.URL.Path, "/token") {
rw.Header().Set("Content-Type", "application/json")
rw.Write([]byte(`{"access_token":"test"}`))
return
}
if strings.Contains(r.URL.Path, "/manifests/") {
requestCount++
if r.Method == http.MethodHead {
rw.WriteHeader(http.StatusForbidden)
return
}
hj, ok := rw.(http.Hijacker)
if !ok {
rw.WriteHeader(http.StatusForbidden)
return
}
conn, _, err := hj.Hijack()
if err != nil {
rw.WriteHeader(http.StatusInternalServerError)
return
}
conn.Close()
return
}
if r.URL.Path == "/v2/" {
rw.WriteHeader(http.StatusOK)
return
}
rw.WriteHeader(http.StatusNotFound)
})

base, options, close := tlsServer(handler)
defer close()

options.Hosts = ConfigureDefaultRegistries(WithClient(options.Client))
resolver := NewResolver(options)
ref := fmt.Sprintf("%s/%s:%s", base, name, tag)

_, _, err := resolver.Resolve(context.Background(), ref)
if err == nil {
t.Fatal("expected error from resolve, got nil")
}

if !strings.Contains(err.Error(), "403") {
t.Errorf("expected 403 in error, got: %s", err.Error())
}
}
Loading