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 support for new bundle specification for attesting/verifying OCI image attestations #3889

Open
wants to merge 37 commits into
base: main
Choose a base branch
from

Conversation

codysoyland
Copy link
Member

@codysoyland codysoyland commented Sep 25, 2024

Summary

This PR adds support for the new Cosign Bundle Specification in cosign attest and cosign verify-attestation.

Related: #3139

To test, run the following (replacing MY_IDENTITY, MY_ISSUER, MY_TRUSTED_ROOT and MY_IMAGE as needed -- trusted root is optional). Note that the new OCI support requires passing --new-bundle-format into both commands.

go run ./cmd/cosign attest --predicate my-predicate.json --new-bundle-format MY_IMAGE
go run ./cmd/cosign verify-attestation --certificate-identity MY_IDENTITY --certificate-oidc-issuer MY_ISSUER --new-bundle-format --trusted-root=MY_TRUSTED_ROOT MY_IMAGE

Full example (using crane, but can instead use docker tag/docker push):

make cosign
docker run -d -p 5050:5000 ghcr.io/project-zot/zot-linux-arm64:latest
crane copy busybox:latest localhost:5050/busybox
echo '{"foo": "bar"}' > predicate.json
./cosign attest --predicate predicate.json --new-bundle-format localhost:5050/busybox:latest
# Dex workflow, assuming GitHub as provider
./cosign verify-attestation --certificate-identity=yourname@example.com --certificate-oidc-issuer=https://github.com/login/oauth --new-bundle-format localhost:5050/busybox:latest

To show that it uses the OCI 1.1 referrers API, you can use oras:

oras discover localhost:5050/busybox:latest
Discovered 1 artifact referencing latest
Digest: sha256:db142d433cdde11f10ae479dbf92f3b13d693fd1c91053da9979728cceb1dc68

Artifact Type                                   Digest
application/vnd.dev.sigstore.bundle.v0.3+json   sha256:3fb46d845fe437667a6b3ed45d2b11dea11c43f8fbe76dd642eb71de2a9b8b77

Release Note

Documentation

Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland codysoyland force-pushed the verify-attestation-bundle-spec branch from 4af8cc0 to e509ec5 Compare November 1, 2024 20:41
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Copy link

codecov bot commented Nov 6, 2024

Codecov Report

Attention: Patch coverage is 10.29412% with 366 lines in your changes missing coverage. Please review.

Project coverage is 35.81%. Comparing base (2ef6022) to head (2c27fc5).
Report is 258 commits behind head on main.

Files with missing lines Patch % Lines
pkg/cosign/verify.go 19.27% 126 Missing and 8 partials ⚠️
pkg/oci/remote/write.go 0.00% 92 Missing ⚠️
cmd/cosign/cli/verify/verify_attestation.go 0.00% 35 Missing ⚠️
cmd/cosign/cli/attest/attest.go 0.00% 32 Missing ⚠️
pkg/oci/remote/signatures.go 0.00% 31 Missing ⚠️
cmd/cosign/cli/attest/common.go 0.00% 10 Missing ⚠️
cmd/cosign/cli/verify/verify.go 0.00% 8 Missing ⚠️
cmd/cosign/cli/attest/attest_blob.go 14.28% 6 Missing ⚠️
cmd/cosign/cli/verify.go 0.00% 6 Missing ⚠️
pkg/cosign/verify_bundle.go 40.00% 4 Missing and 2 partials ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3889      +/-   ##
==========================================
- Coverage   40.10%   35.81%   -4.29%     
==========================================
  Files         155      210      +55     
  Lines       10044    13666    +3622     
==========================================
+ Hits         4028     4895     +867     
- Misses       5530     8142    +2612     
- Partials      486      629     +143     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland codysoyland mentioned this pull request Nov 8, 2024
2 tasks
This reverts commit 67d421a.

Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland codysoyland force-pushed the verify-attestation-bundle-spec branch from af5093d to 524f558 Compare November 22, 2024 16:44
@codysoyland codysoyland marked this pull request as ready for review November 22, 2024 19:55
…cheme natively

Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland codysoyland changed the title Add support for new bundle specification in cosign verify-attestation Add support for new bundle specification for attesting/verifying OCI image attestations Nov 22, 2024
cmd/cosign/cli/attest/attest.go Show resolved Hide resolved
cmd/cosign/cli/attest/common.go Show resolved Hide resolved
cmd/cosign/cli/options/verify.go Show resolved Hide resolved
cmd/cosign/cli/verify/verify_attestation.go Outdated Show resolved Hide resolved
cmd/cosign/cli/verify/verify_attestation.go Outdated Show resolved Hide resolved
pkg/cosign/verify.go Outdated Show resolved Hide resolved
pkg/cosign/verify.go Show resolved Hide resolved
pkg/cosign/verify.go Show resolved Hide resolved
pkg/cosign/verify.go Outdated Show resolved Hide resolved
pkg/cosign/verify.go Outdated Show resolved Hide resolved
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
@codysoyland codysoyland requested a review from a team as a code owner December 10, 2024 20:08
pkg/cosign/verify.go Show resolved Hide resolved
pkg/cosign/verify.go Outdated Show resolved Hide resolved
pkg/cosign/verify.go Outdated Show resolved Hide resolved
// Wrap TrustedMaterial
vTrustedMaterial := &verifyTrustedMaterial{TrustedMaterial: co.TrustedMaterial}

// If TrustedMaterial is not set, fetch it from TUF (TODO: should this even be done? Old verifier requires co.RootCerts to be set)
Copy link
Contributor

Choose a reason for hiding this comment

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

If they haven't set --trusted-root then they could be using other flags or relying on the TUF v1 setup which might be pointed to something other than the public good instance. I understand that this is only meant to be called when --new-bundle-format is set but I'm worried that this would be surprising behavior if the PGI trusted root is used while the cached TUF metadata is pointed somewhere else.

pkg/cosign/verify.go Outdated Show resolved Hide resolved
}

for _, verified := range bundlesVerified {
atLeastOneBundleVerified = atLeastOneBundleVerified || verified
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you clarify why it's okay for only one bundle to be verified?

@@ -25,7 +25,9 @@ import (
func Referrers(d name.Digest, artifactType string, opts ...Option) (*v1.IndexManifest, error) {
o := makeOptions(name.Repository{}, opts...)
rOpt := o.ROpt
rOpt = append(rOpt, remote.WithFilter("artifactType", artifactType))
if artifactType != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there not a more specific artifact type we can filter by when getting attestations?

Copy link
Member Author

Choose a reason for hiding this comment

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

The artifactType for bundles is something like application/vnd.dev.sigstore.bundle.v0.3+json. Since the version number is baked into the type, we can't use the filter feature here. Therefore, getBundles uses an empty string as the artifactType in the call to Referrers.

@@ -361,7 +377,7 @@ func attestVerify(t *testing.T, predicateType, attestation, goodCue, badCue stri
}

// Now attest the image
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc}
ko := options.KeyOpts{KeyRef: privKeyPath, PassFunc: passFunc, NewBundleFormat: newBundleFormat}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe out of scope for this PR but a note for the future, this is setting NewBundleFormat without setting TrustedRootPath, which means the TrustedMaterial will default to using PGI. We're trying to avoid making external network calls to live services. The only reason that isn't a problem here is because this test is using a local key pair and is not uploading to Rekor, so it doesn't matter what verification material is used. But a useful test in the future might be to have this use ephemeral keys from Fulcio (localhost:5555) and upload the entry to Rekor (localhost:3000) so that the full verification path with a locally generated trust root could be tested.

@@ -156,13 +163,113 @@ type CheckOpts struct {
// IgnoreTlog skip tlog verification
IgnoreTlog bool

// UseSignedTimestamps use signed timestamps if available
UseSignedTimestamps bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this potentially something that could be split out into its own PR, so that each PR has a clear singular focus?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good idea for something we can split out... I think this is needed because the existing timestamp verification in cosign doesn't work with TUF/PGI, and is only enforced if you provide a timestamp cert chain to the verification subcommands.

codysoyland and others added 7 commits December 18, 2024 10:22
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
Co-authored-by: Colleen Murphy <cmurphy@users.noreply.github.com>
Signed-off-by: Cody Soyland <codysoyland@github.com>
)

// VerifyNewBundle verifies a SigstoreBundle with the given parameters
func VerifyNewBundle(_ context.Context, co *CheckOpts, artifactPolicyOption verify.ArtifactPolicyOption, bundle verify.SignedEntity) (*verify.VerificationResult, error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Consider splitting this into separate PR and refactoring existing verifyNewBundle.

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.

3 participants