From 0c92519d01b30996594cc16832fc52f0702a4855 Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Mon, 9 May 2022 12:04:53 -0400 Subject: [PATCH 1/3] Make HashMap fall back to RtlGenRandom if BCryptGenRandom fails Issue #84096 changed the hashmap RNG to use BCryptGenRandom instead of RtlGenRandom on Windows. Mozilla Firefox started experiencing random failures in env_logger::Builder::new() (Issue #94098) during initialization of their unsandboxed main process with an "Access Denied" error message from BCryptGenRandom(), which is used by the HashMap contained in env_logger::Builder The root cause appears to be a virus scanner or other software interfering with BCrypt DLLs loading. This change adds a fallback option if BCryptGenRandom is unusable for whatever reason. It will fallback to RtlGenRandom in this case. Fixes #94098 --- library/std/src/sys/windows/c.rs | 4 ++ library/std/src/sys/windows/rand.rs | 83 +++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/windows/c.rs b/library/std/src/sys/windows/c.rs index 0692da1d79519..0bb6fee60c92e 100644 --- a/library/std/src/sys/windows/c.rs +++ b/library/std/src/sys/windows/c.rs @@ -788,6 +788,10 @@ if #[cfg(not(target_vendor = "uwp"))] { #[link(name = "advapi32")] extern "system" { + // Forbidden when targeting UWP + #[link_name = "SystemFunction036"] + pub fn RtlGenRandom(RandomBuffer: *mut u8, RandomBufferLength: ULONG) -> BOOLEAN; + // Allowed but unused by UWP pub fn OpenProcessToken( ProcessHandle: HANDLE, diff --git a/library/std/src/sys/windows/rand.rs b/library/std/src/sys/windows/rand.rs index de73e9154b45e..ec6c40d2f4978 100644 --- a/library/std/src/sys/windows/rand.rs +++ b/library/std/src/sys/windows/rand.rs @@ -1,8 +1,69 @@ use crate::io; use crate::mem; +use crate::sync; use crate::sys::c; +// The kinds of HashMap RNG that may be available +#[derive(Clone, Copy, Debug, PartialEq)] +enum HashMapRng { + Preferred, + Fallback, +} + pub fn hashmap_random_keys() -> (u64, u64) { + match get_hashmap_rng() { + HashMapRng::Preferred => { + preferred_rng().expect("couldn't generate random bytes with preferred RNG") + } + HashMapRng::Fallback => { + fallback_rng().unwrap().expect("couldn't generate random bytes with fallback RNG") + } + } +} + +// Returns the HashMap RNG that should be used +// +// Panics if they are both broken +fn get_hashmap_rng() -> HashMapRng { + // Assume that if the preferred RNG is broken the first time we use it, it likely means + // that: the DLL has failed to load, there is no point to calling it over-and-over again, + // and we should cache the result + static INIT: sync::Once = sync::Once::new(); + static mut HASHMAP_RNG: HashMapRng = HashMapRng::Preferred; + + unsafe { + INIT.call_once(|| HASHMAP_RNG = choose_hashmap_rng()); + HASHMAP_RNG + } +} + +// Test whether we should use the preferred or fallback RNG +// +// If the preferred RNG is successful, we choose it. Otherwise, if the fallback RNG is successful, +// we choose that +// +// Panics if both the preferred and the fallback RNG are both non-functional +fn choose_hashmap_rng() -> HashMapRng { + let preferred_error = match preferred_rng() { + Ok(_) => return HashMapRng::Preferred, + Err(e) => e, + }; + + // On UWP, there is no fallback + let fallback_result = fallback_rng() + .unwrap_or_else(|| panic!("preferred RNG broken: `{}`, no fallback", preferred_error)); + + match fallback_result { + Ok(_) => return HashMapRng::Fallback, + Err(fallback_error) => panic!( + "preferred RNG broken: `{}`, fallback RNG broken: `{}`", + preferred_error, fallback_error + ), + } +} + +// Generate random numbers using the preferred RNG function (BCryptGenRandom) +fn preferred_rng() -> Result<(u64, u64), io::Error> { use crate::ptr; let mut v = (0, 0); @@ -14,8 +75,22 @@ pub fn hashmap_random_keys() -> (u64, u64) { c::BCRYPT_USE_SYSTEM_PREFERRED_RNG, ) }; - if ret != 0 { - panic!("couldn't generate random bytes: {}", io::Error::last_os_error()); - } - return v; + + if ret == 0 { Ok(v) } else { Err(io::Error::last_os_error()) } +} + +// Generate random numbers using the fallback RNG function (RtlGenRandom) +#[cfg(not(target_vendor = "uwp"))] +fn fallback_rng() -> Option> { + let mut v = (0, 0); + let ret = + unsafe { c::RtlGenRandom(&mut v as *mut _ as *mut u8, mem::size_of_val(&v) as c::ULONG) }; + + Some(if ret != 0 { Ok(v) } else { Err(io::Error::last_os_error()) }) +} + +// We can't use RtlGenRandom with UWP, so there is no fallback +#[cfg(target_vendor = "uwp")] +fn fallback_rng() -> Option> { + None } From 3de6c2ca33f45b283ebb177bb29c0c756c6b75cb Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Fri, 13 May 2022 18:14:03 -0400 Subject: [PATCH 2/3] Address review feedback --- library/std/src/sys/windows/rand.rs | 53 ++++++++++++----------------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/library/std/src/sys/windows/rand.rs b/library/std/src/sys/windows/rand.rs index ec6c40d2f4978..836fc14898600 100644 --- a/library/std/src/sys/windows/rand.rs +++ b/library/std/src/sys/windows/rand.rs @@ -1,9 +1,9 @@ use crate::io; +use crate::lazy; use crate::mem; -use crate::sync; use crate::sys::c; -// The kinds of HashMap RNG that may be available +/// The kinds of HashMap RNG that may be available #[derive(Clone, Copy, Debug, PartialEq)] enum HashMapRng { Preferred, @@ -16,44 +16,35 @@ pub fn hashmap_random_keys() -> (u64, u64) { preferred_rng().expect("couldn't generate random bytes with preferred RNG") } HashMapRng::Fallback => { - fallback_rng().unwrap().expect("couldn't generate random bytes with fallback RNG") + fallback_rng().expect("couldn't generate random bytes with fallback RNG") } } } -// Returns the HashMap RNG that should be used -// -// Panics if they are both broken +/// Returns the HashMap RNG that should be used +/// +/// Panics if they are both broken fn get_hashmap_rng() -> HashMapRng { // Assume that if the preferred RNG is broken the first time we use it, it likely means // that: the DLL has failed to load, there is no point to calling it over-and-over again, // and we should cache the result - static INIT: sync::Once = sync::Once::new(); - static mut HASHMAP_RNG: HashMapRng = HashMapRng::Preferred; - - unsafe { - INIT.call_once(|| HASHMAP_RNG = choose_hashmap_rng()); - HASHMAP_RNG - } + static VALUE: lazy::SyncOnceCell = lazy::SyncOnceCell::new(); + *VALUE.get_or_init(choose_hashmap_rng) } -// Test whether we should use the preferred or fallback RNG -// -// If the preferred RNG is successful, we choose it. Otherwise, if the fallback RNG is successful, -// we choose that -// -// Panics if both the preferred and the fallback RNG are both non-functional +/// Test whether we should use the preferred or fallback RNG +/// +/// If the preferred RNG is successful, we choose it. Otherwise, if the fallback RNG is successful, +/// we choose that +/// +/// Panics if both the preferred and the fallback RNG are both non-functional fn choose_hashmap_rng() -> HashMapRng { let preferred_error = match preferred_rng() { Ok(_) => return HashMapRng::Preferred, Err(e) => e, }; - // On UWP, there is no fallback - let fallback_result = fallback_rng() - .unwrap_or_else(|| panic!("preferred RNG broken: `{}`, no fallback", preferred_error)); - - match fallback_result { + match fallback_rng() { Ok(_) => return HashMapRng::Fallback, Err(fallback_error) => panic!( "preferred RNG broken: `{}`, fallback RNG broken: `{}`", @@ -62,7 +53,7 @@ fn choose_hashmap_rng() -> HashMapRng { } } -// Generate random numbers using the preferred RNG function (BCryptGenRandom) +/// Generate random numbers using the preferred RNG function (BCryptGenRandom) fn preferred_rng() -> Result<(u64, u64), io::Error> { use crate::ptr; @@ -79,18 +70,18 @@ fn preferred_rng() -> Result<(u64, u64), io::Error> { if ret == 0 { Ok(v) } else { Err(io::Error::last_os_error()) } } -// Generate random numbers using the fallback RNG function (RtlGenRandom) +/// Generate random numbers using the fallback RNG function (RtlGenRandom) #[cfg(not(target_vendor = "uwp"))] -fn fallback_rng() -> Option> { +fn fallback_rng() -> Result<(u64, u64), io::Error> { let mut v = (0, 0); let ret = unsafe { c::RtlGenRandom(&mut v as *mut _ as *mut u8, mem::size_of_val(&v) as c::ULONG) }; - Some(if ret != 0 { Ok(v) } else { Err(io::Error::last_os_error()) }) + if ret != 0 { Ok(v) } else { Err(io::Error::last_os_error()) } } -// We can't use RtlGenRandom with UWP, so there is no fallback +/// We can't use RtlGenRandom with UWP, so there is no fallback #[cfg(target_vendor = "uwp")] -fn fallback_rng() -> Option> { - None +fn fallback_rng() -> Result<(u64, u64), io::Error> { + Err(io::const_io_error!(io::ErrorKind::Unsupported, "unsupported on UWP")) } From aba3454aa136bce4f65978e5bf50683380fbddf3 Mon Sep 17 00:00:00 2001 From: Chris Martin Date: Mon, 16 May 2022 13:49:12 -0400 Subject: [PATCH 3/3] Improve error message for fallback RNG failure --- library/std/src/sys/windows/rand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/windows/rand.rs b/library/std/src/sys/windows/rand.rs index 836fc14898600..22e024d8552ec 100644 --- a/library/std/src/sys/windows/rand.rs +++ b/library/std/src/sys/windows/rand.rs @@ -83,5 +83,5 @@ fn fallback_rng() -> Result<(u64, u64), io::Error> { /// We can't use RtlGenRandom with UWP, so there is no fallback #[cfg(target_vendor = "uwp")] fn fallback_rng() -> Result<(u64, u64), io::Error> { - Err(io::const_io_error!(io::ErrorKind::Unsupported, "unsupported on UWP")) + Err(io::const_io_error!(io::ErrorKind::Unsupported, "RtlGenRandom() not supported on UWP")) }