Commit 5a59b66f authored by Filippo Valsorda's avatar Filippo Valsorda Committed by Brad Fitzpatrick

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/28818Reviewed-by: default avatarAdam Langley <agl@golang.org>
Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent 2e4dc86b
...@@ -1235,6 +1235,10 @@ func (c *Conn) Handshake() error { ...@@ -1235,6 +1235,10 @@ func (c *Conn) Handshake() error {
} }
if c.handshakeErr == nil { if c.handshakeErr == nil {
c.handshakes++ 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 return c.handshakeErr
} }
......
...@@ -15,6 +15,7 @@ import ( ...@@ -15,6 +15,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"math/big"
"net" "net"
"os" "os"
"os/exec" "os/exec"
...@@ -1123,3 +1124,47 @@ func TestBuffering(t *testing.T) { ...@@ -1123,3 +1124,47 @@ func TestBuffering(t *testing.T) {
t.Errorf("expected server handshake to complete with only two writes, but saw %d", n) 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)
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment