Skip to content

Skip IP lookups in validating URLs on certain conditions

Stan Hu requested to merge sh-refactor-gitlab-http-validate into master

What does this MR do and why?

Previously anytime ApplicationSetting were updated UrlBlocker would attempt to resolve the IPs and determine whether they were allowed by the current settings. However, in an offline environment, services like Diagrams.net may not be resolved even if they are enabled by default.

This commit skips the validations that require resolving IP addresses if there are no restrictions on outgoing requests:

  • Allow requests to the local network is checked
  • Allow requests to the local network from system hooks is checked
  • DNS rebinding attack protection is disabled
  • Block all requests, except for IP addresses, IP ranges, and domain names defined in the allowlist (deny_all_requests_except_allowed) is disabled

Note that the URL validators in ApplicationSetting only pass in the current deny_all_requests_except_allowed setting, so if that is active then IP resolution will occur.

Relates to #467524 (closed)

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

How to set up and validate locally

  1. I applied this diff to check my sanity:
diff --git a/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb
index 11d92d74e34e..63af6208f797 100644
--- a/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb
+++ b/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb
@@ -82,6 +82,7 @@ def validate_url_with_proxy!(
           )
 
           unless deny_all_requests_except_allowed || dns_rebind_protection || !allow_local_network || !allow_localhost
+            STDERR.puts "=== skipping IP lookups for #{url}"
             return Result.new(uri, nil, true)
           end
  1. Then after gdk restart rails-web, I attempted to update an arbitrary setting in /admin -> Settings`.
  2. gdk tail rails-web showed this:
2024-06-17_17:49:21.82864 rails-web                        : === skipping IP validation for https://embed.diagrams.net
2024-06-17_17:49:21.84411 rails-web                        : === skipping IP validation for https://REDACTED@new-sentry.gitlab.net/31
2024-06-17_17:49:21.84418 rails-web                        : === skipping IP validation for https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-runner/releases
Edited by Stan Hu

Merge request reports