Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue for stabilizing randomness #27703

Closed
alexcrichton opened this issue Aug 12, 2015 · 18 comments
Closed

Tracking issue for stabilizing randomness #27703

alexcrichton opened this issue Aug 12, 2015 · 18 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

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 rust-lang/rfcs#1242.

cc @huonw

@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Aug 12, 2015
@nagisa
Copy link
Member

nagisa commented Aug 12, 2015

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
Copy link
Member

aturon commented Jan 27, 2016

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

@alexcrichton
Copy link
Member Author

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
Copy link
Contributor

briansmith commented Feb 1, 2016

Suggestions:

  • Don't expose ChaChaRng through a std:: module. Some systems need to ensure that any cryptographic algorithm in use is certified, and making ChaChaRng part of the standard library would be counterproductive for them.
  • Similar to how we can override allocators, provide a way to override the CSPRNG so that (a) the application can swap in a CSRNG that is known to be good according to its standards (see previous item), and (b) so that the applicaiton can swap in a CSPRNG that takes advantage of special knowledge the application has. For example, If the application is only going to be deployed on systems that have the getrandom syscall, then the application may wish to swap in an implementation that avoids the /dev/urandom fallback completely. This is especially the case for an application that wants to use seccomp-bpf to prevent the use of open/read/etc.
  • Provide a mechanism separate from the Rng trait to fill a slice with (secure) random bytes. The Rng trait is very badly designed as it mixes all kinds of concerns that most applications don't have. This mixing causes trouble for some applications. For example, Rng has next_f32 and next_f64 methods, which means that it cannot be fully implemented on any system that does not have floating point support and/or which wants to avoid linking in the floating point libraries. This in turn means that such applications cannot use OsRng.

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link

+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
Copy link
Contributor

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: rust-random/rand#108 and rust-random/rand#123

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Jul 22, 2017
@burdges
Copy link

burdges commented Jul 27, 2017

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
Copy link
Contributor

joshlf commented Jul 28, 2017

@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
Copy link

burdges commented Jul 28, 2017

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
Copy link
Contributor

joshlf commented Jul 28, 2017

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
Copy link

burdges commented Jul 29, 2017

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
Copy link
Contributor

joshlf commented Aug 2, 2017

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
Copy link
Contributor

The std::rand module was removed in 6bc8f16 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants