Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing functions requiring CryptoRng #1319

Closed
ignassew opened this issue Jun 17, 2023 · 8 comments
Closed

Testing functions requiring CryptoRng #1319

ignassew opened this issue Jun 17, 2023 · 8 comments

Comments

@ignassew
Copy link

I want to test encrypting some value with rsa public key using the RSA crate., but it only accepts CryptoRng for the rng: RSA/keys.rs#L141

StepRng doesn't implement CryptoRng. I could go around this by creating my own mocking rng, but it doesn't fix the core problem.

I think StepRng should support CryptoRng. It's obviously not a cryptographically secure rng, but it being in the mock module should be enough of a warning to not use it in actual code.

If this issue gets accepted, I can make a PR for it.

What are your thoughts?

@newpavlov
Copy link
Member

newpavlov commented Jun 17, 2023

You should use CSPRNG like rand_chacha::ChaCha8Rng seeded with a fixed seed:

use rand_chacha::{ChaCha8Rng, rand_core::SeedableRng};

let mut rng = ChaCha8Rng::seed_from_u64(42);

@ignassew
Copy link
Author

My test involves comparing encrypted output from other language (python) which doesn't implement ChaCha in their standard library.

I want to compare the output of these 2 functions:

import base64

from Crypto.Cipher import PKCS1_v1_5
from Crypto.PublicKey import RSA

rsa_public_key = "MIGfMA0GC...vwIDAQAB"
rsa_public_key = RSA.import_key(base64.b64decode(rsa_public_key))


def grb(n: int) -> bytes:
    return b"\x01"


cipher = PKCS1_v1_5.new(rsa_public_key, randfunc=grb)
x = cipher.encrypt("x".encode())

print(base64.b64encode(x))

This will return: b'cRU5r4Z...XKIUwA4wE='

Also the rust implementation:

use rand::{prelude::*, rngs::mock::StepRng};
use rsa::{
    pkcs8::DecodePublicKey, PaddingScheme, PublicKey, RsaPublicKey,
};

struct CryptoStepRng {
    inner: StepRng,
}

impl CryptoStepRng {
    pub fn new(initial: u64, increment: u64) -> Self {
        CryptoStepRng {
            inner: StepRng::new(initial, increment),
        }
    }
}

impl RngCore for CryptoStepRng {
    fn next_u32(&mut self) -> u32 {
        self.inner.next_u32()
    }

    fn next_u64(&mut self) -> u64 {
        self.inner.next_u64()
    }

    fn fill_bytes(&mut self, dest: &mut [u8]) {
        self.inner.fill_bytes(dest)
    }

    fn try_fill_bytes(
        &mut self,
        dest: &mut [u8],
    ) -> Result<(), rand::Error> {
        self.inner.try_fill_bytes(dest)
    }
}

impl CryptoRng for CryptoStepRng {}

fn main() {
    let mut mrng = CryptoStepRng::new(1, 0);

    let rsa_public_key_string = "MIGfMA0GC...vwIDAQAB";
    let rsa_public_key_bytes =
        base64::decode(rsa_public_key_string).unwrap();
    let rsa_public_key = RsaPublicKey::from_public_key_der(
        &rsa_public_key_bytes.as_slice(),
    )
    .unwrap();

    let aes_key_encrypted = rsa_public_key
        .encrypt(&mut mrng, PaddingScheme::new_pkcs1v15_encrypt(), b"x")
        .unwrap();
    let aes_key_encrypted = base64::encode(aes_key_encrypted);
    println!("{}", &aes_key_encrypted);
}

As expected, also returns: b'cRU5r4Z...XKIUwA4wE='

This is one of the example where it is useful for StepRng to have CryptoRng implemented. Trying to figure out how to generate ChaCha rng in python would be a lot more complicated.

@vks
Copy link
Collaborator

vks commented Jun 18, 2023

If you really want to, you can always implement you own RNG type by wrapping StepRng and implement CryptoRng for it.

@ignassew
Copy link
Author

ignassew commented Jun 18, 2023

That's exactly what I'm doing in the provided example, but I think StepRng should support that outside of the box, and I don't see any reason why it doesn't.
It's a mock ring, and I think it's usefulness could be greatly improved by adding CryptoRng support.

@newpavlov
Copy link
Member

newpavlov commented Jun 18, 2023

StepRng is a very bad PRNG. In some cases it may even make test code unusable, e.g. imagine key generation which needs the most significant bit set to 1. Using it to test cryptographic code is certainly a bad practice, so I don't think we should make it easier. In your case it would be better to use something like ReadRng, which would use a fixed random string instead of an input file.

I am inclined to close this issue as "won't fix".

@ignassew ignassew closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2023
@Fethbita
Copy link

Fethbita commented Oct 4, 2024

Hi! I know this is a closed issue but I would like to bring attention to this just for a short while. I propose adding some examples in the documentation or the book to test functions that use rand crate, in my particular example OsRng.fill_bytes. I would like to mock this in my unit tests so that it returns hardcoded values because I want to test that the function works as expected with the test vectors provided by standards bodies such as NIST or ICAO.

I believe this is quite a common case where functions that use random generators need to be unit tested and documenting a process would be very helpful.

@newpavlov
Copy link
Member

@Fethbita
Please open a separate issue or (even better) PR for this.

I think that mocking OsRng is a bad approach. Any kind of mocking will devolve into a mess because of its globality (how can you mock it in the presence of several tests which require their own values while they are executed in parallel?). Instead you should implement your tested algorithm generically over RngCore/CryptoRng with OsRng being the default. This way you will be able to use pseudo-RNG with hardcoded stream in tests, while users will work with properly generated entropy.

@Fethbita
Copy link

Fethbita commented Oct 4, 2024

Thanks for the tip @newpavlov. As recommended, I went with a PR: rust-random/book#64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants