From 4c9f9a8c49e9f25096ee3b6982b197e9efa6dd60 Mon Sep 17 00:00:00 2001 From: Mary Umoh Date: Thu, 29 Jun 2017 16:03:46 -0700 Subject: Bug 1355169 - Add rate-limiting to show_bug.cgi and rest.cgi * fix mistake * Update * Updates * remove other file --- Bugzilla.pm | 28 ++++++++++++++++---- Bugzilla/Config/Admin.pm | 33 ++++++++++++++++++++++++ Bugzilla/WebService/Bug.pm | 5 +++- show_bug.cgi | 6 ++++- template/en/default/admin/params/admin.html.tmpl | 11 ++++++++ template/en/default/global/user-error.html.tmpl | 4 +++ 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 object corresponding to the currently logged in user. =item C -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 @@ -1034,7 +1052,7 @@ Cusage_mode> will return the current state of this flag. =item C -Determines whether or not installation should be silent. See +Determines whether or not installation should be silent. See L for the C constants. =item C 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; @@ -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 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 get_b[%''%]ug which covers JSONRPC, XMLRPC, REST and BZAPI single +[% terms.bug %] access methods, and show_b[%''%]ug 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 -- cgit v1.2.3-24-g4f1b