From e3e9794aa49f61e5b034608488034daa01125c85 Mon Sep 17 00:00:00 2001 From: Hugo Landau Date: Tue, 23 May 2023 12:23:06 +0100 Subject: [PATCH] QUIC APL: Correct implementation of time callback override Reviewed-by: Tomas Mraz Reviewed-by: Matt Caswell Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/21029) --- include/internal/quic_ssl.h | 9 +++++---- ssl/quic/quic_impl.c | 34 +++++++++++++++++++++++++++------- test/quic_tserver_test.c | 3 ++- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/include/internal/quic_ssl.h b/include/internal/quic_ssl.h index f8469c4fa7..7ea5ce8063 100644 --- 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 d00f4455b0..9cfc253bdc 100644 --- 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 eefb5ae743..beed40d76c 100644 --- 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))))