Commit f2e6a738 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'dm-gitlab-http-blocked-url-error' into 'master'

Raise more descriptive errors when URLs are blocked

See merge request gitlab-org/gitlab-ce!18058
parents 2f17b4cb b290d929
......@@ -46,6 +46,8 @@ class Projects::ServicesController < Projects::ApplicationController
else
{ error: true, message: 'Validations failed.', service_response: @service.errors.full_messages.join(',') }
end
rescue Gitlab::HTTP::BlockedUrlError => e
{ error: true, message: 'Test failed.', service_response: e.message }
end
def success_message
......
......@@ -28,7 +28,11 @@ module Projects
def add_repository_to_project
if project.external_import? && !unknown_url?
raise Error, 'Blocked import URL.' if Gitlab::UrlBlocker.blocked_url?(project.import_url, valid_ports: Project::VALID_IMPORT_PORTS)
begin
Gitlab::UrlBlocker.validate!(project.import_url, valid_ports: Project::VALID_IMPORT_PORTS)
rescue Gitlab::UrlBlocker::BlockedUrlError => e
raise Error, "Blocked import URL: #{e.message}"
end
end
# We should skip the repository for a GitHub import or GitLab project import,
......
......@@ -4,8 +4,8 @@
# protect against Server-side Request Forgery (SSRF).
class ImportableUrlValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
if Gitlab::UrlBlocker.blocked_url?(value, valid_ports: Project::VALID_IMPORT_PORTS)
record.errors.add(attribute, "imports are not allowed from that URL")
end
Gitlab::UrlBlocker.validate!(value, valid_ports: Project::VALID_IMPORT_PORTS)
rescue Gitlab::UrlBlocker::BlockedUrlError => e
record.errors.add(attribute, "is blocked: #{e.message}")
end
end
......@@ -4,6 +4,8 @@
# calling internal IP or services.
module Gitlab
class HTTP
BlockedUrlError = Class.new(StandardError)
include HTTParty # rubocop:disable Gitlab/HTTParty
connection_adapter ProxyHTTPConnectionAdapter
......
......@@ -10,8 +10,12 @@
module Gitlab
class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter
def connection
if !allow_local_requests? && blocked_url?
raise URI::InvalidURIError
unless allow_local_requests?
begin
Gitlab::UrlBlocker.validate!(uri, allow_local_network: false)
rescue Gitlab::UrlBlocker::BlockedUrlError => e
raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}"
end
end
super
......@@ -19,10 +23,6 @@ module Gitlab
private
def blocked_url?
Gitlab::UrlBlocker.blocked_url?(uri, allow_private_networks: false)
end
def allow_local_requests?
options.fetch(:allow_local_requests, allow_settings_local_requests?)
end
......
......@@ -2,48 +2,84 @@ require 'resolv'
module Gitlab
class UrlBlocker
class << self
def blocked_url?(url, allow_private_networks: true, valid_ports: [])
return false if url.nil?
BlockedUrlError = Class.new(StandardError)
blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"]
blocked_ips.concat(Socket.ip_address_list.map(&:ip_address))
class << self
def validate!(url, allow_localhost: false, allow_local_network: true, valid_ports: [])
return true if url.nil?
begin
uri = Addressable::URI.parse(url)
# Allow imports from the GitLab instance itself but only from the configured ports
return false if internal?(uri)
rescue Addressable::URI::InvalidURIError
raise BlockedUrlError, "URI is invalid"
end
return true if blocked_port?(uri.port, valid_ports)
return true if blocked_user_or_hostname?(uri.user)
return true if blocked_user_or_hostname?(uri.hostname)
# Allow imports from the GitLab instance itself but only from the configured ports
return true if internal?(uri)
addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM)
server_ips = addrs_info.map(&:ip_address)
port = uri.port || uri.default_port
validate_port!(port, valid_ports) if valid_ports.any?
validate_user!(uri.user)
validate_hostname!(uri.hostname)
return true if (blocked_ips & server_ips).any?
return true if !allow_private_networks && private_network?(addrs_info)
rescue Addressable::URI::InvalidURIError
return true
begin
addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM)
rescue SocketError
return false
return true
end
validate_localhost!(addrs_info) unless allow_localhost
validate_local_network!(addrs_info) unless allow_local_network
true
end
def blocked_url?(*args)
validate!(*args)
false
rescue BlockedUrlError
true
end
private
def blocked_port?(port, valid_ports)
return false if port.blank? || valid_ports.blank?
def validate_port!(port, valid_ports)
return if port.blank?
# Only ports under 1024 are restricted
return if port >= 1024
return if valid_ports.include?(port)
raise BlockedUrlError, "Only allowed ports are #{valid_ports.join(', ')}, and any over 1024"
end
def validate_user!(value)
return if value.blank?
return if value =~ /\A\p{Alnum}/
port < 1024 && !valid_ports.include?(port)
raise BlockedUrlError, "Username needs to start with an alphanumeric character"
end
def blocked_user_or_hostname?(value)
return false if value.blank?
def validate_hostname!(value)
return if value.blank?
return if value =~ /\A\p{Alnum}/
value !~ /\A\p{Alnum}/
raise BlockedUrlError, "Hostname needs to start with an alphanumeric character"
end
def validate_localhost!(addrs_info)
local_ips = ["127.0.0.1", "::1", "0.0.0.0"]
local_ips.concat(Socket.ip_address_list.map(&:ip_address))
return if (local_ips & addrs_info.map(&:ip_address)).empty?
raise BlockedUrlError, "Requests to localhost are not allowed"
end
def validate_local_network!(addrs_info)
return unless addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
raise BlockedUrlError, "Requests to the local network are not allowed"
end
def internal?(uri)
......@@ -60,10 +96,6 @@ module Gitlab
(uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
end
def private_network?(addrs_info)
addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
end
def config
Gitlab.config
end
......
......@@ -12,11 +12,11 @@ describe Gitlab::HTTP do
end
it 'deny requests to localhost' do
expect { described_class.get('http://localhost:3003') }.to raise_error(URI::InvalidURIError)
expect { described_class.get('http://localhost:3003') }.to raise_error(Gitlab::HTTP::BlockedUrlError)
end
it 'deny requests to private network' do
expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(URI::InvalidURIError)
expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(Gitlab::HTTP::BlockedUrlError)
end
context 'if allow_local_requests set to true' do
......@@ -41,7 +41,7 @@ describe Gitlab::HTTP do
context 'if allow_local_requests set to false' do
it 'override the global value and ban requests to localhost or private network' do
expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(URI::InvalidURIError)
expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(Gitlab::HTTP::BlockedUrlError)
end
end
end
......
......@@ -74,13 +74,13 @@ describe Gitlab::UrlBlocker do
expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
end
context 'when allow_private_networks is' do
let(:private_networks) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
context 'when allow_local_network is' do
let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
let(:fake_domain) { 'www.fakedomain.fake' }
context 'true (default)' do
it 'does not block urls from private networks' do
private_networks.each do |ip|
local_ips.each do |ip|
stub_domain_resolv(fake_domain, ip)
expect(described_class).not_to be_blocked_url("http://#{fake_domain}")
......@@ -94,14 +94,14 @@ describe Gitlab::UrlBlocker do
context 'false' do
it 'blocks urls from private networks' do
private_networks.each do |ip|
local_ips.each do |ip|
stub_domain_resolv(fake_domain, ip)
expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_private_networks: false)
expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_local_network: false)
unstub_domain_resolv
expect(described_class).to be_blocked_url("http://#{ip}", allow_private_networks: false)
expect(described_class).to be_blocked_url("http://#{ip}", allow_local_network: false)
end
end
end
......
......@@ -224,14 +224,14 @@ describe Project do
project2 = build(:project, import_url: 'http://localhost:9000/t.git')
expect(project2).to be_invalid
expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed')
end
it "does not allow blocked import_url port" do
project2 = build(:project, import_url: 'http://github.com:25/t.git')
expect(project2).to be_invalid
expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443')
end
describe 'project pending deletion' do
......
......@@ -156,7 +156,7 @@ describe Projects::ImportService do
result = described_class.new(project, user).execute
expect(result[:status]).to eq :error
expect(result[:message]).to end_with 'Blocked import URL.'
expect(result[:message]).to include('Requests to localhost are not allowed')
end
it 'fails with port 25' do
......@@ -165,7 +165,7 @@ describe Projects::ImportService do
result = described_class.new(project, user).execute
expect(result[:status]).to eq :error
expect(result[:message]).to end_with 'Blocked import URL.'
expect(result[:message]).to include('Only allowed ports are 22, 80, 443')
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