RustCrypto / RSA

RSA implementation in pure Rust
Apache License 2.0
533 stars 144 forks source link

Generic digest for rsa::pkcs1v15::Signature? #364

Open QuinnWilton opened 12 months ago

QuinnWilton commented 12 months ago

Hey Tony, long time no chat!

I'm using RustCrypto in a project involving JOSE. I have some fairly open ended extensibility requirements, so I'm leveraging RustCrypto's generic signature traits as much as possible, to support users defining their own signing implementations, such as over WebCrypto with wasm, or using an HSM. As part of this, I've defined a trait for mapping RustCrypto signature encodings to their corresponding algorithm name in JWA.

This is generally working great, for most signature types:

pub trait JWSSignature: SignatureEncoding {
    const ALGORITHM: jose_jwa::Signing;
}

impl JWSSignature for ecdsa::Signature<p256::NistP256> {
    const ALGORITHM: jose_jwa::Signing = jose_jwa::Signing::Es256;
}

impl JWSSignature for ecdsa::Signature<k256::Secp256k1> {
    const ALGORITHM: jose_jwa::Signing = jose_jwa::Signing::Es256K;
}

But I run into issues with RSASSA-PKCS1-v1_5 and RSASSA-PSS signatures, as implemented in the rsa crate, because their corresponding signature types don't specify the hash function used in the signature, and so there isn't a 1:1 mapping between signatures and their JWA name.

For example:

impl JWSSignature for rsa::pkcs1v15::Signature {
    // Might be Rs256, Rs384, or Rs512
    const ALGORITHM: jose_jwa::Signing = ???;
}

This information is available on the corresponding signing key types for both variants of RSA signatures, so I'm wondering whether it would make sense for the digest type to also be tracked on the signatures themselves, in order to differentiate between signatures which use different hash functions.

In my case, it's not the end of the world if this isn't supported, because no one should really be deploying RSASSA-PKCS1-v1_5 using SHA-512 anyway, but that I ran into this limitation in the first place had me wondering whether it was a gap in the API worth thinking about.

Thanks for all the great work here, I'm really enjoying how the whole ecosystem slots together so nicely :)

QuinnWilton commented 12 months ago

Ah! It actually looks like you suggested essentially this design a few years ago: https://github.com/RustCrypto/RSA/issues/34#issuecomment-570364262

Is that still something that you think would be valuable? I personally also like the idea of including the MGF type for PSS too.

tarcieri commented 12 months ago

In the ecdsa crate I ended up adding a SignatureWithOid type for these sorts of use cases: https://docs.rs/ecdsa/0.16.8/ecdsa/struct.SignatureWithOid.html

A generic type parameter is another option but gets tricky if you encounter runtime dynamism like something like SignatureWithOid can handle.

Alternatively you can make your own newtype wrapper parameterized with your own algorithm identifier, similar to SignatureWithOid (but blissfully OID-free!)