Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:62959
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAAyV7nFe0LUE4tObPaz3Mp1M657tmjxvNJo_fG-PeNSCJW01pw@mail.gmail.com>
References: <CAAyV7nHZ6Kr7yKF0B-UgauN6eKKxRsqCztP9w9hUzOdiPLHYAw@mail.gmail.com>
	<CADNQb0USW3j7BWEjvDwOdCKoLy3xVJ8eCRjZ610ekKmTL7R9VQ@mail.gmail.com>
	<CAAyV7nGA=1pWLj1G+X1EvoNB0jb4ji6oJ2tGZNwXH=CCae_9wA@mail.gmail.com>
	<CADNQb0XRrh21QX1qmqvHpvONRSOARXQpC8XroCFiw2z6F2Px5Q@mail.gmail.com>
	<CAAyV7nGSfZPbt23+0Yy2HGdc-gh2=Hf83=y2_e6V=pXi9Tg2XA@mail.gmail.com>
	<CAAyV7nFe0LUE4tObPaz3Mp1M657tmjxvNJo_fG-PeNSCJW01pw@mail.gmail.com>
Date: Wed, 12 Sep 2012 12:02:27 -0400
Message-ID: <CAAyV7nG7kJrWeAtphNManq9+ZnDuEchniN-PqOiRvRjx6zqkPw@mail.gmail.com>
To: Hannes Magnusson <hannes.magnusson@gmail.com>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary=f46d0437452139eace04c98351aa
Subject: Re: [PHP-DEV] [VOTE] Add simplified password hashing API
From: ircmaxell@gmail.com (Anthony Ferrara)

--f46d0437452139eace04c98351aa
Content-Type: text/plain; charset=ISO-8859-1

All,

I have added the tests and ensured that everything seems pretty clean. I
have opened a Pull Request for this item as I would like to get more eyes
on it (especially since it touches crypt()). Please review the PR and
comment away.

https://github.com/php/php-src/pull/191/files

Once it looks good, I'll merge it in. I just wanted to get more eyes on the
diff first...

Thanks

Anthony

On Wed, Sep 12, 2012 at 10:02 AM, Anthony Ferrara <ircmaxell@gmail.com>wrote:

> Hello all,
>
> I've closed the vote and it's been accepted with a vote total of 19:0,
> unanimous. I've moved the RFC into Accepted.
>
> I'm going to add the remaining tests, and then move it into master later.
>
> As for the PECL extension route, I'll work on splitting it into a PECl
> extension for 5.3/5.4 at the same time.
>
> Thanks all,
>
> Anthony
>
> On Tue, Sep 11, 2012 at 7:58 AM, Anthony Ferrara <ircmaxell@gmail.com>wrote:
>
>> Hannes,
>>
>> > First off, this has been discussed on the list for literally months.
>>>  Why
>>> > wait until the day before voting can end before bringing this up?
>>>
>>> So commenting is strictly forbidden during votes?
>>
>>
>> Not in the least. Just pointing out that this discussion could have been
>> better if it was started much earlier (prior to the time and effort being
>> put into it). Just pointing it out. There's nothing "wrong" (procedurally)
>> about bringing it up now, just that it would have been nice to hear this
>> point earlier in the discussion...
>>
>>
>>> > Secondly, the main reason for not developing this as an extension is
>>> that
>>> > there's really no benefit to it. There are little to no performance
>>> gains to
>>> > be had by the C implementation. It can live quite as easily as a PHP
>>> > library.
>>>
>>> The benefit is that it can be tested properly and bugs discovered and
>>> ironed out first.
>>>
>>
>> I'm not so sure about that. If it was widely adopted as an extension,
>> sure. But I would predict a very limited adoption. Almost purely academic.
>> The reason is that there's no advantage to doing it in C other than to
>> provide a working API for those who don't understand how to build it (or
>> realize the benefits thereto). Those are the same people who wouldn't be
>> able to install a PECL extension (know-how or access). And projects who
>> realize this is needed are likely already using one of the many libraries
>> available.
>>
>> So I'm not sure the install rate would be anywhere high enough to work
>> any significant issues out. Additionally, there are testing phases for the
>> release that would hopefully catch any major issues prior to 5.5 going
>> gold...
>>
>>
>>> This is not the sort of thing you want to get security bug reports the
>>> day after its released in core.
>>>
>>
>> Sure. Which is why I've been going around looking for security reviews
>> (posted to the crypt-dev list on openwall, and to
>> security.stackexchange.com).
>>
>>
>>> If your ego is big enough you can guarantee you have tested this
>>> thoroughly and want it to become the recommended way.. You have to be
>>> damn sure you don't fuck it up.
>>>
>>
>> I want to say that this is really unfair. By saying it this way you've
>> pushed me into a corner. I disagree that your suggestion would have a
>> result of any significant gain in security. Yet I cannot disagree with you
>> without coming across as an egotist now that you said it that way. Please
>> try to keep things civil without setting up traps to try to prove your
>> point. I don't appreciate this remark at all...
>>
>>
>>> This is exactly the sort of thing that doesn't need to be developed in
>>> the core tree, but can later be merged in once proven successful.
>>>
>>
>> As indicated before, if that's the case, then it would never be merged.
>> Because this would never be successful as a stand alone PECL module. The
>> primary reason for adding it is so that people who don't know any better
>> would be able to use a secure API. That is the antithesis of a PECL
>> extension.
>>
>>
>>> >> Especially considering the patch is unfinished.
>>> >
>>> >
>>> > Aside from adding a few more tests, what's unfinished? If you're
>>> referring
>>> > to the line in the RFC, I just haven't updated it. The patch has been
>>> worked
>>> > on and is in a place where I'd be comfortable submitting it...
>>>
>>> The test suite seems very limited, and the code seems to be waiting
>>> for more algorithms to be implemented.
>>>
>>
>> Yes, the test suite needs to be expanded. I tested it additionally with
>> the PHPUnit tests that I wrote for the PHP fallback, so it's been tested
>> fairly well... I just need to expand the phpt test suite included in core...
>>
>> As far as more algorithms, right now there's no reason to add more
>> algorithms. The whole point was to never add anything but the strongest
>> algorithm available. Which today is bcrypt. But as time goes on, and better
>> algorithms become available, the API is designed so that they can be
>> implemented. But today, there's no other algorithm that should be
>> implemented. (SCrypt is a candidate, but since it lacks crypt(3) bindings,
>> and is still very young, it's inclusion isn't prudent at this point).
>>
>> There's a section to the RFC for adding new algorithms as time goes on:
>> https://wiki.php.net/rfc/password_hash#updating_password_default
>>
>> Anthony
>>
>
>

--f46d0437452139eace04c98351aa--