Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75697
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.220.175 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXaJBKL8b=ewy1VY5kWZ5G__yr=4kpJrFfxBrxr3EbntaA@mail.gmail.com>
References: <CAGa2bXYP4L-EVjqiTvrOjbiRvzQJ7zyTQyu78O1AEEkk7nwEhQ@mail.gmail.com>
	<CAESVnVqBxo0VCYM_4V4_KzrD1+-F-0ipumrr_a=mAwaYje10Og@mail.gmail.com>
	<CAGa2bXa48gEEcE_DWE27B3UHD0Yi12bFVZsx-9t_anHunkj5Dg@mail.gmail.com>
	<CAHMUw2FLW06ABT=1RoH2gMuM7HbGDdSnLzxqfv8wNVp0rzfw+Q@mail.gmail.com>
	<CAGa2bXZF5sznggoYRFTtngO32b60NVG=6KqUBDB3guGb7_8oMA@mail.gmail.com>
	<CAHMUw2GyQOevKfZXuyd7S-PPUnShVVeoRvZNHK3q9w1yEe1wow@mail.gmail.com>
	<CADajX4C9sip=57QGaacZVq9cmAFhH6CJWpvSSjB7c921TGsJMQ@mail.gmail.com>
	<CAAyV7nFva4u-pW0Ago-VpkJ=-RuBvE_fjCg27Wza8_T6M0oYHA@mail.gmail.com>
	<CAGa2bXYkdBjoSpDJekPq=hxbty7GexkjyJnPNpH4vM4+v-v64Q@mail.gmail.com>
	<CAAyV7nF6HcdUcx_nYC2=87mkEGNk2AAhz3vtH9VGY0orwnzY+w@mail.gmail.com>
	<CAGa2bXaJBKL8b=ewy1VY5kWZ5G__yr=4kpJrFfxBrxr3EbntaA@mail.gmail.com>
Date: Sat, 19 Jul 2014 11:27:40 -0400
Message-ID: <CAAyV7nFE8XMgejPeRj5+ie6X+YH_w4y+Un9nksDdkqF86oo4hg@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: Adam Harvey <aharvey@php.net>, Tjerk Meesters <tjerk.meesters@gmail.com>, 
	Sara Golemon <pollita@php.net>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] crypt() BC issue
From: ircmaxell@gmail.com (Anthony Ferrara)

Yasuo

> I'll suggest users to use SHA512 raw output as password to
> remove 72 chars limitation if it is needed.

Then you either misunderstood what I was saying, or completely ignored it.

> Raising E_NOTICE for too long password for PASSWORD_BCRYPT
> makes sense. I'll add it later.
>
> https://bugs.php.net/bug.php?id=67653

Please, don't change anything without first proposing an RFC so that
sufficient discussion can occur. Literally every single person in this
thread has disagreed with you, yet you want to go ahead and make
changes. It's not saying that you are wrong, but that discussion needs
to happen before any changes to security sensitive code and
documentation.

Also, Deprecating crypt() without first discussing it (and having an
RFC to vote on) is not cool (and has been reverted):
http://svn.php.net/viewvc/phpdoc/en/trunk/reference/strings/functions/crypt.xml?r1=333973&r2=334296



> Generally speaking, it would not be serious issue. 72 bytes constant prefix
> would
> not be used most likely.

Correct. So if it's not a serious issue, why are we talking about
adding serious warnings to documentation pages (big red boxes), and
notices and pre-hashing?

> However, bug like this in "authentication" code must be detected  and
> fixed.

It's not a bug. It's by design. You could argue if it's a good design
or not, but it's very much by design.

> If password should be truncated, it should be truncated by app developers
> explicitly and
> notified users that their password had been truncated.  IMHO.

We disagree. So rather than just committing changes, why not draft up
an RFC with your suggestions, which can be discussed and voted upon?




> There's already a notice about this in the password_hash() docs, one that
> almost looks like is designed to scare users, which is bad. Throwing an
> E_NOTICE will cause more problems than it would supposedly solve.

I agree 100%. It's on my list of things to do to "clean up" that
warning, to word it to be less scary. And I agree 100% on the notice
side as well.

> Application developers should just state this limitation on their
> registration/password change pages, anything else is pointless.

I don't think so. If the end user enters a 72 character+ password, the
chances of a collision because of the truncation are so small that
it's not worth worrying about.

However, it's worth documenting, as what you don't want is people
inventing their own crypto by prefixing their own secret:

    password_hash(hash('sha512', SECRET_KEY) . $password, PASSWORD_DEFAULT)

Which would then always produce the same hash (since the sha512 hash
is 128 bytes long, truncation would happen too early).

We should be educating people to avoid their own crypto. That was the
entire point of the password_hash APIs. Make it so simple that it's
difficult to screw up. Yet people will always find a way. The only way
to solve that problem is education. And adding big red boxes isn't
education. It's fear.

Anthony