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>

diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go
index 03895a723fa25670946dd973378014c9ed04f901..e6d85aa26391fa0aada9c676cbabc642f6a93a95 100644 (file)
--- a/src/crypto/tls/conn.go
+++ b/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

diff --git a/src/crypto/tls/conn_test.go b/src/crypto/tls/conn_test.go
index 5e5c7a2e96e9bf1e6b9b17930937898e2e58b60e..e58077e6927884d3b524e7643f6bd6b3e34bd984 100644 (file)
--- a/src/crypto/tls/conn_test.go
+++ b/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()
+}