bug in 3.1.0 (sha1 defaulted instead of sha256) and allowing us to upgrade users passwords to new algorithms behind the scenes.

[BrandonCopley]

There is a bug in 3.1.0 version of passport-local-mongoose in line 52.
if (crypto.pbkdf2.length >= 6) {
This line in Node >4 is an odd line, because crypto.pbkdf2 is not something that has length, because of this no digestAlgorithm is specified and sha1 is used. This is kind of a big deal 4.0 fixes this bug, however those of us that went into production with 3.1.0 have a massive problem on our hands. Our users passwords are "insecurely" stored as sha1 and we need to upgrade to sha256 at minimum, although if you're on 64 bit machines you should upgrade to sha512 instead and you'll get faster performance and a more secure passcode.

I am thinking we should build the capabilities into here that allow us to take users that are stored with older algorithms and upgrade those users seamlessly in the background without the users knowledge.

It appears we can do this through a few different ways.

1. By changing code inside of User.authenticate() to grab the "hashing algorithm" just like we grab the salt, this requires storing the hashing algorithm in the User DB. If no algorithm is set we use the older algorithm to attempt login, if however the algorithm is set we use the algorithm specified by the user object.

2. By having 2 settings in our options, digestAlgorithm, digestAlgorithmDeprecated. We first try the latest method, if it fails, then we try the deprecated method, if the deprecated method succeeds we take the password that we still have access to and rehash the password to use the new algorithm.

The 2nd method appears to be seamless and may be a great feature for the future, because someone may want to change their digest method from sha256 to sha512 or to any other digest algorithms should these algorithms become insecure in the future. sha1 is not 100% cracked, so using sha1 is still viable for security, but it is proven that in the future we could crack it, the same will be true of sha256 and sha512 eventually.

@saintedlama what are your thoughts on this, and if I were to pull request this which would you prefer to be developed?

[BrandonCopley]

@saintedlama option 1 is what @toddbluhm has an existing pull request for. I think that's great, but I personally am a bigger proponent of option 2 because it eliminates work for the developers and will minimally affect internal functions.

[toddbluhm]

@BrandonCopley Personally, I like option 2 as well. I hated having to track whether a doc was old/outdated or not, but at that time that was the best I could come up with for the system I was working with. I have since switched companies and don't even use this lib anymore. Looking at it again after all this time and I think option 2 sounds the best because of its simplicity. Again, I hated having to track old versus new docs.

[BrandonCopley]

@toddbluhm Agreed, tracking old/new docs sounds good initially, but more complex than it should be. It's night in Austria now, but hopefully we hear from @saintedlama soon with what he would like to see done and hopefully we can get a pull request shot off that works and allows us to update and build this into 4.x without breaking any existing functionality.

[saintedlama]

I have to think about it - perhaps we have a third option to do the upgrade. In this context using a sha-1 is not really insecure but I mean why stay on something old and outdated if we have the option to upgrade

[BrandonCopley]

@saintedlama You are right that using sha-1 isn't insecure, but it sure isn't secure enough longterm, just like sha256, etc. will eventually fall out of fashion. What do you think we can add to the code so that we can seamlessly upgrade our users passwords?

[kevinohara80]

Since this is still hanging out there, I'm curious if anyone has come up with a interim/userland solution for this?

[FdezRomero]

@saintedlama Since SHA-1 has finally been "broken" with collision attacks and widely recognized as being insecure, a decision needs to be taken so we can start moving users to SHA-256 or better. I'm willing to contribute with whatever I can 👍