From: Matt Caswell Date: Fri, 13 Sep 2024 15:00:22 +0000 (+0100) Subject: Simplify the QUIC time override handling X-Git-Tag: openssl-3.5.0-alpha1~359 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ac69d0649a4cb92b79efd1b7ae8eda413468ef9d;p=thirdparty%2Fopenssl.git 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 Reviewed-by: Viktor Dukhovni (Merged from https://github.com/openssl/openssl/pull/25457) --- diff --git a/fuzz/quic-client.c b/fuzz/quic-client.c index 9550a6e86bf..3037bf6a271 100644 --- 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 691793fcb07..b2c6b38f0d5 100644 --- 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 fac069fe535..2848945cd40 100644 --- 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 592c2ffabff..c9319a9376f 100644 --- 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 6272f4b7614..2243472226e 100644 --- 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 671426e04fa..85a130351f6 100644 --- 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 b3f68ff3493..dbd5c8cb474 100644 --- 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 26c738cb5cf..0604a14a037 100644 --- 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 4f30eb14cec..37ada76a76d 100644 --- 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 680606e59d2..dbad8afe597 100644 --- 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 9fac202abec..9b90d36d9f7 100644 --- 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 330827ab5ae..0b91e2f5244 100644 --- 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() */