diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-27 12:05:56 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-27 12:05:56 +0300 |
commit | cc5d0271c249636bae1de55de9c2bf815d669afa (patch) | |
tree | 01b5b05c2376fca5a854459460a317c5fef96889 | |
parent | 529bc7e23ba25fb310c73a3d47759bfdd8b97a0a (diff) |
Add latest changes from gitlab-org/gitlab@master
-rw-r--r-- | Gemfile | 6 | ||||
-rw-r--r-- | Gemfile.lock | 6 | ||||
-rw-r--r-- | config/initializers/rack_attack_git_basic_auth.rb | 20 | ||||
-rw-r--r-- | config/initializers/rack_attack_logging.rb | 6 | ||||
-rw-r--r-- | spec/requests/git_http_spec.rb | 26 |
5 files changed, 33 insertions, 31 deletions
@@ -259,9 +259,6 @@ gem 'loofah', '~> 2.2' # Working with license gem 'licensee', '~> 8.9' -# Protect against bruteforcing -gem 'rack-attack', '~> 4.4.1' - # Ace editor gem 'ace-rails-ap', '~> 4.1.0' @@ -293,6 +290,9 @@ gem 'base32', '~> 0.3.0' gem "gitlab-license", "~> 1.0" +# Protect against bruteforcing +gem 'rack-attack', '~> 6.2.0' + # Sentry integration gem 'sentry-raven', '~> 2.9' diff --git a/Gemfile.lock b/Gemfile.lock index 134ee9a2c10..d3e98209694 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -734,8 +734,8 @@ GEM rack (2.0.7) rack-accept (0.4.5) rack (>= 0.4) - rack-attack (4.4.1) - rack + rack-attack (6.2.0) + rack (>= 1.0, < 3) rack-cors (1.0.2) rack-oauth2 (1.9.3) activesupport @@ -1256,7 +1256,7 @@ DEPENDENCIES puma (~> 3.12) puma_worker_killer rack (~> 2.0.7) - rack-attack (~> 4.4.1) + rack-attack (~> 6.2.0) rack-cors (~> 1.0.0) rack-oauth2 (~> 1.9.3) rack-proxy (~> 0.6.0) diff --git a/config/initializers/rack_attack_git_basic_auth.rb b/config/initializers/rack_attack_git_basic_auth.rb index 6a721826170..219920b2b19 100644 --- a/config/initializers/rack_attack_git_basic_auth.rb +++ b/config/initializers/rack_attack_git_basic_auth.rb @@ -1,14 +1,12 @@ -rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled'] +# Tell the Rack::Attack Rack middleware to maintain an IP blacklist. +# We update the blacklist in Gitlab::Auth::IpRateLimiter. +Rack::Attack.blocklist('Git HTTP Basic Auth') do |req| + next false unless Gitlab.config.rack_attack.git_basic_auth.enabled -unless Rails.env.test? || !rack_attack_enabled - # Tell the Rack::Attack Rack middleware to maintain an IP blacklist. We will - # update the blacklist from Grack::Auth#authenticate_user. - Rack::Attack.blacklist('Git HTTP Basic Auth') do |req| - Rack::Attack::Allow2Ban.filter(req.ip, Gitlab.config.rack_attack.git_basic_auth) do - # This block only gets run if the IP was not already banned. - # Return false, meaning that we do not see anything wrong with the - # request at this time - false - end + Rack::Attack::Allow2Ban.filter(req.ip, Gitlab.config.rack_attack.git_basic_auth) do + # This block only gets run if the IP was not already banned. + # Return false, meaning that we do not see anything wrong with the + # request at this time + false end end diff --git a/config/initializers/rack_attack_logging.rb b/config/initializers/rack_attack_logging.rb index be7c2175cb2..a95cb09755b 100644 --- a/config/initializers/rack_attack_logging.rb +++ b/config/initializers/rack_attack_logging.rb @@ -2,8 +2,10 @@ # # Adds logging for all Rack Attack blocks and throttling events. -ActiveSupport::Notifications.subscribe('rack.attack') do |name, start, finish, request_id, req| - if [:throttle, :blacklist].include? req.env['rack.attack.match_type'] +ActiveSupport::Notifications.subscribe(/rack_attack/) do |name, start, finish, request_id, payload| + req = payload[:request] + + if [:throttle, :blocklist].include? req.env['rack.attack.match_type'] rack_attack_info = { message: 'Rack_Attack', env: req.env['rack.attack.match_type'], diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 1cabfb55803..67c8056becb 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -450,16 +450,22 @@ describe 'Git HTTP requests' do context "when authentication fails" do context "when the user is IP banned" do before do - Gitlab.config.rack_attack.git_basic_auth['enabled'] = true + stub_rack_attack_setting(enabled: true) end - it "responds with status 401" do + it "responds with status 403" do expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) - allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return('1.2.3.4') + expect(Gitlab::AuthLogger).to receive(:error).with({ + message: 'Rack_Attack', + env: :blocklist, + remote_ip: '127.0.0.1', + request_method: 'GET', + path: "/#{path}/info/refs?service=git-upload-pack" + }) clone_get(path, env) - expect(response).to have_gitlab_http_status(:unauthorized) + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -493,7 +499,7 @@ describe 'Git HTTP requests' do context "when the user isn't blocked" do before do - Gitlab.config.rack_attack.git_basic_auth['enabled'] = true + stub_rack_attack_setting(enabled: true, bantime: 1.minute, findtime: 5.minutes, maxretry: 2, ip_whitelist: []) end it "resets the IP in Rack Attack on download" do @@ -652,9 +658,11 @@ describe 'Git HTTP requests' do response.status end + include_context 'rack attack cache store' + it "repeated attempts followed by successful attempt" do options = Gitlab.config.rack_attack.git_basic_auth - maxretry = options[:maxretry] - 1 + maxretry = options[:maxretry] ip = '1.2.3.4' allow_any_instance_of(ActionDispatch::Request).to receive(:ip).and_return(ip) @@ -666,12 +674,6 @@ describe 'Git HTTP requests' do expect(attempt_login(true)).to eq(200) expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey - - maxretry.times.each do - expect(attempt_login(false)).to eq(401) - end - - Rack::Attack::Allow2Ban.reset(ip, options) end end |