keystonejs / keystone-utils

⚠️ Archived - Legacy Node.js utility library used by KeystoneJS and you!
http://keystonejs.com
MIT License
56 stars 20 forks source link

random string with CSPRNG #24

Open ericelliott opened 8 years ago

ericelliott commented 8 years ago

A CSPRNG random string generator with a CLI would be very useful for things like generating the cookie secret, etc...

Is something like that already available?

The current Math.random() implementation is quite weak, cryptographically speaking, and should not be used for those purposes.

morenoh149 commented 8 years ago

https://github.com/jcoglan/node-csprng

ericelliott commented 8 years ago

Cool. Should we replace our random string util with this? Maybe our generators should also offer to generate the cookie secret? =)

morenoh149 commented 8 years ago

Do the generators already generate a cookie secret? looks like https://github.com/keystonejs/generator-keystone/search?utf8=%E2%9C%93&q=cookieSecret&type=Code this.cookieSecret is defined and then placed in the .env file. Or is that not working?

morenoh149 commented 8 years ago

@JedWatson does keystone-utils have to work in the browser as well? this affects the solution

ericelliott commented 8 years ago

Yes, but it uses Math.random() to do it, which is not safe for this purpose. It should use a CSPRNG like crypto.randomBytes() (used in node-csprng).

I believe it's safe to use crypto.randomBytes() in the browser using browserify and Webpack.

morenoh149 commented 8 years ago

https://github.com/keystonejs/generator-keystone/blob/89419a6fb41b0cd744937f441c2d46229b775a30/app/index.js#L284 uses utils.randomString and that's implemented using jed's own randomkey package https://github.com/keystonejs/keystone-utils/blob/master/lib/index.js#L232 maybe the randomkey package should be augmented to provide a CSPRNG option.

I'm inclined to simply use crypto.randomBytes in the generator instead of changing the utils too much.