diff --git CHANGELOG.txt CHANGELOG.txt
index 0486ce0..055d5b6 100644
--- CHANGELOG.txt
+++ CHANGELOG.txt
@@ -13,10 +13,11 @@ Drupal 7.0, xxxx-xx-xx (development version)
This offers increased scalability and data integrity.
- Security:
* Protected cron.php -- cron will only run if the proper key is provided.
- * Implemented much stronger password hashes that are also compatible with the
- Portable PHP password hashing framework.
- * Implemented a pluggable password hashing API supporting alternative
- hashing and authentication schemes.
+ * Implemented a pluggable password system and much stronger password hashes
+ that are compatible with the Portable PHP password hashing framework.
+ * Rate limited login attempts to prevent brute-force password guessing, and
+ improved the flood control API to allow variable time windows and
+ identifiers for limiting user access to resources.
- Usability:
* Improved installer requirements check.
* Improved support for integration of WYSIWYG editors.
diff --git includes/common.inc includes/common.inc
index 7758686..260bce4 100644
--- includes/common.inc
+++ includes/common.inc
@@ -1276,39 +1276,70 @@ function valid_url($url, $absolute = FALSE) {
*/
/**
- * Register an event for the current visitor (hostname/IP) to the flood control mechanism.
+ * Register an event for the current visitor to the flood control mechanism.
*
* @param $name
* The name of an event.
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
*/
-function flood_register_event($name) {
+function flood_register_event($name, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
db_insert('flood')
->fields(array(
'event' => $name,
- 'hostname' => ip_address(),
+ 'identifier' => $identifier,
'timestamp' => REQUEST_TIME,
))
->execute();
}
/**
- * Check if the current visitor (hostname/IP) is allowed to proceed with the specified event.
+ * Make the flood control mechanism forget about an event for the current visitor.
+ *
+ * @param $name
+ * The name of an event.
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
+ */
+function flood_clear_event($name, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
+ db_delete('flood')
+ ->condition('event', $name)
+ ->condition('identifier', $identifier)
+ ->execute();
+}
+
+/**
+ * Check if the current visitor is allowed to proceed with the specified event.
*
* The user is allowed to proceed if he did not trigger the specified event more
- * than $threshold times per hour.
+ * than $threshold times in the specified time window.
*
* @param $name
* The name of the event.
* @param $threshold
- * The maximum number of the specified event per hour (per visitor).
+ * The maximum number of the specified event allowed per time window.
+ * @param $window
+ * Optional number of seconds over which to look for events. Defaults to
+ * 3600 (1 hour).
+ * @param $identifier
+ * Optional identifier (defaults to the current user's IP address).
* @return
* True if the user did not exceed the hourly threshold. False otherwise.
*/
-function flood_is_allowed($name, $threshold) {
- $number = db_query("SELECT COUNT(*) FROM {flood} WHERE event = :event AND hostname = :hostname AND timestamp > :timestamp", array(
+function flood_is_allowed($name, $threshold, $window = 3600, $identifier = NULL) {
+ if (!isset($identifier)) {
+ $identifier = ip_address();
+ }
+ $number = db_query("SELECT COUNT(*) FROM {flood} WHERE event = :event AND identifier = :identifier AND timestamp > :timestamp", array(
':event' => $name,
- ':hostname' => ip_address(),
- ':timestamp' => REQUEST_TIME - 3600))
+ ':identifier' => $identifier,
+ ':timestamp' => REQUEST_TIME - $window))
->fetchField();
return ($number < $threshold);
}
diff --git modules/system/system.install modules/system/system.install
index c684b65..3882aca 100644
--- modules/system/system.install
+++ modules/system/system.install
@@ -759,8 +759,8 @@ function system_schema() {
'not null' => TRUE,
'default' => '',
),
- 'hostname' => array(
- 'description' => 'Hostname of the visitor.',
+ 'identifier' => array(
+ 'description' => 'Identifier of the visitor, such as an IP address or hostname.',
'type' => 'varchar',
'length' => 128,
'not null' => TRUE,
@@ -775,7 +775,7 @@ function system_schema() {
),
'primary key' => array('fid'),
'indexes' => array(
- 'allow' => array('event', 'hostname', 'timestamp'),
+ 'allow' => array('event', 'identifier', 'timestamp'),
),
);
@@ -2220,6 +2220,17 @@ function system_update_7030() {
}
/**
+* Alter field hostname to identifier in the {flood} table.
+ */
+function system_update_7031() {
+ $ret = array();
+ db_drop_index($ret, 'flood', 'allow');
+ db_change_field($ret, 'flood', 'hostname', 'identifier', array('type' => 'varchar', 'length' => 128, 'not null' => TRUE, 'default' => ''));
+ db_add_index($ret, 'flood', 'allow', array('event', 'identifier', 'timestamp'));
+ return $ret;
+}
+
+/**
* @} End of "defgroup updates-6.x-to-7.x"
* The next series of updates should start at 8000.
*/
diff --git modules/user/user.module modules/user/user.module
index 668b508..bd9da42 100644
--- modules/user/user.module
+++ 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_uid'] = 0;
+ 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_uid'] = $account->uid;
+ 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_uid'])) {
+ if ($form_state['flood_control_uid'] > 0) {
+ 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;
}
/**
diff --git modules/user/user.test modules/user/user.test
index c05c38e..843a561 100644
--- modules/user/user.test
+++ modules/user/user.test
@@ -159,6 +159,120 @@ class UserValidationTestCase extends DrupalWebTestCase {
}
}
+class UserLoginTestCase extends DrupalWebTestCase {
+ public static function getInfo() {
+ return array(
+ 'name' => 'User login',
+ 'description' => 'Ensure that login works as expected.',
+ 'group' => 'User',
+ );
+ }
+
+ /**
+ * Test the global login flood control.
+ */
+ function testGlobalLoginFloodControl() {
+ // Set the global login limit.
+ variable_set('user_failed_login_ip_limit', 10);
+ // Set a high per-user limit out so that it is not relevant in the test.
+ variable_set('user_failed_login_user_limit', 4000);
+
+ $user1 = $this->drupalCreateUser(array());
+ $incorrect_user1 = clone $user1;
+ $incorrect_user1->pass_raw .= 'incorrect';
+
+ // Try 2 failed logins.
+ for ($i = 0; $i < 2; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // A successful login will not reset the IP-based flood control count.
+ $this->drupalLogin($user1);
+ $this->drupalLogout();
+
+ // Try 8 more failed logins, they should not trigger the flood control
+ // mechanism.
+ for ($i = 0; $i < 8; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // The next login trial should result in an IP-based flood error message.
+ $this->assertFailedLogin($incorrect_user1, 0);
+
+ // A login with the correct password should also result in a flood error
+ // message.
+ $this->assertFailedLogin($user1, 0);
+ }
+
+ /**
+ * Test the per-user login flood control.
+ */
+ function testPerUserLoginFloodControl() {
+ // Set a high global limit out so that it is not relevant in the test.
+ variable_set('user_failed_login_ip_limit', 4000);
+ // Set the per-user login limit.
+ variable_set('user_failed_login_user_limit', 3);
+
+ $user1 = $this->drupalCreateUser(array());
+ $incorrect_user1 = clone $user1;
+ $incorrect_user1->pass_raw .= 'incorrect';
+
+ $user2 = $this->drupalCreateUser(array());
+
+ // Try 2 failed logins.
+ for ($i = 0; $i < 2; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // A successful login will reset the per-user flood control count.
+ $this->drupalLogin($user1);
+ $this->drupalLogout();
+
+ // Try 3 failed logins for user 1, they will not trigger flood control.
+ for ($i = 0; $i < 3; $i++) {
+ $this->assertFailedLogin($incorrect_user1);
+ }
+
+ // Try one successful attempt for user 2, it should not trigger any
+ // flood control.
+ $this->drupalLogin($user2);
+ $this->drupalLogout();
+
+ // Try one more attempt for user 1, it should be rejected, even if the
+ // correct password has been used.
+ $this->assertFailedLogin($user1, $user1->uid);
+ }
+
+ /**
+ * Make an unsuccessful login attempt.
+ *
+ * @param $account
+ * A user object with name and pass_raw attributes for the login attempt.
+ * @param $flood_uid
+ * Whether or not to expect that the flood control mechanism will be
+ * triggered.
+ */
+ function assertFailedLogin($account, $flood_uid = NULL) {
+ $edit = array(
+ 'name' => $account->name,
+ 'pass' => $account->pass_raw,
+ );
+ $this->drupalPost('user', $edit, t('Log in'));
+ if (isset($flood_uid)) {
+ if ($flood_uid > 0) {
+ $this->assertRaw(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 {
+ // No uid, so the limit is IP-based.
+ $this->assertRaw(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 {
+ $this->assertText(t('Sorry, unrecognized username or password. Have you forgotten your password?'));
+ }
+ }
+}
+
class UserCancelTestCase extends DrupalWebTestCase {
public static function getInfo() {
return array(