Windows: RtlGenRandom vs BCryptGenRandom about getrandom HOT 11 CLOSED

So it looks like Go's final decision was based on three main sources:

• What we (Rust) did
• What BoringSSL does (this then determines what Chrome does)
• What Firefox does

They didn't really provide their own independent reasoning for preferring one over another. We're changing what Rust does, so the main arguments in favor of RtlGenRandom come from this BoringSSL thread and this BoringSSL CL where the BoringSSL folks decided to not adopt BCryptGenRandom as the universal way to get randomness on Windows.

The main downside is that BCryptGenRandom (on its very first call with BCRYPT_USE_SYSTEM_PREFERRED_RNG) might read some preferences from the Windows registry, which is not allowed inside the Chromium sandbox.

Both RtlGenRandom and BCryptGenRandom will be slow on their first call:

• RtlGenRandom (part of advapi32.dll) has to load bcrypt.dll which contains the Windows cryptographic code
• BCryptGenRandom is part of bcrypt.dll, so no additional DLL loading is necessary, but it has to find the "system preferred" RNG, which might require reading values from the windows registry

Windows 10 introduced CNG Algorithm Pseudo-handles to avoid these issues, which would allow us to invoke BCryptGenRandom like:

BCryptGenRandom(
    BCRYPT_RNG_ALG_HANDLE,
    chunk.as_mut_ptr(),
    chunk.len() as u32,
    0,
)

but that (maybe) only works on Windows 10. In all cases, performance was basically the same (as they all call the same bcrypt-internal crypto code).

My decision would be to switch to BCryptGenRandom. The advantages of avoiding advapi32.dll and having only one implementation outweigh the very Chromium-specific sandboxing concerns.

If either sandboxing or performance ever become an issue (unlikely), I think we should just try to invoke BCryptGenRandom with BCRYPT_RNG_ALG_HANDLE first, before falling back to invoking it with BCRYPT_USE_SYSTEM_PREFERRED_RNG.

from getrandom.

saw this thread linked from the golang issue.

My decision would be to switch to BCryptGenRandom

that is a very wise decision, because if one day a user decides to use the rust "crypto" wrappers and asks you "what is the underlying algorithm implementation behind RtlGenRandom and is it compliant with standard X?", only a Microsoft employee would be able to answer for sure and there is no documented guarantee whether such an implementation detail can change for RtlGenRandom in Windows-next without any notice.

from getrandom.

I believe XP is still technically supported by std (https://forge.rust-lang.org/platform-support.html#tier-3) and that's why it needs both implementations. When XP support is officially dropped then the RtlGenRandom version can be removed and BCryptGenRandom can be used for all Windows targets. I suggest that getrandom just copies std for now.

from getrandom.

Right after the final decision will be made? IIRC Rust Windows XP support was arguably broken for a long time, so it should not be a problem. I also think that ideally we should do it before releasing rand v0.8.

from getrandom.

Hi,

Mostly because I was asked to :) rust-lang/rust#60260 (comment)

My understanding is that Microsoft advises to use CryptGenRandom over RtlGenRandom, but CryptGenRandom is deprecated and the "next generation cryptography API" should be used instead, and BCryptGenRandom is part of that API, so if you have the choice, BcryptGenRandom is probably to be favored, however it seems in the context of Rust, there are some historical reasons for keeping RtlGenRandom

In the case of UWP the choice is easy, you can't use RtlGenRandom

from getrandom.

Ah, indeed. In that case I think we can use RtlGenRandom fallback only for XP targets, and BCryptGenRandom for all other Windows targets.

UPD: Never mind, we can't distinguish between XP and later versions by using only target triplet.

Relevant discussion: https://internals.rust-lang.org/t/8745

from getrandom.

Relevant issue: openssl/openssl#8644

So we will have to wait for dropping of Windows XP (and hopefully Vista) from the list of supported Rust targets, until then we will have to keep using RtlGenRandom.

from getrandom.

I'd recommend using git blame and doing some digging to learn about the rationale for the usage in Rust's standard library. Doing that, for example, leads to rust-lang/rust#45370 which has a lot more information about why things are the way they are.

from getrandom.

The compiler team considers dropping Windows XP support, assuming it will be done, I think we can drop its support in getrandom v0.2 as well.

from getrandom.

SGTM, how long after they drop support should we update the library? Given that rand 0.8 isn't out yet, I'd be fine doing it sooner.

from getrandom.

In case you're interested, Go had some discussion on this and decided to stick with RtlGenRandom for non-UWP: golang/go#33542

from getrandom.