diff --git a/Gemfile b/Gemfile
index c0e31fb6c5c598c1d00e3064bf2ec491810073f6..493bd42ce3a890a76ab8018563615b665ce444c3 100644
--- a/Gemfile
+++ b/Gemfile
@@ -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 134ee9a2c10ba36f2b30894be61ca0afd985079f..d3e98209694c33c4620b07248691dbf05a96a1bd 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 6a721826170ac1283c56ca73f89a1d635aab8be7..219920b2b199f626149d47509ad3c56854bc4c53 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 be7c2175cb2dc7cac20c8dc1280a638fd9c03ab6..a95cb09755b1fe5787c1a9059f11a044c98e056a 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 1cabfb558031c6efc5df1af506828958538672eb..67c8056becb6f8d6c1b750859254ba2261ea2388 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -450,16 +450,22 @@
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 @@
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 @@ def attempt_login(include_password)
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 @@ def attempt_login(include_password)
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