diff options
author | Douwe Maan <douwe@selenight.nl> | 2019-04-21 13:03:26 +0300 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2019-05-30 16:47:31 +0300 |
commit | a9bcddee4c2653cbf2254d893299393e3778e7df (patch) | |
tree | 0c81c5358bce244da7cf9f9f684234a7f4a2dfd0 /lib/gitlab | |
parent | 88241108c4d9807e5c312b11c910b3072bc6f120 (diff) |
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.
Diffstat (limited to 'lib/gitlab')
-rw-r--r-- | lib/gitlab/http.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/http_connection_adapter.rb (renamed from lib/gitlab/proxy_http_connection_adapter.rb) | 17 | ||||
-rw-r--r-- | lib/gitlab/url_blocker.rb | 61 |
3 files changed, 58 insertions, 22 deletions
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 |