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.

Sign up for GitHub

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

Jump to bottom

Add support for intermediate certificates when verifiying #1631

Merged
merged 2 commits into from
Mar 23, 2022
Merged

Add support for intermediate certificates when verifiying #1631

merged 2 commits into from
Mar 23, 2022

Conversation

haydentherapper
Copy link
Contributor

@haydentherapper haydentherapper commented Mar 17, 2022 •
edited
Loading

This adds an intermediate CA certificate pool to CheckOpts, allowing for
those using the Cosign library to pass intermediate CA certificates to
validate a certificate chain.

Next steps:

  • Add an option for specifying chain by command line. I need to look into how --cert is working currently.
  • Research if we can add the chain to a Rekor entry too

Signed-off-by: Hayden Blauzvern hblauzvern@google.com

Summary

Ticket Link

Ref #1554

Release Note

Added library support for intermediate CA certificates when verifying
Added support for verifying OCI images with a certificate chain

@haydentherapper
Copy link
Contributor Author

cc @bburky @dlorenc

@haydentherapper haydentherapper marked this pull request as draft March 17, 2022 23:32
@haydentherapper
Copy link
Contributor Author

Moving to draft, going to work on some of the other TODOs in this PR

@dlorenc
Copy link
Member

dlorenc commented Mar 18, 2022

Nice!

hectorj2f
hectorj2f reviewed Mar 18, 2022
View reviewed changes
pkg/cosign/verify.go Outdated Show resolved Hide resolved
hectorj2f
hectorj2f reviewed Mar 21, 2022
View reviewed changes
Copy link
Contributor

@hectorj2f hectorj2f left a comment

Choose a reason for hiding this comment

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

lgtm

@bburky
Copy link
Contributor

bburky commented Mar 21, 2022 •
edited
Loading

This looks good, and is definitely needed as part of validating the chain correctly.

Is the intent to connect this to Chain() in pkg/oci/internal/signature/layer.go? I think all of the certs in the chain annotation can be loaded into co.IntermediateCerts? (the final cert in the chain will be a root, but I think you can provide it as an intermediate for validation purposes, trust in it comes from it being present in co.RootCerts too.)

Edit: I see you mentioned "Include the intermediates from the OCI Chain annotation" in this issue too now. Looks good.

@haydentherapper
Copy link
Contributor Author

Yep, planning to add that to this PR before moving it out of a draft! Wrote up a document outlining all of the places we need to make updates to support intermediates, feel free to take a look.

@haydentherapper
Add support for intermediate certificates when verifiying
3e7fbf5 
This adds an intermediate CA certificate pool to CheckOpts, allowing for
those using the Cosign library to pass intermediate CA certificates to
validate a certificate chain.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2022 •
edited
Loading

Codecov Report

Merging #1631 (0b30662) into main (2fcf30e) will increase coverage by 0.29%.
The diff coverage is 48.38%.

❗ Current head 0b30662 differs from pull request most recent head 0eee964. Consider uploading reports for the commit 0eee964 to get more accurate results

@@            Coverage Diff             @@
##             main    #1631      +/-   ##
==========================================
+ Coverage   27.97%   28.27%   +0.29%     
==========================================
  Files         137      137              
  Lines        7820     7847      +27     
==========================================
+ Hits         2188     2219      +31     
+ Misses       5403     5387      -16     
- Partials      229      241      +12
Impacted Files Coverage Δ
pkg/cosign/verify.go 16.43% <48.38%> (+4.76%) ⬆️

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 2fcf30e...0eee964. Read the comment docs.

@haydentherapper
Populate intermediate certs from the OCI chain annotation
ed0aa58 
This adds support for verifying OCI signatures with chains
that include more than one certificate in the chain, a root and
subordinate.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper haydentherapper marked this pull request as ready for review March 22, 2022 00:21
@haydentherapper
Copy link
Contributor Author

Added support for extracting the chain from the OCI annotation.

@haydentherapper
Copy link
Contributor Author

@dlorenc @hectorj2f Do y'all have any other comments?

@dlorenc
Copy link
Member

dlorenc commented Mar 23, 2022

The plan in your doc sgtm, thanks! This pr looks good too!

@dlorenc dlorenc merged commit 7402eb4 into sigstore:main Mar 23, 2022
@github-actions github-actions bot added this to the v1.7.0 milestone Mar 23, 2022
@hectorj2f
Copy link
Contributor

@haydentherapper lgtm, thanks

@haydentherapper haydentherapper deleted the add-intermediates branch March 24, 2022 22:41
@bburky bburky mentioned this pull request Mar 29, 2022
Closed
2 tasks
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
@haydentherapper @mlieberman85
Add support for intermediate certificates when verifiying (sigstore#1631
8709a62 
)

* Add support for intermediate certificates when verifiying

This adds an intermediate CA certificate pool to CheckOpts, allowing for
those using the Cosign library to pass intermediate CA certificates to
validate a certificate chain.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>

* Populate intermediate certs from the OCI chain annotation

This adds support for verifying OCI signatures with chains
that include more than one certificate in the chain, a root and
subordinate.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hectorj2f hectorj2f hectorj2f left review comments

@dlorenc dlorenc dlorenc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7.0
Development

Successfully merging this pull request may close these issues.
5 participants
@haydentherapper @dlorenc @bburky @codecov-commenter @hectorj2f