QUIC TLS: Ensure QUIC_TLS is ticked between each processed RX packet

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22476)

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index ef6ad1508720e1c5cdb0904aa86586cad6e41bcf..fe2a924433102a60d0daabf886eed56a2caaba63 100644 (file)
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -46,10 +46,11 @@
 
 static void ch_save_err_state(QUIC_CHANNEL *ch);
 static void ch_rx_pre(QUIC_CHANNEL *ch);
-static int ch_rx(QUIC_CHANNEL *ch);
+static int ch_rx(QUIC_CHANNEL *ch, int channel_only);
 static int ch_tx(QUIC_CHANNEL *ch);
 static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags);
-static void ch_rx_handle_packet(QUIC_CHANNEL *ch);
+static int ch_tick_tls(QUIC_CHANNEL *ch, int channel_only);
+static void ch_rx_handle_packet(QUIC_CHANNEL *ch, int channel_only);
 static OSSL_TIME ch_determine_next_tick_deadline(QUIC_CHANNEL *ch);
 static int ch_retry(QUIC_CHANNEL *ch,
                     const unsigned char *retry_token,
@@ -1850,9 +1851,6 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
     OSSL_TIME now, deadline;
     QUIC_CHANNEL *ch = arg;
     int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0;
-    uint64_t error_code;
-    const char *error_msg;
-    ERR_STATE *error_state = NULL;
 
     /*
      * When we tick the QUIC connection, we do everything we need to do
@@ -1898,21 +1896,16 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
 
         do {
             /* Process queued incoming packets. */
-            ch_rx(ch);
+            ch->did_tls_tick        = 0;
+            ch->have_new_rx_secret  = 0;
+            ch_rx(ch, channel_only);
 
             /*
              * Allow the handshake layer to check for any new incoming data and
              * generate new outgoing data.
              */
-            ch->have_new_rx_secret = 0;
-            if (!channel_only) {
-                ossl_quic_tls_tick(ch->qtls);
-
-                if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
-                                            &error_state))
-                    ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
-                                                                 error_msg, error_state);
-            }
+            if (!ch->did_tls_tick)
+                ch_tick_tls(ch, channel_only);
 
             /*
              * If the handshake layer gave us a new secret, we need to do RX
@@ -1983,6 +1976,28 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
            && ossl_qtx_get_queue_len_datagrams(ch->qtx) > 0);
 }
 
+static int ch_tick_tls(QUIC_CHANNEL *ch, int channel_only)
+{
+    uint64_t error_code;
+    const char *error_msg;
+    ERR_STATE *error_state = NULL;
+
+    if (channel_only)
+        return 1;
+
+    ch->did_tls_tick = 1;
+    ossl_quic_tls_tick(ch->qtls);
+
+    if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
+                                &error_state)) {
+        ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
+                                                     error_msg, error_state);
+        return 0;
+    }
+
+    return 1;
+}
+
 /* Process incoming datagrams, if any. */
 static void ch_rx_pre(QUIC_CHANNEL *ch)
 {
@@ -2042,7 +2057,7 @@ static void ch_rx_check_forged_pkt_limit(QUIC_CHANNEL *ch)
 }
 
 /* Process queued incoming packets and handle frames, if any. */
-static int ch_rx(QUIC_CHANNEL *ch)
+static int ch_rx(QUIC_CHANNEL *ch, int channel_only)
 {
     int handled_any = 0;
     const int closing = ossl_quic_channel_is_closing(ch);
@@ -2070,7 +2085,7 @@ static int ch_rx(QUIC_CHANNEL *ch)
             ch_update_ping_deadline(ch);
         }
 
-        ch_rx_handle_packet(ch); /* best effort */
+        ch_rx_handle_packet(ch, channel_only); /* best effort */
 
         /*
          * Regardless of the outcome of frame handling, unref the packet.
@@ -2122,7 +2137,7 @@ static int bio_addr_eq(const BIO_ADDR *a, const BIO_ADDR *b)
 }
 
 /* Handles the packet currently in ch->qrx_pkt->hdr. */
-static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
+static void ch_rx_handle_packet(QUIC_CHANNEL *ch, int channel_only)
 {
     uint32_t enc_level;
     int old_have_processed_any_pkt = ch->have_processed_any_pkt;
@@ -2323,6 +2338,10 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
 
         /* This packet contains frames, pass to the RXDP. */
         ossl_quic_handle_frames(ch, ch->qrx_pkt); /* best effort */
+
+        if (ch->did_crypto_frame)
+            ch_tick_tls(ch, channel_only);
+
         break;
 
     case QUIC_PKT_TYPE_VERSION_NEG:
@@ -2726,10 +2745,6 @@ int ossl_quic_channel_set_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio)
  */
 int ossl_quic_channel_start(QUIC_CHANNEL *ch)
 {
-    uint64_t error_code;
-    const char *error_msg;
-    ERR_STATE *error_state = NULL;
-
     if (ch->is_server)
         /*
          * This is not used by the server. The server moves to active
@@ -2758,14 +2773,8 @@ int ossl_quic_channel_start(QUIC_CHANNEL *ch)
     ch->doing_proactive_ver_neg = 0; /* not currently supported */
 
     /* Handshake layer: start (e.g. send CH). */
-    ossl_quic_tls_tick(ch->qtls);
-
-    if (ossl_quic_tls_get_error(ch->qtls, &error_code, &error_msg,
-                                &error_state)) {
-        ossl_quic_channel_raise_protocol_error_state(ch, error_code, 0,
-                                                     error_msg, error_state);
+    if (!ch_tick_tls(ch, /*channel_only=*/0))
         return 0;
-    }
 
     ossl_quic_reactor_tick(&ch->rtor, 0); /* best effort */
     return 1;

diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h
index 77dc5dd7bc4d935e20d59977878d6fcc50cd0a9f..f0ac74242087ab683cdf86c7f14016e4e5370d85 100644 (file)
--- a/ssl/quic/quic_channel_local.h
+++ b/ssl/quic/quic_channel_local.h
@@ -388,6 +388,11 @@ struct quic_channel_st {
      */
     unsigned int                    have_new_rx_secret      : 1;
 
+    /* Have we ever called QUIC_TLS yet during RX processing? */
+    unsigned int                    did_tls_tick            : 1;
+    /* Has any CRYPTO frame been processed during this tick? */
+    unsigned int                    did_crypto_frame        : 1;
+
     /*
      * Have we sent an ack-eliciting packet since the last successful packet
      * reception? Used to determine when to bump idle timer (see RFC 9000 s.

diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
index f2a564862b9ac8996cf5799f39046ea044ca7a38..97c6d6095dc4abc56c87504b317106a05c59004f 100644 (file)
--- a/ssl/quic/quic_rx_depack.c
+++ b/ssl/quic/quic_rx_depack.c
@@ -317,6 +317,7 @@ static int depack_do_frame_crypto(PACKET *pkt, QUIC_CHANNEL *ch,
         return 0;
     }
 
+    ch->did_crypto_frame = 1;
     *datalen = f.len;
 
     return 1;
@@ -1422,6 +1423,8 @@ int ossl_quic_handle_frames(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpacket)
     if (ch == NULL)
         goto end;
 
+    ch->did_crypto_frame = 0;
+
     /* Initialize |ackm_data| (and reinitialize |ok|)*/
     memset(&ackm_data, 0, sizeof(ackm_data));
     /*

diff --git a/test/quicapitest.c b/test/quicapitest.c
index 83221885bcc7d33b60324b74687e30999bb7051a..51d6eea245ea3ed883185fb88210df8e14b3a8cc 100644 (file)
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -1057,15 +1057,15 @@ static int non_io_retry_cert_verify_cb(X509_STORE_CTX *ctx, void *arg)
 {
     int idx = SSL_get_ex_data_X509_STORE_CTX_idx();
     SSL *ssl;
-    int *ctr = (int *)arg;
+    const int *allow = (int *)arg;
 
     /* this should not happen but check anyway */
     if (idx < 0
         || (ssl = X509_STORE_CTX_get_ex_data(ctx, idx)) == NULL)
         return 0;
 
-    /* If this is the first time we've been called then retry */
-    if (((*ctr)++) == 0)
+    /* If this is our first attempt then retry */
+    if (*allow == 0)
         return SSL_set_retry_verify(ssl);
 
     /* Otherwise do nothing - verification succeeds. Continue as normal */
@@ -1082,7 +1082,7 @@ static int test_non_io_retry(int idx)
     SSL *clientquic = NULL;
     QUIC_TSERVER *qtserv = NULL;
     int testresult = 0;
-    int flags = 0, ctr = 0;
+    int flags = 0, allow = 0;
 
     if (idx >= 1 && !qtest_supports_blocking())
         return TEST_skip("Blocking tests not supported in this build");
@@ -1091,7 +1091,7 @@ static int test_non_io_retry(int idx)
     if (!TEST_ptr(cctx))
         goto err;
 
-    SSL_CTX_set_cert_verify_callback(cctx, non_io_retry_cert_verify_cb, &ctr);
+    SSL_CTX_set_cert_verify_callback(cctx, non_io_retry_cert_verify_cb, &allow);
 
     flags = (idx >= 1) ? QTEST_FLAG_BLOCK : 0;
     if (!TEST_true(qtest_create_quic_objects(libctx, cctx, NULL, cert, privkey,
@@ -1099,8 +1099,11 @@ static int test_non_io_retry(int idx)
                                              NULL))
             || !TEST_true(qtest_create_quic_connection_ex(qtserv, clientquic,
                             SSL_ERROR_WANT_RETRY_VERIFY))
-            || !TEST_int_eq(SSL_want(clientquic), SSL_RETRY_VERIFY)
-            || !TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
+            || !TEST_int_eq(SSL_want(clientquic), SSL_RETRY_VERIFY))
+        goto err;
+
+    allow = 1;
+    if (!TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
         goto err;
 
     testresult = 1;