Search code examples
phpsecuritypasswordscryptographypassword-hash

Is this a good hashing password function in PHP? If not, why not?


I'm wondering if this function (which is in part taken from a ~2 year old phpBB version), is good enough.

If not, why?
And how would you change it (making the transition seamless for existing users) ?

The result of hash_pwd() is what will be saved in a DB.

function hash_pwd($password)
{
    $itoa64 = './0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';

    $random_state = $this->unique_id();
    $random = '';
    $count = 6;

    if (($fh = @fopen('/dev/urandom', 'rb')))
    {
        $random = fread($fh, $count);
        fclose($fh);
    }

    if (strlen($random) < $count)
    {
        $random = '';

        for ($i = 0; $i < $count; $i += 16)
        {
            $random_state = md5($this->unique_id() . $random_state);
            $random .= pack('H*', md5($random_state));
        }
        $random = substr($random, 0, $count);
    }

    $hash = $this->_hash_crypt_private($password, $this->_hash_gensalt_private($random, $itoa64), $itoa64);

    if (strlen($hash) == 34)
    {
        return $hash;
    }

    return false;
}


function unique_id()
{
    $val = microtime();
    $val = md5($val);

    return substr($val, 4, 16);
}

function _hash_crypt_private($password, $setting, &$itoa64)
{
    $output = '*';

    // Check for correct hash
    if (substr($setting, 0, 3) != '$H$')
    {
        return $output;
    }

    $count_log2 = strpos($itoa64, $setting[3]);

    if ($count_log2 < 7 || $count_log2 > 30)
    {
        return $output;
    }

    $count = 1 << $count_log2;
    $salt = substr($setting, 4, 8);

    if (strlen($salt) != 8)
    {
        return $output;
    }

    /**
    * We're kind of forced to use MD5 here since it's the only
    * cryptographic primitive available in all versions of PHP
    * currently in use.  To implement our own low-level crypto
    * in PHP would result in much worse performance and
    * consequently in lower iteration counts and hashes that are
    * quicker to crack (by non-PHP code).
    */
    if (PHP_VERSION >= 5)
    {
        $hash = md5($salt . $password, true);
        do
        {
            $hash = md5($hash . $password, true);
        }
        while (--$count);
    }
    else
    {
        $hash = pack('H*', md5($salt . $password));
        do
        {
            $hash = pack('H*', md5($hash . $password));
        }
        while (--$count);
    }

    $output = substr($setting, 0, 12);
    $output .= $this->_hash_encode64($hash, 16, $itoa64);

    return $output;
}

function _hash_gensalt_private($input, &$itoa64, $iteration_count_log2 = 6)
{
    if ($iteration_count_log2 < 4 || $iteration_count_log2 > 31)
    {
        $iteration_count_log2 = 8;
    }

    $output = '$H$';
    $output .= $itoa64[min($iteration_count_log2 + ((PHP_VERSION >= 5) ? 5 : 3), 30)];
    $output .= $this->_hash_encode64($input, 6, $itoa64);

    return $output;
}

function _hash_encode64($input, $count, &$itoa64)
{
    $output = '';
    $i = 0;

    do
    {
        $value = ord($input[$i++]);
        $output .= $itoa64[$value & 0x3f];

        if ($i < $count)
        {
            $value |= ord($input[$i]) << 8;
        }

        $output .= $itoa64[($value >> 6) & 0x3f];

        if ($i++ >= $count)
        {
            break;
        }

        if ($i < $count)
        {
            $value |= ord($input[$i]) << 16;
        }

        $output .= $itoa64[($value >> 12) & 0x3f];

        if ($i++ >= $count)
        {
            break;
        }

        $output .= $itoa64[($value >> 18) & 0x3f];
    }
    while ($i < $count);

    return $output;
}

Solution

  • The code that you have given is a port of PHPASS, specifically the "portable" algorithm. Note the qualification of portable. This will only apply to the phpass library if you pass true as the second constructor parameter. From here on out in this answer, phpass refers ONLY to the portable algorithm, and not the library itself. The library will do bcrypt by default if you do not explicitly specify portable...

    The PHPBB team did not develop this themselves (a very good thing), but ported it from phpass directly (arguable).

    There are a few questions we should ask here:

    Is It Bad?

    The short answer is no, it's not bad. It offers pretty good security. If you have code on this right now, I wouldn't be in a rush to get off it. It's adequate for most usages. But with that said, there are far better alternatives if you were starting a new project that I wouldn't pick it.

    What are some weaknesses?

    • Relative To pbkdf2: The phpass algorithm uses hash() where pbkdf2() uses hash_hmac(). Now, a HMAC runs 2 hashes for every call internally, but the PHP implementation only takes approximately 1.6 times the execution of a single call to hash() (isn't C wonderful?). So we get 2 hashes from hash_hmac in 62% of the time it would take hash() to execute 2 hashes.

      What does that mean? Well, for a given runtime, pbkdf2 will run approximately 37.5% more hashes than the phpass algorithm. More hashes in a given time == good, because it results in more computation being performed.

      So pbkdf2 is approximately 37.5% stronger than phpass when using the same primitive (md5 in this case). But pbkdf2 can also take stronger primitives. So we can use pbkdf2 with sha512 to gain a very significant advantage over the phpass algorithm (mainly because sha512 is a harder algorithm with more computation than md5).

      This means that not only is pbkdf2 able to generate more computations in the same amount of time, it's able to generate harder computations.

      With that said, the difference isn't overly significant. It's very much measurable, and pbkdf2 is definitely "stronger" than phpass.

    • Relative To bcrypt: This is a lot harder of a comparison to make. But let's look at the surface of it. phpass uses md5, and a loop in PHP. pbkdf2 uses any primitive (in C) and a loop in PHP. bcrypt uses a custom algorithm all in C (meaning it's a different algorithm from any available hash). So right of the bat, bcrypt has a significant advantage just do to the fact that the algorithm is all in C. This allows for more "computation" per unit time. Thereby making it a more efficient slow algorithm (more computations in the given runtime).

      But just as important as how many computations it does is the quality of the computations. This could an entire research paper, but in short it comes down to the fact that the computations that bcrypt uses internally are much harder to perform than a normal hash function.

      One example of the stronger nature of bcrypt is the fact that bcrypt uses a far larger internal state than a normal hash function. SHA512 uses a 512 bit internal state to compute against a block of 1024 bits. bcrypt uses instead about 32kb of internal state to compute against a single block of 576 bits. The fact that bcrypt's internal state is so much bigger than SHA512 (and md5 and phpass) partially accounts for the stronger nature of bcrypt.

    Should It Be Avoided

    For new projects, absolutely. It's not that it's bad. It isn't. It's that there are demonstrably stronger algorithms out there (by orders of magnitude). So why not use them?

    For further proof of how bcrypt is stronger, check out the Slides from Password13 (PDF) which launched a 25 GPU cluster for cracking password hashes. Here are the relevant results:

    • md5($password)
      • 180 BILLION guesses per second
      • 9.4 Hours - All possible 8 character passwords
    • sha1($password)
      • 61 BILLION guesses per second
      • 27 Hours - All possible 8 character passwords
    • md5crypt (which is very similar to phpass with a cost of 10):
      • 77 Million guesses per second
      • 2.5 Years - All possible 8 character passwords
    • bcrypt with a cost of 5
      • 71 Thousand guesses per second
      • 2700 Years - All possible 8 character passwords

    Note: all possible 8 character passwords are using a 94 character set:

    a-zA-Z0-9~`!@#$%^&*()_+-={}|[]\:";'<>,.?/
    

    The Bottom Line

    So if you're writing new code, without a doubt use bcrypt. If you have phpass or pbkdf2 in production now, you may want to upgrade, but it's not a clear cut "you're significantly vulnerable".