Simplify the QUIC time override handling

Centralise the storage of the override in the QUIC_ENGINE rather than in
the QUIC_CONNECTION. We can now set the override on any type of QUIC SSL
object as needed.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/25457)

diff --git a/fuzz/quic-client.c b/fuzz/quic-client.c
index 9550a6e86bf7a0953626457fd94d897be7f73bbd..3037bf6a271c8db4b5f461217f5788e015818ace 100644 (file)
--- a/fuzz/quic-client.c
+++ b/fuzz/quic-client.c
@@ -78,7 +78,7 @@ int FuzzerTestOneInput(const uint8_t *buf, size_t len)
         goto end;
 
     fake_now = ossl_ms2time(1);
-    if (!ossl_quic_conn_set_override_now_cb(client, fake_now_cb, NULL))
+    if (!ossl_quic_set_override_now_cb(client, fake_now_cb, NULL))
         goto end;
 
     peer_addr = BIO_ADDR_new();

diff --git a/include/internal/quic_engine.h b/include/internal/quic_engine.h
index 691793fcb07d10721a9ac697aded69daae2b666e..b2c6b38f0d55af13b87211d7aa27edb7c21ff28c 100644 (file)
--- a/include/internal/quic_engine.h
+++ b/include/internal/quic_engine.h
@@ -53,9 +53,6 @@ typedef struct quic_engine_args_st {
      */
     CRYPTO_MUTEX    *mutex;
 
-    OSSL_TIME       (*now_cb)(void *arg);
-    void            *now_cb_arg;
-
     /* Flags to pass when initialising the reactor. */
     uint64_t        reactor_flags;
 } QUIC_ENGINE_ARGS;
@@ -76,6 +73,18 @@ CRYPTO_MUTEX *ossl_quic_engine_get0_mutex(QUIC_ENGINE *qeng);
 /* Gets the current time. */
 OSSL_TIME ossl_quic_engine_get_time(QUIC_ENGINE *qeng);
 
+/*
+ * Some use cases really need actual time rather than "fake" time. Convert a
+ * fake time into a real time. If tm is before the current fake time then the
+ * current time is returned.
+ */
+OSSL_TIME ossl_quic_engine_make_real_time(QUIC_ENGINE *qeng, OSSL_TIME tm);
+
+/* Override the callback for getting the current time */
+void ossl_quic_engine_set_time_cb(QUIC_ENGINE *qeng,
+                                  OSSL_TIME (*now_cb)(void *arg),
+                                  void *now_cb_arg);
+
 /* For testing use. While enabled, ticking is not performed. */
 void ossl_quic_engine_set_inhibit_tick(QUIC_ENGINE *qeng, int inhibit);
 

diff --git a/include/internal/quic_ssl.h b/include/internal/quic_ssl.h
index fac069fe5355c08dfa9f2332e8c190bacb0213fb..2848945cd409baf1aaf59ec9314a683305e08163 100644 (file)
--- a/include/internal/quic_ssl.h
+++ b/include/internal/quic_ssl.h
@@ -124,9 +124,9 @@ __owur int ossl_quic_set_write_buffer_size(SSL *s, size_t size);
  * overridden at any time, expect strange results if you change it after
  * connecting.
  */
-int ossl_quic_conn_set_override_now_cb(SSL *s,
-                                       OSSL_TIME (*now_cb)(void *arg),
-                                       void *now_cb_arg);
+int ossl_quic_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

diff --git a/include/internal/quic_thread_assist.h b/include/internal/quic_thread_assist.h
index 592c2ffabffebda8200caf7cec4d6aa507264fe0..c9319a9376fb8bf13e7737e7e873e6b760bfa13f 100644 (file)
--- a/include/internal/quic_thread_assist.h
+++ b/include/internal/quic_thread_assist.h
@@ -47,8 +47,6 @@ typedef struct quic_thread_assist_st {
     CRYPTO_CONDVAR *cv;
     CRYPTO_THREAD *t;
     int teardown, joined;
-    OSSL_TIME (*now_cb)(void *arg);
-    void *now_cb_arg;
 } QUIC_THREAD_ASSIST;
 
 /*
@@ -58,9 +56,7 @@ typedef struct quic_thread_assist_st {
  * not affect the state of the mutex.
  */
 int ossl_quic_thread_assist_init_start(QUIC_THREAD_ASSIST *qta,
-                                       QUIC_CHANNEL *ch,
-                                       OSSL_TIME (*now_cb)(void *arg),
-                                       void *now_cb_arg);
+                                       QUIC_CHANNEL *ch);
 
 /*
  * Request the thread assist helper to begin stopping the assist thread. This

diff --git a/ssl/quic/quic_engine.c b/ssl/quic/quic_engine.c
index 6272f4b7614c9bfc1b2a9ea93368a486f0297c39..2243472226ec8d7cbacd0ad82c32067c45b98a7c 100644 (file)
--- a/ssl/quic/quic_engine.c
+++ b/ssl/quic/quic_engine.c
@@ -33,8 +33,6 @@ QUIC_ENGINE *ossl_quic_engine_new(const QUIC_ENGINE_ARGS *args)
     qeng->libctx            = args->libctx;
     qeng->propq             = args->propq;
     qeng->mutex             = args->mutex;
-    qeng->now_cb            = args->now_cb;
-    qeng->now_cb_arg        = args->now_cb_arg;
 
     if (!qeng_init(qeng, args->reactor_flags)) {
         OPENSSL_free(qeng);
@@ -84,6 +82,31 @@ OSSL_TIME ossl_quic_engine_get_time(QUIC_ENGINE *qeng)
     return qeng->now_cb(qeng->now_cb_arg);
 }
 
+OSSL_TIME ossl_quic_engine_make_real_time(QUIC_ENGINE *qeng, OSSL_TIME tm)
+{
+    OSSL_TIME offset;
+
+    if (qeng->now_cb != NULL
+            && !ossl_time_is_zero(tm)
+            && !ossl_time_is_infinite(tm)) {
+
+        offset = qeng->now_cb(qeng->now_cb_arg);
+
+        /* If tm is earlier than offset then tm will end up as "now" */
+        tm = ossl_time_add(ossl_time_subtract(tm, offset), ossl_time_now());
+    }
+
+    return tm;
+}
+
+void ossl_quic_engine_set_time_cb(QUIC_ENGINE *qeng,
+                                  OSSL_TIME (*now_cb)(void *arg),
+                                  void *now_cb_arg)
+{
+    qeng->now_cb = now_cb;
+    qeng->now_cb_arg = now_cb_arg;
+}
+
 void ossl_quic_engine_set_inhibit_tick(QUIC_ENGINE *qeng, int inhibit)
 {
     qeng->inhibit_tick = (inhibit != 0);

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 671426e04fa38afc8f26a5ea3d1235cb3ee445a8..85a130351f610487b9a81b6970865320367c4bf3 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -44,26 +44,6 @@ static int quic_mutation_allowed(QUIC_CONNECTION *qc, int req_active);
 static void qctx_maybe_autotick(QCTX *ctx);
 static int qctx_should_autotick(QCTX *ctx);
 
-/*
- * QUIC Front-End I/O API: Common Utilities
- * ========================================
- */
-
-static OSSL_TIME get_time(QUIC_CONNECTION *qc)
-{
-    if (qc->override_now_cb != NULL)
-        return qc->override_now_cb(qc->override_now_cb_arg);
-    else
-        return ossl_time_now();
-}
-
-static OSSL_TIME get_time_cb(void *arg)
-{
-    QUIC_CONNECTION *qc = arg;
-
-    return get_time(qc);
-}
-
 /*
  * QCTX is a utility structure which provides information we commonly wish to
  * unwrap upon an API call being dispatched to us, namely:
@@ -867,19 +847,18 @@ int ossl_quic_clear(SSL *s)
     return 0;
 }
 
-int ossl_quic_conn_set_override_now_cb(SSL *s,
-                                       OSSL_TIME (*now_cb)(void *arg),
-                                       void *now_cb_arg)
+int ossl_quic_set_override_now_cb(SSL *s,
+                                  OSSL_TIME (*now_cb)(void *arg),
+                                  void *now_cb_arg)
 {
     QCTX ctx;
 
-    if (!expect_quic_conn_only(s, &ctx))
+    if (!expect_quic_any(s, &ctx))
         return 0;
 
     qctx_lock(&ctx);
 
-    ctx.qc->override_now_cb     = now_cb;
-    ctx.qc->override_now_cb_arg = now_cb_arg;
+    ossl_quic_engine_set_time_cb(ctx.obj->engine, now_cb, now_cb_arg);
 
     qctx_unlock(&ctx);
     return 1;
@@ -1354,15 +1333,7 @@ int ossl_quic_get_event_timeout(SSL *s, struct timeval *tv, int *is_infinite)
         return 1;
     }
 
-    /*
-     * TODO(QUIC SERVER): The clock override callback belongs at the engine
-     * (DOMAIN) not the QC layer, which would make `basetime` independent of
-     * the object type.
-     */
-    if (ctx.qc)
-        basetime = get_time(ctx.qc);
-    else
-        basetime = ossl_time_now();
+    basetime = ossl_quic_engine_get_time(ctx.obj->engine);
 
     qctx_unlock(&ctx);
 
@@ -1791,8 +1762,7 @@ static int create_channel(QUIC_CONNECTION *qc, SSL_CTX *ctx)
 #if defined(OPENSSL_THREADS)
     engine_args.mutex         = qc->mutex;
 #endif
-    engine_args.now_cb        = get_time_cb;
-    engine_args.now_cb_arg    = qc;
+
     if (need_notifier_for_domain_flags(ctx->domain_flags))
         engine_args.reactor_flags |= QUIC_REACTOR_FLAG_USE_NOTIFIER;
 
@@ -1846,9 +1816,7 @@ static int ensure_channel_started(QCTX *ctx)
 
 #if !defined(OPENSSL_NO_QUIC_THREAD_ASSIST)
         if (qc->is_thread_assisted)
-            if (!ossl_quic_thread_assist_init_start(&qc->thread_assist, qc->ch,
-                                                    qc->override_now_cb,
-                                                    qc->override_now_cb_arg)) {
+            if (!ossl_quic_thread_assist_init_start(&qc->thread_assist, qc->ch)) {
                 QUIC_RAISE_NON_NORMAL_ERROR(ctx, ERR_R_INTERNAL_ERROR,
                                             "failed to start assist thread");
                 return 0;
@@ -4266,6 +4234,7 @@ SSL *ossl_quic_new_listener(SSL_CTX *ctx, uint64_t flags)
 #if defined(OPENSSL_THREADS)
     engine_args.mutex   = ql->mutex;
 #endif
+
     if (need_notifier_for_domain_flags(ctx->domain_flags))
         engine_args.reactor_flags |= QUIC_REACTOR_FLAG_USE_NOTIFIER;
 
@@ -4589,6 +4558,7 @@ SSL *ossl_quic_new_domain(SSL_CTX *ctx, uint64_t flags)
 #if defined(OPENSSL_THREADS)
     engine_args.mutex   = qd->mutex;
 #endif
+
     if (need_notifier_for_domain_flags(domain_flags))
         engine_args.reactor_flags |= QUIC_REACTOR_FLAG_USE_NOTIFIER;
 

diff --git a/ssl/quic/quic_local.h b/ssl/quic/quic_local.h
index b3f68ff34934baaaae39317a9cb3aca71e37a23f..dbd5c8cb4745fccb44d51f3b8c8f8a1d5c8b3b55 100644 (file)
--- a/ssl/quic/quic_local.h
+++ b/ssl/quic/quic_local.h
@@ -167,10 +167,6 @@ struct quic_conn_st {
     QUIC_THREAD_ASSIST              thread_assist;
 #  endif
 
-    /* If non-NULL, used instead of ossl_time_now(). Used for testing. */
-    OSSL_TIME                       (*override_now_cb)(void *arg);
-    void                            *override_now_cb_arg;
-
     /* Number of XSOs allocated. Includes the default XSO, if any. */
     size_t                          num_xso;
 

diff --git a/ssl/quic/quic_thread_assist.c b/ssl/quic/quic_thread_assist.c
index 26c738cb5cfa644a7dbad129d8dad65c318a4a71..0604a14a037c41152d6a6c8e57840b7a62973eaa 100644 (file)
--- a/ssl/quic/quic_thread_assist.c
+++ b/ssl/quic/quic_thread_assist.c
@@ -22,6 +22,7 @@ static unsigned int assist_thread_main(void *arg)
     QUIC_THREAD_ASSIST *qta = arg;
     CRYPTO_MUTEX *m = ossl_quic_channel_get_mutex(qta->ch);
     QUIC_REACTOR *rtor;
+    QUIC_ENGINE *eng = ossl_quic_channel_get0_engine(qta->ch);
 
     ossl_crypto_mutex_lock(m);
 
@@ -34,17 +35,11 @@ static unsigned int assist_thread_main(void *arg)
             break;
 
         deadline = ossl_quic_reactor_get_tick_deadline(rtor);
-        if (qta->now_cb != NULL
-                && !ossl_time_is_zero(deadline)
-                && !ossl_time_is_infinite(deadline)) {
-            /*
-             * ossl_crypto_condvar_wait_timeout needs to use real time for the
-             * deadline
-             */
-            deadline = ossl_time_add(ossl_time_subtract(deadline,
-                                                        qta->now_cb(qta->now_cb_arg)),
-                                     ossl_time_now());
-        }
+        /*
+         * ossl_crypto_condvar_wait_timeout needs to use real time for the
+         * deadline
+         */
+        deadline = ossl_quic_engine_make_real_time(eng, deadline);
         ossl_crypto_condvar_wait_timeout(qta->cv, m, deadline);
 
         /*
@@ -69,9 +64,7 @@ static unsigned int assist_thread_main(void *arg)
 }
 
 int ossl_quic_thread_assist_init_start(QUIC_THREAD_ASSIST *qta,
-                                       QUIC_CHANNEL *ch,
-                                       OSSL_TIME (*now_cb)(void *arg),
-                                       void *now_cb_arg)
+                                       QUIC_CHANNEL *ch)
 {
     CRYPTO_MUTEX *mutex = ossl_quic_channel_get_mutex(ch);
 
@@ -81,8 +74,6 @@ int ossl_quic_thread_assist_init_start(QUIC_THREAD_ASSIST *qta,
     qta->ch         = ch;
     qta->teardown   = 0;
     qta->joined     = 0;
-    qta->now_cb     = now_cb;
-    qta->now_cb_arg = now_cb_arg;
 
     qta->cv = ossl_crypto_condvar_new();
     if (qta->cv == NULL)

diff --git a/ssl/quic/quic_tserver.c b/ssl/quic/quic_tserver.c
index 4f30eb14cec8de257671a4989a530a013909784a..37ada76a76da09563fc6498441f49dd28a1729b7 100644 (file)
--- a/ssl/quic/quic_tserver.c
+++ b/ssl/quic/quic_tserver.c
@@ -122,12 +122,13 @@ QUIC_TSERVER *ossl_quic_tserver_new(const QUIC_TSERVER_ARGS *args,
     engine_args.libctx          = srv->args.libctx;
     engine_args.propq           = srv->args.propq;
     engine_args.mutex           = srv->mutex;
-    engine_args.now_cb          = srv->args.now_cb;
-    engine_args.now_cb_arg      = srv->args.now_cb_arg;
 
     if ((srv->engine = ossl_quic_engine_new(&engine_args)) == NULL)
         goto err;
 
+    ossl_quic_engine_set_time_cb(srv->engine, srv->args.now_cb,
+                                 srv->args.now_cb_arg);
+
     port_args.channel_ctx       = srv->ctx;
     port_args.is_multi_conn     = 1;
 

diff --git a/test/helpers/quictestlib.c b/test/helpers/quictestlib.c
index 680606e59d252f8005f46c474847ad51679f3389..dbad8afe597c3bd15764e5a141c7cd20e74a7a7f 100644 (file)
--- a/test/helpers/quictestlib.c
+++ b/test/helpers/quictestlib.c
@@ -306,7 +306,7 @@ int qtest_create_quic_objects(OSSL_LIB_CTX *libctx, SSL_CTX *clientctx,
         using_fake_time = 1;
         qtest_reset_time();
         tserver_args.now_cb = fake_now_cb;
-        (void)ossl_quic_conn_set_override_now_cb(*cssl, fake_now_cb, NULL);
+        (void)ossl_quic_set_override_now_cb(*cssl, fake_now_cb, NULL);
     } else {
         using_fake_time = 0;
     }

diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c
index 9fac202abecc1a160214fb4ce4e9b735a1fd301f..9b90d36d9f7ea98c47d53b909fd25431c904475f 100644 (file)
--- a/test/quic_multistream_test.c
+++ b/test/quic_multistream_test.c
@@ -795,7 +795,7 @@ static int helper_init(struct helper *h, const char *script_name,
         goto err;
 
     /* Use custom time function for virtual time skip. */
-    if (!TEST_true(ossl_quic_conn_set_override_now_cb(h->c_conn, get_time, h)))
+    if (!TEST_true(ossl_quic_set_override_now_cb(h->c_conn, get_time, h)))
         goto err;
 
     /* Takes ownership of our reference to the BIO. */

diff --git a/test/quic_tserver_test.c b/test/quic_tserver_test.c
index 330827ab5ae195d3a1a8a934e6dad25925af407f..0b91e2f52449326f184e9647119a2dabd6971416 100644 (file)
--- a/test/quic_tserver_test.c
+++ b/test/quic_tserver_test.c
@@ -166,7 +166,7 @@ static int do_test(int use_thread_assist, int use_fake_time, int use_inject)
         goto err;
 
     if (use_fake_time)
-        if (!TEST_true(ossl_quic_conn_set_override_now_cb(c_ssl, fake_now, NULL)))
+        if (!TEST_true(ossl_quic_set_override_now_cb(c_ssl, fake_now, NULL)))
             goto err;
 
     /* 0 is a success for SSL_set_alpn_protos() */