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

Bcrypt phpversion requirement #1622

Closed
barryvdh opened this issue Jun 12, 2013 · 3 comments
Closed

Bcrypt phpversion requirement #1622

barryvdh opened this issue Jun 12, 2013 · 3 comments

Comments

@barryvdh
Copy link
Contributor

Bcrypt hashing requires php 5.3.7, because of the needed support for $2y. But as described in ircmaxell/password_compat#10, there are versions where this support is backported. The readme also states This library requires PHP >= 5.3.7 OR a version that has the $2y fix backported into it (such as RedHat provides).

When running on such a php version, I tried the version-test.php from the password_compat library, and it passes on the 5.3.3 Redhat version. But when using the Laravel Hashing functions, a new \RuntimeException("Bcrypt hashing requires PHP 5.3.7"); is thrown, because of the version check.

Removing this check works fine and passwords are generated okay. Is is possible to remove the password check, or perhaps set a variable to overrule this check?

I can see why you would want to check by default, but a way to enable this without changing the vendor files would be appreciated.

@bencorlett
Copy link
Contributor

I said this exact thing ages ago so I agree fully. In Sentry, we check if the hashed password === false. If it does, we bitch out then, and then only.

@taylorotwell
Copy link
Member

Yeah checking for false password would probably be better on our end. I'll make that change.

@bencorlett
Copy link
Contributor

For reference https://github.com/cartalyst/sentry/blob/master/src/Cartalyst/Sentry/Hashing/NativeHasher.php#L31-L42

On 13/06/2013, at 12:50 PM, Taylor Otwell [email protected] wrote:

Yeah checking for false password would probably be better on our end. I'll make that change.


Reply to this email directly or view it on GitHub.

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

3 participants