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 handling of keyless verification for all verify commands #3761

Merged
merged 10 commits into from
Jul 11, 2024

Conversation

dmitris
Copy link
Contributor

@dmitris dmitris commented Jul 2, 2024

Summary

Copy the handling of non-Fulcio keys from the verify to all the other verify commands (verify-attestation,
verify-blob, verify-blob-attestations).

Fix #3759.

Release Note

  • add keyless verification and --ca-roots / --ca-intermediates parameters to the
    verify-attestation, verify-blob, and verify-blob-attestations commands (in addition
    to verify)

Documentation

TODO - create a corresponding https://github.com/sigstore/docs PR, in particular in https://docs.sigstore.dev/verifying/verify/#local-verifications need to mention:

Verify image with local certificate and local CA roots and optional CA intermediate certificates file(s):

$ cosign verify --certificate cosign.crt --ca-roots ca-roots.crt [--ca-intermediates=ca-intermediates.crt] --certificate-oidc-issuer https://issuer.example.com --certificate-identity [email protected] user/demo

The command-line help as in cosign verify-blob --help already mentions the new parameter even though they don't yet properly work until this change is merged and released.

Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 30.76923% with 18 lines in your changes missing coverage. Please review.

Project coverage is 37.06%. Comparing base (2ef6022) to head (88881c9).
Report is 153 commits behind head on main.

Files Patch % Lines
cmd/cosign/cli/verify.go 40.00% 9 Missing ⚠️
cmd/cosign/cli/verify/verify_attestation.go 0.00% 3 Missing ⚠️
cmd/cosign/cli/verify/verify_blob.go 40.00% 2 Missing and 1 partial ⚠️
cmd/cosign/cli/verify/verify_blob_attestation.go 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3761      +/-   ##
==========================================
- Coverage   40.10%   37.06%   -3.04%     
==========================================
  Files         155      200      +45     
  Lines       10044    12280    +2236     
==========================================
+ Hits         4028     4552     +524     
- Misses       5530     7180    +1650     
- Partials      486      548      +62     

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

@dmitris dmitris force-pushed the issue3759 branch 22 times, most recently from 4ab0e21 to 97c5573 Compare July 9, 2024 11:07
@dmitris
Copy link
Contributor Author

dmitris commented Jul 9, 2024

Verification: the script https://github.com/dmitris/cosign-keyless/blob/main/verify-blob.sh fails with the trunk's version of cosign:

$ ./verify-blob.sh
Wrote signature to file README.md.sig
cosign verify-blob (with --certificate-chain):
Verified OK
cosign verify-blob (with --ca-roots):
Error: cert verification failed: x509: certificate signed by unknown authority. Check your TUF root (see cosign initialize) or set a custom root with env var SIGSTORE_ROOT_FILE
main.go:74: error during command execution: cert verification failed: x509: certificate signed by unknown authority. Check your TUF root (see cosign initialize) or set a custom root with env var SIGSTORE_ROOT_FILE

but works with the version build in this PR's branch:

$ COSIGN=$HOME/gh/sigstore/cosign/cosign ./verify-blob.sh
Wrote signature to file README.md.sig
/Users/dsavints/gh/sigstore/cosign/cosign verify-blob (with --certificate-chain):
Verified OK
/Users/dsavints/gh/sigstore/cosign/cosign verify-blob (with --ca-roots):
Verified OK

Similarly with cosign verify-blob --bundle - https://github.com/dmitris/cosign-keyless/blob/main/verify-blob-bundle.sh.

@dmitris dmitris marked this pull request as ready for review July 9, 2024 12:08
Copy link
Member

@cpanato cpanato left a comment

Choose a reason for hiding this comment

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

looks good so far, as you mention to me, i would like to have some tests for those

@dmitris dmitris force-pushed the issue3759 branch 3 times, most recently from b6ed504 to c6094b2 Compare July 9, 2024 16:06
@dmitris dmitris force-pushed the issue3759 branch 2 times, most recently from fb59f93 to 339224b Compare July 9, 2024 16:43
Copy the handling of non-Fulcio keys from the verify
to all other verify commands (verify-attestation,
verify-blob, verify-blob-attestations).

Fix sigstore#3759.

Signed-off-by: Dmitry S. <[email protected]>
Signed-off-by: Dmitry S <[email protected]>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

cmd/cosign/cli/verify/verify_attestation.go Show resolved Hide resolved
test/e2e_test.go Outdated Show resolved Hide resolved
Signed-off-by: Dmitry S <[email protected]>
@haydentherapper haydentherapper merged commit bdcbf44 into sigstore:main Jul 11, 2024
22 checks passed
@dmitris dmitris deleted the issue3759 branch July 11, 2024 17:19
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.

feature: 'cosign verify-*' add flags --ca-roots and --ca-intermediates to allow multiple CA roots
3 participants