crypto/tls: expose HelloRetryRequest state

This commit adds fields to the ClientHelloInfo and ConnectionState
structures to represent hello retry request state information.

ClientHelloInfo gains a new HelloRetryRequest bool field that indicates
if the client hello was sent in response to a TLS 1.3 hello retry
request message previously emitted by the server.

ConnectionState gains a new HelloRetryRequest bool field that indicates
(depending on the connection role) whether the client received a TLS 1.3
hello retry request message from the server, or whether the server sent
such a message to a client.

Fixes #74425

Change-Id: Ic1a5290b8a4ba1568da1d2c2cf9f148150955fa5
Reviewed-on: https://go-review.googlesource.com/c/go/+/717440
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Daniel McCarney <daniel@binaryparadox.net>

diff --git a/api/next/74425.txt b/api/next/74425.txt
new file mode 100644 (file)
index 0000000..f204476
--- /dev/null
+++ b/api/next/74425.txt
@@ -0,0 +1,2 @@
+pkg crypto/tls, type ConnectionState struct, HelloRetryRequest bool #74425
+pkg crypto/tls, type ClientHelloInfo struct, HelloRetryRequest bool #74425

diff --git a/doc/next/6-stdlib/99-minor/crypto/tls/74425.md b/doc/next/6-stdlib/99-minor/crypto/tls/74425.md
new file mode 100644 (file)
index 0000000..8280f24
--- /dev/null
+++ b/doc/next/6-stdlib/99-minor/crypto/tls/74425.md
@@ -0,0 +1,5 @@
+The new [ClientHelloInfo.HelloRetryRequest] field indicates if the ClientHello
+was sent in response to a HelloRetryRequest message. The new
+[ConnectionState.HelloRetryRequest] field indicates if the server
+sent a HelloRetryRequest, or if the client received a HelloRetryRequest,
+depending on connection role.

diff --git a/src/crypto/tls/bogo_shim_test.go b/src/crypto/tls/bogo_shim_test.go
index 02a943c13cfdd923139b12c95171412d29ec6257..a14386a61c23f45eed67f711af4f158b11751f09 100644 (file)
--- a/src/crypto/tls/bogo_shim_test.go
+++ b/src/crypto/tls/bogo_shim_test.go
@@ -476,11 +476,11 @@ func bogoShim() {
                                log.Fatal("did not expect ECH, but it was accepted")
                        }
 
-                       if *expectHRR && !cs.testingOnlyDidHRR {
+                       if *expectHRR && !cs.HelloRetryRequest {
                                log.Fatal("expected HRR but did not do it")
                        }
 
-                       if *expectNoHRR && cs.testingOnlyDidHRR {
+                       if *expectNoHRR && cs.HelloRetryRequest {
                                log.Fatal("expected no HRR but did do it")
                        }
 

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index c8e65e4d3c0ee5969bb5340c1b01b2580ceecba6..d809624b880a00ada4f54bef187d08e526461ba8 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -304,12 +304,13 @@ type ConnectionState struct {
        // client side.
        ECHAccepted bool
 
+       // HelloRetryRequest indicates whether we sent a HelloRetryRequest if we
+       // are a server, or if we received a HelloRetryRequest if we are a client.
+       HelloRetryRequest bool
+
        // ekm is a closure exposed via ExportKeyingMaterial.
        ekm func(label string, context []byte, length int) ([]byte, error)
 
-       // testingOnlyDidHRR is true if a HelloRetryRequest was sent/received.
-       testingOnlyDidHRR bool
-
        // testingOnlyPeerSignatureAlgorithm is the signature algorithm used by the
        // peer to sign the handshake. It is not set for resumed connections.
        testingOnlyPeerSignatureAlgorithm SignatureScheme
@@ -469,6 +470,10 @@ type ClientHelloInfo struct {
        // connection to fail.
        Conn net.Conn
 
+       // HelloRetryRequest indicates whether the ClientHello was sent in response
+       // to a HelloRetryRequest message.
+       HelloRetryRequest bool
+
        // config is embedded by the GetCertificate or GetConfigForClient caller,
        // for use with SupportsCertificate.
        config *Config

diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go
index 2de120a1329f873516242e2924d605ba6c7dffe5..c04c7a506e039c9b066cccedf3ef362dcb012e04 100644 (file)
--- a/src/crypto/tls/conn.go
+++ b/src/crypto/tls/conn.go
@@ -1612,7 +1612,7 @@ func (c *Conn) connectionStateLocked() ConnectionState {
        state.Version = c.vers
        state.NegotiatedProtocol = c.clientProtocol
        state.DidResume = c.didResume
-       state.testingOnlyDidHRR = c.didHRR
+       state.HelloRetryRequest = c.didHRR
        state.testingOnlyPeerSignatureAlgorithm = c.peerSigAlg
        state.CurveID = c.curveID
        state.NegotiatedProtocolIsMutual = true

diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go
index 6020c0f055c7761398b49be75ef109bacef1ec9c..e7de0b59119b8ab011a27aa2fb6aaa85e588f731 100644 (file)
--- a/src/crypto/tls/handshake_client_test.go
+++ b/src/crypto/tls/handshake_client_test.go
@@ -626,7 +626,7 @@ func TestHandshakeClientHelloRetryRequest(t *testing.T) {
                args:   []string{"-cipher", "ECDHE-RSA-AES128-GCM-SHA256", "-curves", "P-256"},
                config: config,
                validate: func(cs ConnectionState) error {
-                       if !cs.testingOnlyDidHRR {
+                       if !cs.HelloRetryRequest {
                                return errors.New("expected HelloRetryRequest")
                        }
                        return nil

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index a2cf176a86c0d872faa040e21e43bae37acca810..2d3efffcf054f89f32aa2b37e773c1f060f9de72 100644 (file)
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -1021,6 +1021,7 @@ func clientHelloInfo(ctx context.Context, c *Conn, clientHello *clientHelloMsg)
                SupportedVersions: supportedVersions,
                Extensions:        clientHello.extensions,
                Conn:              c.conn,
+               HelloRetryRequest: c.didHRR,
                config:            c.config,
                ctx:               ctx,
        }

diff --git a/src/crypto/tls/handshake_server_test.go b/src/crypto/tls/handshake_server_test.go
index 43183db2a197705d9478a87d8ce56bfb96a6836b..7e35c252593658a3cb626daecb12bccea0189107 100644 (file)
--- a/src/crypto/tls/handshake_server_test.go
+++ b/src/crypto/tls/handshake_server_test.go
@@ -905,14 +905,30 @@ func TestHandshakeServerHelloRetryRequest(t *testing.T) {
        config := testConfig.Clone()
        config.CurvePreferences = []CurveID{CurveP256}
 
+       var clientHelloInfoHRR bool
+       var getCertificateCalled bool
+       eeCert := config.Certificates[0]
+       config.Certificates = nil
+       config.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
+               getCertificateCalled = true
+               clientHelloInfoHRR = clientHello.HelloRetryRequest
+               return &eeCert, nil
+       }
+
        test := &serverTest{
                name:    "HelloRetryRequest",
                command: []string{"openssl", "s_client", "-no_ticket", "-ciphersuites", "TLS_CHACHA20_POLY1305_SHA256", "-curves", "X25519:P-256"},
                config:  config,
                validate: func(cs ConnectionState) error {
-                       if !cs.testingOnlyDidHRR {
+                       if !cs.HelloRetryRequest {
                                return errors.New("expected HelloRetryRequest")
                        }
+                       if !getCertificateCalled {
+                               return errors.New("expected GetCertificate to be called")
+                       }
+                       if !clientHelloInfoHRR {
+                               return errors.New("expected ClientHelloInfo.HelloRetryRequest to be true")
+                       }
                        return nil
                },
        }
@@ -920,19 +936,38 @@ func TestHandshakeServerHelloRetryRequest(t *testing.T) {
 }
 
 // TestHandshakeServerKeySharePreference checks that we prefer a key share even
-// if it's later in the CurvePreferences order.
+// if it's later in the CurvePreferences order, and that the client hello HRR
+// field is correctly represented.
 func TestHandshakeServerKeySharePreference(t *testing.T) {
        config := testConfig.Clone()
        config.CurvePreferences = []CurveID{X25519, CurveP256}
 
+       // We also use this test as a convenient place to assert the ClientHelloInfo
+       // HelloRetryRequest field is _not_ set for a non-HRR hello.
+       var clientHelloInfoHRR bool
+       var getCertificateCalled bool
+       eeCert := config.Certificates[0]
+       config.Certificates = nil
+       config.GetCertificate = func(clientHello *ClientHelloInfo) (*Certificate, error) {
+               getCertificateCalled = true
+               clientHelloInfoHRR = clientHello.HelloRetryRequest
+               return &eeCert, nil
+       }
+
        test := &serverTest{
                name:    "KeySharePreference",
                command: []string{"openssl", "s_client", "-no_ticket", "-ciphersuites", "TLS_CHACHA20_POLY1305_SHA256", "-curves", "P-256:X25519"},
                config:  config,
                validate: func(cs ConnectionState) error {
-                       if cs.testingOnlyDidHRR {
+                       if cs.HelloRetryRequest {
                                return errors.New("unexpected HelloRetryRequest")
                        }
+                       if !getCertificateCalled {
+                               return errors.New("expected GetCertificate to be called")
+                       }
+                       if clientHelloInfoHRR {
+                               return errors.New("expected ClientHelloInfo.HelloRetryRequest to be false")
+                       }
                        return nil
                },
        }

diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index 93ec8643f679743fcb3046642f6c65367883b21a..6905f53949933fa19763577b70d0a82eb8b3ca47 100644 (file)
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -2090,17 +2090,17 @@ func TestHandshakeMLKEM(t *testing.T) {
                                }
                        }
                        if test.expectHRR {
-                               if !ss.testingOnlyDidHRR {
+                               if !ss.HelloRetryRequest {
                                        t.Error("server did not use HRR")
                                }
-                               if !cs.testingOnlyDidHRR {
+                               if !cs.HelloRetryRequest {
                                        t.Error("client did not use HRR")
                                }
                        } else {
-                               if ss.testingOnlyDidHRR {
+                               if ss.HelloRetryRequest {
                                        t.Error("server used HRR")
                                }
-                               if cs.testingOnlyDidHRR {
+                               if cs.HelloRetryRequest {
                                        t.Error("client used HRR")
                                }
                        }