-
Notifications
You must be signed in to change notification settings - Fork 658
Have cosign sign support bundle format #4316
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4316 +/- ##
==========================================
- Coverage 40.10% 34.31% -5.79%
==========================================
Files 155 216 +61
Lines 10044 15009 +4965
==========================================
+ Hits 4028 5151 +1123
- Misses 5530 9194 +3664
- Partials 486 664 +178 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
haydentherapper
left a comment
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.
Amazing work on this! It's clear that this is going to greatly improve maintainability of this codebase with this change as sign and attest start looking quite similar.
| } | ||
|
|
||
| func signDigestBundle(ctx context.Context, digest name.Digest, ko options.KeyOpts, signOpts options.SignOptions, sv *SignerVerifier) error { | ||
| digestParts := strings.Split(digest.DigestStr(), ":") |
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 currently an expectation that the digest of the image is provided when signing a container? Could we compute this ourselves? Should we compute this regardless to verify the provided digest is correct?
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.
Yeah, before we get to this point cosign sign parses the OCI reference and uses that to compute the digest. Here we're just splitting the algorithm from the digest (that cosign sign has already computed) in order to create the in-toto subject.
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.
Perfect! Just wanted to check that wasn't just a user-provided digest.
| if err == nil { | ||
| pubKey = &pk | ||
| } | ||
| bundleBytes, err := cbundle.MakeNewBundle(pubKey, rekorEntry, payload, signedPayload, signerBytes, timestampBytes) |
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.
Why does MakeNewBundle take both a signer and the public key? Can this be simplified?
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.
This is a holdover from how the private function was before - it first tried to get a certificate from cryptoutils.UnmarshalCertificatesFromPEM(signer) and if it was unable to then it did pubKey, err := sv.PublicKey().
When I made this function public, it was creating an import loop with sign.SignerVerifier, so I opted to replace that option with *crypto.PublicKey. We could try to cram / overload signerBytes, but it's more work for the places we call MakeBundle() 🤷
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.
Eh, we can clean this up later with a larger refactor.
|
|
||
| must(cmd.Exec(ctx, args), t) | ||
| } | ||
|
|
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.
If possible, can we have an e2e test that demonstrates interactions with fulcio and rekor as well? There's an example above and in my signingconfig PR.
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 was able to get Rekor working the e2e test, but not Fulcio 😞
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.
Have you tried cosign sign --new-bundle-format without providing a --key? Just want to make sure it is working with ephemeral keys/Fulcio as expected.
What was the error you got in testing? I can try to get something working later on in the SigningConfig PR.
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.
Yes, sorry, I should have phrased that better. I was unable to get Fulcio working locally in the e2e test. I was able to get Fulcio verification working generally (see #4316 (comment)).
But I uploaded what I was working on locally, and it seemed to work in the GitHub workflow!
|
What do we want to do about |
|
Here's a good command for testing compatibility with GitHub Artifact Attestations: |
haydentherapper
left a comment
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.
Great work on this!
Signed-off-by: Zach Steindler <[email protected]>
Signed-off-by: Zach Steindler <[email protected]>
Signed-off-by: Zach Steindler <[email protected]>
Make constant for cosign sign in-toto predicate Update experimental OCI help string Signed-off-by: Zach Steindler <[email protected]>
Signed-off-by: Zach Steindler <[email protected]>
Signed-off-by: Zach Steindler <[email protected]>
…ulcio) Signed-off-by: Zach Steindler <[email protected]>
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cosign](https://github.com/sigstore/cosign) | minor | `2.5.3` -> `2.6.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>sigstore/cosign (cosign)</summary> ### [`v2.6.0`](https://github.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v260) [Compare Source](sigstore/cosign@v2.5.3...v2.6.0) v2.6.0 introduces a number of new features, including: - Signing an in-toto statement rather than Cosign constructing one from a predicate, along with verifying a statement's subject using a digest and digest algorithm rather than providing a file reference ([#​4306](sigstore/cosign#4306)) - Uploading a signature and its verification material (a ["bundle"](https://github.com/sigstore/protobuf-specs/blob/main/protos/sigstore_bundle.proto)) as an OCI Image 1.1 referring artifact, completing [#​3927](sigstore/cosign#3927) ([#​4316](sigstore/cosign#4316)) - Providing service URLs for signing and attesting using a [SigningConfig](https://github.com/sigstore/protobuf-specs/blob/4df5baadcdb582a70c2bc032e042c0a218eb3841/protos/sigstore_trustroot.proto#L185). Note that this is required when using a [Rekor v2](https://github.com/sigstore/rekor-tiles) instance ([#​4319](sigstore/cosign#4319)) Example generation and verification of a signed in-toto statement: ``` cosign attest-blob --new-bundle-format=true --bundle="digest-key-test.sigstore.json" --key="cosign.key" --statement="../sigstore-go/examples/sigstore-go-signing/intoto.txt" cosign verify-blob-attestation --bundle="digest-key-test.sigstore.json" --key=cosign.pub --type=unused --digest="b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9" --digestAlg="sha256" ``` Example container signing and verification using the new bundle format and referring artifacts: ``` cosign sign --new-bundle-format=true ghcr.io/user/alpine@sha256:a19367999603840546b8612572e338ec076c6d1f2fec61760a9e11410f546733 cosign verify --new-bundle-format=true ghcr.io/user/alpine@sha256:a19367999603840546b8612572e338ec076c6d1f2fec61760a9e11410f546733 ``` Example usage of a signing config provided by the public good instance's TUF repository: ``` cosign sign-blob --use-signing-config --bundle sigstore.json README.md cosign verify-blob --new-bundle-format --bundle sigstore.json --certificate-identity $EMAIL --certificate-oidc-issuer $ISSUER --use-signed-timestamps README.md ``` v2.6.0 leverages sigstore-go's signing and verification APIs gated behind these new flags. In an upcoming major release, we will be updating Cosign to default to producing and consuming bundles to align with all other Sigstore SDKs. #### Features - Add to `attest-blob` the ability to supply a complete in-toto statement, and add to `verify-blob-attestation` the ability to verify with just a digest ([#​4306](sigstore/cosign#4306)) - Have cosign sign support bundle format ([#​4316](sigstore/cosign#4316)) - Add support for SigningConfig for sign-blob/attest-blob, support Rekor v2 ([#​4319](sigstore/cosign#4319)) - Add support for SigningConfig in sign/attest ([#​4371](sigstore/cosign#4371)) - Support self-managed keys when signing with sigstore-go ([#​4368](sigstore/cosign#4368)) - Don't require timestamps when verifying with a key ([#​4337](sigstore/cosign#4337)) - Don't load content from TUF if trusted root path is specified ([#​4347](sigstore/cosign#4347)) - Add a terminal spinner while signing with sigstore-go ([#​4402](sigstore/cosign#4402)) - Require exclusively a SigningConfig or service URLs when signing ([#​4403](sigstore/cosign#4403)) - Remove SHA256 assumption in sign-blob/verify-blob ([#​4050](sigstore/cosign#4050)) - Bump sigstore-go, support alternative hash algorithms with keys ([#​4386](sigstore/cosign#4386)) #### Breaking API Changes - `sign.SignerFromKeyOpts` no longer generates a key. Instead, it returns whether or not the client needs to generate a key, and if so, clients should call `sign.KeylessSigner`. This allows clients to more easily manage key generation. #### Bug Fixes - Verify subject with bundle only when checking claims ([#​4320](sigstore/cosign#4320)) - Fixes to cosign sign / verify for the new bundle format ([#​4346](sigstore/cosign#4346)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMTMuNSIsInVwZGF0ZWRJblZlciI6IjQxLjExMy41IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
Summary
This is an implementation of #3927, but it also finishes the last secondary goal on #3139, which is just over 2 years old 🥳
More explicitly, it allows you to use
cosign signto product Sigstore protobuf signatures that we want everyone to move to, as well as slight updates tocosign verifyso it can verify those bundles.These bundle signatures will show up in
cosign treeif you include--experimental-oci11=true, but it will not work withcosign download, which does not look like it has been updated in awhile.I think this is the last step before we enable
--new-bundle-format=trueeverywhere (and maybe also--experimental-oci11=truein some places).You can test it with something like:
And then verify with something like:
Here's an example of what that bundle looks like:
And here's what the old signature looked like:
Release Note
cosign sign --new-bundle-format=trueto store signature in Sigstore protobuf bundlesDocumentation
Doc updates are in PR