Skip DNS rebinding protection when HTTP_PROXY environment is set
Related to: Importing a github project requires DNS resolut... (#37941 - closed) and Importing a github project requires DNS resolut... (#37941 - closed)
GitHub Import documentation has the following note:
If GitLab is behind a HTTP/HTTPS proxy, you must populate the allowlist for local requests with
github.com
andapi.github.com
to solve the hostname. For more information, read the issue Importing a GitHub project requires DNS resolution even when behind a proxy.
According to what was discussed in #37941 (closed), the issue occurs in a situation where the GitLab Self-Managed instance is installed in a server that is configured not to resolve domain names as all network requests are supposed to go via a proxy.
However, GitHub Import doesn't respect this configuration and attempts to resolve GitHub.com DNS, which results in an error since the Self Managed instance can't resolve DNS.
As a workaround, the GitHub Import documentation instructs the user to add Github.com
to the allowlist for local requests; however, the correct solution is to prevent GitHub Import from trying to resolve the DNS as in such configuration should be handled via the proxy.
Solution
GitHub Import tries to resolve GitHub's IP in lib/gitlab/octokit/middleware.rb as part of the DNS rebind protection. However, when the server can't resolve the domain's IP, we could skip the DNS protection
diff --git a/lib/gitlab/octokit/middleware.rb b/lib/gitlab/octokit/middleware.rb
index a3c0fdcf467..43aee303f22 100644
--- a/lib/gitlab/octokit/middleware.rb
+++ b/lib/gitlab/octokit/middleware.rb
@@ -8,7 +8,11 @@ def initialize(app)
end
def call(env)
- Gitlab::UrlBlocker.validate!(env[:url], allow_localhost: allow_local_requests?, allow_local_network: allow_local_requests?)
+ Gitlab::UrlBlocker.validate!(env[:url],
+ allow_localhost: allow_local_requests?,
+ allow_local_network: allow_local_requests?,
+ dns_rebind_protection: dns_rebind_protection?
+ )
@app.call(env)
end
@@ -18,6 +22,12 @@ def call(env)
def allow_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
+
+ def dns_rebind_protection?
+ Gitlab::CurrentSettings.dns_rebinding_protection_enabled?
+ end
end
end
end
diff --git a/lib/gitlab/http_connection_adapter.rb b/lib/gitlab/http_connection_adapter.rb
index 7b1657d3854..aa027acf25c 100644
--- a/lib/gitlab/http_connection_adapter.rb
+++ b/lib/gitlab/http_connection_adapter.rb
@@ -58,8 +58,6 @@ def allow_object_storage?
end
def dns_rebind_protection?
- return false if Gitlab.http_proxy_env?
-
Gitlab::CurrentSettings.dns_rebinding_protection_enabled?
end
diff --git a/lib/gitlab/url_blocker.rb b/lib/gitlab/url_blocker.rb
index 1e447923a39..c6c8ea8540a 100644
--- a/lib/gitlab/url_blocker.rb
+++ b/lib/gitlab/url_blocker.rb
@@ -119,8 +119,8 @@ def get_address_info(uri, dns_rebind_protection)
end
rescue SocketError
# If the dns rebinding protection is not enabled or the domain
- # is allowed we avoid the dns rebinding checks
- return if domain_allowed?(uri) || !dns_rebind_protection
+ # is allowed, or HTTP_PROXY is set we avoid the dns rebinding checks
+ return if domain_allowed?(uri) || !dns_rebind_protection || Gitlab.http_proxy_env?
# In the test suite we use a lot of mocked urls that are either invalid or
# don't exist. In order to avoid modifying a ton of tests and factories
Note
-
After applying this change, we can update the documentation GitHub Import's documentation saying that adding Github.com to the allowlist for local requests is no longer necessary after milestone X.X. -
Also, we should include in the HTTP_PROXY environment's documentation a note explaining that the protection is skipped if the DNS can't be resolved and the environment is set -
Also adding a security warning to https://docs.gitlab.com/omnibus/settings/environment-variables.html could be a good idea too. "If you use a proxy, DNS rebind protection will be disabled".