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

Use getrandom crate #62082

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 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
20 changes: 20 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,15 @@ dependencies = [
"unicode-width 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "getrandom"
version = "0.1.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "git2"
version = "0.9.1"
Expand Down Expand Up @@ -1392,6 +1401,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
name = "lazy_static"
version = "1.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"spin 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "lazycell"
Expand Down Expand Up @@ -3495,6 +3507,11 @@ dependencies = [
"winapi 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
]

[[package]]
name = "spin"
version = "0.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"

[[package]]
name = "stable_deref_trait"
version = "1.1.0"
Expand All @@ -3512,6 +3529,7 @@ dependencies = [
"core 0.0.0",
"dlmalloc 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"fortanix-sgx-abi 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
"getrandom 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
"hashbrown 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)",
"panic_abort 0.0.0",
Expand Down Expand Up @@ -4486,6 +4504,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum fwdansi 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "34dd4c507af68d37ffef962063dfa1944ce0dd4d5b82043dbab1dabe088610c3"
"checksum generic-array 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ef25c5683767570c2bbd7deba372926a55eaae9982d7726ee2a1050239d45b9d"
"checksum getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)" = "72327b15c228bfe31f1390f93dd5e9279587f0463836393c9df719ce62a3e450"
"checksum getrandom 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "82d68102e030c3d41969bb76ebf55ed731fd946a0a91e0fa7e02f6392cc6db21"
"checksum git2 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "924b2e7d2986e625dcad89e8a429a7b3adee3c3d71e585f4a66c4f7e78715e31"
"checksum git2-curl 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f21f0550fd5d3f7c5adb94797fcd3d1002d7fc1fa349c82fe44f3c97ef80b62c"
"checksum glob 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574"
Expand Down Expand Up @@ -4668,6 +4687,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8"
"checksum smallvec 0.6.10 (registry+https://github.com/rust-lang/crates.io-index)" = "ab606a9c5e214920bb66c458cd7be8ef094f813f20fe77a54cc7dbfff220d4b7"
"checksum socket2 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)" = "c4d11a52082057d87cb5caa31ad812f4504b97ab44732cd8359df2e9ff9f48e7"
"checksum spin 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "44363f6f51401c34e7be73db0db371c04705d35efbe9f7d6082e03a921a32c55"
"checksum stable_deref_trait 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ffbc596e092fe5f598b12ef46cc03754085ac2f4d8c739ad61c4ae266cc3b3fa"
"checksum string_cache 0.7.3 (registry+https://github.com/rust-lang/crates.io-index)" = "25d70109977172b127fe834e5449e5ab1740b9ba49fa18a2020f509174f25423"
"checksum string_cache_codegen 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1eea1eee654ef80933142157fdad9dd8bc43cf7c74e999e369263496f04ff4da"
Expand Down
1 change: 1 addition & 0 deletions src/libstd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ compiler_builtins = { version = "0.1.16" }
profiler_builtins = { path = "../libprofiler_builtins", optional = true }
unwind = { path = "../libunwind" }
hashbrown = { version = "0.4.0", features = ['rustc-dep-of-std'] }
getrandom = "0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Caveat: this won't work on bare WASM since getrandom currently requires a feature flag to enable either stdweb or wasm_bindgen. There was mention of making wasm_bindgen the default; we might want to do this first?

Copy link
Contributor Author

@newpavlov newpavlov Jun 25, 2019

Choose a reason for hiding this comment

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

Personally I would prefer if a separate target will be introduced and wasm32-unknown-wunknown will stay assumptions-free regarding executor environment.

Copy link
Contributor Author

@newpavlov newpavlov Jun 25, 2019

Choose a reason for hiding this comment

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

IIUC without enabled stdweb or wasm_bindgen features outside of WASI and Emscripten it will use a dummy impl, so it will work in the same way as it does today by using a constant seed value.


[dependencies.backtrace]
version = "0.3.29"
Expand Down
18 changes: 14 additions & 4 deletions src/libstd/collections/hash/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2445,13 +2445,23 @@ impl RandomState {
// iteration order allows a form of DOS attack. To counter that we
// 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())
thread_local!(static KEYS: Cell<[u64; 2]> = {
let mut buf = [0u8; 16];
// In case of `Error::UNAVAILABLE` we use a constant value
// for the following whitelisted targets
let whitelisted = cfg!(target="wasm32-unknown-unknown");
Copy link
Member

@kennytm kennytm Jun 29, 2019

Choose a reason for hiding this comment

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

You could cfg out a match branch.

match getrandom::getrandom(&mut buf) {
    Ok(()) => {},
    #[cfg(target = "wasm32-unknown-unknown")]
    Err(Error::UNAVAILABLE) => {},
    Err(err) => panic!("getrandom failure: {:?}", err),
}

match getrandom::getrandom(&mut buf) {
Ok(()) => (),
newpavlov marked this conversation as resolved.
Show resolved Hide resolved
Err(Error::UNAVAILABLE) if whitelisted => (),
Err(err) => panic!("getrandom failure: {:?}", err),
}
Copy link
Member

@cuviper cuviper Aug 14, 2019

Choose a reason for hiding this comment

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

On wasm32-unknown-unknown, this will effectively be:

if false {
    getrandom::getrandom(&mut buf).unwrap();
}

... so it does require the getrandom dependency, even though this is dead code.

If you #[cfg(...)] gate this expression instead, it won't always need getrandom.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But IIUC Cargo determines crates to be compiled only by looking at Cargo.toml, without taking crate code into account. So even if a dependency is not used, it will be still compiled, which in our case means a compilation error.

I forgot that we can use cfg like this:

#[cfg(..)]
getrandom::getrandom(&mut buf).unwrap();

I will replace those lines in the next commit.

let n = u128::from_ne_bytes(buf);
Cell::new([n as u64, (n >> 64) as u64])
});

KEYS.with(|keys| {
let (k0, k1) = keys.get();
keys.set((k0.wrapping_add(1), k1));
let [k0, k1] = keys.get();
keys.set([k0.wrapping_add(1), k1]);
RandomState { k0: k0, k1: k1 }
})
}
Expand Down
8 changes: 0 additions & 8 deletions src/libstd/sys/cloudabi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,3 @@ pub unsafe fn abort_internal() -> ! {
}

pub use libc::strlen;

pub fn hashmap_random_keys() -> (u64, u64) {
unsafe {
let mut v = mem::uninitialized();
libc::arc4random_buf(&mut v as *mut _ as *mut libc::c_void, mem::size_of_val(&v));
v
}
}
2 changes: 0 additions & 2 deletions src/libstd/sys/redox/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use crate::io::ErrorKind;

pub use libc::strlen;
pub use self::rand::hashmap_random_keys;

#[path = "../unix/alloc.rs"]
pub mod alloc;
Expand All @@ -23,7 +22,6 @@ pub mod os;
pub mod path;
pub mod pipe;
pub mod process;
pub mod rand;
pub mod rwlock;
pub mod stack_overflow;
pub mod stdio;
Expand Down
3 changes: 0 additions & 3 deletions src/libstd/sys/redox/rand.rs

This file was deleted.

15 changes: 0 additions & 15 deletions src/libstd/sys/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,21 +137,6 @@ pub unsafe extern "C" fn __rust_abort() {
abort_internal();
}

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

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

pub trait TryIntoInner<Inner>: Sized {
Expand Down
2 changes: 0 additions & 2 deletions src/libstd/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use crate::io::ErrorKind;
#[cfg(all(not(rustdoc), target_os = "l4re"))] pub use crate::os::linux as platform;
#[cfg(all(not(rustdoc), target_os = "hermit"))] pub use crate::os::hermit as platform;

pub use self::rand::hashmap_random_keys;
pub use libc::strlen;

#[macro_use]
Expand Down Expand Up @@ -47,7 +46,6 @@ pub mod os;
pub mod path;
pub mod pipe;
pub mod process;
pub mod rand;
pub mod rwlock;
pub mod stack_overflow;
pub mod thread;
Expand Down
176 changes: 0 additions & 176 deletions src/libstd/sys/unix/rand.rs

This file was deleted.

10 changes: 0 additions & 10 deletions src/libstd/sys/wasi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,6 @@ pub unsafe fn abort_internal() -> ! {
libc::abort()
}

pub fn hashmap_random_keys() -> (u64, u64) {
let mut ret = (0u64, 0u64);
unsafe {
let base = &mut ret as *mut (u64, u64) as *mut libc::c_void;
let len = mem::size_of_val(&ret);
cvt_wasi(libc::__wasi_random_get(base, len)).unwrap();
}
return ret
}

#[doc(hidden)]
pub trait IsMinusOne {
fn is_minus_one(&self) -> bool;
Expand Down
9 changes: 0 additions & 9 deletions src/libstd/sys/wasm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,6 @@ pub unsafe fn abort_internal() -> ! {
ExitSysCall::perform(1)
}

// We don't have randomness yet, but I totally used a random number generator to
// generate these numbers.
//
// More seriously though this is just for DOS protection in hash maps. It's ok
// if we don't do that on wasm just yet.
pub fn hashmap_random_keys() -> (u64, u64) {
(1, 2)
}

// Implement a minimal set of system calls to enable basic IO
pub enum SysCallIndex {
Read = 0,
Expand Down
2 changes: 0 additions & 2 deletions src/libstd/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use crate::path::PathBuf;
use crate::time::Duration;

pub use libc::strlen;
pub use self::rand::hashmap_random_keys;

#[macro_use] pub mod compat;

Expand All @@ -31,7 +30,6 @@ pub mod os_str;
pub mod path;
pub mod pipe;
pub mod process;
pub mod rand;
pub mod rwlock;
pub mod stack_overflow;
pub mod thread;
Expand Down
Loading