TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
873 stars 228 forks source link

ERR_TLS_CERT_ALTNAME_INVALID when attempting to connect to proxy that has a certificate with only 127.0.0.1 as CN/SAN #131

Closed mbargiel closed 1 year ago

mbargiel commented 2 years ago

I have a setup where I'm testing various proxy configurations using a custom self-signed certificate authority to test "valid" user configurations. I pass the custom ca root to my tests using the TLS request option ca. Recently I ran into an issue where it's not possible to connect to a proxy server over HTTPS despite having set ca.

Repro steps: 1) Install mitmproxy (e.g. brew install mitmproxy) 2) In a terminal, run mitmdump with default configuration 3) In another terminal, inspect the mitmproxy-generated certificate with openssl by running the following command: openssl s_client -connect localhost:8080 </dev/null 2>/dev/null | openssl x509 -noout -text --> Observe the certificate only declares Subject: CN=127.0.0.1 and X509v3 Subject Alternative Name: IP Address:127.0.0.1. 4) Attempt to use https-proxy-agent with https_proxy=https://localhost:8080. --> Observe failure (ERR_TLS_CERT_ALTNAME_INVALID...)

I see the code handling the upstream TLS connection uses opts.host as a fallback in case opts.servername is not provided (https://github.com/TooTallNate/node-https-proxy-agent/blob/master/src/agent.ts#L141), but such fallback logic is not implemented for the proxy host connection itself. Would you take a PR to implement the same kind of logic around the proxy tls.connect call itself (https://github.com/TooTallNate/node-https-proxy-agent/blob/master/src/agent.ts#L95)?

mbargiel commented 2 years ago

I think it would be as simple as

        if (secureProxy) {
            debug('Creating `tls.Socket`: %o', proxy);
-           socket = tls.connect(proxy as tls.ConnectionOptions);
+           const servername = proxy.servername || proxy.host; 
+           socket = tls.connect({ ...proxy, servername } as tls.ConnectionOptions);
        } else {
TooTallNate commented 1 year ago

This module has gone through a large refactor and modernization. I am closing this issue as a bit of house cleaning. If you feel that this issue still exists in the latest release, feel free to open a new issue.