From 4fb7e083a868946db08db9ef3bc807e21c8fc961 Mon Sep 17 00:00:00 2001 From: Daniel McCarney Date: Mon, 3 Nov 2025 14:47:42 -0500 Subject: [PATCH] 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Filippo Valsorda Reviewed-by: Cherry Mui Auto-Submit: Daniel McCarney --- api/next/74425.txt | 2 + .../6-stdlib/99-minor/crypto/tls/74425.md | 5 +++ src/crypto/tls/bogo_shim_test.go | 4 +- src/crypto/tls/common.go | 11 +++-- src/crypto/tls/conn.go | 2 +- src/crypto/tls/handshake_client_test.go | 2 +- src/crypto/tls/handshake_server.go | 1 + src/crypto/tls/handshake_server_test.go | 41 +++++++++++++++++-- src/crypto/tls/tls_test.go | 8 ++-- 9 files changed, 62 insertions(+), 14 deletions(-) create mode 100644 api/next/74425.txt create mode 100644 doc/next/6-stdlib/99-minor/crypto/tls/74425.md diff --git a/api/next/74425.txt b/api/next/74425.txt new file mode 100644 index 0000000000..f204476d65 --- /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 index 0000000000..8280f24c2c --- /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 02a943c13c..a14386a61c 100644 --- 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 c8e65e4d3c..d809624b88 100644 --- 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 2de120a132..c04c7a506e 100644 --- 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 6020c0f055..e7de0b5911 100644 --- 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 a2cf176a86..2d3efffcf0 100644 --- 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 43183db2a1..7e35c25259 100644 --- 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 93ec8643f6..6905f53949 100644 --- 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") } } -- 2.52.0