Enhance quic_tserver test to fully test thread assisted mode

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

5 files changed, 177 insertions, 16 deletions

diff --git a/include/internal/quic_ssl.h b/include/internal/quic_ssl.h
index e4af2c57be..0b6c3f298f 100644
--- a/include/internal/quic_ssl.h
+++ b/include/internal/quic_ssl.h
@@ -73,6 +73,13 @@ void ossl_quic_conn_set_override_now_cb(SSL *s,
                                         OSSL_TIME (*now_cb)(void *arg),
                                         void *now_cb_arg);
 
+/*
+ * Condvar waiting in the assist thread doesn't support time faking as it relies
+ * on the OS's notion of time, thus this is used in test code to force a
+ * spurious wakeup instead.
+ */
+void ossl_quic_conn_force_assist_thread_wake(SSL *s);
+
 # endif
 
 #endif
diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 8ffb5b99e4..2360809295 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -67,6 +67,7 @@ static int ch_discard_el(QUIC_CHANNEL *ch,
                          uint32_t enc_level);
 static void ch_on_idle_timeout(QUIC_CHANNEL *ch);
 static void ch_update_idle(QUIC_CHANNEL *ch);
+static void ch_update_ping_deadline(QUIC_CHANNEL *ch);
 static void ch_raise_net_error(QUIC_CHANNEL *ch);
 static void ch_on_terminating_timeout(QUIC_CHANNEL *ch);
 static void ch_start_terminating(QUIC_CHANNEL *ch,
@@ -1305,6 +1306,13 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
     if (!ossl_time_is_zero(deadline) && ossl_time_compare(now, deadline) >= 0)
         ossl_ackm_on_timeout(ch->ackm);
 
+    /* If a ping is due, inform TXP. */
+    if (ossl_time_compare(now, ch->ping_deadline) >= 0) {
+        int pn_space = ossl_quic_enc_level_to_pn_space(ch->tx_enc_level);
+
+        ossl_quic_tx_packetiser_schedule_ack_eliciting(ch->txp, pn_space);
+    }
+
     /* Write any data to the network due to be sent. */
     ch_tx(ch);
 
@@ -1380,6 +1388,7 @@ static int ch_rx(QUIC_CHANNEL *ch)
         ossl_qrx_pkt_release(ch->qrx_pkt);
         ch->qrx_pkt = NULL;
 
+        ch->have_sent_ack_eliciting_since_rx = 0;
         handled_any = 1;
     }
 
@@ -1569,6 +1578,8 @@ undesirable:
 /* Try to generate packets and if possible, flush them to the network. */
 static int ch_tx(QUIC_CHANNEL *ch)
 {
+    int sent_ack_eliciting = 0;
+
     if (ch->state == QUIC_CHANNEL_STATE_TERMINATING_CLOSING) {
         /*
          * While closing, only send CONN_CLOSE if we've received more traffic
@@ -1590,10 +1601,24 @@ static int ch_tx(QUIC_CHANNEL *ch)
      * flush any queued packets which we already generated.
      */
     switch (ossl_quic_tx_packetiser_generate(ch->txp,
-                                             TX_PACKETISER_ARCHETYPE_NORMAL)) {
+                                             TX_PACKETISER_ARCHETYPE_NORMAL,
+                                             &sent_ack_eliciting)) {
     case TX_PACKETISER_RES_SENT_PKT:
         ch->have_sent_any_pkt = 1; /* Packet was sent */
+
+        /*
+         * RFC 9000 s. 10.1. 'An endpoint also restarts its idle timer when
+         * sending an ack-eliciting packet if no other ack-eliciting packets
+         * have been sentsince last receiving and processing a packet.'
+         */
+        if (sent_ack_eliciting && !ch->have_sent_ack_eliciting_since_rx) {
+            ch_update_idle(ch);
+            ch->have_sent_ack_eliciting_since_rx = 1;
+        }
+
+        ch_update_ping_deadline(ch);
         break;
+
     case TX_PACKETISER_RES_NO_PKT:
         break; /* No packet was sent */
     default:
@@ -1650,6 +1675,14 @@ static OSSL_TIME ch_determine_next_tick_deadline(QUIC_CHANNEL *ch)
         deadline = ossl_time_min(deadline,
                                  ch->idle_deadline);
 
+    /*
+     * When do we need to send an ACK-eliciting packet to reset the idle
+     * deadline timer for the peer?
+     */
+    if (!ossl_time_is_infinite(ch->ping_deadline))
+        deadline = ossl_time_min(deadline,
+                                 ch->ping_deadline);
+
     return deadline;
 }
 
@@ -2051,6 +2084,30 @@ static void ch_update_idle(QUIC_CHANNEL *ch)
             ossl_ms2time(ch->max_idle_timeout));
 }
 
+/*
+ * Updates our ping deadline, which determines when we next generate a ping if
+ * we don't have any other ACK-eliciting frames to send.
+ */
+static void ch_update_ping_deadline(QUIC_CHANNEL *ch)
+{
+    if (ch->max_idle_timeout > 0) {
+        /*
+         * Maximum amount of time without traffic before we send a PING to
+         * keep the connection open. Usually we use max_idle_timeout/2, but
+         * ensure the period never exceeds 25 seconds to ensure NAT devices
+         * don't have their state time out (RFC 9000 s. 10.1.2).
+         */
+        OSSL_TIME max_span
+            = ossl_time_divide(ossl_ms2time(ch->max_idle_timeout), 2);
+
+        max_span = ossl_time_min(max_span, ossl_ms2time(25000));
+
+        ch->ping_deadline = ossl_time_add(get_time(ch), max_span);
+    } else {
+        ch->ping_deadline = ossl_time_infinite();
+    }
+}
+
 /* Called when the idle timeout expires. */
 static void ch_on_idle_timeout(QUIC_CHANNEL *ch)
 {
diff --git a/ssl/quic/quic_channel_local.h b/ssl/quic/quic_channel_local.h
index 57db9792a9..bd8f95b7a5 100644
--- a/ssl/quic/quic_channel_local.h
+++ b/ssl/quic/quic_channel_local.h
@@ -176,6 +176,12 @@ struct quic_channel_st {
     OSSL_TIME                       idle_deadline;
 
     /*
+     * Deadline at which we should send an ACK-eliciting packet to ensure
+     * idle timeout does not occur.
+     */
+    OSSL_TIME                       ping_deadline;
+
+    /*
      * State tracking. QUIC connection-level state is best represented based on
      * whether various things have happened yet or not, rather than as an
      * explicit FSM. We do have a coarse state variable which tracks the basic
@@ -270,6 +276,13 @@ struct quic_channel_st {
      * Used to determine if we need to check our RX queues again.
      */
     unsigned int                    have_new_rx_secret      : 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.
+     * 10.1).
+     */
+    unsigned int                    have_sent_ack_eliciting_since_rx    : 1;
 };
 
 # endif
diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 766a88633a..483b694c7d 100644
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -256,6 +256,14 @@ void ossl_quic_conn_set_override_now_cb(SSL *s,
     qc->override_now_cb_arg = now_cb_arg;
 }
 
+void ossl_quic_conn_force_assist_thread_wake(SSL *s)
+{
+    QUIC_CONNECTION *qc = QUIC_CONNECTION_FROM_SSL(s);
+
+    if (qc->is_thread_assisted && qc->started)
+        ossl_quic_thread_assist_notify_deadline_changed(&qc->thread_assist);
+}
+
 /*
  * QUIC Front-End I/O API: Network BIO Configuration
  * =================================================
diff --git a/test/quic_tserver_test.c b/test/quic_tserver_test.c
index a9c08e9e0c..eeb23b4b90 100644
--- a/test/quic_tserver_test.c
+++ b/test/quic_tserver_test.c
@@ -12,11 +12,13 @@
 #include "internal/common.h"
 #include "internal/sockets.h"
 #include "internal/quic_tserver.h"
+#include "internal/quic_ssl.h"
 #include "internal/time.h"
 #include "testutil.h"
 
 static const char msg1[] = "The quick brown fox jumped over the lazy dogs.";
 static char msg2[1024], msg3[1024];
+static OSSL_TIME fake_time;
 
 static const char *certfile, *keyfile;
 
@@ -29,7 +31,17 @@ static int is_want(SSL *s, int ret)
 
 static unsigned char scratch_buf[2048];
 
-static int test_tserver_actual(int use_thread_assist, int use_inject)
+static OSSL_TIME fake_now(void *arg)
+{
+    return fake_time;
+}
+
+static OSSL_TIME real_now(void *arg)
+{
+    return ossl_time_now();
+}
+
+static int do_test(int use_thread_assist, int use_fake_time, int use_inject)
 {
     int testresult = 0, ret;
     int s_fd = -1, c_fd = -1;
@@ -45,11 +57,15 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
     SSL *c_ssl = NULL;
     short port = 8186;
     int c_connected = 0, c_write_done = 0, c_begin_read = 0, s_read_done = 0;
-    int c_wait_eos = 0;
+    int c_wait_eos = 0, c_done_eos = 0;
+    int c_start_idle_test = 0, c_done_idle_test = 0;
     size_t l = 0, s_total_read = 0, s_total_written = 0, c_total_read = 0;
+    size_t idle_units_done = 0;
     int s_begin_write = 0;
     OSSL_TIME start_time;
     unsigned char alpn[] = { 8, 'o', 's', 's', 'l', 't', 'e', 's', 't' };
+    OSSL_TIME (*now_cb)(void *arg) = use_fake_time ? fake_now : real_now;
+    size_t limit_ms = 1000;
 
     ina.s_addr = htonl(0x7f000001UL);
 
@@ -84,8 +100,12 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
     if (!BIO_up_ref(s_net_bio))
         goto err;
 
+    fake_time = ossl_ms2time(1000);
+
     tserver_args.net_rbio = s_net_bio;
     tserver_args.net_wbio = s_net_bio;
+    if (use_fake_time)
+        tserver_args.now_cb = fake_now;
 
     if (!TEST_ptr(tserver = ossl_quic_tserver_new(&tserver_args, certfile,
                                                   keyfile))) {
@@ -129,6 +149,9 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
     if (!TEST_ptr(c_ssl = SSL_new(c_ctx)))
         goto err;
 
+    if (use_fake_time)
+        ossl_quic_conn_set_override_now_cb(c_ssl, fake_now, NULL);
+
     /* 0 is a success for SSL_set_alpn_protos() */
     if (!TEST_false(SSL_set_alpn_protos(c_ssl, alpn, sizeof(alpn))))
         goto err;
@@ -153,21 +176,23 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
     if (!TEST_true(SSL_set_blocking_mode(c_ssl, 0)))
         goto err;
 
-    start_time = ossl_time_now();
+    start_time = now_cb(NULL);
 
     for (;;) {
-        if (ossl_time_compare(ossl_time_subtract(ossl_time_now(), start_time),
-                              ossl_ms2time(1000)) >= 0) {
+        if (ossl_time_compare(ossl_time_subtract(now_cb(NULL), start_time),
+                              ossl_ms2time(limit_ms)) >= 0) {
             TEST_error("timeout while attempting QUIC server test");
             goto err;
         }
 
-        ret = SSL_connect(c_ssl);
-        if (!TEST_true(ret == 1 || is_want(c_ssl, ret)))
-            goto err;
+        if (!c_start_idle_test) {
+            ret = SSL_connect(c_ssl);
+            if (!TEST_true(ret == 1 || is_want(c_ssl, ret)))
+                goto err;
 
-        if (ret == 1)
-            c_connected = 1;
+            if (ret == 1)
+                c_connected = 1;
+        }
 
         if (c_connected && !c_write_done) {
             if (!TEST_int_eq(SSL_write(c_ssl, msg1, sizeof(msg1) - 1),
@@ -229,7 +254,7 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
             }
         }
 
-        if (c_wait_eos) {
+        if (c_wait_eos && !c_done_eos) {
             unsigned char c;
 
             ret = SSL_read_ex(c_ssl, &c, sizeof(c), &l);
@@ -245,16 +270,50 @@ static int test_tserver_actual(int use_thread_assist, int use_inject)
                                  SSL_ERROR_ZERO_RETURN))
                     goto err;
 
-                /* DONE */
-                break;
+                c_done_eos = 1;
+                if (use_thread_assist && use_fake_time) {
+                    if (!TEST_true(ossl_quic_tserver_is_connected(tserver)))
+                        goto err;
+                    c_start_idle_test = 1;
+                    limit_ms = 120000; /* extend time limit */
+                } else {
+                    /* DONE */
+                    break;
+                }
+            }
+        }
+
+        if (c_start_idle_test && !c_done_idle_test) {
+            /* This is more than our default idle timeout of 30s. */
+            if (idle_units_done < 600) {
+                fake_time = ossl_time_add(fake_time, ossl_ms2time(100));
+                ++idle_units_done;
+                ossl_quic_conn_force_assist_thread_wake(c_ssl);
+            } else {
+                c_done_idle_test = 1;
             }
         }
 
+        if (c_done_idle_test) {
+            /*
+             * If we have finished the fake idling duration, the connection
+             * should still be healthy in TA mode.
+             */
+            if (!TEST_true(ossl_quic_tserver_is_connected(tserver)))
+                goto err;
+
+            /* DONE */
+            break;
+        }
+
         /*
          * This is inefficient because we spin until things work without
          * blocking but this is just a test.
          */
-        SSL_tick(c_ssl);
+        if (!c_start_idle_test || c_done_idle_test)
+            /* Inhibit manual ticking during idle test to test TA mode. */
+            SSL_tick(c_ssl);
+
         ossl_quic_tserver_tick(tserver);
 
         if (use_inject) {
@@ -310,6 +369,21 @@ static int test_tserver(int idx)
     return test_tserver_actual(use_thread_assist, use_inject);
 }
 
+static int test_tserver_simple(void)
+{
+    return do_test(/*thread_assisted=*/0, /*fake_time=*/0, /*use_inject=*/0);
+}
+
+static int test_tserver_thread(void)
+{
+    return do_test(/*thread_assisted=*/1, /*fake_time=*/0, /*use_inject=*/0);
+}
+
+static int test_tserver_thread_fake_time(void)
+{
+    return do_test(/*thread_assisted=*/1, /*fake_time=*/1, /*use_inject=*/0);
+}
+
 OPT_TEST_DECLARE_USAGE("certfile privkeyfile\n")
 
 int setup_tests(void)
@@ -323,6 +397,8 @@ int setup_tests(void)
             || !TEST_ptr(keyfile = test_get_argument(1)))
         return 0;
 
-    ADD_ALL_TESTS(test_tserver, 4);
+    ADD_TEST(test_tserver_simple);
+    ADD_TEST(test_tserver_thread);
+    ADD_TEST(test_tserver_thread_fake_time);
     return 1;
 }