HTTP Agent using Host header for SNI server_name

Nevon commented 3 years ago

Version: v14.15.1`
Platform: Darwin C02DFH5SMD6M 19.6.0 Darwin Kernel Version 19.6.0: Tue Nov 10 00:10:30 PST 2020; root:xnu-6153.141.10~1/RELEASE_X86_64 x86_64
Subsystem: http

What steps will reproduce the bug?

I'm not 100% sure that this is a bug, but it was surprising behavior that is different from any other http clients I've looked at (curl, python's httplib, nginx).

If you make an HTTPS request specifying a host header that is different from the request host, the servername used for SNI will be set to the value of the host header, unless the host header contains an IP address or you explicitly pass a servername in the agent options.

https://github.com/nodejs/node/blob/b0c0111b04201bf99fde0fe0616b9fdf1f33655d/lib/http.js#L1086-L1092

What is the expected behavior?

This was very surprising to me as the host header is part of the HTTP layer, and the servername belongs to the TLS layer, and I have never seen a client behave this way. I would have expected the host (not the host header) to be used as the default servername.

For example using curl:

$ curl -vvI https://www.google.com -H "Host: https://not-google.com"
*   Trying 172.217.21.164...
* TCP_NODELAY set
* Connected to www.google.com (172.217.21.164) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/cert.pem
  CApath: none
* TLSv1.2 (OUT), TLS handshake, Client hello (1):
* TLSv1.2 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-ECDSA-CHACHA20-POLY1305
* ALPN, server accepted to use h2
* Server certificate:
*  subject: C=US; ST=California; L=Mountain View; O=Google LLC; CN=www.google.com
*  start date: Jan  5 12:13:00 2021 GMT
*  expire date: Mar 30 12:12:59 2021 GMT
*  subjectAltName: host "www.google.com" matched cert's "www.google.com"
*  issuer: C=US; O=Google Trust Services; CN=GTS CA 1O1
*  SSL certificate verify ok.
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* Using Stream ID: 1 (easy handle 0x7fecd500f600)
> HEAD / HTTP/2
> Host: https://not-google.com
> User-Agent: curl/7.64.1
> Accept: */*
>

As you can see, the certificate is accepted:

subjectAltName: host "www.google.com" matched cert's "www.google.com"

If we do the same thing in Node, the certificate will be rejected as the certificate host name does not match the server name that we send:

const https = require('https');
const options = {
  headers: {
    host: 'https://not-google.com'
  }
};

https.get('https://google.com', options, (res) => {}).on('error', console.error);

This will result in [ERR_TLS_CERT_ALTNAME_INVALID]: Hostname/IP does not match certificate's altnames.

Changing this so that the host is used as the default servername instead of the host header is a small and simple change, but my question would be if this is done intentionally, and if so, why?

Additional information

I was not able to find any reference in the git history of why this decision was made - only that it was made back in 2012.
Having a mismatched servername and host header is domain fronting, but it doesn't seem like something the HTTP client should be responsible for preventing. And the current design still allows for it by just setting the servername yourself.

schamberg97 commented 3 years ago

The only reason I can think of is that this approach allows you to lazily query the server by its IP to avoid DNS resolution, without defining host and servername separately. Which sounds like a bit of a bad idea to me, tbh

dnlup commented 3 years ago

SNI should be the conceptual equivalent of virtual hosting for HTTPS, so I imagine the reason could be for providing that behavior. I can't say if it's right or wrong, though.

xuemengfei commented 3 years ago

https://github.com/nodejs/node/issues/22389 Found an issue that has been discussed since 2018.

nodejs / node