From 4986ebcf24d48bafe3f0901b97e38f9544f86652 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 11 Dec 2017 17:21:00 +1100 Subject: [PATCH] FEATURE: optional default off global per ip rate limiter --- config/discourse_defaults.conf | 5 ++ lib/middleware/request_tracker.rb | 47 ++++++++++++ lib/rate_limiter.rb | 8 +- .../middleware/request_tracker_spec.rb | 73 +++++++++++++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf index 7809dbedf59..2945e772364 100644 --- a/config/discourse_defaults.conf +++ b/config/discourse_defaults.conf @@ -175,3 +175,8 @@ max_user_api_reqs_per_minute = 20 max_user_api_reqs_per_day = 2880 max_admin_api_reqs_per_key_per_minute = 60 + +max_requests_per_ip_per_minute = 200 +max_requests_per_ip_per_10_seconds = 50 +# global rate limiter will simply warn if the limit is exceeded, can be warn, block or none +max_requests_per_ip_mode = none diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb index 897b5ab1cf4..16b720ad5ce 100644 --- a/lib/middleware/request_tracker.rb +++ b/lib/middleware/request_tracker.rb @@ -126,6 +126,13 @@ class Middleware::RequestTracker end def call(env) + result = nil + + if rate_limit(env) + result = [429, {}, ["Slow down, too Many Requests from this IP Address"]] + return result + end + env["discourse.request_tracker"] = self MethodProfiler.start if @@detailed_request_loggers result = @app.call(env) @@ -135,6 +142,46 @@ class Middleware::RequestTracker log_request_info(env, result, info) unless env["discourse.request_tracker.skip"] end + def rate_limit(env) + if ( + GlobalSetting.max_requests_per_ip_mode == "block" || + GlobalSetting.max_requests_per_ip_mode == "warn" + ) + + ip = Rack::Request.new(env).ip + + limiter10 = RateLimiter.new( + nil, + "global_ip_limit_10_#{ip}", + GlobalSetting.max_requests_per_ip_per_10_seconds, + 10, + global: true + ) + + limiter60 = RateLimiter.new( + nil, + "global_ip_limit_60_#{ip}", + GlobalSetting.max_requests_per_ip_per_10_seconds, + 10, + global: true + ) + + type = 10 + begin + limiter10.performed! + type = 60 + limiter60.performed! + rescue RateLimiter::LimitExceeded + if GlobalSetting.max_requests_per_ip_mode == "warn" + Rails.logger.warn("Global IP rate limit exceeded for #{ip} type: #{type}") + false + else + true + end + end + end + end + def log_later(data, host) Scheduler::Defer.later("Track view", _db = nil) do self.class.log_request_on_site(data, host) diff --git a/lib/rate_limiter.rb b/lib/rate_limiter.rb index a9d82a959ad..0b3344606f3 100644 --- a/lib/rate_limiter.rb +++ b/lib/rate_limiter.rb @@ -28,6 +28,12 @@ class RateLimiter $redis.delete_prefixed(RateLimiter.key_prefix) end + def self.clear_all_global! + $redis.without_namespace.keys("GLOBAL::#{key_prefix}*").each do |k| + $redis.without_namespace.del k + end + end + def build_key(type) "#{RateLimiter.key_prefix}:#{@user && @user.id}:#{type}" end @@ -38,7 +44,7 @@ class RateLimiter @key = build_key(type) @max = max @secs = secs - @global = false + @global = global end def clear! diff --git a/spec/components/middleware/request_tracker_spec.rb b/spec/components/middleware/request_tracker_spec.rb index b61a995d22f..94d76b94c16 100644 --- a/spec/components/middleware/request_tracker_spec.rb +++ b/spec/components/middleware/request_tracker_spec.rb @@ -68,6 +68,79 @@ describe Middleware::RequestTracker do end + context "rate limiting" do + + class TestLogger + attr_accessor :warnings + + def initialize + @warnings = 0 + end + + def warn(*args) + @warnings += 1 + end + end + + before do + RateLimiter.enable + RateLimiter.clear_all_global! + + @old_logger = Rails.logger + Rails.logger = TestLogger.new + end + + after do + RateLimiter.disable + Rails.logger = @old_logger + end + + let :middleware do + app = lambda do |env| + [200, {}, ["OK"]] + end + + Middleware::RequestTracker.new(app) + end + + it "does nothing by default" do + global_setting :max_requests_per_ip_per_10_seconds, 1 + + status, _ = middleware.call(env) + status, _ = middleware.call(env) + + expect(status).to eq(200) + end + + it "does warn if rate limiter is enabled" do + global_setting :max_requests_per_ip_per_10_seconds, 1 + global_setting :max_requests_per_ip_mode, 'warn' + + status, _ = middleware.call(env) + status, _ = middleware.call(env) + + expect(Rails.logger.warnings).to eq(1) + expect(status).to eq(200) + end + + it "does block if rate limiter is enabled" do + global_setting :max_requests_per_ip_per_10_seconds, 1 + global_setting :max_requests_per_ip_mode, 'block' + + env1 = env("REMOTE_ADDR" => "1.1.1.1") + env2 = env("REMOTE_ADDR" => "1.1.1.2") + + status, _ = middleware.call(env1) + status, _ = middleware.call(env1) + + expect(status).to eq(429) + + status, _ = middleware.call(env2) + expect(status).to eq(200) + + end + end + context "callbacks" do def app(result, sql_calls: 0, redis_calls: 0) lambda do |env|