MJ Convento

/

PHP 7.2, libsodium, and the password hashing we finally fixed

1,258 words, about 6 min read

The users table on this client’s app has 4,812 rows and three generations of password hashing in one column: 3,104 bare md5() strings, 1,690 sha1() strings with a per-user value in a password_salt column, and 18 bcrypt hashes from somebody’s abandoned attempt in 2016. I inherited it in March. We are eleven weeks past the fix now and I can finally write down what worked, because the fix is not the one I wanted.

PHP 7.2.0 landed on 30 November 2017. We are on 7.2.9 on this app, with 7.2.10 out on Thursday and scheduled for next week’s window. Getting there was its own two days: the count() of a non-countable value now raises an E_WARNING, which produced 2,400 lines in the first hour from one helper that counted a null, and undefined constants were promoted from notice to warning in the same release, which found a bare DB_PREFIX in a file nobody had opened since 2013.

What 7.2 actually shipped for crypto

Three things in the release announcement touch this work. Sodium is a core extension now, so modern authenticated encryption is available without PECL. The password hashing API gained Argon2i through PASSWORD_ARGON2I plus three constants for the default cost factors. And mcrypt is gone from core. The one piece of new syntax I have used since is the object type, usable for parameter and return typing of any object, which replaced two @param mixed doc blocks in our serializer with something the engine checks.

The mcrypt removal was the part that forced work. The RFC deprecated every mcrypt_* function with an E_DEPRECATED in 7.1 and moved the extension out of core in the release after, which turned out to be 7.2. This app had two mcrypt_encrypt() calls behind a long-lived remember-me cookie. Rewriting against openssl_encrypt() was not a like for like swap: mcrypt padded plaintext with null bytes and OpenSSL uses PKCS#7, so old cookies decrypted with trailing nulls attached. We logged every mismatch for a week, then stopped trying to read the old format and let those sessions expire.

Argon2i is a build flag, not a language feature

I wanted Argon2i and did not get it, and the reason is in the RFC rather than in any release blurb. Support comes from passing --with-password-argon2[=DIR] to configure, which links against libargon2. If PHP was not built with that flag, the constant does not exist and neither does the algorithm. The RFC says plainly that libargon2 was not widely packaged and that using the feature would mean compiling the library yourself.

So I checked instead of assuming. php -r 'var_dump(defined("PASSWORD_ARGON2I"));' on every box: true on the two web servers, false on the worker that runs the imports, because that one has a hand-compiled 7.2 from February that predates anybody caring. A hash written on a web server would have been unverifiable on the worker, which does password checks for an internal endpoint. That is a fleet problem, not a code problem, and it is the sort of thing you find at 2am if you find it after deploying.

The other detail worth reading before choosing Argon2i is the cost table. The RFC’s defaults are 1024 KiB of memory, a time cost of 2, and 2 threads, set deliberately low so they do not exhaust small or shared machines, with an explicit instruction to raise them for your own hardware. Defaults that mild are not a security setting, they are a starting point. Meanwhile PASSWORD_DEFAULT stayed bcrypt in 7.2: the Argon2i RFC dropped its proposal to change the default. Argon2id, which is the variant now recommended over Argon2i, is accepted for 7.3, not available here.

We shipped bcrypt through PASSWORD_DEFAULT with cost at 12, timed at 230ms on the smallest box we own. When the worker is rebuilt and every server reports the same algorithms, Argon2 becomes a one line change to a constant, and by then 7.3 will be out and the choice will be Argon2id.

You cannot bulk convert hashes

I spent an afternoon looking for a migration that would convert 4,794 old rows in one pass. There isn’t one, and the reason is the whole point of hashing: password_hash() needs the password, and all I have is the digest. A hash is not a reversible encoding of the plaintext.

The tempting workaround is to wrap: store password_hash(md5($plain), PASSWORD_DEFAULT) and keep applying md5() to every login attempt before verifying. It works. It also makes md5 a permanent part of the verification path, collapses the input to 32 hex characters, and means the migration can never be finished. We rejected it in about ten minutes.

The rehash on login, and what is left

That leaves the only honest option: rehash at the moment the plaintext is briefly in memory, which is login. password_needs_rehash() does the deciding, and the useful part is documented behaviour: it returns true when the hash does not implement the given algorithm and options, so an md5 or sha1 string reports true for the same reason a cost-10 bcrypt hash does.

final class Login
{
    private const OPTIONS = ['cost' => 12];

    public function attempt(string $email, string $plain): ?User
    {
        $user = $this->users->findByEmail($email);

        if ($user === null || !$this->verify($user, $plain)) {
            return null;
        }

        // Legacy md5 and sha1 strings are not recognised by the password
        // API, so this returns true for them too.
        if (password_needs_rehash($user->password, PASSWORD_DEFAULT, self::OPTIONS)) {
            $this->users->replacePassword(
                $user->id,
                password_hash($plain, PASSWORD_DEFAULT, self::OPTIONS)
            );
        }

        return $user;
    }

    private function verify(User $user, string $plain): bool
    {
        $hash = $user->password;

        if (strncmp($hash, '$2y$', 4) === 0) {
            return password_verify($plain, $hash);
        }

        if ($user->passwordSalt !== null) {
            return hash_equals($hash, sha1($user->passwordSalt . $plain));
        }

        return hash_equals($hash, md5($plain));
    }
}

replacePassword() also sets password_salt to null in the same statement, because a stale salt beside a bcrypt hash is exactly the kind of leftover that gets someone to write a clever fallback two years from now. The legacy branches use hash_equals() rather than ===, which does nothing for md5’s real problem but costs nothing either.

Eleven weeks in: 2,977 of 4,812 rows are bcrypt, 1,835 are not, and 1,402 of those have not logged in since 2016. Rehash on login converts active users and does nothing at all for dormant ones, which is the part nobody tells you. The plan is a forced reset mail to the tail in November and a hard cutoff after that, and I would rather do it in November than explain in a year why the column still contains md5.

Sources