Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113021
MIME-Version: 1.0
References: <CAG9XoMQQLEWQABqy+cMzCcs_EeST5AQT2XxDwuK7EXDgVSQb0A@mail.gmail.com>
 <CAAyV7nGdUB7oNdjGdKSBEtZHOyoKnfgJNp8TXzV7fCFV0KsgQA@mail.gmail.com>
In-Reply-To: <CAAyV7nGdUB7oNdjGdKSBEtZHOyoKnfgJNp8TXzV7fCFV0KsgQA@mail.gmail.com>
Date: Fri, 29 Jan 2021 09:45:08 -0500
Message-ID: <CAES=RMvWUkiRS89V4v0_TEmtq7HD0XLXb+xfOoyOQKYL2r8U2g@mail.gmail.com>
To: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000009e30af05ba0b0d58"
Subject: Re: [PHP-DEV] password_verify() and unknown algos
From: jr@rushlow.dev (Jesse Rushlow)

--0000000000009e30af05ba0b0d58
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

>
> There are definitely a fair number of applications that use the above
> method to ensure backwards compatibility and a solid upgrade path, and as
> such I would be resistant to adding warnings/errors/exceptions here.
>

I think Anthony makes a valid point, to preserve BC adding errors /
exceptions may not be the best approach by default. However having a third
param to password_verify(), that is false by default, that would allow for
an exception to be thrown in the event of an unknown algo / bad hash might
be a better path forward and would be totally beneficial in my opinion.

Thanks
Jesse Rushlow

On Thu, Jan 28, 2021 at 11:42 AM Anthony Ferrara <ircmaxell@gmail.com>
wrote:

> On Wed, Jan 27, 2021 at 11:27 AM Benjamin Morel <benjamin.morel@gmail.com=
>
> wrote:
>
> > Hi internals,
> >
> > I just spent some time debugging an authentication issue after upgradin=
g
> > PHP, and realized that it was due to ext-sodium not being installed, so
> > password_verify() would always return false for argon2i hashes.
> >
> > Digging a bit more, I realized that password_verify() does not complain
> if
> > the algorithm is unknown, or if the hash string is malformed:
> >
> >     var_export(password_verify('passw0rd', 'any/string%as|a$hash')); //
> > false
> >
> > Shouldn't it throw an exception, or a least trigger a warning, when the
> > algorithm is unknown, or the hash is malformed? Returning false IMO,
> should
> > mean "I recognize this hash, but it doesn't match your password". "I
> don't
> > recognize this hash" is an application issue and should be reported.
> >
> > What do you think?
> >
> >
> Hey all,
>
> I just wanted to drop in and make a note here. When I designed
> password_verify, the lack of validation/errors on invalid hases was 100%
> explicit and intentional. The primary reason is to support multiple
> validation strategies in order to allow for migration paths between
> different storage mechanisms.
>
> Take an example where someone used to use Wordpress's custom phpass based
> $P$ system. They now want to migrate to bcrypt/argon/etc. If
> password_verify threw errors the validation system would need to introspe=
ct
> each hash to determine how to validate it (note, this happens in
> password_verify and the other validators already). But without errors, it
> becomes a simple fallthrough:
>
> function validate($password, $hash) {
>     if (password_verify($password, $hash)) return true;
>     if (legacy_verify($password, $hash) || other_legacy_verify($password,
> $hash)) {
>         update_password_hash($password);
>         return true;
>     }
>     return false;
> }
>
> There are definitely a fair number of applications that use the above
> method to ensure backwards compatibility and a solid upgrade path, and as
> such I would be resistant to adding warnings/errors/exceptions here.
>
> The case for "it's an application issue" is definitely valid, though that=
's
> also what password_info was added to provide (it is easy to introspect).
>
> One other point I'd make is to normally suggest not disclosing any
> information about cryptographic material via error handling mechanisms
> (it's too easy to expose to attackers). This is one of those dev-x/securi=
ty
> tradeoffs. Does not throwing a warning improve security? Not in most case=
s.
> But can throwing a warning make an attackers job easier (or at least poin=
t
> them in the right direction)? Perhaps.
>
> My $0.02 at least
>
> Anthony
>
> =E2=80=94 Benjamin
> >
>

--0000000000009e30af05ba0b0d58--