Password security and performance #474
Description
Out of curiosity, is there a reason why this project doesn't use bcrypt? It's my understanding that bcrypt is recommended over pbkdf2 in node, typically. I believe bcrypt is more secure for shorter pass phrases, however will start to loose out to pbkdf2 when pass phrases are over 55 characters (which typically isn't normal for user passwords). The node bcrypt libraries also provide useful features for comparing passwords, etc.
Aside from that, I noticed that user.model.js doesn't implement the node crypto functions in an async manner. IE, I would expect something like:
UserSchema
.virtual('password')
.set(function(password) {
this._password = password;
this.makeSalt(function(err, salt) {
this.salt = salt;
// Async encryptPassword
this.encryptPassword(password, function(err, hashedPassword) {
this.hashedPassword = hashedPassword;
});
});
})
.get(function() {
return this._password;
});I started to rewrite and stopped at makeSalt to get some thought generation from you fine folk.
makeSalt: function(byteSize, callback) {
var defaultByteSize = 16;
if (typeof arguments[0] === 'function') {
callback = arguments[0];
byteSize = defaultByteSize;
} else if (typeof arguments[1] === 'function') {
callback = arguments[1];
}
if (!byteSize) {
byteSize = defaultByteSize;
}
if (!callback) {
return crypto.randomBytes(byteSize).toString('base64');
}
return crypto.randomBytes(byteSize, function(err, salt) {
if (err) callback(err);
callback(null, salt.toString('base64'));
});
},