Set TLS SNI hostname to be the same as Host field in header

[wen-long]

In http, Host field is correctly set from struct requestHeader, while https also need this to verify server cert, SNI hostname also be used in TLS handshake client hello to tell server which host to connect

SNI hostname don't contains port, so it should be either hostNoPort or Host field from requestHeader

[ghost]

See https://play.golang.org/p/1E8omJr8N5_U. The net/http package uses the host from the request URL to verify the server cert, not the host from the request header. This package should do the same thing.

If someone should decide that this change is ok, then the PR should include a test that fails without change. Also, I think the code will be clearer if the name is set from r.Host:

if cfg.ServerName == "" {
                     h, _, err := net.SplitHostPort(.rHost)
                     if err != nil {
                           return nil, nil, err
                     }
		cfg.ServerName = h
	}

[wen-long]

I did a test before make this PR, just Dial 1.1.1.1(any cloudflare IP) with a http.Header contains correct domain,it fails with below

dial:x509: certificate is valid for *.cloudflare-dns.com, cloudflare-dns.com, not 1.1.1.1:443

@stevenscott89 your code approved my PR. Pls check https://play.golang.org/p/iZy0uY-McX4 I added some Println,the result is below

r.Host	r.URL.Host
127.0.0.1:2	127.0.0.1:2	fails
badhost.com	127.0.0.1:2	works

we see that only if r.Host is domain it goes right.

some code like

	u := url.URL{Scheme: "https", Host: "1.1.1.1", Path: "/echo"}
	var head http.Header
	if obfsHost != nil {
		head = http.Header{
			"Host": []string{"example.com"},
		}
	}
	log.Println(u.String(), head)
	ws, _, err := websocket.DefaultDialer.Dial(u.String(), head)

will failed baseuse old code doesn't use example.com in TLS connect.

Setting Host field in header just like

1. self DNS resolve. change hosts file or build own dnsmasq and add any record
2. like --resolve option in curl curl -I -k https://www.cloudflare.com/ --resolve "www.cloudflare.com:443:1.1.1.1" -v and this will work(cause cloudflare uses cloudflare)

In conclusion, hostname in URL is used to build TCP connection,But Host in Header settings is to be used by http headers and https SNI hostname. if Host is setted to some string, we should use it both in http and https.

BYW hostname used to build TCP connection and Host field in http head, https sni hostname is definitely needs to be stored as TWO variable

[elithrar]

To be precise: - The ServerName field (as part of the SNI extension to TLS) does not allow IP address literals - https://tools.ietf.org/html/rfc6066 (pg. 6) - "Hostnames" must be DNS-valid names - There still must be a test as part of this PR that validates hostnames (pass), hostname:port (fail) and IP literals (fail)

…

On Mon, Sep 17, 2018 at 10:17 AM wen-long ***@***.***> wrote: I did a test before make this PR, just Dial 1.1.1.1(any cloudflare IP) with a http.Header contains correct domain,it fails with below dial:x509: certificate is valid for *.cloudflare-dns.com, cloudflare-dns.com, not 1.1.1.1:443 @stevenscott89 <https://github.com/stevenscott89> your code approved my PR. Pls check https://play.golang.org/p/iZy0uY-McX4 I added some Println,the result is below r.Host r.URL.Host 127.0.0.1:2 127.0.0.1:2 fails badhost.com 127.0.0.1:2 works we see that only if r.Host is domain it goes right. some code like u := url.URL{Scheme: "https", Host: "1.1.1.1", Path: "/echo"} var head http.Header if obfsHost != nil { head = http.Header{ "Host": []string{"example.com"}, } } log.Println(u.String(), head) ws, _, err := websocket.DefaultDialer.Dial(u.String(), head) will failed baseuse old code doesn't use example.com in TLS connect. Setting Host field in header just like 1. self DNS resolve. change hosts file or build own dnsmasq and add any record 2. like --resolve option in curl curl -I -k https://www.cloudflare.com/ --resolve "www.cloudflare.com:443:1.1.1.1" -v and this will work(cause cloudflare uses cloudflare) In conclusion, hostname in URL is used to build TCP connection,But Host in Header settings is to be used by http headers and https SNI hostname. if Host is setted to some string, we should use it both in http and https. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABIcFulpMDC6IMJGao9fCCJQ1A3bbnGks5ub9kbgaJpZM4Wrc-Q> .

[ghost]

Here's an improved example: https://play.golang.org/p/YI8QJUEP1Nv

r.Host	r.URL.Host	Dialed address	Host header on server	TLS verify
example.com	example.com	example.com:443	example.com	ok
127.0.0.1:2	127.0.0.1:2	127.0.0.1:2	127.0.0.1:2	ok
badhost.com	example.com	example.com:443	badhost.com	ok
bahdhost.com	127.0.0.1:2	127.0.0.1:2	badhost.com	ok
example.com	badhost.com	badhost.com:443		fails

The net/http client does not use r.Host (which is copied to the host header) to verify the server name or to dial the connection.

A dial function is the equivalent of curl --resolve, not setting the host header.

[wen-long]

why not try use ip in url？ its a matter of priority between host in url and header. if url host is ip, net/http uses r.Host, but old code dont.it mistakes to use ip instead of looking for Host in header Steven Scott <notifications@github.com> 于 2018年9月18日周二 02:04写道：

…

Here's an improved example: https://play.golang.org/p/ayWNGYGolvu r.Host r.URL.Host Dialed address Host header on server example.com example.com example.com:443 example.com works badhost.com example.com example.com:443 badhost.com works example.com badhost.com badhost.com:443 fails The net/http client does not use r.Host (which is copied to the host header) to verify the server name or to dial the connection. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#428 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEEJ_vgdEX26uD5dJtYUtInoMITAegCHks5ub-QygaJpZM4Wrc-Q> .

[ghost]

if url host is ip, net/http uses r.Host

I cannot observe this using a blackbox test, nor can I find code in net/http that does this. Perhaps I missed the code. I find the net/http transport code to be somewhat complex.

In any case, this package should remain consistent with net/http.

[ghost]

See the proposed new test for host names in PR #429. The test in #429 locks in current host name handling and confirms that the websocket client matches the net/http client.

The current PR fails the new test added #429.

Use a dial function to dial the cloudfare IP address.

[wen-long]

Maybe the right way is to set TLSClientConfig.ServerName in dialer, before call Dial, instead of letting inner mechanism infer the right servername.

Learn a lot from this discuss, thank you
gonna close this