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

Add --platform flag to cosign sbom download #1975

Merged
merged 9 commits into from
Jun 11, 2022

Conversation

puerco
Copy link
Member

@puerco puerco commented Jun 8, 2022

Summary

This PR adds a --platform flag to cosign sbom download . When downloading an SBOM from a multiarch index, the new flag lets the user specify the SBOM of one of the images listed in the index.

When trying to download an SBOM from an inbox that does not have one attached, cosign will now present the user with the available architectures:

This multiarch image does not have an SBOM attached at the index level.
Try using --platform with one of the following architectures:
 linux/amd64, linux/arm/v5, linux/arm/v7, linux/arm64/v8, linux/386, linux/mips64le, 
 linux/ppc64le, linux/s390x, windows/amd64:10.0.20348.707, windows/amd64:10.0.17763.2928

Error: no SBOM found attached to image index

Ticket Link

Fixes #1086

/cc @jdolitsky @imjasonh @mattmoor

Release Note

- New `--platform` flag  enables downloading a specific sbom from a multi-arch image
- Improved handling of error messages when a multiarch index does not have an sbom attached

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit implements the --platform flag of `cosign sbom download` that allows the
user to fetch an sbom for a specific arch.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2022

Codecov Report

Merging #1975 (b2f9054) into main (5f302b1) will decrease coverage by 6.01%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1975      +/-   ##
==========================================
- Coverage   34.71%   28.69%   -6.02%     
==========================================
  Files         153      133      -20     
  Lines       10037     8092    -1945     
==========================================
- Hits         3484     2322    -1162     
+ Misses       6166     5463     -703     
+ Partials      387      307      -80     
Impacted Files Coverage Δ
cmd/cosign/cli/download.go 0.00% <0.00%> (ø)
cmd/cosign/cli/options/download.go 0.00% <0.00%> (ø)
pkg/oci/remote/image.go 73.68% <0.00%> (ø)
pkg/policy/attestation.go 38.63% <0.00%> (-4.41%) ⬇️
pkg/cosign/verify.go 29.70% <0.00%> (-3.08%) ⬇️
cmd/cosign/cli/verify/verify_blob.go 10.03% <0.00%> (-0.26%) ⬇️
pkg/cosign/tuf/testutils.go 0.00% <0.00%> (ø)
cmd/cosign/cli/verify/verify.go 0.00% <0.00%> (ø)
pkg/cosign/kubernetes/client.go 0.00% <0.00%> (ø)
cmd/cosign/cli/options/predicate.go 0.00% <0.00%> (ø)
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f302b1...b2f9054. Read the comment docs.

@imjasonh
Copy link
Member

imjasonh commented Jun 8, 2022

So, I think we can proceed with this, and we'll be better off than we are today, but I think we might want to think about all the crazy ways people may expect to use this.

The main problem is that there may be multiple matching platforms, e.g.,:

  • --platform=linux matches all Linux manifests, of which there might be many, but there may only be one.
  • --platform=linux/arm matches linux/arm/v5 and linux/arm/v7 (this is pretty common, I think distroless supports both of these, but some images might not), and
  • getting into maximum crazy mode, --platform=windows/amd64:10.0.17763 matches Windows on x86_64, with the OS version 10.0.17763 (corresponding to Windows 10 (1809)), of which there may be multiple patch releases of, e.g., 10.0.17763.2928, 10.0.17763.1312, etc.

In this case I think we should probably handle multiple matching platforms with an error, since we don't know which one to download (and we shouldn't download multiple). The problem is that --platform=linux/arm will match multiple platforms, and --platform=windows/amd64:10.0.17763 won't exactly match any platform.

I'd suggest testing this out against ghcr.io/google/ko, which has attached SBOMs, and has the linux/arm/v5 and linux/arm/v7 issue, and two Windows platforms (though Windows images won't have SBOMs due to ko-build/ko#535)

@imjasonh
Copy link
Member

imjasonh commented Jun 8, 2022

This conversation is making me want to finally provide a method in ggcr to filter manifests by platform, encoding all this bonkers crazy logic.

This commit modifies the --platform flag in cosign download to improve
the handling of platform strings and matching.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
This commit refactors the platforms logic to its own image and improves the
user help when there is no sbom at the index level. cosign will now
show the user which platforms are available to use with --platform.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Add single image with platform case and update args in calls to download functions

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
@puerco
Copy link
Member Author

puerco commented Jun 9, 2022

OK, I've improved the platform parsing by following the way ko does it. It should now be ready to go, PTAL!

Here is a sample of the new platform handling

❯ cosign  download sbom ghcr.io/google/ko
WARNING: Downloading SBOMs this way does not ensure its authenticity. If you want to ensure a tamper-proof SBOM, download it using 'cosign download attestation <image uri>' or verify its signature.

This multiarch image does not have an SBOM attached at the index level.
Try using --platform with one of the following architectures:
 linux/amd64, linux/arm/v5, linux/arm/v7, linux/arm64/v8, linux/386, linux/mips64le, linux/ppc64le, linux/s390x, windows/amd64:10.0.20348.707, windows/amd64:10.0.17763.2928

Error: no SBOM found attached to image index
main.go:52: error during command execution: no SBOM found attached to image index
exit status 1


❯ cosign download sbom --platform=linux/arm ghcr.io/google/ko
WARNING: Downloading SBOMs this way does not ensure its authenticity. If you want to ensure a tamper-proof SBOM, download it using 'cosign download attestation <image uri>' or verify its signature.
Error: platform spec matches more than one image architecture: linux/arm/v5, linux/arm/v7
main.go:52: error during command execution: platform spec matches more than one image architecture: linux/arm/v5, linux/arm/v7
exit status 1

❯ cosign download sbom --platform=linux/arm/v7 ghcr.io/google/ko
WARNING: Downloading SBOMs this way does not ensure its authenticity. If you want to ensure a tamper-proof SBOM, download it using 'cosign download attestation <image uri>' or verify its signature.
Found SBOM of media type: text/spdx
SPDXVersion: SPDX-2.2
// rest of sbom....

@puerco puerco changed the title WIP cosign sbom download --platform Add --platform flag to cosign sbom download Jun 9, 2022
cmd/cosign/cli/download/sbom.go Outdated Show resolved Hide resolved
cmd/cosign/cli/download/sbom.go Outdated Show resolved Hide resolved
cmd/cosign/cli/download/sbom.go Outdated Show resolved Hide resolved
cmd/cosign/cli/download/sbom.go Show resolved Hide resolved
@@ -944,7 +949,7 @@ func TestAttachSBOM(t *testing.T) {
// Upload it!
must(attach.SBOMCmd(ctx, options.RegistryOptions{}, "./testdata/bom-go-mod.spdx", "spdx", imgName), t)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should attach also take a platform? 🤔

Copy link
Member Author

@puerco puerco Jun 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll add --platform to cosign attach sbom in a follow-up

Refacor listing platform to a method of platformList

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
imjasonh
imjasonh previously approved these changes Jun 10, 2022
return strings.Join(r, ", ")
}

func SBOMCmd(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not blocking) It looks like the []string return value is only ever read in test/e2e_test.go, and never in the actual CLI command. Is that right?

Maybe we should just have this return error alone, and refactor tests to not rely on the []string return value.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. I think the command was meant to return more than one sbom but it is short-circuited to spit out the one sbom. We should discuss if it will be returning more than one SBOM and decide how to change this. I think the logic to download the SBOMs (and the rest) should me moved away from the CLI to a more general package.

}

func findSignedImage(ctx context.Context, sii oci.SignedImageIndex, iDigest v1.Hash) (se oci.SignedEntity, err error) {
if err := walk.SignedEntity(ctx, sii, func(ctx context.Context, e oci.SignedEntity) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sii.SignedImage(iDigest) should do the same, without making you write the walk logic yourself. Let me know if that doesn't work for some reason.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay thanks for the tip @imjasonh 🎉 I pushed another commit dropping the function and switching to SignedImage()

This commit drops the findSignedImage() function in favor of the
SignedImge() function of the OCI package.

Signed-off-by: Adolfo García Veytia (Puerco) <[email protected]>
@dlorenc dlorenc merged commit ec590b7 into sigstore:main Jun 11, 2022
@github-actions github-actions bot added this to the v1.10.0 milestone Jun 11, 2022
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

Successfully merging this pull request may close these issues.

cosign download sbom for multi-arch images
4 participants