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

Implement io::Entropy and refactor random data generation #108874

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions library/std/src/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ use crate::error::Error;
use crate::fmt::{self, Debug};
#[allow(deprecated)]
use crate::hash::{BuildHasher, Hash, Hasher, SipHasher13};
use crate::io::{entropy, Read};
use crate::iter::FusedIterator;
use crate::ops::Index;
use crate::sys;

/// A [hash map] implemented with quadratic probing and SIMD lookup.
///
Expand Down Expand Up @@ -3122,7 +3122,17 @@ impl RandomState {
// increment one of the seeds on every RandomState creation, giving
// every corresponding HashMap a different iteration order.
thread_local!(static KEYS: Cell<(u64, u64)> = {
Cell::new(sys::hashmap_random_keys())
if crate::sys::entropy::INSECURE_HASHMAP {
Cell::new((1, 2))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not blocking, but one thing I was thinking of proposing as a path to make RandomState still work without libstd is allowing users to provide their own entropy function, and it would be nice if this were factored in a way that always took that path, rather than hard-coding in an insecure constant for the state.

Since no one is going to rely explicitly on the (1, 2) state for these cases by design, I think it'd be better to just provide a random eight bytes that get sent through the path instead. That way everything goes through the "entropy" path of decoding the bytes, even if the bytes are static.

} else {
let mut v = [0u8; 16];
let mut entropy = entropy();
entropy.set_insecure(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the purpose is to prevent hash DOS, attacks, wouldn't setting insecure be counter to that? Or am I missing the purpose of this flag?

entropy.read_exact(&mut v).expect("failed to generate random keys for hashmap");
let key1 = v[..8].try_into().unwrap();
let key2 = v[8..].try_into().unwrap();
Cell::new((u64::from_ne_bytes(key1), u64::from_ne_bytes(key2)))
}
});

KEYS.with(|keys| {
Expand Down
125 changes: 125 additions & 0 deletions library/std/src/io/entropy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use super::{BorrowedBuf, BorrowedCursor, Read, Result};
use crate::sys::entropy as sys;

/// A reader which returns random bytes from the system entropy source.
///
/// This struct is generally created by calling [`entropy()`]. Please
/// see the documentation of [`entropy()`] for more details.
#[derive(Debug)]
#[unstable(feature = "io_entropy", issue = "none")]
pub struct Entropy {
insecure: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could probably use sys::Entropy directly here (and define a set_insecure method for it)

}

impl Entropy {
pub(crate) fn set_insecure(&mut self, insecure: bool) {
self.insecure = insecure;
}
}

#[unstable(feature = "io_entropy", issue = "none")]
impl Read for Entropy {
#[inline]
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
sys::Entropy { insecure: self.insecure }.read(buf)
}

#[inline]
fn read_exact(&mut self, buf: &mut [u8]) -> Result<()> {
let mut buf = BorrowedBuf::from(buf);
self.read_buf_exact(buf.unfilled())
}

#[inline]
fn read_buf(&mut self, buf: BorrowedCursor<'_>) -> Result<()> {
sys::Entropy { insecure: self.insecure }.read_buf(buf)
}

#[inline]
fn read_buf_exact(&mut self, buf: BorrowedCursor<'_>) -> Result<()> {
sys::Entropy { insecure: self.insecure }.read_buf_exact(buf)
}
}

/// Constructs a new handle to the system entropy source.
///
/// Reads from the resulting reader will return high-quality random data that
/// is suited for cryptographic purposes (by the standard of the platform defaults).
///
/// Be aware that, because the data is of very high quality, reading high amounts
/// of data can be very slow, and potentially slow down other processes requiring
/// random data. Use a pseudo-random number generator if speed is important.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we do this elsewhere in libstd with other crates, perhaps we could just straight-up recommend the rand crate for this purpose? Especially since this is an org-maintained crate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have been trying to not advertise external crates from std docs. And there are other potential options beside rand even if they're more niche.

///
/// # Platform sources
///
/// | OS | Source
/// |------------------|--------
/// | Linux, Android | [`getrandom`][1] if available, otherwise [`/dev/urandom`][2] after successfully polling [`/dev/random`][2]
/// | Windows | [`BCryptGenRandom`][3], falling back to [`RtlGenRandom`][4]
/// | macOS | [`getentropy`][5] if available, falling back to [`/dev/urandom`][6]
/// | OpenBSD | [`getentropy`][7]
/// | iOS, watchOS | [`SecRandomCopyBytes`][8]
/// | FreeBSD | [`kern.arandom`][9]
/// | NetBSD | [`kern.arandom`][10]
/// | Fuchsia | [`zx_cprng_draw`][11]
/// | WASM | *Unsupported*
/// | WASI | [`random_get`][12]
/// | Emscripten | [`getentropy`][7]
/// | Redox | `rand:`
/// | VxWorks | `randABytes` after checking entropy pool initialization with `randSecure`
/// | Haiku | `/dev/urandom`
/// | ESP-IDF, Horizon | [`getrandom`][1]
/// | Other UNIXes | `/dev/random`
/// | Hermit | [`read_entropy`][13]
/// | SGX | [`RDRAND`][14]
/// | SOLID | `SOLID_RNG_SampleRandomBytes`
///
/// [1]: https://man7.org/linux/man-pages/man2/getrandom.2.html
/// [2]: https://man7.org/linux/man-pages/man7/random.7.html
/// [3]: https://learn.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom
/// [4]: https://learn.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom
/// [5]: https://www.unix.com/man-page/mojave/2/getentropy/
/// [6]: https://www.unix.com/man-page/mojave/4/random/
/// [7]: https://man.openbsd.org/getentropy.2
/// [8]: https://developer.apple.com/documentation/security/1399291-secrandomcopybytes?language=objc
/// [9]: https://man.freebsd.org/cgi/man.cgi?query=random&sektion=4&manpath=FreeBSD+13.1-RELEASE+and+Ports
/// [10]: https://man.netbsd.org/rnd.4
/// [11]: https://fuchsia.dev/fuchsia-src/reference/syscalls/cprng_draw
/// [12]: https://github.com/WebAssembly/WASI/blob/main/legacy/preview1/docs.md
/// [13]: https://docs.rs/hermit-abi/latest/hermit_abi/fn.read_entropy.html
/// [14]: https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html
///
/// # Examples
///
/// Generating a seed for a random number generator:
///
/// ```rust
/// #![feature(io_entropy)]
///
/// # use std::io::Result;
/// # fn main() -> Result<()> {
/// use std::io::{entropy, Read};
///
/// let mut seed = [0u8; 32];
/// entropy().read_exact(&mut seed)?;
/// println!("seed: {seed:?}");
/// # Ok(())
/// # }
/// ```
///
/// Implementing your very own `/dev/random`:
///
/// ```rust, no_run
/// #![feature(io_entropy)]
///
/// use std::io::{copy, entropy, stdout};
///
/// fn main() {
/// let _ = copy(&mut entropy(), &mut stdout());
/// }
/// ```
#[inline]
#[unstable(feature = "io_entropy", issue = "none")]
pub fn entropy() -> Entropy {
Entropy { insecure: false }
}
10 changes: 10 additions & 0 deletions library/std/src/io/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ use crate::sys_common::memchr;

#[stable(feature = "bufwriter_into_parts", since = "1.56.0")]
pub use self::buffered::WriterPanicked;
#[unstable(feature = "io_entropy", issue = "none")]
pub use self::entropy::{entropy, Entropy};
#[unstable(feature = "raw_os_error_ty", issue = "107792")]
pub use self::error::RawOsError;
pub(crate) use self::stdio::attempt_print_to_stderr;
Expand Down Expand Up @@ -289,6 +291,7 @@ pub(crate) use error::const_io_error;
mod buffered;
pub(crate) mod copy;
mod cursor;
mod entropy;
mod error;
mod impls;
pub mod prelude;
Expand Down Expand Up @@ -351,6 +354,13 @@ where
}
}

/// Implement the `read` method by forwarding to `read_buf`.
// FIXME(joboet): remove once #106643 is merged.
pub(crate) fn default_read<R: Read + ?Sized>(r: &mut R, buf: &mut [u8]) -> Result<usize> {
let mut buf = BorrowedBuf::from(buf);
r.read_buf(buf.unfilled()).map(|()| buf.len())
}

// This uses an adaptive system to extend the vector when it fills. We want to
// avoid paying to allocate and zero a huge chunk of memory if the reader only
// has 4 bytes while still making large reads if the reader does have a ton
Expand Down
14 changes: 14 additions & 0 deletions library/std/src/io/readbuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,20 @@ impl<'a> BorrowedCursor<'a> {
&mut self.buf.buf[self.buf.filled..]
}

/// Returns a pointer to the current position of the cursor, with provenance to the
/// rest of the buffer.
///
/// The resulting pointer may not be used to deinitialize any bytes in the initialized
/// section of the buffer.
// FIXME(joboet): Maybe make this public?
#[allow(unused)]
pub(crate) fn as_ptr(&mut self) -> *mut u8 {
// SAFETY: deinitialization requires `unsafe` in the caller, where they have to make
// sure not to deinitialize any bytes. The burden of prove is moved to the pointer
// operation. Just returning a pointer is sound.
unsafe { self.as_mut().as_mut_ptr().cast() }
}

/// Advance the cursor by asserting that `n` bytes have been filled.
///
/// After advancing, the `n` bytes are no longer accessible via the cursor and can only be
Expand Down
22 changes: 22 additions & 0 deletions library/std/src/sys/hermit/entropy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
use super::{abi, cvt};
use crate::io::{default_read, BorrowedCursor, Read, Result};

pub const INSECURE_HASHMAP: bool = false;

pub struct Entropy {
pub insecure: bool,
}

impl Read for Entropy {
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
default_read(self, buf)
}

fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> Result<()> {
unsafe {
let len = cvt(abi::read_entropy(buf.as_ptr(), buf.capacity(), 0))?;
buf.advance(len as usize);
Ok(())
}
}
}
15 changes: 1 addition & 14 deletions library/std/src/sys/hermit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub mod alloc;
pub mod args;
#[path = "../unix/cmath.rs"]
pub mod cmath;
pub mod entropy;
pub mod env;
pub mod fd;
pub mod fs;
Expand Down Expand Up @@ -75,20 +76,6 @@ pub fn abort_internal() -> ! {
}
}

pub fn hashmap_random_keys() -> (u64, u64) {
let mut buf = [0; 16];
let mut slice = &mut buf[..];
while !slice.is_empty() {
let res = cvt(unsafe { abi::read_entropy(slice.as_mut_ptr(), slice.len(), 0) })
.expect("failed to generate random hashmap keys");
slice = &mut slice[res as usize..];
}

let key1 = buf[..8].try_into().unwrap();
let key2 = buf[8..].try_into().unwrap();
(u64::from_ne_bytes(key1), u64::from_ne_bytes(key2))
}

// This function is needed by the panic runtime. The symbol is named in
// pre-link args for the target specification, so keep that in sync.
#[cfg(not(test))]
Expand Down
5 changes: 3 additions & 2 deletions library/std/src/sys/sgx/abi/usercalls/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cmp;
use crate::io::{Error as IoError, ErrorKind, IoSlice, IoSliceMut, Result as IoResult};
use crate::sys::rand::rdrand64;
use crate::sys::entropy::rdrand64;
use crate::time::{Duration, Instant};

pub(crate) mod alloc;
Expand Down Expand Up @@ -164,7 +164,8 @@ pub fn wait(event_mask: u64, mut timeout: u64) -> IoResult<u64> {
// trusted to ensure accurate timeouts.
if let Ok(timeout_signed) = i64::try_from(timeout) {
let tenth = timeout_signed / 10;
let deviation = (rdrand64() as i64).checked_rem(tenth).unwrap_or(0);
let rand = rdrand64().unwrap_or_else(|| rtabort!("Failed to obtain random data"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: There's no need to abort here when rdrand64() returns None. This code is just there to avoid developers from relying on the accuracy of timeouts. Using a default (as in the original code) is fine here.
There's even a small availability issue with the new code. The rdrand instruction can be executed by any userspace program. So an attacker may be able to execute this instruction over and over again until the pool of random values is exhausted. This will cause the enclave to abort here. SGX does not provide any security guarantees related to availability, but this attack may be executed to make enclaves abort even when the attacker would not be able to do so through other means (e.g., they don't have the access rights to stop the process running the enclave). On the other hand, when the enclave needs to do a cryptographic operation, there's no other way than to abort. Hence, it's a bit of a nitpick :)

let deviation = (rand as i64).checked_rem(tenth).unwrap_or(0);
timeout = timeout_signed.saturating_add(deviation) as _;
}
}
Expand Down
36 changes: 36 additions & 0 deletions library/std/src/sys/sgx/entropy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use crate::io::{const_io_error, default_read, BorrowedCursor, ErrorKind, Read, Result};

pub const INSECURE_HASHMAP: bool = false;

pub struct Entropy {
pub insecure: bool,
}

impl Read for Entropy {
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
default_read(self, buf)
}

fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> Result<()> {
if buf.capacity() != 0 {
let rand = rdrand64()
.ok_or(const_io_error!(ErrorKind::WouldBlock, "no random data available"))?;
buf.append(&rand.to_ne_bytes()[..usize::min(buf.capacity(), 8)]);
Ok(())
} else {
Ok(())
}
}
}

pub(super) fn rdrand64() -> Option<u64> {
unsafe {
let mut ret: u64 = 0;
for _ in 0..10 {
if crate::arch::x86_64::_rdrand64_step(&mut ret) == 1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a maintainer of sgx but this does not offer high quality entropy. See the "Generating seeds with rdrand" section in https://www.intel.com/content/www/us/en/developer/articles/guide/intel-digital-random-number-generator-drng-software-implementation-guide.html for how to do it (which we probably don't want in std...)

Copy link
Member Author

@joboet joboet Mar 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... don't quite get that section, unfortunately.

IIUC (please feel free to correct me), RDRAND is like /dev/urandom in that it uses a CSPRNG to generate more random data from a single, truly random seed. Because of the size of the seed, constant reseeding and the strength of the PRNG, the output should not be in any practical way less secure that RDSEED. But that doesn't explain why anyone would use RDSEED...

Edit: Ah, I missed the part about forward and backward prediction resistance... Still, I don't know if it's really, truly necessary here.

I didn't change this, because getrandom uses RDRAND on SGX. Note that it does not consider values 0 or u64::MAX valid because of hardware bugs on AMD devices. Since SGX is Intel-only, I guess it isn't a concern here?

If RDRAND is truly the wrong option here, I guess the alternative is doing what I did for Hermit here and use RDSEED to seed our own PRNG.

CC @raoulstrackx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...the DRNG using the RDRAND instruction is useful for generating high-quality keys for cryptographic protocols, and the RSEED instruction is provided for seeding software-based pseudorandom number generators (PRNGs)

I think that sums it up nicely. If someone needs stronger guarantees for a PRNG, they should take a look at what exactly happens in getrandom. The documentation explicitly mentions the randomness comes from rdrand. To me that looks fine, but I'm not a cryptographic. @zugzwang can you pitch in?

Indeed, AMD hardware bugs have no impact on SGX and the values 0 and u64::MAX should not be handled differently.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Intel documentation looks clear, and there is useful discussion on security levels here.

return Some(ret);
}
}
None
}
}
19 changes: 1 addition & 18 deletions library/std/src/sys/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub mod alloc;
pub mod args;
#[path = "../unix/cmath.rs"]
pub mod cmath;
pub mod entropy;
pub mod env;
pub mod fd;
#[path = "../unsupported/fs.rs"]
Expand Down Expand Up @@ -144,24 +145,6 @@ pub extern "C" fn __rust_abort() {
abort_internal();
}

pub mod rand {
pub fn rdrand64() -> u64 {
unsafe {
let mut ret: u64 = 0;
for _ in 0..10 {
if crate::arch::x86_64::_rdrand64_step(&mut ret) == 1 {
return ret;
}
}
rtabort!("Failed to obtain random data");
}
}
}

pub fn hashmap_random_keys() -> (u64, u64) {
(self::rand::rdrand64(), self::rand::rdrand64())
}

pub use crate::sys_common::{AsInner, FromInner, IntoInner};

pub trait TryIntoInner<Inner>: Sized {
Expand Down
31 changes: 31 additions & 0 deletions library/std/src/sys/solid/entropy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
use super::abi;
use super::error::SolidError;
use crate::io::{default_read, BorrowedCursor, Read, Result};

pub const INSECURE_HASHMAP: bool = false;

pub struct Entropy {
pub insecure: bool,
}

impl Read for Entropy {
fn read(&mut self, buf: &mut [u8]) -> Result<usize> {
default_read(self, buf)
}

fn read_buf(&mut self, mut buf: BorrowedCursor<'_>) -> Result<()> {
SolidError::err_if_negative(unsafe {
abi::SOLID_RNG_SampleRandomBytes(buf.as_mut().as_mut_ptr().cast(), buf.capacity())
})
.map_err(|e| e.as_io_error())?;

unsafe {
buf.advance(buf.capacity());
Ok(())
}
}

fn read_buf_exact(&mut self, buf: BorrowedCursor<'_>) -> Result<()> {
self.read_buf(buf)
}
}
Loading