password_verify() and unknown algos

On Wed, Jan 27, 2021 at 10:27 AM Benjamin Morel benjamin.morel@gmail.com
wrote:

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

+1

Two very different error cases deserve two very different handling modes.
Technically the tools exist to examine a hash and compare it against a
known list of algos, but that's honestly more work that the problem should
require. Matching passwords should be idiot proof.

-Sara

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.

Relevantly, password_hash() throws a ValueError for an unknown $algo
parameter as of 8.0:
https://heap.space/xref/php-src/ext/standard/password.c?r=3e01f5af#663

It would probably make sense to throw the same error if
php_password_algo_identify doesn't recognise the ident in the hash.

Regards,

--
Rowan Tommins
[IMSoP]

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 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 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

— Benjamin