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

Support PKCS1 encoded and non-ECDSA CT log public keys #1806

Merged
merged 3 commits into from Apr 27, 2022

Conversation

haydentherapper
Copy link
Contributor

@haydentherapper haydentherapper commented Apr 26, 2022

This came up while testing out staging, which uses a PKCS1 encoded
public key. We should be flexible on the supported key format.

This also relaxes the requirement that CT log keys from TUF are ECDSA keys.

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

Summary

Ticket Link

Fixes

Release Note

Added support for non-ECDSA and PKCS1-encoded CT log public keys from TUF

This came up while testing out staging, which uses a PKCS1 encoded
public key. We should be flexible on the supported key format.

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

cc @dlorenc - If 1.8 hasn't gone out yet, getting this in too would be nice!

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
dlorenc
dlorenc previously approved these changes Apr 26, 2022
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

@dlorenc - pushed another commit that removes the requirement that the key is an ECDSA key - I don't see any reason this should be enforced.

@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2022

Codecov Report

Merging #1806 (b9280fb) into main (db323cd) will increase coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #1806      +/-   ##
==========================================
+ Coverage   32.73%   32.75%   +0.01%     
==========================================
  Files         147      147              
  Lines        9313     9327      +14     
==========================================
+ Hits         3049     3055       +6     
- Misses       5907     5915       +8     
  Partials      357      357              
Impacted Files Coverage Δ
cmd/cosign/cli/fulcio/fulcioverifier/ctl/verify.go 49.26% <50.00%> (+1.40%) ⬆️
pkg/cosign/tuf/client.go 61.68% <0.00%> (-0.82%) ⬇️
cmd/cosign/cli/verify/verify.go 0.00% <0.00%> (ø)
cmd/cosign/cli/verify/verify_attestation.go 0.00% <0.00%> (ø)
cmd/cosign/cli/fulcio/fulcioroots/fulcioroots.go 36.36% <0.00%> (+0.42%) ⬆️
pkg/cosign/verify.go 29.95% <0.00%> (+0.47%) ⬆️

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 db323cd...b9280fb. Read the comment docs.

@haydentherapper haydentherapper changed the title Support PKCS1 encoded CT log public keys Support PKCS1 encoded and non-ECDSA CT log public keys Apr 26, 2022
@dlorenc dlorenc merged commit 9ef6b20 into sigstore:main Apr 27, 2022
@github-actions github-actions bot added this to the v1.8.0 milestone Apr 27, 2022
@haydentherapper haydentherapper deleted the support-pkcs1 branch April 27, 2022 21:45
mlieberman85 pushed a commit to mlieberman85/cosign that referenced this pull request May 6, 2022
* Support PKCS1 encoded CT log public keys

This came up while testing out staging, which uses a PKCS1 encoded
public key. We should be flexible on the supported key format.

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

* Update comment

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

* Remove requirement that key is ECDSA

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants