On Sun, Jun 07, 2009 at 11:06:45PM +0200, Pierre Joye wrote: > Thank for your work, I'm the one who merged your implementation to PHP > (we had a discussion about it btw, per email if you remember :).
Yes, I recalled that, and this is why I CC'ed you on my posting. > Thanks for the review and the notice, I'll update the code tomorrow > (5.3+ have it). I see that the __x86_64__ check went in. Thanks! > > Finally, I'd like to ask whoever made the following change about the > > rationale behind it: > > Backward compatibility was the reason. I like to drop this thing in > php6 but it was not possible to do it in a minor release (I discussed > it with Stefan Esser last year). I'm not sure yet how but certainly by > providing another function or changing the API to allow one to disable > the old behavior. Here's what I found out: 1. The documentation at http://www.php.net/manual/en/function.crypt.php remains wrong. It says: "CRYPT_BLOWFISH - Blowfish encryption with a sixteen character salt starting with $2$ or $2a$" Well, the salt string should be 22 characters (after the "$2a$nn$" prefix, or 29 characters including this prefix). The salt is 16 bytes, not 16 characters. 2. "Example #3 Using crypt() with different encryption types" at http://www.php.net/manual/en/function.crypt.php is also wrong, in a different way. It has: echo 'Blowfish: ' . crypt('rasmuslerdorf', '$2a$07$rasmuslerd...........$') . "\n"; Apparently, this works with the workaround in the Suhosin patch, and thus with current PHP 5.3.0 development code, but it fails on systems that have bcrypt hashes supported natively (in libc or libcrypt) and do not have the workaround in there. This comment is just right: http://www.php.net/manual/en/function.crypt.php#71748 3. Yes, I now see that the workaround came from a revision of my code in the Suhosin patch. However, it is not clear to me what exactly this workaround was for, and backwards compatibility with what it provides now. Is it just compatibility with the above incorrect example in the documentation? If so, I see no problem with dropping it now, because the example did not work universally anyway (it failed on many or probably even on most systems that had support for bcrypt hashes at all). I hope these observations are of some help. A somewhat related concern: what does PHP's crypt() return on error? In my crypt_blowfish package, I had a higher-level wrapper function that would guarantee return of a string not matching the input "salt" string, thus guaranteeing that authentication will fail. I don't think PHP currently has similar code; perhaps it should. Also, documentation should be revised to document behavior of crypt() on error, for both historical and current/future versions of PHP. Because in many cases this depends on the underlying system libraries, I suggest that advice be given to treat any return strings shorter than 13 characters as indicating an error. In fact, PHP itself could start to implement the safe behavior by checking whether the would-be crypt() return string is shorter than 13 characters (regardless of where it came from). If so, return "*0" if the input salt string does not start with "*0", or return "*1" if the input salt string starts with "*0". Here's an excerpt from wrapper.c in my crypt_blowfish package: output[0] = '*'; output[1] = '0'; output[2] = '\0'; if (setting[0] == '*' && setting[1] == '0') output[1] = '1'; Thanks, Alexander -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
