Commit b599fb4f authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Add DNS rebinding protection settings

parent 5dd3b753
...@@ -160,6 +160,7 @@ module ApplicationSettingsHelper ...@@ -160,6 +160,7 @@ module ApplicationSettingsHelper
:akismet_api_key, :akismet_api_key,
:akismet_enabled, :akismet_enabled,
:allow_local_requests_from_hooks_and_services, :allow_local_requests_from_hooks_and_services,
:dns_rebinding_protection_enabled,
:archive_builds_in_human_readable, :archive_builds_in_human_readable,
:authorized_keys_enabled, :authorized_keys_enabled,
:auto_devops_enabled, :auto_devops_enabled,
......
...@@ -21,6 +21,7 @@ module ApplicationSettingImplementation ...@@ -21,6 +21,7 @@ module ApplicationSettingImplementation
after_sign_up_text: nil, after_sign_up_text: nil,
akismet_enabled: false, akismet_enabled: false,
allow_local_requests_from_hooks_and_services: false, allow_local_requests_from_hooks_and_services: false,
dns_rebinding_protection_enabled: true,
authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
container_registry_token_expire_delay: 5, container_registry_token_expire_delay: 5,
default_artifacts_expire_in: '30 days', default_artifacts_expire_in: '30 days',
......
...@@ -8,4 +8,12 @@ ...@@ -8,4 +8,12 @@
= f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do = f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do
Allow requests to the local network from hooks and services Allow requests to the local network from hooks and services
.form-group
.form-check
= f.check_box :dns_rebinding_protection_enabled, class: 'form-check-input'
= f.label :dns_rebinding_protection_enabled, class: 'form-check-label' do
= _('Enforce DNS rebinding attack protection')
%span.form-text.text-muted
= _('Resolves IP addresses once and uses them to submit requests')
= f.submit 'Save changes', class: "btn btn-success" = f.submit 'Save changes', class: "btn btn-success"
# frozen_string_literal: true
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddDnsRebindingProtectionEnabledToApplicationSettings < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:application_settings, :dns_rebinding_protection_enabled,
:boolean,
default: true,
allow_null: false)
end
def down
remove_column(:application_settings, :dns_rebinding_protection_enabled)
end
end
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20190506135400) do ActiveRecord::Schema.define(version: 20190529142545) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -189,6 +189,7 @@ ActiveRecord::Schema.define(version: 20190506135400) do ...@@ -189,6 +189,7 @@ ActiveRecord::Schema.define(version: 20190506135400) do
t.string "encrypted_external_auth_client_key_pass_iv" t.string "encrypted_external_auth_client_key_pass_iv"
t.string "lets_encrypt_notification_email" t.string "lets_encrypt_notification_email"
t.boolean "lets_encrypt_terms_of_service_accepted", default: false, null: false t.boolean "lets_encrypt_terms_of_service_accepted", default: false, null: false
t.boolean "dns_rebinding_protection_enabled", default: true, null: false
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id", using: :btree
end end
......
...@@ -40,6 +40,7 @@ module Gitlab ...@@ -40,6 +40,7 @@ module Gitlab
SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}.freeze SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}.freeze
VERSION = File.read(root.join("VERSION")).strip.freeze VERSION = File.read(root.join("VERSION")).strip.freeze
INSTALLATION_TYPE = File.read(root.join("INSTALLATION_TYPE")).strip.freeze INSTALLATION_TYPE = File.read(root.join("INSTALLATION_TYPE")).strip.freeze
HTTP_PROXY_ENV_VARS = %w(http_proxy https_proxy HTTP_PROXY HTTPS_PROXY).freeze
def self.com? def self.com?
# Check `gl_subdomain?` as well to keep parity with gitlab.com # Check `gl_subdomain?` as well to keep parity with gitlab.com
...@@ -66,6 +67,10 @@ module Gitlab ...@@ -66,6 +67,10 @@ module Gitlab
end end
end end
def self.http_proxy_env?
HTTP_PROXY_ENV_VARS.any? { |name| ENV[name] }
end
def self.process_name def self.process_name
return 'sidekiq' if Sidekiq.server? return 'sidekiq' if Sidekiq.server?
return 'console' if defined?(Rails::Console) return 'console' if defined?(Rails::Console)
......
...@@ -14,7 +14,8 @@ module Gitlab ...@@ -14,7 +14,8 @@ module Gitlab
def connection def connection
begin begin
@uri, hostname = Gitlab::UrlBlocker.validate!(uri, allow_local_network: allow_local_requests?, @uri, hostname = Gitlab::UrlBlocker.validate!(uri, allow_local_network: allow_local_requests?,
allow_localhost: allow_local_requests?) allow_localhost: allow_local_requests?,
dns_rebind_protection: dns_rebind_protection?)
rescue Gitlab::UrlBlocker::BlockedUrlError => e rescue Gitlab::UrlBlocker::BlockedUrlError => e
raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}" raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}"
end end
...@@ -30,6 +31,12 @@ module Gitlab ...@@ -30,6 +31,12 @@ module Gitlab
options.fetch(:allow_local_requests, allow_settings_local_requests?) options.fetch(:allow_local_requests, allow_settings_local_requests?)
end end
def dns_rebind_protection?
return false if Gitlab.http_proxy_env?
Gitlab::CurrentSettings.dns_rebinding_protection_enabled?
end
def allow_settings_local_requests? def allow_settings_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services? Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
end end
......
...@@ -18,7 +18,21 @@ module Gitlab ...@@ -18,7 +18,21 @@ module Gitlab
# enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true. # enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true.
# #
# Returns an array with [<uri>, <original-hostname>]. # Returns an array with [<uri>, <original-hostname>].
def validate!(url, ports: [], schemes: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false) # rubocop:disable Metrics/CyclomaticComplexity # rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/ParameterLists
def validate!(
url,
ports: [],
schemes: [],
allow_localhost: false,
allow_local_network: true,
ascii_only: false,
enforce_user: false,
enforce_sanitization: false,
dns_rebind_protection: true)
# rubocop:enable Metrics/CyclomaticComplexity
# rubocop:enable Metrics/ParameterLists
return [nil, nil] if url.nil? return [nil, nil] if url.nil?
# Param url can be a string, URI or Addressable::URI # Param url can be a string, URI or Addressable::URI
...@@ -45,15 +59,17 @@ module Gitlab ...@@ -45,15 +59,17 @@ module Gitlab
return [uri, nil] return [uri, nil]
end end
protected_uri_with_hostname = enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
# Allow url from the GitLab instance itself but only for the configured hostname and ports # Allow url from the GitLab instance itself but only for the configured hostname and ports
return enforce_uri_hostname(addrs_info, uri, hostname) if internal?(uri) return protected_uri_with_hostname if internal?(uri)
validate_localhost!(addrs_info) unless allow_localhost validate_localhost!(addrs_info) unless allow_localhost
validate_loopback!(addrs_info) unless allow_localhost validate_loopback!(addrs_info) unless allow_localhost
validate_local_network!(addrs_info) unless allow_local_network validate_local_network!(addrs_info) unless allow_local_network
validate_link_local!(addrs_info) unless allow_local_network validate_link_local!(addrs_info) unless allow_local_network
enforce_uri_hostname(addrs_info, uri, hostname) protected_uri_with_hostname
end end
def blocked_url?(*args) def blocked_url?(*args)
...@@ -74,17 +90,15 @@ module Gitlab ...@@ -74,17 +90,15 @@ module Gitlab
# #
# The original hostname is used to validate the SSL, given in that scenario # The original hostname is used to validate the SSL, given in that scenario
# we'll be making the request to the IP address, instead of using the hostname. # we'll be making the request to the IP address, instead of using the hostname.
def enforce_uri_hostname(addrs_info, uri, hostname) def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
address = addrs_info.first address = addrs_info.first
ip_address = address&.ip_address ip_address = address&.ip_address
if ip_address && ip_address != hostname return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
uri = uri.dup
uri.hostname = ip_address
return [uri, hostname]
end
[uri, nil] uri = uri.dup
uri.hostname = ip_address
[uri, hostname]
end end
def get_port(uri) def get_port(uri)
......
...@@ -3701,6 +3701,9 @@ msgstr "" ...@@ -3701,6 +3701,9 @@ msgstr ""
msgid "Ends at (UTC)" msgid "Ends at (UTC)"
msgstr "" msgstr ""
msgid "Enforce DNS rebinding attack protection"
msgstr ""
msgid "Enter at least three characters to search" msgid "Enter at least three characters to search"
msgstr "" msgstr ""
...@@ -8092,6 +8095,9 @@ msgstr "" ...@@ -8092,6 +8095,9 @@ msgstr ""
msgid "Resolved by %{resolvedByName}" msgid "Resolved by %{resolvedByName}"
msgstr "" msgstr ""
msgid "Resolves IP addresses once and uses them to submit requests"
msgstr ""
msgid "Response metrics (AWS ELB)" msgid "Response metrics (AWS ELB)"
msgstr "" msgstr ""
......
...@@ -332,16 +332,19 @@ describe 'Admin updates settings' do ...@@ -332,16 +332,19 @@ describe 'Admin updates settings' do
end end
context 'Network page' do context 'Network page' do
it 'Enable outbound requests' do it 'Changes Outbound requests settings' do
visit network_admin_application_settings_path visit network_admin_application_settings_path
page.within('.as-outbound') do page.within('.as-outbound') do
check 'Allow requests to the local network from hooks and services' check 'Allow requests to the local network from hooks and services'
# Enabled by default
uncheck 'Enforce DNS rebinding attack protection'
click_button 'Save changes' click_button 'Save changes'
end end
expect(page).to have_content "Application settings saved successfully" expect(page).to have_content "Application settings saved successfully"
expect(Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services).to be true expect(Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services).to be true
expect(Gitlab::CurrentSettings.dns_rebinding_protection_enabled).to be false
end end
end end
......
...@@ -47,6 +47,38 @@ describe Gitlab::HTTPConnectionAdapter do ...@@ -47,6 +47,38 @@ describe Gitlab::HTTPConnectionAdapter do
end end
end end
context 'when DNS rebinding protection is disabled' do
it 'sets up the connection' do
stub_application_setting(dns_rebinding_protection_enabled: false)
uri = URI('https://example.org')
connection = described_class.new(uri).connection
expect(connection).to be_a(Net::HTTP)
expect(connection.address).to eq('example.org')
expect(connection.hostname_override).to eq(nil)
expect(connection.addr_port).to eq('example.org')
expect(connection.port).to eq(443)
end
end
context 'when http(s) environment variable is set' do
it 'sets up the connection' do
stub_env('https_proxy' => 'https://my.proxy')
uri = URI('https://example.org')
connection = described_class.new(uri).connection
expect(connection).to be_a(Net::HTTP)
expect(connection.address).to eq('example.org')
expect(connection.hostname_override).to eq(nil)
expect(connection.addr_port).to eq('example.org')
expect(connection.port).to eq(443)
end
end
context 'when local requests are allowed' do context 'when local requests are allowed' do
it 'sets up the connection' do it 'sets up the connection' do
uri = URI('https://example.org') uri = URI('https://example.org')
......
...@@ -46,6 +46,41 @@ describe Gitlab::UrlBlocker do ...@@ -46,6 +46,41 @@ describe Gitlab::UrlBlocker do
expect(hostname).to be(nil) expect(hostname).to be(nil)
end end
end end
context 'disabled DNS rebinding protection' do
context 'when URI is internal' do
let(:import_url) { 'http://localhost' }
it 'returns URI and no hostname' do
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
expect(uri).to eq(Addressable::URI.parse('http://localhost'))
expect(hostname).to be(nil)
end
end
context 'when the URL hostname is a domain' do
let(:import_url) { 'https://example.org' }
it 'returns URI and no hostname' do
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
expect(uri).to eq(Addressable::URI.parse('https://example.org'))
expect(hostname).to eq(nil)
end
end
context 'when the URL hostname is an IP address' do
let(:import_url) { 'https://93.184.216.34' }
it 'returns URI and no hostname' do
uri, hostname = described_class.validate!(import_url, dns_rebind_protection: false)
expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
expect(hostname).to be(nil)
end
end
end
end end
describe '#blocked_url?' do describe '#blocked_url?' do
......
...@@ -109,4 +109,34 @@ describe Gitlab do ...@@ -109,4 +109,34 @@ describe Gitlab do
expect(described_class.ee?).to eq(false) expect(described_class.ee?).to eq(false)
end end
end end
describe '.http_proxy_env?' do
it 'returns true when lower case https' do
stub_env('https_proxy', 'https://my.proxy')
expect(described_class.http_proxy_env?).to eq(true)
end
it 'returns true when upper case https' do
stub_env('HTTPS_PROXY', 'https://my.proxy')
expect(described_class.http_proxy_env?).to eq(true)
end
it 'returns true when lower case http' do
stub_env('http_proxy', 'http://my.proxy')
expect(described_class.http_proxy_env?).to eq(true)
end
it 'returns true when upper case http' do
stub_env('HTTP_PROXY', 'http://my.proxy')
expect(described_class.http_proxy_env?).to eq(true)
end
it 'returns false when not set' do
expect(described_class.http_proxy_env?).to eq(false)
end
end
end end
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