Skip to content

Commit ef57834

Browse files
crypto/tls: fix flaky handshake cancellation tests
Simplified both tests significantly by removing logic for writing the client/server side messages. The flake was likely because of a race between the closing of the local pipe from inside the test and closing of the pipe from within the handshakeContext goroutine. Wait to close the local pipe in the test until after the test has finished running. Fixes #45106 Fixes #45299 Change-Id: If7ca75aeff7df70cda03c934fa9d8513276d465d Reviewed-on: https://go-review.googlesource.com/c/go/+/305250 Trust: Johan Brandhorst-Satzkorn <[email protected]> Trust: Katie Hockman <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]>
1 parent dba8928 commit ef57834

File tree

2 files changed

+43
-17
lines changed

2 files changed

+43
-17
lines changed

src/crypto/tls/handshake_client_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package tls
66

77
import (
88
"bytes"
9+
"context"
910
"crypto/rsa"
1011
"crypto/x509"
1112
"encoding/base64"
@@ -20,6 +21,7 @@ import (
2021
"os/exec"
2122
"path/filepath"
2223
"reflect"
24+
"runtime"
2325
"strconv"
2426
"strings"
2527
"testing"
@@ -2511,3 +2513,35 @@ func testResumptionKeepsOCSPAndSCT(t *testing.T, ver uint16) {
25112513
serverConfig.Certificates[0].SignedCertificateTimestamps, ccs.SignedCertificateTimestamps)
25122514
}
25132515
}
2516+
2517+
// TestClientHandshakeContextCancellation tests that cancelling
2518+
// the context given to the client side conn.HandshakeContext
2519+
// interrupts the in-progress handshake.
2520+
func TestClientHandshakeContextCancellation(t *testing.T) {
2521+
c, s := localPipe(t)
2522+
ctx, cancel := context.WithCancel(context.Background())
2523+
unblockServer := make(chan struct{})
2524+
defer close(unblockServer)
2525+
go func() {
2526+
cancel()
2527+
<-unblockServer
2528+
_ = s.Close()
2529+
}()
2530+
cli := Client(c, testConfig)
2531+
// Initiates client side handshake, which will block until the client hello is read
2532+
// by the server, unless the cancellation works.
2533+
err := cli.HandshakeContext(ctx)
2534+
if err == nil {
2535+
t.Fatal("Client handshake did not error when the context was canceled")
2536+
}
2537+
if err != context.Canceled {
2538+
t.Errorf("Unexpected client handshake error: %v", err)
2539+
}
2540+
if runtime.GOARCH == "wasm" {
2541+
t.Skip("conn.Close does not error as expected when called multiple times on WASM")
2542+
}
2543+
err = cli.Close()
2544+
if err == nil {
2545+
t.Error("Client connection was not closed when the context was canceled")
2546+
}
2547+
}

src/crypto/tls/handshake_server_test.go

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1946,37 +1946,29 @@ func TestAESCipherReordering13(t *testing.T) {
19461946
}
19471947
}
19481948

1949+
// TestServerHandshakeContextCancellation tests that cancelling
1950+
// the context given to the server side conn.HandshakeContext
1951+
// interrupts the in-progress handshake.
19491952
func TestServerHandshakeContextCancellation(t *testing.T) {
19501953
c, s := localPipe(t)
1951-
clientConfig := testConfig.Clone()
1952-
clientErr := make(chan error, 1)
19531954
ctx, cancel := context.WithCancel(context.Background())
1954-
defer cancel()
1955+
unblockClient := make(chan struct{})
1956+
defer close(unblockClient)
19551957
go func() {
1956-
defer close(clientErr)
1957-
defer c.Close()
1958-
clientHello := &clientHelloMsg{
1959-
vers: VersionTLS10,
1960-
random: make([]byte, 32),
1961-
cipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
1962-
compressionMethods: []uint8{compressionNone},
1963-
}
1964-
cli := Client(c, clientConfig)
1965-
_, err := cli.writeRecord(recordTypeHandshake, clientHello.marshal())
19661958
cancel()
1967-
clientErr <- err
1959+
<-unblockClient
1960+
_ = c.Close()
19681961
}()
19691962
conn := Server(s, testConfig)
1963+
// Initiates server side handshake, which will block until a client hello is read
1964+
// unless the cancellation works.
19701965
err := conn.HandshakeContext(ctx)
19711966
if err == nil {
19721967
t.Fatal("Server handshake did not error when the context was canceled")
19731968
}
19741969
if err != context.Canceled {
19751970
t.Errorf("Unexpected server handshake error: %v", err)
19761971
}
1977-
if err := <-clientErr; err != nil {
1978-
t.Errorf("Unexpected client error: %v", err)
1979-
}
19801972
if runtime.GOARCH == "wasm" {
19811973
t.Skip("conn.Close does not error as expected when called multiple times on WASM")
19821974
}

0 commit comments

Comments
 (0)