-
Notifications
You must be signed in to change notification settings - Fork 503
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
Add support for DSSE envelope for attestation in imagetools #2194
Conversation
d2dbc26
to
8f248cc
Compare
util/imagetools/loader.go
Outdated
@@ -283,7 +284,9 @@ func (l *loader) scanSBOM(ctx context.Context, fetcher remotes.Fetcher, r *resul | |||
return nil, errors.Errorf("referenced image %s not found", dgst) | |||
} | |||
for _, layer := range mfst.manifest.Layers { | |||
if layer.MediaType == "application/vnd.in-toto+json" && layer.Annotations["in-toto.io/predicate-type"] == "https://spdx.dev/Document" { | |||
if (layer.MediaType == "application/vnd.in-toto+json" || | |||
layer.MediaType == "application/vnd.in-toto.spdx+dsse") && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have an image we can look at with this mime type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supporting this for imagetools inspect
is not a big issue I think but generally I don't see why the DSSE envelope should be needed for OCI structures. There is already envelope for referring to data in the form of OCI descriptor, it works natively and doesn't have the base64 overhead and obfuscation issues.
util/imagetools/loader.go
Outdated
return nil, err | ||
} | ||
|
||
decoded, err := base64.StdEncoding.DecodeString(dsse.Payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the payload be trusted if signatures (format) have not been verified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One could argue it should be trusted at the same level as one that isn't signed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a mechanism in place to throw warnings for imagetools inspect
commands? if so, adding a warning that the statements haven't been verified would help here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could potentially use logrus
here to print a warning along the lines of warning: sbom statements are not verified
until if/when this code checks signatures. I haven't seen many examples in the code base other than
buildx/util/cobrautil/cobrautil.go
Line 37 in a6228ed
logrus.Warningf("Unknown flag name %q", name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this shouldn't be SBOM specific and if we support this wrapper it should be same for provenance as well. Hopefully most of the code could be shared.
@tonistiigi is the provenance signed as well? |
If you are talking about attestations signed by buildkit then none of them are signed atm. Afaics these are for inspecting images from custom tooling. If SBOM can be wrapped with DSSE in this case then I don't see why provenance wouldn't as well. |
@tonistiigi the DSSE is used for signing the attestation so provenance should only need a DSSE envelope if it is signed as well. |
@LaurentGoderre are you just looking to sign the SBOM? Is there a reason why not just sign both SBOM and provenance attestations? |
@colinhemmings I am not sure if we are, I was asking. |
@tonistiigi said:
I agree here. Any format imagetools supports should also be supported when it is wrapped in a DSSE. SBOM, provenance, and whatever else imagetools supports that can be wrapped in a DSSE. |
8f248cc
to
184f443
Compare
184f443
to
abf5f79
Compare
…tools Signed-off-by: Laurent Goderre <laurent.goderre@docker.com>
abf5f79
to
b748185
Compare
@tonistiigi @whalelines I made more generic so it can be expanded to other DSSE formats. |
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
When running
docker buildx imagetools inspect --format '{{ JSON .SBOM }}
and the attestations use theapplication/vnd.in-toto+json
mime type, the In-TOTO predicate is returned. However, then the attestation are wrapped in a DSSE envelope using theapplication/vnd.in-toto.spdx+dsse
mime type, the same command returns and empty object ({}
).The changes add support for the DSSE mime type and base64 decode the envelope and returnb the payload object to stay consistent with images that do not use a wraper.