rchouinard / phpass

PHP Password Library: Easy, secure password management for PHP
http://rchouinard.github.com/phpass/
MIT License
246 stars 28 forks source link

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

Open alisqi opened 12 years ago

alisqi commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

photodude commented 7 years ago

@rchouinard What's the status of this?