crypto/tls: tls client-side test recordings don't include the close notify message

[rygrange]

Go version

go version devel go1.24-7f87b82955 Mon Sep 30 18:47:31 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/ubuntu/.cache/go-build'
GOENV='/home/ubuntu/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/ubuntu/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/ubuntu/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/ubuntu/code/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/ubuntu/code/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-7f87b82955 Mon Sep 30 18:47:31 2024 +0000'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/ubuntu/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2701037647=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I compiled go master from source by going into src and running make.bash, after downloading, untarring, and setting my GOROOT_BOOTSTRAP to a Go 1.23.1 precompiled binary tarball for linux amd64. After compiling master and setting the PATH with the go/bin directory, I found the current openssl version to use for updating tls testdata in src/crypto/tls/handshake_test.go:85. From here I got the latest release source code for OpenSSL 1.1.1 which was 1.1.1w. Then I ran ./Configure enable-weak-ssl-ciphers no-shared linux-x86_64 and then make to compile. Testing the command apps/openssl version worked fine:

$ apps/openssl version
OpenSSL 1.1.1w  11 Sep 2023

Next I exported the PATH as stated in the comment in the Go code:

$ export PATH=$(pwd)/apps:$PATH
$ openssl version
OpenSSL 1.1.1w  11 Sep 2023

The test dependencies are all installed. Next I will try a test that should verify the bug.
Here are the steps for running the test that will cause the failure:

1. run a test with tls test recordings and see it pass with no changes (TestHandshakeClientECDHEECDSAAESGCM)
2. remove the recording for that test (the file Client-TLSv12-ECDHE-ECDSA-AES-GCM)
3. run the test with update and see it fail both during the first run and succeed the update run, updating the recording
4. run the test without update again and see it fail
5. compare the old and new recordings and see that the final close message was not recorded

What did you see happen?

$ go version
go version devel go1.24-7f87b82955 Mon Sep 30 18:47:31 2024 +0000 linux/amd64
$ go test crypto/tls -run=^TestHandshakeClientECDHEECDSAAESGCM$ -v     
=== RUN   TestHandshakeClientECDHEECDSAAESGCM
=== RUN   TestHandshakeClientECDHEECDSAAESGCM/TLSv12
=== PAUSE TestHandshakeClientECDHEECDSAAESGCM/TLSv12
=== CONT  TestHandshakeClientECDHEECDSAAESGCM/TLSv12
--- PASS: TestHandshakeClientECDHEECDSAAESGCM (0.00s)
    --- PASS: TestHandshakeClientECDHEECDSAAESGCM/TLSv12 (0.00s)
PASS
ok      crypto/tls      0.018s

$ rm src/crypto/tls/testdata/Client-TLSv12-ECDHE-ECDSA-AES-GCM

$ go test crypto/tls -v -update -run=^TestHandshakeClientECDHEECDSAAESGCM$
=== RUN   TestHandshakeClientECDHEECDSAAESGCM
=== RUN   TestHandshakeClientECDHEECDSAAESGCM/TLSv12
    handshake_client_test.go:307: failed to load data from testdata/Client-TLSv12-ECDHE-ECDSA-AES-GCM: open testdata/Client-TLSv12-ECDHE-ECDSA-AES-GCM: no such file or directory
=== RUN   TestHandshakeClientECDHEECDSAAESGCM/TLSv12#update
    handshake_client_test.go:451: Wrote testdata/Client-TLSv12-ECDHE-ECDSA-AES-GCM
--- FAIL: TestHandshakeClientECDHEECDSAAESGCM (0.13s)
    --- FAIL: TestHandshakeClientECDHEECDSAAESGCM/TLSv12 (0.00s)
    --- PASS: TestHandshakeClientECDHEECDSAAESGCM/TLSv12#update (0.13s)
FAIL
FAIL    crypto/tls      0.144s
FAIL

$ go test crypto/tls -v -run=^TestHandshakeClientECDHEECDSAAESGCM$ 
=== RUN   TestHandshakeClientECDHEECDSAAESGCM
=== RUN   TestHandshakeClientECDHEECDSAAESGCM/TLSv12
=== PAUSE TestHandshakeClientECDHEECDSAAESGCM/TLSv12
=== CONT  TestHandshakeClientECDHEECDSAAESGCM/TLSv12
    handshake_test.go:263: expected read, got write
--- FAIL: TestHandshakeClientECDHEECDSAAESGCM (0.00s)
    --- FAIL: TestHandshakeClientECDHEECDSAAESGCM/TLSv12 (0.00s)
FAIL
FAIL    crypto/tls      0.017s
FAIL

New recording without close notify

Old with close notify

What did you expect to see?

I expected the test to have succeeded with ok when running the test again (step 4) after rerecording the tls test in step 3. The messages in the tls recording should include the close notify record, meaning when the test is replaying the recording it should match properly on the final message when the deferred client.Close() call is occurring. This will mean that all the recorded messages will match and the test will succeed. Fixing this issue will fix this recording problem for all client handshake tests that use this recording test harness. This fix would likely need to be made in both master and release-branch.go1.23 to ensure that new recordings that need to be made in client tls handshake tests function properly.

[gabyhelp]

Related Issues and Documentation

• crypto/tls: unable to update some crypto/tls recorded tests #31809 (closed)
• crypto/tls: TestVersion incorrectly tests "tls10default" #51709 (closed)
• crypto/tls: client connection is staying open after server has closed it #34796 (closed)
• crypto/tls: implement TLS 1.3 PSK authentication (server side)
• crypto/tls: client did not fail when lack of a certificate and server requires one #59012 (closed)
• crypto/tls: Dial to TLS endpoint requiring ClientAuth does not error #33368 (closed)
• crypto/tls: failed tls.Conn.Write returns a permanent error
• crypto/tls: TLS13 handshake does not fail if no cipherSuites overlap between server and client #30109 (closed)
• crypto/tls: tls deadlock at handshake stage during my concurrent tests #27136 (closed)
• crypto/tls: implement TLS 1.3 middlebox compatibility mode

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[cherrymui]

cc @FiloSottile @rolandshoemaker @golang/security

[rygrange]

I am working on submitting a fix for this in gerrit (will be Change-Id: I93898de32abd89659a32ed240df6daea5aeaa7fc). I just need to wait until I get an employer CLA agreement added to my account.

[gopherbot]

Change https://go.dev/cl/620395 mentions this issue: crypto/tls: include close notify in client tls test recordings