Коммит ed622fcc создал по автору Mayra Cabrera's avatar Mayra Cabrera
Просмотр файлов

Moves protected path to gitlab-rails 

- Adds 4 columns to application_settings
  - 3 to mimic the configuration of existing throttles
  - 1 to store the protected paths on database
- Set default protected paths (taken from Omnibus)
- Add new section on admin panel to personalize protected paths
configuration
- Includes additional protected paths throttles.
- Throttles were renamed to 'rack_attack_gitlab_rails'.rb, otherwise the
omnibus file will overwrite this file.
- If the settings are enabled, they will take precedence over the
Omnibus settings
владелец 0844ec02
Скрыть пробелы
Построчно Рядом
app/helpers/application_settings_helper.rb
Просмотр файла @ ed622fcc
......@@ -265,6 +265,10 @@ def visible_attributes
:throttle_unauthenticated_enabled,
:throttle_unauthenticated_period_in_seconds,
:throttle_unauthenticated_requests_per_period,
:throttle_protected_paths_enabled,
:throttle_protected_paths_period_in_seconds,
:throttle_protected_paths_requests_per_period,
:protected_paths_raw,
:time_tracking_limit_to_hours,
:two_factor_grace_period,
:unique_ips_limit_enabled,
......
app/models/application_setting.rb
Просмотр файла @ ed622fcc
......@@ -210,6 +210,11 @@ class ApplicationSetting < ApplicationRecord
presence: true,
if: :static_objects_external_storage_url?
validates :protected_paths,
length: { maximum: 100, message: N_('is too long (maximum is 100 entries)') },
allow_nil: false,
if: :protected_paths_exists?
SUPPORTED_KEY_TYPES.each do |type|
validates :"#{type}_key_restriction", presence: true, key_restriction: { type: type }
end
......@@ -314,4 +319,16 @@ def self.cache_backend
def recaptcha_or_login_protection_enabled
recaptcha_enabled || login_recaptcha_protection_enabled
end
private
# On GitLab 12.3, a migration that checks validations of this
# model was added. Because 'protected_paths' column is not present
# at that point, the validation of protected path fails.
# We added this conditional to only execute validation of
# 'protected_paths' if the column is present.
# https://gitlab.com/gitlab-org/gitlab-ce/issues/66685
def protected_paths_exists?
::Gitlab::Database.cached_column_exists?(self.class.table_name, 'protected_paths')
end
end
app/models/application_setting_implementation.rb
Просмотр файла @ ed622fcc
......@@ -4,7 +4,7 @@ module ApplicationSettingImplementation
extend ActiveSupport::Concern
include Gitlab::Utils::StrongMemoize
DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace
STRING_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace
| # or
\s # any whitespace character
| # or
......@@ -16,6 +16,19 @@ module ApplicationSettingImplementation
FORBIDDEN_KEY_VALUE = KeyRestrictionValidator::FORBIDDEN
SUPPORTED_KEY_TYPES = %i[rsa dsa ecdsa ed25519].freeze
DEFAULT_PROTECTED_PATHS = [
'/users/password',
'/users/sign_in',
'/api/v3/session.json',
'/api/v3/session',
'/api/v4/session.json',
'/api/v4/session',
'/users',
'/users/confirmation',
'/unsubscribes/',
'/import/github/personal_access_token'
].freeze
class_methods do
def defaults
{
......@@ -92,6 +105,10 @@ def defaults
throttle_unauthenticated_enabled: false,
throttle_unauthenticated_period_in_seconds: 3600,
throttle_unauthenticated_requests_per_period: 3600,
throttle_protected_paths_enabled: false,
throttle_protected_paths_in_seconds: 10,
throttle_protected_paths_per_period: 60,
protected_paths: DEFAULT_PROTECTED_PATHS,
time_tracking_limit_to_hours: false,
two_factor_grace_period: 48,
unique_ips_limit_enabled: false,
......@@ -149,11 +166,11 @@ def domain_blacklist_raw
end
def domain_whitelist_raw=(values)
self.domain_whitelist = domain_strings_to_array(values)
self.domain_whitelist = strings_to_array(values)
end
def domain_blacklist_raw=(values)
self.domain_blacklist = domain_strings_to_array(values)
self.domain_blacklist = strings_to_array(values)
end
def domain_blacklist_file=(file)
......@@ -167,7 +184,7 @@ def outbound_local_requests_whitelist_raw
def outbound_local_requests_whitelist_raw=(values)
clear_memoization(:outbound_local_requests_whitelist_arrays)
self.outbound_local_requests_whitelist = domain_strings_to_array(values)
self.outbound_local_requests_whitelist = strings_to_array(values)
end
def add_to_outbound_local_requests_whitelist(values_array)
......@@ -200,8 +217,16 @@ def outbound_local_requests_whitelist_arrays
end
end
def protected_paths_raw
array_to_string(self.protected_paths)
end
def protected_paths_raw=(values)
self.protected_paths = strings_to_array(values)
end
def asset_proxy_whitelist=(values)
values = domain_strings_to_array(values) if values.is_a?(String)
values = strings_to_array(values) if values.is_a?(String)
# make sure we always whitelist the running host
values << Gitlab.config.gitlab.host unless values.include?(Gitlab.config.gitlab.host)
......@@ -316,11 +341,11 @@ def array_to_string(arr)
arr&.join("\n")
end
def domain_strings_to_array(values)
def strings_to_array(values)
return [] unless values
values
.split(DOMAIN_LIST_SEPARATOR)
.split(STRING_LIST_SEPARATOR)
.map(&:strip)
.reject(&:empty?)
.uniq
......
app/views/admin/application_settings/_protected_paths.html.haml 0 → 100644
Просмотр файла @ ed622fcc
= form_for @application_setting, url: network_admin_application_settings_path(anchor: 'js-protected-paths-settings'), html: { class: 'fieldset-form' } do |f|
= form_errors(@application_setting)
%fieldset
.form-group
.form-check
= f.check_box :throttle_protected_paths_enabled, class: 'form-check-input'
= f.label :throttle_protected_paths_enabled, class: 'form-check-label' do
= _('Enable protected paths rate limit')
%span.form-text.text-muted
= _('Helps reduce request volume for protected paths')
.form-group
= f.label :throttle_protected_paths_requests_per_period, 'Max requests per period per user', class: 'label-bold'
= f.number_field :throttle_protected_paths_requests_per_period, class: 'form-control'
.form-group
= f.label :throttle_protected_paths_period_in_seconds, 'Rate limit period in seconds', class: 'label-bold'
= f.number_field :throttle_protected_paths_period_in_seconds, class: 'form-control'
.form-group
= f.label :protected_paths, class: 'label-bold' do
- relative_url_link = 'https://docs.gitlab.com/omnibus/settings/configuration.html#configuring-a-relative-url-for-gitlab'
- relative_url_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: relative_url_link }
= _('All paths are relative to the GitLab URL. Do not include %{relative_url_link_start}relative URL%{relative_url_link_end}.').html_safe % { relative_url_link_start: relative_url_link_start, relative_url_link_end: '</a>'.html_safe }
= f.text_area :protected_paths_raw, placeholder: '/users/sign_in,/users/password', class: 'form-control', rows: 10
= f.submit 'Save changes', class: 'btn btn-success'
app/views/admin/application_settings/network.html.haml
Просмотр файла @ ed622fcc
......@@ -34,3 +34,14 @@
= _('Allow requests to the local network from hooks and services.')
.settings-content
= render 'outbound'
%section.settings.as-protected-paths.no-animate#js-protected-paths-settings{ class: ('expanded' if expanded_by_default?) }
.settings-header
%h4
= _('Protected Paths')
%button.btn.btn-default.js-settings-toggle{ type: 'button' }
= expanded_by_default? ? _('Collapse') : _('Expand')
%p
= _('Configure paths to be protected by Rack Attack')
.settings-content
= render 'protected_paths'
changelogs/unreleased/mc-moves-protected-path-throttle-to-gitlab-rails.yml 0 → 100644
Просмотр файла @ ed622fcc
---
title: Allow users to configure protected paths from Admin panel
merge_request: 31246
author:
type: added
config/initializers/rack_attack.rb.example удалено 100644 → 0
Просмотр файла @ 0844ec02
# 1. Rename this file to rack_attack.rb
# 2. Review the paths_to_be_protected and add any other path you need protecting
#
# If you change this file in a Merge Request, please also create a Merge Request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests
paths_to_be_protected = [
"#{Rails.application.config.relative_url_root}/users/password",
"#{Rails.application.config.relative_url_root}/users/sign_in",
"#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session.json",
"#{Rails.application.config.relative_url_root}/api/#{API::API.version}/session",
"#{Rails.application.config.relative_url_root}/users",
"#{Rails.application.config.relative_url_root}/users/confirmation",
"#{Rails.application.config.relative_url_root}/unsubscribes/",
"#{Rails.application.config.relative_url_root}/import/github/personal_access_token"
]
# Create one big regular expression that matches strings starting with any of
# the paths_to_be_protected.
paths_regex = Regexp.union(paths_to_be_protected.map { |path| /\A#{Regexp.escape(path)}/ })
rack_attack_enabled = Gitlab.config.rack_attack.git_basic_auth['enabled']
unless Rails.env.test? || !rack_attack_enabled
Rack::Attack.throttle('protected paths', limit: 10, period: 60.seconds) do |req|
if req.post? && req.path =~ paths_regex
req.ip
end
end
end
config/initializers/rack_attack_global.rb → config/initializers/rack_attack_gitlab_rails.rb
Просмотр файла @ ed622fcc
......@@ -20,6 +20,13 @@ def self.authenticated_web_options
period_proc = proc { |req| settings.throttle_authenticated_web_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc }
end
def self.protected_paths_options
limit_proc = proc { |req| settings.throttle_protected_paths_requests_per_period }
period_proc = proc { |req| settings.throttle_protected_paths_period_in_seconds.seconds }
{ limit: limit_proc, period: period_proc }
end
end
class Rack::Attack
......@@ -42,6 +49,28 @@ class Rack::Attack
req.authenticated_user_id([:api, :rss, :ics])
end
throttle('throttle_unauthenticated_protected_paths', Gitlab::Throttle.protected_paths_options) do |req|
Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
req.unauthenticated? &&
!req.should_be_skipped? &&
req.protected_path? &&
req.ip
end
throttle('throttle_authenticated_protected_paths_api', Gitlab::Throttle.protected_paths_options) do |req|
Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
req.api_request? &&
req.protected_path? &&
req.authenticated_user_id([:api])
end
throttle('throttle_authenticated_protected_paths_web', Gitlab::Throttle.protected_paths_options) do |req|
Gitlab::Throttle.settings.throttle_protected_paths_enabled &&
req.web_request? &&
req.protected_path? &&
req.authenticated_user_id([:api, :rss, :ics])
end
class Request
def unauthenticated?
!authenticated_user_id([:api, :rss, :ics])
......@@ -66,5 +95,23 @@ def should_be_skipped?
def web_request?
!api_request?
end
def protected_path?
!protected_path_regex.nil?
end
def protected_path_regex
path =~ protected_paths_regex
end
private
def protected_paths
Gitlab::CurrentSettings.current_application_settings.protected_paths
end
def protected_paths_regex
Regexp.union(protected_paths.map { |path| /\A#{Regexp.escape(path)}/ })
end
end
end
config/initializers/rack_attack_logging.rb
Просмотр файла @ ed622fcc
......@@ -12,10 +12,18 @@
path: req.fullpath
}
if %w(throttle_authenticated_api throttle_authenticated_web).include? req.env['rack.attack.matched']
throttles_with_user_information = [
:throttle_authenticated_api,
:throttle_authenticated_web,
:throttle_authenticated_protected_paths_api,
:throttle_authenticated_protected_paths_web
]
if throttles_with_user_information.include? req.env['rack.attack.matched'].to_sym
user_id = req.env['rack.attack.match_discriminator']
user = User.find_by(id: user_id)
rack_attack_info[:throttle_type] = req.env['rack.attack.matched']
rack_attack_info[:user_id] = user_id
rack_attack_info[:username] = user.username unless user.nil?
end
......
db/migrate/20190801142441_add_throttle_protected_path_columns.rb 0 → 100644
Просмотр файла @ ed622fcc
# frozen_string_literal: true
class AddThrottleProtectedPathColumns < ActiveRecord::Migration[5.2]
DOWNTIME = false
DEFAULT_PROTECTED_PATHS = [
'/users/password',
'/users/sign_in',
'/api/v3/session.json',
'/api/v3/session',
'/api/v4/session.json',
'/api/v4/session',
'/users',
'/users/confirmation',
'/unsubscribes/',
'/import/github/personal_access_token'
]
def change
add_column :application_settings, :throttle_protected_paths_enabled, :boolean, default: true, null: false
add_column :application_settings, :throttle_protected_paths_requests_per_period, :integer, default: 10, null: false
add_column :application_settings, :throttle_protected_paths_period_in_seconds, :integer, default: 60, null: false
add_column :application_settings, :protected_paths, :string, array: true, limit: 255, default: DEFAULT_PROTECTED_PATHS
end
end
db/schema.rb
Просмотр файла @ ed622fcc
......@@ -286,6 +286,10 @@
t.string "encrypted_asset_proxy_secret_key_iv"
t.string "static_objects_external_storage_url", limit: 255
t.string "static_objects_external_storage_auth_token", limit: 255
t.boolean "throttle_protected_paths_enabled", default: true, null: false
t.integer "throttle_protected_paths_requests_per_period", default: 10, null: false
t.integer "throttle_protected_paths_period_in_seconds", default: 60, null: false
t.string "protected_paths", limit: 255, default: ["/users/password", "/users/sign_in", "/api/v3/session.json", "/api/v3/session", "/api/v4/session.json", "/api/v4/session", "/users", "/users/confirmation", "/unsubscribes/", "/import/github/personal_access_token"], array: true
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 ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
......
lib/gitlab/auth/ip_rate_limiter.rb
Просмотр файла @ ed622fcc
......@@ -24,6 +24,7 @@ def register_fail!
# Allow2Ban.filter will return false if this IP has not failed too often yet
@banned = Rack::Attack::Allow2Ban.filter(ip, config) do
# If we return false here, the failure for this IP is ignored by Allow2Ban
# If we return true here, the count for the IP is incremented.
ip_can_be_banned?
end
end
......
locale/gitlab.pot
Просмотр файла @ ed622fcc
......@@ -984,6 +984,9 @@ msgstr ""
msgid "All merge conflicts were resolved. The merge request can now be merged."
msgstr ""
msgid "All paths are relative to the GitLab URL. Do not include %{relative_url_link_start}relative URL%{relative_url_link_end}."
msgstr ""
msgid "All projects"
msgstr ""
......@@ -3204,6 +3207,9 @@ msgstr ""
msgid "Configure limits for web and API requests."
msgstr ""
msgid "Configure paths to be protected by Rack Attack"
msgstr ""
msgid "Configure push mirrors."
msgstr ""
......@@ -4399,6 +4405,9 @@ msgstr ""
msgid "Enable or disable version check and usage ping."
msgstr ""
msgid "Enable protected paths rate limit"
msgstr ""
msgid "Enable reCAPTCHA or Akismet and set IP limits. For reCAPTCHA, we currently only support %{recaptcha_v2_link_start}v2%{recaptcha_v2_link_end}"
msgstr ""
......@@ -5793,6 +5802,9 @@ msgstr ""
msgid "Helps prevent bots from creating accounts."
msgstr ""
msgid "Helps reduce request volume for protected paths"
msgstr ""
msgid "Hide archived projects"
msgstr ""
......@@ -9355,6 +9367,9 @@ msgstr ""
msgid "Protected Branch"
msgstr ""
msgid "Protected Paths"
msgstr ""
msgid "Protected Tag"
msgstr ""
......@@ -13910,6 +13925,9 @@ msgstr ""
msgid "is not an email you own"
msgstr ""
msgid "is too long (maximum is 100 entries)"
msgstr ""
msgid "is too long (maximum is 1000 entries)"
msgstr ""
......
spec/models/application_setting_spec.rb
Просмотр файла @ ed622fcc
......@@ -51,6 +51,10 @@
it { is_expected.to allow_value(nil).for(:static_objects_external_storage_url) }
it { is_expected.to allow_value(http).for(:static_objects_external_storage_url) }
it { is_expected.to allow_value(https).for(:static_objects_external_storage_url) }
it { is_expected.to allow_value(['/example'] * 100).for(:protected_paths) }
it { is_expected.not_to allow_value(['/example'] * 101).for(:protected_paths) }
it { is_expected.not_to allow_value(nil).for(:protected_paths) }
it { is_expected.to allow_value([]).for(:protected_paths) }
context "when user accepted let's encrypt terms of service" do
before do
......
spec/requests/rack_attack_global_spec.rb
Просмотр файла @ ed622fcc
......@@ -12,7 +12,9 @@
throttle_authenticated_api_requests_per_period: 100,
throttle_authenticated_api_period_in_seconds: 1,
throttle_authenticated_web_requests_per_period: 100,
throttle_authenticated_web_period_in_seconds: 1
throttle_authenticated_web_period_in_seconds: 1,
throttle_authenticated_protected_paths_request_per_period: 100,
throttle_authenticated_protected_paths_in_seconds: 1
}
end
......@@ -35,6 +37,10 @@
let(:url_api_internal) { '/api/v4/internal/check' }
before do
# Disabling protected paths throttle, otherwise requests to
# '/users/sign_in' are caught by this throttle.
settings_to_set[:throttle_protected_paths_enabled] = false
# Set low limits
settings_to_set[:throttle_unauthenticated_requests_per_period] = requests_per_period
settings_to_set[:throttle_unauthenticated_period_in_seconds] = period_in_seconds
......@@ -203,6 +209,102 @@
it_behaves_like 'rate-limited web authenticated requests'
end
describe 'protected paths' do
context 'unauthenticated requests' do
let(:protected_path_that_does_not_require_authentication) do
'/users/confirmation'
end
before do
settings_to_set[:throttle_protected_paths_requests_per_period] = requests_per_period # 1
settings_to_set[:throttle_protected_paths_period_in_seconds] = period_in_seconds # 10_000
end
context 'when protected paths throttle is disabled' do
before do
settings_to_set[:throttle_protected_paths_enabled] = false
stub_application_setting(settings_to_set)
end
it 'allows requests over the rate limit' do
(1 + requests_per_period).times do
get protected_path_that_does_not_require_authentication
expect(response).to have_http_status 200
end
end
end
context 'when protected paths throttle is enabled' do
before do
settings_to_set[:throttle_protected_paths_enabled] = true
stub_application_setting(settings_to_set)
end
it 'rejects requests over the rate limit' do
requests_per_period.times do
get protected_path_that_does_not_require_authentication
expect(response).to have_http_status 200
end
expect_rejection { get protected_path_that_does_not_require_authentication }
end
end
end
context 'API requests authenticated with personal access token', :api do
let(:user) { create(:user) }
let(:token) { create(:personal_access_token, user: user) }
let(:other_user) { create(:user) }
let(:other_user_token) { create(:personal_access_token, user: other_user) }
let(:throttle_setting_prefix) { 'throttle_protected_paths' }
let(:api_partial_url) { '/users' }
let(:protected_paths) do
[
'/api/v4/users'
]
end
before do
settings_to_set[:protected_paths] = protected_paths
stub_application_setting(settings_to_set)
end
context 'with the token in the query string' do
let(:get_args) { [api(api_partial_url, personal_access_token: token)] }
let(:other_user_get_args) { [api(api_partial_url, personal_access_token: other_user_token)] }
it_behaves_like 'rate-limited token-authenticated requests'
end
context 'with the token in the headers' do
let(:get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(token)) }
let(:other_user_get_args) { api_get_args_with_token_headers(api_partial_url, personal_access_token_headers(other_user_token)) }
it_behaves_like 'rate-limited token-authenticated requests'
end
end
describe 'web requests authenticated with regular login' do
let(:throttle_setting_prefix) { 'throttle_protected_paths' }
let(:user) { create(:user) }
let(:url_that_requires_authentication) { '/dashboard/snippets' }
let(:protected_paths) do
[
url_that_requires_authentication
]
end
before do
settings_to_set[:protected_paths] = protected_paths
stub_application_setting(settings_to_set)
end
it_behaves_like 'rate-limited web authenticated requests'
end
end
def api_get_args_with_token_headers(partial_url, token_headers)
["/api/#{API::API.version}#{partial_url}", params: nil, headers: token_headers]
end
......
spec/services/application_settings/update_service_spec.rb
Просмотр файла @ ed622fcc
......@@ -295,4 +295,26 @@
expect(application_settings.raw_blob_request_limit).to eq(600)
end
end
context 'when protected path settings are passed' do
let(:params) do
{
throttle_protected_paths_enabled: 1,
throttle_protected_paths_period_in_seconds: 600,
throttle_protected_paths_requests_per_period: 100,
protected_paths_raw: "/users/password\r\n/users/sign_in\r\n"
}
end
it 'updates protected path settings' do
subject.execute
application_settings.reload
expect(application_settings.throttle_protected_paths_enabled).to be_truthy
expect(application_settings.throttle_protected_paths_period_in_seconds).to eq(600)
expect(application_settings.throttle_protected_paths_requests_per_period).to eq(100)
expect(application_settings.protected_paths).to eq(['/users/password', '/users/sign_in'])
end
end
end
spec/support/shared_examples/requests/rack_attack_shared_examples.rb
Просмотр файла @ ed622fcc
......@@ -8,6 +8,14 @@
# * period_in_seconds
# * period
shared_examples_for 'rate-limited token-authenticated requests' do
let(:throttle_types) do
{
"throttle_protected_paths" => "throttle_authenticated_protected_paths_api",
"throttle_authenticated_api" => "throttle_authenticated_api",
"throttle_authenticated_web" => "throttle_authenticated_web"
}
end
before do
# Set low limits
settings_to_set[:"#{throttle_setting_prefix}_requests_per_period"] = requests_per_period
......@@ -84,7 +92,8 @@
request_method: 'GET',
path: get_args.first,
user_id: user.id,
username: user.username
username: user.username,
throttle_type: throttle_types[throttle_setting_prefix]
}
expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once
......@@ -116,6 +125,13 @@
# * period_in_seconds
# * period
shared_examples_for 'rate-limited web authenticated requests' do
let(:throttle_types) do
{
"throttle_protected_paths" => "throttle_authenticated_protected_paths_web",
"throttle_authenticated_web" => "throttle_authenticated_web"
}
end
before do
login_as(user)
......@@ -196,7 +212,8 @@
request_method: 'GET',
path: '/dashboard/snippets',
user_id: user.id,
username: user.username
username: user.username,
throttle_type: throttle_types[throttle_setting_prefix]
}
expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once
......
Поддерживает Markdown
0% или .
You are about to add 0 people to the discussion. Proceed with caution.
Сначала завершите редактирование этого сообщения!
Пожалуйста, зарегистрируйтесь или чтобы прокомментировать