New API design

[str4d]

Prompted by RustCrypto/signatures#25 (comment) and my desire to get #18 and #26 merged 😄

The main design difference compared to the current traits is that instead of making the "padding scheme" a parameter of the pubkey sign/verify functions, we make each scheme a first-class primitive that wraps the common public or private key. It needs to be an explicit choice by the user anyway.

Current draft proposal (as of 2020-03-07) (without changes to the signature crate, and without handling the encryption cases):

// This module has been merged into master
mod raw {
 pub trait EncryptionPrimitive {
 /// Do NOT use directly! Only for implementors.
 fn raw_encryption_primitive(&self, plaintext: &[u8]) -> Result<Vec<u8>>;
 }

 pub trait DecryptionPrimitive {
 /// Do NOT use directly! Only for implementors.
 fn raw_decryption_primitive<R: Rng>(
 &self,
 rng: Option<&mut R>,
 ciphertext: &[u8],
 ) -> Result<Vec<u8>>;
 }
}

mod key {
 pub trait PublicKeyParts {
 /// Returns the modulus of the key.
 fn n(&self) -> &BigUint;
 /// Returns the public exponent of the key.
 fn e(&self) -> &BigUint;
 /// Returns the modulus size in bytes. Raw signatures and ciphertexts for
 /// or by this public key will have the same size.
 fn size(&self) -> usize {
 (self.n().bits() + 7) / 8
 }
 }

 pub trait PrivateKey: crate::raw::DecryptionPrimitive + PublicKeyParts {
 /// Could have functions like this for usability?
 pub fn sign_pkcs1v15(&self) -> crate::pkcs1v15::Signer;
 pub fn sign_pkcs1v15_blinded<R: Rng>(&self, rng: R) -> crate::pkcs1v15::Signer;
 pub fn sign_pss(&self) -> crate::pss::Signer;
 pub fn sign_pss_blinded(&self) -> crate::pss::Signer;
 }

 pub trait PublicKey: crate::raw::EncryptionPrimitive + PublicKeyParts {
 /// Could have functions like this for usability?
 pub fn verify_pkcs1v15(&self) -> crate::pkcs1v15::Verifier;
 pub fn verify_pss(&self) -> crate::pss::Verifier;
 }

 pub struct RSAPrivateKey { ... }

 impl crate::raw::DecryptionPrimitive for RSAPrivateKey { ... }

 impl PublicKeyParts for RSAPrivateKey { ... }

 impl PrivateKey for RSAPrivateKey {
 pub fn sign_pkcs1v15(&self) -> crate::pkcs1v15::Signer {
 crate::pkcs1v15::Signer::unblinded(self)
 }

 pub fn sign_pkcs1v15_blinded<R: Rng>(&self, rng: R) -> crate::pkcs1v15::Signer {
 crate::pkcs1v15::Signer::blinded(rng, self)
 }

 pub fn sign_pss(&self) -> crate::pss::Signer {
 crate::pss::Signer::unblinded(self)
 }

 pub fn sign_pss_blinded(&self) -> crate::pss::Signer {
 crate::pss::Signer::blinded(self)
 }
 }

 pub struct RSAPublicKey { ... }

 impl crate::raw::EncryptionPrimitive for RSAPublicKey { ... }

 impl PublicKeyParts for RSAPublicKey { ... }

 impl PublicKey for RSAPublicKey {
 pub fn verify_pkcs1v15(&self) -> crate::pkcs1v15::Verifier {
 crate::pkcs1v15::Verifier::new(self)
 }

 pub fn verify_pss(&self) -> crate::pss::Verifier {
 crate::pss::Verifier::new(self)
 }
 }
}

/// PKCS#1 v1.5 signing (and encryption?)
mod pkcs1v15 {
 use signature::Error;

 use crate::{
 key::{PrivateKey, PublicKey},
 raw::DecryptionPrimitive,
 };

 pub struct Signature {
 bytes: Vec<u8>,
 }

 impl signature::Signature for Signature {
 fn from_bytes(bytes: impl AsRef<[u8]>) -> Result<Self, Error> {
 // Parse a PKCS#1 v1.5 signature here non-contextually
 // (i.e. length can't be verified as we don't know n)
 }
 }

 // Technically all we need is K: DecryptionPrimitive, but PrivateKey
 // is the correct user-level encapsulation, and we should keep
 // DecryptionPrimitive internal as much as possible.
 pub struct Signer<'a, R: Rng, K: PrivateKey> {
 // RefCell in lieu of a stateful or randomizable signature trait
 rng: Option<RefCell<R>>,
 priv_key: &'a K,
 }

 impl<'a, R: Rng, K: PrivateKey> Signer<'a, R, K> {
 pub fn unblinded(priv_key: &'a K) -> Self {
 Signer { rng: None, priv_key }
 }

 pub fn blinded(rng: R, priv_key: &'a K) -> Self {
 Signer { rng: Some(RefCell::new(rng)), priv_key }
 }
 }

 impl<'a, R: Rng, K: PrivateKey> signature::Signer<Signature> for Signer<'a, R, K> {
 fn try_sign(&self, msg: &[u8]) -> Result<Signature, Error> {
 // Sign the message directly (equivalent to current None case)
 }
 }

 impl<'a, R: Rng, K: PrivateKey> signature::DigestSigner<D: Digest, Signature> for Signer<'a, R, K> {
 fn try_sign_digest(&self, digest: D) -> Result<Signature, Error> {
 // Sign the digest (equivalent to current Some(Hash) case)
 }
 }

 pub struct Verifier<'a, PK: PublicKey> {
 pub_key: &'a PK,
 }

 impl<'a, PK: PublicKey> Verifier<'a, PK> {
 pub fn new(pub_key: &'a PK) -> Self {
 Verifier { pub_key }
 }
 }

 impl<'a, PK: PublicKey> signature::Verifier<Signature> for Verifier<'a, PK> {
 fn verify(&self, msg: &[u8], signature: &Signature) -> Result<(), Error> {
 // Verify the message directly (equivalent to current None case)
 }
 }

 impl<'a, PK: PublicKey> signature::DigestVerifier<D: Digest, Signature> for Verifier<'a, PK> {
 fn verify_digest(&self, digest: D, signature: &Signature) -> Result<(), Error> {
 // Verify the digest (equivalent to current Some(Hash) case)
 }
 }
}

/// PSS signing
mod pss {
 use signature::Error;

 use crate::{
 key::{PrivateKey, PublicKey},
 raw::DecryptionPrimitive,
 };

 pub struct Signature {
 bytes: Vec<u8>,
 }

 impl signature::Signature for Signature {
 fn from_bytes(bytes: impl AsRef<[u8]>) -> Result<Self, Error> {
 // Parse a PSS signature here non-contextually
 // (i.e. length can't be verified as we don't know n)
 }
 }

 pub struct Signer<'a, R: Rng, K: PrivateKey> {
 // RefCell in lieu of a stateful or randomizable signature trait
 rng: RefCell<R>,
 priv_key: &'a K,
 salt_len: Option<usize>,
 blind: bool
 }

 impl<'a, R: Rng, K: PrivateKey> Signer<'a, R, K> {
 pub fn unblinded(rng: R, priv_key: &'a K, salt_len: Option<usize>) -> Self {
 Signer { rng: RefCell::new(rng), priv_key, salt_len, blind: false }
 }

 pub fn blinded(rng: R, priv_key: &'a K, salt_len: Option<usize>) -> Self {
 Signer { rng: RefCell::new(rng), priv_key, salt_len, blind: true }
 }
 }

 impl<'a, R: Rng, K: PrivateKey> signature::DigestSigner<D: Digest, Signature> for Signer<'a, R, K> {
 fn try_sign_digest(&self, digest: D) -> Result<Signature, Error> { ... }
 }

 pub struct Verifier<'a, PK: PublicKey> {
 pub_key: &'a PK,
 }

 impl<'a, PK: PublicKey> Verifier<'a, PK> {
 pub fn new(pub_key: &'a PK) -> Self {
 Verifier { pub_key }
 }
 }

 impl<'a, PK: PublicKey> signature::DigestVerifier<D: Digest, Signature> for Verifier<'a, PK> {
 fn verify_digest(&self, digest: D, signature: &Signature) -> Result<(), Error> { ... }
 }
}

// Do these improve usability?
pub use pkcs1v15::{
 Signature as Pkcs1v15Signature,
 Signer as Pkcs1v15Signer,
 Verifier as Pkcs1v15Verifier,
};
pub use pss::{
 Signature as PssSignature,
 Signer as PssSigner,
 Verifier as PssVerifier,
};

I'll update the proposal in this post as we discuss it.

[str4d]

Just to be clear, the above is intended as something concrete we can stare at and work on, not as the final proposal (though I think the above is a reasonable design).

[tarcieri]

This looks good at first glance, and is more or less how the ecdsa crate works today.

Something else to consider is incorporating more of the parameters for how the signature is computed in the signature type itself, e.g. making pkcs1v15::Signature and pss::Signature generic around a digest applied to the message (and in the case of PSS, making it generic around an MGF type as well).

I'd probably recommend avoiding that initially and circling back on it later.

[str4d]

Updated draft to use PrivateKey, PublicKey traits inside Signer / Verifier structs, so they can be generalised over the backend (e.g. HSMs) a la #32.

[dignifiedquire]

The api looks pretty good to me. The main question for me is how do we integrate this with the encryption part, as that is kinda needed for getting it integrated into the crate

[str4d]

My suggestion (which I'll write up next) is to have equivalent Encryptor / Decryptor objects. We could maybe treat these as block ciphers and use the block-cipher-trait traits, but it feels like not the right fit to me, so probably just equivalent methods (and maybe traits within the rsa crate). Then the full suite would be:

crate::{
 oaep::{Decryptor, Encryptor},
 pkcs1v15::{Decryptor, Encryptor, Signer, Verifier},
 pss::{Signer, Verifier},
};

[dignifiedquire]

I worry a bit about this api becoming quite complex, having to create multiple structs for each operation seems like quite a lot of mental overhead for the user of the api, when all these operations could just be straightforwad function calls like they are now.

[dignifiedquire]

Not saying the current API is perfect, not even great, but I am a big advocate of making cryptographic primitives easier to use, not harder.

[str4d]

The struct-based API would be used something like this:

use rsa::{pss::Signer, RsaPrivateKey};
use sha2::{Digest, Sha256};
use signature::DigestSigner;

let privkey = RsaPrivateKey::from_wherever();
let signer = Signer::unblinded(&privkey);

let digest = Sha256::digest(msg);
let signature = signer.sign_digest(digest);

This is where the APIs on PublicKey and PrivateKey could perhaps still be useful for usability:

use rsa::RsaPrivateKey;
use sha2::{Digest, Sha256};
use signature::DigestSigner;

let privkey = RsaPrivateKey::from_wherever();
let signer = privkey.sign_pss();

let digest = Sha256::digest(msg);
let signature = signer.sign_digest(digest);

This only saves an import, as the separate privkey object is still necessary for lifetimes. Alternatively, the Signer and Verifier structs could move the PublicKey / PrivateKey internally, in which case you could do:

use rsa::key::RsaPrivateKey;
use sha2::{Digest, Sha256};
use signature::DigestSigner;

let signer = RsaPrivateKey::from_wherever().sign_pss();

let digest = Sha256::digest(msg);
let signature = signer.sign_digest(digest);

The trade-off here is that this isn't as usable for HSM backends (where you probably only have a single reference to the connection that you are passing around) or for use-cases where the same key is used for both encryption and signing in the same logic (does this happen?)

[str4d]

Writing the above three more compactly (for more options to stare at):

use rsa::{pss, RsaPrivateKey};
use sha2::{Digest, Sha256};
use signature::DigestSigner;

let privkey = RsaPrivateKey::from_wherever();
let digest = Sha256::digest(msg);
let signature = pss::Signer::unblinded(&privkey).sign_digest(digest);

use rsa::RsaPrivateKey;
use signature::DigestSigner;
use sha2::{Digest, Sha256};

let privkey = RsaPrivateKey::from_wherever();
let digest = Sha256::digest(msg);
let signature = privkey.sign_pss().sign_digest(digest);

use rsa::RsaPrivateKey;
use signature::DigestSigner;
use sha2::{Digest, Sha256};

let signer = RsaPrivateKey::from_wherever().sign_pss();
let digest = Sha256::digest(msg);
let signature = signer.sign_digest(digest);

[tarcieri]

Note that you can use the signature_derive crate to derive a Signer and Verifier impl for types which impl DigestSigner and DigestVerifier. Here's an example:

https://github.com/tendermint/signatory/blob/develop/signatory-secp256k1/src/lib.rs#L25
https://github.com/tendermint/signatory/blob/develop/signatory-secp256k1/src/lib.rs#L80

That would reduce this:

use signature::DigestSigner;

let signer = PrivateKey::from_wherever().sign_pss();

let digest = Sha256::digest(msg);
let signature = signer.sign_digest(digest);

to:

use signature::Signer;

let signer = PrivateKey::from_wherever().sign_pss();
let signature = signer.sign(msg);

[str4d]

And the current API for comparison:

use rsa::{
 hash::Hashes,
 PaddingScheme, RSAPrivateKey,
};
use sha2::{Digest, Sha256};

let privkey = RsaPrivateKey::from_wherever();
let digest = Sha256::digest(msg);
let signature = privkey.sign(Padding::PKCS1v15, Some(Hashes::SHA2_256), digest.as_bytes());

[str4d]

Note that you can use the signature_derive crate to derive a Signer and Verifier impl for types which impl DigestSigner and DigestVerifier.

I considered adding that to my example, but I wondered whether that might result in confusing behaviour, given that for PKCS#1 v1.5 we'd be using the Signer interface for signing undigested input. Or should instead have the Signer interface accept any length message (rather than within the RSA modulus size) and have an RSA-specific impl Digest for NoneDigest solely for use in the signer API? Which is more in keeping with the signature API?

[dignifiedquire]

where the same key is used for both encryption and signing in the same logic (does this happen?)

it does happen sometimes for RSA in my experience

[tarcieri]

I considered adding that to my example, but I wondered whether that might result in confusing behaviour, given that for PKCS#1 v1.5 we'd be using the Signer interface for signing undigested input.

Ugh yes RSASSA-PKCS#1v1.5, it is definitely a wart there, but I'd also argue it doesn't quite fit the (perhaps underdocumented) DigestSigner contract, which is intended for something more like a Fiat-Shamir scheme.

where the same key is used for both encryption and signing in the same logic (does this happen?)

It absolutely happens in pre-1.3 TLS when the same RSA key is permitted to be used for both key encipherment and signing, and both (EC)DHE and RSA key transport ciphersuites are enabled.

[dignifiedquire]

Maybe we could provide wrapper methods, that hide the struct generation?

use signature::DigestSigner;
let digest = Sha256::digest(msg);
let signature = PrivateKey::from_wherever().sign_pss(digest);


fn sign_pss(&self, digest) -> Signature {
 let signer = self.create_pss_signer(); // this is what is currently named sign_pss
 signer.sign(digest)
}