Windows: Switch to using ProcessPRNG by default.

[josephlr]

ProcessPRNG was introduced in Windows 10, which is the minimum supported Windows for most Rust targets. Upstream change: rust-lang/rust#121337

BoringSSL now uses ProcessPRNG: https://github.com/google/boringssl/blob/2db0eb3f96a5756298dcd7f9319e56a98585bd10/crypto/rand_extra/windows.c#L64
Golang also switched to ProcessPRNG: https://go-review.googlesource.com/c/go/+/536235

Should also help with the Chromium Sandbox: https://issues.chromium.org/issues/40277768

[josephlr]

It looks like this won't be quite as easy as we would like. The main issue is that BCryptPrimitives.dll doesn't have a corresponding library (.lib file), so we can't just use link(name="bcryptprimitives"). We are left with 4 options:

1. Use #[link(kind = "raw-dylib")]
   • Only stabilized in Rust 1.71
   • Adds call to dlltool during build time (on *-windows-gnu targets)
   • Not supported by cranelift
2. Use windows-sys crate
   • Takes external dependency (but it's similar to libc so thats fine)
   • MSRV is 1.60
3. Manually write the DLL loading code
   • This is what Go does and BoringSSL does
   • Requires calling LoadLibrary and GetProcAddress
   • Synchronization needed (LazyUsize or Similar)
4. Build our own .lib file like the windows crate does
   • Allows us to avoid calling LoadLibrary and similar functions.
   • Requires maintaining libraries for the ~8 different windows targets
   • Requires figuring out how to build .lib files.

[ChrisDenton]

The fourth option would be to include your own pre-built .lib file (possibly generated by using raw-dylib and storing the output or else copying the windows-rs lib generator code).

Adds call to dlltool during build time

Note that msvc targets don't require dlltool.

[josephlr]

The fourth option would be to include your own pre-built .lib file (possibly generated by using raw-dylib and storing the output or else copying the windows-rs lib generator code).

Adds call to dlltool during build time

Note that msvc targets don't require dlltool.

Thanks! Updated the list. I do think that maintaining a collection of .lib files for each windows target is a lot more effort than just writing the ~20 lines of DLL loading code.

@ChrisDenton I have a question. If getrandom lazily calls LoadLibraryA/GetProcAddress on first use, while libstd uses raw-dylib, will this cause any issues? When experimenting, it seems like LoadLibraryA/GetProcAddress always return the same handle/address when run in the same process, so I think there's some sort of caching?

[josephlr]

@newpavlov @ChrisDenton I thinking of the following approach which would combine approaches (1) and (3).

• Add a build.rs script to check the compiler version:
  • Output cargo::rustc-cfg=raw_dylib if version >= 1.65 (See Stabilize raw-dylib for non-x86 rust-lang/rust#99916)
  • Output cargo::rustc-cfg=raw_dylib_x86 if version >= 1.71 (See Stabilize raw-dylib, link_ordinal, import_name_type and -Cdlltool rust-lang/rust#109677)
• In Windows.rs we have a cfg_if block like:

cfg_if! {
    if #[cfg(all(raw_dylib, not(target_arch = "x86")))] {
        #[link(name = "bcryptprimitives", kind = "raw-dylib")]
        extern "system" { fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL; }
    } else if #[cfg(all(raw_dylib_x86, target_arch = "x86"))] {
        #[link(name = "bcryptprimitives", kind = "raw-dylib", import_name_type = "undecorated")]
        extern "system" { fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL; }
    } else {
        unsafe fn ProcessPrng(pbdata: *mut u8, cbdata: usize) -> BOOL {
            // Do the lazy DLL loading
        }
    }
}

[ChrisDenton]

@ChrisDenton I have a question. If getrandom lazily calls LoadLibraryA/GetProcAddress on first use, while libstd uses raw-dylib, will this cause any issues? When experimenting, it seems like LoadLibraryA/GetProcAddress always return the same handle/address when run in the same process, so I think there's some sort of caching?

There's no problem with that. A DLL is only loaded once. Loading it more times indeed just returns the same module handle and increases the reference count.

[newpavlov]

@josephlr

combine approaches (1) and (3)

It would introduce a build script which will do nothing on all other targets.

We may consider delaying migration to ProcessPrng until getrandom v0.3, which can have a sufficiently high MSRV. Considering that rand is in the process of preparing new v0.9 release, it may be a good time to cut v0.3 for us as well.

[josephlr]

@newpavlov good idea. Looks like Rand 0.9's MSRV is 1.61, do you know if 1.71 would be too high?

I think regardless, we should move to ProcessPrng, and just always do the DLL loading (I can put up a draft PR to see how it looks).

[ChrisDenton]

I should add that the standard library is in general moving away from dynamic dll loading because it causes issues when used in some scenarios. E.g. attempting to load a library while the loader lock is already held can cause a deadlock or crash. This is especially important for std because HashMaps are used everywhere and, by default, use RandomState.

[josephlr]

Thanks for the heads up @ChrisDenton getrandom is also a transitive dependency of most crates in practice, so we also want to be sensitive to these concerns.

I should add that the standard library is in general moving away from dynamic dll loading because it causes issues when used in some scenarios. E.g. attempting to load a library while the loader lock is already held can cause a deadlock or crash. This is especially important for std because HashMaps are used everywhere and, by default, use RandomState.

Do you know if there is any way to call ProcessPrng on versions of Rust before 1.71 without loading a DLL? Or if there's a way to check if a DLL is already loaded (which it almost always will be) without calling LoadLibraryA?

In general, I wouldn't think that the approach taken by Go and BoringSSL would break too many things in practice, but it might just be that Google doesn't run into those specific problems. It might be the case that moving to ProcessPRNG with DLL loading isn't prefect, but fixes more problems than it creates.

Alternatively, we could just bump the MSRV to 1.71 on Windows, and have a lower MSRV on all other platforms.

[ChrisDenton]

Do you know if there is any way to call ProcessPrng on versions of Rust before 1.71 without loading a DLL?

The only way would be to use a pre-created lib file in one of the ways listed above (i.e. using the windows-targets crate or distributing a lib file with this crate).

Or if there's a way to check if a DLL is already loaded (which it almost always will be) without calling LoadLibraryA?

GetModuleHandleExA can be used to get a module that's already loaded.

In general, I wouldn't think that the approach taken by Go and BoringSSL would break too many things in practice, but it might just be that Google doesn't run into those specific problems. It might be the case that moving to ProcessPRNG with DLL loading isn't prefect, but fixes more problems than it creates.

For sure. It's fine in the vast majority of cases. It's only really TLS constructors/destructors or pre/post C main that may be an issue and only then if the DLL is not already loaded. Rust itself does not have any built-in support for TLS constructors (that's done lazily) or for running code pre/post main. However, people do use crates that implement such things. One example was someone setting up a logger when a DLL is loaded where the logger used a HashMap.

[briansmith]

I imagine many sandboxes would block LoadLibrary too. Also, some projects do try to avoid dynamic loading in general. It would be good to have a way to opt out of using LoadLibrary, e.g. by having a msrv-1_71 feature.

[briansmith]

@josephlr wrote:

@newpavlov good idea. Looks like Rand 0.9's MSRV is 1.61, do you know if 1.71 would be too high?

Many projects are sticking to MSRV <= 1.63 for the purpose of supporting Debian 8 which is out of mainstream support but which has some kind of extended (paid) support. Even some people who are infamous for their historical MSRV policies are at least trying to do so, so I imagine it is likely going to be difficult to move the MSRV beyond 1.63. I do support moving the MSRV to 1.63 though.

[newpavlov]

If we want to support Rust 1.63, then using windows-sys probably will be the best option. I would prefer to not introduce a Windows-specific feature.

Would code based on windows-sys have the same difficulties with sandboxes?