aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormkanat%bugzilla.org <>2009-12-13 20:46:24 +0000
committermkanat%bugzilla.org <>2009-12-13 20:46:24 +0000
commit72cb2bc73e71f54c2223bb78af29fee888590b53 (patch)
tree45aacc0944bd4b7d4b7391b0bff7bc67b15c722e
parentcb4a8bf4954c38d06358c4a7509f3fac6fb1e705 (diff)
downloadbugs-72cb2bc73e71f54c2223bb78af29fee888590b53.tar
bugs-72cb2bc73e71f54c2223bb78af29fee888590b53.tar.gz
bugs-72cb2bc73e71f54c2223bb78af29fee888590b53.tar.bz2
bugs-72cb2bc73e71f54c2223bb78af29fee888590b53.tar.xz
bugs-72cb2bc73e71f54c2223bb78af29fee888590b53.zip
Bug 355283: Lock out a user account on a particular IP for 30 minutes if they fail to log in 5 times from that IP.
Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit
-rw-r--r--Bugzilla/Auth.pm56
-rw-r--r--Bugzilla/Auth/Verify/DB.pm46
-rw-r--r--Bugzilla/Config/Common.pm11
-rw-r--r--Bugzilla/Config/Core.pm3
-rw-r--r--Bugzilla/Constants.pm10
-rw-r--r--Bugzilla/DB/Schema.pm20
-rw-r--r--Bugzilla/User.pm85
-rw-r--r--template/en/default/email/lockout.txt.tmpl39
-rw-r--r--template/en/default/global/user-error.html.tmpl15
9 files changed, 266 insertions, 19 deletions
diff --git a/Bugzilla/Auth.pm b/Bugzilla/Auth.pm
index 8e18f8699..40150f0ed 100644
--- a/Bugzilla/Auth.pm
+++ b/Bugzilla/Auth.pm
@@ -32,6 +32,9 @@ use fields qw(
use Bugzilla::Constants;
use Bugzilla::Error;
+use Bugzilla::Mailer;
+use Bugzilla::Util qw(datetime_from);
+use Bugzilla::User::Setting ();
use Bugzilla::Auth::Login::Stack;
use Bugzilla::Auth::Verify::Stack;
use Bugzilla::Auth::Persist::Cookie;
@@ -162,7 +165,10 @@ sub _handle_login_result {
# the password was just wrong. (This makes it harder for a cracker
# to find account names by brute force)
elsif ($fail_code == AUTH_LOGINFAILED or $fail_code == AUTH_NO_SUCH_USER) {
- ThrowUserError("invalid_username_or_password");
+ my $remaining_attempts = MAX_LOGIN_ATTEMPTS
+ - ($result->{failure_count} || 0);
+ ThrowUserError("invalid_username_or_password",
+ { remaining => $remaining_attempts });
}
# The account may be disabled
elsif ($fail_code == AUTH_DISABLED) {
@@ -173,6 +179,40 @@ sub _handle_login_result {
ThrowUserError("account_disabled",
{'disabled_reason' => $result->{user}->disabledtext});
}
+ elsif ($fail_code == AUTH_LOCKOUT) {
+ my $attempts = $user->account_ip_login_failures;
+
+ # We want to know when the account will be unlocked. This is
+ # determined by the 5th-from-last login failure (or more/less than
+ # 5th, if MAX_LOGIN_ATTEMPTS is not 5).
+ my $determiner = $attempts->[scalar(@$attempts) - MAX_LOGIN_ATTEMPTS];
+ my $unlock_at = datetime_from($determiner->{login_time},
+ Bugzilla->local_timezone);
+ $unlock_at->add(minutes => LOGIN_LOCKOUT_INTERVAL);
+
+ # If we were *just* locked out, notify the maintainer about the
+ # lockout.
+ if ($result->{just_locked_out}) {
+ # We're sending to the maintainer, who may be not a Bugzilla
+ # account, but just an email address. So we use the
+ # installation's default language for sending the email.
+ my $default_settings = Bugzilla::User::Setting::get_defaults();
+ my $template = Bugzilla->template_inner($default_settings->{lang});
+ my $vars = {
+ locked_user => $user,
+ attempts => $attempts,
+ unlock_at => $unlock_at,
+ };
+ my $message;
+ $template->process('email/lockout.txt.tmpl', $vars, \$message)
+ || ThrowTemplateError($template->error);
+ MessageToMTA($message);
+ }
+
+ $unlock_at->set_time_zone($user->timezone);
+ ThrowUserError('account_locked',
+ { ip_addr => $determiner->{ip_addr}, unlock_at => $unlock_at });
+ }
# If we get here, then we've run out of options, which shouldn't happen.
else {
ThrowCodeError("authres_unhandled", { value => $fail_code });
@@ -234,6 +274,11 @@ various fields to be used in the error message.
An incorrect username or password was given.
+The hashref may also contain a C<failure_count> element, which specifies
+how many times the account has failed to log in within the lockout
+period (see L</AUTH_LOCKOUT>). This is used to warn the user when
+he is getting close to being locked out.
+
=head2 C<AUTH_NO_SUCH_USER>
This is an optional more-specific version of C<AUTH_LOGINFAILED>.
@@ -251,6 +296,15 @@ should never be communicated to the user, for security reasons.
The user successfully logged in, but their account has been disabled.
Usually this is throw only by C<Bugzilla::Auth::login>.
+=head2 C<AUTH_LOCKOUT>
+
+The user's account is locked out after having failed to log in too many
+times within a certain period of time (as specified by
+L<Bugzilla::Constants/LOGIN_LOCKOUT_INTERVAL>).
+
+The hashref will also contain a C<user> element, representing the
+L<Bugzilla:User> whose account is locked out.
+
=head1 LOGIN TYPES
The C<login> function (below) can do different types of login, depending
diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm
index 695671a31..d8794472e 100644
--- a/Bugzilla/Auth/Verify/DB.pm
+++ b/Bugzilla/Auth/Verify/DB.pm
@@ -41,37 +41,51 @@ sub check_credentials {
my $dbh = Bugzilla->dbh;
my $username = $login_data->{username};
- my $user_id = login_to_id($username);
+ my $user = new Bugzilla::User({ name => $username });
- return { failure => AUTH_NO_SUCH_USER } unless $user_id;
+ return { failure => AUTH_NO_SUCH_USER } unless $user;
- $login_data->{bz_username} = $username;
- my $password = $login_data->{password};
+ $login_data->{user} = $user;
+ $login_data->{bz_username} = $user->login;
+
+ if ($user->account_is_locked_out) {
+ return { failure => AUTH_LOCKOUT, user => $user };
+ }
- trick_taint($username);
- my ($real_password_crypted) = $dbh->selectrow_array(
- "SELECT cryptpassword FROM profiles WHERE userid = ?",
- undef, $user_id);
+ my $password = $login_data->{password};
+ my $real_password_crypted = $user->cryptpassword;
# Using the internal crypted password as the salt,
# crypt the password the user entered.
my $entered_password_crypted = bz_crypt($password, $real_password_crypted);
-
- return { failure => AUTH_LOGINFAILED }
- if $entered_password_crypted ne $real_password_crypted;
+
+ if ($entered_password_crypted ne $real_password_crypted) {
+ # Record the login failure
+ $user->note_login_failure();
+
+ # Immediately check if we are locked out
+ if ($user->account_is_locked_out) {
+ return { failure => AUTH_LOCKOUT, user => $user,
+ just_locked_out => 1 };
+ }
+
+ return { failure => AUTH_LOGINFAILED,
+ failure_count => scalar(@{ $user->account_ip_login_failures }),
+ };
+ }
# The user's credentials are okay, so delete any outstanding
- # password tokens they may have generated.
- Bugzilla::Token::DeletePasswordTokens($user_id, "user_logged_in");
+ # password tokens or login failures they may have generated.
+ Bugzilla::Token::DeletePasswordTokens($user->id, "user_logged_in");
+ $user->clear_login_failures();
# If their old password was using crypt() or some different hash
# than we're using now, convert the stored password to using
# whatever hashing system we're using now.
my $current_algorithm = PASSWORD_DIGEST_ALGORITHM;
if ($real_password_crypted !~ /{\Q$current_algorithm\E}$/) {
- my $new_crypted = bz_crypt($password);
- $dbh->do('UPDATE profiles SET cryptpassword = ? WHERE userid = ?',
- undef, $new_crypted, $user_id);
+ $user->set_password($password);
+ $user->update();
}
return $login_data;
diff --git a/Bugzilla/Config/Common.pm b/Bugzilla/Config/Common.pm
index 95866b032..6924761f3 100644
--- a/Bugzilla/Config/Common.pm
+++ b/Bugzilla/Config/Common.pm
@@ -34,6 +34,7 @@ package Bugzilla::Config::Common;
use strict;
+use Email::Address;
use Socket;
use Bugzilla::Util;
@@ -50,7 +51,7 @@ use base qw(Exporter);
check_user_verify_class
check_mail_delivery_method check_notification check_utf8
check_bug_status check_smtp_auth check_theschwartz_available
- check_maxattachmentsize
+ check_maxattachmentsize check_email
);
# Checking functions for the various values
@@ -94,6 +95,14 @@ sub check_regexp {
return $@;
}
+sub check_email {
+ my ($value) = @_;
+ if ($value !~ $Email::Address::mailbox) {
+ return "must be a valid email address.";
+ }
+ return "";
+}
+
sub check_sslbase {
my $url = shift;
if ($url ne '') {
diff --git a/Bugzilla/Config/Core.pm b/Bugzilla/Config/Core.pm
index 91426b30a..1bfebfa69 100644
--- a/Bugzilla/Config/Core.pm
+++ b/Bugzilla/Config/Core.pm
@@ -43,7 +43,8 @@ sub get_param_list {
{
name => 'maintainer',
type => 't',
- default => 'THE MAINTAINER HAS NOT YET BEEN SET'
+ default => 'please.set.the.maintainer.parameter@administration.parameters',
+ checker => \&check_email,
},
{
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 0be6d1efa..e052d2ecb 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -55,6 +55,7 @@ use File::Basename;
AUTH_LOGINFAILED
AUTH_DISABLED
AUTH_NO_SUCH_USER
+ AUTH_LOCKOUT
USER_PASSWORD_MIN_LENGTH
@@ -149,6 +150,8 @@ use File::Basename;
MAX_TOKEN_AGE
MAX_LOGINCOOKIE_AGE
+ MAX_LOGIN_ATTEMPTS
+ LOGIN_LOCKOUT_INTERVAL
SAFE_PROTOCOLS
LEGAL_CONTENT_TYPES
@@ -227,6 +230,7 @@ use constant AUTH_ERROR => 2;
use constant AUTH_LOGINFAILED => 3;
use constant AUTH_DISABLED => 4;
use constant AUTH_NO_SUCH_USER => 5;
+use constant AUTH_LOCKOUT => 6;
# The minimum length a password must have.
use constant USER_PASSWORD_MIN_LENGTH => 6;
@@ -373,6 +377,12 @@ use constant MAX_TOKEN_AGE => 3;
# How many days a logincookie will remain valid if not used.
use constant MAX_LOGINCOOKIE_AGE => 30;
+# Maximum failed logins to lock account for this IP
+use constant MAX_LOGIN_ATTEMPTS => 5;
+# If the maximum login attempts occur during this many minutes, the
+# account is locked.
+use constant LOGIN_LOCKOUT_INTERVAL => 30;
+
# Protocols which are considered as safe.
use constant SAFE_PROTOCOLS => ('afs', 'cid', 'ftp', 'gopher', 'http', 'https',
'irc', 'mid', 'news', 'nntp', 'prospero', 'telnet',
diff --git a/Bugzilla/DB/Schema.pm b/Bugzilla/DB/Schema.pm
index f34f05e2f..a2df26425 100644
--- a/Bugzilla/DB/Schema.pm
+++ b/Bugzilla/DB/Schema.pm
@@ -23,6 +23,7 @@
# Lance Larsh <lance.larsh@oracle.com>
# Dennis Melentyev <dennis.melentyev@infopulse.com.ua>
# Akamai Technologies <bugzilla-dev@akamai.com>
+# Elliotte Martin <emartin@everythingsolved.com>
package Bugzilla::DB::Schema;
@@ -982,6 +983,25 @@ use constant ABSTRACT_SCHEMA => {
],
},
+ login_failure => {
+ FIELDS => [
+ user_id => {TYPE => 'INT3', NOTNULL => 1,
+ REFERENCES => {TABLE => 'profiles',
+ COLUMN => 'userid',
+ DELETE => 'CASCADE'}},
+ login_time => {TYPE => 'DATETIME', NOTNULL => 1},
+ ip_addr => {TYPE => 'varchar(40)', NOTNULL => 1},
+ ],
+ INDEXES => [
+ # We do lookups by every item in the table simultaneously, but
+ # having an index with all three items would be the same size as
+ # the table. So instead we have an index on just the smallest item,
+ # to speed lookups.
+ login_failure_user_id_idx => ['user_id'],
+ ],
+ },
+
+
# "tokens" stores the tokens users receive when a password or email
# change is requested. Tokens provide an extra measure of security
# for these changes.
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3e6f3b6ba..e8ea2878e 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -65,6 +65,11 @@ use base qw(Bugzilla::Object Exporter);
# Constants
#####################################################################
+# Used as the IP for authentication failures for password-lockout purposes
+# when there is no IP (for example, if we're doing authentication from the
+# command line for some reason).
+use constant NO_IP => '255.255.255.255';
+
use constant USER_MATCH_MULTIPLE => -1;
use constant USER_MATCH_FAILED => 0;
use constant USER_MATCH_SUCCESS => 1;
@@ -247,6 +252,15 @@ sub is_disabled { $_[0]->disabledtext ? 1 : 0; }
sub showmybugslink { $_[0]->{showmybugslink}; }
sub email_disabled { $_[0]->{disable_mail}; }
sub email_enabled { !($_[0]->{disable_mail}); }
+sub cryptpassword {
+ my $self = shift;
+ # We don't store it because we never want it in the object (we
+ # don't want to accidentally dump even the hash somewhere).
+ my ($pw) = Bugzilla->dbh->selectrow_array(
+ 'SELECT cryptpassword FROM profiles WHERE userid = ?',
+ undef, $self->id);
+ return $pw;
+}
sub set_authorizer {
my ($self, $authorizer) = @_;
@@ -1655,6 +1669,54 @@ sub create {
return $user;
}
+###########################
+# Account Lockout Methods #
+###########################
+
+sub account_is_locked_out {
+ my $self = shift;
+ my $login_failures = scalar @{ $self->account_ip_login_failures };
+ return $login_failures >= MAX_LOGIN_ATTEMPTS ? 1 : 0;
+}
+
+sub note_login_failure {
+ my $self = shift;
+ my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+ trick_taint($ip_addr);
+ Bugzilla->dbh->do("INSERT INTO login_failure (user_id, ip_addr, login_time)
+ VALUES (?, ?, LOCALTIMESTAMP(0))",
+ undef, $self->id, $ip_addr);
+ delete $self->{account_ip_login_failures};
+}
+
+sub clear_login_failures {
+ my $self = shift;
+ my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+ trick_taint($ip_addr);
+ Bugzilla->dbh->do(
+ 'DELETE FROM login_failure WHERE user_id = ? AND ip_addr = ?',
+ undef, $self->id, $ip_addr);
+ delete $self->{account_ip_login_failures};
+}
+
+sub account_ip_login_failures {
+ my $self = shift;
+ my $dbh = Bugzilla->dbh;
+ my $time = $dbh->sql_interval(LOGIN_LOCKOUT_INTERVAL, 'MINUTE');
+ my $ip_addr = Bugzilla->cgi->remote_addr || NO_IP;
+ trick_taint($ip_addr);
+ $self->{account_ip_login_failures} ||= Bugzilla->dbh->selectall_arrayref(
+ "SELECT login_time, ip_addr, user_id FROM login_failure
+ WHERE user_id = ? AND login_time > LOCALTIMESTAMP(0) - $time
+ AND ip_addr = ?
+ ORDER BY login_time", {Slice => {}}, $self->id, $ip_addr);
+ return $self->{account_ip_login_failures};
+}
+
+###############
+# Subroutines #
+###############
+
sub is_available_username {
my ($username, $old_username) = @_;
@@ -1848,6 +1910,29 @@ groups.
=back
+=head2 Account Lockout
+
+=over
+
+=item C<account_is_locked_out>
+
+Returns C<1> if the account has failed to log in too many times recently,
+and thus is locked out for a period of time. Returns C<0> otherwise.
+
+=item C<account_ip_login_failures>
+
+Returns an arrayref of hashrefs, that contains information about the recent
+times that this account has failed to log in from the current remote IP.
+The hashes contain C<ip_addr>, C<login_time>, and C<user_id>.
+
+=item C<note_login_failure>
+
+This notes that this account has failed to log in, and stores the fact
+in the database. The storing happens immediately, it does not wait for
+you to call C<update>.
+
+=back
+
=head2 Other Methods
=over
diff --git a/template/en/default/email/lockout.txt.tmpl b/template/en/default/email/lockout.txt.tmpl
new file mode 100644
index 000000000..ac6525779
--- /dev/null
+++ b/template/en/default/email/lockout.txt.tmpl
@@ -0,0 +1,39 @@
+[%# The contents of this file are subject to the Mozilla Public
+ # License Version 1.1 (the "License"); you may not use this file
+ # except in compliance with the License. You may obtain a copy of
+ # the License at http://www.mozilla.org/MPL/
+ #
+ # Software distributed under the License is distributed on an "AS
+ # IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+ # implied. See the License for the specific language governing
+ # rights and limitations under the License.
+ #
+ # The Original Code is the Bugzilla Bug Tracking System.
+ #
+ # The Initial Developer of the Original Code is the Mozilla Corporation.
+ # Portions created by the Initial Developer are Copyright (C) 2008
+ # the Initial Developer. All Rights Reserved.
+ #
+ # Contributor(s):
+ # Max Kanat-Alexander <mkanat@bugzilla.org>
+ #%]
+
+[% PROCESS global/variables.none.tmpl %]
+
+From: [% Param('mailfrom') %]
+To: [% Param('maintainer') %]
+Subject: [[% terms.Bugzilla %]] Account Lock-Out: [% locked_user.login %] ([% attempts.0.ip_addr %])
+X-Bugzilla-Type: admin
+
+The IP address [% attempts.0.ip_addr %] failed too many login attempts (
+[%- constants.MAX_LOGIN_ATTEMPTS +%]) for
+the account [% locked_user.login %].
+
+The login attempts occurred at these times:
+
+[% FOREACH login = attempts %]
+ [%+ login.login_time FILTER time %]
+[% END %]
+
+This IP will be able to log in again using this account at
+[%+ unlock_at FILTER time %].
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index c4602f7d8..1d72fbd71 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -77,6 +77,12 @@
that login name.
[% END %]
+ [% ELSIF error == "account_locked" %]
+ [% title = "Account Locked" %]
+ Your IP ([% ip_addr FILTER html %]) has been locked out of this
+ account until [% unlock_at FILTER time %], as you have
+ exceeded the maximum number of login attempts.
+
[% ELSIF error == "alias_has_comma_or_space" %]
[% title = "Invalid Characters In Alias" %]
The alias you entered, <em>[% alias FILTER html %]</em>,
@@ -962,6 +968,15 @@
[% ELSIF error == "invalid_username_or_password" %]
[% title = "Invalid Username Or Password" %]
The username or password you entered is not valid.
+ [%# People get two login attempts before being warned about
+ # being locked out.
+ #%]
+ [% IF remaining <= 2 %]
+ If you do not enter the correct password after
+ [%+ remaining FILTER html %] more attempt(s), you will be
+ locked out of this account for
+ [%+ constants.LOGIN_LOCKOUT_INTERVAL FILTER html %] minutes.
+ [% END %]
[% ELSIF error == "json_rpc_post_only" %]
For security reasons, you may only use JSON-RPC with the POST