Conventions for RNG APIs with getrandom feature

[tarcieri]

In many places we document how to import and pass OsRng, often providing a getrandom feature that re-exports rand_core.

Instead of that, we can add #[cfg(feature = "getrandom")]-gated functions which pass OsRng for you.

For example:

#[cfg(feature = "getrandom")]
use rand_core::OsRng;

impl PrivateKey {
    pub fn generate(rng: &mut impl CryptoRngCore) -> Self { ... }

    #[cfg(feature = "getrandom")]
    pub fn generate_from_os_rng() -> Self {
        Self::generate(&mut OsRng)
    }
}

If we go this route, some questions:

1. What should the convention for the CryptoRng(Core)-parameterized methods be? We use both generate and random.
2. What should the convention be for the getrandom-gated wrapper which passes OsRng for you? Should we make that shorter to be more convenient?
3. Should we enable getrandom by default for even more convenience?

[burdges]

It's mildly off topic here, but getrandom_or_panic often handles these concerns higher up fairly well.

[newpavlov]

We also should decide whether we want to expose potential errors in our API. If yes, then we also should introduce something like fn try_generate_from_os_rng() -> Result<Self, getrandom::Error> { .. }. But we probably need a better naming scheme...

Should we enable getrandom by default for even more convenience?

It should be fine in implementation crates, but I don't think we should do it in the trait crates. We should be careful with accidentally making it impossible to disable.

[tarcieri]

We also should decide whether we want to expose potential errors in our API.

@newpavlov is that going to change upstream in rand_core? e.g. in the RngCore trait.

The idea proposed in this issue is to keep the method parameterized on CryptoRng(Core) so embedded users and other users on platforms that getrandom doesn't support can continue to do so, but plugging in OsRng in a separate wrapper method when available.

If OsRng/RngCore ends up returning an error, we should too, but if not I think we should do what the trait does and have the function be infallible.

[newpavlov]

@tarcieri
The RngCore trait supports returning error since introduction of the rand_core crate. See the try_fill_bytes method. We introduced it specifically for cryptographic applications.

The idea proposed in this issue is to keep the method parameterized on CryptoRng(Core) so embedded users and other users on platforms that getrandom doesn't support can continue to do so

Arguably, embedded users should use the custom backend capability in getrandom.

As I see it, we probably should support generic RNGs, but mostly for performance reasons. User-space RNGs (such as rand's ThreadRng) can be much faster than OS entropy source, without significant sacrifice of security properties.

[tarcieri]

Oh interesting re: try_fill_bytes. I wasn't aware that existed.

So I guess we should switch to fallible RNGs when making breaking changes.

Arguably, embedded users should use the custom backend capability in getrandom

getrandom doesn't even seem to compile on e.g. thumbv7 targets:

   Compiling getrandom v0.2.9
error: target is not supported, for more information see: https://docs.rs/getrandom/#unsupported-targets
   --> /Users/tarcieri/.cargo/registry/src/index.crates.io-6f17d22bba15001f/getrandom-0.2.9/src/lib.rs:286:9
    |
286 | /         compile_error!("target is not supported, for more information see: \
287 | |                         https://docs.rs/getrandom/#unsupported-targets");
    | |________________________________________________________________________^

[tarcieri]

Most crates are now using &mut impl CryptoRngCore.

In the next breaking releases we can migrate to the fallible API.

I guess there's still an open question of conventions for an explicit RNG versus implicit OsRng. My preference there would be to change the methods that accept an explicit RNG to *_with_rng, e.g. generate would use OsRng and be getrandom gated, whereas generate_with_rng would accept an RNG parameter.