fallback random number generators considered insecure

[timkuijsten]

I've read a lot of things since the LibreSSL fork about not using a userland CSPRNG, simply because secure randomness can only be assured by the kernel [1][2][3][4].

I'm wondering why these rules would not apply to JavaScript programs. I suspect that this library would be more secure if it by default hard fails if neither Web Crypto nor Nodejs crypto are available, instead of falling back to the "userland" ISAACs PRNG.

[1] http://sockpuppet.org/blog/2014/02/25/safely-generate-random-numbers/ [2] https://news.ycombinator.com/item?id=8034710 [3] http://news.sixgun.org/2014/07/21/linux-kernel-libressl/ [4] http://lwn.net/Articles/606141/

[dcodeIO]

See: https://github.com/dcodeIO/bcrypt.js/tree/master/dist

The default build comes without ISAAC. Using it is optional.

[timkuijsten]

Sounds good!

It looks like ISAAC is built around Math.random [1] which is not considered a CSPRNG [2]. Maybe you can tell me anything about the security assurances ISAAC provides and if it's comparable to a native Web Crypto API implementation?

The API of setRandomFallback and the text on https://github.com/dcodeIO/bcrypt.js/tree/master/dist do not state anything about any possible differences in security when using a fallback like ISAAC. I'm a bit worried that a reader might not realize that choosing a custom fallback instead of using the native solution might negatively impact the security of the generated hashes.

[1] https://github.com/rubycon/isaac.js/blob/master/isaac.js#L103 [2] https://stackoverflow.com/questions/5651789/is-math-random-cryptographically-secure

[dcodeIO]

More information on ISAAC is available at:

• Implementation used:
  https://github.com/rubycon/isaac.js
• Original implementation
  http://burtleburtle.net/bob/rand/isaacafa.html
• General information
  http://en.wikipedia.org/wiki/ISAAC_(cipher)
• Discussion
  http://crypto.stackexchange.com/questions/1623/is-isaac-considered-secure-as-a-cryptographic-random-number-generator
  http://www.reddit.com/r/programming/comments/1xl7yc/isaac_a_pseudorandom_number_generator_thats/

[timkuijsten]

Nice warning in the docs.

Following that crypto.stackexchange.com link it states: "The major issue is on the way which it is initially seeded.". Seeing that ISAAC is initially seeded with the cryptographically insecure Math.random my worries are not completely taken away.

Same for the reddit link: "This original article intentionally left out information on how to seed this CSPRNG, but the author later clarified that you can use any number of bits as the seed and simply repeat it until the rsl array is saturated. Seed it with ~1kb of data from /dev/random if you want, seed it with the 64-bit system time (although that ruins the point of being cryptographically secure), etc. The more bits with more entropy you provide, the stronger the security."

Another source suggest that it's important that the seed should have enough entropy. When reading about Math.random in different browsers it looks like this is not the function that can provide that kind of strong entropy.

But, since I'm not authoritative on this subject it's all a bit of speculation on my side.

[timkuijsten]

Though https://github.com/fpirsch/twin-bcrypt/issues/2#issuecomment-59554161 seems to underwrite the importance of good entropy, even when seeding ISAAC.

[dcodeIO]

I've added a basic entropy accumulator that uses some common sources of randomness to seed ISAAC, like mouse and touch movements, timestamps, Math.random and a bit of timing information.

See: https://github.com/dcodeIO/bcrypt.js/blob/master/src/bcrypt/prng/accum.js

This should already be a lot better than using Math.random alone, but it's surely still far from what's possible. I have also added a note to the distributions page referencing the accumulator in the hope that it can serve as a good starting point for everyone interested in improving it.

Some open questions besides "Is this implementation sane?" are:

• Is it really OK to seed ISAAC with an array of integers holding a quite varying amount of entropy? It is not, an accumulating stream cipher should be used (see next).
• Is it worth the overhead (both in size and speed) to add another stream cipher for pooling? Yes, it is.
• Are there any environments that are likely to not generate sufficient entropy using the currently implemented collectors?

Please let me know of possible improvements or, even better, send me pull requests.

Edit: It looks to me like providing ISAAC bundled with bcrypt.js is terrible. There should probably be another independent library providing a) a stream cipher accumulating suitable entropy sources and b) ISAAC or a similar CSPRNG seeded from it.

[timkuijsten]

I like the decision to take out any fallback PRNG and warn that if the user supplies it's own that it should by a Cryptographically Secure PRNG.

I still have doubts about actively providing setRandomFallback to the user. I'm curious about any use case where the user should provide their own CSPRNG instead of using the one provided by the environment, and if it's used, statistics about those that do it right (so without degrading security) vs. those that compromise the security of their software without realizing.

Personally, I would go a step further and take out setRandomFallback from the public api. If the user really has a CSPRNG and neither the WebCrypto API, nor Nodejs crypto is available, they can always implement a global crypto.randomBytes so that this module picks that up. I just doubt every user of setRandomFallback (if any) knows why and how to use it securely.

[dcodeIO]

I'm curious about any use case where the user should provide their own CSPRNG instead of using the one provided by the environment

If Web Crypto API is available, whatever is specified with setRandomFallback is not used at all.

Personally, I would go a step further and take out setRandomFallback from the public api. If the user really has a CSPRNG and neither the WebCrypto API, nor Nodejs crypto is available, they can always implement a global crypto.randomBytes so that this module picks that up

In case of doubt and for what it's worth I'd prefer to keep it considering exotic environments that are neither a real browser nor node.js. Something like SharpJS for example, or even Cordova (e.g. on older Android), where supplying a function returning a plain array instead of taking a typed array to populate is easier to do.

[timkuijsten]

"If Web Crypto API is available, whatever is specified with setRandomFallback is not used at all."

That's good to know! Tnx for clarifying.

[wu-lee]

If Web Crypto API is available, whatever is specified with setRandomFallback is not used at all.

Can I suggest you document this point? I only spotted it after digging around in the code and finding the link to this issue.

[Edit: just to be clear, it does say that the WebCrypto API may be used if available, but not that it will always be used if available, even if you set the fallback]