Commit 831d60aa authored by Oswaldo Ferreira's avatar Oswaldo Ferreira

Add DNS rebinding protection settings

parent 7c13c20a
......@@ -127,6 +127,7 @@ module ApplicationSettingsHelper
:akismet_api_key,
:akismet_enabled,
:allow_local_requests_from_hooks_and_services,
:dns_rebinding_protection_enabled,
:archive_builds_in_human_readable,
:authorized_keys_enabled,
:auto_devops_enabled,
......
......@@ -8,4 +8,12 @@
= f.label :allow_local_requests_from_hooks_and_services, class: 'form-check-label' do
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"
# 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 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20190312071108) do
ActiveRecord::Schema.define(version: 20190529142545) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
......@@ -177,6 +177,7 @@ ActiveRecord::Schema.define(version: 20190312071108) do
t.string "runners_registration_token_encrypted"
t.integer "local_markdown_version", default: 0, null: false
t.integer "first_day_of_week", default: 0, 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
end
......
......@@ -40,6 +40,7 @@ module Gitlab
SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}
VERSION = File.read(root.join("VERSION")).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?
# Check `gl_subdomain?` as well to keep parity with gitlab.com
......@@ -62,6 +63,10 @@ module Gitlab
Object.const_defined?(:License)
end
def self.http_proxy_env?
HTTP_PROXY_ENV_VARS.any? { |name| ENV[name] }
end
def self.process_name
return 'sidekiq' if Sidekiq.server?
return 'console' if defined?(Rails::Console)
......
......@@ -14,7 +14,8 @@ module Gitlab
def connection
begin
@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
raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}"
end
......@@ -30,6 +31,12 @@ module Gitlab
options.fetch(:allow_local_requests, allow_settings_local_requests?)
end
def dns_rebind_protection?
return false if Gitlab.http_proxy_env?
Gitlab::CurrentSettings.dns_rebinding_protection_enabled?
end
def allow_settings_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_hooks_and_services?
end
......
......@@ -18,7 +18,21 @@ module Gitlab
# enforce_sanitization - Raises error if URL includes any HTML/CSS/JS tags and argument is true.
#
# Returns an array with [<uri>, <original-hostname>].
def validate!(url, ports: [], protocols: [], 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?
# Param url can be a string, URI or Addressable::URI
......@@ -45,15 +59,17 @@ module Gitlab
return [uri, nil]
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
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_loopback!(addrs_info) unless allow_localhost
validate_local_network!(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
def blocked_url?(*args)
......@@ -74,17 +90,15 @@ module Gitlab
#
# 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.
def enforce_uri_hostname(addrs_info, uri, hostname)
def enforce_uri_hostname(addrs_info, uri, hostname, dns_rebind_protection)
address = addrs_info.first
ip_address = address&.ip_address
if ip_address && ip_address != hostname
uri = uri.dup
uri.hostname = ip_address
return [uri, hostname]
end
return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
[uri, nil]
uri = uri.dup
uri.hostname = ip_address
[uri, hostname]
end
def get_port(uri)
......
......@@ -3034,6 +3034,9 @@ msgstr ""
msgid "Ends at (UTC)"
msgstr ""
msgid "Enforce DNS rebinding attack protection"
msgstr ""
msgid "Enter in your Bitbucket Server URL and personal access token below"
msgstr ""
......@@ -6438,6 +6441,9 @@ msgstr ""
msgid "Resolved"
msgstr ""
msgid "Resolves IP addresses once and uses them to submit requests"
msgstr ""
msgid "Response metrics (AWS ELB)"
msgstr ""
......
......@@ -325,16 +325,19 @@ describe 'Admin updates settings' do
end
context 'Network page' do
it 'Enable outbound requests' do
it 'Changes Outbound requests settings' do
visit network_admin_application_settings_path
page.within('.as-outbound') do
check 'Allow requests to the local network from hooks and services'
# Enabled by default
uncheck 'Enforce DNS rebinding attack protection'
click_button 'Save changes'
end
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.dns_rebinding_protection_enabled).to be false
end
end
......
......@@ -47,6 +47,38 @@ describe Gitlab::HTTPConnectionAdapter do
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
it 'sets up the connection' do
uri = URI('https://example.org')
......
......@@ -46,6 +46,41 @@ describe Gitlab::UrlBlocker do
expect(hostname).to be(nil)
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
describe '#blocked_url?' do
......
......@@ -109,4 +109,34 @@ describe Gitlab do
expect(described_class.ee?).to eq(false)
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
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