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

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

Open
fidgi opened this issue Jan 19, 2021 · 4 comments
Open

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

fidgi opened this issue Jan 19, 2021 · 4 comments

Comments

@fidgi
Copy link

fidgi commented Jan 19, 2021

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

recrsn commented Feb 3, 2021

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

@fidgi
Copy link
Author

fidgi commented Feb 3, 2021

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

rmunn commented Jul 19, 2021

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

ssysm commented Jan 29, 2023

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

if (salt[1] != '$') {
switch (salt[1]) {
case 'a':
case 'b':
salt++;
break;
default:
return false;
}
}

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants