From 7c6b82a522405dd87b3961f11bfba20532f26e91 Mon Sep 17 00:00:00 2001 From: Kijin Sung Date: Wed, 12 Nov 2014 19:28:00 +0900 Subject: [PATCH] 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}

+
+