From cecd7d0d0706306adb5a90a9ff443e5ff87aa058 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 8 Jan 2018 08:39:17 +1100
Subject: [PATCH] FEATURE: global rate limiter can bypass local IPs

---
 config/discourse_defaults.conf                |  5 ++-
 lib/middleware/request_tracker.rb             | 22 +++++++++--
 .../middleware/request_tracker_spec.rb        | 39 +++++++++++++++++++
 3 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/config/discourse_defaults.conf b/config/discourse_defaults.conf
index 2945e772364..488ead77d54 100644
--- a/config/discourse_defaults.conf
+++ b/config/discourse_defaults.conf
@@ -178,5 +178,8 @@ 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
+# global rate limiter will simply warn if the limit is exceeded, can be warn+block, warn, block or none
 max_requests_per_ip_mode = none
+
+# bypass rate limiting any IP resolved as a private IP
+max_requests_rate_limit_on_private = false
diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb
index 70a01532fe1..2b2e318d993 100644
--- a/lib/middleware/request_tracker.rb
+++ b/lib/middleware/request_tracker.rb
@@ -142,14 +142,27 @@ class Middleware::RequestTracker
     log_request_info(env, result, info) unless env["discourse.request_tracker.skip"]
   end
 
+  PRIVATE_IP = /^(127\.)|(192\.168\.)|(10\.)|(172\.1[6-9]\.)|(172\.2[0-9]\.)|(172\.3[0-1]\.)|(::1$)|([fF][cCdD])/
+
+  def is_private_ip?(ip)
+    ip = IPAddr.new(ip) rescue nil
+    !!(ip && ip.to_s.match?(PRIVATE_IP))
+  end
+
   def rate_limit(env)
+
     if (
       GlobalSetting.max_requests_per_ip_mode == "block" ||
-      GlobalSetting.max_requests_per_ip_mode == "warn"
+      GlobalSetting.max_requests_per_ip_mode == "warn" ||
+      GlobalSetting.max_requests_per_ip_mode == "warn+block"
     )
 
       ip = Rack::Request.new(env).ip
 
+      if !GlobalSetting.max_requests_rate_limit_on_private
+        return false if is_private_ip?(ip)
+      end
+
       limiter10 = RateLimiter.new(
         nil,
         "global_ip_limit_10_#{ip}",
@@ -172,9 +185,12 @@ class Middleware::RequestTracker
         type = 60
         limiter60.performed!
       rescue RateLimiter::LimitExceeded
-        if GlobalSetting.max_requests_per_ip_mode == "warn"
+        if (
+          GlobalSetting.max_requests_per_ip_mode == "warn" ||
+          GlobalSetting.max_requests_per_ip_mode == "warn+block"
+        )
           Rails.logger.warn("Global IP rate limit exceeded for #{ip}: #{type} second rate limit, uri: #{env["REQUEST_URI"]}")
-          false
+          !(GlobalSetting.max_requests_per_ip_mode == "warn")
         else
           true
         end
diff --git a/spec/components/middleware/request_tracker_spec.rb b/spec/components/middleware/request_tracker_spec.rb
index 94d76b94c16..d27ca361edb 100644
--- a/spec/components/middleware/request_tracker_spec.rb
+++ b/spec/components/middleware/request_tracker_spec.rb
@@ -112,6 +112,45 @@ describe Middleware::RequestTracker do
       expect(status).to eq(200)
     end
 
+    it "blocks private IPs if not skipped" do
+      global_setting :max_requests_per_ip_per_10_seconds, 1
+      global_setting :max_requests_per_ip_mode, 'warn+block'
+      global_setting :max_requests_rate_limit_on_private, true
+
+      env1 = env("REMOTE_ADDR" => "127.0.0.2")
+
+      status, _ = middleware.call(env1)
+      status, _ = middleware.call(env1)
+
+      expect(Rails.logger.warnings).to eq(1)
+      expect(status).to eq(429)
+    end
+
+    it "does nothing for private IPs if skipped" do
+      global_setting :max_requests_per_ip_per_10_seconds, 1
+      global_setting :max_requests_per_ip_mode, 'warn+block'
+      global_setting :max_requests_rate_limit_on_private, false
+
+      env1 = env("REMOTE_ADDR" => "127.0.3.1")
+
+      status, _ = middleware.call(env1)
+      status, _ = middleware.call(env1)
+
+      expect(Rails.logger.warnings).to eq(0)
+      expect(status).to eq(200)
+    end
+
+    it "does warn if rate limiter is enabled via warn+block" do
+      global_setting :max_requests_per_ip_per_10_seconds, 1
+      global_setting :max_requests_per_ip_mode, 'warn+block'
+
+      status, _ = middleware.call(env)
+      status, _ = middleware.call(env)
+
+      expect(Rails.logger.warnings).to eq(1)
+      expect(status).to eq(429)
+    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'