[5.7] Add the ability to skip algorithm checking

[donovanhare]

See #25458.

Upon updating to 5.7 a portion of my user accounts can no longer login (#25458) due to the algorithm checking in place. I think it would be wise to allow us to toggle this feature as and when needed.

Although ideally there should be complete consistency in terms of the hashing algorithm one uses, sometime this is not the case. For me, I have hashes that have originated from other applications and unfortunately some of them appear to have used an older version of bycrpt.
https://en.wikipedia.org/wiki/Bcrypt#Versioning_history

Below we have an example of the issue that I am running into. Because PHP doesn't recognise my bcrypt hash.

Example

$hash = '$2a$10$Wrk/FoakWwkX/tT0A/No5uu3IZrVu/e27QHTgpjHlPUQS3HwK0ei2';
password_verify('Test12345', $hash); // bool(true) 
password_get_info($hash); // array(3) { ["algo"]=> int(0) ["algoName"]=> string(7) "unknown" ["options"]=> array(0) { } }

Because the above returns an algoName of unknown the exception is thrown.

public function check($value, $hashedValue, array $options = [])
{
    if ($this->info($hashedValue)['algoName'] !== 'bcrypt') {
        throw new RuntimeException('This password does not use the Bcrypt algorithm.');
    }
    return parent::check($value, $hashedValue, $options);
}

https://github.com/laravel/framework/blob/5.7/src/Illuminate/Hashing/BcryptHasher.php#L60

PHP's password_get_info() function only recognises the $2y$ prefix. https://github.com/php/php-src/blob/master/ext/standard/password.c#L81

Solution

I have added an option that you can specify in you config to turn on and off the algorithm checking (obviously on by default).

    'bcrypt' [
        'check' => false
    ],       

The issue in the ticket (#25458) is slightly different to my use case, but check it out.

If the is a more intelligent way of going about fixing this, do let me know.

[sisve]

I imagine that another name (like verifyAlgorithm or checkAlgorithmName) is easier to understand than the generic check.

[donovanhare]

Very true, verifyAlgorithm is a good suggestion - I have changed it.

If that is deemed too long, verify?

[taylorotwell]

I suggest just extending BcryptHasher or whatever hasher you want and adding the behavior you want. Then register it using HashManager::extend and using that as your driver.

[CyrilMazur]

@taylorotwell I'm not sure you realise how that breaks existing apps, and will break also new apps in the future when new hashing algos will replace the existing ones?

[kamui545]

I believe that this breaking change was a hasty decision.

I don't see any real gain out of this, for the sake of backward compatibility and to support future hashing algorithm, it should be consistent with how password_verify works.

I also opened an issue on the framework repo: #25586

[donovanhare]

While I have overcome my issue using @taylorotwell's suggestion, I think @kamui545's has a decent argument.

---

Future Algorithms
Let's say we had a new bcrypt version ($2z) tomorrow and PHP updated their algorithm checker for PHP 7.3.

Upon updating PHP all (bcrypt) hashes checked on login would now throw an exception without updating Laravel - as the password hashes are no longer valid in Laravel's eyes.

What would be the solution in this situation?

Backwards Compatibility
See #25586

What are your thoughts @taylorotwell?

[crynobone]

This has been fixed in 5.7.4

[donovanhare]

Rejoice! 🎉

[taylorotwell]

Defaulted verification to false so it just works how it has in years past.