summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMary Umoh <umohm12@gmail.com>2017-06-30 01:03:46 +0200
committerDylan William Hardison <dylan@hardison.net>2017-07-07 00:19:20 +0200
commit4c9f9a8c49e9f25096ee3b6982b197e9efa6dd60 (patch)
tree21fd41e87f0838321f4494f784fd94bc1f1b679f
parent662b0801c0e429b7d83c2ad6ed47a0293f10ff5e (diff)
downloadbugzilla-4c9f9a8c49e9f25096ee3b6982b197e9efa6dd60.tar.gz
bugzilla-4c9f9a8c49e9f25096ee3b6982b197e9efa6dd60.tar.xz
Bug 1355169 - Add rate-limiting to show_bug.cgi and rest.cgi
* fix mistake * Update * Updates * remove other file
-rw-r--r--Bugzilla.pm28
-rw-r--r--Bugzilla/Config/Admin.pm33
-rw-r--r--Bugzilla/WebService/Bug.pm5
-rwxr-xr-xshow_bug.cgi6
-rw-r--r--template/en/default/admin/params/admin.html.tmpl11
-rw-r--r--template/en/default/global/user-error.html.tmpl4
6 files changed, 80 insertions, 7 deletions
diff --git a/Bugzilla.pm b/Bugzilla.pm
index b6dcd58ab..2d59d4171 100644
--- a/Bugzilla.pm
+++ b/Bugzilla.pm
@@ -55,6 +55,7 @@ use File::Basename;
use File::Spec::Functions;
use Safe;
use Sys::Syslog qw(:DEFAULT);
+use JSON::XS qw(decode_json);
use parent qw(Bugzilla::CPAN);
@@ -156,7 +157,7 @@ sub init_page {
}
# If Bugzilla is shut down, do not allow anything to run, just display a
- # message to the user about the downtime and log out. Scripts listed in
+ # message to the user about the downtime and log out. Scripts listed in
# SHUTDOWNHTML_EXEMPT are exempt from this message.
#
# This code must go here. It cannot go anywhere in Bugzilla::CGI, because
@@ -202,7 +203,7 @@ sub init_page {
if (i_am_cgi()) {
# Set the HTTP status to 503 when Bugzilla is down to avoid pages
# being indexed by search engines.
- print Bugzilla->cgi->header(-status => 503,
+ print Bugzilla->cgi->header(-status => 503,
-retry_after => SHUTDOWNHTML_RETRY_AFTER);
}
my $t_output;
@@ -773,6 +774,23 @@ sub elastic {
$class->process_cache->{elastic} //= Bugzilla::Elastic->new();
}
+sub check_rate_limit {
+ my ($class, $name, $id) = @_;
+ my $params = Bugzilla->params;
+ if ($params->{rate_limit_active}) {
+ my $rules = decode_json($params->{rate_limit_rules});
+ my $limit = $rules->{$name};
+ unless ($limit) {
+ warn "no rules for $name!";
+ return 0;
+ }
+ if (Bugzilla->memcached->should_rate_limit("$name:$id", @$limit)) {
+ Bugzilla->audit("[rate_limit] $id exceeds rate limit $name: " . join("/", @$limit));
+ ThrowUserError("rate_limit");
+ }
+ }
+}
+
# Private methods
# Per-process cleanup. Note that this is a plain subroutine, not a method,
@@ -936,8 +954,8 @@ progress, returns the C<Bugzilla::User> object corresponding to the currently
logged in user.
=item C<sudo_request>
-This begins an sudo session for the current request. It is meant to be
-used when a session has just started. For normal use, sudo access should
+This begins an sudo session for the current request. It is meant to be
+used when a session has just started. For normal use, sudo access should
normally be set at login time.
=item C<login>
@@ -1034,7 +1052,7 @@ C<Bugzilla->usage_mode> will return the current state of this flag.
=item C<installation_mode>
-Determines whether or not installation should be silent. See
+Determines whether or not installation should be silent. See
L<Bugzilla::Constants> for the C<INSTALLATION_MODE> constants.
=item C<installation_answers>
diff --git a/Bugzilla/Config/Admin.pm b/Bugzilla/Config/Admin.pm
index 74748d3d8..5f10bfef3 100644
--- a/Bugzilla/Config/Admin.pm
+++ b/Bugzilla/Config/Admin.pm
@@ -12,6 +12,9 @@ use strict;
use warnings;
use Bugzilla::Config::Common;
+use JSON::XS qw(decode_json);
+use List::MoreUtils qw(all);
+use Scalar::Util qw(looks_like_number);
our $sortkey = 200;
@@ -44,6 +47,19 @@ sub get_param_list {
},
{
+ name => 'rate_limit_active',
+ type => 'b',
+ default => 1,
+ },
+
+ {
+ name => 'rate_limit_rules',
+ type => 'l',
+ default => '{"get_bug": [75, 60], "show_bug": [75, 60]}',
+ checker => \&check_rate_limit_rules,
+ },
+
+ {
name => 'log_user_requests',
type => 'b',
default => 0,
@@ -51,4 +67,21 @@ sub get_param_list {
return @param_list;
}
+sub check_rate_limit_rules {
+ my $rules = shift;
+
+ my $val = eval { decode_json($rules) };
+ return "failed to parse json" unless defined $val;
+ return "value is not HASH" unless ref $val && ref($val) eq 'HASH';
+ return "rules are invalid" unless all {
+ ref($_) eq 'ARRAY' && looks_like_number( $_->[0] ) && looks_like_number( $_->[1] )
+ } values %$val;
+
+ foreach my $required (qw( show_bug get_bug )) {
+ return "missing $required" unless exists $val->{$required};
+ }
+
+ return "";
+}
+
1;
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 78545e129..97dd46d0e 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -22,7 +22,7 @@ use Bugzilla::WebService::Constants;
use Bugzilla::WebService::Util qw(extract_flags filter filter_wants validate translate);
use Bugzilla::Bug;
use Bugzilla::BugMail;
-use Bugzilla::Util qw(trick_taint trim detaint_natural);
+use Bugzilla::Util qw(trick_taint trim detaint_natural remote_ip);
use Bugzilla::Version;
use Bugzilla::Milestone;
use Bugzilla::Status;
@@ -398,6 +398,9 @@ sub _translate_comment {
sub get {
my ($self, $params) = validate(@_, 'ids');
+ unless (Bugzilla->user->id) {
+ Bugzilla->check_rate_limit("get_bug", remote_ip());
+ }
Bugzilla->switch_to_shadow_db() unless Bugzilla->user->id;
my $ids = $params->{ids};
diff --git a/show_bug.cgi b/show_bug.cgi
index d2695a66f..172394781 100755
--- a/show_bug.cgi
+++ b/show_bug.cgi
@@ -20,7 +20,7 @@ use Bugzilla::Keyword;
use Bugzilla::Bug;
use Bugzilla::Hook;
use Bugzilla::CGI;
-use Bugzilla::Util qw(detaint_natural);
+use Bugzilla::Util qw(detaint_natural remote_ip);
my $cgi = Bugzilla->cgi;
my $template = Bugzilla->template;
@@ -28,6 +28,10 @@ my $vars = {};
my $user = Bugzilla->login();
+unless ($user->id) {
+ Bugzilla->check_rate_limit("show_bug", remote_ip());
+}
+
# BMO: add show_bug_format for experimental UI work
my $format_params = {
format => scalar $cgi->param('format'),
diff --git a/template/en/default/admin/params/admin.html.tmpl b/template/en/default/admin/params/admin.html.tmpl
index df0580783..ee19418c7 100644
--- a/template/en/default/admin/params/admin.html.tmpl
+++ b/template/en/default/admin/params/admin.html.tmpl
@@ -23,6 +23,13 @@
desc = "Set up account policies"
%]
+[% rate_limit_rules_desc = BLOCK %]
+This parameter is a json object. It has one or more valid keys, whose values are each of an array [MAX_RATE, SECONDS]. MAX_RATE is the maximum
+number of requests that can occur over SECONDS. The default is [75, 60] or 75 requests
+over 60 seconds. Valid keys are <code>get_b[%''%]ug</code> which covers JSONRPC, XMLRPC, REST and BZAPI single
+[% terms.bug %] access methods, and <code>show_b[%''%]ug</code> which controls show [% terms.bug %]
+[% END %]
+
[% param_descs = {
allowbugdeletion => "The pages to edit products and components can delete all " _
"associated $terms.bugs when you delete a product (or component). " _
@@ -42,5 +49,9 @@
last_visit_keep_days => "This option controls how many days $terms.Bugzilla will " _
"remember when users visit specific ${terms.bugs}.",
+ rate_limit_active => "Allow some types of requests to be rate limited."
+
+ rate_limit_rules => rate_limit_rules_desc
+
log_user_requests => "This option controls logging of authenticated requests in the user_request_log table"}
%]
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 29408e193..9d241ea71 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1812,6 +1812,10 @@
The token you submitted does not exist, has expired, or has
been canceled.
+ [% ELSIF error == "rate_limit" %]
+ [% title = "Rate Limit Exceeded" %]
+ You have exceeded the rate limit.
+
[% ELSIF error == "too_soon_for_new_token" %]
[% title = "Too Soon For New Token" %]
You have requested