Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #29863 [Security] Do not mix password_*() API with libsodium one …
…(chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Do not mix password_*() API with libsodium one

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | n/a
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Argon2IPasswordEncoder uses native `password_hash()` and `password_verify()` functions if the current PHP installation embeds Argon2 support (>=7.2, compiled `--with-password-argon2`).
Otherwise, it fallbacks to the libsodium extension.

This was fine at time the encoder was introduced, but meanwhile libsodium changed the algorithm used by `sodium_crypto_pwhash_str()` which is now argon2id, that goes outside of the scope of the encoder which was designed to deal with `argon2i` only.
Nothing we can do as databases may already contain passwords hashed with argon2id, the encoder must keep validating those.

However, the PHP installation may change as time goes by, and could suddenly embed the Argon2 core integration. In this case, the encoder would use the `password_verify()` function which would fail in case the password was not hashed using argon2i.
This PR prevents it by detecting that argon2id was used, avoiding usage of `password_verify()`.

See https://github.com/jedisct1/libsodium-php/issues/194 and #28093 for references.
Patch cannot be tested as it is platform dependent.

Side note: I'm currently working on a new implementation for 4.3 that will properly supports argon2id (which has been added to the PHP core sodium integration in 7.3) and argon2i, distinctively.

Commits
-------

d6cfde9 [Security] Do not mix usage of password_*() functions and sodium_*() ones
  • Loading branch information
Robin Chalas committed Jan 18, 2019
2 parents e231edd + d6cfde9 commit b972d15
Showing 1 changed file with 3 additions and 1 deletion.
Expand Up @@ -60,7 +60,9 @@ public function encodePassword($raw, $salt)
*/
public function isPasswordValid($encoded, $raw, $salt)
{
if (\PHP_VERSION_ID >= 70200 && \defined('PASSWORD_ARGON2I')) {
// If $encoded was created via "sodium_crypto_pwhash_str()", the hashing algorithm may be "argon2id" instead of "argon2i".
// In this case, "password_verify()" cannot be used.
if (\PHP_VERSION_ID >= 70200 && \defined('PASSWORD_ARGON2I') && (false === strpos($encoded, '$argon2id$'))) {
return !$this->isPasswordTooLong($raw) && password_verify($raw, $encoded);
}
if (\function_exists('sodium_crypto_pwhash_str_verify')) {
Expand Down

0 comments on commit b972d15

Please sign in to comment.