crypto/tls: flush the buffer on handshake errors

Since 2a8c81ff handshake messages are not written directly to wire but
buffered.  If an error happens at the wrong time the alert will be
written to the buffer but never flushed, causing an EOF on the client
instead of a more descriptive alert.

Thanks to Brendan McMillion for reporting this.

Fixes #17037

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

diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go
index ea299930a9315c1b56f2c476d77cd10bb6ec6184..a44d56dcb15315dbd765d23e2fcf2afb0a9b83da 100644 (file)
--- a/src/crypto/tls/conn.go
+++ b/src/crypto/tls/conn.go
@@ -1235,6 +1235,10 @@ func (c *Conn) Handshake() error {
        }
        if c.handshakeErr == nil {
                c.handshakes++
+       } else {
+               // If an error occurred during the hadshake try to flush the
+               // alert that might be left in the buffer.
+               c.flush()
        }
        return c.handshakeErr
 }

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index a5491bcdf36053978b3594d068c16015eef9d1b6..c87ad5babdf2d035708760be5b3c3a02093c661b 100644 (file)
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -15,6 +15,7 @@ import (
        "errors"
        "fmt"
        "io"
+       "math/big"
        "net"
        "os"
        "os/exec"
@@ -1123,3 +1124,47 @@ func TestBuffering(t *testing.T) {
                t.Errorf("expected server handshake to complete with only two writes, but saw %d", n)
        }
 }
+
+func TestAlertFlushing(t *testing.T) {
+       c, s := net.Pipe()
+       done := make(chan bool)
+
+       clientWCC := &writeCountingConn{Conn: c}
+       serverWCC := &writeCountingConn{Conn: s}
+
+       serverConfig := testConfig.Clone()
+
+       // Cause a signature-time error
+       brokenKey := rsa.PrivateKey{PublicKey: testRSAPrivateKey.PublicKey}
+       brokenKey.D = big.NewInt(42)
+       serverConfig.Certificates = []Certificate{{
+               Certificate: [][]byte{testRSACertificate},
+               PrivateKey:  &brokenKey,
+       }}
+
+       go func() {
+               Server(serverWCC, serverConfig).Handshake()
+               serverWCC.Close()
+               done <- true
+       }()
+
+       err := Client(clientWCC, testConfig).Handshake()
+       if err == nil {
+               t.Fatal("client unexpectedly returned no error")
+       }
+
+       const expectedError = "remote error: tls: handshake failure"
+       if e := err.Error(); !strings.Contains(e, expectedError) {
+               t.Fatalf("expected to find %q in error but error was %q", expectedError, e)
+       }
+       clientWCC.Close()
+       <-done
+
+       if n := clientWCC.numWrites; n != 1 {
+               t.Errorf("expected client handshake to complete with one write, but saw %d", n)
+       }
+
+       if n := serverWCC.numWrites; n != 1 {
+               t.Errorf("expected server handshake to complete with one write, but saw %d", n)
+       }
+}