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

Solaris: consistantly use /dev/random source #310

Merged
merged 1 commit into from Oct 23, 2022
Merged

Solaris: consistantly use /dev/random source #310

merged 1 commit into from Oct 23, 2022

Conversation

josephlr
Copy link
Member

On Solaris, we opt to use /dev/random instead of /dev/urandom due to reasons explained in the comments and in this Solaris blog post.

However, we haven't been making the same choice when getting randomness via the getrandom(2) function, as we just pass 0 for the flags. We used to always set GRND_RANDOM, but that was removed in the move from OsRng to this crate.

For context, rust-random/rand#730, #9, and #51 are the major changes to the Solaris/Illumos implementation over the years.

See the solaris documentation for:

I also updated the doucmentation to better reflect when Illumos added the getrandom(2) function.

Finally, a question. #51 removed chunking for the fallback implementation that reads from /dev/random, should we add it back?

Signed-off-by: Joe Richey joerichey@google.com

@josephlr
Copy link
Member Author

As a data point, @briansmith's ring reads from /dev/urandom without chunking.

On Solaris, we opt to use /dev/random source instead of /dev/urandom due
to reasons explained in the comments and
[in this Solaris blog post](https://blogs.oracle.com/solaris/post/solaris-new-system-calls-getentropy2-and-getrandom2).

However, we haven't been making the same choice when getting randomness
via the `getrandom(2)` function, as we just pass `0` for the flags. We
[used to](https://github.com/rust-random/rand/pull/730/files#diff-694d4302a3ff2a976f2fbd34bc05ada22ee61a4e21d2d985beab27f7a809268fR151)
always set `GRND_RANDOM`, but that was removed in the move from `OsRng`
to this crate.

For context, rust-random/rand#730,
#9, and
#51 are the major changes
to the Solaris/Illumos implementation over the years.

See the solaris documentation for:
- [`getrandom(2)`](https://docs.oracle.com/cd/E88353_01/html/E37841/getrandom-2.html)
- [`urandom(4)`](https://docs.oracle.com/cd/E88353_01/html/E37851/urandom-4d.html)

I also updated the doucmentation to better reflect when
[Illumos added the `getrandom(2)` function](https://www.illumos.org/issues/9971#change-23483).

Signed-off-by: Joe Richey <joerichey@google.com>
@newpavlov
Copy link
Member

Finally, a question. #51 removed chunking for the fallback implementation that reads from /dev/random, should we add it back?

Isn't "chunking" handled automatically by sys_fill_exact? We have the exception for Emscripten, but I don't think we need a similar code for Solaris.

@josephlr
Copy link
Member Author

Finally, a question. #51 removed chunking for the fallback implementation that reads from /dev/random, should we add it back?

Isn't "chunking" handled automatically by sys_fill_exact? We have the exception for Emscripten, but I don't think we need a similar code for Solaris.

You're correct. Looking at the Solaris documentation again, it seems like read()s from /dev/random and calls to getrandom() behave differently when they are passed buffers which are "too long". The read() call will succeed and just return fewer bytes (so using sys_fill_exact is fine), while calls to getrandom() will just fail with EINVAL.

So the implementation in this PR seems correct.

@josephlr josephlr merged commit f2d7662 into master Oct 23, 2022
@josephlr josephlr deleted the solaris branch October 23, 2022 20:46
@newpavlov newpavlov mentioned this pull request Apr 2, 2023
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

Successfully merging this pull request may close these issues.

None yet

2 participants