FAQ
Hi Alexander,
On Sun, Mar 27, 2016 at 10:16 PM, Solar Designer wrote:
Hi,

This commit:

commit 03315d9625dc87515f1dfbf1cc7d53c4451b5ec9
Author: Pierre Joye <pajoye@php.net>
Date: Mon Jul 18 21:26:29 2011 +0000

- update blowfish to 1.2 (Solar Designer)

documented this hack:

$ git show 03315d9625dc87515f1dfbf1cc7d53c4451b5ec9 | fgrep -i hack
+ if (tmp == '$') break; /* PHP hack */ \
+ while (dptr < end) /* PHP hack */

I vaguely recall a PHP developer (Pierre?) mentioning this at the time,
and I guess I didn't object strongly enough - perhaps because the hack
itself was in there before.
I cannot remember why I added this comment there but as far as I can
tell now this code was present before, actually since the very first
version I use when I first make blowfish always available for PHP
using your implementation. See:

https://github.com/php/php-src/commit/1e820eca02dcf322b41fd2fe4ed2a6b8309f8ab5#diff-243b169f3d5e753b5314e8eb994e36bc

I cannot say why it is specific to PHP. Is it something that may have
been present in your implementation earlier and then removed but kept
in PHP for some BC reasons?

Well, this PHP-specific behavior confuses people in several ways. It's
especially weird that the behavior may be seen or may be gone on the
same PHP version depending on whether it uses PHP's bundled (and hacked
as above) copy of crypt_blowfish or an implementation of bcrypt provided
by the underlying system. Maybe PHP should not use the system's crypt()
for whatever hash types it has bundled code for - and not only because
of this issue.

As to the hack above, I didn't review it closely (I disapprove of it
anyhow), but maybe it doesn't even work reliably when the code is
compiled in: some people are reporting that salts are padded with dots
(which correspond to all zero bits) and some that salts are padded with
'$' signs. I've just reproduced the latter on CentOS 7.1:

$ php -r 'echo crypt("", "\$2a\$05\$0123456789"), "\n";'
$2a$05$0123456789$$$$$$$$$$$.UQ9ZZoltOie8zhLfF5KWLrJBeoos.S6
$ php -v
PHP 5.4.16 (cli) (built: Jun 23 2015 21:17:27)

Maybe there's an extra hack somewhere causing this, but I can't find it
easily. I'm not looking at the exact branch/package for CentOS, though.
Maybe it was a temporary PHP 5.4 thing.

While padding with dots (zero bits) produces valid bcrypt hash strings
(it's just that the input salt strings were invalid), padding with '$'
signs produces invalid bcrypt hash strings.

In practical terms, padding with dots (zero bits) is an unpleasant
surprise for people who choose to store their invalid salts and hashes
into separate database fields (and then authentication stops working
after some upgrade or migration, where the new system provides its
non-hacked bcrypt), and padding with '$' signs is also a similar
unpleasant surprise for people who store the full hash strings as
returned by crypt(). While those strings might work in the same way
with some bcrypt implementations, they may also be rejected (ideally,
like upstream crypt_blowfish does) or processed differently.

I'm not sure whether/how PHP should recover from this now.


--
Pierre

@pierrejoye | http://www.libgd.org

Search Discussions

Discussion Posts

Previous

Follow ups

Related Discussions

Discussion Navigation
viewthread | post
posts ‹ prev | 2 of 3 | next ›
Discussion Overview
groupphp-internals @
categoriesphp
postedMar 27, '16 at 3:16p
activeMar 28, '16 at 2:22p
posts3
users2
websitephp.net

2 users in discussion

Solar Designer: 2 posts Pierre Joye: 1 post

People

Translate

site design / logo © 2019 Grokbase