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

Windows RNG: Use BCRYPT_RNG_ALG_HANDLE by default #101325

Merged
merged 1 commit into from
Sep 3, 2022
Merged
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
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ pub fn nt_success(status: NTSTATUS) -> bool {
status >= 0
}

pub const BCRYPT_USE_SYSTEM_PREFERRED_RNG: DWORD = 0x00000002;
pub const BCRYPT_RNG_ALG_HANDLE: usize = 0x81;

#[repr(C)]
pub struct UNICODE_STRING {
Expand Down
41 changes: 37 additions & 4 deletions library/std/src/sys/windows/rand.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,49 @@
//! # Random key generation
//!
//! This module wraps the RNG provided by the OS. There are a few different
//! ways to interface with the OS RNG so it's worth exploring each of the options.
//! Note that at the time of writing these all go through the (undocumented)
//! `bcryptPrimitives.dll` but they use different route to get there.
//!
//! Originally we were using [`RtlGenRandom`], however that function is
//! deprecated and warns it "may be altered or unavailable in subsequent versions".
//!
//! So we switched to [`BCryptGenRandom`] with the `BCRYPT_USE_SYSTEM_PREFERRED_RNG`
//! flag to query and find the system configured RNG. However, this change caused a small
//! but significant number of users to experience panics caused by a failure of
//! this function. See [#94098].
//!
//! The current version changes this to use the `BCRYPT_RNG_ALG_HANDLE`
//! [Pseudo-handle], which gets the default RNG algorithm without querying the
//! system preference thus hopefully avoiding the previous issue.
//! This is only supported on Windows 10+ so a fallback is used for older versions.
//!
//! [#94098]: https://github.com/rust-lang/rust/issues/94098
//! [`RtlGenRandom`]: https://docs.microsoft.com/en-us/windows/win32/api/ntsecapi/nf-ntsecapi-rtlgenrandom
//! [`BCryptGenRandom`]: https://docs.microsoft.com/en-us/windows/win32/api/bcrypt/nf-bcrypt-bcryptgenrandom
//! [Pseudo-handle]: https://docs.microsoft.com/en-us/windows/win32/seccng/cng-algorithm-pseudo-handles
use crate::io;
use crate::mem;
use crate::ptr;
use crate::sys::c;

/// Generates high quality secure random keys for use by [`HashMap`].
///
/// This is used to seed the default [`RandomState`].
///
/// [`HashMap`]: crate::collections::HashMap
/// [`RandomState`]: crate::collections::hash_map::RandomState
pub fn hashmap_random_keys() -> (u64, u64) {
let mut v = (0, 0);
let ret = unsafe {
let size = mem::size_of_val(&v).try_into().unwrap();
c::BCryptGenRandom(
ptr::null_mut(),
&mut v as *mut _ as *mut u8,
mem::size_of_val(&v) as c::ULONG,
c::BCRYPT_USE_SYSTEM_PREFERRED_RNG,
// BCRYPT_RNG_ALG_HANDLE is only supported in Windows 10+.
// So for Windows 8.1 and Windows 7 we'll need a fallback when this fails.
Copy link
Member

@thomcc thomcc Sep 2, 2022

Choose a reason for hiding this comment

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

This means we'll use RtlGenRandom on those, but that's fine because we know it won't change or go away on them (as they've already been released).

Copy link
Member Author

Choose a reason for hiding this comment

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

If I were more confident that this does indeed fix the issue some people are having, I'd be minded to remove RtlGenRandom and have a BCryptGenRandom based fallback. But the only way to really test is via Firefox's crash stats, which obviously isn't great. Maybe I'll try it in the next release cycle if I'm feeling more certain at that point.

ptr::invalid_mut(c::BCRYPT_RNG_ALG_HANDLE),
ptr::addr_of_mut!(v).cast(),
size,
0,
)
};
if ret != 0 { fallback_rng() } else { v }
Expand Down