crypto/tls: don't modify Config.Certificates in BuildNameToCertificate

The Config does not own the memory pointed to by the Certificate slice.
Instead, opportunistically use Certificate.Leaf and let the application
set it if it desires the performance gain.

This is a partial rollback of CL 107627. See the linked issue for the
full explanation.

Fixes #28744

Change-Id: I33ce9e6712e3f87939d9d0932a06d24e48ba4567
Reviewed-on: https://go-review.googlesource.com/c/149098Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/crypto/tls/common.go

@@ -871,14 +871,14 @@ func (c *Config) BuildNameToCertificate() {
 	c.NameToCertificate = make(map[string]*Certificate)
 	for i := range c.Certificates {
 		cert := &c.Certificates[i]
-		if cert.Leaf == nil {
-			x509Cert, err := x509.ParseCertificate(cert.Certificate[0])
+		x509Cert := cert.Leaf
+		if x509Cert == nil {
+			var err error
+			x509Cert, err = x509.ParseCertificate(cert.Certificate[0])
 			if err != nil {
 				continue
 			}
-			cert.Leaf = x509Cert
 		}
-		x509Cert := cert.Leaf
 		if len(x509Cert.Subject.CommonName) > 0 {
 			c.NameToCertificate[x509Cert.Subject.CommonName] = cert
 		}

src/crypto/tls/tls_test.go

@@ -1087,3 +1087,25 @@ func TestEscapeRoute(t *testing.T) {
 		t.Errorf("Client negotiated version %x, expected %x", cs.Version, VersionTLS12)
 	}
 }
+
+// Issue 28744: Ensure that we don't modify memory
+// that Config doesn't own such as Certificates.
+func TestBuildNameToCertificate_doesntModifyCertificates(t *testing.T) {
+	c0 := Certificate{
+		Certificate: [][]byte{testRSACertificate},
+		PrivateKey:  testRSAPrivateKey,
+	}
+	c1 := Certificate{
+		Certificate: [][]byte{testSNICertificate},
+		PrivateKey:  testRSAPrivateKey,
+	}
+	config := testConfig.Clone()
+	config.Certificates = []Certificate{c0, c1}
+
+	config.BuildNameToCertificate()
+	got := config.Certificates
+	want := []Certificate{c0, c1}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("Certificates were mutated by BuildNameToCertificate\nGot: %#v\nWant: %#v\n", got, want)
+	}
+}