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

One PHPUnit test fails in 2.0.2 (and 2.0.2-p1) #3

Open
alisqi opened this issue Jul 5, 2012 · 8 comments
Open

One PHPUnit test fails in 2.0.2 (and 2.0.2-p1) #3

alisqi opened this issue Jul 5, 2012 · 8 comments
Assignees
Milestone

Comments

@alisqi
Copy link

alisqi commented Jul 5, 2012

PHPUnit output for 2.0.2 and 2.0.2-p1

PHPUnit 3.6.11 by Sebastian Bergmann.

Configuration read from /home/jay/phpass/rchouinard-phpass-2dc6e32/tests/phpunit.xml

.....F.................................

Time: 8 seconds, Memory: 5.75Mb

There was 1 failure:

1) Phpass\Hash\Adapter\ExtDesTest::adapterGeneratesSameHashGivenOriginalSaltAndPasswordString
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'_zzz1OacgfuRYUi4zYeQ'
+'_zPXZFl8kpcQw'

/home/jay/phpass/rchouinard-phpass-2dc6e32/tests/library/Phpass/Hash/Adapter/ExtDesTest.php:133

FAILURES!
Tests: 39, Assertions: 57, Failures: 1.

Generating code coverage report in HTML format ... done

I've run the tests several times. The expected hash is different every time, but the actual stays the same.

Configuration (two machines):

$ php --version
PHP 5.3.10-1ubuntu3.2 with Suhosin-Patch (cli) (built: Jun 13 2012 17:20:55)
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies
    with Xdebug v2.1.0, Copyright (c) 2002-2010, by Derick Rethans

$ php --version
PHP 5.4.4-2 (cli) (built: Jun 19 2012 07:38:55)
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.4.0, Copyright (c) 1998-2012 Zend Technologies
    with Xdebug v2.2.0, Copyright (c) 2002-2012, by Derick Rethans
@alisqi
Copy link
Author

alisqi commented Jul 5, 2012

Hm, the tests do pass on my Windows machine! I don't have a clue what configuration setting would make just one test fail on my linux environments.

@alisqi
Copy link
Author

alisqi commented Jul 5, 2012

Woah, I might have stumbled on a bug in PHP's crypt!

As I said in my original post, the actual hash is the same each time I run the tests. So I tried calling crypt by hand:

$ php -a
Interactive shell

php > echo crypt('password', '_zzz1OacgfuRYUi4zYeQ');
_zPXZFl8kpcQw
php > echo crypt('password', '_zzz1WwWwWwWwWwWwWwW');
_zPXZFl8kpcQw
php > echo crypt('password', '_zz');
_zPXZFl8kpcQw
php > echo crypt('password', '_z');
_zPXZFl8kpcQw
php > echo crypt('password', '_z2');
_zPXZFl8kpcQw
php > echo crypt('password', '_');
__XQ.2NdBaLME
php > echo crypt('password', '_a');
_aL0dOhJ7ObMw
php > echo crypt('password', '_ab');
_aL0dOhJ7ObMw
php > echo crypt('password', '_aba');
_aL0dOhJ7ObMw

It looks like the hash is always the same, as long as the salt starts with the same character after the underscore. Yet the PHP documentation says:

CRYPT_EXT_DES - Extended DES-based hash. The "salt" is a 9-character string consisting of an underscore followed by 4 bytes of iteration count and 4 bytes of salt.

It looks like the implementation of CRYPT_EXT_DES is screwed up in my PHP installs.

@rchouinard
Copy link
Owner

It looks like your installation is falling back to CRYPT_STD_DES instead of using the expected CRYPT_EXT_DES. I try to account for issues like this in the library, but it's very difficult in some cases.

I'll note that this bug does not appear under Travis CI nor any of my hosts with Zend Server, including Ubuntu 12.04. It's likely an issue specific to your installation.

If you could, clone the latest master and run the tests from that version. The version in master is 2.1-dev, and contains some additional error handling to counteract some quirks in various versions of PHP's crypt - which is exactly why I think PHP needs a library like this :-).

@alisqi
Copy link
Author

alisqi commented Jul 6, 2012

I'm running standard Ubuntu and Debian installs of PHP, so something definitely smells fishy here.
Here's the current master branch:

$ git clone https://github.com/rchouinard/phpass.git
$ cd phpass/tests
$ phpunit
PHPUnit 3.6.11 by Sebastian Bergmann.

Configuration read from /home/jay/phpass/phpass/tests/phpunit.xml

....F..F...............................................

Time: 17 seconds, Memory: 6.50Mb

There were 2 failures:

1) Phpass\Hash\Adapter\ExtDesTest::knownTestVectorsBehaveAsExpected
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'*0'
+'_zzD.2.nIWzugGxYyy0g'

/home/jay/phpass/phpass/tests/library/Phpass/Hash/Adapter/ExtDesTest.php:62

2) Phpass\Hash\Adapter\ExtDesTest::adapterConsistentlyGeneratesHashStrings
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'_7C/.n3SAnK9HgyxiSU.'
+'*0'

/home/jay/phpass/phpass/tests/library/Phpass/Hash/Adapter/ExtDesTest.php:143

FAILURES!
Tests: 55, Assertions: 192, Failures: 2.

And here is crypt() run at our web host:

$ php -r 'echo crypt("password", "_z24"), " - ", crypt("password", "_z2"), "\n";'
_zPXZFl8kpcQw - _zPXZFl8kpcQw
$ php --version
PHP 5.3.10-1byte1 (cli) (built: Feb 23 2012 16:02:43)
Copyright (c) 1997-2012 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies

@rchouinard
Copy link
Owner

I would say for some reason your system lacks support for CRYPT_EXT_DES. That isn't supposed to happen in php 5.3+, but I'm not surprised from the issues I've encountered with crypt() so far.

Everything else is passing, so unless you require the ExtDes module the library is still safe to use on your system. I'll have to investigate the Debian php package a bit and see what's going on, and obviously need to add some additional checks to the library.

@alisqi
Copy link
Author

alisqi commented Jul 6, 2012

PHP reports

$ php -r 'echo CRYPT_EXT_DES;'
1

But apparently it's lying.

I don't need it. I use the default (which is BLOWFISH, right?).

@rchouinard rchouinard reopened this Jul 6, 2012
@ghost ghost assigned rchouinard Jul 6, 2012
@rchouinard
Copy link
Owner

Yes, the default is bcrypt with a cost factor of 12.

@photodude
Copy link

@rchouinard What's the status of this?

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