QUIC APL: Correct implementation of time callback override

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

diff --git a/include/internal/quic_ssl.h b/include/internal/quic_ssl.h
index f8469c4fa74b8094f450f02bb2e788edfe2d1980..7ea5ce8063229c68a49b3a5526264bcf1a068c81 100644 (file)
--- a/include/internal/quic_ssl.h
+++ b/include/internal/quic_ssl.h
@@ -93,12 +93,13 @@ __owur int ossl_quic_get_conn_close_info(SSL *ssl,
                                          size_t info_len);
 
 /*
- * Used to override ossl_time_now() for debug purposes. Must be called before
+ * Used to override ossl_time_now() for debug purposes. While this may be
+ * overridden at any time, expect strange results if you change it after
  * connecting.
  */
-void ossl_quic_conn_set_override_now_cb(SSL *s,
-                                        OSSL_TIME (*now_cb)(void *arg),
-                                        void *now_cb_arg);
+int 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

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index d00f4455b0172c6d4dec595aa773299581eaaf14..9cfc253bdccf5870aec47c2a54f1826279e665e3 100644 (file)
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -59,6 +59,21 @@ static int block_until_pred(QUIC_CONNECTION *qc,
                                               qc->mutex);
 }
 
+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:
@@ -490,17 +505,22 @@ int ossl_quic_clear(SSL *s)
     return 1;
 }
 
-void ossl_quic_conn_set_override_now_cb(SSL *s,
-                                        OSSL_TIME (*now_cb)(void *arg),
-                                        void *now_cb_arg)
+int ossl_quic_conn_set_override_now_cb(SSL *s,
+                                       OSSL_TIME (*now_cb)(void *arg),
+                                       void *now_cb_arg)
 {
     QCTX ctx;
 
     if (!expect_quic(s, &ctx))
-        return;
+        return 0;
+
+    quic_lock(ctx.qc);
 
     ctx.qc->override_now_cb     = now_cb;
     ctx.qc->override_now_cb_arg = now_cb_arg;
+
+    quic_unlock(ctx.qc);
+    return 1;
 }
 
 void ossl_quic_conn_force_assist_thread_wake(SSL *s)
@@ -889,7 +909,7 @@ int ossl_quic_get_event_timeout(SSL *s, struct timeval *tv, int *is_infinite)
         return 1;
     }
 
-    *tv = ossl_time_to_timeval(ossl_time_subtract(deadline, ossl_time_now()));
+    *tv = ossl_time_to_timeval(ossl_time_subtract(deadline, get_time(ctx.qc)));
     *is_infinite = 0;
     quic_unlock(ctx.qc);
     return 1;
@@ -1146,8 +1166,8 @@ static int create_channel(QUIC_CONNECTION *qc)
     args.is_server  = qc->as_server;
     args.tls        = qc->tls;
     args.mutex      = qc->mutex;
-    args.now_cb     = qc->override_now_cb;
-    args.now_cb_arg = qc->override_now_cb_arg;
+    args.now_cb     = get_time_cb;
+    args.now_cb_arg = qc;
 
     qc->ch = ossl_quic_channel_new(&args);
     if (qc->ch == NULL)

diff --git a/test/quic_tserver_test.c b/test/quic_tserver_test.c
index eefb5ae743b400e2807b8c319a5ee540deda7a35..beed40d76c66a6d079388cecd735ef0a374dfa7f 100644 (file)
--- a/test/quic_tserver_test.c
+++ b/test/quic_tserver_test.c
@@ -165,7 +165,8 @@ static int do_test(int use_thread_assist, int use_fake_time, int use_inject)
         goto err;
 
     if (use_fake_time)
-        ossl_quic_conn_set_override_now_cb(c_ssl, fake_now, NULL);
+        if (!TEST_true(ossl_quic_conn_set_override_now_cb(c_ssl, fake_now, NULL)))
+            goto err;
 
     /* 0 is a success for SSL_set_alpn_protos() */
     if (!TEST_false(SSL_set_alpn_protos(c_ssl, alpn, sizeof(alpn))))