Search code examples
phpmysqlhashpasswordscrypt

Not Rehashing Properly


We have a PHP class that reads the database on user credentials, rehash SHA1 => BCrypt as needed. Works great, however these days we are developing a WinForms app, and using Chilkat BCrypt assembly we are noticing weird effects to the rehashing procedure.

Account class:

Username/Password sent. Checks existing row if user has SHA1, if so we rehash to BCrypt, otherwise we ignore and log them in (hash verification successful).

Now that is all fine, however Chilkat uses "2a" while our classes writes "2y" for the revisions in the final hashes. This prevents logging into both App & Website! Its one or the other due to the rehashing (both bcrypt hashes, never SHA1 found for our test account).

So we learned this was the case by generating hashes using public "generator" sites.

Is there a problem with our code?!

if (is_array($row))
            {
                if (password_verify($passwd, $row['password']))
                {
                    /* Check if password needs rehashing, if so then continue1 */
                    //if (password_needs_rehash(sha1($row['password']) , PASSWORD_DEFAULT))
                    if (preg_match('/^[0-9a-f]{40}$/i', $row['password'])) /* Added for App login, as both hashes are different revisions (2y => 2a) */
                    {
                        /* Rehash existing password algorithm to BCRYPT and update */
                        $newhash = password_hash($passwd, PASSWORD_BCRYPT);

                        try
                        {
                            $stmt = $pdo->prepare("UPDATE users SET password = ?, last_access = NOW(), last_login = NOW() WHERE user_id = ?");
                            $stmt->execute([$newhash, $row['user_id']]);
                        }
                        catch(PDOException $e)
                        {

                            /* If there is a PDO exception, throw a STAccountsException exception. */
                            $STe = new \STAccountsException($e);
                            $STe->setVarData(['error' => 'Rehash password failed']);
                            //$STe->setVarData(['query' => 'UPDATE users SET password = ?, last_access = NOW(), last_login = NOW() WHERE user_id = ?', 'values' => [$newhash, $row['user_id']]]);
                            throw $STe;
                        }
                    }
}

I tried to better detect hashes using

if (preg_match('/^[0-9a-f]{40}$/i', $row['password'])) {}

And that allows login in both website and app! However SHA1 hashes never rehash now. This just makes no sense, this class has never failed for 2 years according to our database records and user feedback.

Any assistance appreciated!

UPDATE

This seems to convert "2y" to "2a" in the generated hash. This does not seem very compliant.

PHP update:

$hashgenerated = password_hash($passwd, PASSWORD_BCRYPT);
$newhash = str_replace('2y', '2a', mb_substr($hashgenerated, 0, 3)) . mb_substr($hashgenerated, 3);

VB.NET Update:

Dim newString As String = Replace(storedHash, "2y", "2a")

Both do the same thing, however VB.NET replaces the hash on the fly before reading it and successfully logs in user without any password modifications!

There must be a better way. It is simply replacing "2y" to "2a" now.


Solution

  • Went with the VB replace instead. This simply acts as a middle "passover", only it is passing the substituted identifiers. It works as expected, and avoids updating user passwords on the PHP login side.

    Dim newString As String = Replace(storedHash, "2y", "2a")
    Dim passwordValid As Boolean = crypt.BCryptVerify(password, newString)
    If (passwordValid = True) Then
       MsgBox("Hash is valid")
    End If