From 05119a91751a861bfcfd1089a1c114afdc64a5e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 7 Sep 2022 00:44:56 +0200 Subject: [PATCH 1/2] Update c/image after https://github.com/containers/image/pull/1299 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit > go get github.com/containers/image/v5@main > make vendor Signed-off-by: Miloslav Trmač --- go.mod | 2 +- go.sum | 3 +- .../image/v5/docker/docker_client.go | 31 ++++++------- .../image/v5/docker/docker_image.go | 4 +- .../image/v5/docker/docker_image_dest.go | 13 ++---- .../image/v5/docker/docker_image_src.go | 41 ++++++++++------- .../containers/image/v5/docker/errors.go | 44 +++++++++++++++++-- .../image/v5/oci/archive/oci_src.go | 15 +++++++ .../containers/image/v5/oci/layout/oci_src.go | 11 +++++ .../image/v5/oci/layout/oci_transport.go | 2 +- .../containers/image/v5/version/version.go | 4 +- vendor/modules.txt | 2 +- 12 files changed, 121 insertions(+), 51 deletions(-) diff --git a/go.mod b/go.mod index e56db083b1..b2bb87f963 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/containers/buildah v1.28.0 github.com/containers/common v0.50.1 github.com/containers/conmon v2.0.20+incompatible - 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/psgo v1.7.3 github.com/containers/storage v1.43.0 diff --git a/go.sum b/go.sum index 5cc2d3794e..75dede9174 100644 --- a/go.sum +++ b/go.sum @@ -413,8 +413,9 @@ github.com/containers/common v0.50.1 h1:AYRAf1xyahNVRez49KIkREInNf36SQx1lyLY9M95 github.com/containers/common v0.50.1/go.mod h1:XnWlXPyE9Ky+8v8MfYWJZFnejkprAkUeo0DTWmSiwcY= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= -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/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 c80e918569..36a85329df 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -174,7 +174,7 @@ github.com/containers/common/version # github.com/containers/conmon v2.0.20+incompatible ## explicit github.com/containers/conmon/runner/config -# 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 From 33858c1cf85d635a6b84346719b1353a091c353c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Wed, 7 Sep 2022 18:28:15 +0200 Subject: [PATCH 2/2] Update tests for changed error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Miloslav Trmač --- test/system/150-login.bats | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/system/150-login.bats b/test/system/150-login.bats index b85007f0b3..27689452ff 100644 --- a/test/system/150-login.bats +++ b/test/system/150-login.bats @@ -180,7 +180,7 @@ EOF run_podman 125 push --authfile=$authfile \ --tls-verify=false $IMAGE \ localhost:${PODMAN_LOGIN_REGISTRY_PORT}/badpush:1 - is "$output" ".*: unauthorized: authentication required" \ + is "$output" ".* checking whether a blob .* exists in localhost:${PODMAN_LOGIN_REGISTRY_PORT}/badpush: authentication required" \ "auth error on push" } @@ -253,7 +253,7 @@ function _test_skopeo_credential_sharing() { run skopeo inspect "$@" --tls-verify=false docker://$registry/$destname echo "$output" is "$status" "1" "skopeo inspect - exit status" - is "$output" ".*: unauthorized: authentication required" \ + is "$output" ".*: authentication required" \ "auth error on skopeo inspect" }