From 82c31a9addfe87e91b512abb982d2223fa4ed730 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 27 Jun 2019 15:44:46 -0700 Subject: [PATCH] Support CIDR notation in IP rate limiter This will make it possible to whitelist multiple IP addresses (e.g. 192.168.0.1/24). --- .../sh-support-subnets-ip-rate-limiter.yml | 5 ++ doc/security/rack_attack.md | 5 +- lib/gitlab/auth/ip_rate_limiter.rb | 17 ++++- spec/lib/gitlab/auth/ip_rate_limiter_spec.rb | 65 +++++++++++++++++++ spec/support/helpers/stub_configuration.rb | 5 ++ 5 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/sh-support-subnets-ip-rate-limiter.yml create mode 100644 spec/lib/gitlab/auth/ip_rate_limiter_spec.rb 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