rust-random / rand

A Rust library for random number generation.
https://crates.io/crates/rand
Other
1.67k stars 431 forks source link

Non-panicking version of `SeedableRng::from_entropy`? #1344

Closed andoalon closed 1 year ago

andoalon commented 1 year ago

Background

What is your motivation? Seeding from entropy is convenient but might not be required for non-security uses of RNGs (e.g. videogames). An application with such a use-case might want to prioritize avoiding a panic (even if it's rare) over getting a secure, unpredictable seed. There might be a secondary, potentially worse quality but still acceptable seed source for these applications (or even falling back to a hardcoded seed might be acceptable in some cases). Catching the panic would work, but only if unwinding panics are being used (which is not a given).

What type of application is this? (E.g. cryptography, game, numerical simulation) Game.

Feature request

Offer a non-panicking version of SeedableRng::from_entropy (maybe SeedableRng::try_from_entropy) that returns an Option or Result.

The crate used to implement this(getrandom) already provides the necessary functionality. The question would be, if returning a Result is preferred (over Option), whether to expose the error type from getrandom or what kind of error information we would like to expose.

In my project I made a version of this, using getrandom directly, that looks like this:

pub fn try_from_entropy<R: rand::SeedableRng>() -> Result<R, getrandom::Error> {
    let mut seed = R::Seed::default();
    getrandom::getrandom(seed.as_mut()).map(|()| R::from_seed(seed))
}
dhardy commented 1 year ago

We have been replacing several panicking APIs with Result variants; possibly we should do the same here. TODO: investigate potential breakage caused.

In case of failure there are still plenty of options for generating insecure seeds, though I don't know how necessary they are.

Question: do you have a need to handle failure from getrandom or is this simply a theoretical issue? Because failure in normal environments (a full OS outside of early-boot conditions) is very unlikely.

andoalon commented 1 year ago

We have been replacing several panicking APIs with Result variants; possibly we should do the same here. TODO: investigate potential breakage caused.

Good to know!

Question: do you have a need to handle failure from getrandom or is this simply a theoretical issue? Because failure in normal environments (a full OS outside of early-boot conditions) is very unlikely.

I don't need it. I'm aware that this failing is a very rare occurrence that I'll probably never encounter myself. It's totally a theoretical issue, I'm being extra defensive here.

In case of failure there are still plenty of options for generating insecure seeds, though I don't know how necessary they are.

Fair point. Well, that's why I would probably suggest leaving the original function the way it is, and adding the second try_* version for the paranoid (or the ones who really need it). Kind of like
indexing vs get() on slices, or
Query::single() vs Query::get_single() in bevy

dhardy commented 1 year ago

there are still plenty of options for generating insecure seeds

To further this point, we don't really provide any such facilities. Theoretically these could span from very simple things like memory-address or date/time to the (abandoned) JitterRng to hardware RNG modules to examining things like packet timing and user input. But almost no software beyond low-level OS code and maybe the occasional application paired with a hardware token can justify needing to use any of these, and as such they should remain beyond the scope of the Rand project.

My advice: don't worry about it unless you know you need to. And in that case you can simply use your own version of from_entropy (which may not involve getrandom at all).

As such I'm tempted to close this with no change, despite using Result in various other places. @newpavlov?

andoalon commented 1 year ago

Thank you for the advice. Just to clarify: I don't expect rand to provide other sources of randomness. It's just that having the option to handle the rare case where from_entropy fails would be nice. Feel free to close the PR. Thank you again