Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manifest digest mismatch pushing to private docker registry #2963

Closed
laflechejonathan opened this issue Jul 14, 2022 · 3 comments
Closed

manifest digest mismatch pushing to private docker registry #2963

laflechejonathan opened this issue Jul 14, 2022 · 3 comments

Comments

@laflechejonathan
Copy link

laflechejonathan commented Jul 14, 2022

I'm exploring integrating buildkit with some internal tooling at my company, which including pushing container images to a private registry we've written in-house.

Buildkit fails to push with errors like:

error: failed to solve: failed commit on ref
"manifest-sha256:6ecb3bd918d6fc34639ec852f2f064b9fc20d22a63cb67b2c9a53afc295cc663":
got digest sha256:9660b67ae9e0c7b828f2c500367cd2f95cbeb703d2c1c164bec95d5ed5b48253,
expected sha256:6ecb3bd918d6fc34639ec852f2f064b9fc20d22a63cb67b2c9a53afc295cc663

This is actually the last step of the pushing process so the error can be ignored, but it got me wondering whether our registry implementation is wrong, or if this is a bug in buildkit.

Looking at this oci spec:

Upon successful completion of the request, the response MUST have code 201 Created and MUST have the following header:

Location:
With being a pullable blob URL.

I don't see any clear statement that the digest returned by the registry must match the one generated by the client. For the manifest in particular, there doesn't seem to be a canonical way to marshal it into JSON, which is why the digests differ between client and server in this case.

Am I missing something, or is the buildkit client too strict here?

@thaJeztah
Copy link
Member

there doesn't seem to be a canonical way to marshal it into JSON, which is why the digests differ between client and server in this case.

That's correct; there are some recommendations about using some canonicalization of the JSON, but (see the OCI distribution spec appendix and the Digest section in the OCI imager spect, the digest is calculated over the byte data, so effectively the JSON should be treated as an opaque blob (for purpose of calculating the digest). There is a mention about handling those digests, but it's a bit ambiguous as this section is described in the image spec, but referenced by the distribution spec;

Before consuming content targeted by a descriptor from untrusted sources, the byte content SHOULD be verified against the digest string.

However;

I don't see any clear statement that the digest returned by the registry must match the one generated by the client.

In this case, the error comes from validating the Docker-Content-Digest, which is handled by containerd's distribution code;

actual, err := digest.Parse(resp.Header.Get("Docker-Content-Digest"))
if err != nil {
return fmt.Errorf("invalid content digest in response: %w", err)
}
if actual != expected {
return fmt.Errorf("got digest %s, expected %s", actual, expected)
}

Handling Docker-Content-Digest appears to have landed in the Pull section of the spec (probably needs to be moved or at least referenced in the other section as well) https://github.com/opencontainers/distribution-spec/blob/v1.0.1/spec.md#pull

I do agree that the section on "push" could be clearer; there is a mention of the digest having to match the uploaded content; https://github.com/opencontainers/distribution-spec/blob/v1.0.1/spec.md#single-post

Likewise, the MUST match the blob's digest.

However, it leaves some things open to interpretation. AFAIK, given that registries provide a content-addressable store, the expectation should be that an uploaded artefact is stored verbatim, and after uploading can be accessed by its given digest, but the lack of documentation around that makes it indeed rather open to interpretation, and could be interpreted as either;

  • "the registry calculates the digest of the uploaded byte data, and includes it in the response for the client to verify"
  • "the registry verifies the byte data to match the digest query-parameter; on success it returns a 201 (but is allowed to re-marshal the JSON and produce its own digest (??) )"

It may be worth opening a ticket in the OCI distribution spec issue tracker (or a pull request to propose changes).

@tonistiigi
Copy link
Member

Your registry is not supposed to reencode the JSON uploaded by the client. The exact bytes should be stored and returned on the next GET.

@laflechejonathan
Copy link
Author

Thanks for the detailed explanation! I'll move the discussion over to the spec repo and close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants