Ensure client read app data 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 8134afa33101ab1963b747c6d86f1a8a3f82b28a..ba4500dd659715ba1d358715f4b62120bf746f2b 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -911,6 +911,17 @@ WORK_STATE ossl_statem_client_post_work(SSL_CONNECTION *s, WORK_STATE wst)
                     /* SSLfatal() already called */
                     return WORK_ERROR;
                 }
+                /*
+                 * For QUIC we deferred setting up these keys until now so
+                 * that we can ensure write keys are always set up before read
+                 * keys.
+                 */
+                if (SSL_IS_QUIC_HANDSHAKE(s)
+                        && !ssl->method->ssl3_enc->change_cipher_state(s,
+                            SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
+                    /* SSLfatal() already called */
+                    return WORK_ERROR;
+                }
             }
         }
         break;

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index cb1e4f8c4b8eb7e0648c26585363b70a292a1ef3..1e11d077f9e06be7e9a0f217c2d0891a48ee4c0c 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -938,8 +938,18 @@ MSG_PROCESS_RETURN tls_process_finished(SSL_CONNECTION *s, PACKET *pkt)
                 /* SSLfatal() already called */
                 return MSG_PROCESS_ERROR;
             }
-            if (!ssl->method->ssl3_enc->change_cipher_state(s,
-                    SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
+
+            /*
+             * For non-QUIC we set up the client's app data read keys now, so
+             * that we can go straight into reading 0.5RTT data from the server.
+             * For QUIC we don't do that, and instead defer setting up the keys
+             * until after we have set up the write keys in order to ensure that
+             * write keys are always set up before read keys (so that if we read
+             * a message we have the correct keys in place to ack it)
+             */
+            if (!SSL_IS_QUIC_HANDSHAKE(s)
+                    && !ssl->method->ssl3_enc->change_cipher_state(s,
+                        SSL3_CC_APPLICATION | SSL3_CHANGE_CIPHER_CLIENT_READ)) {
                 /* SSLfatal() already called */
                 return MSG_PROCESS_ERROR;
             }

diff --git a/ssl/tls13_enc.c b/ssl/tls13_enc.c
index f4af3ad3c89073dcb2415db634b6d0ef9f23f02d..0aa97648c4eda0c5ac42595a0b9ed9f561dbb041 100644 (file)
--- a/ssl/tls13_enc.c
+++ b/ssl/tls13_enc.c
@@ -658,6 +658,7 @@ int tls13_change_cipher_state(SSL_CONNECTION *s, int which)
             label = server_application_traffic;
             labellen = sizeof(server_application_traffic) - 1;
             log_label = SERVER_APPLICATION_LABEL;
+            hash = s->server_finished_hash;
         }
     }