search

delvedor / hpagent

A ready to use http and https agent for working with proxies that keeps connections alive!
MIT License
182 stars 36 forks source link

Fix TLS bug preventing HTTPS-over-HTTP(S) proxying #51

Closed freeqaz closed 2 years ago

freeqaz commented 2 years ago

Without this fix, you see this error when attempting to use an HTTPS proxy.

Error [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames: Host: foo.com. is not in the cert's altnames: DNS:*.bar.com, DNS:bar.com

The servername attribute is able to fix this behavior when passed to the underlying tls.connect() call inside of the https module. (see the Node docs)

This was very painful to debug but now I understand how CONNECT works extremely well! Cheers.

FIxes #43

ckcr4lyf commented 2 years ago

This is essential as otherwise the requests hostname is attempted to be matched with the proxy's cert.

+1 from me thanks

ckcr4lyf commented 2 years ago

If they don't do it, do you think it's possible for me to checkout their branch into another, add a unit test, and make another MR?

While still having @freeqaz as an author / contributor? (I can't commit to their branch obviously)

delvedor commented 2 years ago

@ckcr4lyf yes, it should be possible.

arnaudruffin commented 2 years ago

I had the same issue and I can confirm that this fix works perfectly ! Thanks @freeqaz. While waiting for this PR to be merged il will just copy/paste your code with the right mentions...

ckcr4lyf commented 2 years ago

I am working on the unittest for this. Just to explain a bit more on Free's fix, I setup the unit test with two domains, one for the server (HTTP target) - server.unit-test.com , one for the proxy - proxy-domain.net .

When I comment out Free's fix, the SSL-over-SSL test sends the SNI for server.unit-test.com in TLS handshake with proxy: image

The failure error is: (note: disabled the TLS_REJECT env var, otherwise this cant be tested!)

    code: 'ERR_TLS_CERT_ALTNAME_INVALID',
    host: 'server.unit-test.com',
    reason: 'Host: server.unit-test.com. is not cert\'s CN: proxy-domain.net',
    message: 'Hostname/IP does not match certificate\'s altnames: Host: server.unit-test.com. is not cert\'s CN: proxy-domain.net',

But with the fix, it will correctly use the proxy's hostname in the SNI header for the initial handshake with the proxy: image

adding the unit test is a bit complicated, hopefully you guys wont mind reviewing it :smile:

ckcr4lyf commented 2 years ago

@delvedor I have added unit tests (and changed quite a bit...) in a new MR, building on Free's work -> https://github.com/delvedor/hpagent/pull/65

delvedor commented 2 years ago

Closed by https://github.com/delvedor/hpagent/pull/65.