search

ericelliott / credential

Easy password hashing and verification in Node. Protects against brute force, rainbow tables, and timing attacks.
MIT License
347 stars 28 forks source link

fix #12 and fix #13 (update constant time string comparison) #19

Closed tjconcept closed 9 years ago

tjconcept commented 9 years ago

I've merged the work of @brianmhunt with my recent changes.

ericelliott commented 9 years ago

This looks like a drop in replacement for https://github.com/ericelliott/credential/pull/13. Should we close that one?

tjconcept commented 9 years ago

Yes, it is. It's basically just a rebasing of @brianmhunt's work.

And yes, it should automatically close issue and PR when merged.

ericelliott commented 9 years ago

Awesome. Thanks!

ericelliott commented 9 years ago

I'm OK to merge this once we have a statistics test.

tjconcept commented 9 years ago

@ericelliott heard anything from SO or other channels on this? I keep ending with the conclusion that hashes aren't vulnerable - but it's still just based on my gut.

ericelliott commented 9 years ago

Nothing new... I'm reaching the same conclusion, but I don't feel confident making the call to pull the constant time from the code. Now that we have a constant time test, I'm cool with merging this. I haven't run the tests myself on this branch. If both of you have run them, one of you should press the merge button. =) I have some errands to run, or I'd do it right now.

tjconcept commented 9 years ago

I stumbled upon https://github.com/freewil/scmp through one of your earlier posts in another auth-related module and think it seems neat.

brianmhunt commented 9 years ago

Thanks @tjconcept -- scmp is the same idea as the comparison I wrote, but it it early-outs when one string lengths are inequal (which it should not).

tjconcept commented 9 years ago

@brianmhunt I see, but aren't hashes always same length?

brianmhunt commented 9 years ago

If one consistently uses the same hashing function then yes. If one varies the hashing function, the the hash size may change.

It would be a matter of principle more than practicality. On considering it, one can still get the key size from the top-end (i.e. by varying up the length of the posit as opposed to varying down), so I am not sure there is any extra value here.

A "strong" constant-time function will do e.g. 5k comparisons for every comparison. Regardless of key size the comparison takes the same (long) length of time.

ericelliott commented 9 years ago

Assume that early bailout on shorter hash matters. =)

brianmhunt commented 9 years ago

Yeah, I imagine some nefarious folks scanning a table for SHA-1 hashes (vs. SHA-256/etc).

ericelliott commented 9 years ago

OK... I pulled and tested this branch. Looks good to me.