Closed ret2kw closed 7 years ago
Glad you found the other threads. Having a built-in crypto.timingSafeEqual
is exactly what this has needed to put the discussions to rest. I've re-titled this issue and hopefully we can get a patch to use that when available, and the old HMAC trick (with better comments this time :-) as a fallback for older versions.
can anyone explain to me about what the sha1 function is used for,is it nessary?
@AnthonyYY See https://github.com/tj/node-cookie-signature/issues/19
I think this has been addressed so I'll close this thread out, let me know if not!
Happened to take a peek at your code in the process of looking at an app using express-session.
https://github.com/tj/node-cookie-signature/blob/master/index.js#L42
Hashing the resulting hmac's doesn't increase the security, in fact it could probably be argued it lowers it as you are taking a sha256 hmac and making it rely on the weaker sha1 hash. I don't think this leads to an exploitable bug, but you should really just compare the resulting mac's directly to each other without messing with them.edit: Read the rationalizations on the other closed tickets for people who pointed out sha1
Appreciate it is somewhat new, but any reason not to use crypto.timingSafeEqual() and get rid of the sha1 to avoid any timing side-channels?
https://nodejs.org/api/crypto.html#crypto_crypto_timingsafeequal_a_b