Updating methods to use buffers, adding tests, addressing feedback #164
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,15 +10,16 @@ const LENGTH = 32 * 8 | |
|
||
/** pbkdf2 string creator | ||
* | ||
* @param {bitArray|String} password The password. | ||
* @param {String} salt The salt. | ||
* @return {String} the derived key. | ||
* @param {Buffer} input The password hex buffer. | ||
* @param {Buffer} salt The salt string buffer. | ||
* @return {Buffer} the derived key hex buffer. | ||
*/ | ||
function derive(password, salt) { | ||
var saltBits = sjcl.codec.utf8String.toBits(salt) | ||
function derive(input, salt) { | ||
var password = Buffer.isBuffer(input) ? input.toString() : input | ||
var saltBits = sjcl.codec.utf8String.toBits(salt.toString()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there should be a safer way to get from Buffers to bitArrays. Passing through UTF-8 seems perilous. Maybe pass through hex instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked to @zaach he will updated it, we need to fix the sjcl bundle. |
||
var result = sjcl.misc.pbkdf2(password, saltBits, ITERATIONS, LENGTH, sjcl.misc.hmac) | ||
|
||
return P(sjcl.codec.hex.fromBits(result)) | ||
return P(Buffer(sjcl.codec.hex.fromBits(result), 'hex')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, that feels safer |
||
} | ||
|
||
module.exports.derive = derive | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not part of your patch, I know, but isn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked to @zaach about this, we'll check that, but module.exports should be okay for now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it convenient to make this always take a Buffer? Variable-type arguments look like bug-magnets to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we convert this from an sjcl byteArray back into a Buffer? @zaach do we need to add the sjcl byte codec?