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>
This commit is contained in:
parent
433c5c3eec
commit
37110e801a
4
conn.go
4
conn.go
@ -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 (
|
|||||||
"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) {
|
|||||||
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user