RustCrypto / RSA

RSA implementation in pure Rust
Apache License 2.0
536 stars 148 forks source link

Can not pass `padding` as an argument anymore #260

Closed avalon1610 closed 1 year ago

avalon1610 commented 1 year ago

It's very inconvenient after this PR #244 when I want to pass some padding through argument. I tried

fn foo(padding: &dyn PaddingScheme) {}  // error, PaddingScheme is not object safe
fn foo<P: PaddingScheme>(padding: P) {} // I need get padding method in runtime, not suitable

enum MyPaddingScheme {
    Oaep(Oaep),
    Pkcs1(Pkcs1v15Encrypt)
}

impl rsa::PaddingScheme for MyPaddingScheme {
    fn decrypt(Rng: CryptoRngCore, Priv: PrivateKey>(    // error, trait PrivateKey is private
        self,
        rng: Option<&mut Rng>,
        priv_key: &Priv,
        ciphertext: &[u8]
    ) -> rsa::errors::Result<Vec<u8>> {
        todo!()
    }
}

Any workaround will be appreciated

tarcieri commented 1 year ago

The PrivateKey trait being private seems like an oversight /cc @dignifiedquire @str4d

tarcieri commented 1 year ago

@avalon1610 as far as how to structure your code, I'd back even farther up to where you might prospectively even construct your hypothetical MyPaddingScheme enum...

Why can't you just call decrypt after it's constructed? Why are you passing it around like a value?

If you really want to do something like that, how about:

enum MyPaddingScheme {
    Oaep(Oaep),
    Pkcs1v15(Pkcs1v15Encrypt)
}

impl MyPaddingScheme {
    fn decrypt(Rng: CryptoRngCore>(self, rng: Option<&mut Rng>, priv_key: &Priv, ciphertext: &[u8]) -> rsa::errors::Result<Vec<u8>> {
        match self {
            Self::Oaep(oaep) => priv_key.decrypt(oaep, rng, priv_key, ciphertext),
            Self::Pkcs1v15(pkcs) => priv_key.decrypt(pkcs, rng, priv_key, ciphertext),
        }
    }
}

...but that's just adding indirection using an enum, rather than just calling the appropriate decrypt API after constructing the PaddingScheme.

tarcieri commented 1 year ago

I opened #261 to discuss potentially trying to make PaddingScheme and SignatureScheme object safe, as I agree that would be helpful if it were possible.

avalon1610 commented 1 year ago

Thanks for your suggestion, I'll try to refactor my code.

Why can't you just call decrypt after it's constructed? Why are you passing it around like a value?

Because that will make a lot of duplicate code. like

fn decrypt(padding: &str) {
    match padding {
        "OAEP" => {
            let p = Oaep::new::<Sha256>();
            if let Err(e) = p.decrypt(/* use key1 */) {
                if let Err(e) = p.decrypt(/* user key 2 */) {
                    // more retry using different keys
                }
            }
        }
        "Pkcs1v15" => {
            // almost same code as previous match path
        }
    }
}

Surely I can use macro, but using macro make code not that clear

tarcieri commented 1 year ago

If all you're doing is calling decrypt, you should be able to write the body as function which is generic over the padding mode

avalon1610 commented 1 year ago

Yeah, you are right, I forgot I could do this. Thanks