crypto/tls: don't hold lock when closing underlying net.Conn.

There's no need to hold the handshake lock across this call and it can
lead to deadlocks if the net.Conn calls back into the tls.Conn.

Fixes #18426.

Change-Id: Ib1b2813cce385949d970f8ad2e52cfbd1390e624
Reviewed-on: https://go-review.googlesource.com/36561
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

src/crypto/tls/conn.go

@@ -1206,10 +1206,10 @@ func (c *Conn) Close() error {
 	var alertErr error
 
 	c.handshakeMutex.Lock()
-	defer c.handshakeMutex.Unlock()
 	if c.handshakeComplete {
 		alertErr = c.closeNotify()
 	}
+	c.handshakeMutex.Unlock()
 
 	if err := c.conn.Close(); err != nil {
 		return err

src/crypto/tls/conn_test.go

@@ -241,3 +241,34 @@ func TestDynamicRecordSizingWithAEAD(t *testing.T) {
 	config.CipherSuites = []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256}
 	runDynamicRecordSizingTest(t, config)
 }
+
+// hairpinConn is a net.Conn that makes a “hairpin” call when closed, back into
+// the tls.Conn which is calling it.
+type hairpinConn struct {
+	net.Conn
+	tlsConn *Conn
+}
+
+func (conn *hairpinConn) Close() error {
+	conn.tlsConn.ConnectionState()
+	return nil
+}
+
+func TestHairpinInClose(t *testing.T) {
+	// This tests that the underlying net.Conn can call back into the
+	// tls.Conn when being closed without deadlocking.
+	client, server := net.Pipe()
+	defer server.Close()
+	defer client.Close()
+
+	conn := &hairpinConn{client, nil}
+	tlsConn := Server(conn, &Config{
+		GetCertificate: func(*ClientHelloInfo) (*Certificate, error) {
+			panic("unreachable")
+		},
+	})
+	conn.tlsConn = tlsConn
+
+	// This call should not deadlock.
+	tlsConn.Close()
+}