]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Wire radix QUIC test framework clock to simulated time
authorAndrew Dinh <andrewd@openssl.org>
Sun, 12 Jul 2026 16:50:05 +0000 (23:50 +0700)
committerNorbert Pocs <norbertp@openssl.org>
Tue, 21 Jul 2026 09:22:31 +0000 (11:22 +0200)
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ý <sashan@openssl.org>
Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Bob Beck <beck@openssl.org>
MergeDate: Tue Jul 21 09:22:53 2026
(Merged from https://github.com/openssl/openssl/pull/31889)

test/radix/quic_bindings.c
test/radix/quic_ops.c

index 7dc46f7950d7daba1d31b1a31b353615bd5f7434..63bcecad99b215b7c96416c626cb3b5e9c0eb3c0 100644 (file)
@@ -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;
 }
 
index f0229075963da797d24b60bd9bf20cd108934d38..ed129d0ff7fbea2512bf19ffbf763496de2e332e 100644 (file)
@@ -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: