Skip to content

Update to net-http v0.4.1 and patch Net::HTTP#connect to fix SSRF protection

Stan Hu requested to merge sh-upgrade-net-http into master

What does this MR do and why?

Our DNS rebinding protection works like this:

  1. Given a hostname (e.g. gitlab.com), resolve the IP address.
  2. Validate the IP address isn't some banned address (e.g. the EC2 metadata endpoint).
  3. In Net::HTTP and the SSL::SSLSocket, store the hostname_override with the actual hostname.
  4. Make an Net::HTTP#connect call with the IP address.
  5. When Net::HTTP#connect sets up the Server Name Indication (SNI), use hostname_override to set the hostname so that the IP address isn't used there.
  6. Do something similar with SSL::SSLSocket#post_connection_check.

However, DNS rebinding protection stopped worked in net-http v0.2.2 due to https://github.com/ruby/net-http/pull/36. That change disables SNI if an IP address is used, but the whole point of DNS rebinding protection is to pass in an validated IP address. I created an upstream issue in https://github.com/ruby/net-http/issues/141 to provide first-class support for SSRF protection, but there's been no movement on this. A refactor in https://github.com/ruby/net-http/pull/172 still has not been reviewed.

This merge request does a number of things:

  1. Upgrades net-http to v0.4.1 and deletes patches that are no longer needed with the upgrade.
  2. Vendors the Net:HTTP#connect code and patches https://github.com/ruby/net-http/pull/36 to use the hostname_override as the address to verify. See e2c04629 for the diff.
  3. Adds a test to ensure that HTTPS and SNI works.

Relates to:

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. In /admin/application_settings/network, make sure DNS rebinding protection is enabled:

image

  1. Without the patch (git checkout 199e7602784c0e3e329a0e7f18be22967522803b), bin/rails console will fail:
[1] pry(main)> Gitlab::HTTP.get('https://gitlab.com')
OpenSSL::SSL::SSLError: SSL_connect returned=1 errno=0 peeraddr=172.65.251.78:443 state=error: ssl/tls alert handshake failure
from /Users/stanhu/.asdf/installs/ruby/3.2.3/lib/ruby/gems/3.2.0/gems/net-protocol-0.1.3/lib/net/protocol.rb:46:in `connect_nonblock'
  1. With this patch, this should succeed:
[2] pry(main)> Gitlab::HTTP.get('https://gitlab.com')[0..100]
=> "<!doctype html>\n<html data-n-head-ssr lang=\"en-us\" data-n-head=\"%7B%22lang%22:%7B%22ssr%22:%22en-us%2"
  1. Now let's verify that DNS rebind protection still works. Disable access to all localhost addresses:

image

  1. Wait a minute, and now repeat with localhost. This should be blocked:
[3] pry(main)> Gitlab::HTTP.get('http://localhost')
Gitlab::HTTP_V2::BlockedUrlError: URL is blocked: Requests to localhost are not allowed
from /Users/stanhu/gdk-ee/gitlab/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb:65:in `rescue in validate_url_with_proxy!'
Caused by Gitlab::HTTP_V2::UrlBlocker::BlockedUrlError: Requests to localhost are not allowed
from /Users/stanhu/gdk-ee/gitlab/gems/gitlab-http/lib/gitlab/http_v2/url_blocker.rb:329:in `validate_localhost'
Edited by Stan Hu

Merge request reports