Merge branch 'sh-support-subnets-ip-rate-limiter' into 'master'

Support CIDR notation in IP rate limiter

See merge request gitlab-org/gitlab-ce!30146

5 files changed, 94 insertions, 3 deletions

diff --git a/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml b/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml
new file mode 100644
index 00000000000..3e78c58c764
--- /dev/null
+++ b/changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml
@@ -0,0 +1,5 @@
+---
+title: Support CIDR notation in IP rate limiter
+merge_request: 30146
+author:
+type: changed
diff --git a/doc/security/rack_attack.md b/doc/security/rack_attack.md
index fa4b0d1fb09..8695b5d2194 100644
--- a/doc/security/rack_attack.md
+++ b/doc/security/rack_attack.md
@@ -53,8 +53,9 @@ For more information on how to use these options check out
 The following settings can be configured:
 
 - `enabled`: By default this is set to `false`. Set this to `true` to enable Rack Attack.
-- `ip_whitelist`: Whitelist any IPs from being blocked. They must be formatted as strings within a ruby array.
-   For example, `["127.0.0.1", "127.0.0.2", "127.0.0.3"]`.
+- `ip_whitelist`: Whitelist any IPs from being blocked. They must be formatted as strings within a Ruby array.
+   CIDR notation is supported in GitLab v12.1 and up.
+   For example, `["127.0.0.1", "127.0.0.2", "127.0.0.3", "192.168.0.1/24"]`.
 - `maxretry`: The maximum amount of times a request can be made in the
    specified time.
 - `findtime`: The maximum amount of time that failed requests can count against an IP
diff --git a/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb
index 81e616fa20a..0b7055b3256 100644
--- a/lib/gitlab/auth/ip_rate_limiter.rb
+++ b/lib/gitlab/auth/ip_rate_limiter.rb
@@ -3,6 +3,8 @@
 module Gitlab
   module Auth
     class IpRateLimiter
+      include ::Gitlab::Utils::StrongMemoize
+
       attr_reader :ip
 
       def initialize(ip)
@@ -37,7 +39,20 @@ module Gitlab
       end
 
       def ip_can_be_banned?
-        config.ip_whitelist.exclude?(ip)
+        !trusted_ip?
+      end
+
+      def trusted_ip?
+        trusted_ips.any? { |netmask| netmask.include?(ip) }
+      end
+
+      def trusted_ips
+        strong_memoize(:trusted_ips) do
+          config.ip_whitelist.map do |proxy|
+            IPAddr.new(proxy)
+          rescue IPAddr::InvalidAddressError
+          end.compact
+        end
       end
     end
   end
diff --git a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb
new file mode 100644
index 00000000000..8d6bf45ab30
--- /dev/null
+++ b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb
@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Auth::IpRateLimiter, :use_clean_rails_memory_store_caching do
+  let(:ip) { '10.2.2.3' }
+  let(:whitelist) { ['127.0.0.1'] }
+  let(:options) do
+    {
+      enabled: true,
+      ip_whitelist: whitelist,
+      bantime: 1.minute,
+      findtime: 1.minute,
+      maxretry: 2
+    }
+  end
+
+  subject { described_class.new(ip) }
+
+  before do
+    stub_rack_attack_setting(options)
+  end
+
+  after do
+    subject.reset!
+  end
+
+  describe '#register_fail!' do
+    it 'bans after 3 consecutive failures' do
+      expect(subject.banned?).to be_falsey
+
+      3.times { subject.register_fail! }
+
+      expect(subject.banned?).to be_truthy
+    end
+
+    shared_examples 'whitelisted IPs' do
+      it 'does not ban after max retry limit' do
+        expect(subject.banned?).to be_falsey
+
+        3.times { subject.register_fail! }
+
+        expect(subject.banned?).to be_falsey
+      end
+    end
+
+    context 'with a whitelisted netmask' do
+      before do
+        options[:ip_whitelist] = ['127.0.0.1', '10.2.2.0/24', 'bad']
+        stub_rack_attack_setting(options)
+      end
+
+      it_behaves_like 'whitelisted IPs'
+    end
+
+    context 'with a whitelisted IP' do
+      before do
+        options[:ip_whitelist] = ['10.2.2.3']
+        stub_rack_attack_setting(options)
+      end
+
+      it_behaves_like 'whitelisted IPs'
+    end
+  end
+end
diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb
index 0d591f038ce..c372a3f0e49 100644
--- a/spec/support/helpers/stub_configuration.rb
+++ b/spec/support/helpers/stub_configuration.rb
@@ -95,6 +95,11 @@ module StubConfiguration
     allow(Gitlab.config.gitlab_shell).to receive_messages(to_settings(messages))
   end
 
+  def stub_rack_attack_setting(messages)
+    allow(Gitlab.config.rack_attack).to receive(:git_basic_auth).and_return(messages)
+    allow(Gitlab.config.rack_attack.git_basic_auth).to receive_messages(to_settings(messages))
+  end
+
   private
 
   # Modifies stubbed messages to also stub possible predicate versions