Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113015
MIME-Version: 1.0
References: <CAG9XoMQQLEWQABqy+cMzCcs_EeST5AQT2XxDwuK7EXDgVSQb0A@mail.gmail.com>
In-Reply-To: <CAG9XoMQQLEWQABqy+cMzCcs_EeST5AQT2XxDwuK7EXDgVSQb0A@mail.gmail.com>
Date: Thu, 28 Jan 2021 11:42:20 -0500
Message-ID: <CAAyV7nGdUB7oNdjGdKSBEtZHOyoKnfgJNp8TXzV7fCFV0KsgQA@mail.gmail.com>
To: Benjamin Morel <benjamin.morel@gmail.com>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000ee2c3605b9f892f1"
Subject: Re: [PHP-DEV] password_verify() and unknown algos
From: ircmaxell@gmail.com (Anthony Ferrara)

--000000000000ee2c3605b9f892f1
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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 upgrading
> 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 i=
f
> 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, shou=
ld
> 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 introspect
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/security
tradeoffs. Does not throwing a warning improve security? Not in most cases.
But can throwing a warning make an attackers job easier (or at least point
them in the right direction)? Perhaps.

My $0.02 at least

Anthony

=E2=80=94 Benjamin
>

--000000000000ee2c3605b9f892f1--