Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: SNI in TLS Handshake should match Request.Host #22704

Closed
cavaliercoder opened this issue Nov 14, 2017 · 10 comments
Closed

net/http: SNI in TLS Handshake should match Request.Host #22704

cavaliercoder opened this issue Nov 14, 2017 · 10 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@cavaliercoder
Copy link

In net/http, setting Request.Host successfully sets the HTTP Host header in a client request, instructing the remote server which virtual host config to use to service the request. For TLS connections however, the host component of the request URL is exclusively used to set the SNI in the TLS Handshake.

If Request.Url.Hostname() does not match Request.Host, connections to a SNI enabled server will likely fail. The server will likely reject the TLS handshake with unrecognized_name(112) (see: https://tools.ietf.org/html/rfc4366#section-4).

To fix this issue, the SNI of the TLS handshake must match Request.Host.

I have been able to force the desired behavior with the following code - though this requires a discrete Transport for each target virtual host.

// request sent to '127.0.0.1:443'
req, _ := http.NewRequest("GET", "https://127.0.0.1/example", nil)

// virtual host set to 'example.com'
req.Host =  "example.com"

// SNI set to 'example.com'
client := http.Client{
	Transport: &http.Transport{
		TLSClientConfig: &tls.Config{
			ServerName:         req.Host, // here
		},
	},
}

client.Do(req)

Side issue: The unrecognized_name(112) TLS error could be added to crypto/tls/alert.go?

Are there any risks I have not considered for this change? Is there an idiomatic solution I haven't considered? Thoughts welcomed! Thanks.

@odeke-em
Copy link
Member

/cc @agl @bradfitz

@bradfitz
Copy link
Contributor

@cavaliercoder, the https://golang.org/pkg/net/http/#Request.URL docs say:

        // For client requests, the URL's Host specifies the server to
        // connect to, while the Request's Host field optionally
        // specifies the Host header value to send in the HTTP
        // request.
        URL *url.URL
...
        // For client requests Host optionally overrides the Host
        // header to send. If empty, the Request.Write method uses
        // the value of URL.Host. Host may contain an international
        // domain name.
        Host string

Have you tried setting URL.Host instead of Host?

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 14, 2017
@cavaliercoder
Copy link
Author

Yes. The request connects to the correct server, but the SNI and Host header are incorrect and the server does not know which certificate and virtual host to serve.

Setting Request.Host solves half the issue: telling the remove server which virtual host to serve. There's a second half to the problem: the SNI also needs to be set during TLS handshake so the remote server knows which certificate to serve.

@bradfitz
Copy link
Contributor

What are you trying to do?

I know how SNI works and how net/http works, but I don't know what the use case is here. Nobody else has complained in 8 years or so about the behavior. The closest we've ever had is #14404.

@cavaliercoder
Copy link
Author

I'm writing a tool to stress test and unit test web servers. I can edit /etc/hosts to direct the tests where they belong, but this quickly breaks down when we want to run the same tests against multiple servers from a CI server. It also sucks when testing our CDN staging network whose IPs need rediscovery for every test run.

@bradfitz
Copy link
Contributor

The typical solution to that is setting one of the various http.Transport Dial fields and overriding how DNS resolution works.

It's fine to have multiple Transports active, as long as you remember to close their idle connections when you're done with them.

@cavaliercoder
Copy link
Author

Awesome, I'll head down that path and report back. Thanks for your advice!

@bradfitz bradfitz added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Nov 14, 2017
@nullchinchilla
Copy link

This would break usage of net/http for domain fronting, an anti-censorship technique specifically relying on sending different hosts in SNI and HTTP headers. Such a use of net/http is not rare: censorship-resistant tunneling software such as Lantern and my own project Geph both use net/http this way.

@cavaliercoder
Copy link
Author

The following snippet got the job done for me:

dialer = &net.Dialer{
	Timeout:   30 * time.Second,
	KeepAlive: 30 * time.Second,
	DualStack: true,
}

client := http.Client{
	Transport: &http.Transport{
		DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
			// redirect all connections to 127.0.0.1
			addr = "127.0.0.1" + addr[strings.LastIndex(addr, ":"):]
			return dialer.DialContext(ctx, network, addr)
		},
	},
}

The above http.Client will send all requests to 127.0.0.1, no matter what hostname is specified in a http.Request's URL. The hostname in the URL is used to set the Host request header and SNI name as desired.

@cavaliercoder
Copy link
Author

Thanks for taking a look at this @bradfitz. Love your work!

archisgore pushed a commit to vulcand/oxy that referenced this issue Jul 7, 2018
Sometime since I first wrote the ACME (Let's Encrypt) code,
and today, golang needs this extra ServerName to be set explicitly
in the TLSConfig.

This is to avoid domain fronting.

golang/go#22704
archisgore pushed a commit to vulcand/oxy that referenced this issue Jul 7, 2018
* Set ServerName explicitly in TLSConfig to ensure SNI works

Sometime since I first wrote the ACME (Let's Encrypt) code,
and today, golang needs this extra ServerName to be set explicitly
in the TLSConfig.

This is to avoid domain fronting.

golang/go#22704

* go fmt
@golang golang locked and limited conversation to collaborators Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants