oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.47k stars 2.78k forks source link

Bun.password.verify bcrypt interoperability (accept $2$, $2a$, $2c$, $2x$, $2y$) #9078

Open mantissa7 opened 9 months ago

mantissa7 commented 9 months ago

What version of Bun is running?

1.0.28

What platform is your computer?

Darwin 23.3.0 arm64 arm

What steps can reproduce the bug?

Bun.password.verify using bcrypt does not work well with hashes created by other libraries. Creating a hash using Bcrypt.Net-Next for example returns false when used as the parameter for bun.password.verify(password, dotnet_bcrypt_hash), however using a hash generated by Bun.password.hash is verified successfully by Bcrypt.Net-Next.

Manually substituting $2a$ for $2b$ fixes the issue.

const password = 'test123'
const hash = '$2a$06$wouz5xNIhZrkWBw7LdMKFu.BBH76dGWsn0IZ06t47nOFac9GIgDB2' // select crypt('test123', gen_salt('bf')) as hash;

const isMatch = await Bun.password.verify(password, hash, 'bcrypt'); 
// false

const fixed_hash = hash.replace(/^\$2a\$/, '$2b$');
const isMatchNow = await Bun.password.verify(password, fixed_hash, 'bcrypt'); 
// true

What is the expected behavior?

Bun.password.verify should return true for a correct password-hash comparison generated by other libraries.

What do you see instead?

No response

Additional information

Discord #help reference: https://discord.com/channels/876711213126520882/1209423413282869249/1209423413282869249

argosphil commented 9 months ago

I'm not sure whether there's an exhaustive list of prefixes that we must recognize, or whether it's everything matching 2.?.

In any case, we can fix this in the Zig code (and maybe it's already been fixed and we can close this after our next Zig update), or work around it in Bun. Which one's better?

sequencerr commented 9 months ago

I'm not sure whether there's an exhaustive list of prefixes that we must recognize, or whether it's everything matching 2.?.

https://en.wikipedia.org/wiki/Bcrypt#Versioning_history

https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#deviations There is something about The crypt_blowfish 8-bit bug

Maybe zig lib just don't support old bcrypt versions?

sequencerr commented 9 months ago

does not work well with hashes created by other libraries.

does not work with other versions*

argosphil commented 9 months ago

Thanks for the links! If I'm reading this correctly, if OpenBSD generated a $2a$ hash for a password of 260 bytes (that's "just" 65 non-BMP codepoints), there will actually be a password of 4 bytes that verifies. I don't think that's a problem we can solve at this point, so maybe the regexp approach of replacing $2a$ by $2b$ after verifying that the source of these hashes does not truncate passwords at 256 bytes is the best we can do.

Maybe a better error code and a comment in the source, though?