Bug 1355169 - Add rate-limiting to show_bug.cgi and rest.cgi

* fix mistake

* Update

* Updates

* remove other file

diff --git a/Bugzilla.pm b/Bugzilla.pm
index b6dcd58ab881d0ac7d1ef1178d04cb7fd57d0a25..2d59d417159bec7614a46a3d5139abc5b721cf2e 100644 (file)
--- 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 74748d3d81fcc4b73f1770212ee4458f2a4b9aba..5f10bfef34fa319c88f4d0818f7e3c68b0985fee 100644 (file)
--- 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;
 
@@ -43,6 +46,19 @@ sub get_param_list {
    checker => \&check_numeric
   },
 
+  {
+    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',
@@ -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 78545e129cc9bc722e200216df277ae90a586fdd..97dd46d0e6d87f12ccacb1e09a29a3d0944e1fb2 100644 (file)
--- 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 d2695a66f899018f671394c5e1ddc053df1057b1..17239478131e00a09fa67cdfa8f15ff155499fb5 100755 (executable)
--- 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 df0580783c90ccfa00f86e05f27fa6d5ed70eedb..ee19418c7bf8af1e32b81c22eb1640012df0ecf5 100644 (file)
--- 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 29408e19300e64012041338d6ed1842acd5d8cc9..9d241ea71947282196816c02da23b5be3100dc7d 100644 (file)
--- 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