search

ircmaxell / password_compat

Compatibility with the password_* functions that ship with PHP 5.5
MIT License
2.15k stars 421 forks source link

Known Salt Regression in v1.0.4 #80

Closed bcremer closed 9 years ago

bcremer commented 9 years ago

In an internal testsuite I discovered the following regression regarding known salts in the v1.0.4 release:

Testcase

public function testKnownSaltRegression() {
    $hash = password_hash("foobar", PASSWORD_BCRYPT, array("cost" => 04, "salt" => "OyuaiuF.Bcw7mNCak/3Au7c="));
    $this->assertEquals('$2y$04$T3l1YWl1Ri5CY3c3bU5DYOiIKqbbPpdcIiqpnkLoRwJkELw/uCaUO', $hash);
}

Resultmatrix

PHP v1.0.3 v1.0.4
5.5.14 ✔ ✔
5.4.39 ✔ ✗

It's not really an issue for me because in the real application, a known salt is never used. I suspect the provided salt OyuaiuF.Bcw7mNCak/3Au7c= is simply malformed/not valid and the issue is related to https://github.com/ircmaxell/password_compat/pull/38.

Because this is not a real issue for me feel free to close this issue.

Nevertheless I would love to understand what's the root cause of this regression and what was wrong with the salt in the first place. I also fully understand that providing custom salts is a bad idea and will most probably remove that from my internal interfaces completely.

ircmaxell commented 9 years ago

That's correct. The change was applied from #39, but I never made it upstream.

That's irrelevant now that salts are going to be deprecated (I will deprecate them in this library as well). So :-)