Raise more descriptive errors when URLs are blocked

6 files changed, 52 insertions, 26 deletions

diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb
index a34024f4f80..a3828acc50b 100644
--- a/app/services/projects/import_service.rb
+++ b/app/services/projects/import_service.rb
@@ -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,
diff --git a/app/validators/importable_url_validator.rb b/app/validators/importable_url_validator.rb
index 3ec1594e202..cafb43e69a2 100644
--- a/app/validators/importable_url_validator.rb
+++ b/app/validators/importable_url_validator.rb
@@ -4,8 +4,10 @@
 # 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")
+    begin
+      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
diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb
index 96558872a37..9aca3b0fb26 100644
--- a/lib/gitlab/http.rb
+++ b/lib/gitlab/http.rb
@@ -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
diff --git a/lib/gitlab/proxy_http_connection_adapter.rb b/lib/gitlab/proxy_http_connection_adapter.rb
index c70d6f4cd84..65ea8c22309 100644
--- a/lib/gitlab/proxy_http_connection_adapter.rb
+++ b/lib/gitlab/proxy_http_connection_adapter.rb
@@ -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_private_networks: 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
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 0f9f939e204..af6c389b4a2 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -2,34 +2,45 @@ 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_private_networks: 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)
+          return true if internal?(uri)
 
-          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)
+          raise BlockedUrlError, "Port is blocked" if blocked_port?(uri.port, valid_ports)
+          raise BlockedUrlError, "User is blocked" if blocked_user_or_hostname?(uri.user)
+          raise BlockedUrlError, "Hostname is blocked" if blocked_user_or_hostname?(uri.hostname)
 
           addrs_info = Addrinfo.getaddrinfo(uri.hostname, 80, nil, :STREAM)
-          server_ips = addrs_info.map(&:ip_address)
 
-          return true if (blocked_ips & server_ips).any?
-          return true if !allow_private_networks && private_network?(addrs_info)
+          if !allow_localhost && localhost?(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
-          return true
+          raise BlockedUrlError, "URI is invalid"
         rescue SocketError
-          return false
+          return
         end
 
+        true
+      end
+
+      def blocked_url?(*args)
+        validate!(*args)
+
         false
+      rescue BlockedUrlError
+        true
       end
 
       private
@@ -60,6 +71,13 @@ module Gitlab
           (uri.port.blank? || uri.port == config.gitlab_shell.ssh_port)
       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)
         addrs_info.any? { |addr| addr.ipv4_private? || addr.ipv6_sitelocal? }
       end
diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
index b0bc081a3c8..d0dadfa78da 100644
--- a/spec/lib/gitlab/http_spec.rb
+++ b/spec/lib/gitlab/http_spec.rb
@@ -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