crypto/tls: set ServerName and unset TLSUnique in ConnectionState in TLS 1.3

Fix a couple overlooked ConnectionState fields noticed by net/http
tests, and add a test in crypto/tls. Spun off CL 147638 to keep that one
cleanly about enabling TLS 1.3.

Change-Id: I9a6c2e68d64518a44be2a5d7b0b7b8d78c98c95d
Reviewed-on: https://go-review.googlesource.com/c/148900
Run-TryBot: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

4 files changed, 121 insertions, 4 deletions

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 62d786aeae..fefb68adc7 100644
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -208,8 +208,8 @@ type ConnectionState struct {
 	ServerName                  string                // server name requested by client, if any (server side only)
 	PeerCertificates            []*x509.Certificate   // certificate chain presented by remote peer
 	VerifiedChains              [][]*x509.Certificate // verified chains built from PeerCertificates
-	SignedCertificateTimestamps [][]byte              // SCTs from the server, if any
-	OCSPResponse                []byte                // stapled OCSP response from server, if any
+	SignedCertificateTimestamps [][]byte              // SCTs from the peer, if any
+	OCSPResponse                []byte                // stapled OCSP response from peer, if any
 
 	// ekm is a closure exposed via ExportKeyingMaterial.
 	ekm func(label string, context []byte, length int) ([]byte, error)
@@ -219,7 +219,7 @@ type ConnectionState struct {
 	// because resumption does not include enough context (see
 	// https://mitls.org/pages/attacks/3SHAKE#channelbindings). This will
 	// change in future versions of Go once the TLS master-secret fix has
-	// been standardized and implemented.
+	// been standardized and implemented. It is not defined in TLS 1.3.
 	TLSUnique []byte
 }
 
diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go
index 6786d19748..f61d43203f 100644
--- a/src/crypto/tls/conn.go
+++ b/src/crypto/tls/conn.go
@@ -1378,7 +1378,7 @@ func (c *Conn) ConnectionState() ConnectionState {
 		state.VerifiedChains = c.verifiedChains
 		state.SignedCertificateTimestamps = c.scts
 		state.OCSPResponse = c.ocspResponse
-		if !c.didResume {
+		if !c.didResume && c.vers != VersionTLS13 {
 			if c.clientFinishedIsFirst {
 				state.TLSUnique = c.clientFinished[:]
 			} else {
diff --git a/src/crypto/tls/handshake_server_tls13.go b/src/crypto/tls/handshake_server_tls13.go
index 4d13ff39d9..a689096cae 100644
--- a/src/crypto/tls/handshake_server_tls13.go
+++ b/src/crypto/tls/handshake_server_tls13.go
@@ -213,6 +213,7 @@ GroupSelection:
 		return errors.New("tls: invalid client key share")
 	}
 
+	c.serverName = hs.clientHello.serverName
 	return nil
 }
 
diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index e9abe01280..fac3522b7d 100644
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -916,3 +916,119 @@ func TestConnectionStateMarshal(t *testing.T) {
 		t.Errorf("json.Marshal failed on ConnectionState: %v", err)
 	}
 }
+
+func TestConnectionState(t *testing.T) {
+	issuer, err := x509.ParseCertificate(testRSACertificateIssuer)
+	if err != nil {
+		panic(err)
+	}
+	rootCAs := x509.NewCertPool()
+	rootCAs.AddCert(issuer)
+
+	now := func() time.Time { return time.Unix(1476984729, 0) }
+
+	const alpnProtocol = "golang"
+	const serverName = "example.golang"
+	var scts = [][]byte{[]byte("dummy sct 1"), []byte("dummy sct 2")}
+	var ocsp = []byte("dummy ocsp")
+
+	for _, v := range []uint16{VersionTLS12, VersionTLS13} {
+		var name string
+		switch v {
+		case VersionTLS12:
+			name = "TLSv12"
+		case VersionTLS13:
+			name = "TLSv13"
+		}
+		t.Run(name, func(t *testing.T) {
+			config := &Config{
+				Time:         now,
+				Rand:         zeroSource{},
+				Certificates: make([]Certificate, 1),
+				MaxVersion:   v,
+				RootCAs:      rootCAs,
+				ClientCAs:    rootCAs,
+				ClientAuth:   RequireAndVerifyClientCert,
+				NextProtos:   []string{alpnProtocol},
+				ServerName:   serverName,
+			}
+			config.Certificates[0].Certificate = [][]byte{testRSACertificate}
+			config.Certificates[0].PrivateKey = testRSAPrivateKey
+			config.Certificates[0].SignedCertificateTimestamps = scts
+			config.Certificates[0].OCSPStaple = ocsp
+
+			ss, cs, err := testHandshake(t, config, config)
+			if err != nil {
+				t.Fatalf("Handshake failed: %v", err)
+			}
+
+			if ss.Version != v || cs.Version != v {
+				t.Errorf("Got versions %x (server) and %x (client), expected %x", ss.Version, cs.Version, v)
+			}
+
+			if !ss.HandshakeComplete || !cs.HandshakeComplete {
+				t.Errorf("Got HandshakeComplete %v (server) and %v (client), expected true", ss.HandshakeComplete, cs.HandshakeComplete)
+			}
+
+			if ss.DidResume || cs.DidResume {
+				t.Errorf("Got DidResume %v (server) and %v (client), expected false", ss.DidResume, cs.DidResume)
+			}
+
+			if ss.CipherSuite == 0 || cs.CipherSuite == 0 {
+				t.Errorf("Got invalid cipher suite: %v (server) and %v (client)", ss.CipherSuite, cs.CipherSuite)
+			}
+
+			if ss.NegotiatedProtocol != alpnProtocol || cs.NegotiatedProtocol != alpnProtocol {
+				t.Errorf("Got negotiated protocol %q (server) and %q (client), expected %q", ss.NegotiatedProtocol, cs.NegotiatedProtocol, alpnProtocol)
+			}
+
+			if !cs.NegotiatedProtocolIsMutual {
+				t.Errorf("Got false NegotiatedProtocolIsMutual on the client side")
+			}
+			// NegotiatedProtocolIsMutual on the server side is unspecified.
+
+			if ss.ServerName != serverName {
+				t.Errorf("Got server name %q, expected %q", ss.ServerName, serverName)
+			}
+			if cs.ServerName != "" {
+				t.Errorf("Got unexpected server name on the client side")
+			}
+
+			if len(ss.PeerCertificates) != 1 || len(cs.PeerCertificates) != 1 {
+				t.Errorf("Got %d (server) and %d (client) peer certificates, expected %d", len(ss.PeerCertificates), len(cs.PeerCertificates), 1)
+			}
+
+			if len(ss.VerifiedChains) != 1 || len(cs.VerifiedChains) != 1 {
+				t.Errorf("Got %d (server) and %d (client) verified chains, expected %d", len(ss.VerifiedChains), len(cs.VerifiedChains), 1)
+			} else if len(ss.VerifiedChains[0]) != 2 || len(cs.VerifiedChains[0]) != 2 {
+				t.Errorf("Got %d (server) and %d (client) long verified chain, expected %d", len(ss.VerifiedChains[0]), len(cs.VerifiedChains[0]), 2)
+			}
+
+			if len(cs.SignedCertificateTimestamps) != 2 {
+				t.Errorf("Got %d SCTs, expected %d", len(cs.SignedCertificateTimestamps), 2)
+			}
+			if !bytes.Equal(cs.OCSPResponse, ocsp) {
+				t.Errorf("Got OCSPs %x, expected %x", cs.OCSPResponse, ocsp)
+			}
+			// Only TLS 1.3 supports OCSP and SCTs on client certs.
+			if v == VersionTLS13 {
+				if len(ss.SignedCertificateTimestamps) != 2 {
+					t.Errorf("Got %d client SCTs, expected %d", len(ss.SignedCertificateTimestamps), 2)
+				}
+				if !bytes.Equal(ss.OCSPResponse, ocsp) {
+					t.Errorf("Got client OCSPs %x, expected %x", ss.OCSPResponse, ocsp)
+				}
+			}
+
+			if v == VersionTLS13 {
+				if ss.TLSUnique != nil || cs.TLSUnique != nil {
+					t.Errorf("Got TLSUnique %x (server) and %x (client), expected nil in TLS 1.3", ss.TLSUnique, cs.TLSUnique)
+				}
+			} else {
+				if ss.TLSUnique == nil || cs.TLSUnique == nil {
+					t.Errorf("Got TLSUnique %x (server) and %x (client), expected non-nil", ss.TLSUnique, cs.TLSUnique)
+				}
+			}
+		})
+	}
+}