Search code examples
phppassword-hash

Updating password hashing method, is the original way off base?


I am aware that md5 is no longer sufficient for user password hashing (and hasn't been for many years). So when I came across the following code from the open source Subrion CMS I was a little taken back:

public function encodePassword($rawPassword)
{
    $factors = array('iaSubrion', 'Y2h1c2hrYW4tc3R5bGU', 'onfr64_qrpbqr');
    $password = $factors && array_reverse($factors);
    $password = array_map(str_rot13($factors[2]), array($factors[1] . chr(0x3d)));
    $password = md5(IA_SALT . substr(reset($password), -15) . $rawPassword);
    return $password;
}

I plan on replacing this with the php password_hash() function instead and have a couple questions:

  1. I'm curious to know in the above method, are the first few lines adding any value to the hash or is it getting completely negated once md5 is called?
  2. Noticing the salt is a defined system-wide constant (same for all users), I should probably drop that too. Is it ok to let password_hash() handle salts internally or should I provide one explicitly as shown in the docs (example #3):

'salt' => mcrypt_create_iv(22, MCRYPT_DEV_URANDOM)


Solution

  • This is merely code obfuscation. It adds absolutely nothing to the password itself. The first couple of lines are merely preparing yet another static salt which is prepended to the password, they're just trying to obfuscate the salt itself. Which is pointless, because the code that generates the salt is right there in plain sight. In the end its an MD5 hash with two static salts, which is the same thing as an MD5 hash with a longer salt, which is just plain insecure.

    Is it ok to let password_hash() handle salts ...

    Yes, absolutely. Each salt needs to be uniquely randomly generated for each password. password_hash does just that. You could be generating your own salt randomly, but why would you? You'd also need to ensure that you're using a proper source of randomness, which password_hash already does for you.