From 1da6ef52c44fd38785391d3a94af8e969344bc12 Mon Sep 17 00:00:00 2001 From: Angie Byron Date: Sat, 8 Aug 2009 20:52:33 +0000 Subject: #485974 by pwolanin, Damien Tournoud, mr.baileys: Improved security by limiting the number of allowed login attempts. --- modules/user/user.module | 98 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 17 deletions(-) (limited to 'modules/user/user.module') diff --git a/modules/user/user.module b/modules/user/user.module index 584e7ce74..ae21574ce 100644 --- a/modules/user/user.module +++ b/modules/user/user.module @@ -1618,39 +1618,102 @@ function user_login_name_validate($form, &$form_state) { * is set to the matching user ID. */ function user_login_authenticate_validate($form, &$form_state) { - user_authenticate($form_state); + $password = trim($form_state['values']['pass']); + if (!empty($form_state['values']['name']) && !empty($password)) { + // Do not allow any login from the current user's IP if the limit has been + // reached. Default is 50 failed attempts allowed in one hour. This is + // independent of the per-user limit to catch attempts from one IP to log + // in to many different user accounts. We have a reasonably high limit + // since there may be only one apparent IP for all users at an institution. + if (!flood_is_allowed('failed_login_attempt_ip', variable_get('user_failed_login_ip_limit', 50), variable_get('user_failed_login_ip_window', 3600))) { + $form_state['flood_control_triggered'] = 'ip'; + return; + } + $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $form_state['values']['name']))->fetchObject(); + if ($account) { + if (variable_get('user_failed_login_identifier_uid_only', FALSE)) { + // Register flood events based on the uid only, so they apply for any + // IP address. This is the most secure option. + $identifier = $account->uid; + } + else { + // The default identifier is a combination of uid and IP address. This + // is less secure but more resistant to denial-of-service attacks that + // could lock out all users with public user names. + $identifier = $account->uid . '-' . ip_address(); + } + $form_state['flood_control_user_identifier'] = $identifier; + + // Don't allow login if the limit for this user has been reached. + // Default is to allow 5 failed attempts every 6 hours. + if (!flood_is_allowed('failed_login_attempt_user', variable_get('user_failed_login_user_limit', 5), variable_get('user_failed_login_user_window', 21600), $identifier)) { + $form_state['flood_control_triggered'] = 'user'; + return; + } + } + // We are not limited by flood control, so try to authenticate. + // Set $form_state['uid'] as a flag for user_login_final_validate(). + $form_state['uid'] = user_authenticate($form_state['values']['name'], $password); + } } /** - * A validate handler on the login form. Should be the last validator. Sets an - * error if user has not been authenticated yet. + * The final validation handler on the login form. + * + * Sets a form error if user has not been authenticated, or if too many + * logins have been attempted. This validation function should always + * be the last one. */ function user_login_final_validate($form, &$form_state) { if (empty($form_state['uid'])) { - form_set_error('name', t('Sorry, unrecognized username or password. Have you forgotten your password?', array('@password' => url('user/password')))); - watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name'])); + // Always register an IP-based failed login event. + flood_register_event('failed_login_attempt_ip'); + // Register a per-user failed login event. + if (isset($form_state['flood_control_user_identifier'])) { + flood_register_event('failed_login_attempt_user', $form_state['flood_control_user_identifier']); + } + + if (isset($form_state['flood_control_triggered'])) { + if ($form_state['flood_control_triggered'] == 'user') { + form_set_error('name', format_plural(variable_get('user_failed_login_user_limit', 5), 'Sorry, there has been more than one failed login attempt for this account. It is temporarily blocked. Please try again later, or request a new password.', 'Sorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password')))); + } + else { + // We did not find a uid, so the limit is IP-based. + form_set_error('name', t('Sorry, too many failed login attempts from your IP address. This IP address is temporarily blocked. Please try again later, or request a new password.', array('@url' => url('user/password')))); + } + } + else { + form_set_error('name', t('Sorry, unrecognized username or password. Have you forgotten your password?', array('@password' => url('user/password')))); + watchdog('user', 'Login attempt failed for %user.', array('%user' => $form_state['values']['name'])); + } + } + elseif (isset($form_state['flood_control_user_identifier'])) { + // Clear past failures for this user so as not to block a user who might + // log in and out more than once in an hour. + flood_clear_event('failed_login_attempt_user', $form_state['flood_control_user_identifier']); } } /** - * Try to log in the user locally. $form_state['uid'] is set to - * a user ID if successful. + * Try to validate the user's login credentials locally. * - * @param $form_state - * Form submission state with at least 'name' and 'pass' keys. + * @param $name + * User name to authenticate. + * @param $password + * A plain-text password, such as trimmed text from form values. + * @return + * The user's uid on success, or FALSE on failure to authenticate. */ -function user_authenticate(&$form_state) { - $password = trim($form_state['values']['pass']); - // Name and pass keys are required. - if (!empty($form_state['values']['name']) && !empty($password)) { - $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $form_state['values']['name']))->fetchObject(); +function user_authenticate($name, $password) { + $uid = FALSE; + if (!empty($name) && !empty($password)) { + $account = db_query("SELECT * FROM {users} WHERE name = :name AND status = 1", array(':name' => $name))->fetchObject(); if ($account) { // Allow alternate password hashing schemes. require_once DRUPAL_ROOT . '/' . variable_get('password_inc', 'includes/password.inc'); if (user_check_password($password, $account)) { - - // Successful authentication. Set a flag for user_login_final_validate(). - $form_state['uid'] = $account->uid; + // Successful authentication. + $uid = $account->uid; // Update user to new password scheme if needed. if (user_needs_new_hash($account)) { @@ -1665,6 +1728,7 @@ function user_authenticate(&$form_state) { } } } + return $uid; } /** -- cgit v1.2.3