search

kelektiv / node.bcrypt.js

bcrypt for NodeJs
MIT License
7.4k stars 511 forks source link

Support of $2y$ prefixed password in compare functions #849

Open fidgi opened 3 years ago

fidgi commented 3 years ago

Hi,

We have to support user's accounts from a legacy php-based system that encrypts passwords in bcrypt but with a $2y$ prefix. Can it be possible to support this prefix in the compare and compareSync function assuming it is compatible with 2b ?

Regards,

recrsn commented 3 years ago

Is it feasible for you to replace y with b? It is compatible with b

fidgi commented 3 years ago

That what I did in a local variable before invoking the compare function but I consider this as a hack and I'd prefer having 2y supported natively by the library .

rmunn commented 3 years ago

Encountered this myself matching against some passwords in a database that were generated by legacy PHP code I'm trying to replace. I was able to work around the lack of matching $2y$ prefixes by doing the following:

const user = /* fetch user from DB */
const hashPass = /^\$2y\$/.test(user.password) ? '$2a$' + user.password.slice(4) : user.password
const match = await bcrypt.compare(clearPass, hashPass)
if (match) { /* login */ }

But I'd rather just be able to pass the user.password value directly into bcrypt.compare. Is there any reason not to support $2x$ and $2y$ prefixes, other than personal preference? It's not like there's an RFC that mandates $2a$ and $2b$ only. Besides, some tools still generate $2y$ today. For example, following the instructions at https://unix.stackexchange.com/a/419855, I ran htpasswd -bnBC 10 "" password | tr -d ':\n' and got $2y$10$su3VmyX96cZhMy1Hswn3ZehHSiMa0qqwnS0BMTAlYFhOgVViP6R4O. The htpasswd tool I used came from version 2.4.48 of the apache2-utils package, so it's not like this is an ancient version.

Since there are tools today that generate $2y$ prefixes, doesn't it make sense to support them equally alongside $2a$ and $2b$ prefixes?

ssysm commented 1 year ago

Ran into this issue today and after reading the native code it seems like this is more a implementation decision rather than a personal preference. At the salt header check https://github.com/kelektiv/node.bcrypt.js/blob/11d2ddd185c163314bd91754c5803002d929b4ff/src/bcrypt_node.cc#L29-L38

The reason why it didn't support $2x$ or $2y$ is probably have to do with a bug in crypt_blowfish module in used in a really old PHP version from 2011. The discussion around the bug suggest that replacing $2a$ with $2x$ and only let crypt_blowfish module use the $2y$ header. I think nobody else used $2x$ or $2y$ as a header for bcrypt.