From 3b1ab5a3a0a10798ea9a1547b6cb50182edaeb5b Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Tue, 21 Feb 2023 10:18:59 +0000 Subject: [PATCH] Enhance quic_tserver test to fully test thread assisted mode Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/20348) --- include/internal/quic_ssl.h | 7 +++ ssl/quic/quic_channel.c | 59 ++++++++++++++++++- ssl/quic/quic_channel_local.h | 13 +++++ ssl/quic/quic_impl.c | 8 +++ test/quic_tserver_test.c | 106 +++++++++++++++++++++++++++++----- 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 @@ -175,6 +175,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 @@ -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; }