Protect Gitlab::HTTP against DNS rebinding attack

Gitlab::HTTP now resolves the hostname only once, verifies the IP is not
blocked, and then uses the same IP to perform the actual request, while
passing the original hostname in the `Host` header and SSL SNI field.

29 files changed, 419 insertions, 90 deletions

diff --git a/changelogs/unreleased/dm-http-hostname-override.yml b/changelogs/unreleased/dm-http-hostname-override.yml
new file mode 100644
index 00000000000..f84f36a0010
--- /dev/null
+++ b/changelogs/unreleased/dm-http-hostname-override.yml
@@ -0,0 +1,5 @@
+---
+title: Protect Gitlab::HTTP against DNS rebinding attack
+merge_request:
+author:
+type: security
diff --git a/config/initializers/hipchat_client_patch.rb b/config/initializers/hipchat_client_patch.rb
index 1879ecb15fb..51bd48af320 100644
--- a/config/initializers/hipchat_client_patch.rb
+++ b/config/initializers/hipchat_client_patch.rb
@@ -2,14 +2,14 @@
 # This monkey patches the HTTParty used in https://github.com/hipchat/hipchat-rb.
 module HipChat
   class Client
-    connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter
+    connection_adapter ::Gitlab::HTTPConnectionAdapter
   end
 
   class Room
-    connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter
+    connection_adapter ::Gitlab::HTTPConnectionAdapter
   end
 
   class User
-    connection_adapter ::Gitlab::ProxyHTTPConnectionAdapter
+    connection_adapter ::Gitlab::HTTPConnectionAdapter
   end
 end
diff --git a/config/initializers/http_hostname_override.rb b/config/initializers/http_hostname_override.rb
new file mode 100644
index 00000000000..58dd380326f
--- /dev/null
+++ b/config/initializers/http_hostname_override.rb
@@ -0,0 +1,49 @@
+# This override allows passing `@hostname_override` to the SNI protocol,
+# which is used to lookup the correct SSL certificate in the
+# request handshake process.
+#
+# Given we've forced the HTTP request to be sent to the resolved
+# IP address in a few scenarios (e.g.: `Gitlab::HTTP` through
+# `Gitlab::UrlBlocker.validate!`), we need to provide the _original_
+# hostname via SNI in order to have a clean connection setup.
+#
+# This is ultimately needed in order to avoid DNS rebinding attacks
+# through HTTP requests.
+#
+class OpenSSL::SSL::SSLContext
+  attr_accessor :hostname_override
+end
+
+class OpenSSL::SSL::SSLSocket
+  module HostnameOverride
+    # rubocop: disable Gitlab/ModuleWithInstanceVariables
+    def hostname=(hostname)
+      super(@context.hostname_override || hostname)
+    end
+
+    def post_connection_check(hostname)
+      super(@context.hostname_override || hostname)
+    end
+    # rubocop: enable Gitlab/ModuleWithInstanceVariables
+  end
+
+  prepend HostnameOverride
+end
+
+class Net::HTTP
+  attr_accessor :hostname_override
+  SSL_IVNAMES << :@hostname_override
+  SSL_ATTRIBUTES << :hostname_override
+
+  module HostnameOverride
+    def addr_port
+      return super unless hostname_override
+
+      addr = hostname_override
+      default_port = use_ssl? ? Net::HTTP.https_default_port : Net::HTTP.http_default_port
+      default_port == port ? addr : "#{addr}:#{port}"
+    end
+  end
+
+  prepend HostnameOverride
+end
diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb
index bcd9e2be35f..55ed7e7e837 100644
--- a/lib/gitlab/http.rb
+++ b/lib/gitlab/http.rb
@@ -11,7 +11,7 @@ module Gitlab
 
     include HTTParty # rubocop:disable Gitlab/HTTParty
 
-    connection_adapter ProxyHTTPConnectionAdapter
+    connection_adapter HTTPConnectionAdapter
 
     def self.perform_request(http_method, path, options, &block)
       super
diff --git a/lib/gitlab/proxy_http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb
index a64cb47e77e..9ccf0653903 100644
--- a/lib/gitlab/proxy_http_connection_adapter.rb
+++ b/lib/gitlab/http_connection_adapter.rb
@@ -10,17 +10,18 @@
 #
 # This option will take precedence over the global setting.
 module Gitlab
-  class ProxyHTTPConnectionAdapter < HTTParty::ConnectionAdapter
+  class HTTPConnectionAdapter < HTTParty::ConnectionAdapter
     def connection
-      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
+      begin
+        @uri, hostname = Gitlab::UrlBlocker.validate!(uri, allow_local_network: allow_local_requests?,
+                                                           allow_localhost: allow_local_requests?)
+      rescue Gitlab::UrlBlocker::BlockedUrlError => e
+        raise Gitlab::HTTP::BlockedUrlError, "URL '#{uri}' is blocked: #{e.message}"
       end
 
-      super
+      super.tap do |http|
+        http.hostname_override = hostname if hostname
+      end
     end
 
     private
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 641ba70ef83..81935f2b052 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -8,38 +8,52 @@ module Gitlab
     BlockedUrlError = Class.new(StandardError)
 
     class << self
-      def validate!(url, ports: [], schemes: [], allow_localhost: false, allow_local_network: true, ascii_only: false, enforce_user: false, enforce_sanitization: false)
-        return true if url.nil?
+      # Validates the given url according to the constraints specified by arguments.
+      #
+      # ports - Raises error if the given URL port does is not between given ports.
+      # allow_localhost - Raises error if URL resolves to a localhost IP address and argument is true.
+      # allow_local_network - Raises error if URL resolves to a link-local address and argument is true.
+      # ascii_only - Raises error if URL has unicode characters and argument is true.
+      # enforce_user - Raises error if URL user doesn't start with alphanumeric characters 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>].
+      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
+        return [nil, nil] if url.nil?
 
         # Param url can be a string, URI or Addressable::URI
         uri = parse_url(url)
 
         validate_html_tags!(uri) if enforce_sanitization
 
-        # Allow imports from the GitLab instance itself but only from the configured ports
-        return true if internal?(uri)
-
+        hostname = uri.hostname
         port = get_port(uri)
-        validate_scheme!(uri.scheme, schemes)
-        validate_port!(port, ports) if ports.any?
-        validate_user!(uri.user) if enforce_user
-        validate_hostname!(uri.hostname)
-        validate_unicode_restriction!(uri) if ascii_only
+
+        unless internal?(uri)
+          validate_scheme!(uri.scheme, schemes)
+          validate_port!(port, ports) if ports.any?
+          validate_user!(uri.user) if enforce_user
+          validate_hostname!(hostname)
+          validate_unicode_restriction!(uri) if ascii_only
+        end
 
         begin
-          addrs_info = Addrinfo.getaddrinfo(uri.hostname, port, nil, :STREAM).map do |addr|
+          addrs_info = Addrinfo.getaddrinfo(hostname, port, nil, :STREAM).map do |addr|
             addr.ipv6_v4mapped? ? addr.ipv6_to_ipv4 : addr
           end
         rescue SocketError
-          return true
+          return [uri, nil]
         end
 
+        # 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)
+
         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
 
-        true
+        enforce_uri_hostname(addrs_info, uri, hostname)
       end
 
       def blocked_url?(*args)
@@ -52,6 +66,27 @@ module Gitlab
 
       private
 
+      # Returns the given URI with IP address as hostname and the original hostname respectively
+      # in an Array.
+      #
+      # It checks whether the resolved IP address matches with the hostname. If not, it changes
+      # the hostname to the resolved IP address.
+      #
+      # 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)
+        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
+
+        [uri, nil]
+      end
+
       def get_port(uri)
         uri.port || uri.default_port
       end
diff --git a/spec/controllers/projects/ci/lints_controller_spec.rb b/spec/controllers/projects/ci/lints_controller_spec.rb
index 323a32575af..cc6ac83ca38 100644
--- a/spec/controllers/projects/ci/lints_controller_spec.rb
+++ b/spec/controllers/projects/ci/lints_controller_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Projects::Ci::LintsController do
+  include StubRequests
+
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
 
@@ -70,7 +72,7 @@ describe Projects::Ci::LintsController do
 
     context 'with a valid gitlab-ci.yml' do
       before do
-        WebMock.stub_request(:get, remote_file_path).to_return(body: remote_file_content)
+        stub_full_request(remote_file_path).to_return(body: remote_file_content)
         project.add_developer(user)
 
         post :create, params: { namespace_id: project.namespace, project_id: project, content: content }
diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
index d8a61618e77..46d68097fff 100644
--- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::External::File::Remote do
+  include StubRequests
+
   let(:context) { described_class::Context.new(nil, '12345', nil, Set.new) }
   let(:params) { { remote: location } }
   let(:remote_file) { described_class.new(params, context) }
@@ -46,7 +48,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
   describe "#valid?" do
     context 'when is a valid remote url' do
       before do
-        WebMock.stub_request(:get, location).to_return(body: remote_file_content)
+        stub_full_request(location).to_return(body: remote_file_content)
       end
 
       it 'returns true' do
@@ -92,7 +94,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
   describe "#content" do
     context 'with a valid remote file' do
       before do
-        WebMock.stub_request(:get, location).to_return(body: remote_file_content)
+        stub_full_request(location).to_return(body: remote_file_content)
       end
 
       it 'returns the content of the file' do
@@ -114,7 +116,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
       let(:location) { 'https://asdasdasdaj48ggerexample.com' }
 
       before do
-        WebMock.stub_request(:get, location).to_raise(SocketError.new('Some HTTP error'))
+        stub_full_request(location).to_raise(SocketError.new('Some HTTP error'))
       end
 
       it 'is nil' do
@@ -144,7 +146,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
 
     context 'when timeout error has been raised' do
       before do
-        WebMock.stub_request(:get, location).to_timeout
+        stub_full_request(location).to_timeout
       end
 
       it 'returns error message about a timeout' do
@@ -154,7 +156,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
 
     context 'when HTTP error has been raised' do
       before do
-        WebMock.stub_request(:get, location).to_raise(Gitlab::HTTP::Error)
+        stub_full_request(location).to_raise(Gitlab::HTTP::Error)
       end
 
       it 'returns error message about a HTTP error' do
@@ -164,7 +166,7 @@ describe Gitlab::Ci::Config::External::File::Remote do
 
     context 'when response has 404 status' do
       before do
-        WebMock.stub_request(:get, location).to_return(body: remote_file_content, status: 404)
+        stub_full_request(location).to_return(body: remote_file_content, status: 404)
       end
 
       it 'returns error message about a timeout' do
diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb
index 136974569de..e068b786b02 100644
--- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::External::Mapper do
+  include StubRequests
+
   set(:project) { create(:project, :repository) }
   set(:user) { create(:user) }
 
@@ -18,7 +20,7 @@ describe Gitlab::Ci::Config::External::Mapper do
   end
 
   before do
-    WebMock.stub_request(:get, remote_url).to_return(body: file_content)
+    stub_full_request(remote_url).to_return(body: file_content)
   end
 
   describe '#process' do
diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb
index 0f58a4f1d44..856187371e1 100644
--- a/spec/lib/gitlab/ci/config/external/processor_spec.rb
+++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config::External::Processor do
+  include StubRequests
+
   set(:project) { create(:project, :repository) }
   set(:another_project) { create(:project, :repository) }
   set(:user) { create(:user) }
@@ -42,7 +44,7 @@ describe Gitlab::Ci::Config::External::Processor do
       let(:values) { { include: remote_file, image: 'ruby:2.2' } }
 
       before do
-        WebMock.stub_request(:get, remote_file).to_raise(SocketError.new('Some HTTP error'))
+        stub_full_request(remote_file).and_raise(SocketError.new('Some HTTP error'))
       end
 
       it 'raises an error' do
@@ -75,7 +77,7 @@ describe Gitlab::Ci::Config::External::Processor do
       end
 
       before do
-        WebMock.stub_request(:get, remote_file).to_return(body: external_file_content)
+        stub_full_request(remote_file).to_return(body: external_file_content)
       end
 
       it 'appends the file to the values' do
@@ -145,7 +147,7 @@ describe Gitlab::Ci::Config::External::Processor do
         allow_any_instance_of(Gitlab::Ci::Config::External::File::Local)
           .to receive(:fetch_local_content).and_return(local_file_content)
 
-        WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content)
+        stub_full_request(remote_file).to_return(body: remote_file_content)
       end
 
       it 'appends the files to the values' do
@@ -191,7 +193,8 @@ describe Gitlab::Ci::Config::External::Processor do
       end
 
       it 'takes precedence' do
-        WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content)
+        stub_full_request(remote_file).to_return(body: remote_file_content)
+
         expect(processor.perform[:image]).to eq('ruby:2.2')
       end
     end
@@ -231,7 +234,8 @@ describe Gitlab::Ci::Config::External::Processor do
           HEREDOC
         end
 
-        WebMock.stub_request(:get, 'http://my.domain.com/config.yml').to_return(body: 'remote_build: { script: echo Hello World }')
+        stub_full_request('http://my.domain.com/config.yml')
+          .to_return(body: 'remote_build: { script: echo Hello World }')
       end
 
       context 'when project is public' do
@@ -273,8 +277,10 @@ describe Gitlab::Ci::Config::External::Processor do
 
     context 'when config includes an external configuration file via SSL web request' do
       before do
-        stub_request(:get, 'https://sha256.badssl.com/fake.yml').to_return(body: 'image: ruby:2.6', status: 200)
-        stub_request(:get, 'https://self-signed.badssl.com/fake.yml')
+        stub_full_request('https://sha256.badssl.com/fake.yml', ip_address: '8.8.8.8')
+          .to_return(body: 'image: ruby:2.6', status: 200)
+
+        stub_full_request('https://self-signed.badssl.com/fake.yml', ip_address: '8.8.8.9')
           .to_raise(OpenSSL::SSL::SSLError.new('SSL_connect returned=1 errno=0 state=error: certificate verify failed (self signed certificate)'))
       end
 
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 092e9f242b7..7f336ee853e 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Config do
+  include StubRequests
+
   set(:user) { create(:user) }
 
   let(:config) do
@@ -216,8 +218,7 @@ describe Gitlab::Ci::Config do
     end
 
     before do
-      WebMock.stub_request(:get, remote_location)
-        .to_return(body: remote_file_content)
+      stub_full_request(remote_location).to_return(body: remote_file_content)
 
       allow(project.repository)
         .to receive(:blob_data_at).and_return(local_file_content)
diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb
index 0d998d89d73..29276d5b686 100644
--- a/spec/lib/gitlab/ci/yaml_processor_spec.rb
+++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb
@@ -3,6 +3,8 @@ require 'spec_helper'
 module Gitlab
   module Ci
     describe YamlProcessor do
+      include StubRequests
+
       subject { described_class.new(config, user: nil) }
 
       describe '#build_attributes' do
@@ -648,7 +650,7 @@ module Gitlab
             end
 
             before do
-              WebMock.stub_request(:get, 'https://gitlab.com/awesome-project/raw/master/.before-script-template.yml')
+              stub_full_request('https://gitlab.com/awesome-project/raw/master/.before-script-template.yml')
                 .to_return(
                   status: 200,
                   headers: { 'Content-Type' => 'application/json' },
diff --git a/spec/lib/gitlab/http_connection_adapter_spec.rb b/spec/lib/gitlab/http_connection_adapter_spec.rb
new file mode 100644
index 00000000000..af033be8e5b
--- /dev/null
+++ b/spec/lib/gitlab/http_connection_adapter_spec.rb
@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::HTTPConnectionAdapter do
+  describe '#connection' do
+    context 'when local requests are not allowed' do
+      it 'sets up the connection' do
+        uri = URI('https://example.org')
+
+        connection = described_class.new(uri).connection
+
+        expect(connection).to be_a(Net::HTTP)
+        expect(connection.address).to eq('93.184.216.34')
+        expect(connection.hostname_override).to eq('example.org')
+        expect(connection.addr_port).to eq('example.org')
+        expect(connection.port).to eq(443)
+      end
+
+      it 'raises error when it is a request to local address' do
+        uri = URI('http://172.16.0.0/12')
+
+        expect { described_class.new(uri).connection }
+          .to raise_error(Gitlab::HTTP::BlockedUrlError,
+                          "URL 'http://172.16.0.0/12' is blocked: Requests to the local network are not allowed")
+      end
+
+      it 'raises error when it is a request to localhost address' do
+        uri = URI('http://127.0.0.1')
+
+        expect { described_class.new(uri).connection }
+          .to raise_error(Gitlab::HTTP::BlockedUrlError,
+                          "URL 'http://127.0.0.1' is blocked: Requests to localhost are not allowed")
+      end
+
+      context 'when port different from URL scheme is used' do
+        it 'sets up the addr_port accordingly' do
+          uri = URI('https://example.org:8080')
+
+          connection = described_class.new(uri).connection
+
+          expect(connection.address).to eq('93.184.216.34')
+          expect(connection.hostname_override).to eq('example.org')
+          expect(connection.addr_port).to eq('example.org:8080')
+          expect(connection.port).to eq(8080)
+        end
+      end
+    end
+
+    context 'when local requests are allowed' do
+      it 'sets up the connection' do
+        uri = URI('https://example.org')
+
+        connection = described_class.new(uri, allow_local_requests: true).connection
+
+        expect(connection).to be_a(Net::HTTP)
+        expect(connection.address).to eq('93.184.216.34')
+        expect(connection.hostname_override).to eq('example.org')
+        expect(connection.addr_port).to eq('example.org')
+        expect(connection.port).to eq(443)
+      end
+
+      it 'sets up the connection when it is a local network' do
+        uri = URI('http://172.16.0.0/12')
+
+        connection = described_class.new(uri, allow_local_requests: true).connection
+
+        expect(connection).to be_a(Net::HTTP)
+        expect(connection.address).to eq('172.16.0.0')
+        expect(connection.hostname_override).to be(nil)
+        expect(connection.addr_port).to eq('172.16.0.0')
+        expect(connection.port).to eq(80)
+      end
+
+      it 'sets up the connection when it is localhost' do
+        uri = URI('http://127.0.0.1')
+
+        connection = described_class.new(uri, allow_local_requests: true).connection
+
+        expect(connection).to be_a(Net::HTTP)
+        expect(connection.address).to eq('127.0.0.1')
+        expect(connection.hostname_override).to be(nil)
+        expect(connection.addr_port).to eq('127.0.0.1')
+        expect(connection.port).to eq(80)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
index 6c37c157f5d..158f77cab2c 100644
--- a/spec/lib/gitlab/http_spec.rb
+++ b/spec/lib/gitlab/http_spec.rb
@@ -1,6 +1,28 @@
 require 'spec_helper'
 
 describe Gitlab::HTTP do
+  include StubRequests
+
+  context 'when allow_local_requests' do
+    it 'sends the request to the correct URI' do
+      stub_full_request('https://example.org:8080', ip_address: '8.8.8.8').to_return(status: 200)
+
+      described_class.get('https://example.org:8080', allow_local_requests: false)
+
+      expect(WebMock).to have_requested(:get, 'https://8.8.8.8:8080').once
+    end
+  end
+
+  context 'when not allow_local_requests' do
+    it 'sends the request to the correct URI' do
+      stub_full_request('https://example.org:8080')
+
+      described_class.get('https://example.org:8080', allow_local_requests: true)
+
+      expect(WebMock).to have_requested(:get, 'https://8.8.8.9:8080').once
+    end
+  end
+
   describe 'allow_local_requests_from_hooks_and_services is' do
     before do
       WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')
@@ -21,6 +43,8 @@ describe Gitlab::HTTP do
 
       context 'if allow_local_requests set to true' do
         it 'override the global value and allow requests to localhost or private network' do
+          stub_full_request('http://localhost:3003')
+
           expect { described_class.get('http://localhost:3003', allow_local_requests: true) }.not_to raise_error
         end
       end
@@ -32,6 +56,8 @@ describe Gitlab::HTTP do
       end
 
       it 'allow requests to localhost' do
+        stub_full_request('http://localhost:3003')
+
         expect { described_class.get('http://localhost:3003') }.not_to raise_error
       end
 
@@ -49,7 +75,7 @@ describe Gitlab::HTTP do
 
   describe 'handle redirect loops' do
     before do
-      WebMock.stub_request(:any, "http://example.org").to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep"))
+      stub_full_request("http://example.org", method: :any).to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep"))
     end
 
     it 'handles GET requests' do
diff --git a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
index 7c4ac62790e..21a227335cd 100644
--- a/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
+++ b/spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do
+  include StubRequests
+
   let(:example_url) { 'http://www.example.com' }
   let(:strategy) { subject.new(url: example_url, http_method: 'post') }
   let!(:project) { create(:project, :with_export) }
@@ -35,7 +37,7 @@ describe Gitlab::ImportExport::AfterExportStrategies::WebUploadStrategy do
 
     context 'when upload fails' do
       it 'stores the export error' do
-        stub_request(:post, example_url).to_return(status: [404, 'Page not found'])
+        stub_full_request(example_url, method: :post).to_return(status: [404, 'Page not found'])
 
         strategy.execute(user, project)
 
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 445a56ab0d8..416f8cfece9 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -2,6 +2,52 @@
 require 'spec_helper'
 
 describe Gitlab::UrlBlocker do
+  describe '#validate!' do
+    context 'when URI is nil' do
+      let(:import_url) { nil }
+
+      it 'returns no URI and hostname' do
+        uri, hostname = described_class.validate!(import_url)
+
+        expect(uri).to be(nil)
+        expect(hostname).to be(nil)
+      end
+    end
+
+    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)
+
+        expect(uri).to eq(Addressable::URI.parse('http://[::1]'))
+        expect(hostname).to eq('localhost')
+      end
+    end
+
+    context 'when the URL hostname is a domain' do
+      let(:import_url) { 'https://example.org' }
+
+      it 'returns URI and hostname' do
+        uri, hostname = described_class.validate!(import_url)
+
+        expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
+        expect(hostname).to eq('example.org')
+      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)
+
+        expect(uri).to eq(Addressable::URI.parse('https://93.184.216.34'))
+        expect(hostname).to be(nil)
+      end
+    end
+  end
+
   describe '#blocked_url?' do
     let(:ports) { Project::VALID_IMPORT_PORTS }
 
@@ -208,7 +254,7 @@ describe Gitlab::UrlBlocker do
       end
 
       def stub_domain_resolv(domain, ip)
-        address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false)
+        address = double(ip_address: ip, ipv4_private?: true, ipv6_link_local?: false, ipv4_loopback?: false, ipv6_loopback?: false, ipv4?: false)
         allow(Addrinfo).to receive(:getaddrinfo).with(domain, any_args).and_return([address])
         allow(address).to receive(:ipv6_v4mapped?).and_return(false)
       end
diff --git a/spec/lib/mattermost/session_spec.rb b/spec/lib/mattermost/session_spec.rb
index 77fea5b2d24..346455067a7 100644
--- a/spec/lib/mattermost/session_spec.rb
+++ b/spec/lib/mattermost/session_spec.rb
@@ -2,6 +2,7 @@ require 'spec_helper'
 
 describe Mattermost::Session, type: :request do
   include ExclusiveLeaseHelpers
+  include StubRequests
 
   let(:user) { create(:user) }
 
@@ -24,7 +25,7 @@ describe Mattermost::Session, type: :request do
     let(:location) { 'http://location.tld' }
     let(:cookie_header) {'MMOAUTH=taskik8az7rq8k6rkpuas7htia; Path=/;'}
     let!(:stub) do
-      WebMock.stub_request(:get, "#{mattermost_url}/oauth/gitlab/login")
+      stub_full_request("#{mattermost_url}/oauth/gitlab/login")
         .to_return(headers: { 'location' => location, 'Set-Cookie' => cookie_header }, status: 302)
     end
 
@@ -63,7 +64,7 @@ describe Mattermost::Session, type: :request do
         end
 
         before do
-          WebMock.stub_request(:get, "#{mattermost_url}/signup/gitlab/complete")
+          stub_full_request("#{mattermost_url}/signup/gitlab/complete")
             .with(query: hash_including({ 'state' => state }))
             .to_return do |request|
               post "/oauth/token",
@@ -80,7 +81,7 @@ describe Mattermost::Session, type: :request do
               end
             end
 
-          WebMock.stub_request(:post, "#{mattermost_url}/api/v4/users/logout")
+          stub_full_request("#{mattermost_url}/api/v4/users/logout", method: :post)
             .to_return(headers: { Authorization: 'token thisworksnow' }, status: 200)
         end
 
diff --git a/spec/models/project_services/assembla_service_spec.rb b/spec/models/project_services/assembla_service_spec.rb
index 7742e33e901..2c86c0ec7be 100644
--- a/spec/models/project_services/assembla_service_spec.rb
+++ b/spec/models/project_services/assembla_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe AssemblaService do
+  include StubRequests
+
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -23,12 +25,12 @@ describe AssemblaService do
       )
       @sample_data = Gitlab::DataBuilder::Push.build_sample(project, user)
       @api_url = 'https://atlas.assembla.com/spaces/project_name/github_tool?secret_key=verySecret'
-      WebMock.stub_request(:post, @api_url)
+      stub_full_request(@api_url, method: :post)
     end
 
     it "calls Assembla API" do
       @assembla_service.execute(@sample_data)
-      expect(WebMock).to have_requested(:post, @api_url).with(
+      expect(WebMock).to have_requested(:post, stubbed_hostname(@api_url)).with(
         body: /#{@sample_data[:before]}.*#{@sample_data[:after]}.*#{project.path}/
       ).once
     end
diff --git a/spec/models/project_services/bamboo_service_spec.rb b/spec/models/project_services/bamboo_service_spec.rb
index 08c510f09df..65d227a17f9 100644
--- a/spec/models/project_services/bamboo_service_spec.rb
+++ b/spec/models/project_services/bamboo_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe BambooService, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
+  include StubRequests
 
   let(:bamboo_url) { 'http://gitlab.com/bamboo' }
 
@@ -257,7 +258,7 @@ describe BambooService, :use_clean_rails_memory_store_caching do
   end
 
   def stub_bamboo_request(url, status, body)
-    WebMock.stub_request(:get, url).to_return(
+    stub_full_request(url).to_return(
       status: status,
       headers: { 'Content-Type' => 'application/json' },
       body: body
diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb
index 091d4d8f695..ca196069055 100644
--- a/spec/models/project_services/buildkite_service_spec.rb
+++ b/spec/models/project_services/buildkite_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe BuildkiteService, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
+  include StubRequests
 
   let(:project) { create(:project) }
 
@@ -110,10 +111,9 @@ describe BuildkiteService, :use_clean_rails_memory_store_caching do
     body ||= %q({"status":"success"})
     buildkite_full_url = 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=123'
 
-    WebMock.stub_request(:get, buildkite_full_url).to_return(
-      status: status,
-      headers: { 'Content-Type' => 'application/json' },
-      body: body
-    )
+    stub_full_request(buildkite_full_url)
+      .to_return(status: status,
+                 headers: { 'Content-Type' => 'application/json' },
+                 body: body)
   end
 end
diff --git a/spec/models/project_services/campfire_service_spec.rb b/spec/models/project_services/campfire_service_spec.rb
index bf4c52fc7ab..0d3dd89e93b 100644
--- a/spec/models/project_services/campfire_service_spec.rb
+++ b/spec/models/project_services/campfire_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe CampfireService do
+  include StubRequests
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -49,39 +51,37 @@ describe CampfireService do
     it "calls Campfire API to get a list of rooms and speak in a room" do
       # make sure a valid list of rooms is returned
       body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms.json')
-      WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return(
+
+      stub_full_request(@rooms_url).with(basic_auth: @auth).to_return(
         body: body,
         status: 200,
         headers: @headers
       )
+
       # stub the speak request with the room id found in the previous request's response
       speak_url = 'https://project-name.campfirenow.com/room/123/speak.json'
-      WebMock.stub_request(:post, speak_url).with(basic_auth: @auth)
+      stub_full_request(speak_url, method: :post).with(basic_auth: @auth)
 
       @campfire_service.execute(@sample_data)
 
-      expect(WebMock).to have_requested(:get, @rooms_url).once
-      expect(WebMock).to have_requested(:post, speak_url).with(
-        body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/
-      ).once
+      expect(WebMock).to have_requested(:get, stubbed_hostname(@rooms_url)).once
+      expect(WebMock).to have_requested(:post, stubbed_hostname(speak_url))
+                           .with(body: /#{project.path}.*#{@sample_data[:before]}.*#{@sample_data[:after]}/).once
     end
 
     it "calls Campfire API to get a list of rooms but shouldn't speak in a room" do
       # return a list of rooms that do not contain a room named 'test-room'
       body = File.read(Rails.root + 'spec/fixtures/project_services/campfire/rooms2.json')
-      WebMock.stub_request(:get, @rooms_url).with(basic_auth: @auth).to_return(
+      stub_full_request(@rooms_url).with(basic_auth: @auth).to_return(
         body: body,
         status: 200,
         headers: @headers
       )
-      # we want to make sure no request is sent to the /speak endpoint, here is a basic
-      # regexp that matches this endpoint
-      speak_url = 'https://verySecret:X@project-name.campfirenow.com/room/.*/speak.json'
 
       @campfire_service.execute(@sample_data)
 
-      expect(WebMock).to have_requested(:get, @rooms_url).once
-      expect(WebMock).not_to have_requested(:post, /#{speak_url}/)
+      expect(WebMock).to have_requested(:get, 'https://8.8.8.9/rooms.json').once
+      expect(WebMock).not_to have_requested(:post, '*/room/.*/speak.json')
     end
   end
 end
diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb
index 773b8b7890f..dde46c82df6 100644
--- a/spec/models/project_services/pivotaltracker_service_spec.rb
+++ b/spec/models/project_services/pivotaltracker_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe PivotaltrackerService do
+  include StubRequests
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -53,12 +55,12 @@ describe PivotaltrackerService do
     end
 
     before do
-      WebMock.stub_request(:post, url)
+      stub_full_request(url, method: :post)
     end
 
     it 'posts correct message' do
       service.execute(push_data)
-      expect(WebMock).to have_requested(:post, url).with(
+      expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
         body: {
           'source_commit' => {
             'commit_id' => '21c12ea',
@@ -85,14 +87,14 @@ describe PivotaltrackerService do
         service.execute(push_data(branch: 'master'))
         service.execute(push_data(branch: 'v10'))
 
-        expect(WebMock).to have_requested(:post, url).twice
+        expect(WebMock).to have_requested(:post, stubbed_hostname(url)).twice
       end
 
       it 'does not post message if branch is not in the list' do
         service.execute(push_data(branch: 'mas'))
         service.execute(push_data(branch: 'v11'))
 
-        expect(WebMock).not_to have_requested(:post, url)
+        expect(WebMock).not_to have_requested(:post, stubbed_hostname(url))
       end
     end
   end
diff --git a/spec/models/project_services/pushover_service_spec.rb b/spec/models/project_services/pushover_service_spec.rb
index d2a45f48705..380f02739bc 100644
--- a/spec/models/project_services/pushover_service_spec.rb
+++ b/spec/models/project_services/pushover_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe PushoverService do
+  include StubRequests
+
   describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -57,13 +59,13 @@ describe PushoverService do
         sound: sound
       )
 
-      WebMock.stub_request(:post, api_url)
+      stub_full_request(api_url, method: :post, ip_address: '8.8.8.8')
     end
 
     it 'calls Pushover API' do
       pushover.execute(sample_data)
 
-      expect(WebMock).to have_requested(:post, api_url).once
+      expect(WebMock).to have_requested(:post, 'https://8.8.8.8/1/messages.json').once
     end
   end
 end
diff --git a/spec/models/project_services/teamcity_service_spec.rb b/spec/models/project_services/teamcity_service_spec.rb
index 96dccae733b..1c434b25205 100644
--- a/spec/models/project_services/teamcity_service_spec.rb
+++ b/spec/models/project_services/teamcity_service_spec.rb
@@ -4,6 +4,7 @@ require 'spec_helper'
 
 describe TeamcityService, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
+  include StubRequests
 
   let(:teamcity_url) { 'http://gitlab.com/teamcity' }
 
@@ -212,7 +213,7 @@ describe TeamcityService, :use_clean_rails_memory_store_caching do
 
     body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
 
-    WebMock.stub_request(:get, teamcity_full_url).with(basic_auth: auth).to_return(
+    stub_full_request(teamcity_full_url).with(basic_auth: auth).to_return(
       status: status,
       headers: { 'Content-Type' => 'application/json' },
       body: body
diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb
index b6e8d74c2e9..0e2f3face71 100644
--- a/spec/requests/api/system_hooks_spec.rb
+++ b/spec/requests/api/system_hooks_spec.rb
@@ -1,12 +1,14 @@
 require 'spec_helper'
 
 describe API::SystemHooks do
+  include StubRequests
+
   let(:user) { create(:user) }
   let(:admin) { create(:admin) }
   let!(:hook) { create(:system_hook, url: "http://example.com") }
 
   before do
-    stub_request(:post, hook.url)
+    stub_full_request(hook.url, method: :post)
   end
 
   describe "GET /hooks" do
@@ -68,6 +70,8 @@ describe API::SystemHooks do
     end
 
     it 'sets default values for events' do
+      stub_full_request('http://mep.mep', method: :post)
+
       post api('/hooks', admin), params: { url: 'http://mep.mep' }
 
       expect(response).to have_gitlab_http_status(201)
@@ -78,6 +82,8 @@ describe API::SystemHooks do
     end
 
     it 'sets explicit values for events' do
+      stub_full_request('http://mep.mep', method: :post)
+
       post api('/hooks', admin),
         params: {
           url: 'http://mep.mep',
diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
index f4470b50753..75d534c59bf 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
+++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
@@ -2,6 +2,8 @@
 require 'spec_helper'
 
 describe Projects::LfsPointers::LfsDownloadService do
+  include StubRequests
+
   let(:project) { create(:project) }
   let(:lfs_content) { SecureRandom.random_bytes(10) }
   let(:oid) { Digest::SHA256.hexdigest(lfs_content) }
@@ -62,7 +64,7 @@ describe Projects::LfsPointers::LfsDownloadService do
   describe '#execute' do
     context 'when file download succeeds' do
       before do
-        WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+        stub_full_request(download_link).to_return(body: lfs_content)
       end
 
       it_behaves_like 'lfs object is created'
@@ -104,7 +106,7 @@ describe Projects::LfsPointers::LfsDownloadService do
       let(:size) { 1 }
 
       before do
-        WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+        stub_full_request(download_link).to_return(body: lfs_content)
       end
 
       it_behaves_like 'no lfs object is created'
@@ -118,7 +120,7 @@ describe Projects::LfsPointers::LfsDownloadService do
 
     context 'when downloaded lfs file has a different oid' do
       before do
-        WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+        stub_full_request(download_link).to_return(body: lfs_content)
         allow_any_instance_of(Digest::SHA256).to receive(:hexdigest).and_return('foobar')
       end
 
@@ -136,7 +138,7 @@ describe Projects::LfsPointers::LfsDownloadService do
       let(:lfs_object) { LfsDownloadObject.new(oid: oid, size: size, link: download_link_with_credentials) }
 
       before do
-        WebMock.stub_request(:get, download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content)
+        stub_full_request(download_link).with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNzd29yZA==' }).to_return(body: lfs_content)
       end
 
       it 'the request adds authorization headers' do
@@ -149,7 +151,7 @@ describe Projects::LfsPointers::LfsDownloadService do
       let(:local_request_setting) { true }
 
       before do
-        WebMock.stub_request(:get, download_link).to_return(body: lfs_content)
+        stub_full_request(download_link, ip_address: '192.168.2.120').to_return(body: lfs_content)
       end
 
       it_behaves_like 'lfs object is created'
@@ -173,7 +175,8 @@ describe Projects::LfsPointers::LfsDownloadService do
 
         with_them do
           before do
-            WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+            stub_full_request(download_link, ip_address: '192.168.2.120')
+              .to_return(status: 301, headers: { 'Location' => redirect_link })
           end
 
           it_behaves_like 'no lfs object is created'
@@ -184,8 +187,8 @@ describe Projects::LfsPointers::LfsDownloadService do
         let(:redirect_link) { "http://example.com/"}
 
         before do
-          WebMock.stub_request(:get, download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
-          WebMock.stub_request(:get, redirect_link).to_return(body: lfs_content)
+          stub_full_request(download_link).to_return(status: 301, headers: { 'Location' => redirect_link })
+          stub_full_request(redirect_link).to_return(body: lfs_content)
         end
 
         it_behaves_like 'lfs object is created'
diff --git a/spec/services/submit_usage_ping_service_spec.rb b/spec/services/submit_usage_ping_service_spec.rb
index 78df9bf96bf..653f17a4324 100644
--- a/spec/services/submit_usage_ping_service_spec.rb
+++ b/spec/services/submit_usage_ping_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe SubmitUsagePingService do
+  include StubRequests
+
   context 'when usage ping is disabled' do
     before do
       stub_application_setting(usage_ping_enabled: false)
@@ -99,7 +101,7 @@ describe SubmitUsagePingService do
   end
 
   def stub_response(body)
-    stub_request(:post, 'https://version.gitlab.com/usage_data')
+    stub_full_request('https://version.gitlab.com/usage_data', method: :post)
       .to_return(
         headers: { 'Content-Type' => 'application/json' },
         body: body.to_json
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index 75ba2479b63..37bafc0c002 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe WebHookService do
+  include StubRequests
+
   let(:project) { create(:project) }
   let(:project_hook) { create(:project_hook) }
   let(:headers) do
@@ -67,11 +69,11 @@ describe WebHookService do
       let(:project_hook) { create(:project_hook, url: 'https://demo:demo@example.org/') }
 
       it 'uses the credentials' do
-        WebMock.stub_request(:post, url)
+        stub_full_request(url, method: :post)
 
         service_instance.execute
 
-        expect(WebMock).to have_requested(:post, url).with(
+        expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
           headers: headers.merge('Authorization' => 'Basic ZGVtbzpkZW1v')
         ).once
       end
@@ -82,11 +84,11 @@ describe WebHookService do
       let(:project_hook) { create(:project_hook, url: 'https://demo@example.org/') }
 
       it 'uses the credentials anyways' do
-        WebMock.stub_request(:post, url)
+        stub_full_request(url, method: :post)
 
         service_instance.execute
 
-        expect(WebMock).to have_requested(:post, url).with(
+        expect(WebMock).to have_requested(:post, stubbed_hostname(url)).with(
           headers: headers.merge('Authorization' => 'Basic ZGVtbzo=')
         ).once
       end
diff --git a/spec/support/helpers/stub_requests.rb b/spec/support/helpers/stub_requests.rb
new file mode 100644
index 00000000000..5cad35282c0
--- /dev/null
+++ b/spec/support/helpers/stub_requests.rb
@@ -0,0 +1,40 @@
+module StubRequests
+  IP_ADDRESS_STUB = '8.8.8.9'.freeze
+
+  # Fully stubs a request using WebMock class. This class also
+  # stubs the IP address the URL is translated to (DNS lookup).
+  #
+  # It expects the final request to go to the `ip_address` instead the given url.
+  # That's primarily a DNS rebind attack prevention of Gitlab::HTTP
+  # (see: Gitlab::UrlBlocker).
+  #
+  def stub_full_request(url, ip_address: IP_ADDRESS_STUB, port: 80, method: :get)
+    stub_dns(url, ip_address: ip_address, port: port)
+
+    url = stubbed_hostname(url, hostname: ip_address)
+    WebMock.stub_request(method, url)
+  end
+
+  def stub_dns(url, ip_address:, port: 80)
+    url = parse_url(url)
+    socket = Socket.sockaddr_in(port, ip_address)
+    addr = Addrinfo.new(socket)
+
+    # See Gitlab::UrlBlocker
+    allow(Addrinfo).to receive(:getaddrinfo)
+                         .with(url.hostname, url.port, nil, :STREAM)
+                         .and_return([addr])
+  end
+
+  def stubbed_hostname(url, hostname: IP_ADDRESS_STUB)
+    url = parse_url(url)
+    url.hostname = hostname
+    url.to_s
+  end
+
+  private
+
+  def parse_url(url)
+    url.is_a?(URI) ? url : URI(url)
+  end
+end