patternphpMinor
SHA256 password hash generation and verification
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:
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:
Improvements
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 saltImprovements
- Removed
$saltfrom 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 saltContext
StackExchange Code Review Q#9282, answer score: 3
Revisions (0)
No revisions yet.