HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

SHA256 password hash generation and verification

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
hashpasswordgenerationandsha256verification

Problem

I would like to optimise this code. It seems a bit verbose, particularly with the 'elseif' that doesn't do anything.

This code either:

  • takes a plain text password, generates a salt, and returns them both OR



  • takes a plain text password and a salt with which to encrypt the password, and returns them both (even though the salt has already been supplied).



class Member extends ActiveRecord\Model {

private function process_password($password, $salt = '') {
    /**
     * Process a password to encrypt it for storage or verification.
     * @param string - plain text password for processing.
     * @access private
     * @return array Containing the encrypted password and the salt.
     */

    //Check if the salt has been supplied. If not, generate one.
    if (!$salt) {
        $salt = bin2hex(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM));
    } elseif ($salt && strlen($salt) == 64)  {
        //Do nothing, the salt has already been supplied here.
    } else {
        log_message('info', 'Supplied password to process_password() was not the correct 64-byte length.');
        return false;
    }

    $hashed_password = hash('sha256', $password.$salt);

    return array('password' => $hashed_password, 'salt' => $salt);

}
}

Solution

How about the following:

if (!$salt) {
    $salt = bin2hex(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM));
} elseif (strlen($salt) !== 64)  {
    log_message('info', 'Supplied password to process_password() was not the correct 64-byte length.');
    return false;
}
// if we're here, we have a valid salt


Improvements

  • Removed $salt from second condition (already known to be TRUE)



  • Flipped length logic, moved else code into second block



  • Removed empty condition block

Code Snippets

if (!$salt) {
    $salt = bin2hex(mcrypt_create_iv(32, MCRYPT_DEV_URANDOM));
} elseif (strlen($salt) !== 64)  {
    log_message('info', 'Supplied password to process_password() was not the correct 64-byte length.');
    return false;
}
// if we're here, we have a valid salt

Context

StackExchange Code Review Q#9282, answer score: 3

Revisions (0)

No revisions yet.