Add RateLimiter to RawController
* Limits raw requests to 300 per minute and per raw path. * Add a new attribute to ApplicationSettings so user can change this value on their instance. * Uses Gitlab::ActionRateLimiter to limit the raw requests. * Add a new method into ActionRateLimiter to log the event into auth.log Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/48717
This commit is contained in:
parent
b70dbabb63
commit
3cefc5d7df
12 changed files with 291 additions and 17 deletions
|
@ -106,6 +106,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
|
|||
:lets_encrypt_notification_email,
|
||||
:lets_encrypt_terms_of_service_accepted,
|
||||
:domain_blacklist_file,
|
||||
:raw_blob_request_limit,
|
||||
disabled_oauth_sign_in_sources: [],
|
||||
import_sources: [],
|
||||
repository_storages: [],
|
||||
|
|
|
@ -8,10 +8,30 @@ class Projects::RawController < Projects::ApplicationController
|
|||
before_action :require_non_empty_project
|
||||
before_action :assign_ref_vars
|
||||
before_action :authorize_download_code!
|
||||
before_action :show_rate_limit, only: [:show]
|
||||
|
||||
def show
|
||||
@blob = @repository.blob_at(@commit.id, @path)
|
||||
|
||||
send_blob(@repository, @blob, inline: (params[:inline] != 'false'))
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def show_rate_limit
|
||||
limiter = ::Gitlab::ActionRateLimiter.new(action: :show_raw_controller)
|
||||
|
||||
return unless limiter.throttled?([@project, @commit, @path], raw_blob_request_limit)
|
||||
|
||||
limiter.log_request(request, :raw_blob_request_limit, current_user)
|
||||
|
||||
flash[:alert] = _('You cannot access the raw file. Please wait a minute.')
|
||||
redirect_to project_blob_path(@project, File.join(@ref, @path))
|
||||
end
|
||||
|
||||
def raw_blob_request_limit
|
||||
Gitlab::CurrentSettings
|
||||
.current_application_settings
|
||||
.raw_blob_request_limit
|
||||
end
|
||||
end
|
||||
|
|
|
@ -98,7 +98,8 @@ module ApplicationSettingImplementation
|
|||
commit_email_hostname: default_commit_email_hostname,
|
||||
protected_ci_variables: false,
|
||||
local_markdown_version: 0,
|
||||
outbound_local_requests_whitelist: []
|
||||
outbound_local_requests_whitelist: [],
|
||||
raw_blob_request_limit: 300
|
||||
}
|
||||
end
|
||||
|
||||
|
|
|
@ -15,4 +15,10 @@
|
|||
AuthorizedKeysCommand. Click on the help icon for more details.
|
||||
= link_to icon('question-circle'), help_page_path('administration/operations/fast_ssh_key_lookup')
|
||||
|
||||
.form-group
|
||||
= f.label :raw_blob_request_limit, _('Raw blob request rate limit per minute'), class: 'label-bold'
|
||||
= f.number_field :raw_blob_request_limit, class: 'form-control'
|
||||
.form-text.text-muted
|
||||
= _('Highest number of requests per minute for each raw path, default to 300. To disable throttling set to 0.')
|
||||
|
||||
= f.submit 'Save changes', class: "btn btn-success"
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Add Rate Request Limiter to RawController#show endpoint
|
||||
merge_request: 30635
|
||||
author:
|
||||
type: added
|
|
@ -0,0 +1,9 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
class AddRawBlobRequestLimitToApplicationSettings < ActiveRecord::Migration[5.2]
|
||||
DOWNTIME = false
|
||||
|
||||
def change
|
||||
add_column :application_settings, :raw_blob_request_limit, :integer, default: 300, null: false
|
||||
end
|
||||
end
|
|
@ -10,7 +10,7 @@
|
|||
#
|
||||
# It's strongly recommended that you check this file into your version control system.
|
||||
|
||||
ActiveRecord::Schema.define(version: 2019_07_15_114644) do
|
||||
ActiveRecord::Schema.define(version: 2019_07_15_142138) do
|
||||
|
||||
# These are extensions that must be enabled in order to support this database
|
||||
enable_extension "pg_trgm"
|
||||
|
@ -229,6 +229,7 @@ ActiveRecord::Schema.define(version: 2019_07_15_114644) do
|
|||
t.boolean "time_tracking_limit_to_hours", default: false, null: false
|
||||
t.string "grafana_url", default: "/-/grafana", null: false
|
||||
t.string "outbound_local_requests_whitelist", limit: 255, array: true
|
||||
t.integer "raw_blob_request_limit", default: 300, null: false
|
||||
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
|
||||
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
|
||||
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id"
|
||||
|
|
|
@ -33,16 +33,48 @@ module Gitlab
|
|||
# Increments the given key and returns true if the action should
|
||||
# be throttled.
|
||||
#
|
||||
# key - An array of ActiveRecord instances
|
||||
# threshold_value - The maximum number of times this action should occur in the given time interval
|
||||
# key - An array of ActiveRecord instances or strings
|
||||
# threshold_value - The maximum number of times this action should occur in the given time interval. If number is zero is considered disabled.
|
||||
def throttled?(key, threshold_value)
|
||||
self.increment(key) > threshold_value
|
||||
threshold_value > 0 &&
|
||||
self.increment(key) > threshold_value
|
||||
end
|
||||
|
||||
# Logs request into auth.log
|
||||
#
|
||||
# request - Web request to be logged
|
||||
# type - A symbol key that represents the request.
|
||||
# current_user - Current user of the request, it can be nil.
|
||||
def log_request(request, type, current_user)
|
||||
request_information = {
|
||||
message: 'Action_Rate_Limiter_Request',
|
||||
env: type,
|
||||
ip: request.ip,
|
||||
request_method: request.request_method,
|
||||
fullpath: request.fullpath
|
||||
}
|
||||
|
||||
if current_user
|
||||
request_information.merge!({
|
||||
user_id: current_user.id,
|
||||
username: current_user.username
|
||||
})
|
||||
end
|
||||
|
||||
Gitlab::AuthLogger.error(request_information)
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def action_key(key)
|
||||
serialized = key.map { |obj| "#{obj.class.model_name.to_s.underscore}:#{obj.id}" }.join(":")
|
||||
serialized = key.map do |obj|
|
||||
if obj.is_a?(String)
|
||||
"#{obj}"
|
||||
else
|
||||
"#{obj.class.model_name.to_s.underscore}:#{obj.id}"
|
||||
end
|
||||
end.join(":")
|
||||
|
||||
"action_rate_limiter:#{action}:#{serialized}"
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5298,6 +5298,9 @@ msgstr[1] ""
|
|||
msgid "Hide values"
|
||||
msgstr ""
|
||||
|
||||
msgid "Highest number of requests per minute for each raw path, default to 300. To disable throttling set to 0."
|
||||
msgstr ""
|
||||
|
||||
msgid "Highest role:"
|
||||
msgstr ""
|
||||
|
||||
|
@ -8717,6 +8720,9 @@ msgstr ""
|
|||
msgid "Rake Tasks Help"
|
||||
msgstr ""
|
||||
|
||||
msgid "Raw blob request rate limit per minute"
|
||||
msgstr ""
|
||||
|
||||
msgid "Read more"
|
||||
msgstr ""
|
||||
|
||||
|
@ -12442,6 +12448,9 @@ msgstr ""
|
|||
msgid "You can test your .gitlab-ci.yml in %{linkStart}CI Lint%{linkEnd}."
|
||||
msgstr ""
|
||||
|
||||
msgid "You cannot access the raw file. Please wait a minute."
|
||||
msgstr ""
|
||||
|
||||
msgid "You cannot impersonate a blocked user"
|
||||
msgstr ""
|
||||
|
||||
|
|
|
@ -3,6 +3,8 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Projects::RawController do
|
||||
include RepoHelpers
|
||||
|
||||
let(:project) { create(:project, :public, :repository) }
|
||||
|
||||
describe 'GET #show' do
|
||||
|
@ -46,5 +48,98 @@ describe Projects::RawController do
|
|||
let(:filename) { 'lfs_object.iso' }
|
||||
let(:filepath) { "be93687/files/lfs/#{filename}" }
|
||||
end
|
||||
|
||||
context 'when the endpoint receives requests above the limit', :clean_gitlab_redis_cache do
|
||||
let(:file_path) { 'master/README.md' }
|
||||
|
||||
before do
|
||||
stub_application_setting(raw_blob_request_limit: 5)
|
||||
end
|
||||
|
||||
it 'prevents from accessing the raw file' do
|
||||
execute_raw_requests(requests: 6, project: project, file_path: file_path)
|
||||
|
||||
expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.')
|
||||
expect(response).to redirect_to(project_blob_path(project, file_path))
|
||||
end
|
||||
|
||||
it 'logs the event on auth.log' do
|
||||
attributes = {
|
||||
message: 'Action_Rate_Limiter_Request',
|
||||
env: :raw_blob_request_limit,
|
||||
ip: '0.0.0.0',
|
||||
request_method: 'GET',
|
||||
fullpath: "/#{project.full_path}/raw/#{file_path}"
|
||||
}
|
||||
|
||||
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
|
||||
|
||||
execute_raw_requests(requests: 6, project: project, file_path: file_path)
|
||||
end
|
||||
|
||||
context 'when the request uses a different version of a commit' do
|
||||
it 'prevents from accessing the raw file' do
|
||||
# 3 times with the normal sha
|
||||
commit_sha = project.repository.commit.sha
|
||||
file_path = "#{commit_sha}/README.md"
|
||||
|
||||
execute_raw_requests(requests: 3, project: project, file_path: file_path)
|
||||
|
||||
# 3 times with the modified version
|
||||
modified_sha = commit_sha.gsub(commit_sha[0..5], commit_sha[0..5].upcase)
|
||||
modified_path = "#{modified_sha}/README.md"
|
||||
|
||||
execute_raw_requests(requests: 3, project: project, file_path: modified_path)
|
||||
|
||||
expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.')
|
||||
expect(response).to redirect_to(project_blob_path(project, modified_path))
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the throttling has been disabled' do
|
||||
before do
|
||||
stub_application_setting(raw_blob_request_limit: 0)
|
||||
end
|
||||
|
||||
it 'does not prevent from accessing the raw file' do
|
||||
execute_raw_requests(requests: 10, project: project, file_path: file_path)
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with case-sensitive files' do
|
||||
it 'prevents from accessing the specific file' do
|
||||
create_file_in_repo(project, 'master', 'master', 'readme.md', 'Add readme.md')
|
||||
create_file_in_repo(project, 'master', 'master', 'README.md', 'Add README.md')
|
||||
|
||||
commit_sha = project.repository.commit.sha
|
||||
file_path = "#{commit_sha}/readme.md"
|
||||
|
||||
# Accessing downcase version of readme
|
||||
execute_raw_requests(requests: 6, project: project, file_path: file_path)
|
||||
|
||||
expect(flash[:alert]).to eq('You cannot access the raw file. Please wait a minute.')
|
||||
expect(response).to redirect_to(project_blob_path(project, file_path))
|
||||
|
||||
# Accessing upcase version of readme
|
||||
file_path = "#{commit_sha}/README.md"
|
||||
|
||||
execute_raw_requests(requests: 1, project: project, file_path: file_path)
|
||||
|
||||
expect(response).to have_gitlab_http_status(200)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def execute_raw_requests(requests:, project:, file_path:)
|
||||
requests.times do
|
||||
get :show, params: {
|
||||
namespace_id: project.namespace,
|
||||
project_id: project,
|
||||
id: file_path
|
||||
}
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -1,11 +1,9 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe Gitlab::ActionRateLimiter do
|
||||
describe Gitlab::ActionRateLimiter, :clean_gitlab_redis_cache do
|
||||
let(:redis) { double('redis') }
|
||||
let(:user) { create(:user) }
|
||||
let(:project) { create(:project) }
|
||||
let(:key) { [user, project] }
|
||||
let(:cache_key) { "action_rate_limiter:test_action:user:#{user.id}:project:#{project.id}" }
|
||||
|
||||
subject { described_class.new(action: :test_action, expiry_time: 100) }
|
||||
|
||||
|
@ -13,17 +11,98 @@ describe Gitlab::ActionRateLimiter do
|
|||
allow(Gitlab::Redis::Cache).to receive(:with).and_yield(redis)
|
||||
end
|
||||
|
||||
it 'increases the throttle count and sets the expire time' do
|
||||
expect(redis).to receive(:incr).with(cache_key).and_return(1)
|
||||
expect(redis).to receive(:expire).with(cache_key, 100)
|
||||
shared_examples 'action rate limiter' do
|
||||
it 'increases the throttle count and sets the expiration time' do
|
||||
expect(redis).to receive(:incr).with(cache_key).and_return(1)
|
||||
expect(redis).to receive(:expire).with(cache_key, 100)
|
||||
|
||||
expect(subject.throttled?(key, 1)).to be false
|
||||
expect(subject.throttled?(key, 1)).to be_falsy
|
||||
end
|
||||
|
||||
it 'returns true if the key is throttled' do
|
||||
expect(redis).to receive(:incr).with(cache_key).and_return(2)
|
||||
expect(redis).not_to receive(:expire)
|
||||
|
||||
expect(subject.throttled?(key, 1)).to be_truthy
|
||||
end
|
||||
|
||||
context 'when throttling is disabled' do
|
||||
it 'returns false and does not set expiration time' do
|
||||
expect(redis).not_to receive(:incr)
|
||||
expect(redis).not_to receive(:expire)
|
||||
|
||||
expect(subject.throttled?(key, 0)).to be_falsy
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
it 'returns true if the key is throttled' do
|
||||
expect(redis).to receive(:incr).with(cache_key).and_return(2)
|
||||
expect(redis).not_to receive(:expire)
|
||||
context 'when the key is an array of only ActiveRecord models' do
|
||||
let(:key) { [user, project] }
|
||||
|
||||
expect(subject.throttled?(key, 1)).to be true
|
||||
let(:cache_key) do
|
||||
"action_rate_limiter:test_action:user:#{user.id}:project:#{project.id}"
|
||||
end
|
||||
|
||||
it_behaves_like 'action rate limiter'
|
||||
end
|
||||
|
||||
context 'when they key a combination of ActiveRecord models and strings' do
|
||||
let(:project) { create(:project, :public, :repository) }
|
||||
let(:commit) { project.repository.commit }
|
||||
let(:path) { 'app/controllers/groups_controller.rb' }
|
||||
let(:key) { [project, commit, path] }
|
||||
|
||||
let(:cache_key) do
|
||||
"action_rate_limiter:test_action:project:#{project.id}:commit:#{commit.sha}:#{path}"
|
||||
end
|
||||
|
||||
it_behaves_like 'action rate limiter'
|
||||
end
|
||||
|
||||
describe '#log_request' do
|
||||
let(:file_path) { 'master/README.md' }
|
||||
let(:type) { :raw_blob_request_limit }
|
||||
let(:fullpath) { "/#{project.full_path}/raw/#{file_path}" }
|
||||
|
||||
let(:request) do
|
||||
double('request', ip: '127.0.0.1', request_method: 'GET', fullpath: fullpath)
|
||||
end
|
||||
|
||||
let(:base_attributes) do
|
||||
{
|
||||
message: 'Action_Rate_Limiter_Request',
|
||||
env: type,
|
||||
ip: '127.0.0.1',
|
||||
request_method: 'GET',
|
||||
fullpath: fullpath
|
||||
}
|
||||
end
|
||||
|
||||
context 'without a current user' do
|
||||
let(:current_user) { nil }
|
||||
|
||||
it 'logs information to auth.log' do
|
||||
expect(Gitlab::AuthLogger).to receive(:error).with(base_attributes).once
|
||||
|
||||
subject.log_request(request, type, current_user)
|
||||
end
|
||||
end
|
||||
|
||||
context 'with a current_user' do
|
||||
let(:current_user) { create(:user) }
|
||||
|
||||
let(:attributes) do
|
||||
base_attributes.merge({
|
||||
user_id: current_user.id,
|
||||
username: current_user.username
|
||||
})
|
||||
end
|
||||
|
||||
it 'logs information to auth.log' do
|
||||
expect(Gitlab::AuthLogger).to receive(:error).with(attributes).once
|
||||
|
||||
subject.log_request(request, type, current_user)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -180,4 +180,20 @@ describe ApplicationSettings::UpdateService do
|
|||
described_class.new(application_settings, admin, { home_page_url: 'http://foo.bar' }).execute
|
||||
end
|
||||
end
|
||||
|
||||
context 'when raw_blob_request_limit is passsed' do
|
||||
let(:params) do
|
||||
{
|
||||
raw_blob_request_limit: 600
|
||||
}
|
||||
end
|
||||
|
||||
it 'updates raw_blob_request_limit value' do
|
||||
subject.execute
|
||||
|
||||
application_settings.reload
|
||||
|
||||
expect(application_settings.raw_blob_request_limit).to eq(600)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue