RSA icon indicating copy to clipboard operation
RSA copied to clipboard

Please Implement Clone for PaddingScheme

Open littleTitan opened this issue 4 years ago • 1 comments

Please implement the recommended traits for the enum PaddingScheme.

Clone is missing from PaddingScheme. Please fix this.

While copy may not be possible due to problems when copying RngCore--namely that an accidental copy may generate the same numbers--, clone is an important implementation which is advised by the creators of rand. With the added implementation of box-clone on Box<dyn DynDigest> and on Box<dyn RngCore>, written out in full bellow, it should be possible to derive Clone on PaddingScheme.

for DynDigest:

pub trait DynDigest {
    ...
}

pub trait DynDigestBoxClone {
    fn clone_dyn_digest(&self) -> Box<dyn DynDigest>;
}

impl<T> DynDigestBoxClone for T where T: 'static + DynDigest + Clone {
    fn clone_dyn_digest(&self) -> Box<dyn DynDigest> {
        Box::new(self.clone())
    }
}

impl Clone for Box<dyn DynDigest> {
    fn clone(&self) -> Self {
        self.clone_dyn_digest()
    }
}

and the same for Box<dyn RngCore>:

use rand::RngCore;

pub trait RngCoreClone: RngCore {
    fn clone_rng_core(&self) -> Box<dyn RngCoreClone>;
}

impl<T: 'static + RngCore + Clone> RngCoreClone for T {
    fn clone_rng_core(&self) -> Box<dyn RngCoreClone> {
        Box::new(self.clone())
    }
}

impl Clone for Box<dyn RngCoreClone> {
    fn clone(&self) -> Box<dyn RngCoreClone> {
        self.clone_rng_core()
    }
}

and switch to using Box<dyn RngCoreClone> rather than Box<dyn RngCore>.

Thanks.

Best regards, littleTitan

littleTitan avatar Dec 30 '21 01:12 littleTitan

namely that an accidental copy may generate the same numbers--, clone is an important implementation which is advised by the creators of rand

Cloning the RNG feels like a huge footgun here. I'm pretty sure generating the same salt twice in PSS will cripple its security, which this proposal would make way too easy to do accidentally.

FWIW, this is an enum, so you can easily write your own "clone" method from user code, e.g.

fn clone_with_thread_rng(scheme: &PaddingScheme) {
    match scheme {
        PaddingScheme::PKCS1v15Encrypt => PaddingScheme::PKCS1v15Encrypt,
        PaddingScheme::PKCS1v15Sign { hash } => PaddingScheme::PKCS1v15Sign { hash },
        PaddingScheme::OAEP { digest, mgf_digest, label } => PaddingScheme::OAEP { digest: digest.box_clone(), mgf_digest: mgf_digest.box_clone(), label: label.clone() },
        PaddingScheme::PSS { _salt_rng, digest, salt_len } => PaddingScheme::PSS { salt_rng: Box::new(rand::thread_rng()), digest: digest.box_clone(), salt_len },
    }
}

This will create a new instance of PaddingScheme with only salt_rng changed to a new instance of thread_rng(), which may be enough for your use-cases.

roblabla avatar Jan 10 '22 16:01 roblabla

After #244, PaddingScheme is now a trait, so this is no longer relevant

tarcieri avatar Apr 18 '23 20:04 tarcieri