Commit 1da6ef52 authored by webchick's avatar webchick

#485974 by pwolanin, Damien Tournoud, mr.baileys: Improved security by...

#485974 by pwolanin, Damien Tournoud, mr.baileys: Improved security by limiting the number of allowed login attempts.
parent 78e3681c
......@@ -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.
......
......@@ -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);
}
......
......@@ -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'),
),
);
......@@ -2233,6 +2233,17 @@ function system_update_7031() {
return $ret;
}
/**
* Alter field hostname to identifier in the {flood} table.
*/
function system_update_7032() {
$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.
......
......@@ -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. <a href="@password">Have you forgotten your password?</a>', 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 <a href="@url">request a new password</a>.', 'Sorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Please try again later, or <a href="@url">request a new password</a>.', 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 <a href="@url">request a new password</a>.', array('@url' => url('user/password'))));
}
}
else {
form_set_error('name', t('Sorry, unrecognized username or password. <a href="@password">Have you forgotten your password?</a>', 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;
}
/**
......
......@@ -159,6 +159,123 @@ class UserValidationTestCase extends DrupalWebTestCase {
}
}
/**
* Functional tests for user logins, including rate limiting of login attempts.
*/
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, 'ip');
// A login with the correct password should also result in a flood error
// message.
$this->assertFailedLogin($user1, 'ip');
}
/**
* 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, 'user');
}
/**
* Make an unsuccessful login attempt.
*
* @param $account
* A user object with name and pass_raw attributes for the login attempt.
* @param $flood_trigger
* Whether or not to expect that the flood control mechanism will be
* triggered.
*/
function assertFailedLogin($account, $flood_trigger = NULL) {
$edit = array(
'name' => $account->name,
'pass' => $account->pass_raw,
);
$this->drupalPost('user', $edit, t('Log in'));
if (isset($flood_trigger)) {
if ($flood_trigger == 'user') {
$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 <a href="@url">request a new password</a>.', 'Sorry, there have been more than @count failed login attempts for this account. It is temporarily blocked. Please try again later, or <a href="@url">request a new password</a>.', 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 <a href="@url">request a new password</a>.', 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(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment