crypto/tls: TLSv1.3 connection fails with invalid PSK binder

[tsaarni]

What version of Go are you using (go version)?

The bug impacts following versions

• Go1.19.6 or later
• Go1.20.1 or later

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

The bug is not OS dependent

What did you do?

1. I have written TLSv1.3 client and server written in go.
2. In the server I have set CurvePreferences with non-default list of curves.
3. In the client I'm using default list of curves.
4. In the client I have enabled TLS session resumption by setting ClientSessionCache

What did you expect to see?

When the client establishes second TLS session, I expected the TLS handshake to succeed.

What did you see instead?

When the client establishes second TLS session, the server will reject it with alert invalid PSK binder.

[tsaarni]

I'm attaching reproduction case. Here is the failure when running it

$ go run reproduce-tls-resumption-failure.go
Client: sending request to https://localhost:8443
Server: received request from 127.0.0.1:56926
Client: received response from server with status: 200 OK
Client: sleeping 5 sec...
Client: sending request to https://localhost:8443
2023/04/04 11:10:13 http: TLS handshake error from 127.0.0.1:56934: tls: invalid PSK binder
panic: Get "https://localhost:8443": remote error: tls: error decrypting message

The example code is here:

reproduce-tls-resumption-failure.go.txt

Same code is uploaded to https://goplay.tools/snippet/ur5TpD7hXe4

[gopherbot]

Change https://go.dev/cl/481955 mentions this issue: crypto/tls: fix PSK binder calculation

[tsaarni]

Hi @rolandshoemaker 👋 I may have bumped into a bug that was introduced with #58001. I submitted a proposal for a fix here #59425. I'd greatly appreciate it if you had an opportunity to take a look. Thank you!

[mknyszek]

CC @golang/security

[tsaarni]

Here is a summary how the bug gets triggered:

The bug is in TLSv1.3 handshake code and only impacts TLSv1.3 clients. Servers are not impacted. TLSv1.2 implementation is not impacted.

The bug was introduced by #58001

For the client to be impacted, client must have explicitly enabled TLS resumption by setting e.g. ClientSessionCache: tls.NewLRUClientSessionCache(num) in client's tls.Config structure. Client also needs to hold the SSL context and re-use it for multiple requests to trigger resumption

The bug happens in PSK resumption if client and server having incompatible preferred EC curve. For example, it can happen when the server has set CurvePreferences: []tls.CurveID{tls.CurveP521, tls.CurveP384, tls.CurveP256} in tls.Config but client uses the Go default list. Client will send ClientHello with x25519 but if server preferences does not include x25519, server will respond with HelloRetryRequest according to the spec (link) and set secp512r1 from server's CurvePreferences.

The reception of HelloRetryRequest response from the server will trigger the bug in client when above conditions are true. There is a typo in processHelloRetryRequest() which causes ClientHello message to be sent to server with invalid PSK binders hash. When server will process the new ClientHello with invalid PSK binders field, it will abort the handshake with invalid PSK binder alert.

[rolandshoemaker]

Sorry, I was out on vacation. Thanks for catching this!

@gopherbot please open backport issues for this, it's a regression from a security fix.

[gopherbot]

Backport issue(s) opened: #59539 (for 1.19), #59540 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.