add certificate based verification #159
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xynnn007
previously approved these changes
Nov 15, 2022
| @@ -0,0 +1,211 @@ | |||
| use super::VerificationConstraint; | |||
There was a problem hiding this comment.
I like the refactoring here, making all the Constraint as submodule
| @@ -206,13 +206,21 @@ OSWS1X9vPavpiQOoTTGC0xX57OojUadxF1cdQmrsiReWg2Wn4FneJfa8xw== | |||
| pub subject_issuer: Option<String>, | |||
| pub not_before: DateTime<chrono::Utc>, | |||
| pub not_after: DateTime<chrono::Utc>, | |||
| pub private_key: pkey::PKey<pkey::Private>, | |||
There was a problem hiding this comment.
Not related to this PR. I found that the x509 cert used here depends on openssl, although all dev-dependency. Do we need to convert openssl -> RustCrypto (https://docs.rs/x509-cert/latest/x509_cert/) ?
There was a problem hiding this comment.
openssl is only used in tests (hence dev-dependency), as we have rustls-tls selected via features for the rest. I would be glad to substitute it as dev-dependency though.
There was a problem hiding this comment.
I'm fine moving away from the openssl crate from the unit tests, but I think this has a low priority.
We have to think about the pros and cons about this move.
From my POV:
Pros
- Move to something that is easier to consume. The documentation and the UX of the
opensslcrate is suboptimal
Cons
- We've already invested quite some time building wrappers around it inside of the unit test helpers
- We have access to all the features of openssl, with other libraries we might have to wait for some things to be implemented
- The certificates created inside of the unit tests use the same code a regular user would leverage when using the
opensslcli tool. What I mean is, we can rule our potential bugs introduced by the alternative library we would pick.
The openssl crate is not user friendly and we've already invested some time building around it inside of the unit tests. We have to po
There was a problem hiding this comment.
Consuming openssl-created certs in tests is an upside indeed, I think is worth the hassle. Let me backtrack my previous position. And given that we would need to find substitutes for all the openssl usages we have right now, plus it being low prio, I'm ok as it is.
Make some certificate related functions public inside of the crate. This allows their reuse. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
CertificatePool: define two different verification functions, depending on how the certificate to be verified is encoded (DER/PEM). Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Implement the `TryFrom` trait that allows to convert a x509 SubjectPublicKeyInfo into a CosignVerificationKey. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Move each verifier into its own `.rs` file. The original `verification_constraint.rs` file was too big. Note: the public API is not affected by this refactor. Everything keeps working as expected for the consumers of our library. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Add a new verification constraint that uses a certificate provided by the user to verify signatures. This can be used to verify all the signatures created via `cosign sign --cert` or with a PKCS11 token Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Expand the verify example to handle also certificate based verification. This is similar to what `cosign verify --cert` does. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Fix the inline documentation of `PublicKeyVerifier::try_from` about which RSA padding scheme is used when loading the key from a PEM file. Signed-off-by: Flavio Castelli <fcastelli@suse.com>
flavio
force-pushed
the
add-certificate-based-verification
branch
from
5e6da99
to
23f4c92
Compare
Merged
flavio
added a commit
to flavio/sigstore-rs
that referenced
this pull request
Nov 24, 2022
Fixes ===== * Fix typo in cosign/mod.rs doc comment by @danbev in sigstore#148 * Fix typo in KeyPair trait doc comment by @danbev in sigstore#149 * Update cached requirement from 0.39.0 to 0.40.0 by @dependabot in sigstore#154 * Fix typos in PublicKeyVerifier doc comments by @danbev in sigstore#155 * Fix: CI error for auto deref by @Xynnn007 in sigstore#160 * Fix typo and grammar in signature_layers.rs by @danbev in sigstore#161 * Remove unused imports in examples/rekor by @danbev in sigstore#162 * Update link to verification example by @danbev in sigstore#156 * Fix typos in from_encrypted_pem doc comments by @danbev in sigstore#164 * Fix typos in doc comments by @danbev in sigstore#163 * Update path to fulcio-cert in verify example by @danbev in sigstore#168 Enhancements ============ * Add getter functions for LogEntry fields by @lkatalin in sigstore#147 * Add TreeSize alias to Rekor by @avery-blanchard in sigstore#151 * Updates for parsing hashedrekord LogEntry by @lkatalin in sigstore#152 * Add certificate based verification by @flavio in sigstore#159 * Add support for OCI Image signing (spec v1.0) by @Xynnn007 in sigstore#158 Contributors ============ * Avery Blanchard (@avery-blanchardmade) * Daniel Bevenius (@danbev) * Flavio Castelli (@flavio) * Lily Sturmann (@lkatalin) * Xynnn (@Xynnn007) Signed-off-by: Flavio Castelli <fcastelli@suse.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces the ability to perform signature verifications using a x509 certificate.
This fixes issue #157
Note, some internal refactoring have been done to implement this feature. The public API of the crate has not changed.