-
Notifications
You must be signed in to change notification settings - Fork 725
Utils and libraries: back porting bitcoin RNG improvement #576
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
Conversation
|
Found the PR for their memory cleanse method: bitcoin#11196 |
|
utACK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK, However the changes from bitcoin#11196 must be added. We don't want to add more dependencies on OpenSSL.
I'd also like to try to rebase it on top of #549 and check for incompatibilities and see if compilation goes flawlessly.
|
Should we close this? Since it's inactive and continued in #643. |
|
Continued in #643 |
b7dda92 [Log] Replace a string by the function name in a log (warrows) 977f089 [Refactor] Use arrays instead of unic vars in Chacha20 (warrows) d8abe32 [Random] Add a missing include (warrows) 27663b8 Do not permit copying FastRandomContexts (Pieter Wuille) 64e03e6 Bugfix: randbytes should seed when needed (non reachable issue) (Pieter Wuille) e8f12aa Check if sys/random.h is required for getentropy on OSX. (James Hilliard) de85c7a Add attribute [[noreturn]] (C++11) to functions that will not return (practicalswift) df46c7f Fix resource leak (Dag Robole) d426d85 Clarify entropy source (Pieter Wuille) 30a320b Use cpuid intrinsics instead of asm code (Pieter Wuille) 0c21204 random: fix crash on some 64bit platforms (Cory Fields) b8bbb9c Use rdrand as entropy source on supported platforms (Pieter Wuille) 8e19443 [Tests] Fix compilation (warrows) f53edec [Rand/test] scripted-diff: Use new naming style for insecure_rand* functions (warrows) 272f3a5 [Random / tests] scripted-diff: Use randbits/bool instead of randrange (warrows) 0173ee3 Replace rand() & ((1 << N) - 1) with randbits(N) (Pieter Wuille) 250de74 Replace more rand() % NUM by randranges (Pieter Wuille) d690413 [Random / tests] scripted-diff: use insecure_rand256/randrange more (warrows) 4a811ff Merge test_random.h into test_bitcoin.h (Pieter Wuille) f275e63 Add various insecure_rand wrappers for tests (Pieter Wuille) 602af4f Add FastRandomContext::rand256() and ::randbytes() (Pieter Wuille) 9054978 Add perf counter data to GetStrongRandBytes state in scheduler (Matt Corallo) 17dd13e Add internal method to add new random data to our internal RNG state (Matt Corallo) c7a1602 Use sanity check timestamps as entropy (Pieter Wuille) f671fe9 Test that GetPerformanceCounter() increments (Pieter Wuille) dcb536f Use hardware timestamps in RNG seeding (Pieter Wuille) 7c3f290 [Random] Fix compilation (warrows) 22b7895 random: only use getentropy on openbsd (Cory Fields) e5750e5 Add a FastRandomContext::randrange and use it (Pieter Wuille) 2a0f6cd Switch FastRandomContext to ChaCha20 (Pieter Wuille) 401ca7d Introduce FastRandomContext::randbool() (Pieter Wuille) 3d056d6 Add ChaCha20 (Pieter Wuille) 3c97f3f Kill insecure_random and associated global state (Wladimir J. van der Laan) 68ba16c Maintain state across GetStrongRandBytes calls (Pieter Wuille) c3c399e random: Add fallback if getrandom syscall not available (Wladimir J. van der Laan) 7a8111f sanity: Move OS random to sanity check function (Wladimir J. van der Laan) 387c2e9 squashme: comment that NUM_OS_RANDOM_BYTES should not be changed lightly (Wladimir J. van der Laan) 9e8c266 util: Specific GetOSRandom for Linux/FreeBSD/OpenBSD (Wladimir J. van der Laan) f989b86 Don't use assert for catching randomness failures (Pieter Wuille) a15419e Always require OS randomness when generating secret keys (Pieter Wuille) Pull request description: Since #576 hasn't changed in over a month, here is a reworked version of it. So in this PR: -We add the memory_cleanse function from upstream, to remove a number of OpenSSL calls. -We use OS randomness in addition to OpenSSL randomness (see #576 for why it's needed). ACKs for top commit: random-zebra: ACK b7dda92 furszy: ACK [`b7dda92`](b7dda92) Tree-SHA512: d92cbc14d844263ced753248e646f5cd4f03ec37546f50ff1b558fc3076b6d777c7efcb899c0400bc510e21311b5bd93d9aca26d811033fedb370f8457204035
The referenced bitcoin PR: bitcoin#7891 .
Issues with OPENSSL RNG: https://eprint.iacr.org/2016/367.pdf .
Note: Bitcoin core uses a utility function in /src/support/cleanse.cpp instead of OPENSLL_cleanse so the cherrypicked code had to be changed slightly to reflect the difference. I'm not sure exactly what the motivation is behind using their utility method but that might be something worth looking into in the future.