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

Verify /dev/urandom has been seeded before using it. #558

Closed
DemiMarie opened this issue Jul 20, 2017 · 9 comments
Closed

Verify /dev/urandom has been seeded before using it. #558

DemiMarie opened this issue Jul 20, 2017 · 9 comments
Milestone

Comments

@DemiMarie
Copy link

libsodium polls on /dev/random to make sure the kernel’s PRNG is seeded. This can uncover real problems in virtualized environments.

@ghost
Copy link

ghost commented Jul 20, 2017

Ring only uses /dev/urandom as a last resort because it is slow.

Can't you do this yourself?

@briansmith
Copy link
Owner

@DemiMarie I recommend people disable the default dev_urandom_fallback feature, which has the same effect with less code.

@briansmith
Copy link
Owner

@briansmith briansmith changed the title Poll on /dev/random on Linux Verify /dev/urandom has been seeded before using it. Jun 15, 2019
@briansmith
Copy link
Owner

I generalized the title of this issue so that it doesn't presume a particular implementation strategy. I agree that it is worth doing something here.

@DemiMarie I recommend people disable the default dev_urandom_fallback feature, which has the same effect with less code.

I wish this worked better. In practice it is pretty hard to disable the dev_urandom_fallback feature because any crate that forgets to do so will implicitly enable it. :(

@josephlr
Copy link
Contributor

There are many ways we could go about doing this:

  1. Read a single byte from /dev/random
    a. This is how getrandom (the OS-specific code for rand) does it: link
    b. It's very simple to implement in safe Rust
    c. It has the issue that it consumes a byte from /dev/random, which could make other users of /dev/random very slightly slower.
    d. Opens two files, although only one is ever open at a time.
  2. Use the RNDGETENTCNT ioctl on /dev/urandom to query the amount of kernel randomness in a loop
    a. This is what BoringSSL does: link
    b. Only uses a single file.
    b. More complex and requires unsafe Rust.
    c. We would need to decide what we do inside the loop to not burn CPU. BoringSSL sleeps for 0.25 seconds. We could use thread::yield_now().
  3. Poll /dev/random using libc::poll
    a. This is how libsodium does it: link
    b. More complex and requires unsafe Rust.
    c. Opens two files, although only one is ever open at a time.

I'm quite inclined to go with approach (1). It's simple and avoiding the downsides in (1c) and (1d) don't seem worth the engineering/maintenance complexity.

@josephlr
Copy link
Contributor

Regardless of which approach we implement, we should get rid of the dev_urandom_fallback feature. It's no longer necessary from a security perspective after this issue is resolved. If people really want to not read from a file, they can just disable all uses of std using the appropriate Cargo feature.

@briansmith
Copy link
Owner

briansmith commented Jun 18, 2019

1 Read a single byte from /dev/random
3 Poll /dev/random using libc::poll

Approaches based on /dev/random will block even when /dev/urandom is seeded, when the entropy estimate for /dev/random is too low. This is why I think #2 makes the most sense of the three options presented.

@josephlr
Copy link
Contributor

This is why I think #2 makes the most sense of the three options presented.

Unfortunately this also has the problem that it goes to 0 if /dev/urandom is seeded, but the entropy estimate for /dev/random is too low. Per the random(4) manpage, RNDGETENTCNT does the same thing on /dev/random and /dev/urandom (source code). I don't really think there is a way to perfectly solve this problem.

Given this, the only downside with approch (1) is that it "debits" 1 byte of entropy from /dev/random. I don't think it's a huge concern, but if you do approch (3) seems best from a CPU overhead perspective.

@briansmith
Copy link
Owner

Fixed in 0.17.0 by switching to getrandom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants