>
> 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 <ircmax...@gmail.com>
wrote:
> On Wed, Jan 27, 2021 at 11:27 AM Benjamin Morel <benjamin.mo...@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
> >
>
