Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RNG strengthening (10ms once every minute) #15224

Merged
merged 2 commits into from
May 18, 2019

Conversation

sipa
Copy link
Member

@sipa sipa commented Jan 21, 2019

This patch improves the built-in RNG using hash strengthening.

At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

@hebasto
Copy link
Member

hebasto commented Jan 21, 2019

Is there a rationale for values "10ms" and "1000 iterations"?

@sipa sipa force-pushed the 201901_rand_strengthen branch from f504541 to e20422a Compare January 21, 2019 21:59
@sipa
Copy link
Member Author

sipa commented Jan 21, 2019

@hebasto Sort of.

10ms as it's a amount that shouldn't cause any measurable latency in the background thread (where generally things run that aren't very time critical anyway).

1000 seems a nice trade-off between time spent on getting the time, and getting entropy out. If we'd get the clock every iteration, we'd be spending more time in reading the clock than actually strenghtening anything. If we'd only get it every 1000000 iterations, we'd probably only read the clock once and not get much entropy from that.

@hebasto
Copy link
Member

hebasto commented Jan 21, 2019

Concept ACK.

@practicalswift
Copy link
Contributor

@sipa What would be the appropriate way to test/measure the improvement this change brings?

src/Makefile.am Outdated Show resolved Hide resolved
Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK, lightly reviewed e20422a. Not seeing any issues except what @laanwj brought up.

src/random.cpp Outdated Show resolved Hide resolved
src/random.cpp Outdated Show resolved Hide resolved
@sipa sipa force-pushed the 201901_rand_strengthen branch from e20422a to 78681aa Compare January 23, 2019 19:20
src/random.cpp Outdated Show resolved Hide resolved
@gmaxwell
Copy link
Contributor

I'd still prefer to see this using 4x SHA256^2-64b since that function has a much better ratio of current performance to attacker performance, but I grok the startup time interactions with function auto-detection make that messy. I'm okay with using sha512 for now in light of that!

@sipa sipa force-pushed the 201901_rand_strengthen branch from 78681aa to f20fbec Compare January 24, 2019 19:47
@gmaxwell
Copy link
Contributor

We should contemplate increasing the strenghtening time on the first run. Rationale: usually long term private keys will get generated within one minute of first startup when only the startup strengthening will have applied. Adding an extra 90ms on startup won't be significantly noticeable on top of the existing startup time, and would increase resistance to search by 10x. Alternative: Once we've started carrying randomness across restarts in a file, a prolonged startup strengthen would only be needed when there is no saved randomness.

src/random.cpp Outdated Show resolved Hide resolved
src/random.cpp Outdated Show resolved Hide resolved
@laanwj laanwj added this to the 0.19.0 milestone Feb 16, 2019
@sipa sipa force-pushed the 201901_rand_strengthen branch 2 times, most recently from e7a718c to c5b3659 Compare May 3, 2019 23:00
@sipa
Copy link
Member Author

sipa commented May 3, 2019

Adressed @Empact's nits, and made the first strengthening run 100ms long. Also fixed a bug where it would run every 60 ms instead of every minute...

@gmaxwell
Copy link
Contributor

gmaxwell commented May 6, 2019

utACK

src/random.cpp Outdated Show resolved Hide resolved
// Produce output from inner state and feed it to outer hasher.
inner_hasher.Finalize(buffer);
hasher.Write(buffer, sizeof(buffer));
// Try to clean up.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why "try"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The C++ language has no way of guaranteeing memory gets cleaned up (it may have copied it elsewhere in memory, for starters, or detect that the clearing has no observable effect and optimize it away).

sipa added 2 commits May 6, 2019 15:15
Once every minute, this will feed the RNG state through repeated SHA512
for 10ms. The timings of that operation are used as entropy source as
well.
@sipa sipa force-pushed the 201901_rand_strengthen branch from c5b3659 to 3cb9ce8 Compare May 6, 2019 22:15
@pstratem
Copy link
Contributor

utACK 3cb9ce8

@laanwj laanwj merged commit 3cb9ce8 into bitcoin:master May 18, 2019
laanwj added a commit that referenced this pull request May 18, 2019
3cb9ce8 Document strenghtening (Pieter Wuille)
1d207bc Add hash strengthening to the RNG (Pieter Wuille)

Pull request description:

  This patch improves the built-in RNG using hash strengthening.

  At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

ACKs for commit 3cb9ce:
  pstratem:
    utACK 3cb9ce8

Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 18, 2019
3cb9ce8 Document strenghtening (Pieter Wuille)
1d207bc Add hash strengthening to the RNG (Pieter Wuille)

Pull request description:

  This patch improves the built-in RNG using hash strengthening.

  At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

ACKs for commit 3cb9ce:
  pstratem:
    utACK 3cb9ce8

Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
@maflcko
Copy link
Member

maflcko commented May 20, 2019

@sipa
Copy link
Member Author

sipa commented May 20, 2019

@MarcoFalke Interesting. Is that considered too much? We could disable the strengthening time, or make it configurable... though at the cost of some complexity (the RNG may be used before command line options are parsed...)

@maflcko
Copy link
Member

maflcko commented May 21, 2019

Not really. Just mentioned because it might be interesting to some.

deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 19, 2020
Summary:
```
This patch improves the built-in RNG using hash strengthening.

At startup, and once every minute, 32 bytes of entropy are produced from
the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into
the RNG, together with high-precision timestamps obtained every 1000
iterations.
```

Backport of core [[bitcoin/bitcoin#15224 | PR15224]].

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6139
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Apr 14, 2021
cecbf6c Use secure.h header for secure allocators (Fuzzbawls)
d9f67da net: add ifaddrs.h include (fanquake)
e906436 build: check if -lsocket is required with *ifaddrs (fanquake)
414f405 rand: only try and use freeifaddrs if available (fanquake)
3a039d6 build: avoid getifaddrs when unavailable (Cory Fields)
77bddd7 Use GetStrongRandBytes in gmp bignum initialization (Fuzzbawls)
b70b26f Fix typo in comment in randomenv.cpp (Fuzzbawls)
fec460c Put bounds on the number of CPUID leaves explored (Pieter Wuille)
41ab1ff Fix CPUID subleaf iteration (Pieter Wuille)
8a9bbb1 Move events_hasher into RNGState() (Pieter Wuille)
88c2ae5 random: mark RandAddPeriodic and SeedPeriodic as noexcept (fanquake)
81d382f doc: correct random.h docs after bitcoin#17270 (fanquake)
f363ea9 Seed RNG with precision timestamps on receipt of net messages. (Matt Corallo)
7d6ddcb Run background seeding periodically instead of unpredictably (Pieter Wuille)
4679181 Add information gathered through getauxval() (Pieter Wuille)
88d97d0 Feed CPUID data into RNG (Pieter Wuille)
8f5b9c9 Use sysctl for seeding on MacOS/BSD (Pieter Wuille)
67de246 Gather additional entropy from the environment (Pieter Wuille)
6142e1f Seed randomness with process id / thread id / various clocks (Pieter Wuille)
7bde8b7 [MOVEONLY] Move cpuid code from random to compat/cpuid (Fuzzbawls)
52b5336 [MOVEONLY] Move perfmon data gathering to new randomenv module (Pieter Wuille)
27cf995 doc: minor corrections in random.cpp (fanquake)
fccd2b8 doc: correct function name in ReportHardwareRand() (fanquake)
909473e Fix FreeBSD build by including utilstrencodings.h (Fuzzbawls)
630931f break circular dependency: random/sync -> util -> random/sync (Fuzzbawls)
5eed08c random: remove call to RAND_screen() (Windows only) (fanquake)
ada9868 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake)
22a7121 Fix non-deterministic coverage of test DoS_mapOrphans (Fuzzbawls)
79e7fd3 Add ChaCha20 bench (Jonas Schnelli)
6966aa9 Add ChaCha20 encryption option (XOR) (Jonas Schnelli)
28c9cdb tests: Add script checking for deterministic line coverage (practicalswift)
c82e359 test: Make bloom tests deterministic (MarcoFalke)
7b33223 Document strenghtening (Pieter Wuille)
0190dec Add hash strengthening to the RNG (Pieter Wuille)
67e336d Use RdSeed when available, and reduce RdRand load (Pieter Wuille)
4ffda1f Document RNG design in random.h (Pieter Wuille)
2b6381e Use secure allocator for RNG state (Pieter Wuille)
080deb3 Encapsulate RNGState better (Pieter Wuille)
787d72f DRY: Implement GetRand using FastRandomContext::randrange (Pieter Wuille)
5bc2583 Sprinkle some sweet noexcepts over the RNG code (Pieter Wuille)
774899f Remove hwrand_initialized. (Pieter Wuille)
698d133 Switch all RNG code to the built-in PRNG. (Pieter Wuille)
038a45a Integrate util/system's CInit into RNGState (Fuzzbawls)
5f20e62 Abstract out seeding/extracting entropy into RNGState::MixExtract (Pieter Wuille)
298f97c Add thread safety annotations to RNG state (Pieter Wuille)
2326535 Rename some hardware RNG related functions (Pieter Wuille)
d76ee83 Automatically initialize RNG on first use. (Pieter Wuille)
1a5dbc5 Don't log RandAddSeedPerfmon details (Pieter Wuille)
32e6c42 Simplify testing RNG code (Fuzzbawls)
972effa Make unit tests use the insecure_rand_ctx exclusively (Fuzzbawls)
af52bf5 Use a FastRandomContext in LimitOrphanTxSize (Fuzzbawls)
746d466 Introduce a Shuffle for FastRandomContext and use it in wallet (Fuzzbawls)
1cdf124 Use a local FastRandomContext in a few more places in net (Fuzzbawls)
e862564 Make addrman use its local RNG exclusively (Fuzzbawls)
94b2ead Make FastRandomContext support standard C++11 RNG interface (Pieter Wuille)

Pull request description:

  This is a collection of upstream PRs that have been backported to bring our RNG (`src/random`) code more up-to-date. The following upstream PRs have been included here:

  - bitcoin#12742
  - bitcoin#14624
    - some of this had already been merged previously
  - bitcoin#14955
  - bitcoin#15250
  - bitcoin#15224
  - bitcoin#15324
  - bitcoin#15296
  - bitcoin#15512
  - bitcoin#16878
  - bitcoin#17151
  - bitcoin#17191
  - bitcoin#13236
  - bitcoin#13314
  - bitcoin#17169
  - bitcoin#17270
    -  omitted last commit as our testing framework doesn't support it currently
    - omitted bitcoin@64e1e02, to be pulled in after our time utility is updated in a separate PR
  - bitcoin#17573
  - bitcoin#17507
  - bitcoin#17670
  - bitcoin#17527
  - bitcoin#14127
  - bitcoin#21486

ACKs for top commit:
  furszy:
    ACK cecbf6c with a minor nit that can be easily tackled later.
  random-zebra:
    rebase utACK cecbf6c and merging...

Tree-SHA512: 3463b693cc9bddc1ec15228d264a794f5c2f159073fafa2ccf6e2563abfeb4369e49505f97ca84f2478ca792bd07b66d2cd83c58044d6a0cae6af42d22f5784b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 14, 2021
3cb9ce8 Document strenghtening (Pieter Wuille)
1d207bc Add hash strengthening to the RNG (Pieter Wuille)

Pull request description:

  This patch improves the built-in RNG using hash strengthening.

  At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

ACKs for commit 3cb9ce:
  pstratem:
    utACK 3cb9ce8

Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 14, 2021
3cb9ce8 Document strenghtening (Pieter Wuille)
1d207bc Add hash strengthening to the RNG (Pieter Wuille)

Pull request description:

  This patch improves the built-in RNG using hash strengthening.

  At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

ACKs for commit 3cb9ce:
  pstratem:
    utACK 3cb9ce8

Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 15, 2021
3cb9ce8 Document strenghtening (Pieter Wuille)
1d207bc Add hash strengthening to the RNG (Pieter Wuille)

Pull request description:

  This patch improves the built-in RNG using hash strengthening.

  At startup, and once every minute, 32 bytes of entropy are produced from the RNG, repeatedly hashed using SHA512 for 10ms, and then fed back into the RNG, together with high-precision timestamps obtained every 1000 iterations.

ACKs for commit 3cb9ce:
  pstratem:
    utACK 3cb9ce8

Tree-SHA512: 4fb6f61639b392697beb81c5f0903f79f10dd1087bed7f34de2abb5c22704a671e37b2d828ed141492491863efb1e7d1fa04408a1d32c9de2f2cc8ac406bbe57
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.