node-keygen icon indicating copy to clipboard operation
node-keygen copied to clipboard

Not cryptographically random

Open markstos opened this issue 9 years ago • 2 comments

Math.random() is not cryptographically random. It seems worth mentioning this in the README, or switching to a random number generator which is considered cryptographically random, such as crypto.randomBytes in Node.js.

markstos avatar Jul 06 '15 18:07 markstos

You're right @markstos. I'm actually quiet busy but I've added it to my todo list. If you like, feel free to create a pull request.

mrcrgl avatar Jul 09 '15 15:07 mrcrgl

I'm not sure how to fix it, as the cryptographically secure API returns random bytes, not a random number:

https://nodejs.org/api/crypto.html#crypto_crypto_randombytes_size_callback

I guess you could do something like:

  • Convert random byte string to hex encoding
  • Convert hex to base 10
parseInt(crypto.randomBytes(16).toString('hex'),16);

The first "16" is the number of bytes, while the second "16" tells parseInt to convert from base 16.

The problem with this approach is I'm not sure what the lower and upper bounds are for the random numbers generated.

Also, Math.random() always succeeds, while in theory randomBytes() can run out of entropy and throw.

Personally, I don't need all the options, so I use the return value from this directly as the random string I need:

crypto.randomBytes(16).toString('hex')

markstos avatar Jul 09 '15 16:07 markstos