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

Add tests for behavior when mcrypt isn't installed #79

Open
Torniojaws opened this issue Apr 23, 2016 · 4 comments
Open

Add tests for behavior when mcrypt isn't installed #79

Torniojaws opened this issue Apr 23, 2016 · 4 comments

Comments

@Torniojaws
Copy link

Torniojaws commented Apr 23, 2016

It would be good to notify that the PHP implementation requires PHP 7, because it needs the random_bytes function that is only available in PHP 7 (http://php.net/manual/en/function.random-bytes.php). It can be added to the far more common PHP 5.x with a 3rd party plugin though: https://github.com/paragonie/random_compat

But it is very hard to notice that this requires PHP 7 and spend quite a while wondering why it fails silently on PHP 5.x. There are no Exceptions thrown and no other warnings, even with error_reporting(E_ALL);

@defuse
Copy link
Owner

defuse commented Apr 23, 2016

It definitely shouldn't require PHP 7. It falls back to using mcrypt_create_iv if random_bytes isn't available. See the code here:

https://github.com/defuse/password-hashing/blob/master/PasswordStorage.php#L36-L48

If both random_bytes and mcrypt_create_iv are unavailable then you should be seeing a CannotPerformOperationException thrown.

I'm concerned that you aren't seeing that exception get thrown. Could you double check that that's the case? If the library isn't throwing an exception that's definitely a bug.

@defuse
Copy link
Owner

defuse commented Jun 7, 2016

@Torniojaws: Is it still broken for you?

@Torniojaws
Copy link
Author

Torniojaws commented Jun 9, 2016

I got it working with the "random_compat" library I mentioned in my original post. Basically just unzip them to a subdir and then require the "main" php of the library.

require __DIR__.'/lib/random.php';
class InvalidHashException extends (etc...)

@defuse
Copy link
Owner

defuse commented Jun 9, 2016

Glad you got it working! That shouldn't have been necessary. Perhaps mcrypt_create_iv is missing and that was failing silently. I'll leave this ticket open for us to test what happens when the mcrypt extension isn't installed.

@defuse defuse changed the title Requires PHP 7 What happens when mcrypt isn't installed? Mar 9, 2017
@defuse defuse closed this as completed May 2, 2017
@defuse defuse reopened this May 2, 2017
@defuse defuse changed the title What happens when mcrypt isn't installed? Add tests for behavior when mcrypt isn't installed May 2, 2017
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

2 participants