diff --git a/cmd/skopeo/inspect.go b/cmd/skopeo/inspect.go index 63528fe554..f18a8ef2da 100644 --- a/cmd/skopeo/inspect.go +++ b/cmd/skopeo/inspect.go @@ -18,6 +18,7 @@ import ( "github.com/containers/image/v5/transports" "github.com/containers/image/v5/types" "github.com/containers/skopeo/cmd/skopeo/inspect" + "github.com/docker/distribution/registry/api/errcode" v1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" "github.com/spf13/cobra" @@ -203,12 +204,26 @@ func (opts *inspectOptions) run(args []string, stdout io.Writer) (retErr error) } outputData.RepoTags, err = docker.GetRepositoryTags(ctx, sys, img.Reference()) if err != nil { - // some registries may decide to block the "list all tags" endpoint - // gracefully allow the inspect to continue in this case. Currently - // the IBM Bluemix container registry has this restriction. - // In addition, AWS ECR rejects it with 403 (Forbidden) if the "ecr:ListImages" - // action is not allowed. - if !strings.Contains(err.Error(), "401") && !strings.Contains(err.Error(), "403") { + // Some registries may decide to block the "list all tags" endpoint; + // gracefully allow the inspect to continue in this case: + fatalFailure := true + // - AWS ECR rejects it if the "ecr:ListImages" action is not allowed. + // https://github.com/containers/skopeo/issues/726 + var ec errcode.ErrorCoder + if ok := errors.As(err, &ec); ok && ec.ErrorCode() == errcode.ErrorCodeDenied { + fatalFailure = false + } + // - public.ecr.aws does not implement the endpoint at all, and fails with 404: + // https://github.com/containers/skopeo/issues/1230 + // This is actually "code":"NOT_FOUND", and the parser doesn’t preserve that. + // So, also check the error text. + if ok := errors.As(err, &ec); ok && ec.ErrorCode() == errcode.ErrorCodeUnknown { + var e errcode.Error + if ok := errors.As(err, &e); ok && e.Code == errcode.ErrorCodeUnknown && e.Message == "404 page not found" { + fatalFailure = false + } + } + if fatalFailure { return fmt.Errorf("Error determining repository tags: %w", err) } logrus.Warnf("Registry disallows tag list retrieval; skipping") diff --git a/cmd/skopeo/sync.go b/cmd/skopeo/sync.go index 6884d8b18c..bf66e33362 100644 --- a/cmd/skopeo/sync.go +++ b/cmd/skopeo/sync.go @@ -216,15 +216,7 @@ func getImageTags(ctx context.Context, sysCtx *types.SystemContext, repoRef refe } tags, err := docker.GetRepositoryTags(ctx, sysCtx, dockerRef) if err != nil { - var unauthorizedForCredentials docker.ErrUnauthorizedForCredentials - if errors.As(err, &unauthorizedForCredentials) { - // Some registries may decide to block the "list all tags" endpoint. - // Gracefully allow the sync to continue in this case. - logrus.Warnf("Registry disallows tag list retrieval: %s", err) - tags = nil - } else { - return nil, fmt.Errorf("Error determining repository tags for image %s: %w", name, err) - } + return nil, fmt.Errorf("Error determining repository tags for repo %s: %w", name, err) } return tags, nil diff --git a/go.mod b/go.mod index bf12e2e784..4619d7fe5e 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,10 @@ go 1.17 require ( github.com/containers/common v0.50.1 - github.com/containers/image/v5 v5.23.0 + github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58 github.com/containers/ocicrypt v1.1.6 github.com/containers/storage v1.43.0 + github.com/docker/distribution v2.8.1+incompatible github.com/opencontainers/go-digest v1.0.0 github.com/opencontainers/image-spec v1.1.0-rc2 github.com/opencontainers/image-tools v1.0.0-rc3 @@ -31,7 +32,6 @@ require ( github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a // indirect github.com/cyphar/filepath-securejoin v0.2.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/docker/distribution v2.8.1+incompatible // indirect github.com/docker/docker v20.10.18+incompatible // indirect github.com/docker/docker-credential-helpers v0.7.0 // indirect github.com/docker/go-connections v0.4.0 // indirect diff --git a/go.sum b/go.sum index 0f55437135..12ac1749f8 100644 --- a/go.sum +++ b/go.sum @@ -386,8 +386,9 @@ github.com/containernetworking/plugins v1.0.1/go.mod h1:QHCfGpaTwYTbbH+nZXKVTxNB github.com/containernetworking/plugins v1.1.1/go.mod h1:Sr5TH/eBsGLXK/h71HeLfX19sZPp3ry5uHSkI4LPxV8= github.com/containers/common v0.50.1 h1:AYRAf1xyahNVRez49KIkREInNf36SQx1lyLY9M95wQI= github.com/containers/common v0.50.1/go.mod h1:XnWlXPyE9Ky+8v8MfYWJZFnejkprAkUeo0DTWmSiwcY= -github.com/containers/image/v5 v5.23.0 h1:Uv/n8zsHVUBBJK2rfBUHbN4CutHHmsQeyi4f80lAOf8= github.com/containers/image/v5 v5.23.0/go.mod h1:EXFFGEsL99S6aqLqK2mQJ3yrNh6Q05UCHt4mhF9JNoM= +github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58 h1:VgX3CTXXkoSQFIr70Wsg59jioTwz5JUcV6q6BScWhh8= +github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58/go.mod h1:2JJxA5K1NFpA3FtrK+Csmdlj++5oveB7CsXhekEJsIU= github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a h1:spAGlqziZjCJL25C6F1zsQY05tfCKE9F5YwtEWWe6hU= github.com/containers/libtrust v0.0.0-20200511145503-9c3a6c22cd9a/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY= github.com/containers/ocicrypt v1.0.1/go.mod h1:MeJDzk1RJHv89LjsH0Sp5KTY3ZYkjXO/C+bKAeWFIrc= diff --git a/integration/check_test.go b/integration/check_test.go index 1ff2564925..56f245e473 100644 --- a/integration/check_test.go +++ b/integration/check_test.go @@ -49,25 +49,25 @@ func (s *SkopeoSuite) TearDownSuite(c *check.C) { //func skopeoCmd() func (s *SkopeoSuite) TestVersion(c *check.C) { - wanted := fmt.Sprintf(".*%s version %s.*", skopeoBinary, version.Version) - assertSkopeoSucceeds(c, wanted, "--version") + assertSkopeoSucceeds(c, fmt.Sprintf(".*%s version %s.*", skopeoBinary, version.Version), + "--version") } func (s *SkopeoSuite) TestCanAuthToPrivateRegistryV2WithoutDockerCfg(c *check.C) { - wanted := ".*manifest unknown: manifest unknown.*" - assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", "--creds="+s.regV2WithAuth.username+":"+s.regV2WithAuth.password, fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url)) + assertSkopeoFails(c, ".*manifest unknown.*", + "--tls-verify=false", "inspect", "--creds="+s.regV2WithAuth.username+":"+s.regV2WithAuth.password, fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url)) } func (s *SkopeoSuite) TestNeedAuthToPrivateRegistryV2WithoutDockerCfg(c *check.C) { - wanted := ".*unauthorized: authentication required.*" - assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url)) + assertSkopeoFails(c, ".*authentication required.*", + "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url)) } func (s *SkopeoSuite) TestCertDirInsteadOfCertPath(c *check.C) { - wanted := ".*unknown flag: --cert-path.*" - assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-path=/") - wanted = ".*unauthorized: authentication required.*" - assertSkopeoFails(c, wanted, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-dir=/etc/docker/certs.d/") + assertSkopeoFails(c, ".*unknown flag: --cert-path.*", + "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-path=/") + assertSkopeoFails(c, ".*authentication required.*", + "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2WithAuth.url), "--cert-dir=/etc/docker/certs.d/") } // TODO(runcom): as soon as we can push to registries ensure you can inspect here @@ -75,10 +75,8 @@ func (s *SkopeoSuite) TestCertDirInsteadOfCertPath(c *check.C) { func (s *SkopeoSuite) TestNoNeedAuthToPrivateRegistryV2ImageNotFound(c *check.C) { out, err := exec.Command(skopeoBinary, "--tls-verify=false", "inspect", fmt.Sprintf("docker://%s/busybox:latest", s.regV2.url)).CombinedOutput() c.Assert(err, check.NotNil, check.Commentf(string(out))) - wanted := ".*manifest unknown.*" - c.Assert(string(out), check.Matches, "(?s)"+wanted) // (?s) : '.' will also match newlines - wanted = ".*unauthorized: authentication required.*" - c.Assert(string(out), check.Not(check.Matches), "(?s)"+wanted) // (?s) : '.' will also match newlines + c.Assert(string(out), check.Matches, "(?s).*manifest unknown.*") // (?s) : '.' will also match newlines + c.Assert(string(out), check.Not(check.Matches), "(?s).*unauthorized: authentication required.*") // (?s) : '.' will also match newlines } func (s *SkopeoSuite) TestInspectFailsWhenReferenceIsInvalid(c *check.C) { @@ -86,28 +84,28 @@ func (s *SkopeoSuite) TestInspectFailsWhenReferenceIsInvalid(c *check.C) { } func (s *SkopeoSuite) TestLoginLogout(c *check.C) { - wanted := "^Login Succeeded!\n$" - assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url) + assertSkopeoSucceeds(c, "^Login Succeeded!\n$", + "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url) // test --get-login returns username - wanted = fmt.Sprintf("^%s\n$", s.regV2WithAuth.username) - assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--get-login", s.regV2WithAuth.url) + assertSkopeoSucceeds(c, fmt.Sprintf("^%s\n$", s.regV2WithAuth.username), + "login", "--tls-verify=false", "--get-login", s.regV2WithAuth.url) // test logout - wanted = fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url) - assertSkopeoSucceeds(c, wanted, "logout", s.regV2WithAuth.url) + assertSkopeoSucceeds(c, fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url), + "logout", s.regV2WithAuth.url) } func (s *SkopeoSuite) TestCopyWithLocalAuth(c *check.C) { - wanted := "^Login Succeeded!\n$" - assertSkopeoSucceeds(c, wanted, "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url) + assertSkopeoSucceeds(c, "^Login Succeeded!\n$", + "login", "--tls-verify=false", "--username="+s.regV2WithAuth.username, "--password="+s.regV2WithAuth.password, s.regV2WithAuth.url) // copy to private registry using local authentication imageName := fmt.Sprintf("docker://%s/busybox:mine", s.regV2WithAuth.url) assertSkopeoSucceeds(c, "", "copy", "--dest-tls-verify=false", testFQIN+":latest", imageName) // inspect from private registry assertSkopeoSucceeds(c, "", "inspect", "--tls-verify=false", imageName) // logout from the registry - wanted = fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url) - assertSkopeoSucceeds(c, wanted, "logout", s.regV2WithAuth.url) + assertSkopeoSucceeds(c, fmt.Sprintf("^Removed login credentials for %s\n$", s.regV2WithAuth.url), + "logout", s.regV2WithAuth.url) // inspect from private registry should fail after logout - wanted = ".*unauthorized: authentication required.*" - assertSkopeoFails(c, wanted, "inspect", "--tls-verify=false", imageName) + assertSkopeoFails(c, ".*authentication required.*", + "inspect", "--tls-verify=false", imageName) } diff --git a/integration/copy_test.go b/integration/copy_test.go index b9587caeff..372478ca31 100644 --- a/integration/copy_test.go +++ b/integration/copy_test.go @@ -1036,7 +1036,8 @@ func (s *CopySuite) TestCopyVerifyingMirroredSignatures(c *check.C) { assertSkopeoSucceeds(c, "", "--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"remap:remapped", dirDest) // To be extra clear about the semantics, verify that the signedPrefix (primary) location never exists // and only the remapped prefix (mirror) is accessed. - assertSkopeoFails(c, ".*initializing source docker://localhost:5006/myns/mirroring-primary:remapped:.*manifest unknown: manifest unknown.*", "--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"primary:remapped", dirDest) + assertSkopeoFails(c, ".*initializing source docker://localhost:5006/myns/mirroring-primary:remapped:.*manifest unknown.*", + "--policy", policy, "--registries.d", registriesDir, "--registries-conf", "fixtures/registries.conf", "copy", "--src-tls-verify=false", regPrefix+"primary:remapped", dirDest) } func (s *SkopeoSuite) TestCopySrcWithAuth(c *check.C) { diff --git a/integration/sync_test.go b/integration/sync_test.go index 39d7f48ea0..16569bafe7 100644 --- a/integration/sync_test.go +++ b/integration/sync_test.go @@ -534,7 +534,7 @@ func (s *SyncSuite) TestFailsNoSourceImages(c *check.C) { assertSkopeoFails(c, ".*No images to sync found in .*", "sync", "--scoped", "--dest-tls-verify=false", "--src", "dir", "--dest", "docker", tmpDir, v2DockerRegistryURL) - assertSkopeoFails(c, ".*No images to sync found in .*", + assertSkopeoFails(c, ".*Error determining repository tags for repo docker.io/library/hopefully_no_images_will_ever_be_called_like_this: fetching tags list: requested access to the resource is denied.*", "sync", "--scoped", "--dest-tls-verify=false", "--src", "docker", "--dest", "docker", "hopefully_no_images_will_ever_be_called_like_this", v2DockerRegistryURL) } @@ -544,11 +544,11 @@ func (s *SyncSuite) TestFailsWithDockerSourceNoRegistry(c *check.C) { tmpDir := c.MkDir() //untagged - assertSkopeoFails(c, ".*invalid status code from registry 404.*", + assertSkopeoFails(c, ".*StatusCode: 404.*", "sync", "--scoped", "--src", "docker", "--dest", "dir", regURL, tmpDir) //tagged - assertSkopeoFails(c, ".*invalid status code from registry 404.*", + assertSkopeoFails(c, ".*StatusCode: 404.*", "sync", "--scoped", "--src", "docker", "--dest", "dir", regURL+":thetag", tmpDir) } @@ -557,11 +557,11 @@ func (s *SyncSuite) TestFailsWithDockerSourceUnauthorized(c *check.C) { tmpDir := c.MkDir() //untagged - assertSkopeoFails(c, ".*Registry disallows tag list retrieval.*", + assertSkopeoFails(c, ".*requested access to the resource is denied.*", "sync", "--scoped", "--src", "docker", "--dest", "dir", repo, tmpDir) //tagged - assertSkopeoFails(c, ".*unauthorized: authentication required.*", + assertSkopeoFails(c, ".*requested access to the resource is denied.*", "sync", "--scoped", "--src", "docker", "--dest", "dir", repo+":thetag", tmpDir) } @@ -570,7 +570,7 @@ func (s *SyncSuite) TestFailsWithDockerSourceNotExisting(c *check.C) { tmpDir := c.MkDir() //untagged - assertSkopeoFails(c, ".*invalid status code from registry 404.*", + assertSkopeoFails(c, ".*repository name not known to registry.*", "sync", "--scoped", "--src-tls-verify=false", "--src", "docker", "--dest", "dir", repo, tmpDir) //tagged diff --git a/systemtest/040-local-registry-auth.bats b/systemtest/040-local-registry-auth.bats index a8e7d27f82..670697def2 100644 --- a/systemtest/040-local-registry-auth.bats +++ b/systemtest/040-local-registry-auth.bats @@ -24,22 +24,22 @@ function setup() { @test "auth: credentials on command line" { # No creds run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/nonesuch - expect_output --substring "unauthorized: authentication required" + expect_output --substring "authentication required" # Wrong user run_skopeo 1 inspect --tls-verify=false --creds=baduser:badpassword \ docker://localhost:5000/nonesuch - expect_output --substring "unauthorized: authentication required" + expect_output --substring "authentication required" # Wrong password run_skopeo 1 inspect --tls-verify=false --creds=$testuser:badpassword \ docker://localhost:5000/nonesuch - expect_output --substring "unauthorized: authentication required" + expect_output --substring "authentication required" # Correct creds, but no such image run_skopeo 1 inspect --tls-verify=false --creds=$testuser:$testpassword \ docker://localhost:5000/nonesuch - expect_output --substring "manifest unknown: manifest unknown" + expect_output --substring "manifest unknown" # These should pass run_skopeo copy --dest-tls-verify=false --dcreds=$testuser:$testpassword \ @@ -64,7 +64,7 @@ function setup() { podman logout localhost:5000 run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/busybox:mine - expect_output --substring "unauthorized: authentication required" + expect_output --substring "authentication required" } @test "auth: copy with --src-creds and --dest-creds" { @@ -94,7 +94,7 @@ function setup() { # inspect without authfile: should fail run_skopeo 1 inspect --tls-verify=false docker://localhost:5000/busybox:mine - expect_output --substring "unauthorized: authentication required" + expect_output --substring "authentication required" # inspect with authfile: should work run_skopeo inspect --tls-verify=false --authfile $TESTDIR/test.auth docker://localhost:5000/busybox:mine diff --git a/vendor/github.com/containers/image/v5/docker/docker_client.go b/vendor/github.com/containers/image/v5/docker/docker_client.go index c7ef38b9da..307fed67fe 100644 --- a/vendor/github.com/containers/image/v5/docker/docker_client.go +++ b/vendor/github.com/containers/image/v5/docker/docker_client.go @@ -313,8 +313,14 @@ func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password return err } defer resp.Body.Close() - - return httpResponseToError(resp, "") + if resp.StatusCode != http.StatusOK { + err := registryHTTPResponseToError(resp) + if resp.StatusCode == http.StatusUnauthorized { + err = ErrUnauthorizedForCredentials{Err: err} + } + return err + } + return nil } // SearchResult holds the information of each matching image @@ -411,7 +417,7 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { - err := httpResponseToError(resp, "") + err := registryHTTPResponseToError(resp) logrus.Errorf("error getting search results from v2 endpoint %q: %v", registry, err) return nil, fmt.Errorf("couldn't search registry %q: %w", registry, err) } @@ -816,7 +822,7 @@ func (c *dockerClient) detectPropertiesHelper(ctx context.Context) error { defer resp.Body.Close() logrus.Debugf("Ping %s status %d", url.Redacted(), resp.StatusCode) if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusUnauthorized { - return httpResponseToError(resp, "") + return registryHTTPResponseToError(resp) } c.challenges = parseAuthHeader(resp.Header) c.scheme = scheme @@ -956,9 +962,10 @@ func (c *dockerClient) getBlob(ctx context.Context, ref dockerReference, info ty if err != nil { return nil, 0, err } - if err := httpResponseToError(res, "Error fetching blob"); err != nil { + if res.StatusCode != http.StatusOK { + err := registryHTTPResponseToError(res) res.Body.Close() - return nil, 0, err + return nil, 0, fmt.Errorf("fetching blob: %w", err) } cache.RecordKnownLocation(ref.Transport(), bicTransportScope(ref), info.Digest, newBICLocationReference(ref)) return res.Body, getBlobSize(res), nil @@ -982,13 +989,8 @@ func (c *dockerClient) getOCIDescriptorContents(ctx context.Context, ref dockerR // isManifestUnknownError returns true iff err from fetchManifest is a “manifest unknown” error. func isManifestUnknownError(err error) bool { - var errs errcode.Errors - if !errors.As(err, &errs) || len(errs) == 0 { - return false - } - err = errs[0] - ec, ok := err.(errcode.ErrorCoder) - if !ok { + var ec errcode.ErrorCoder + if !errors.As(err, &ec) { return false } return ec.ErrorCode() == v2.ErrorCodeManifestUnknown @@ -1037,9 +1039,8 @@ func (c *dockerClient) getExtensionsSignatures(ctx context.Context, ref dockerRe return nil, err } defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("downloading signatures for %s in %s: %w", manifestDigest, ref.ref.Name(), handleErrorResponse(res)) + return nil, fmt.Errorf("downloading signatures for %s in %s: %w", manifestDigest, ref.ref.Name(), registryHTTPResponseToError(res)) } body, err := iolimits.ReadAtMost(res.Body, iolimits.MaxSignatureListBodySize) diff --git a/vendor/github.com/containers/image/v5/docker/docker_image.go b/vendor/github.com/containers/image/v5/docker/docker_image.go index 3e8dbbee13..6a4331e335 100644 --- a/vendor/github.com/containers/image/v5/docker/docker_image.go +++ b/vendor/github.com/containers/image/v5/docker/docker_image.go @@ -77,8 +77,8 @@ func GetRepositoryTags(ctx context.Context, sys *types.SystemContext, ref types. return nil, err } defer res.Body.Close() - if err := httpResponseToError(res, "fetching tags list"); err != nil { - return nil, err + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("fetching tags list: %w", registryHTTPResponseToError(res)) } var tagsHolder struct { diff --git a/vendor/github.com/containers/image/v5/docker/docker_image_dest.go b/vendor/github.com/containers/image/v5/docker/docker_image_dest.go index 44b45c472c..80a8efbb02 100644 --- a/vendor/github.com/containers/image/v5/docker/docker_image_dest.go +++ b/vendor/github.com/containers/image/v5/docker/docker_image_dest.go @@ -244,7 +244,7 @@ func (d *dockerImageDestination) blobExists(ctx context.Context, repo reference. logrus.Debugf("... not present") return false, -1, nil default: - return false, -1, fmt.Errorf("failed to read from destination repository %s: %d (%s)", reference.Path(d.ref.ref), res.StatusCode, http.StatusText(res.StatusCode)) + return false, -1, fmt.Errorf("checking whether a blob %s exists in %s: %w", digest, repo.Name(), registryHTTPResponseToError(res)) } } @@ -487,15 +487,10 @@ func successStatus(status int) bool { return status >= 200 && status <= 399 } -// isManifestInvalidError returns true iff err from client.HandleErrorResponse is a “manifest invalid” error. +// isManifestInvalidError returns true iff err from registryHTTPResponseToError is a “manifest invalid” error. func isManifestInvalidError(err error) bool { - errors, ok := err.(errcode.Errors) - if !ok || len(errors) == 0 { - return false - } - err = errors[0] - ec, ok := err.(errcode.ErrorCoder) - if !ok { + var ec errcode.ErrorCoder + if ok := errors.As(err, &ec); !ok { return false } diff --git a/vendor/github.com/containers/image/v5/docker/docker_image_src.go b/vendor/github.com/containers/image/v5/docker/docker_image_src.go index b0e8779710..d726ecd057 100644 --- a/vendor/github.com/containers/image/v5/docker/docker_image_src.go +++ b/vendor/github.com/containers/image/v5/docker/docker_image_src.go @@ -28,6 +28,10 @@ import ( "github.com/sirupsen/logrus" ) +// maxLookasideSignatures is an arbitrary limit for the total number of signatures we would try to read from a lookaside server, +// even if it were broken or malicious and it continued serving an enormous number of items. +const maxLookasideSignatures = 128 + type dockerImageSource struct { impl.Compat impl.PropertyMethodsInitialize @@ -372,12 +376,9 @@ func (s *dockerImageSource) GetBlobAt(ctx context.Context, info types.BlobInfo, res.Body.Close() return nil, nil, private.BadPartialRequestError{Status: res.Status} default: - err := httpResponseToError(res, "Error fetching partial blob") - if err == nil { - err = fmt.Errorf("invalid status code returned when fetching blob %d (%s)", res.StatusCode, http.StatusText(res.StatusCode)) - } + err := registryHTTPResponseToError(res) res.Body.Close() - return nil, nil, err + return nil, nil, fmt.Errorf("fetching partial blob: %w", err) } } @@ -451,6 +452,10 @@ func (s *dockerImageSource) getSignaturesFromLookaside(ctx context.Context, inst // NOTE: Keep this in sync with docs/signature-protocols.md! signatures := []signature.Signature{} for i := 0; ; i++ { + if i >= maxLookasideSignatures { + return nil, fmt.Errorf("server provided %d signatures, assuming that's unreasonable and a server error", maxLookasideSignatures) + } + url := lookasideStorageURL(s.c.signatureBase, manifestDigest, i) signature, missing, err := s.getOneSignature(ctx, url) if err != nil { @@ -496,10 +501,19 @@ func (s *dockerImageSource) getOneSignature(ctx context.Context, url *url.URL) ( } defer res.Body.Close() if res.StatusCode == http.StatusNotFound { + logrus.Debugf("... got status 404, as expected = end of signatures") return nil, true, nil } else if res.StatusCode != http.StatusOK { return nil, false, fmt.Errorf("reading signature from %s: status %d (%s)", url.Redacted(), res.StatusCode, http.StatusText(res.StatusCode)) } + + contentType := res.Header.Get("Content-Type") + if mimeType := simplifyContentType(contentType); mimeType == "text/html" { + logrus.Warnf("Signature %q has Content-Type %q, unexpected for a signature", url.Redacted(), contentType) + // Don’t immediately fail; the lookaside spec does not place any requirements on Content-Type. + // If the content really is HTML, it’s going to fail in signature.FromBlob. + } + sigBlob, err := iolimits.ReadAtMost(res.Body, iolimits.MaxSignatureBodySize) if err != nil { return nil, false, err @@ -605,16 +619,16 @@ func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerRefere return err } defer get.Body.Close() - manifestBody, err := iolimits.ReadAtMost(get.Body, iolimits.MaxManifestBodySize) - if err != nil { - return err - } switch get.StatusCode { case http.StatusOK: case http.StatusNotFound: return fmt.Errorf("Unable to delete %v. Image may not exist or is not stored with a v2 Schema in a v2 registry", ref.ref) default: - return fmt.Errorf("Failed to delete %v: %s (%v)", ref.ref, manifestBody, get.Status) + return fmt.Errorf("deleting %v: %w", ref.ref, registryHTTPResponseToError(get)) + } + manifestBody, err := iolimits.ReadAtMost(get.Body, iolimits.MaxManifestBodySize) + if err != nil { + return err } manifestDigest, err := manifest.Digest(manifestBody) @@ -630,13 +644,8 @@ func deleteImage(ctx context.Context, sys *types.SystemContext, ref dockerRefere return err } defer delete.Body.Close() - - body, err := iolimits.ReadAtMost(delete.Body, iolimits.MaxErrorBodySize) - if err != nil { - return err - } if delete.StatusCode != http.StatusAccepted { - return fmt.Errorf("Failed to delete %v: %s (%v)", deletePath, string(body), delete.Status) + return fmt.Errorf("deleting %v: %w", ref.ref, registryHTTPResponseToError(delete)) } for i := 0; ; i++ { diff --git a/vendor/github.com/containers/image/v5/docker/errors.go b/vendor/github.com/containers/image/v5/docker/errors.go index 9c02e27e31..74fe17648c 100644 --- a/vendor/github.com/containers/image/v5/docker/errors.go +++ b/vendor/github.com/containers/image/v5/docker/errors.go @@ -4,6 +4,9 @@ import ( "errors" "fmt" "net/http" + + "github.com/docker/distribution/registry/api/errcode" + "github.com/sirupsen/logrus" ) var ( @@ -33,7 +36,7 @@ func httpResponseToError(res *http.Response, context string) error { case http.StatusTooManyRequests: return ErrTooManyRequests case http.StatusUnauthorized: - err := handleErrorResponse(res) + err := registryHTTPResponseToError(res) return ErrUnauthorizedForCredentials{Err: err} default: if context != "" { @@ -47,12 +50,47 @@ func httpResponseToError(res *http.Response, context string) error { // registry func registryHTTPResponseToError(res *http.Response) error { err := handleErrorResponse(res) - if e, ok := err.(*unexpectedHTTPResponseError); ok { + // len(errs) == 0 should never be returned by handleErrorResponse; if it does, we don't modify it and let the caller report it as is. + if errs, ok := err.(errcode.Errors); ok && len(errs) > 0 { + // The docker/distribution registry implementation almost never returns + // more than one error in the HTTP body; it seems there is only one + // possible instance, where the second error reports a cleanup failure + // we don't really care about. + // + // The only _common_ case where a multi-element error is returned is + // created by the handleErrorResponse parser when OAuth authorization fails: + // the first element contains errors from a WWW-Authenticate header, the second + // element contains errors from the response body. + // + // In that case the first one is currently _slightly_ more informative (ErrorCodeUnauthorized + // for invalid tokens, ErrorCodeDenied for permission denied with a valid token + // for the first error, vs. ErrorCodeUnauthorized for both cases for the second error.) + // + // Also, docker/docker similarly only logs the other errors and returns the + // first one. + if len(errs) > 1 { + logrus.Debugf("Discarding non-primary errors:") + for _, err := range errs[1:] { + logrus.Debugf(" %s", err.Error()) + } + } + err = errs[0] + } + switch e := err.(type) { + case *unexpectedHTTPResponseError: response := string(e.Response) if len(response) > 50 { response = response[:50] + "..." } - err = fmt.Errorf("StatusCode: %d, %s", e.StatusCode, response) + // %.0w makes e visible to error.Unwrap() without including any text + err = fmt.Errorf("StatusCode: %d, %s%.0w", e.StatusCode, response, e) + case errcode.Error: + // e.Error() is fmt.Sprintf("%s: %s", e.Code.Error(), e.Message, which is usually + // rather redundant. So reword it without using e.Code.Error() if e.Message is the default. + if e.Message == e.Code.Message() { + // %.0w makes e visible to error.Unwrap() without including any text + err = fmt.Errorf("%s%.0w", e.Message, e) + } } return err } diff --git a/vendor/github.com/containers/image/v5/oci/archive/oci_src.go b/vendor/github.com/containers/image/v5/oci/archive/oci_src.go index e5ad2570ef..6c9ee33402 100644 --- a/vendor/github.com/containers/image/v5/oci/archive/oci_src.go +++ b/vendor/github.com/containers/image/v5/oci/archive/oci_src.go @@ -17,6 +17,17 @@ import ( "github.com/sirupsen/logrus" ) +// ImageNotFoundError is used when the OCI structure, in principle, exists and seems valid enough, +// but nothing matches the “image” part of the provided reference. +type ImageNotFoundError struct { + ref ociArchiveReference + // We may make members public, or add methods, in the future. +} + +func (e ImageNotFoundError) Error() string { + return fmt.Sprintf("no descriptor found for reference %q", e.ref.image) +} + type ociArchiveImageSource struct { impl.Compat @@ -35,6 +46,10 @@ func newImageSource(ctx context.Context, sys *types.SystemContext, ref ociArchiv unpackedSrc, err := tempDirRef.ociRefExtracted.NewImageSource(ctx, sys) if err != nil { + var notFound ocilayout.ImageNotFoundError + if errors.As(err, ¬Found) { + err = ImageNotFoundError{ref: ref} + } if err := tempDirRef.deleteTempDir(); err != nil { return nil, fmt.Errorf("deleting temp directory %q: %w", tempDirRef.tempDirectory, err) } diff --git a/vendor/github.com/containers/image/v5/oci/layout/oci_src.go b/vendor/github.com/containers/image/v5/oci/layout/oci_src.go index b2d963b019..392bd52747 100644 --- a/vendor/github.com/containers/image/v5/oci/layout/oci_src.go +++ b/vendor/github.com/containers/image/v5/oci/layout/oci_src.go @@ -21,6 +21,17 @@ import ( imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" ) +// ImageNotFoundError is used when the OCI structure, in principle, exists and seems valid enough, +// but nothing matches the “image” part of the provided reference. +type ImageNotFoundError struct { + ref ociReference + // We may make members public, or add methods, in the future. +} + +func (e ImageNotFoundError) Error() string { + return fmt.Sprintf("no descriptor found for reference %q", e.ref.image) +} + type ociImageSource struct { impl.Compat impl.PropertyMethodsInitialize diff --git a/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go b/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go index be22bed6d5..c2ead041f1 100644 --- a/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go +++ b/vendor/github.com/containers/image/v5/oci/layout/oci_transport.go @@ -205,7 +205,7 @@ func (ref ociReference) getManifestDescriptor() (imgspecv1.Descriptor, error) { } } if d == nil { - return imgspecv1.Descriptor{}, fmt.Errorf("no descriptor found for reference %q", ref.image) + return imgspecv1.Descriptor{}, ImageNotFoundError{ref} } return *d, nil } diff --git a/vendor/github.com/containers/image/v5/version/version.go b/vendor/github.com/containers/image/v5/version/version.go index 9996737273..21fbae27c1 100644 --- a/vendor/github.com/containers/image/v5/version/version.go +++ b/vendor/github.com/containers/image/v5/version/version.go @@ -8,10 +8,10 @@ const ( // VersionMinor is for functionality in a backwards-compatible manner VersionMinor = 23 // VersionPatch is for backwards-compatible bug fixes - VersionPatch = 0 + VersionPatch = 1 // VersionDev indicates development branch. Releases will be empty string. - VersionDev = "" + VersionDev = "-dev" ) // Version is the specification version that the package types support. diff --git a/vendor/modules.txt b/vendor/modules.txt index 41b6eb04a9..853f0650c9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -55,7 +55,7 @@ github.com/containers/common/pkg/flag github.com/containers/common/pkg/report github.com/containers/common/pkg/report/camelcase github.com/containers/common/pkg/retry -# github.com/containers/image/v5 v5.23.0 +# github.com/containers/image/v5 v5.23.1-0.20221012204947-6ea53742be58 ## explicit; go 1.17 github.com/containers/image/v5/copy github.com/containers/image/v5/directory