-
Notifications
You must be signed in to change notification settings - Fork 134
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
x509-cert: don't bind builder with signer early #1161
x509-cert: don't bind builder with signer early #1161
Conversation
Looks good at first glance |
I think, this will improve the situation. However, monomorphization will still take place. |
The signing API isn't dynamic (yet) so we can't yet avoid monomorphization around the signing operation. It would need fully dynamic counterparts to everything in |
ca9a75f
to
490edd4
Compare
This is a breaking change. I don't think we mean to introduce a |
Yeah, that's fine. We can keep it open until the next breaking release cycle. |
41e0d7b
to
25a4391
Compare
cms/src/builder.rs
Outdated
fn finalize<S>( | ||
&mut self, | ||
_signer: &S, | ||
) -> core::result::Result<Vec<u8>, x509_cert::builder::Error> |
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.
Shouldn't this be independent of x509_cert
?
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.
?
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.
That was just a rebase from this PR on a fresh master.
But I needed to change the signature of finalize because I attach the public key of the signer "late".
Getting the public from the keypair made it so that I could not get away with just a der::Result
.
This is still a breaking change though.
25a4391
to
a561c25
Compare
a561c25
to
bbe0523
Compare
5a3ce21
to
183bbbf
Compare
This is mostly a draft after discussion in RustCrypto#1160 Signed-off-by: Arthur Gautier <[email protected]>
Signed-off-by: Arthur Gautier <[email protected]>
183bbbf
to
ff582c3
Compare
Could
One of the shortcomings of the current API is its assumption that the private key is always available. By adopting this approach, only the build method would necessitate access to the private key (or signer). Users could directly invoke |
Here is an example of the use of this API (before this PR merged): Here is an example of the use of this API (after this PR merged): Both are use-cases where the signer doesn't have direct access to the private material (PKCS11 for use on HSMs, YubiKey in the PIV applet). For YubiHSMs there are also various implementations of signers:
I'd like to come around and make a compatibility crate for TPMs at some point (in https://github.com/parallaxsecond/rust-tss-esapi) too. I'd love help on any of those :) But I think that should address your concerns about hardware tokens. |
Yeah thanks. This looks really awesome. |
This is mostly a draft after discussion in #1160
cc @bkstein @tarcieri @jstayton