Merge #14955: Switch all RNG code to the built-in PRNG #4411

dzutto · 2021-09-10T22:11:05Z

223de8d Document RNG design in random.h (Pieter Wuille)
f2e60ca Use secure allocator for RNG state (Pieter Wuille)
cddb31b Encapsulate RNGState better (Pieter Wuille)
152146e DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
a1f252e Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
4ea8e50 Remove hwrand_initialized. (Pieter Wuille)
9d7032e Switch all RNG code to the built-in PRNG. (Pieter Wuille)
16e40a8 Integrate util/system's CInit into RNGState (Pieter Wuille)
2ccc3d3 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
aae8b9b Add thread safety annotations to RNG state (Pieter Wuille)
d3f54d1 Rename some hardware RNG related functions (Pieter Wuille)
05fde14 Automatically initialize RNG on first use. (Pieter Wuille)
2d1cc50 Don't log RandAddSeedPerfmon details (Pieter Wuille)
6a57ca9 Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille)

Pull request description:

This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state).

It includes a few policy changes (regarding what entropy is seeded when).

Before this PR:

GetRand*:
- OpenSSL
GetStrongRand*:
- CPU cycle counter
- Perfmon data (on Windows, once 10 min)
- /dev/urandom (or equivalent)
- rdrand (if available)
From scheduler when idle:
- CPU cycle counter before and after 1ms sleep
At startup:
- CPU cycle counter before and after 1ms sleep

After this PR:

GetRand*:
- Stack pointer (which indirectly identifies thread and some call stack information)
- rdrand (if available)
- CPU cycle counter
GetStrongRand*:
- Stack pointer (which indirectly identifies thread and some call stack information)
- rdrand (if available)
- CPU cycle counter
- /dev/urandom (or equivalent)
- OpenSSL
- CPU cycle counter again
From scheduler when idle:
- Stack pointer (which indirectly identifies thread and some call stack information)
- rdrand (if available)
- CPU cycle counter before and after 1ms sleep
- Perfmon data (on Windows, once every 10 min)
At startup:
- Stack pointer (which indirectly identifies thread and some call stack information)
- rdrand (if available)
- CPU cycle counter
- /dev/urandom (or equivalent)
- OpenSSL
- CPU cycle counter again
- Perfmon data (on Windows, once every 10 min)

The interface of random.h is also simplified, and documentation is added.

This implements most of bitcoin#14623.

Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20

223de8d Document RNG design in random.h (Pieter Wuille) f2e60ca Use secure allocator for RNG state (Pieter Wuille) cddb31b Encapsulate RNGState better (Pieter Wuille) 152146e DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille) a1f252e Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille) 4ea8e50 Remove hwrand_initialized. (Pieter Wuille) 9d7032e Switch all RNG code to the built-in PRNG. (Pieter Wuille) 16e40a8 Integrate util/system's CInit into RNGState (Pieter Wuille) 2ccc3d3 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille) aae8b9b Add thread safety annotations to RNG state (Pieter Wuille) d3f54d1 Rename some hardware RNG related functions (Pieter Wuille) 05fde14 Automatically initialize RNG on first use. (Pieter Wuille) 2d1cc50 Don't log RandAddSeedPerfmon details (Pieter Wuille) 6a57ca9 Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille) Pull request description: This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state). It includes a few policy changes (regarding what entropy is seeded when). Before this PR: * GetRand*: * OpenSSL * GetStrongRand*: * CPU cycle counter * Perfmon data (on Windows, once 10 min) * /dev/urandom (or equivalent) * rdrand (if available) * From scheduler when idle: * CPU cycle counter before and after 1ms sleep * At startup: * CPU cycle counter before and after 1ms sleep After this PR: * GetRand*: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * GetStrongRand*: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * /dev/urandom (or equivalent) * OpenSSL * CPU cycle counter again * From scheduler when idle: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter before and after 1ms sleep * Perfmon data (on Windows, once every 10 min) * At startup: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * /dev/urandom (or equivalent) * OpenSSL * CPU cycle counter again * Perfmon data (on Windows, once every 10 min) The interface of random.h is also simplified, and documentation is added. This implements most of bitcoin#14623. Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20

PastaPastaPasta · 2021-09-11T04:21:45Z

src/random.cpp

+/* A note on the use of noexcept in the seeding functions below:
+ *
+ * None of the RNG code should ever throw any exception.
+ */


Why is this comment different??

Original patch used MilliSleep() that could throw, and the comment explained that and why related functions were not marked as noexcept.
Later that code has been changed to use UninterruptibleSleep() instead which does not throw.
Our code already has UninterruptibleSleep() instead of MilliSleep as of cbc4a74 (#4229) and so I used it here right away.
In later revisions of Bitcoin that use UninterruptibleSleep() too, the comment looks exactly like above.

PastaPastaPasta · 2021-09-11T04:23:55Z

src/random.cpp

-    // Combine with and update state
-    {
-        WAIT_LOCK(cs_rng_state, lock);
-        hasher.Write(rng_state, sizeof(rng_state));
-        hasher.Write((const unsigned char*)&rng_counter, sizeof(rng_counter));
-        ++rng_counter;
-        hasher.Finalize(buf);
-        memcpy(rng_state, buf + 32, 32);
-    }


@UdjinM6 noting our difference with upstream

Do you mean some other fixes are to be backported? This specific block applied cleanly on cherry-picking as far as I remember, no conflicts in these lines.

@PastaPastaPasta can you clarify pls? I don't see any

Upstream had https://github.com/bitcoin/bitcoin/pull/14955/files#diff-433f996b8149d6d6831ab1ee2978d4cc13dead5721c824175db806e4284ab15bL325-L328 instead of this, and I was just pointing out the difference

https://github.com/dashpay/dash/pull/4411/files/4cbecd4510a62910519725aee8633d8b0166546e#diff-433f996b8149d6d6831ab1ee2978d4cc13dead5721c824175db806e4284ab15bL331-L334 👀 there is some confusion caused by diffs being displayed differently probably cause the code is the same as far as I can tell

Sounds good, just wanted to point it out to make sure we weren't missing smth

PastaPastaPasta

utACK for squash merge, LGTM

UdjinM6

utACK

…y#4411) 223de8d Document RNG design in random.h (Pieter Wuille) f2e60ca Use secure allocator for RNG state (Pieter Wuille) cddb31b Encapsulate RNGState better (Pieter Wuille) 152146e DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille) a1f252e Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille) 4ea8e50 Remove hwrand_initialized. (Pieter Wuille) 9d7032e Switch all RNG code to the built-in PRNG. (Pieter Wuille) 16e40a8 Integrate util/system's CInit into RNGState (Pieter Wuille) 2ccc3d3 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille) aae8b9b Add thread safety annotations to RNG state (Pieter Wuille) d3f54d1 Rename some hardware RNG related functions (Pieter Wuille) 05fde14 Automatically initialize RNG on first use. (Pieter Wuille) 2d1cc50 Don't log RandAddSeedPerfmon details (Pieter Wuille) 6a57ca9 Use FRC::randbytes instead of reading >32 bytes from RNG (Pieter Wuille) Pull request description: This does not remove OpenSSL, but makes our own PRNG the 'main' one; for GetStrongRandBytes, the OpenSSL RNG is still used (indirectly, by feeding its output into our PRNG state). It includes a few policy changes (regarding what entropy is seeded when). Before this PR: * GetRand*: * OpenSSL * GetStrongRand*: * CPU cycle counter * Perfmon data (on Windows, once 10 min) * /dev/urandom (or equivalent) * rdrand (if available) * From scheduler when idle: * CPU cycle counter before and after 1ms sleep * At startup: * CPU cycle counter before and after 1ms sleep After this PR: * GetRand*: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * GetStrongRand*: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * /dev/urandom (or equivalent) * OpenSSL * CPU cycle counter again * From scheduler when idle: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter before and after 1ms sleep * Perfmon data (on Windows, once every 10 min) * At startup: * Stack pointer (which indirectly identifies thread and some call stack information) * rdrand (if available) * CPU cycle counter * /dev/urandom (or equivalent) * OpenSSL * CPU cycle counter again * Perfmon data (on Windows, once every 10 min) The interface of random.h is also simplified, and documentation is added. This implements most of bitcoin#14623. Tree-SHA512: 0120e19bd4ce80a509b5c180a4f29497d299ce8242e25755880851344b825bc2d64a222bc245e659562fb5463fb7c70fbfcf003616be4dc59d0ed6534f93dd20 Co-authored-by: Wladimir J. van der Laan <[email protected]>

PastaPastaPasta reviewed Sep 11, 2021

View reviewed changes

PastaPastaPasta added this to the 18 milestone Sep 11, 2021

PastaPastaPasta approved these changes Sep 11, 2021

View reviewed changes

UdjinM6 approved these changes Sep 11, 2021

View reviewed changes

UdjinM6 merged commit be79070 into dashpay:develop Sep 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge #14955: Switch all RNG code to the built-in PRNG #4411

Merge #14955: Switch all RNG code to the built-in PRNG #4411

Uh oh!

dzutto commented Sep 10, 2021

Uh oh!

PastaPastaPasta Sep 11, 2021

Uh oh!

dzutto Sep 11, 2021

Uh oh!

PastaPastaPasta Sep 11, 2021

Uh oh!

dzutto Sep 11, 2021

Uh oh!

UdjinM6 Sep 11, 2021

Uh oh!

PastaPastaPasta Sep 11, 2021

Uh oh!

UdjinM6 Sep 11, 2021

Uh oh!

PastaPastaPasta Sep 11, 2021

Uh oh!

PastaPastaPasta left a comment

Uh oh!

UdjinM6 left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Merge #14955: Switch all RNG code to the built-in PRNG #4411

Merge #14955: Switch all RNG code to the built-in PRNG #4411

Uh oh!

Conversation

dzutto commented Sep 10, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants