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

x509-cert: How to use dynamic signer with Builder? #1160

Open
jstayton opened this issue Jul 20, 2023 · 6 comments
Open

x509-cert: How to use dynamic signer with Builder? #1160

jstayton opened this issue Jul 20, 2023 · 6 comments

Comments

@jstayton
Copy link

Hey 👋🏻

I'm trying to use a dynamic signer when using RequestBuilder, but I'm running into this:

the size for values of type dyn Signer<_> cannot be known at compilation time
the trait Sized is not implemented for dyn Signer<_>

Here's a distillation of what I'm attempting:

use p256::pkcs8::LineEnding;
use rand_core::OsRng;
use std::error::Error;
use std::str::FromStr;
use x509_cert::{builder::RequestBuilder, name::Name};

fn main() -> Result<(), Box<dyn Error>> {
    let req_signer: Box<dyn p256::ecdsa::signature::Signer<_>>;

    if true {
        req_signer = Box::new(p256::ecdsa::SigningKey::random(&mut OsRng));
    } else {
        req_signer = Box::new(p384::ecdsa::SigningKey::random(&mut OsRng));
    }

    let subject = Name::from_str("CN=Test")?;

    let csr_builder = RequestBuilder::new(subject, req_signer.as_ref())?; // <--- Error

    let csr = csr_builder.build::<p256::ecdsa::DerSignature>()?; // <--- p256 or p384 would need to be used here

    let csr_pem = csr.to_pem(LineEnding::LF)?;

    println!("{}", csr_pem);

    Ok(())
}

Does that make sense? Am I just making a stupid mistake, or approaching this in the wrong way?

Thanks for any help!

@tarcieri
Copy link
Member

The signer parameter has bounds which are not object safe. Notably the Keypair trait uses an associated type.

@baloo one problem with providing the signer to RequestBuilder::new instead of RequestBuilder::build is it makes it a lot more difficult to pass different signers, i.e. RequestBuilder is now monomorphized for each signer instead of reusable.

@baloo
Copy link
Member

baloo commented Jul 21, 2023

It's not an issue for RequestBuilder, the one issue I'm concerned about is for CertificateBuilder where the behavior will change.

CertificateBuilder adds a set of basic extensions and subsequent calls to add_extension will call AsExtension on the object passing in the already added extensions.
The only concern/question I see is for BasicConstraints:

impl crate::ext::AsExtension for BasicConstraints {
fn critical(
&self,
_subject: &crate::name::Name,
_extensions: &[crate::ext::Extension],
) -> bool {
// https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.9
// Conforming CAs MUST include this extension in all CA certificates
// that contain public keys used to validate digital signatures on
// certificates and MUST mark the extension as critical in such
// certificates. This extension MAY appear as a critical or non-
// critical extension in CA certificates that contain public keys used
// exclusively for purposes other than validating digital signatures on
// certificates. Such CA certificates include ones that contain public
// keys used exclusively for validating digital signatures on CRLs and
// ones that contain key management public keys used with certificate
// enrollment protocols. This extension MAY appear as a critical or
// non-critical extension in end entity certificates.
// NOTE(baloo): from the spec, it doesn't appear to hurt if we force the extension
// to be critical.
true
}
}

baloo added a commit to baloo/formats that referenced this issue Jul 22, 2023
This is mostly a draft after discussion in RustCrypto#1160
baloo added a commit to baloo/formats that referenced this issue Jul 25, 2023
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
@bkstein
Copy link
Contributor

bkstein commented Jul 26, 2023

@jstayton I also had this problem and it took me some time to understand it. As @tarcieri mentioned, the call of RequestBuilder::new() is monomorphized by the compiler. This is not possible in your example, as two differently typed signers are passed in. As you can see in @baloo's example, this can be solved by moving/duplicating the call into scopes, where the signer's type can be statically derived. Note that baloo shifted the type parameter from RequestBuilder to RequestBuilder::build(), but the main issue is the same.

@tarcieri
Copy link
Member

@bkstein with #1161 it will only be monomorphized for build, which is probably okay for now.

Designing a 100% dynamic API for this will take some time.

@baloo
Copy link
Member

baloo commented Jul 27, 2023

I'm definitely not fixing signature::KeyPair here. I have no idea how to do that. but at least #1161 makes RequestBuilder reusable.

@tarcieri
Copy link
Member

@baloo it would require a dynamic alternative to signature::Keypair which returns a boxed trait object, but then we'd need to spell out the relevant traits the verifying key needs to support in advance.

Perhaps it would make more sense to define such a trait in x509-cert itself with everything it needs for the certificate builder, and then use blanket impls to wire up to the existing traits.

baloo added a commit to baloo/formats that referenced this issue Dec 13, 2023
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit to baloo/formats that referenced this issue Jan 2, 2024
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit to baloo/formats that referenced this issue Jan 4, 2024
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit to baloo/formats that referenced this issue Jan 19, 2024
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit to baloo/formats that referenced this issue Jan 20, 2024
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit to baloo/formats that referenced this issue Jan 20, 2024
This is mostly a draft after discussion in RustCrypto#1160

Signed-off-by: Arthur Gautier <[email protected]>
baloo added a commit that referenced this issue Jan 20, 2024
* x509-cert: don't bind builder with signer early

This is mostly a draft after discussion in #1160

Signed-off-by: Arthur Gautier <[email protected]>

* cms: pull builder changes

Signed-off-by: Arthur Gautier <[email protected]>

---------

Signed-off-by: Arthur Gautier <[email protected]>
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

No branches or pull requests

4 participants