Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
blazekas opened this issue Jun 28, 2016 · 13 comments
Closed

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

blazekas opened this issue Jun 28, 2016 · 13 comments
Labels
crypto Issues and PRs related to the crypto subsystem. invalid Issues and PRs that are invalid.

Comments

@blazekas
Copy link

blazekas commented Jun 28, 2016

  • Version: v6.2.2
  • Platform: Windows 64-bit
  • Subsystem: crypto

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 mscdex added the crypto Issues and PRs related to the crypto subsystem. label Jun 28, 2016
@mscdex
Copy link
Contributor

mscdex commented Jun 28, 2016

The reason for this is that the default string encoding changed (from 'binary' to 'utf8') in b010c87 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.

@bnoordhuis bnoordhuis added the invalid Issues and PRs that are invalid. label Jun 29, 2016
@cperezvinsite
Copy link

cperezvinsite commented Apr 6, 2017

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
Copy link
Contributor

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
Copy link

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

  • I had a node 0.10 which create a bunch of users
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
Copy link
Contributor

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
Copy link

cperezvinsite commented Apr 6, 2017

@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
Copy link
Member

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

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

@cperezvinsite
Copy link

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
Copy link
Contributor

But note that you'll need to explicitly specify SHA-1 in newer versions of node: #11305

@emauriciomoreno
Copy link

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

@nagendrayadav88
Copy link

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

@karthikn2510
Copy link

karthikn2510 commented May 21, 2019

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. invalid Issues and PRs that are invalid.
Projects
None yet
Development

No branches or pull requests

9 participants