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

[5.7] Mixing hashing algorithms not working #25458

Closed
Joel-Valentine opened this issue Sep 5, 2018 · 3 comments
Closed

[5.7] Mixing hashing algorithms not working #25458

Joel-Valentine opened this issue Sep 5, 2018 · 3 comments

Comments

@Joel-Valentine
Copy link

Joel-Valentine commented Sep 5, 2018

  • Laravel Version: 5.7.1
  • PHP Version: 7.2.5
  • Database Driver & Version: MySQL @ 5.7.23

Description:

The RuntimeException thrown in illuminate/hashing/ArgonHasher.php, check method, breaks backwards compatibility when mixing hashing algorithms.

Currently trying to migrate hashes from bcrypt to Argon2. We rehash passwords after authentication and are running into issues when trying to even login with a bcrypt hash. RuntimeException on line 1 of snippet.

if ($this->attemptLogin($request)) {
	$user = Auth::user();
	if (Hash::needsRehash($user->password)) {
		$user->password = Hash::make($request->input('password'));
		$user->save();
	}
}

Steps To Reproduce:

  1. Have existing users with bcrypt hashes
  2. Use Argon as hashing driver
  3. Login as user with a bcrypt hash

Notes:

Removing throw from ArgonHasher returns to working as expected

@Joel-Valentine Joel-Valentine changed the title Mixing hashing algorithms not working [5.7] Mixing hashing algorithms not working Sep 5, 2018
@donovanhare
Copy link
Contributor

donovanhare commented Sep 5, 2018

I am also having issues with the algorithm checking not quite working as intended.

The hashes I have have been generated over time with different versions of bcrypt and portions have stopped working with Laravel 5.7.

See https://en.wikipedia.org/wiki/Bcrypt#Versioning_history - Bcrypt has had several versions over the years, with the latest identified by the $2y$ prefix.

Now, for whatever reason PHP's password_get_info() function only recognises https://github.com/php/php-src/blob/master/ext/standard/password.c#L81 the $2y$ prefix despite the fact that there are other versions.

Since, behind the scenes $this->info is 'password_get_info()' Laravel now rejects valid bycrpt hashes that don't have $2y$ prefixed. https://github.com/laravel/framework/blob/5.7/src/Illuminate/Hashing/BcryptHasher.php#L60

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) { } }

I'll make a pull with a fix...

@laurencei
Copy link
Contributor

Duplicate of #25586

@crynobone
Copy link
Member

This has been fixed in 5.7.4

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