Raise more descriptive errors when URLs are blocked

This commit is contained in:
Douwe Maan 2018-03-28 19:27:16 +02:00
parent 6b5ec93ad9
commit 2e3bc6a941
No known key found for this signature in database
GPG Key ID: 5976703F65143D36
6 changed files with 52 additions and 26 deletions

View File

@ -28,7 +28,11 @@ module Projects
def add_repository_to_project def add_repository_to_project
if project.external_import? && !unknown_url? 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 end
# We should skip the repository for a GitHub import or GitLab project import, # We should skip the repository for a GitHub import or GitLab project import,

View File

@ -4,8 +4,10 @@
# protect against Server-side Request Forgery (SSRF). # protect against Server-side Request Forgery (SSRF).
class ImportableUrlValidator < ActiveModel::EachValidator class ImportableUrlValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value) def validate_each(record, attribute, value)
if Gitlab::UrlBlocker.blocked_url?(value, valid_ports: Project::VALID_IMPORT_PORTS) begin
record.errors.add(attribute, "imports are not allowed from that URL") 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
end end
end end

View File

@ -4,6 +4,8 @@
# calling internal IP or services. # calling internal IP or services.
module Gitlab module Gitlab
class HTTP class HTTP
BlockedUrlError = Class.new(StandardError)
include HTTParty # rubocop:disable Gitlab/HTTParty include HTTParty # rubocop:disable Gitlab/HTTParty
connection_adapter ProxyHTTPConnectionAdapter connection_adapter ProxyHTTPConnectionAdapter

View File

@ -10,8 +10,12 @@
module Gitlab module Gitlab
class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter
def connection def connection
if !allow_local_requests? && blocked_url? unless allow_local_requests?
raise URI::InvalidURIError begin
Gitlab::UrlBlocker.validate!(uri, allow_private_networks: false)
rescue Gitlab::UrlBlocker::BlockedUrlError => e
raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}"
end
end end
super super
@ -19,10 +23,6 @@ module Gitlab
private private
def blocked_url?
Gitlab::UrlBlocker.blocked_url?(uri, allow_private_networks: false)
end
def allow_local_requests? def allow_local_requests?
options.fetch(:allow_local_requests, allow_settings_local_requests?) options.fetch(:allow_local_requests, allow_settings_local_requests?)
end end

View File

@ -2,34 +2,45 @@ require 'resolv'
module Gitlab module Gitlab
class UrlBlocker class UrlBlocker
class << self BlockedUrlError = Class.new(StandardError)
def blocked_url?(url, allow_private_networks: true, valid_ports: [])
return false if url.nil?
blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"] class << self
blocked_ips.concat(Socket.ip_address_list.map(&:ip_address)) def validate!(url, allow_localhost: false, allow_private_networks: true, valid_ports: [])
return true if url.nil?
begin begin
uri = Addressable::URI.parse(url) uri = Addressable::URI.parse(url)
# Allow imports from the GitLab instance itself but only from the configured ports # Allow imports from the GitLab instance itself but only from the configured ports
return false if internal?(uri) return true if internal?(uri)
return true if blocked_port?(uri.port, valid_ports) raise BlockedUrlError, "Port is blocked" if blocked_port?(uri.port, valid_ports)
return true if blocked_user_or_hostname?(uri.user) raise BlockedUrlError, "User is blocked" if blocked_user_or_hostname?(uri.user)
return true if blocked_user_or_hostname?(uri.hostname) raise BlockedUrlError, "Hostname is blocked" if blocked_user_or_hostname?(uri.hostname)
addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM) addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM)
server_ips = addrs_info.map(&:ip_address)
return true if (blocked_ips & server_ips).any? if !allow_localhost && localhost?(addrs_info)
return true if !allow_private_networks && private_network?(addrs_info) raise BlockedUrlError, "Requests to localhost are blocked"
end
if !allow_private_networks && private_network?(addrs_info)
raise BlockedUrlError, "Requests to the private local network are blocked"
end
rescue Addressable::URI::InvalidURIError rescue Addressable::URI::InvalidURIError
return true raise BlockedUrlError, "URI is invalid"
rescue SocketError rescue SocketError
return false return
end end
true
end
def blocked_url?(*args)
validate!(*args)
false false
rescue BlockedUrlError
true
end end
private private
@ -60,6 +71,13 @@ module Gitlab
(uri.port.blank? || uri.port == config.gitlab_shell.ssh_port) (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
end end
def localhost?(addrs_info)
blocked_ips = ["127.0.0.1", "::1", "0.0.0.0"]
blocked_ips.concat(Socket.ip_address_list.map(&:ip_address))
(blocked_ips & addrs_info.map(&:ip_address)).any?
end
def private_network?(addrs_info) def private_network?(addrs_info)
addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? } addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
end end

View File

@ -12,11 +12,11 @@ describe Gitlab::HTTP do
end end
it 'deny requests to localhost' do 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 end
it 'deny requests to private network' do 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 end
context 'if allow_local_requests set to true' do 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 context 'if allow_local_requests set to false' do
it 'override the global value and ban requests to localhost or private network' 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 end
end end