From: Matt Caswell <matt@openssl.org>
Date: Thu, 29 May 2025 11:16:14 +0000 (+0100)
Subject: Ensure client read handshake secret change occurs after write for QUIC
X-Git-Tag: openssl-3.6.0-alpha1~647
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86e75792622b39a9cf49c0915e58cca5c9d316d3;p=thirdparty%2Fopenssl.git

Ensure client read handshake secret change occurs after write for QUIC

We don't want read secrets to be issue before write for QUIC, because
we want to avoid the situation where we want to ack something we've read
but we don't have the write secret yet.

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27732)
---

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index cdb5e2d599a..8134afa3310 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -1788,9 +1788,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
      */
     if (SSL_CONNECTION_IS_TLS13(s)) {
         if (!ssl->method->ssl3_enc->setup_key_block(s)
-                || !tls13_store_handshake_traffic_hash(s)
-                || !ssl->method->ssl3_enc->change_cipher_state(s,
-                    SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
+                || !tls13_store_handshake_traffic_hash(s)) {
             /* SSLfatal() already called */
             goto err;
         }
@@ -1803,10 +1801,17 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL_CONNECTION *s, PACKET *pkt)
          * are changed. Since QUIC doesn't do TLS early data or need middlebox
          * compat this doesn't cause a problem.
          */
-        if (s->early_data_state == SSL_EARLY_DATA_NONE
-                && (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) == 0
-                && !ssl->method->ssl3_enc->change_cipher_state(s,
+        if (SSL_IS_QUIC_HANDSHAKE(s)
+                || (s->early_data_state == SSL_EARLY_DATA_NONE
+                    && (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) == 0)) {
+            if (!ssl->method->ssl3_enc->change_cipher_state(s,
                     SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_WRITE)) {
+                /* SSLfatal() already called */
+                goto err;
+                    }
+        }
+        if (!ssl->method->ssl3_enc->change_cipher_state(s,
+                    SSL3_CC_HANDSHAKE | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
             /* SSLfatal() already called */
             goto err;
         }
@@ -3817,6 +3822,7 @@ CON_FUNC_RETURN tls_construct_client_certificate(SSL_CONNECTION *s,
      * moment. We need to do it now.
      */
     if (SSL_CONNECTION_IS_TLS13(s)
+            && !SSL_IS_QUIC_HANDSHAKE(s)
             && SSL_IS_FIRST_HANDSHAKE(s)
             && (s->early_data_state != SSL_EARLY_DATA_NONE
                 || (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
@@ -3907,6 +3913,7 @@ CON_FUNC_RETURN tls_construct_client_compressed_certificate(SSL_CONNECTION *sc,
      * moment. We need to do it now.
      */
     if (SSL_IS_FIRST_HANDSHAKE(sc)
+            && !SSL_IS_QUIC_HANDSHAKE(sc)
             && (sc->early_data_state != SSL_EARLY_DATA_NONE
                 || (sc->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
             && (!ssl->method->ssl3_enc->change_cipher_state(sc,
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 4bcefe79a52..dffff631a55 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -626,6 +626,7 @@ CON_FUNC_RETURN tls_construct_finished(SSL_CONNECTION *s, WPACKET *pkt)
      */
     if (SSL_CONNECTION_IS_TLS13(s)
             && !s->server
+            && !SSL_IS_QUIC_HANDSHAKE(s)
             && (s->early_data_state != SSL_EARLY_DATA_NONE
                 || (s->options & SSL_OP_ENABLE_MIDDLEBOX_COMPAT) != 0)
             && s->s3.tmp.cert_req == 0