From ed59911f9ee542da87ae7cddcb2d50da0e785079 Mon Sep 17 00:00:00 2001 From: Dries Buytaert Date: Mon, 31 Mar 2008 20:50:05 +0000 Subject: - Patch #29706 by pwolanin, solardiz, et al: more secure password hashing. This is a big and important patch for Drupal's security. We are switching to much stronger password hashes that are also compatible with the Portable PHP password hashing framework. The new password hashes defeat a number of attacks, including: - The ability to try candidate passwords against multiple hashes at once. - The ability to use pre-hashed lists of candidate passwords. - The ability to determine whether two users have the same (or different) password without actually having to guess one of the passwords. Also implemented a pluggable password hashing API (similar to how an alternate cache mechanism can be used) to allow developers to readily substitute an alternative hashing and authentication scheme. Thanks all! --- modules/user/user.install | 56 +++++++++++++++++++++++++++++++++++++++++++++-- modules/user/user.module | 34 ++++++++++++++++++++++------ 2 files changed, 81 insertions(+), 9 deletions(-) (limited to 'modules/user') diff --git a/modules/user/user.install b/modules/user/user.install index 3f08bc7c2..ac9527e16 100644 --- a/modules/user/user.install +++ b/modules/user/user.install @@ -150,10 +150,10 @@ function user_schema() { ), 'pass' => array( 'type' => 'varchar', - 'length' => 32, + 'length' => 128, 'not null' => TRUE, 'default' => '', - 'description' => t("User's password (md5 hash)."), + 'description' => t("User's password (hashed)."), ), 'mail' => array( 'type' => 'varchar', @@ -295,3 +295,55 @@ function user_schema() { return $schema; } +/** + * @defgroup user-updates-6.x-to-7.x User updates from 6.x to 7.x + * @{ + */ + +/** + * Increase the length of the password field to accommodate better hashes. + * + * Also re-hashes all current passwords to improve security. This may be a + * lengthy process, and is performed batch-wise. + */ +function user_update_7000(&$sandbox) { + $ret = array('#finished' => 0); + // Lower than DRUPAL_HASH_COUNT to make the update run at a reasonable speed. + $hash_count_log2 = 11; + // Multi-part update. + if (!isset($sandbox['user_from'])) { + db_change_field($ret, 'users', 'pass', 'pass', array('type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => '')); + $sandbox['user_from'] = 0; + $sandbox['user_count'] = db_result(db_query("SELECT COUNT(uid) FROM {users}")); + } + else { + require_once variable_get('password_inc', './includes/password.inc'); + // Hash again all current hashed passwords. + $has_rows = FALSE; + // Update this many per page load. + $count = 1000; + $result = db_query_range("SELECT uid, pass FROM {users} WHERE uid > 0 ORDER BY uid", $sandbox['user_from'], $count); + while ($account = db_fetch_array($result)) { + $has_rows = TRUE; + $new_hash = user_hash_password($account['pass'], $hash_count_log2); + if ($new_hash) { + // Indicate an updated password. + $new_hash = 'U'. $new_hash; + db_query("UPDATE {users} SET pass = '%s' WHERE uid = %d", $new_hash, $account['uid']); + } + } + $ret['#finished'] = $sandbox['user_from']/$sandbox['user_count']; + $sandbox['user_from'] += $count; + if (!$has_rows) { + $ret['#finished'] = 1; + $ret[] = array('success' => TRUE, 'query' => "UPDATE {users} SET pass = 'U'. user_hash_password(pass) WHERE uid > 0"); + } + } + return $ret; +} + +/** + * @} End of "defgroup user-updates-6.x-to-7.x" + * The next series of updates should start at 8000. + */ + diff --git a/modules/user/user.module b/modules/user/user.module index 74372eaaf..9f4910146 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -157,7 +157,7 @@ function user_load($array = array()) { } else if ($key == 'pass') { $query[] = "pass = '%s'"; - $params[] = md5($value); + $params[] = $value; } else { $query[]= "LOWER($key) = LOWER('%s')"; @@ -214,7 +214,13 @@ function user_save($account, $array = array(), $category = 'account') { $user_fields = $table['fields']; if (!empty($array['pass'])) { - $array['pass'] = md5($array['pass']); + // Allow alternate password hashing schemes. + require_once variable_get('password_inc', './includes/password.inc'); + $array['pass'] = user_hash_password(trim($array['pass'])); + // Abort if the hashing failed and returned FALSE. + if (!$array['pass']) { + return FALSE; + } } else { // Avoid overwriting an existing password with a blank password. @@ -1283,12 +1289,26 @@ function user_login_final_validate($form, &$form_state) { function user_authenticate($form_values = array()) { global $user; + $password = trim($form_values['pass']); // Name and pass keys are required. - if (!empty($form_values['name']) && !empty($form_values['pass']) && - $account = user_load(array('name' => $form_values['name'], 'pass' => trim($form_values['pass']), 'status' => 1))) { - $user = $account; - user_authenticate_finalize($form_values); - return $user; + if (!empty($form_values['name']) && !empty($password)) { + $account = db_fetch_object(db_query("SELECT * FROM {users} WHERE name = '%s' AND status = 1", $form_values['name'])); + if ($account) { + // Allow alternate password hashing schemes. + require_once variable_get('password_inc', './includes/password.inc'); + if (user_check_password($password, $account)) { + if (user_needs_new_hash($account)) { + $new_hash = user_hash_password($password); + if ($new_hash) { + db_query("UPDATE {users} SET pass = '%s' WHERE uid = %d", $new_hash, $account->uid); + } + } + $account = user_load(array('uid' => $account->uid, 'status' => 1)); + $user = $account; + user_authenticate_finalize($form_values); + return $user; + } + } } } -- cgit v1.2.3