Provide access to the system's random number generator

[joboet]

Proposal

(an alternative to #393)

Motivation

Nearly all targets that std supports (with the exception of WASM) provide access to a cryptographically secure random number generator. Although this functionality is more widespread than even filesystem access, std does not currently have an abstraction for it, leading users to rely on an external crate like getrandom. While that crate is of high quality, this still leads to the duplication of functionality that std already implements, as HashMap is initialized with seeds generated from the system's RNG by default. Also, this complicates the addition of new targets to the ecosystem, as more libraries need to be changed to support this very common use case.

Unfortunately, the current lack of a common abstraction means there is a widespread pattern of users using File::open("/dev/urandom") for easy access to random data (GitHub search). This is not very portable and can be problematic, as "/dev/urandom" returns low-quality randomness on Linux if the entropy pool is not yet initialized (this is unlikely to be a problem for most usecases, but especially on embedded devices, security will suffer).

Solution sketches

A Readable handle type similar to the Stdin/Stdout handles is added to std::io along with its matching constructor:

// in std::io

pub struct Entropy { .. }

impl Read for Entropy { .. }

pub fn entropy() -> Entropy;

Random bytes are generated by using the Read trait's methods:

use std::io::entropy;

let mut bytes = [0; 128];
entropy().read_exact(&mut bytes)?;

While this represents a departure from getrandom's API, using the Read trait is more idiomatic, enables better interplay with other io functions such as copy and allows for optimizations like using an uninitialized buffer with read_buf. Additionally, it enables the configuration of behaviour like blocking until there is enough entropy available¹ through methods on the struct, mirroring API like TcpStream::set_nonblocking.

Links and related work

• @matklad advocating for RNG access in std
• Use getrandom for generating HashMap seed rust#80149 will remove std's random number generation in favour of getrandom

Footnotes

1. which should IMHO be the default, but users may want to depart from this behaviour ↩

[thomcc]

I'm not sure about this. Given that we'll never be a better choice for user code than using getrandom (we cannot offer wasm support except for on WASI due to the fact that std cannot depend on wasm-bindgen or assume it is being used on wasm32-unknown-unknown), I don't know if it's worth adding this — users will end up worse-off for using it than if they reached for crates.io.

(I also have some concerns about providing this via a io::Read implementation, but I'll let others discuss this)

[CryZe]

we cannot offer wasm support except for on WASI [...] users will end up worse-off for using it than if they reached for crates.io.

This is already true for most of std. wasm32-unknown-unknown should've never supported std in the first place, people for example already have to reach out for the instant crate if they want to use std::time. So this isn't any worse than the rest of std.

[thomcc]

It's worse than the status quo though.

[CryZe]

That is true for any addition to std (that can't live in alloc / core) though, as they all can't work on wasm32-unknown-unknown. So unless you are saying std should never add any new API again, I'm still not convinced by that argument.

[joboet]

I'm not sure about this. Given that we'll never be a better choice for user code than using getrandom (we cannot offer wasm support except for on WASI due to the fact that std cannot depend on wasm-bindgen or assume it is being used on wasm32-unknown-unknown), I don't know if it's worth adding this — users will end up worse-off for using it than if they reached for crates.io.

I understand what you mean, that issue also applies to the #![no_std] usage of getrandom, which won't be replaced by this. Adding this API is a compromise: it makes programming on most platforms easier and a little bit more secure, but using the API means code is (in one case) slightly less portable. It's actually more portable in other cases however, because it's easier to support new targets in std than in getrandom (one less crate to change).

Sidenote: Sometimes I wonder whether it would be worth it to add a wasm32-js target that specifically targets the web, but of course, there is no standard API it could use...

[thomcc]

An additional issue is the use of the Read trait. Concretely, it supports incomplete reads if read_exact is not used, which is already a common source of bugs in Rust programs. This would lead to those bugs being security bugs, which isn't the case with a differently shaped API.

CC @BlackHoleFox, who I know has been trying to come up with a design for std randomness off and on for ages.

[joboet]

An additional issue is the use of the Read trait. Concretely, it supports incomplete reads if read_exact is not used, which is already a common source of bugs in Rust programs. This would lead to those bugs being security bugs, which isn't the case with a differently shaped API.

CC @BlackHoleFox, who I know has been trying to come up with a design for std randomness off and on for ages.

Yes, that could be an issue. We can however always change io::Entropy to fill the whole buffer in read, which prevents these kinds of bugs. I think it not doing that is more in the spirit of the Read trait (see rust-lang/rust#107200, edit: that's about the Write trait, but I hope you get my point), but security is, as you point out, quite a good argument against that here.

Using the Read trait is IMHO the most idiomatic approach here and makes uglyinconvenient API like getrandom_uninit redundant.

[clarfonthey]

Don't want to further block this ACP from getting merged, but think it's worth elaborating here how this point is meant to be addressed:

behaviour like blocking until there is enough entropy available1 through methods on the struct, mirroring API like TcpStream::set_nonblocking.

Since currently, the attached PR has an insecure flag in the struct but only exposes changing this to the crate and not publicly, and seems to explicitly set the insecure flag when initialising HashMap seeds.

[joboet]

We definitely do need the insecure flag for internal usage, but I don't think it should be part of a MVP public API which I tried to propose here. Adding it in the future seems very reasonable though.

[the8472]

Note there's #393 which has been accepted.

[joboet]

Ah yes, that makes this ACP unnecessary.

[joboet]

On second thought, I have some issues with the accepted ACP. I'll reopen this so that I have an alternative to link to.

[joboet]

Nevermind, I'm happy with how everything turned out.