lusca icon indicating copy to clipboard operation
lusca copied to clipboard

Crypto could be improved for generating CSRF tokens

Open aredridel opened this issue 10 years ago • 1 comments

https://github.com/krakenjs/lusca/blob/master/lib/token.js#L18 in particular uses pseudoRandomBytes, which generates low quality output until warmed up.

Kraken should probably warm up randomBytes at app boot (not emitting the start event until it gets data) so that the PRNG is seeded well. At that point, pseudoRandomBytes is the same as randomBytes, and randomBytes will never block/throw anyway.

At https://github.com/krakenjs/lusca/blob/master/lib/token.js#L35, we generate and use a salt, but it's combined with the secret by appending, not HMAC, so it would possibly be extensible; that said, a salt should get us exactly nothing, and we should use a fully random token since we're just doing an equality check anyway. There's no use in salting to prevent a rainbow table, because nobody generates a rainbow table for random data anyway; that's for passphrases, not randomness.

aredridel avatar Feb 03 '15 17:02 aredridel

Cool with the pseudoRandomBytes stuff (though it's still better than the original Express implementation which used Math.random, iirc).

Regarding the the concatenation, we just emulated the Express implementation. I'd support us breaking that in a major release if need be.

jasisk avatar Feb 03 '15 19:02 jasisk