From: Andrew Dinh Date: Sun, 12 Jul 2026 16:50:05 +0000 (+0700) Subject: Wire radix QUIC test framework clock to simulated time X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fcba83dc4d2a50581466eadd1598e1dd9c0cea03;p=thirdparty%2Fopenssl.git Wire radix QUIC test framework clock to simulated time script_17/18/19 (key update tests) rely on OP_SKIP_TIME to advance simulated time so that TXKU cooldown/RTT-gated behaviour can be exercised deterministically, but the framework never actually plumbed that simulated clock into the QUIC channels under test -- OP_SKIP_TIME only affected the terp's own bookkeeping, so real key updates only ever occurred by chance regardless of how much time was "skipped". Fix this by: - Calling ossl_quic_set_override_now_cb() in hf_new_ssl() so each QUIC channel actually uses the framework's simulated clock, matching what the older quictestlib.c harness already did via fake_now_cb(). - Activating the server-side connection object in hf_accept_conn() so it gets ticked (SSL_handle_events()) like every other object, driving its internal key-update/timer processing forward. Two correctness issues had to be addressed to make this clock plumbing safe and deterministic: - The clock handed to QUIC channels must be purely virtual (a fixed base time plus an explicit slip that only advances via radix_skip_time()/OP_SKIP_TIME), not real wall-clock time, or protocol-timing-sensitive assertions become flaky depending on how long real cryptographic work takes on a given machine (e.g. much slower on ASAN+UBSAN debug builds), occasionally triggering a spurious second TXKU before the first is confirmed. radix_process is a single static struct reused across every script in the suite, so time_slip must be reset in RADIX_PROCESS_init() -- otherwise a script would inherit the previous script's accumulated slip and see time jump forward all at once for a brand new connection. The virtual clock also needs a small, fixed per-tick advance (do_per_op(), mirroring the old harness's qtest_add_time(1) in its own connect-wait loop), since QUIC's internal timers need to observe time actually passing to make progress during a busy-wait such as hf_connect_wait spinning on SSL_connect(). - time_slip needs its own dedicated mutex (time_m) rather than being protected by the existing global mutex (gm): get_time() is now called by libssl/QUIC's internals (e.g. from within the reactor tick while holding QUIC's own locks), whereas gm is held by test code across calls into libssl (e.g. hf_clear() holds gm while calling SSL_free()). Sharing gm would take gm and QUIC's internal lock in opposite orders across those two paths, risking a real deadlock (confirmed via a ThreadSanitizer lock-order-inversion report). - The terp's own script-execution watchdog (max_execution_time) needs its own now_cb (terp_now) based on real wall-clock time, so that OP_SKIP_TIME doesn't eat into its execution budget, and so it can still catch a genuinely stuck script independent of the virtual protocol clock. Some scripts also legitimately need a larger budget than the terp default on slow or heavily instrumented CI machines. Assisted-by: Claude:claude-sonnet-5 Reviewed-by: Saša Nedvědický Reviewed-by: Neil Horman Reviewed-by: Bob Beck MergeDate: Tue Jul 21 09:22:53 2026 (Merged from https://github.com/openssl/openssl/pull/31889) --- diff --git a/test/radix/quic_bindings.c b/test/radix/quic_bindings.c index 7dc46f7950d..63bcecad99b 100644 --- a/test/radix/quic_bindings.c +++ b/test/radix/quic_bindings.c @@ -67,9 +67,12 @@ typedef struct radix_process_st { /* Process-global state. */ CRYPTO_MUTEX *gm; /* global mutex */ LHASH_OF(RADIX_OBJ) *objs; /* protected by gm */ - OSSL_TIME time_slip; /* protected by gm */ BIO *keylog_out; /* protected by gm */ + CRYPTO_MUTEX *time_m; + OSSL_TIME base_time; /* set once at init, constant thereafter */ + OSSL_TIME time_slip; /* protected by time_m */ + int done_join_all_threads; /* @@ -178,6 +181,8 @@ static int RADIX_PROCESS_init(RADIX_PROCESS *rp, size_t node_idx, size_t process #if defined(OPENSSL_THREADS) if (!TEST_ptr(rp->gm = ossl_crypto_mutex_new())) goto err; + if (!TEST_ptr(rp->time_m = ossl_crypto_mutex_new())) + goto err; #endif if (!TEST_ptr(rp->objs = lh_RADIX_OBJ_new(RADIX_OBJ_hash, RADIX_OBJ_cmp))) @@ -196,12 +201,15 @@ static int RADIX_PROCESS_init(RADIX_PROCESS *rp, size_t node_idx, size_t process rp->process_idx = process_idx; rp->done_join_all_threads = 0; rp->next_thread_idx = 0; + rp->base_time = ossl_time_now(); + rp->time_slip = ossl_time_zero(); return 1; err: lh_RADIX_OBJ_free(rp->objs); rp->objs = NULL; ossl_crypto_mutex_free(&rp->gm); + ossl_crypto_mutex_free(&rp->time_m); return 0; } @@ -456,6 +464,7 @@ static void RADIX_PROCESS_cleanup(RADIX_PROCESS *rp) BIO_free_all(rp->keylog_out); rp->keylog_out = NULL; ossl_crypto_mutex_free(&rp->gm); + ossl_crypto_mutex_free(&rp->time_m); } static RADIX_OBJ *RADIX_PROCESS_get_obj(RADIX_PROCESS *rp, const char *name) @@ -681,18 +690,23 @@ static OSSL_TIME get_time(void *arg) { OSSL_TIME time_slip; - ossl_crypto_mutex_lock(RP()->gm); + ossl_crypto_mutex_lock(RP()->time_m); time_slip = RP()->time_slip; - ossl_crypto_mutex_unlock(RP()->gm); + ossl_crypto_mutex_unlock(RP()->time_m); - return ossl_time_add(ossl_time_now(), time_slip); + return ossl_time_add(RP()->base_time, time_slip); } -ossl_unused static void radix_skip_time(OSSL_TIME t) +static OSSL_TIME terp_now(void *arg) { - ossl_crypto_mutex_lock(RP()->gm); + return ossl_time_now(); +} + +static void radix_skip_time(OSSL_TIME t) +{ + ossl_crypto_mutex_lock(RP()->time_m); RP()->time_slip = ossl_time_add(RP()->time_slip, t); - ossl_crypto_mutex_unlock(RP()->gm); + ossl_crypto_mutex_unlock(RP()->time_m); } static void per_op_tick_obj(RADIX_OBJ *obj) @@ -705,14 +719,17 @@ static void per_op_tick_obj(RADIX_OBJ *obj) static int do_per_op(TERP *terp, void *arg) { + radix_skip_time(ossl_ms2time(1)); lh_RADIX_OBJ_doall(RP()->objs, per_op_tick_obj); return 1; } static int bindings_adjust_terp_config(TERP_CONFIG *cfg) { - cfg->now_cb = get_time; + cfg->now_cb = terp_now; cfg->per_op_cb = do_per_op; + + cfg->max_execution_time = ossl_ms2time(60000); return 1; } diff --git a/test/radix/quic_ops.c b/test/radix/quic_ops.c index f0229075963..ed129d0ff7f 100644 --- a/test/radix/quic_ops.c +++ b/test/radix/quic_ops.c @@ -204,6 +204,11 @@ DEF_FUNC(hf_new_ssl) if (!is_domain && !TEST_true(ssl_attach_bio_dgram(ssl, 0, NULL))) goto err; + if (!TEST_true(ossl_quic_set_override_now_cb(ssl, get_time, NULL))) { + SSL_free(ssl); + goto err; + } + if (!TEST_true(RADIX_PROCESS_set_ssl(RP(), name, ssl))) { SSL_free(ssl); goto err; @@ -342,6 +347,7 @@ DEF_FUNC(hf_accept_conn) SSL_free(conn); goto err; } + radix_activate_obj(RADIX_PROCESS_get_obj(RP(), conn_name)); ok = 1; err: