refactored crypto/password.js and added verifierVersion config parameter #507
Conversation
| sessionToken: sessionToken | ||
| }) | ||
| } | ||
| return password.wrapKb(emailRecord.wrapWrapKb) |
There was a problem hiding this comment.
There's some verb/noun overlap here that's making this hard to follow. Is this unwrapping wrapWrapKb to give you wrapKb? Or is it wrapping it? Is there in fact no difference because of xor encryption? I can't think of a better name, but it may be worth some comments on the definition of this method to clarify exactly what it's for.
There was a problem hiding this comment.
The fact that I had to look at the source of the function to even consider answering your question proves your point :)
Something like xorWrapWrapKey would probably be most accurate but, uh, WTF does that mean. Maybe wrapOrUnwrapWrapKb? (too many wraps!)
Its used to go from wrapWrapKb -> wrapKb and wrapKb -> wrapWrapKb in the code so here's a crazy idea: have 2 names for the same function... getWrapKb and getWrapWrapKb? Silly but easier to read.
There was a problem hiding this comment.
so here's a crazy idea: have 2 names for the same function
I would totally support this. Suggestion: wrapKey() and unwrapKey(), which may be a bit easier to grok in isolation, but perhaps a bit too generic.
|
LGTM |
refactored crypto/password.js and added verifierVersion config parameter
Slightly better password implementation plus the ability to have multiple stretching versions.
I added a version 0 to speed up dev tests that doesn't use scrypt. It saves about 5 seconds on the test suite.
The diff looks horrible, but the changes outside of
crypto/password.jsare pretty straightforward.