Split OS RNG into rand_os

[newpavlov]

This PR moves OS RNG into the separate crate with some code reorganization. Changes should be double checked, as I've only tested them for Linux.

[dhardy]

I wasn't expecting this. Is this your take on the discussion in #290? Because it's not what I had in mind.

Splitting that big os.rs file does make sense though.

[newpavlov]

Nah, I always wanted to move OS RNG into a separate crate (and I am a bit surprised you don't want to), as it does not make much sense to depend on the whole rand when I just want some entropy. Even if one day we'll get some kind of DefaultRng (I don't remember the issue in which we have discussed it), I think it's worth to have a separate OS RNG crate.

I think that entropy and thread RNGs can be implemented on the top of rand_os. BTW I don't like this name that much, and would like to use os-rng instead, but oh well. :)

[dhardy]

@aturon do you see this fitting into the std lib or do you think hashmap_random_keys will remain a minimal parallel implementation of this? Apparently you're not so concerned about failed randomisation?

We already have implementations for some platforms you don't; it should make it easier porting Rust to new languages if this stuff only has to be written once.

[aturon]

I'm probably not the best person to ask (I'm not involved with std these days); @alexcrichton @sfackler thoughts?

[sfackler]

I think providing a direct interface to the OS's secure random facility is a reasonable primitive to provide in the standard library personally. As you mentioned, we already rely on it for hashmap keying, and the interface of pub fn secure_random(buf: &mut [u8]) -> io::Result<()> seems pretty straightforward/minimal.

[newpavlov]

@sfackler
Can this function be made a lang item and without relying on io::Error? Previously it was brought up here. Motivation here is a desire to be able to define "default" RNG source on embedded platforms and even to use a custom RNG instead of OS provided, e.g. by stub or hardware based RNG. (the latter can be a potential security pitfall, but also can be usable for testing purposes)

It could be done using mutable statics, but it's not the best solution.

[sfackler]

Sure, it could be backed by a weak lang item potentially, though that's starting to go further down the API design rabbit hole.

[dhardy]

@sfackler a "simple" function like pub fn secure_random(buf: &mut [u8]) -> io::Result<()> may be the way to go.

Our current interface is a bit more nuanced with detection for initialisation as well as potential errors on use. I think we should at least have reporting for "not ready yet" and "other error" through error codes.

As @newpavlov says, we should try to make something usable with no_std too. The best option is likely requiring user-implementation via lang items when not using std.

There's an additional complication: io::Error is not available outside of std. Some bastardisation of it could be though if the custom variant is unavailable without std, similar to our rand_core::Error.

As you say, it is an API design rabbit-hole which for sure needs an RFC to implement, but I think it would be worth trying to unify our OS random interfaces. Also in #579 we discussed using lang items to solve no_std support, but that didn't seem to be an option outside of the std lib.

---

Unfortunately, there is yet another complication. We use EntropyRng to allow automatic fallback to an alternative source of random data in case the system generator doesn't work. I'm not sure that std should include all the required code for jitter-entropy and a user-space RNG. Perhaps it doesn't matter though since the randomised hasher is only for DOS protection and other uses can get random data via Rand.

[dhardy]

As @newpavlov says in #648 it's probably worth moving forward with this anyway. So I'd like to review (or at least have one independent review) but don't have a lot of time for that myself now. Maybe @tarcieri would like to review?

[newpavlov]

Not sure how to fix errors with doc upload and stdweb test.

[dhardy]

Some doc links are broken; you'll have to search for them:

$ cargo deadlinks --dir target/doc
Linked file at path /home/travis/build/rust-random/rand/target/doc/rand_os/struct.EntropyRng.html does not exist!
Linked file at path /home/travis/build/rust-random/rand/target/doc/rand_os/struct.EntropyRng.html does not exist!
Linked file at path /home/travis/build/rust-random/rand/target/doc/enum.ErrorKind.html does not exist!
Linked file at path /home/travis/build/rust-random/rand/target/doc/trait.RngCore.html does not exist!
Linked file at path /home/travis/build/rust-random/rand/target/doc/trait.RngCore.html does not exist!

Not sure about the other one. I tried restarting it, but I don't expect it to go away...

[dhardy]

@alexcrichton any idea why this WASM build fails? CI log

[newpavlov]

BTW how about moving most of the OsRng documentation to the crate level?

[dhardy]

Sure, go for it. Another option is moving docs to the book, though I'm not sure if it's appropriate in this case.

[dhardy]

Okay, have it your way and just re-export rand_core. Users can do:
use rand_os::{OsRng, rand_core::RngCore};

Then I think the only blocker is the WASM test. I don't have any ideas and don't really have the time to investigate.

[newpavlov]

@ashleygwilliams
Can you help us with the wasm32 cargo web CI test? (error, travis.yml) Also I am interested to hear your opinion on this bit.

[dhardy]

Sorry @newpavlov; I restarted the test runner but it appears it's still based on pre- #660 code. Can you rebase?

[dhardy]

One thing that occurs to me is that if we get an OS-RNG interface in lib core, then we can make OsRng a wrapper around that and put it directly in rand_core (since there will be no extra dependencies). This makes an extra crate like rand_os unnecessary.

I guess it's time to try writing that RFC we talked about in #648 ...

[newpavlov]

This makes an extra crate like rand_os unnecessary.

I don't think so, I've stated my opinion regarding it here.

[dhardy]

@newpavlov could you please rebase on latest master?

[newpavlov]

Done!

[newpavlov]

Well, I've "fixed" the CI issue, one of the reasons behind it was koute/cargo-web#72, though cargo web test --target wasm32-unknown-unknown --features=rand_os/stdweb still fails for some reason, so for the time being I've replaced it with cd rand_os && cargo web test --target wasm32-unknown-unknown --features=stdweb, which works without issues.

[dhardy]

Thanks @newpavlov; I guess that will do.

I made a few tweaks in my branch; could you pull them please: https://github.com/dhardy/rand/tree/rand_os

[newpavlov]

Done.