moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
7.85k stars 1.09k forks source link

ECR security scanning fails on images built using BuildKit v0.11 #3499

Open jedevc opened 1 year ago

jedevc commented 1 year ago

:bug: Originally reported by @farvour in https://github.com/docker/build-push-action/pull/746#issuecomment-1377806123, tracking here since the original issue is about adding attestations fields in docker/build-push-action.

cc @crazy-max @tonistiigi

With the release of Buildkit v0.11, by default, a minimal provenance attestation is created and pushed alongside the image, using the attestation storage. This apparently causes ECR image scans to fail.

https://github.com/docker/build-push-action/pull/746#issuecomment-1377806123:

I am using this action in our workflows to build. I am discovering that ECR does not like the attestation layers pushed when security scanning is enabled on a repository. It results in a Failed on the image UI state. Not a major issue, but sounds like this new feature to this action will let me turn these off with the newer docker buildx (0.10.0+) if my understanding is correct?

While the layers for the attestation storage cannot be unpacked, the OCI image-spec explicitly declares:

This descriptor property has additional restrictions for layers[]. Implementations MUST support at least the following media types: ... Manifests concerned with portability SHOULD use one of the above media types. An encountered mediaType that is unknown to the implementation MUST be ignored.

Ideally, scanners should either:

I don't think there's anything we can do buildkit side? As far as I'm aware, the attestation manifests generated by buildkit are compliant with OCI artifacts.

tonistiigi commented 1 year ago

With the release of Buildkit v0.11, by default, a minimal provenance attestation is created and pushed alongside the image,

Default is only in buildx level.

I don't think there's anything we can do buildkit side?

I think we are doing it correctly. The objects have proper mediatypes that show that blob is in-toto json and not a targz.

FYI @estesp

farvour commented 1 year ago

I did open a ticket with our AWS support to see what their response is. We are in US Gov Cloud in case that makes any difference.

It sounds like these layers just shouldn't be processed by Snyk or the failures bubble up to the UI. They are also un-deletable (as expected) since they are part of the list manifest.

We may have to wait for AWS ECR team to act for a resolution?

farvour commented 1 year ago

I did open a ticket with our AWS support to see what their response is. We are in US Gov Cloud in case that makes any difference.

It sounds like these layers just shouldn't be processed by Snyk or the failures bubble up to the UI. They are also un-deletable (as expected) since they are part of the list manifest.

We may have to wait for AWS ECR team to act for a resolution?

Spoke with our support and had a great call. Sounds like they will bring this back to their product and decide on how to present the layers to the user. I think it made the most sense to at least not try and scan the attestation layers, but still show them in the registry, even if unsupported. That makes sense.

Since none of this affects the actual functionality of the image, registry or tooling it's not blocking anything that I'm aware of (unless someone has alerting or monitoring set up on ECR layer failures, of course). Nonetheless, a resolution should happen.