From 7499a2a6c71bb3425d9dd628e907889d7c3de598 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 12 Nov 2014 19:27:47 +0900 Subject: [PATCH 1/4] Add a new class for improved password hashing --- classes/security/Password.class.php | 341 ++++++++++++++++++++++++++++ config/config.inc.php | 1 + 2 files changed, 342 insertions(+) create mode 100644 classes/security/Password.class.php diff --git a/classes/security/Password.class.php b/classes/security/Password.class.php new file mode 100644 index 000000000..0cbeb59b3 --- /dev/null +++ b/classes/security/Password.class.php @@ -0,0 +1,341 @@ + */ + +/** + * This class can be used to hash passwords using various algorithms and check their validity. + * It is fully compatible with previous defaults, while also supporting bcrypt and pbkdf2. + * + * @file Password.class.php + * @author Kijin Sung (kijin@kijinsung.com) + * @package /classes/security + * @version 1.1 + */ +class Password +{ + /** + * @brief Return the list of hashing algorithms supported by this server + * @return array + */ + public function getSupportedAlgorithms() + { + $retval = array(); + if(version_compare(PHP_VERSION, '5.3.7', '>=') && defined('CRYPT_BLOWFISH')) + { + $retval['bcrypt'] = 'bcrypt'; + } + if(function_exists('hash_hmac') && in_array('sha256', hash_algos())) + { + $retval['pbkdf2'] = 'pbkdf2'; + } + $retval['md5'] = 'md5'; + return $retval; + } + + /** + * @brief Return the best hashing algorithm supported by this server + * @return string + */ + public function getBestAlgorithm() + { + $algos = $this->getSupportedAlgorithms(); + return key($algos); + } + + /** + * @brief Return the currently selected hashing algorithm + * @return string + */ + public function getCurrentlySelectedAlgorithm() + { + if(function_exists('getModel')) + { + $config = getModel('member')->getMemberConfig(); + $algorithm = $config->password_hashing_algorithm; + if(strval($algorithm) === '') + { + $algorithm = 'md5'; // Historical default for XE + } + } + else + { + $algorithm = 'md5'; + } + return $algorithm; + } + + /** + * @brief Return the currently configured work factor for bcrypt and other adjustable algorithms + * @return int + */ + public function getWorkFactor() + { + if(function_exists('getModel')) + { + $config = getModel('member')->getMemberConfig(); + $work_factor = $config->password_hashing_work_factor; + if(!$work_factor || $work_factor < 4 || $work_factor > 31) + { + $work_factor = 8; // Reasonable default + } + } + else + { + $work_factor = 8; + } + return $work_factor; + } + + /** + * @brief Create a hash using the specified algorithm + * @param string $password The password + * @param string $algorithm The algorithm (optional) + * @return string + */ + public function createHash($password, $algorithm = null) + { + if($algorithm === null) + { + $algorithm = $this->getCurrentlySelectedAlgorithm(); + } + if(!array_key_exists($algorithm, $this->getSupportedAlgorithms())) + { + return false; + } + + $password = trim($password); + + switch($algorithm) + { + case 'md5': + return md5($password); + + case 'pbkdf2': + $iterations = pow(2, $this->getWorkFactor() + 5); + $salt = $this->createSecureSalt(12); + $hash = base64_encode($this->pbkdf2($password, $salt, 'sha256', $iterations, 24)); + return 'sha256:'.sprintf('%07d', $iterations).':'.$salt.':'.$hash; + + case 'bcrypt': + return $this->bcrypt($password); + + default: + return false; + } + } + + /** + * @brief Check if a password matches a hash + * @param string $password The password + * @param string $hash The hash + * @param string $algorithm The algorithm (optional) + * @return bool + */ + public function checkPassword($password, $hash, $algorithm = null) + { + $password = trim($password); + + if($algorithm === null) + { + $algorithm = $this->checkAlgorithm($hash); + } + if(!array_key_exists($algorithm, $this->getSupportedAlgorithms())) + { + return false; + } + + switch($algorithm) + { + case 'md5': + return md5($password) === $hash || md5(sha1(md5($password))) === $hash; + + case 'pbkdf2': + $hash = explode(':', $hash); + $hash[3] = base64_decode($hash[3]); + $hash_to_compare = $this->pbkdf2($password, $hash[2], $hash[0], intval($hash[1], 10), strlen($hash[3])); + return $this->strcmpConstantTime($hash_to_compare, $hash[3]); + + case 'bcrypt': + $hash_to_compare = $this->bcrypt($password, $hash); + return $this->strcmpConstantTime($hash_to_compare, $hash); + + default: + return false; + } + } + + /** + * @brief Check the algorithm used to create a hash + * @param string $hash The hash + * @return string + */ + function checkAlgorithm($hash) + { + if(preg_match('/^\$2[axy]\$([0-9]{2})\$/', $hash, $matches)) + { + return 'bcrypt'; + } + elseif(preg_match('/^sha[0-9]+:([0-9]+):/', $hash, $matches)) + { + return 'pbkdf2'; + } + elseif(strlen($hash) === 32 && ctype_xdigit($hash)) + { + return 'md5'; + } + else + { + return false; + } + } + + /** + * @brief Check the work factor of a hash + * @param string $hash The hash + * @return int + */ + function checkWorkFactor($hash) + { + if(preg_match('/^\$2[axy]\$([0-9]{2})\$/', $hash, $matches)) + { + return intval($matches[1], 10); + } + elseif(preg_match('/^sha[0-9]+:([0-9]+):/', $hash, $matches)) + { + return max(0, round(log($matches[1], 2)) - 5); + } + else + { + return false; + } + } + + /** + * @brief Generate a cryptographically secure random string to use as a salt + * @param int $length The number of bytes to return + * @param string $format hex or alnum + * @return string + */ + public function createSecureSalt($length, $format = 'hex') + { + // Find out how many bytes of entropy we really need + $entropy_required_bytes = ceil(($format === 'hex') ? ($length / 2) : ($length * 3 / 4)); + + // Cap entropy to 256 bits from any one source, because anything more is meaningless + $entropy_capped_bytes = min(32, $entropy_required_bytes); + + // Find and use the most secure way to generate a random string + $is_windows = (defined('PHP_OS') && strtoupper(substr(PHP_OS, 0, 3)) === 'WIN'); + if(function_exists('openssl_random_pseudo_bytes') && (!$is_windows || version_compare(PHP_VERSION, '5.4', '>='))) + { + $entropy = openssl_random_pseudo_bytes($entropy_capped_bytes); + } + elseif(function_exists('mcrypt_create_iv') && (!$is_windows || version_compare(PHP_VERSION, '5.3.7', '>='))) + { + $entropy = mcrypt_create_iv($entropy_capped_bytes, MCRYPT_DEV_URANDOM); + } + elseif(function_exists('mcrypt_create_iv') && $is_windows) + { + $entropy = mcrypt_create_iv($entropy_capped_bytes, MCRYPT_RAND); + } + elseif(!$is_windows && @is_readable('/dev/urandom')) + { + $fp = fopen('/dev/urandom', 'rb'); + $entropy = fread($fp, $entropy_capped_bytes); + fclose($fp); + } + else + { + $entropy = ''; + for($i = 0; $i < $entropy_capped_bytes; $i += 2) + { + $entropy .= pack('S', rand(0, 65536) ^ mt_rand(0, 65535)); + } + } + + // Mixing (see RFC 4086 section 5) + $output = ''; + for($i = 0; $i < $entropy_required_bytes; $i += 32) + { + $output .= hash('sha256', $entropy . $i . rand(), true); + } + + // Encode and return the random string + if($format === 'hex') + { + return substr(bin2hex($output), 0, $length); + } + else + { + $salt = substr(base64_encode($output), 0, $length); + $replacements = chr(rand(65, 90)) . chr(rand(97, 122)) . rand(0, 9); + return strtr($salt, '+/=', $replacements); + } + } + + /** + * @brief Generate the PBKDF2 hash of a string using a salt + * @param string $password The password + * @param string $salt The salt + * @param string $algorithm The algorithm (optional, default is sha256) + * @param int $iterations Iteration count (optional, default is 8192) + * @param int $length The length of the hash (optional, default is 32) + * @return string + */ + public function pbkdf2($password, $salt, $algorithm = 'sha256', $iterations = 8192, $length = 24) + { + if(function_exists('hash_pbkdf2')) + { + return hash_pbkdf2($algorithm, $password, $salt, $iterations, $length, true); + } + else + { + $output = ''; + $block_count = ceil($length / strlen(hash($algorithm, '', true))); // key length divided by the length of one hash + for($i = 1; $i <= $block_count; $i++) + { + $last = $salt . pack('N', $i); // $i encoded as 4 bytes, big endian + $last = $xorsum = hash_hmac($algorithm, $last, $password, true); // first iteration + for($j = 1; $j < $iterations; $j++) // The other $count - 1 iterations + { + $xorsum ^= ($last = hash_hmac($algorithm, $last, $password, true)); + } + $output .= $xorsum; + } + return substr($output, 0, $length); + } + } + + /** + * @brief Generate the bcrypt hash of a string using a salt + * @param string $password The password + * @param string $salt The salt (optional, auto-generated if empty) + * @return string + */ + public function bcrypt($password, $salt = null) + { + if($salt === null) + { + $salt = '$2y$'.sprintf('%02d', $this->getWorkFactor()).'$'.$this->createSecureSalt(22, 'alnum'); + } + return crypt($password, $salt); + } + + /** + * @brief Compare two strings in constant time + * @param string $a The first string + * @param string $b The second string + * @return bool + */ + function strcmpConstantTime($a, $b) + { + $diff = strlen($a) ^ strlen($b); + $maxlen = min(strlen($a), strlen($b)); + for($i = 0; $i < $maxlen; $i++) + { + $diff |= ord($a[$i]) ^ ord($b[$i]); + } + return $diff === 0; + } +} +/* End of file : Password.class.php */ +/* Location: ./classes/security/Password.class.php */ diff --git a/config/config.inc.php b/config/config.inc.php index f37b5eb4a..bed056b11 100644 --- a/config/config.inc.php +++ b/config/config.inc.php @@ -283,6 +283,7 @@ if(!defined('__XE_LOADED_CLASS__')) require(_XE_PATH_ . 'classes/mobile/Mobile.class.php'); require(_XE_PATH_ . 'classes/validator/Validator.class.php'); require(_XE_PATH_ . 'classes/frontendfile/FrontEndFileHandler.class.php'); + require(_XE_PATH_ . 'classes/security/Password.class.php'); require(_XE_PATH_ . 'classes/security/Security.class.php'); require(_XE_PATH_ . 'classes/security/IpFilter.class.php'); if(__DEBUG__) From 7c6b82a522405dd87b3961f11bfba20532f26e91 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 12 Nov 2014 19:28:00 +0900 Subject: [PATCH 2/4] Modify member module to make use of improved password hashing --- modules/member/lang/lang.xml | 30 ++++++ modules/member/member.admin.controller.php | 25 ++++- modules/member/member.admin.view.php | 4 +- modules/member/member.class.php | 14 +++ modules/member/member.controller.php | 15 +-- modules/member/member.model.php | 98 ++++++++++--------- .../member/ruleset/insertDefaultConfig.xml | 3 + modules/member/tpl/default_config.html | 26 +++++ 8 files changed, 156 insertions(+), 59 deletions(-) diff --git a/modules/member/lang/lang.xml b/modules/member/lang/lang.xml index 59f123c97..1368cd301 100644 --- a/modules/member/lang/lang.xml +++ b/modules/member/lang/lang.xml @@ -1685,6 +1685,21 @@ + + + + + + + + + + + + + + + @@ -1707,6 +1722,21 @@ + + + + + + + + + + + + + + + diff --git a/modules/member/member.admin.controller.php b/modules/member/member.admin.controller.php index bb31d60b6..2897e1ee9 100644 --- a/modules/member/member.admin.controller.php +++ b/modules/member/member.admin.controller.php @@ -159,8 +159,31 @@ class memberAdminController extends member 'enable_confirm', 'webmaster_name', 'webmaster_email', - 'password_strength' + 'password_strength', + 'password_hashing_algorithm', + 'password_hashing_work_factor', + 'password_hashing_auto_upgrade' ); + + $oPassword = new Password(); + if(!array_key_exists($args->password_hashing_algorithm, $oPassword->getSupportedAlgorithms())) + { + $args->password_hashing_algorithm = 'md5'; + } + + $args->password_hashing_work_factor = intval($args->password_hashing_work_factor, 10); + if($args->password_hashing_work_factor < 4) + { + $args->password_hashing_work_factor = 4; + } + if($args->password_hashing_work_factor > 16) + { + $args->password_hashing_work_factor = 16; + } + if($args->password_hashing_auto_upgrade != 'Y') + { + $args->password_hashing_auto_upgrade = 'N'; + } if((!$args->webmaster_name || !$args->webmaster_email) && $args->enable_confirm == 'Y') { diff --git a/modules/member/member.admin.view.php b/modules/member/member.admin.view.php index d7fe43ade..f1662c382 100644 --- a/modules/member/member.admin.view.php +++ b/modules/member/member.admin.view.php @@ -129,8 +129,10 @@ class memberAdminView extends member */ public function dispMemberAdminConfig() { + $oPassword = new Password(); + Context::set('password_hashing_algos', $oPassword->getSupportedAlgorithms()); + $this->setTemplateFile('default_config'); - } public function dispMemberAdminSignUpConfig() diff --git a/modules/member/member.class.php b/modules/member/member.class.php index 10dbd9a68..ba85688f9 100644 --- a/modules/member/member.class.php +++ b/modules/member/member.class.php @@ -71,6 +71,20 @@ class member extends ModuleObject { if($config->group_image_mark!='Y') $config->group_image_mark = 'N'; if(!$config->password_strength) $config->password_strength = 'normal'; + if(!$config->password_hashing_algorithm) + { + $oPassword = new Password(); + $config->password_hashing_algorithm = $oPassword->getBestAlgorithm(); + } + if(!$config->password_hashing_work_factor) + { + $config->password_hashing_work_factor = 8; + } + if(!$config->password_hashing_auto_upgrade) + { + $config->password_hashing_auto_upgrade = 'Y'; + } + global $lang; $oMemberModel = getModel('member'); // Create a member controller object diff --git a/modules/member/member.controller.php b/modules/member/member.controller.php index 9e3b3fa25..4a86a6ff0 100644 --- a/modules/member/member.controller.php +++ b/modules/member/member.controller.php @@ -1101,7 +1101,7 @@ class memberController extends member } else { - $args->password = md5($output->data->new_password); + $args->password = getModel('member')->hashPassword($args->password); unset($args->denied); } // Back up the value of $Output->data->is_register @@ -1956,7 +1956,7 @@ class memberController extends member $message = Context::getLang('about_password_strength'); return new Object(-1, $message[$config->password_strength]); } - $args->password = md5($args->password); + $args->password = $oMemberModel->hashPassword($args->password); } elseif(!$args->password) unset($args->password); if($oMemberModel->isDeniedID($args->user_id)) return new Object(-1,'denied_user_id'); @@ -2149,7 +2149,7 @@ class memberController extends member return new Object(-1, $message[$config->password_strength]); } - $args->password = md5($args->password); + $args->password = $oMemberModel->hashPassword($args->password); } else $args->password = $orgMemberInfo->password; if(!$args->user_name) $args->user_name = $orgMemberInfo->user_name; @@ -2239,14 +2239,7 @@ class memberController extends member return new Object(-1, $message[$config->password_strength]); } - if($this->useSha1) - { - $args->password = md5(sha1(md5($args->password))); - } - else - { - $args->password = md5($args->password); - } + $args->password = $oMemberModel->hashPassword($args->password); } else if($args->hashed_password) { diff --git a/modules/member/member.model.php b/modules/member/member.model.php index f0f7f70d8..6696f906a 100644 --- a/modules/member/member.model.php +++ b/modules/member/member.model.php @@ -996,65 +996,71 @@ class memberModel extends member /** * @brief Compare plain text password to the password saved in DB + * @param string $hashed_password The hash that was saved in DB + * @param string $password_text The password to check + * @param int $member_srl Set this to member_srl when comparing a member's password (optional) + * @return bool */ function isValidPassword($hashed_password, $password_text, $member_srl=null) { // False if no password in entered - if(!$password_text) return false; - - $isSha1 = ($this->useSha1 && function_exists('sha1')); - - // Return true if the user input is equal to md5 hash value - if($hashed_password == md5($password_text)) + if(!$password_text) { - if($isSha1 && $member_srl > 0) + return false; + } + + // Check the password + $oPassword = new Password(); + $current_algorithm = $oPassword->checkAlgorithm($hashed_password); + $match = $oPassword->checkPassword($password_text, $hashed_password, $current_algorithm); + if(!$match) + { + return false; + } + + // Update the encryption method if necessary + $config = $this->getMemberConfig(); + if($member_srl > 0 && $config->password_hashing_auto_upgrade != 'N') + { + $need_upgrade = false; + + if(!$need_upgrade) + { + $required_algorithm = $oPassword->getCurrentlySelectedAlgorithm(); + if($required_algorithm !== $current_algorithm) $need_upgrade = true; + } + + if(!$need_upgrade) + { + $required_work_factor = $oPassword->getWorkFactor(); + $current_work_factor = $oPassword->checkWorkFactor($hashed_password); + if($current_work_factor !== false && $required_work_factor > $current_work_factor) $need_upgrade = true; + } + + if($need_upgrade === true) { $args = new stdClass(); $args->member_srl = $member_srl; - $args->hashed_password = md5(sha1(md5($password_text))); + $args->hashed_password = $this->hashPassword($password_text, $required_algorithm); $oMemberController = getController('member'); $oMemberController->updateMemberPassword($args); } - return true; } - - // Return true if the user input is equal to the value of mysql_pre4_hash_password - if(mysql_pre4_hash_password($password_text) == $hashed_password) - { - if($isSha1 && $member_srl > 0) - { - $args = new stdClass(); - $args->member_srl = $member_srl; - $args->hashed_password = md5(sha1(md5($password_text))); - $oMemberController = getController('member'); - $oMemberController->updateMemberPassword($args); - } - return true; - } - - // Verify the password by using old_password if the current db is MySQL. If correct, return true. - if(substr(Context::getDBType(),0,5)=='mysql') - { - $oDB = &DB::getInstance(); - if($oDB->isValidOldPassword($password_text, $hashed_password)) - { - if($isSha1 && $member_srl > 0) - { - $args = new stdClass(); - $args->member_srl = $member_srl; - $args->hashed_password = md5(sha1(md5($password_text))); - $oMemberController = getController('member'); - $oMemberController->updateMemberPassword($args); - } - return true; - } - } - - if($isSha1 && $hashed_password == md5(sha1(md5($password_text)))) return true; - - return false; + + return true; + } + + /** + * @brief Create a hash of plain text password + * @param string $password_text The password to hash + * @param string $algorithm The algorithm to use (optional, only set this when you want to use a non-default algorithm) + * @return string + */ + function hashPassword($password_text, $algorithm = null) + { + $oPassword = new Password(); + return $oPassword->createHash($password_text, $algorithm); } - function checkPasswordStrength($password, $strength) { diff --git a/modules/member/ruleset/insertDefaultConfig.xml b/modules/member/ruleset/insertDefaultConfig.xml index 0a9554629..482c00d29 100644 --- a/modules/member/ruleset/insertDefaultConfig.xml +++ b/modules/member/ruleset/insertDefaultConfig.xml @@ -7,5 +7,8 @@ + + + diff --git a/modules/member/tpl/default_config.html b/modules/member/tpl/default_config.html index ccb8d83f9..d04032437 100644 --- a/modules/member/tpl/default_config.html +++ b/modules/member/tpl/default_config.html @@ -29,6 +29,32 @@

{$lang->about_password_strength_config}

+
+ +
+ +

{$lang->about_password_hashing_algorithm}

+
+
+
+ +
+ +

{$lang->about_password_hashing_work_factor}

+
+
+
+ +
+ + +

{$lang->about_password_hashing_auto_upgrade}

+
+
From 8fd32d09beff18c33f5982f8c0c02c09cb006612 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Thu, 13 Nov 2014 12:29:20 +0900 Subject: [PATCH 3/4] Ensure full compatibility with previous versions of XE and migration tools --- classes/security/Password.class.php | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/classes/security/Password.class.php b/classes/security/Password.class.php index 0cbeb59b3..ce4572063 100644 --- a/classes/security/Password.class.php +++ b/classes/security/Password.class.php @@ -132,22 +132,25 @@ class Password */ public function checkPassword($password, $hash, $algorithm = null) { - $password = trim($password); - if($algorithm === null) { $algorithm = $this->checkAlgorithm($hash); } - if(!array_key_exists($algorithm, $this->getSupportedAlgorithms())) - { - return false; - } + + $password = trim($password); switch($algorithm) { case 'md5': return md5($password) === $hash || md5(sha1(md5($password))) === $hash; + case 'mysql_old_password': + return (class_exists('Context') && substr(Context::getDBType(), 0, 5) === 'mysql') ? + DB::getInstance()->isValidOldPassword($password, $hash) : false; + + case 'mysql_password': + return $hash[0] === '*' && substr($hash, 1) === strtoupper(sha1(sha1($password, true))); + case 'pbkdf2': $hash = explode(':', $hash); $hash[3] = base64_decode($hash[3]); @@ -182,6 +185,14 @@ class Password { return 'md5'; } + elseif(strlen($hash) === 16 && ctype_xdigit($hash)) + { + return 'mysql_old_password'; + } + elseif(strlen($hash) === 41 && $hash[0] === '*') + { + return 'mysql_password'; + } else { return false; From 3e198f94f497f27096b9ea127c566b4169e6587b Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Fri, 14 Nov 2014 12:36:00 +0900 Subject: [PATCH 4/4] Always prefer PBKDF2 to bcrypt, for better PHP 5.2 compatibility --- classes/security/Password.class.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/classes/security/Password.class.php b/classes/security/Password.class.php index ce4572063..75ff1e039 100644 --- a/classes/security/Password.class.php +++ b/classes/security/Password.class.php @@ -19,14 +19,14 @@ class Password public function getSupportedAlgorithms() { $retval = array(); - if(version_compare(PHP_VERSION, '5.3.7', '>=') && defined('CRYPT_BLOWFISH')) - { - $retval['bcrypt'] = 'bcrypt'; - } if(function_exists('hash_hmac') && in_array('sha256', hash_algos())) { $retval['pbkdf2'] = 'pbkdf2'; } + if(version_compare(PHP_VERSION, '5.3.7', '>=') && defined('CRYPT_BLOWFISH')) + { + $retval['bcrypt'] = 'bcrypt'; + } $retval['md5'] = 'md5'; return $retval; }