CryptGenRandom is deprecated

[fingera]

CryptGenRandom is deprecated
https://docs.microsoft.com/en-us/windows/desktop/api/wincrypt/nf-wincrypt-cryptgenrandom

[laanwj]

so with this, it will still use the deprecated function with mingw?
(which is used for the distributed binaries)

[ken2812221]

You should add this into build_msvc/*/*.vcxproj and configure.ac instead of adding random library into source code. Once you update configure.ac, the #if defined(_MSC_VER)..#endif thing could be removed.

[ken2812221]

checking _WIN32_WINNT is not necessary. The min version required to run Bitcoin Core is Vista which is the same as the required min version of BCryptGenRandom

[fingera]

thankyou, now the minimum system is windows vista

[ken2812221]

Concept ACK

[laanwj]

utACK on superficial code changes

however, as random generation is critical to wallet security, someone with actual knowledge of windows random APIs should review this

[NicolasDorier]

I would be inclined to NACK, new version of .NET still depend on advapi32.dll CryptGenRandom for secure RNG. I would be more confident to use when microsoft eat their dog food.

[NicolasDorier]

Extract from .NET 4.7.2 released in march 2018, still using CryptGenRandom

https://referencesource.microsoft.com/#mscorlib/system/security/cryptography/rngcryptoserviceprovider.cs

https://referencesource.microsoft.com/#mscorlib/system/security/cryptography/capinative.cs,1b2672c29bebdd55

[fingera]

Windows Vista: The random number generator is based on the hash-based random number generator specified in the FIPS 186-2 standard.
Windows 8: Beginning with Windows 8, the RNG algorithm supports FIPS 186-3. Keys less than or equal to 1024 bits adhere to FIPS 186-2 and keys greater than 1024 to FIPS 186-3.
https://docs.microsoft.com/zh-cn/windows/desktop/SecCNG/cng-algorithm-identifiers#BCRYPT_RNG_ALGORITHM

ok, maybe switch to bcrypt after many years?

[luke-jr]

Could do both?

[fingera]

bitcoin random source:
// First source: OpenSSL's RNG
// Second source: OS RNG
// Third source: HW RNG, if available.

OpenSSL's RNG already append CryptGenRandom(Compile Target < Win7)
OS RNG improve it, so OS RNG use new BCryptGenRandom :)

[sipa]

We will in the near future probably remove OpenSSL as a randomness source.

[fingera]

Remove randomness source must be very carefull.
Can I do it?
What to do now?

[sipa]

No, we're still relying on some of its features that need to be implemented differently first.

I'm just pointing out that OpenSSL using some API already is not a reason to not also use it ourselves.

[fingera]

Add both os source now? Or wait for Microsoft's new API to accept more tests

[laanwj]

I'm closing this for now.
We can do this at some point in the future, but I agree with @NicolasDorier that it's not relevant now.

[fingera]

ok, please read 14116