We want to upgrade our server from PHP 7.4 to PHP 8.1, but when we try this, all of our users get incorrect password messages when they try to log in.
All the hashed passwords in the database were generated using this function:
define("BLOWFISH_PRE", "$2y$05$");
define("BLOWFISH_SUF", "$");
function pwhash($password) {
$allowed_chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789./';
$salt = "";
for($i=0; $i<21; $i++) { // 21 is standard salt length
$salt .= $allowed_chars[mt_rand(0,strlen($allowed_chars)-1)];
}
return crypt($password, BLOWFISH_PRE . $salt . BLOWFISH_SUF);
}
Below is what runs when the user logs in. This works in PHP 7.4 - if the user enters the same password they entered when creating their account, then the hashed user input matches what's in the database. But in PHP 8.1, the crypt function returns the string *0 no matter what gets passed in to it.
$salt = substr($dbValue, 7, 21);
$hashedUserInput = crypt($password, BLOWFISH_PRE . $salt . BLOWFISH_SUF);
echo $hashedUserInput;
Either I need to get that working, or I need to use password_verify. But that's also not working; it always returns false.
echo "password_verify returns: " . password_verify($password, $hash);
How can I get one or the other of these to work?
It turns out this isn't a change in PHP 8.1, or any specific release, but a security fix applied to all then-supported branches. We can see this by setting the salt to something predictable and comparing the result across many versions: the versions where it starts returning "*0" are 8.0.28, 8.1.16, 8.2.3, and 8.3.0.
Looking at the detailed changelogs for those releases, we find Security Bug #81744: Password_verify() always return true with some hash. This sounds relevant, so we follow through to the linked security advisory which has the title:
BCrypt hashes erroneously validate if the salt is cut short by
$
This is interesting - as pointed out by Sammitch and gview in the comments, you are generating 21 characters of salt instead of 22 - but because you are adding BLOWFISH_SUF, you are actually generating 22-character salts ending in $. (Note that there isn't actually a suffix needed in the correct format.)
That's why you had to manually extract the salt and re-run the original hash - the intended use of crypt is to pass in the entire hash when verifying, and the salt and options will be extracted automatically.
So, what was the old code in PHP doing when it found this $?
Following the source code link in the security advisory, we see that the problem was a hack in a macro called BF_safe_atoi64 which broke out of a while loop in a function BF_decode. This is essentially a base64-decoding routine, which is given a target number of output bytes, and reads the input until it can decode enough.
Searching for uses of the function, we find BF_decode(data.binary.salt, &setting[7], 16). That makes sense: base64 takes 4 encoded bytes for 3 raw bytes, so for 16 raw bytes, you need 21⅓ characters of base64, which gives us our 22 readable characters of salt. (It also means that generating 22 random characters would also be wrong; you should have been generating 16 random bytes then encoding them.)
What the old versions of PHP did when they encountered the $ was broke out of that encoding loop, wherever they got to, and then padded the output with enough 0 bytes to give the desired length. So we need to create a salt that ends with that same number of zero bytes once decoded.
Each full loop consumes 4 bytes of input to create 3 bytes of output, so after 5 loops, it has consumed 20 bytes, and output 15. It then needs to get one more output byte from the remaining two input bytes, here:
BF_safe_atoi64(c1, *sptr++);
BF_safe_atoi64(c2, *sptr++);
*dptr++ = (c1 << 2) | ((c2 & 0x30) >> 4);
if (dptr >= end) break;
With the old code, when it tried to read the second of those bytes (c2) with the BF_safe_atoi64 macro, and encountered a $, it broke out of the loop before doing anything with c1. So it turns out the 21st character of your salt was always being ignored.
The particular base64 alphabet used has . for 0, so to get a 0 out from 2 bytes of input is easy: .. If you look at your current hashes, you'll see they already have a . at the end of the salt - this isn't a separator, it's padding added somewhere in the formatting routine to give the expected length.
So it turns out all you need to do is set the 21st character of the salt to a . Since that's always at a fixed offset in the string (7-byte prefix + 20 bytes of actually varying salt), you can just overwrite that byte of the stored hash and verify it without extracting the salt:
// Example generated using your `pwhash` function on an older version of PHP
$hash = '$2y$05$/yNDcx14r6hos5BlB6zDn.deQG1FV34L6x/bZcY4Grn764Lgiy2km';
// Fix the salt to match the old algorithm
$hash[27] = '.';
// Now gives '$2y$05$/yNDcx14r6hos5BlB6zD..deQG1FV34L6x/bZcY4Grn764Lgiy2km'
// Confirm that it verifies
var_dump($hash === crypt('test', $hash));
var_dump(password_verify('test', $hash));
Then, treat any hash where you've had to do this as "needs rehash", and generate a new hash with password_hash while you have the plain text.