Commit d88dd8b3 authored by George Koltsov's avatar George Koltsov
Browse files

Add outbound requests setting for system hooks

This MR adds new application setting to network section
`allow_local_requests_from_system_hooks`. Prior to this change
system hooks were allowed to do local network requests by default
and we are adding an ability for admins to control it.
parent 1513974538d9
......@@ -159,7 +159,8 @@ def visible_attributes
:after_sign_up_text,
:akismet_api_key,
:akismet_enabled,
:allow_local_requests_from_hooks_and_services,
:allow_local_requests_from_web_hooks_and_services,
:allow_local_requests_from_system_hooks,
:dns_rebinding_protection_enabled,
:archive_builds_in_human_readable,
:authorized_keys_enabled,
......
......@@ -21,7 +21,8 @@ def defaults
{
after_sign_up_text: nil,
akismet_enabled: false,
allow_local_requests_from_hooks_and_services: false,
allow_local_requests_from_web_hooks_and_services: false,
allow_local_requests_from_system_hooks: true,
dns_rebinding_protection_enabled: true,
authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
container_registry_token_expire_delay: 5,
......
......@@ -14,8 +14,11 @@ class SystemHook < WebHook
default_value_for :repository_update_events, true
default_value_for :merge_requests_events, false
validates :url, presence: true, public_url: false, system_hook_url: { allow_localhost: lambda(&:allow_local_requests?),
allow_local_network: lambda(&:allow_local_requests?) }
# Allow urls pointing localhost and the local network
def allow_local_requests?
true
Gitlab::CurrentSettings.allow_local_requests_from_system_hooks?
end
end
......@@ -35,6 +35,6 @@ def async_execute(data, hook_name)
# Allow urls pointing localhost and the local network
def allow_local_requests?
false
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end
......@@ -17,8 +17,10 @@ def initialize(hook, data, hook_name)
@hook = hook
@data = data
@hook_name = hook_name.to_s
@request_options = { timeout: Gitlab.config.gitlab.webhook_timeout }
@request_options.merge!(allow_local_requests: true) if @hook.is_a?(SystemHook)
@request_options = {
timeout: Gitlab.config.gitlab.webhook_timeout,
allow_local_requests: hook.allow_local_requests?
}
end
def execute
......
......@@ -107,6 +107,6 @@ def self.allow_setting_local_requests?
# calls this validator.
#
# See https://gitlab.com/gitlab-org/gitlab-ee/issues/9833
ApplicationSetting.current&.allow_local_requests_from_hooks_and_services?
ApplicationSetting.current&.allow_local_requests_from_web_hooks_and_services?
end
end
# frozen_string_literal: true
# SystemHookUrlValidator
#
# Custom validator specifically for SystemHook URLs. This validator works like AddressableUrlValidator but
# it blocks urls pointing to localhost or the local network depending on
# ApplicationSetting.allow_local_requests_from_system_hooks
#
# Example:
#
# class SystemHook < WebHook
# validates :url, system_hook_url: { allow_localhost: true, allow_local_network: true }
# end
#
class SystemHookUrlValidator < AddressableUrlValidator
DEFAULT_OPTIONS = {
allow_localhost: true,
allow_local_network: true
}.freeze
def initialize(options)
options.reverse_merge!(DEFAULT_OPTIONS)
super(options)
end
def self.allow_setting_local_requests?
ApplicationSetting.current&.allow_local_requests_from_system_hooks?
end
end
......@@ -4,9 +4,13 @@
%fieldset
.form-group
.form-check
= f.check_box :allow_local_requests_from_hooks_and_services, class: 'form-check-input'
= f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do
Allow requests to the local network from hooks and services
= f.check_box :allow_local_requests_from_web_hooks_and_services, class: 'form-check-input'
= f.label :allow_local_requests_from_web_hooks_and_services, class: 'form-check-label' do
Allow requests to the local network from web hooks and services
.form-check
= f.check_box :allow_local_requests_from_system_hooks, class: 'form-check-input'
= f.label :allow_local_requests_from_system_hooks, class: 'form-check-label' do
Allow requests to the local network from system hooks
.form-group
= f.label :outbound_local_requests_whitelist_raw, class: 'label-bold' do
......
class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
rename_column :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
end
def down
rename_column :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
end
end
class AddAllowLocalRequestsFromSystemHooksToApplicationSettings < ActiveRecord::Migration[5.2]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:application_settings, :allow_local_requests_from_system_hooks,
:boolean,
default: true,
allow_null: false)
end
def down
remove_column(:application_settings, :allow_local_requests_from_system_hooks)
end
end
......@@ -183,7 +183,7 @@
t.string "external_authorization_service_default_label"
t.boolean "pages_domain_verification_enabled", default: true, null: false
t.string "user_default_internal_regex"
t.boolean "allow_local_requests_from_hooks_and_services", default: false, null: false
t.boolean "allow_local_requests_from_web_hooks_and_services", default: false, null: false
t.float "external_authorization_service_timeout", default: 0.5
t.text "external_auth_client_cert"
t.text "encrypted_external_auth_client_key"
......@@ -230,6 +230,7 @@
t.string "grafana_url", default: "/-/grafana", null: false
t.string "outbound_local_requests_whitelist", limit: 255, default: [], null: false, array: true
t.integer "raw_blob_request_limit", default: 300, null: false
t.boolean "allow_local_requests_from_system_hooks", default: true, 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"
......
......@@ -177,7 +177,8 @@ are listed in the descriptions of the relevant settings.
| `akismet_api_key` | string | required by: `akismet_enabled` | API key for akismet spam protection. |
| `akismet_enabled` | boolean | no | (**If enabled, requires:** `akismet_api_key`) Enable or disable akismet spam protection. |
| `allow_group_owners_to_manage_ldap` | boolean | no | **(PREMIUM)** Set to `true` to allow group owners to manage LDAP |
| `allow_local_requests_from_hooks_and_services` | boolean | no | Allow requests to the local network from hooks and services. |
| `allow_local_requests_from_web_hooks_and_services` | boolean | no | Allow requests to the local network from web hooks and services. |
| `allow_local_requests_from_system_hooks` | boolean | no | Allow requests to the local network from system hooks. |
| `authorized_keys_enabled` | boolean | no | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. |
| `auto_devops_domain` | string | no | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. |
| `auto_devops_enabled` | boolean | no | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. |
......
# frozen_string_literal: true
# This class is part of the Gitlab::HTTP wrapper. Depending on the value
# of the global setting allow_local_requests_from_hooks_and_services this adapter
# of the global setting allow_local_requests_from_web_hooks_and_services this adapter
# will allow/block connection to internal IPs and/or urls.
#
# This functionality can be overridden by providing the setting the option
......@@ -38,7 +38,7 @@ def dns_rebind_protection?
end
def allow_settings_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
end
end
......@@ -128,7 +128,7 @@ def create_or_update_secret(resource)
private
def validate_url!
return if Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
return if Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
Gitlab::UrlBlocker.validate!(api_prefix, allow_local_network: false)
end
......
......@@ -945,7 +945,10 @@ msgstr ""
msgid "Allow rendering of PlantUML diagrams in Asciidoc documents."
msgstr ""
msgid "Allow requests to the local network from hooks and services."
msgid "Allow requests to the local network from web hooks and services."
msgstr ""
msgid "Allow requests to the local network from system hooks."
msgstr ""
msgid "Allow this key to push to repository as well? (Default only allows pull access.)"
......
......@@ -338,14 +338,17 @@
visit network_admin_application_settings_path
page.within('.as-outbound') do
check 'Allow requests to the local network from hooks and services'
check 'Allow requests to the local network from web hooks and services'
# Enabled by default
uncheck 'Allow requests to the local network from system hooks'
# Enabled by default
uncheck 'Enforce DNS rebinding attack protection'
click_button 'Save changes'
end
expect(page).to have_content "Application settings saved successfully"
expect(current_settings.allow_local_requests_from_hooks_and_services).to be true
expect(current_settings.allow_local_requests_from_web_hooks_and_services).to be true
expect(current_settings.allow_local_requests_from_system_hooks).to be false
expect(current_settings.dns_rebinding_protection_enabled).to be false
end
end
......
......@@ -23,14 +23,14 @@
end
end
describe 'allow_local_requests_from_hooks_and_services is' do
describe 'allow_local_requests_from_web_hooks_and_services is' do
before do
WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')
end
context 'disabled' do
before do
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(false)
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(false)
end
it 'deny requests to localhost' do
......@@ -52,7 +52,7 @@
context 'enabled' do
before do
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_hooks_and_services?).and_return(true)
allow(Gitlab::CurrentSettings).to receive(:allow_local_requests_from_web_hooks_and_services?).and_return(true)
end
it 'allow requests to localhost' do
......
......@@ -58,7 +58,7 @@
context 'when local requests are allowed' do
before do
stub_application_setting(allow_local_requests_from_hooks_and_services: true)
stub_application_setting(allow_local_requests_from_web_hooks_and_services: true)
end
it 'allows local addresses' do
......
......@@ -106,7 +106,7 @@
before do
allow(ApplicationSetting)
.to receive(:current)
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true))
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true))
end
it { expect(kubernetes.save).to be_truthy }
......
......@@ -50,7 +50,7 @@
before do
allow(ApplicationSetting)
.to receive(:current)
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: setting))
.and_return(ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: setting))
end
context 'are allowed' do
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment