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>

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

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.

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")
 			}
 

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

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

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

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,
 	}

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
 		},
 	}

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")
 				}
 			}