rust-oci-client icon indicating copy to clipboard operation
rust-oci-client copied to clipboard

image pulling does not validate digests

Open burgerdev opened this issue 1 year ago • 3 comments

When pulling an image by digest, I would expect the library to verify that the digest of the returned resource matches the requested digest.

For testing purposes, I set up a registry that returns the same image for any requested digest:

$ curl -s https://evil-registry.rudy.family/v2/busybox/manifests/sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef | sha256sum
d319b0e3e1745e504544e931cde012fc5470eba649acc8a7b3607402942e5db7  -

Docker rejects to pull this image:

$ docker pull evil-registry.rudy.family/busybox@sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
evil-registry.rudy.family/busybox@sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef: Pulling from busybox
manifest verification failed for digest sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef

Pulling with oci-distribution does not fail, and returns the bogus hash to the user:

let arg = "evil-registry.rudy.family/busybox@sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
let image: Reference = arg.parse().unwrap();
let client = Client::default();
let auth = &oci_distribution::secrets::RegistryAuth::Anonymous;
let accepted_media_types = vec!["application/vnd.docker.image.rootfs.diff.tar.gzip"];

let (_m, d) = client.pull_manifest(&image, auth).await.unwrap();
println!("pull_manifest({}) -> {}", arg, d);

let data = client.pull(&image, auth, accepted_media_types).await.unwrap();
println!("pull({}) -> {:?}", arg, data.digest);
pull_manifest(evil-registry.rudy.family/busybox@sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef) -> sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
pull(evil-registry.rudy.family/busybox@sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef) -> Some("sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef")

The spec says this regarding content verification:

Most clients MAY ignore the [Docker-Content-Digest header], but if it is used, the client MUST verify the value against the uploaded blob data.

The full reproducer is available at https://github.com/burgerdev/evil-registry/.

burgerdev avatar Jul 29 '24 10:07 burgerdev

Thanks for filing this @burgerdev. It looks like the spec also says the same thing about blobs as well as the manifest itself. So we should check for the presence of the header and then validate whatever comes back (manifest or blob) with that digest.

thomastaylor312 avatar Jul 29 '24 22:07 thomastaylor312

Yes, and I think it should be two checks:

  1. Does the Docker-Content-Digest match the returned body?
  2. Does the requested digest match the actual digest?

For (2), the spec is a bit vague around canonical digests. However, I'd argue that pulling an explicitly pinned image should fail if the requested digest is not available, and that it's a registry error if blobs mentioned in a manifest are not available under their digests.

burgerdev avatar Jul 30 '24 06:07 burgerdev

So basically the desired logic is "if my reference contains a digest rather than a version, then validate that the returned content matches that digest"

thomastaylor312 avatar Jul 30 '24 16:07 thomastaylor312

Fyi, I proposed a clarification for the distribution spec in https://github.com/opencontainers/distribution-spec/issues/549.

burgerdev avatar Aug 04 '24 18:08 burgerdev

Thanks for doing that. Looks like there is some decent support for it there, so I'm glad we're implementing it here

thomastaylor312 avatar Aug 05 '24 18:08 thomastaylor312