monero-project / monero

Monero: the secure, private, untraceable cryptocurrency
https://getmonero.org
Other
8.98k stars 3.11k forks source link

Don't use Insecure Userland PRNG for random_scalar() #1271

Open paragonie-scott opened 8 years ago

paragonie-scott commented 8 years ago

Problem

Instead of just directly using the Cryptographically Secure Pseudo-Random Number Generator provided by the operating system, a hash-based userland PRNG is being used instead.

Offending Code

Using a userland PRNG instead of the kernel's CSPRNG is an antipattern (even if it's seeded by the OS). It doesn't create defense-in-depth against an insecure kernel CSPRNG.

See How to Generate Secure Random Numbers in Various Programming Languages.

This is certainly a security issue, but not one that I have an exploit for on-hand.

moneromooo-monero commented 8 years ago

I think that PRNG is the construct described by the authors of Keccak in "Sponge-based pseudo-random number generators", though I'm not 100% sure about it. Changing this to always ask /dev/urandom or getrandom is possible, but I'm a bit worried about what happens if the kernel runs out of entropy. Hopefully it can't get worse than using our own, but some of the code uses a lot of random numbers (eg, output selection rounds). I wonder if we could switch to kernel for key and related randomness, and keep on using the Keccak construct for the rest. Or would that be no problem ?

man 2 getrandom mentions such a case, though how much is too much isn't mentioned, and probably depends on how fast the kernel's entropy pool can replenish on that particular machine.

fluffypony commented 8 years ago

Thanks for opening this issue, @paragonie-scott :)

@moneromooo-monero urandom (and the Windows CryptGenRandom) are non-blocking, so it's not possible to run out of entropy per se. /dev/random can "run out" of entropy, but since urandom is seeded from that (typically using a stream cipher) there's an unlimited amount of randomness that can be derived once /dev/random has enough entropy.

There are only three instances where /dev/random will lack entropy:

  1. A newly installed system's very first boot
  2. A VM cloned from an image, on its very first boot
  3. A live CD/USB boot, assuming it's a fresh boot without persistence

These are alleviated within a couple of minutes at most, and I don't suspect we'll have too many people that will need to create a transaction that quickly in instances 1 and 2. Instance 3 is a possible scenario where it could occur, but seeing as how they'd still have to load things up and get ready to generate a transaction this also shouldn't be a concern.

For all other environments, the state of /dev/random (and the Windows equivalent) is saved to disk and loaded on reboot, so good PRNG entropy is available immediately.

If we're deeply concerned about the risks at first boot we can check system uptime and have a warning prompt on tx creation for the first, say, 5 minutes.

moneromooo-monero commented 8 years ago

OK, that sounds fair enough. And looking at the libsodium source linked from the page above, it seems like a good idea to switch.

anonimal commented 8 years ago

Well... kovri uses cryptopp. Shall we kill two birds with one stone (poor birdy)?

moneromooo-monero commented 8 years ago

Do you have a link to its PRNG code ?

fluffypony commented 8 years ago

@anonimal we use libsodium for crypto_ops_builder already, so either or?

fluffypony commented 8 years ago

Also I have it on good authority that TweetNaCl is working towards formal verification of all of its functions, in which case it would be the preferable library for offloading crypto_ops_builder and some others, which would make cryptocpp a logical choice.

anonimal commented 8 years ago

@moneromooo-monero at the lowest level? AutoSeededRandomPool may be of more interest?

paragonie-scott commented 8 years ago

You might want libsodium instead of TweetNaCl, for a lot of reasons beyond RNGs.

TweetNaCl is good. Libsodium is good. Libsodium offers more. Both are intensely studied.

fluffypony commented 8 years ago

@paragonie-scott excellent points! In my discussions with djb at Ei/PSI a couple of months ago he indicated a preference for TweetNaCl for projects where correctness and safety are most important, which is an influencer. Nonetheless, this will definitely require further evaluation:) Thank you for your assistance!

scott-arciszewski

paragonie-scott commented 8 years ago

Sure thing. :)

Just want to add one more note: All CryptoNote-based currencies have likely inherited the exact same problem, since it originates upstream, and I've been led to believe that fixing it there won't lead to a trickle-down effect for everyone else.

Therefore, if any of the other currencies on this list are still active, it may be a good idea to point them here too.

paragonie-scott commented 8 years ago

I've pointed the projects that seem somewhat active towards here, after verifying that the same problem exists in their codebase. It's likely that I overlooked at least one.

vtnerd commented 7 years ago

There was a discussion recently on Metzdowd about linux RNG. Kernel 3.17 added a new system call, getrandom, which by default only blocks if the kernel RNG has not been properly seeded.

anonimal commented 7 years ago

https://github.com/monero-project/meta/issues/38 is a reminder that we could use OpenSSL's API. Furthermore, Tor is very active with their OpenSSL RNG implementation; I remember this changelog from their semi-recent 2.8.6 release:

  o Minor features (security, RNG):
    - Adjust Tor's use of OpenSSL's RNG APIs so that they absolutely,
      positively are not allowed to fail. Previously we depended on
      internal details of OpenSSL's behavior. Closes ticket 17686.
    - Never use the system entropy output directly for anything besides
      seeding the PRNG. When we want to generate important keys, instead
      of using system entropy directly, we now hash it with the PRNG
      stream. This may help resist certain attacks based on broken OS
      entropy implementations. Closes part of ticket 17694.
    - Use modern system calls (like getentropy() or getrandom()) to
      generate strong entropy on platforms that have them. Closes
      ticket 13696.

Reviewing their implementation and/or simply patching monero's is something to consider instead of waiting for a new library refactor (since we already have OpenSSL available). I'm not endorsing either-or, just dropping a message 😄

ghost commented 7 years ago

@moneromooo-monero Is this still a live issue with all the recent OpenSSL pulls and discussion re: CSPRNGs?

paragonie-scott commented 7 years ago

I would ask Rich Salz (not tagging him directly in case he doesn't have the time/emotional bandwidth to read this thread right now) of the OpenSSL team for details, but https://github.com/openssl/openssl/issues/898

iamsmooth commented 7 years ago

Tor's approach quoted by @anonimal above (not relying exclusively on one source) is similar to what the Bitcoin RNG module does, which we are exploring in #2731

dEBRUYNE-1 commented 6 years ago

+enhancement

dEBRUYNE-1 commented 6 years ago

+important

LouisLoyaX commented 1 year ago

I just did a review of the Bitcoin approach to this and they seem to harvest entropy from multiple sources of randomness as to not trust a single source ( In contrast to Monero approach of just trusting /dev/urandom ).

They combine all this sources of randomness on seed initialization and also update it periodically.