Remove OpenBSD getentropy(2) support for now

[mmcco]

It turns out that Rust's CSPRNG isn't used in as many places as I
thought. We can't treat getentropy(2) quite like Linux's getrandom(2),
and running an entire C-implemented CSPRNG (arc4random) underneath Rust
probably isn't an acceptable option, so let's be safe and back out until
we're more confident.

[semarie]

I am in (private) discussion with @mmcco about that.

First, some background:

• getentropy(2) (http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2) is a very low-level interface.
• For getting crypto-random, the intented functions are arc4random family (http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man3/arc4random.3). This function is provided by default libc under OpenBSD.

The direct use of getentropy at this place in rustc isn't really opportune. It could have more valuable use in rand crate (the external one).

But I think just removing this code isn't the good solution. For me, the short term should be replacing the getentropy call with arc4random_buf function.

@alexcrichton do you have some opinion about that ? or could you cc someone in rust-devs that could help us here ?

@mmcco said also that using arc4random family could not be acceptable solution. I think it is because of last PR that remove C stuff in rustc (but the problem was different, as it was about removing C source file). Here arc4random functions are provided by libc. Does it have some problem to directly use it ?

Thanks for your comments.

[semarie]

for reference the PR that introduce getentropy is #30430.

[mmcco]

@semarie I'm concerned about how much runtime logic and state arc4random includes. I'll look into it more this afternoon. There's an active conversation on internals.rust-lang.org about how much C we want to pull in from the standard library. Most people seem to think that, at least in the short-term, it's necessary to use things like syscall wrappers. However, arc4random is a full CSPRNG, which means that it has far more logic than a wrapper and will store state in the Rust process's address space. I think it's ideal to avoid this if at all possible, especially considering that Rust is perfectly well-suited for implementing a CSPRNG. The Isaac RNG currently in the code base describes itself as being crypto-strength but not yet audited.

IIUC, Rust should transition to using a userspace CSPRNG for performance and reliability regardless of which randomness sources the OS offers. I suspect that it may be easier to work on this rather than adding arc4random support and removing it once Rust has a working crypto-ready CSPRNG.

[mmcco]

The more I think about it, though, adding arc4random support for now seems like a good approach. I just think that a Rust-native CSPRNG is the ideal long-term solution.

[mmcco]

It looks like @semarie and I had some needless paranoia. I submitted the same change to Golang, and we are also discussing the issue of userland CSPRNGs. What I failed to notice is that the reviewer is Matthew Dempsky, the co-creator of getentropy. :-) He assured me that it's safe, so I think we're fine leaving getentropy support in Rust.

I'll ask Matthew for confirmation first, but I plan on closing this patch.

I will likely try to work on Rust's CSPRNG so that we can use it by default for the sake of performance and stability.

[mdempsky]

I'm not at all familiar with Rust, but I'll offer this input:

• Using getentropy directly is no different than reading from /dev/urandom, except that the latter allows reads of >256 bytes too. (It also switches to a slightly different algorithm for reads of >2048 bytes, but that's just a performance optimization.)
• Using libc's arc4random_buf is the officially supported solution, and will also ensure things like if the process calls fork(), then the parent and child will still see separate RNG streams.

[mmcco]

• Using libc's arc4random_buf is the officially supported solution, and will also ensure things like if the process calls fork(), then the parent and child will still see separate RNG streams.

Thanks for addressing this. It was a concern someone raised on IRC and I forgot to relay it here.