rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.93k stars 12.53k forks source link

Tracking issue for stabilizing randomness #27703

Closed alexcrichton closed 6 years ago

alexcrichton commented 9 years ago

Long ago we had std::rand and nowadays we have extern crate rand. This is such core functionality it likely wants to move back into the standard library at some point, and in the meantime this issue serves as a tracking issue for the rand feature in the standard library.

The process for adding this functionality to the standard library will likely look like something along the lines of https://github.com/rust-lang/rfcs/pull/1242.

cc @huonw

nagisa commented 9 years ago

rand is not even sure about algorithms implemented and AFAIR is having a major rewrite in works. I doubt it can go back into libstd anytime soon.

aturon commented 8 years ago

Nominating for discussion at libs meeting: we're not ready to stabilize anything here, but I'd like to discuss the plan.

alexcrichton commented 8 years ago

Unfortunately the libs team didn't get a chance to talk about this in terms of stabilization for 1.8, but I'm going to leave the nominated tag as I think we should chat about this regardless.

briansmith commented 8 years ago

Suggestions:

Obviously, I am biased, but I think a better API for a CSPRNG is the very simple one in ring: https://briansmith.org/rustdoc/ring/rand/index.html. This type of interface is sufficient for every use of a CSPRNG I've ever needed. Note that an implementation of an interface like Rng such as OsRng can easily be built on top of such an interface.

briansmith commented 8 years ago

Note that OsRng::gen_range and probably other methods are not generally safe because they don't take into account the fact that such random numbers should be generated without leaking information about the values of the random numbers. In particular, it would be tempting to use OsRng::gen_range to generate keys for public key operations using some bignum class, but AFAICT it wouldn't be safe since the comparison of the lower/upper bounds to the generated value isn't safe from timing side channels.

briansmith commented 8 years ago

More feedback: Currently rand::Rng::fill_bytes is defined to panic if it fails to generate the necessary random values, and there's no way to avoid those panics. Further, the panic behavior of other methods isn't specified but since they also don't use Result as their return type, they also have no choice other than to panic or abort the process.

For some applications, this is unacceptable. The interface should have results returned using Result.

However, it is also the case that some implementations may experience errors that they cannot avoid, so it may be worth allowing an implementation to panic or abort the process if it has no other choice on failure.

SimonSapin commented 8 years ago

However, it is also the case that some implementations may experience errors that they cannot avoid, so it may be worth allowing an implementation to panic or abort the process if it has no other choice on failure.

That’s Result::unwrap, right?

briansmith commented 8 years ago

However, it is also the case that some implementations may experience errors that they cannot avoid, so it may be worth allowing an implementation to panic or abort the process if it has no other choice on failure. That’s Result::unwrap, right?

I'm assuming that the fill function is changed to return a Result<(), ()>. I'm saying that some implementations of fill might not be able to return Err(()) on failure because the function that they are implemented on top of calls abort on failure. For example, that would be the case if one used BoringSSL's RAND_bytes and an error occurred inside RAND_bytes. (This isn't a concern in ring, though.) It's still a good idea to have it return Result<(), ()> but this is something to consider.

briansmith commented 8 years ago

Just FYI, I outlined what I think such an API should look like at https://briansmith.org/rustdoc/ring/rand/index.html, particularly https://briansmith.org/rustdoc/ring/rand/struct.SystemRandom.html.

aneeshusa commented 8 years ago

+1 from me on the design of SystemRandom from ring; it has the key property of simplicity which means that it's easy to use, that people will use it, and that it's hard to use incorrectly.

Ideal randomness APIs are infallible, for example OpenBSD's getentropy(2), or Linux's getrandom(2) with a wrapper. It would be nice to have an InfallibleRandom trait as well, where there is a fn fill_infallible or similar with no return value (e.g. always succeeds).

Pratyush commented 7 years ago

I'm not sure what the current status of this issue is, but tt would be ideal to have at least some portion of rand be in core, such as the Rng and Rand traits. This would be useful in situations like using Rust in Intel SGX, which has the rdrand instruction. For prior discussion, see: https://github.com/rust-lang-nursery/rand/pull/108 and https://github.com/rust-lang-nursery/rand/issues/123

burdges commented 7 years ago

I agree with @briansmith that OsRng should be brought up to cryptographic standards.

Above that, there are as more users who care about the performance of rand's distributions, Rng methods, etc. than about them being constant time. To be clear, I actually do want even a constant-time Ziggurat algorithm, but very few cryptographic applications need that.

I'd suggest we clearly warn about the side channel vulnerabilities the Rng trait itself, but keep it performance focused. An external ct-rand crate could reimplement the remaining functionality of rand in constant time via a wrapper struct CTRng<R: Rng>(R) with an impl<R: Rng> Rng for CTRng<R> { .. } that provide constant time reimplementations of all default methods and new constant time distributions.

We should remove functionality from Rng that simply cannot be provided in constant time of course. I believe specialization should permit this CTRng<R> wrapper to be optimized for specific R: Rng though, so maybe everything will be okay if we simply put in the work. It's fine if the CTRng<R> wrapper comes with some warning that using an arbitrary R: Rng might fail to produce constant time code, and recommends only using the specialized choices for R.

joshlf commented 7 years ago

@burdges

wrt constant-time generation, I assume you're talking about a userspace CSPRNG seeded from the kernel? There was some discussion in the recent internals thread about this, and I argued that user-space CSPRNGs are dangerous, and some agreed. The takeaway is that the discussion there seems to be leaning towards a) cryptographically-secure generation by default and, b) not providing user-space PRNGs by default.

burdges commented 7 years ago

I meant there should be versions that use constant time comparisons in the methods of Rng, and even a distributions using a Ziggurat algorithm that uses constant time comparisons. These function would obviously not literally constant time themselves, but they avoid timing leaks about the random value actually used, unless used with a bad CSPRNG. Also, I'm claiming these functions could be provided by an external crate, so std need not worry about such niche cryptographic tools.

joshlf commented 7 years ago

Sorry, my point isn't that being constant-time isn't a good thing (and I know that "constant time" should really be "timing doesn't leak information about the seed"), but rather that this discussion seems to assume a user-land CSPRNG. A user-land CSPRNG is fine, but my point is simply that the default should be to not use a user-land CSPRNG for the reasons outlined in the thread I linked to. It's fine if users want to enable it (presuming we document the security risks), but it shouldn't be the default.

burdges commented 7 years ago

I see no user-land CSPRNG assumptions here. There is a discussion about Rng methods returning errors, probably via some new associated type Rng::Error. User-land CSPRNGs cannot avoid that error type because they run out of randomness eventually too.

There are legitimate needs for short lived user-land CSPRNGs in protocols with deterministic commitments, inside KDFs, etc., but those usages are niche enough that an external crate suffices.

joshlf commented 7 years ago

User-land CSPRNGs cannot avoid that error type because they run out of randomness eventually too.

Common misconception, but it's not true. Properly-seeded CSPRNGs can never run out of entropy.

I see no user-land CSPRNG assumptions here.

Probably a misreading on my part. To me, the discussion of constant-time CSPRNGs implied a user-land implementation (constant-timeness isn't a concern when using using kernel-provided randomness - you just get what the kernel gives you, take it or leave it). That's the only reason I was concerned.

SimonSapin commented 6 years ago

The std::rand module was removed in 6bc8f164b09b9994e6a2d4c4ca60d7d36c09d3fe and I’m not aware of any concrete plan to move https://crates.io/crates/rand back into the standard library, so I believe there is nothing to track here. Further discussion should go to https://github.com/rust-lang-nursery/rand/issues