summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-06-03 12:33:59 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-06-03 12:33:59 +0000
commit3a7bf68e34b493870146fa026c9a3da1899ef779 (patch)
treef083f08a2571f941fc3b91a0b11d9dbbdfec8464
parentc45c64ce298fab6eca6c54142ab5844a4b2c5c63 (diff)
parenta1a0f8e6b017f57060bc94d14fd4d37d8756e47d (diff)
downloadgitlab-ce-3a7bf68e34b493870146fa026c9a3da1899ef779.tar.gz
Merge branch 'dm-http-hostname-override' into 'master'
Protect Gitlab::HTTP against DNS rebinding attack See merge request gitlab/gitlabhq!3071
-rw-r--r--app/helpers/application_settings_helper.rb1
-rw-r--r--app/models/application_setting_implementation.rb1
-rw-r--r--app/views/admin/application_settings/_outbound.html.haml8
-rw-r--r--changelogs/unreleased/dm-http-hostname-override.yml5
-rw-r--r--config/initializers/hipchat_client_patch.rb6
-rw-r--r--config/initializers/http_hostname_override.rb49
-rw-r--r--db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb23
-rw-r--r--db/schema.rb3
-rw-r--r--lib/gitlab.rb5
-rw-r--r--lib/gitlab/http.rb2
-rw-r--r--lib/gitlab/http_connection_adapter.rb (renamed from lib/gitlab/proxy_http_connection_adapter.rb)24
-rw-r--r--lib/gitlab/url_blocker.rb75
-rw-r--r--locale/gitlab.pot6
-rw-r--r--spec/controllers/projects/ci/lints_controller_spec.rb4
-rw-r--r--spec/features/admin/admin_settings_spec.rb5
-rw-r--r--spec/lib/gitlab/ci/config/external/file/remote_spec.rb14
-rw-r--r--spec/lib/gitlab/ci/config/external/mapper_spec.rb4
-rw-r--r--spec/lib/gitlab/ci/config/external/processor_spec.rb20
-rw-r--r--spec/lib/gitlab/ci/config_spec.rb5
-rw-r--r--spec/lib/gitlab/ci/yaml_processor_spec.rb4
-rw-r--r--spec/lib/gitlab/http_connection_adapter_spec.rb120
-rw-r--r--spec/lib/gitlab/http_spec.rb28
-rw-r--r--spec/lib/gitlab/import_export/after_export_strategies/web_upload_strategy_spec.rb4
-rw-r--r--spec/lib/gitlab/url_blocker_spec.rb83
-rw-r--r--spec/lib/gitlab_spec.rb30
-rw-r--r--spec/lib/mattermost/session_spec.rb7
-rw-r--r--spec/models/project_services/assembla_service_spec.rb6
-rw-r--r--spec/models/project_services/bamboo_service_spec.rb3
-rw-r--r--spec/models/project_services/buildkite_service_spec.rb10
-rw-r--r--spec/models/project_services/campfire_service_spec.rb24
-rw-r--r--spec/models/project_services/pivotaltracker_service_spec.rb10
-rw-r--r--spec/models/project_services/pushover_service_spec.rb6
-rw-r--r--spec/models/project_services/teamcity_service_spec.rb3
-rw-r--r--spec/requests/api/system_hooks_spec.rb8
-rw-r--r--spec/services/projects/lfs_pointers/lfs_download_service_spec.rb19
-rw-r--r--spec/services/submit_usage_ping_service_spec.rb4
-rw-r--r--spec/services/web_hook_service_spec.rb10
-rw-r--r--spec/support/helpers/stub_requests.rb40
38 files changed, 587 insertions, 92 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 971d1052824..4469118f065 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -160,6 +160,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,
diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index e51619b0f9c..904d650ef96 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -21,6 +21,7 @@ module ApplicationSettingImplementation
after_sign_up_text: nil,
akismet_enabled: false,
allow_local_requests_from_hooks_and_services: false,
+ dns_rebinding_protection_enabled: true,
authorized_keys_enabled: true, # TODO default to false if the instance is configured to use AuthorizedKeysCommand
container_registry_token_expire_delay: 5,
default_artifacts_expire_in: '30 days',
diff --git a/app/views/admin/application_settings/_outbound.html.haml b/app/views/admin/application_settings/_outbound.html.haml
index f4bfb5af385..dd56bb99a06 100644
--- a/app/views/admin/application_settings/_outbound.html.haml
+++ b/app/views/admin/application_settings/_outbound.html.haml
@@ -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"
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/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb b/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb
new file mode 100644
index 00000000000..8835dc8b7ba
--- /dev/null
+++ b/db/migrate/20190529142545_add_dns_rebinding_protection_enabled_to_application_settings.rb
@@ -0,0 +1,23 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index bb59af540fe..9facc1c4bb2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20190524062810) do
+ActiveRecord::Schema.define(version: 20190529142545) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -193,6 +193,7 @@ ActiveRecord::Schema.define(version: 20190524062810) do
t.integer "elasticsearch_replicas", default: 1, null: false
t.text "encrypted_lets_encrypt_private_key"
t.text "encrypted_lets_encrypt_private_key_iv"
+ 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
diff --git a/lib/gitlab.rb b/lib/gitlab.rb
index 3f107fbbf3b..ccaf06c5d6a 100644
--- a/lib/gitlab.rb
+++ b/lib/gitlab.rb
@@ -40,6 +40,7 @@ module Gitlab
SUBDOMAIN_REGEX = %r{\Ahttps://[a-z0-9]+\.gitlab\.com\z}.freeze
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
@@ -66,6 +67,10 @@ module Gitlab
end
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)
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..41eab3658bc 100644
--- a/lib/gitlab/proxy_http_connection_adapter.rb
+++ b/lib/gitlab/http_connection_adapter.rb
@@ -10,17 +10,19 @@
#
# 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?,
+ dns_rebind_protection: dns_rebind_protection?)
+ 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
@@ -29,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
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 641ba70ef83..9a8df719827 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -8,38 +8,68 @@ 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>].
+ # 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
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
+ 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 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
- true
+ protected_uri_with_hostname
end
def blocked_url?(*args)
@@ -52,6 +82,25 @@ 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, dns_rebind_protection)
+ address = addrs_info.first
+ ip_address = address&.ip_address
+
+ return [uri, nil] unless dns_rebind_protection && ip_address && ip_address != hostname
+
+ uri = uri.dup
+ uri.hostname = ip_address
+ [uri, hostname]
+ end
+
def get_port(uri)
uri.port || uri.default_port
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 0d47c574120..b550553f303 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3775,6 +3775,9 @@ msgstr ""
msgid "Ends at (UTC)"
msgstr ""
+msgid "Enforce DNS rebinding attack protection"
+msgstr ""
+
msgid "Enter at least three characters to search"
msgstr ""
@@ -8286,6 +8289,9 @@ msgstr ""
msgid "Resolved by %{resolvedByName}"
msgstr ""
+msgid "Resolves IP addresses once and uses them to submit requests"
+msgstr ""
+
msgid "Response metrics (AWS ELB)"
msgstr ""
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/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb
index f9950b5b03f..c4dbe23f6b4 100644
--- a/spec/features/admin/admin_settings_spec.rb
+++ b/spec/features/admin/admin_settings_spec.rb
@@ -332,16 +332,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
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..930d1f62272
--- /dev/null
+++ b/spec/lib/gitlab/http_connection_adapter_spec.rb
@@ -0,0 +1,120 @@
+# 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 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')
+
+ 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..253366e0789 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -2,6 +2,87 @@
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
+
+ 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
let(:ports) { Project::VALID_IMPORT_PORTS }
@@ -208,7 +289,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/gitlab_spec.rb b/spec/lib/gitlab_spec.rb
index 767b5779a79..e075904b0cc 100644
--- a/spec/lib/gitlab_spec.rb
+++ b/spec/lib/gitlab_spec.rb
@@ -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
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