diff options
author | Dries Buytaert <dries@buytaert.net> | 2008-03-31 20:50:05 +0000 |
---|---|---|
committer | Dries Buytaert <dries@buytaert.net> | 2008-03-31 20:50:05 +0000 |
commit | ed59911f9ee542da87ae7cddcb2d50da0e785079 (patch) | |
tree | 8b7f873dd371ae19d1f678e26ad548c47ff1b0ad /modules | |
parent | 763298455f88e26f286749b5f7ff6c9471742012 (diff) | |
download | brdo-ed59911f9ee542da87ae7cddcb2d50da0e785079.tar.gz brdo-ed59911f9ee542da87ae7cddcb2d50da0e785079.tar.bz2 |
- 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!
Diffstat (limited to 'modules')
-rw-r--r-- | modules/user/user.install | 56 | ||||
-rw-r--r-- | modules/user/user.module | 34 |
2 files changed, 81 insertions, 9 deletions
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; + } + } } } |