nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.77k stars 29.68k forks source link

crypto.pbkdf2Sync return different result that previous node versions #7464

Closed blazekas closed 8 years ago

blazekas commented 8 years ago

crypto.pbkdf2Sync in node version 6 returns different result than in previous node versions. In the code below "password_saved" should match "password_verify" (it does match in node versions prior to 6) -

var crypto = require('crypto');

var password_saved = 'GNZihNRbrOzRMeo9RdnT7ssstgod2fE1mKZ0+AhG8PyuTKevYGjUDHzrdnv7pEYDmoeB4V7POv5DmokIxIX6ng==';
var salt_saved = new Buffer('fhPctjE477+92KQi77+977+9dBYr77+9', 'base64').toString();

var password_entered = 'ramtairidi47';

var password_verify = crypto.pbkdf2Sync(password_entered, salt_saved, 10000, 64, 'sha1').toString('base64');

console.log('password_saved = ' + password_saved);
console.log('password_verify = ' + password_verify);
mscdex commented 8 years ago

The reason for this is that the default string encoding changed (from 'binary' to 'utf8') in b010c8716498dca398e61c388859fea92296feb3 when using strings in crypto functions. Specifically if you change:

var salt_saved = new Buffer('fhPctjE477+92KQi77+977+9dBYr77+9', 'base64').toString();

to:

var salt_saved = new Buffer('fhPctjE477+92KQi77+977+9dBYr77+9', 'base64');

you will see the same (yet still different than password_saved) password_verify result on both v5.x and v6.x.

cperezvinsite commented 7 years ago

I have the same issue I create the salt like this

this.salt = new Buffer(crypto.randomBytes(16).toString('base64'), 'base64');
crypto.pbkdf2Sync(passwd, this.salt, 10000, 512, 'sha512').toString('base64'); // this is the the string I save in DB as password

when an user try to login I hash this password with the "this.salt" iI stored in the user db object and it doesn't work anymore

I update to node 6.9.2 from node 0.10 SO Ubuntu 14 Linux (none) 3.13.0-74-generic #118-Ubuntu SMP Thu Dec 17 22:52:10 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

sam-github commented 7 years ago

new Buffer(crypto.randomBytes(16).toString('base64'), 'base64');

That's a no-op, its the same as crypto.randomBytes(16) without the conversion to a string and back.

The docs don't mention that default salt encoding changed, they just mention password, should be fixed.

They also don't help much in modernizing code.

@blazekas @cperezvinsite You probably figured it out, but it wasn't obvious to me, so for anyone who runs up against this, the way to convert code that passes strings as either salt or password so it will work on node >= 0.10:

var salt = ... // string or buffer
var pass = ... // string or buffer

crypto.pbkdf2Sync(new Buffer(pass, 'binary'), new Buffer(salt, 'binary'), iterations, keylen, 'sha1')

But node0.10 is obsolete, if you just want code that is as safe as possible, and works identically to 0.10, and runs on node >= 4.x, use Buffer.from:

crypto.pbkdf2Sync(Buffer.from(pass, 'binary'), Buffer.from(salt, 'binary'), iterations, keylen, 'sha1')
cperezvinsite commented 7 years ago

@sam-github thanks for the answer but still doesn't works, I will describe my problem

this.salt = new Buffer(crypto.randomBytes(16).toString('base64'), 'base64');
this.passwd = crypto.pbkdf2Sync(passwd, this.salt, 10000, 512, 'sha512').toString('base64');

I saved this as an user, then when n user try to login into the system I get the password from the request and execute this function

var hashPassword = function(passwd) {
  return crypto.pbkdf2Sync(passwd, this.salt, 10000, 512, 'sha512').toString('base64');
};
var authenticate = function(passwd) {
  return this.passwd === this.hashPassword(passwd);
};

if is true means the password is correct.

My problem is, if i created an user using node 0.10 the user login only works in node 0.10, if I create the user in newest versions, it doesn't works in node 0.10, so, I'm sure the problem is in this function

crypto.pbkdf2Sync(passwd, this.salt, 10000, 512, 'sha512').toString('base64');
sam-github commented 7 years ago

To help, I need a small standalone code reproduction, with hard-coded expected input and output, like the original post (which I did get working by constructing Buffer using the old default string encoding).

cperezvinsite commented 7 years ago

@sam-github look my test code looks like this

var crypto = require('crypto');
var passwd = 'iamatest';
var salt = 'iamasaltfortest'
console.log(crypto.pbkdf2Sync(passwd, salt, 10000, 512, 'sha512').toString('base64'));

with node v6.9.2 the output is (with node 4.8.2 i have the same result) o/9H5LTvvwuHF/CH38smpu4uyTon8ZmuxjMfYMtbFpLOhbIw5njaM4nKpERvCrNXCZUqcXQkHNxC73n05ENwdhhTZA6ZFoOkp6Jfo4PjdUrFmDL3OLOmvd4fT+wYU2B6aua7qBYcLO+bFsQJWW7YJHzxEgtX/Xty5KgfOSSC5s+1KKOUHEqEaGy7V07sNOb3J+WiccB95KPY0dTr+AF7ToOWAmFbV4bSnDHoO1BTilEaD2KUzZayLHdwEltp8LUgS7knSjbdIgYGDYWnBzkBmzAnD3biWCOz1miVnlLBerbGGWXwoPlH3wHjGhRZpBtWOMPCqaO5dmvhMW4HIbMP8tF6nD7OarBvGP2iVMC7HVlkCknzCuGSQKC1P5zkrnoFX/mhNVw61SRGbLCnXRnA8Wh3WXexWjWvig1dn/i9YPpGsft+ihLqJYWxuKBzbaayAY6eC1n2SK9KLXFioOICZrledr7FDifMVc+sDkwsUkDNWBmJmF3AdfNRwwHwdmVMNQkvOMnli8y2NFG2UPyzXm9JQSEdKYrOa74IcyBeSMKSmY/au+xie1HHhkLNu2sLwVNx4fSfdSPGcyEaf6ac60PXZvtMYRhXV6AFxnbtYedPWMi+VUyOWVaRvHQepCL32Qed3afAYRjF5ChaZTrVU+eAGubK//XKCHTXyeW/nHk=

with node v0.10.25 the output for the same code is W0S7dema6Og0S/w16kE1G+9fWVryXVPK7kHs6OxGiTICOdx7Ofvry0HDz5VpSP425g98MqiJUiPaF2wVe+QGQAysWGVRjhOaiNhxIFAUebnDW6VmcELbJPfcEQQggIfmoCA/eMNQceljXKQNlD6vGoSrg/ITilcf5r7mtgV6K1ESdAg+QdBr8AWgsTZnK2Ews07kHsgALQQwsgGUiLCFkiOJKcGcjmU6wmMS7LR1FzpjDjNHzsWkMhABoSVk7QZdGBXSwhNS5YpjbCenILM3xLuzM4mX0r3HHl3y+hMtN60pM17EslVaBLcbHQfVwuuY/+4om2RcQDsdguK+naZH7GmGslt3y5F8KDKakSLHxLcLZxJF26Wo3M3jpcNv6kq/4QJeUFDP9RogKxXwztnKhM/Cq4Lp8niDweOWcz2R0orXNpBrvxhYOQJdoKe9bowbkKpKhzpcW6LxiSSrBksweGiSjAYeyjq1FqsU5ZadosHIWdwhayHyyxI8Se+qC0+hyawu5ZaQ6M20umrqZmwACrrmOuOWqyD7e+RT02Zd+hRF76kl0T5wp3cfsrGaGKeMhwa7jmanDiL1H/phVCq9+MoTHW8hVsMBX3XOJQYyKmNgnabLRhnMq+Aot8bZamjYPFONMdqg/g7Lx8ggGg0HUQHhuYFjuGVlWDz97pSYg/Y=

bnoordhuis commented 7 years ago

crypto.pbkdf2Sync(passwd, salt, 10000, 512, 'sha512')

Node v0.10 doesn't support a custom digest, it always uses SHA-1.

cperezvinsite commented 7 years ago

Thanks, I solved my problem changing this code

crypto.pbkdf2Sync(passwd, salt, 10000, 512, 'sha512').toString('base64')

to

 crypto.pbkdf2Sync(passwd, new Buffer(this.salt,'binary'), 10000, 512).toString('base64');
sam-github commented 7 years ago

But note that you'll need to explicitly specify SHA-1 in newer versions of node: https://github.com/nodejs/node/pull/11305

emauriciomoreno commented 7 years ago

Thanks cperezvinsite, i solved too: crypto.pbkdf2Sync(password, new Buffer(this.salt,'binary'),10000,64,'sha1').toString('base64');

nagendrayadav88 commented 6 years ago

Hello Friend , I am facing some problem , crypto.js:694 throw new TypeError( ^

TypeError: The "digest" argument is required and must not be undefined at pbkdf2 (crypto.js:694:11) at Object.exports.pbkdf2Sync (crypto.js:687:10) at model.encryptPassword (C:\Users\DEV_Nagu\Desktop\Faelo-food-order-app-MEAN.js--master\server\api\user\user.model.js:154:19)

Please help me , Thank you

VRamazing commented 6 years ago

@nagendrayadav88 https://nodejs.org/api/crypto.html#crypto_crypto_pbkdf2sync_password_salt_iterations_keylen_digest

karthikn2510 commented 5 years ago

Hello Friend , I am facing some problem , crypto.js:694 throw new TypeError( ^

TypeError: The "digest" argument is required and must not be undefined at pbkdf2 (crypto.js:694:11) at Object.exports.pbkdf2Sync (crypto.js:687:10) at model.encryptPassword (C:\Users\DEV_Nagu\Desktop\Faelo-food-order-app-MEAN.js--master\server\api\user\user.model.js:154:19)

Please help me , Thank you

You have to give the 5th argument as 'sha1'. please specify a digest explicitly.
crypto.pbkdf2Sync(password, salt, iterations, keylen, digest)