RustCrypto / formats

Cryptography-related format encoders/decoders: DER, PEM, PKCS, PKIX
229 stars 122 forks source link

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

Open jstayton opened 1 year ago

jstayton commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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: https://github.com/RustCrypto/formats/blob/53db3d300cfa082931aac99e5544dff35613b6da/x509-cert/src/ext/pkix/constraints/basic.rs#L26-L49

bkstein commented 12 months ago

@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 commented 12 months ago

@bkstein with https://github.com/RustCrypto/formats/pull/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 commented 11 months ago

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

tarcieri commented 11 months ago

@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.