search

koajs / csrf

CSRF tokens for koa
MIT License
264 stars 32 forks source link

Default token signature weaknesses, #7

Closed stash closed 10 years ago

stash commented 10 years ago

I appreciate that you've made the tokenizer and validator extensible. However, secret keys really shouldn't be used with a straight hash algorithm.

var mac = crytpo.createHmac('sha256', secret).update(salt).digest('base64')

SHA1 is also pretty insecure at this point in time. Is there a reason for not using SHA-256?

For HMAC, It's important that the secret length matches the hash function. For SHA1 it's 20 bytes (160 bits), SHA256 is 32 bytes (256 bits), etc. The HMAC RFC (https://tools.ietf.org/html/rfc2104) provides guidance on key lengths.

If possible, addressing these would be a responsible thing to do to make the default more secure.

stash commented 10 years ago

hold on, still need to edit; clicked Post by accident

stash commented 10 years ago

ok, done.

tj commented 10 years ago

SGTM, that's not my domain but seems like it would be non-trivial to bypass per-request tokens, but anything that helps best practice-wise would be nice

stash commented 10 years ago

Yeah, it's not a "must-fix"; definitely "best-practice"

jonathanong commented 10 years ago

i don't mind accepting a PR for this stuff. being super crypto secure isn't necessary for csrf. it's fine now as long as you're using SSL. but yeah, we should probably phase out sha1 in general.