crypto/tls: fix PSK binder calculation

tsaarni · gopherbot · commit 2c70690451f1 · 2023-04-24T13:35:52.000Z
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. Fixes #59424 Change-Id: I2ca8948474275740a36d991c057b62a13392dbb9 GitHub-Last-Rev: 1aad9bc GitHub-Pull-Request: #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>
diff --git 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
@@ -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()

Original file line number	Diff line number	Diff line change
`@@ -259,7 +259,7 @@ func (hs *clientHandshakeStateTLS13) processHelloRetryRequest() error {`
`259`	`259`	`transcript := hs.suite.hash.New()`
`260`	`260`	`transcript.Write([]byte{typeMessageHash, 0, 0, uint8(len(chHash))})`
`261`	`261`	`transcript.Write(chHash)`
`262`		`- if err := transcriptMsg(hs.serverHello, hs.transcript); err != nil {`
	`262`	`+ if err := transcriptMsg(hs.serverHello, transcript); err != nil {`
`263`	`263`	`return err`
`264`	`264`	`}`
`265`	`265`	`helloBytes, err := hs.hello.marshalWithoutBinders()`