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

crypto/tls: Missing attribute in tls.ConnectionState struct #11881

Closed
nathanaelle opened this issue Jul 27, 2015 · 3 comments
Closed

crypto/tls: Missing attribute in tls.ConnectionState struct #11881

nathanaelle opened this issue Jul 27, 2015 · 3 comments

Comments

@nathanaelle
Copy link

Hi,

Summary

In some situations, ConnectionState.ServerName is not enough and the access to the certificate used to negociate the connection is needed.
This issue targets any stateless protocol that can use SNI and an field (inside the protocol) to identify a virtual host.
In Case of HTTP with TLS , SNI ServerName and the Host header provide valid strings that may differ.

Solution

Adding a ServerCertificate *x509.Certificate attribute to tls.ConnectionState struct

tls.Conn has access to tls.Config so ServerCertificate can be set when (*tls.Conn)ConnectionState() is called.

Illustrations of the problem

the context is a HTTPS offloader/reverse-proxy that have 2 differents certificates each for a different *.FQDN in CommonName ; precisely, cert-1 has *.FQDN-1 CommonName and cert-2 has *.FQDN-2 CommonName

Good case (easily produced with any browser)

  1. client connects to the server with a SNI header to foo.FQDN-1 and a Host: foo.FQDN-1
  2. server sets request.TLS.ServerName and call ServeHTTP foo.FQDN-1
  3. inside ServeHTTP, request.TLS.ServerName and req.Host have the same value
  4. ServeHTTP answers a redirection to bar.FQDN-1
  5. client reuses the same connection & Session Ticket because the Certificate's CommonName allow it
  6. inside ServerHTTP , request.TLS.ServerName and req.Host have different values because req.TLS.ServerName was set in the first connection

a call to req.TLS.ServerCertificate.VerifyHostname(req.Host) will return nil to confirm that bar.FQDN-1 can be served on the same connection.

Bad case

  1. client connects to the server with a SNI header to foo.FQDN-1 and a Host: foo.FQDN-1
  2. server sets request.TLS.ServerName and call ServeHTTP foo.FQDN-1
  3. client crafts a HTTP request to foo.FQDN-2 and reuse the previous connection
  4. inside ServerHTTP , request.TLS.ServerName and req.Host have different values

Here, a call to req.TLS.ServerCertificate.VerifyHostname(req.Host) will return an error. this call provide an elegant way to mitigate the situation.

There are others way to mitigate this situation but are also more complicated.

@ianlancetaylor ianlancetaylor changed the title Missing attribute in tls.ConnectionState struct crypto/tls: Missing attribute in tls.ConnectionState struct Jul 27, 2015
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Jul 27, 2015
@ianlancetaylor
Copy link
Contributor

CC @agl @bradfitz

@agl agl self-assigned this Jul 27, 2015
@bradfitz
Copy link
Contributor

Why's it a problem if the the ServerName doesn't match the Host header? If the client wants to reuse the connection, presumably it did so because it saw the cert from the first connection and knew that the server did FQDN-2 as well. Doesn't HTTP/2 actually encourage that behavior?

Leaving for @agl to fix or clarify for me.

@agl
Copy link
Contributor

agl commented Aug 2, 2015

I don't understand this either. If the client chooses to send a request with a different Host header then surely any consequences fall on it?

If you really want to know what certificate was used for a connection you can control it yourself in Go 1.5 with the GetCertificate callback.

@agl agl closed this as completed Aug 2, 2015
@golang golang locked and limited conversation to collaborators Aug 5, 2016
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants