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 <ircmax...@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 >