igrigorik / em-http-request

Asynchronous HTTP Client (EventMachine + Ruby)
1.22k stars 220 forks source link

`verify_peer` is incompatible with using the `proxy` configuration #349

Open jpcamara opened 3 years ago

jpcamara commented 3 years ago

Hi there! Wanted to start off by saying thanks for the gem! I've been using it successfully for years.

With the recent TLS verification patch, a bug seems to have been introduced when combining tls: { verify_peer: true } with a proxy configuration. For example (I don't know of a free proxy I could use, but this would be the setup and you just need to provide your own proxy credentials):

EventMachine.run do
  proxy_host = "proxy.host"
  proxy_port = 123
  proxy_username = ""
  proxy_password = ""

  url = "https://www.google.com"
  opts = {
    tls: { verify_peer: true },
    proxy: {
      host: proxy_host,
      port: proxy_port,
      authorization: [proxy_username, proxy_password]
    }
  }

  ::EventMachine::HttpRequest.new(url, opts).get
end

☝🏼 That code will result in the following error (where proxy.host is whatever your actual proxy host is):

OpenSSL::SSL::SSLError: host "proxy.host" does not match the server certificate
from .../em-http-request-1.1.7/lib/em-http/http_connection.rb:73:in `ssl_handshake_completed'

The problem is in the ssl_handshake_completed method:

def ssl_handshake_completed
  ...
  unless OpenSSL::SSL.verify_certificate_identity(@last_seen_cert, host)
    raise OpenSSL::SSL::SSLError.new(%(host "#{host}" does not match the server certificate))
  ...
end

In the HttpStubConnection, when a proxy opt is set, the host is the host of the proxy (so for our example, proxy.host), not the host of the target url we're proxying to (google.com).

All of the certs identified before this call have been collected from the target (the cert_string is from google.com):

def ssl_verify_peer(cert_string)
  cert = nil
  begin
    cert = OpenSSL::X509::Certificate.new(cert_string)
  rescue OpenSSL::X509::CertificateError
    return false
  end

  @last_seen_cert = cert

So the does not match the server certificate error occurs.

I was able to fix it locally by changing the host check to take the uri from the parent, which has the target url (URI.parse(parent.uri).host):

def ssl_handshake_completed
  ...
  unless OpenSSL::SSL.verify_certificate_identity(@last_seen_cert, URI.parse(parent.uri).host)

This is obviously not ideal, and almost certainly not a solution that would land in the actual gem. But I tried dumping the contents of the stub connection and the parent, and it was the only option I could identify as being currently exposed. All other available options seem to be pointed at the proxy, when the proxy is enabled.

Thanks for you time!