Skip to content

Commit

Permalink
[release-branch.go1.20] crypto/tls: fix PSK binder calculation
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
tsaarni authored and gopherbot committed Apr 24, 2023
1 parent 96452d5 commit 6564061
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
21 changes: 21 additions & 0 deletions tls/handshake_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tls/handshake_client_tls13.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 6564061

Please sign in to comment.