Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:44207
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 72.14.220.156 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :cc:content-type:content-transfer-encoding;
        b=FrsZe7ONymybAX5S/2TAKkuA+MQvEudCk6mgsF6fkPhoPm2IdD/c0LkBuOw3y8ZQtb
         9IogCcJo3DvMkFlJBimyKzbbVLsDq3dauX1+2UwIB6iyL/8bxb+KBhQn58mLiBxvDw2P
         bSREeINSHdvTrhgYj+FTBOhWfeCEdZrefmrRI=
MIME-Version: 1.0
In-Reply-To: <20090608151115.GA2328@openwall.com>
References: <20090607160019.GA30041@openwall.com>
	 <fe05d1540906071406m21556f9o767d46cb76296e8d@mail.gmail.com>
	 <20090608151115.GA2328@openwall.com>
Date: Mon, 8 Jun 2009 20:22:15 +0200
Message-ID: <fe05d1540906081122w1939bf39l80013b23f768c6d@mail.gmail.com>
To: Solar Designer <solar@openwall.com>
Cc: PHP Internals List <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: ext/standard/crypt_blowfish.c
From: pierre.php@gmail.com (Pierre Joye)

hi Alexander,

Thanks for the feedbacks! these questions are the same I had when I
was making crypt portable (having all algos available in any
situation).

Sadly I do not have the time yet to fix the behavior (remove the
workaround/BC hack or the on error behavior). If you like to (or have
the time to :), you can five it a try, that will be very helpful. Or I
can work on that next week, at the soonest.

Cheers,

On Mon, Jun 8, 2009 at 5:11 PM, Solar Designer<solar@openwall.com> wrote:
> 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. =A0Thanks!
>
>> > 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. =A0It 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). =A0The 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. =A0It has:
>
> =A0 =A0echo 'Blowfish: =A0 =A0 ' . crypt('rasmuslerdorf', '$2a$07$rasmusl=
erd...........$') . "\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. =A0This 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. =A0However, it is not clear to me what exactly this
> workaround was for, and backwards compatibility with what it provides
> now. =A0Is it just compatibility with the above incorrect example in the
> documentation? =A0If 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. =A0I don't think PHP
> currently has similar code; perhaps it should. =A0Also, documentation
> should be revised to document behavior of crypt() on error, for both
> historical and current/future versions of PHP. =A0Because 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). =A0If 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:
>
> =A0 =A0 =A0 =A0output[0] =3D '*';
> =A0 =A0 =A0 =A0output[1] =3D '0';
> =A0 =A0 =A0 =A0output[2] =3D '\0';
>
> =A0 =A0 =A0 =A0if (setting[0] =3D=3D '*' && setting[1] =3D=3D '0')
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0output[1] =3D '1';
>
> Thanks,
>
> Alexander
>



--=20
Pierre

http://blog.thepimp.net | http://www.libgd.org