Bug in image reference handling in Docker Driver

[simongdavies]

There is a bug in the way that the docker driver deals with Image references, it ignores any digest value and just use op.Image.Image:

https://github.com/deislabs/cnab-go/blob/26ed63f06aff506ab4bfea63a2a83d341f987580/driver/docker/docker.go#L158-L182

The Kubernetes Driver seems to do this correctly:

https://github.com/deislabs/cnab-go/blob/26ed63f06aff506ab4bfea63a2a83d341f987580/driver/kubernetes/kubernetes.go#L417-L422

Also see #144

[silvin-lubecki]

To me Digest field is a Content Digest you can compute locally, which is different from the Digested Reference returned by the registry. It shouldn't be used to fetch an image as a digested reference. Content Digest is there to let the runtime check the image, like a check sum.
I think we should rename that field to ContentDigest, like we did in the json annotation.

@radu-matei @glyn please correct me if I'm wrong.

[simongdavies]

Interestingly its serialized as contentDigest:

https://github.com/deislabs/cnab-go/blob/26ed63f06aff506ab4bfea63a2a83d341f987580/bundle/bundle.go#L84

Looks like Porter is using it for the Digested Reference.

Either way one of the implementations seems to be wrong and it would be useful to clarify

[glyn]

I think we need to think about this carefully to avoid confusion. Let's start with some definitions (based on the "OCI Image Format Specification").

Docker and OCI images have two types of digest: a repo digest and an image id. A repo digest is the SHA-256 digest of the compressed image manifest. Since compression depends on the implementation of the registry used to store the image, the repo digest doesn't logically exist until the image has been pushed. An image id, on the other hand, is the SHA-256 digest of the uncompressed image configuration, which is independent of the registry implementation.

Both these digests are content addresses of an image in the sense that each uniquely identifies the content (modulo SHA-256 collisions). Note that the docker registry spec refers to the repo digest as a "content digest".

The CNAB spec defines the contentDigest fields in bundle.json as follows, firstly for invocation images:

The contentDigest field MUST contain a digest, in OCI format, to be used to compute the integrity of the image. The calculation of how the image matches the contentDigest is dependent upon image type. (OCI, for example, uses a Merkle tree while VM images are checksums). During bundle development, it may be ideal to omit the contentDigest field and/or skip validation. Once a bundle is ready to be transmitted as a thick or thin bundle, it must have a contentDigest field. If a contentDigest field is present, a runtime MUST validate the image digest prior to executing an action. If the contentDigest is not present, the runtime SHOULD report an error so the user is aware that there is no contentDigest provided. Runtimes MAY allow users to override this behavior and perform actions on bundles that do not have contentDigest values populated.

and then for images other than invocation images:

contentDigest: MUST contain a digest of the contents of the image, in OCI format, to be used to compute the integrity of the image. The calculation of how the image matches the contentDigest is dependent upon image type. (OCI, for example, uses a Merkle tree while VM images use checksums.)

Since both repo digests and image ids are roots of Merkle trees, the CNAB spec doesn't actually prescribe whether repo digest or image id (or indeed some other Merkle tree root digest!) should be used for contentDigest fields of docker/OCI images. This needs clarifying so that CNAB runtimes know how to validate these fields, so I've raised cnabio/cnab-spec#287.

Regardless of which definition we choose, it seems clear that we can't simply append the contentDigest to the end of the image field to create a digested image reference because the image field may already be digested.

There is an efficiency consideration. If a repo digest is used as contentDigest, then checking this could be part of the normal process of dereferencing a docker/OCI image reference (particularly if the image reference is digested and the digest agrees with the contentDigest field). If an image id is used as contentDigest, then there will need to be a separate check of the decompressed image configuration:

• images would need to be downloaded by the CNAB runtime so that the decompressed image configuration could be digested
• such downloading would probably duplicate that performed by the relevant container runtime.