add /dev/(u)random on nt

[G4Vi]

Resolves #1001

Uses ProcessPrng as getrandom does.

When feature detection fails software often falls back to /dev/urandom, this is a problem as development with cosmo is often done on Linux where tests will pass using /dev/urandom, but then won't work on Windows. While it's easy enough to patch and using getrandom is better, this makes building correct ports with cosmopolitan/cosmocc easier.

[ahgamut]

Are there examples of software that have a config check for /dev/urandom but not for getrandom?

[G4Vi]

Are there examples of software that have a config check for /dev/urandom but not for getrandom?

It doesn't do a config check, but Perl defaults to /dev/urandom and doesn't use getrandom at all: https://github.com/Perl/perl5/blob/04d2040ce4136af8389d95e16c75aedcbfa4f9ec/util.c#L4736-L4750

WAMR does a platform config check, rather than one on the symbol:
https://github.com/bytecodealliance/wasm-micro-runtime/blob/4abf288c9431224dfef410bc972feac6dbdb2bc6/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/ssp_config.h#L27-L35
Before I added defined(__COSMOPOLITAN__), it fell back to /dev/urandom: https://github.com/bytecodealliance/wasm-micro-runtime/blob/4abf288c9431224dfef410bc972feac6dbdb2bc6/core/iwasm/libraries/libc-wasi/sandboxed-system-primitives/src/random.c#L69-L80

[mrdomino]

Conceptually I like this change, it definitely is the case that build systems will often check for /dev/urandom on the host 🙄 rather than the target machine to decide whether to make binaries that try to use it.

I have not looked closely at the PR yet and do not currently have time to; I will try to later on, if you haven't heard back from me by Friday and still need a review please ping me.

One thing is that you may want to use CryptGenRandom on Windows - I don't know if getrandom uses that or not yet. It probably should.

[G4Vi]

One thing is that you may want to use CryptGenRandom on Windows - I don't know if getrandom uses that or not yet. It probably should.

Did you mean BCryptGenRandom?, CryptGenRandom is deprecated. CryptGenRandom and RtlGenRandom are just wrappers around ProcessPrng, and I think BCryptGenRandom and ProcessPrng share a lot of the same infrastructure.

https://download.microsoft.com/download/1/c/9/1c9813b8-089c-4fef-b2ad-ad80e79403ba/Whitepaper%20-%20The%20Windows%2010%20random%20number%20generation%20infrastructure.pdf

I think ProcessPrng is sufficient as:

go upgraded from CryptGenRandom to RtlGenRandom https://go-review.googlesource.com/c/go/+/210057 and then on to ProcessPrng https://go-review.googlesource.com/c/go/+/536235.

BoringSSL only uses BCryptGenRandom on UWP as ProcessPrng is unavailable there and it may access resources unavailable in the Chromium sandbox. https://boringssl.googlesource.com/boringssl/+/master/crypto/rand_extra/windows.c

For getrandom on Rust it ends up falling back to RtlGenRandom https://github.com/rust-random/getrandom/blob/master/src/windows.rs as BCryptGenRandom can fail: rust-lang/rust#94098 for hashmap_random_keys Rust just dumped ByCryptGenRandom for ProcessPrng: https://github.com/rust-lang/rust/pull/121337/files#diff-4c3f0c2423bd724c1e578979f565930038dc67b341d779dac9fdaab91fd5e77d

[mrdomino]

Did you mean BCryptGenRandom?

Yep. I am not a Windows guy, I just remembered that function existing and CryptGenRandom was for whatever reason the first search result for whatever I queried.

In any case it sounds like ProcessPrng is the thing to use nowadays, so, great.

[mrdomino]

Added a few comments. Overall I like it and the implementation seems basically sound to me.

I would want to run it by @jart prior to merging since this is a pretty major API change.

[mrdomino]

Oh actually on closer inspection it looks like maybe you should use __getrandom out of libc/calls/syscall_support-sysv.internal.h instead of directly calling ProcessPrng. (The former is just a simple wrapper around the latter on Windows, so shouldn't have any impact.)

[G4Vi]

Oh actually on closer inspection it looks like maybe you should use __getrandom out of libc/calls/syscall_support-sysv.internal.h instead of directly calling ProcessPrng. (The former is just a simple wrapper around the latter on Windows, so shouldn't have any impact.)

This was not done, following @jart 's comments here:
#1001 (comment)

The only real concern to doing this is that getrandom() is a pretty heavy dependency for our read() function to pull in. We'd also have to implement this carefully, since getrandom() depends on read()

Actually, now that I think about it, we're only talking about Windows code, so sys_read_nt() could implement support in a very lightweight way, since it'd only need to depend on RtlGenRandom(). Accepted.

If we need /dev/(u)random emulation on other plaforms we may have to.

[mrdomino]

Hmm, okay, yeah. Using ProcessPrng directly seems fine.

Sorry, I hadn't noticed that issue link previously - in that case I'm fine with this and see no need to bug her again.

ETA: like, since you are always putting your code under a IsWindows() check, any half-decent compiler should be emitting the same machine instructions whether you use __getrandom or ProcessPrng, but I'm not willing to bet any money on that actually being the case and it seems like a small thing to just use the other function directly.

[jart]

Looks good to me.

[jart]

Nice

[mrdomino]

(I approve as well. However weirdly it doesn't seem like you can do that in the UI after something's been merged.)