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

Investigate replacing the ISAAC Rng with directly using the operating system, or an eSTREAM Rng (etc.) #10047

Closed
huonw opened this issue Oct 24, 2013 · 8 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Oct 24, 2013

Non-exhaustive list of possible candidates:

Important considerations:

  • performance
  • statistical/cryptographic strength
  • how far into the realm of cryptography do we want to go? (Not very far, but implementing an PRNG isn't as hard to get correct/test as other areas of cryptography, since (a) its output can be compared byte-for-byte with a known-correct implementation and (b) RNGs (for the most part) lacks some subtle constraints like constant time equality and other side channel attacks that other cryptographic algorithms require.)

The ISAAC Rng currently implemented in std::rand has some known weaknesses, but is generally accepted as cryptographically secure (although it has not received a huge amount of scrutiny).

@pcwalton
Copy link
Contributor

+1 on just using the OS. With the exception of Colin Percival, cryptographers universally tell us to do this.

@brson
Copy link
Contributor

brson commented Oct 24, 2013

I don't have a strong opinion about this as long as it remains fast, but there is a consideration here that if we do this the algorithm our CSPRNG uses will be platform-specific. There may be some value in being able to know that "Rust's CSPRNG is ISAAC", etc.

@thestinger
Copy link
Contributor

@pcwalton: The benefit of using a modern stream cipher would be removing the temptation of having non-cryptographic RNG implementations like a Mersenne twister implementation. I don't think it would be good if creating a hash table always required a system call or a lock (for a buffer).

@brson
Copy link
Contributor

brson commented Apr 18, 2014

Here's an article about the poor multithreaded performance of /dev/urandom on Linux: http://drsnyder.us/2014/04/16/linux-dev-urandom-and-concurrency.html

@JustAPerson
Copy link
Contributor

One interesting comment from that article, by Theodore Ts'o:

Then hopefully curl is using /dev/urandom to initialize its own userspace RNG (many crypto libraries do this; most should do this), so you're not trying to read from /dev/urandom for every single request. /dev/urandom is designed for security, not for speed.

It should be noted that he is the original author of /dev/random

@steveklabnik
Copy link
Member

@huonw what do you think of this ticket today? I feel like our RNG story is pretty solid at this point.

@joshtriplett
Copy link
Member

joshtriplett commented Aug 11, 2016

Switching to the OS RNG (/dev/urandom) doesn't seem like it provides significant value. However, if Rust wants to consider switching to a platform-specific RNG to gain performance (which seems reasonable if the user hasn't explicitly used a seeded RNG to get reproducible result, such as for a game with replays), then several possibilities seem worth investigating, such as the use of hardware random number generators like rdrand when available. Those would provide a major performance improvement for any high-volume use of randomness.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@aturon
Copy link
Member

aturon commented Apr 15, 2017

Closing; this would want an RFC these days, I think.

@aturon aturon closed this as completed Apr 15, 2017
flip1995 pushed a commit to flip1995/rust that referenced this issue Dec 17, 2022
…v, r=xFrednet

Add 1.58 MSRV for `collapsible_str_replace`

The `Pattern` impl for `[char; N]` was added in 1.58

changelog: Enhancement: [`collapsible_str_replace`]: Now takes MSRV into consideration. The minimal version is 1.58
[rust-lang#10047](rust-lang/rust-clippy#10047)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

8 participants