[release-branch.go1.20] crypto/tls: fix PSK binder calculation

When server and client have mismatch in curve preference, the server will
send HelloRetryRequest during TLSv1.3 PSK resumption. There was a bug
introduced by Go1.19.6 or later and Go1.20.1 or later, that makes the client
calculate the PSK binder hash incorrectly. Server will reject the TLS
handshake by sending alert: invalid PSK binder.

For #59424.
Fixes #59540.

Change-Id: I2ca8948474275740a36d991c057b62a13392dbb9
GitHub-Last-Rev: 1aad9bcf27f563449c1a7ed6d0dd1d247cc65713
GitHub-Pull-Request: golang/go#59425
Reviewed-on: https://go-review.googlesource.com/c/go/+/481955
Reviewed-by: Roland Shoemaker <roland@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
(cherry picked from commit 2c70690451f1484607a9172a4c24f78ae832dcb0)
Reviewed-on: https://go-review.googlesource.com/c/go/+/488055
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index 749c9fc95452ad3270ab920a491d8ebbce8cc492..22be38faff46870d2a8f41f064aeeff3cb4ef691 100644 (file)
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -1028,6 +1028,27 @@ func testResumption(t *testing.T, version uint16) {
        deleteTicket()
        testResumeState("WithoutSessionTicket", false)
 
+       // In TLS 1.3, HelloRetryRequest is sent after incorrect key share.
+       // See https://www.rfc-editor.org/rfc/rfc8446#page-14.
+       if version == VersionTLS13 {
+               deleteTicket()
+               serverConfig = &Config{
+                       // Use a different curve than the client to force a HelloRetryRequest.
+                       CurvePreferences: []CurveID{CurveP521, CurveP384, CurveP256},
+                       MaxVersion:       version,
+                       Certificates:     testConfig.Certificates,
+               }
+               testResumeState("InitialHandshake", false)
+               testResumeState("WithHelloRetryRequest", true)
+
+               // Reset serverConfig back.
+               serverConfig = &Config{
+                       MaxVersion:   version,
+                       CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA, TLS_ECDHE_RSA_WITH_RC4_128_SHA},
+                       Certificates: testConfig.Certificates,
+               }
+       }
+
        // Session resumption should work when using client certificates
        deleteTicket()
        serverConfig.ClientCAs = rootCAs

diff --git a/src/crypto/tls/handshake_client_tls13.go b/src/crypto/tls/handshake_client_tls13.go
index fefba01a0611ac337afd0801a4bf3eb5bad9db80..4a8661085ebf57245138022c8449c76fc8375499 100644 (file)
--- a/src/crypto/tls/handshake_client_tls13.go
+++ b/src/crypto/tls/handshake_client_tls13.go
@@ -259,7 +259,7 @@ func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {
                        transcript := hs.suite.hash.New()
                        transcript.Write([]byte{typeMessageHash, 0, 0, uint8(len(chHash))})
                        transcript.Write(chHash)
-                       if err := transcriptMsg(hs.serverHello, hs.transcript); err != nil {
+                       if err := transcriptMsg(hs.serverHello, transcript); err != nil {
                                return err
                        }
                        helloBytes, err := hs.hello.marshalWithoutBinders()