diff --git a/crypto/thread/arch/thread_win.c b/crypto/thread/arch/thread_win.c index f647c68c0a..7f44d1f5cf 100644 --- a/crypto/thread/arch/thread_win.c +++ b/crypto/thread/arch/thread_win.c @@ -160,7 +160,6 @@ static int determine_timeout(OSSL_TIME deadline, DWORD *w_timeout_p) ms = ossl_time2ms(delta); - /* * Amount of time we want to wait is too long for the 32-bit argument to * the Win32 API, so just wait as long as possible. @@ -288,7 +287,7 @@ void ossl_crypto_condvar_free(CRYPTO_CONDVAR **cv) *cv_p = NULL; } -#endif +# endif void ossl_crypto_mem_barrier(void) { diff --git a/doc/designs/quic-design/quic-thread-assist.md b/doc/designs/quic-design/quic-thread-assist.md index 81a6e28a30..e26a00a7cb 100644 --- a/doc/designs/quic-design/quic-thread-assist.md +++ b/doc/designs/quic-design/quic-thread-assist.md @@ -1,7 +1,7 @@ QUIC Thread Assisted Mode Synchronisation Requirements ====================================================== -In thread assisted mode, we spin up a background thread to ensure that periodic +In thread assisted mode, we create a background thread to ensure that periodic QUIC processing is handled in a timely fashion regardless of whether an application is frequently calling (or blocked in) SSL API I/O functions. diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h index bba7256d03..d60e4010bd 100644 --- a/include/internal/quic_channel.h +++ b/include/internal/quic_channel.h @@ -81,7 +81,6 @@ * * Precondition: must not hold channel mutex (unchecked) * Postcondition: channel mutex is not held (by calling thread) - * */ # define QUIC_TAKES_LOCK diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index 9d8acd17fd..a873054f90 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -28,6 +28,13 @@ #define INIT_CRYPTO_BUF_LEN 8192 #define INIT_APP_BUF_LEN 8192 +/* + * Interval before we force a PING to ensure NATs don't timeout. This is based + * on the lowest commonly seen value of 30 seconds as cited in RFC 9000 s. + * 10.1.2. + */ +#define MAX_NAT_INTERVAL (ossl_ms2time(25000)) + static void ch_rx_pre(QUIC_CHANNEL *ch); static int ch_rx(QUIC_CHANNEL *ch); static int ch_tx(QUIC_CHANNEL *ch); @@ -1221,7 +1228,7 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags) { OSSL_TIME now, deadline; QUIC_CHANNEL *ch = arg; - int channel_only = ((flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0); + int channel_only = (flags & QUIC_REACTOR_TICK_FLAG_CHANNEL_ONLY) != 0; /* * When we tick the QUIC connection, we do everything we need to do @@ -2092,15 +2099,15 @@ static void ch_update_ping_deadline(QUIC_CHANNEL *ch) { if (ch->max_idle_timeout > 0) { /* - * Maximum amount of time without traffic before we send a PING to - * keep the connection open. Usually we use max_idle_timeout/2, but - * ensure the period never exceeds 25 seconds to ensure NAT devices - * don't have their state time out (RFC 9000 s. 10.1.2). + * Maximum amount of time without traffic before we send a PING to keep + * the connection open. Usually we use max_idle_timeout/2, but ensure + * the period never exceeds the assumed NAT interval to ensure NAT + * devices don't have their state time out (RFC 9000 s. 10.1.2). */ OSSL_TIME max_span = ossl_time_divide(ossl_ms2time(ch->max_idle_timeout), 2); - max_span = ossl_time_min(max_span, ossl_ms2time(25000)); + max_span = ossl_time_min(max_span, MAX_NAT_INTERVAL); ch->ping_deadline = ossl_time_add(get_time(ch), max_span); } else { diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index b293bf7bd9..e510918bf7 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -202,7 +202,7 @@ void ossl_quic_free(SSL *s) /* Note: SSL_free calls OPENSSL_free(qc) for us */ SSL_free(qc->tls); - ossl_crypto_mutex_free(&qc->mutex); + ossl_crypto_mutex_free(&qc->mutex); /* freed while still locked */ } /* SSL method init */ @@ -529,8 +529,10 @@ int ossl_quic_get_net_read_desired(QUIC_CONNECTION *qc) quic_lock(qc); - if (qc->ch == NULL) + if (qc->ch == NULL) { + quic_unlock(qc); return 0; + } ret = ossl_quic_reactor_net_read_desired(ossl_quic_channel_get_reactor(qc->ch)); quic_unlock(qc); @@ -545,8 +547,10 @@ int ossl_quic_get_net_write_desired(QUIC_CONNECTION *qc) quic_lock(qc); - if (qc->ch == NULL) + if (qc->ch == NULL) { + quic_unlock(qc); return 0; + } ret = ossl_quic_reactor_net_write_desired(ossl_quic_channel_get_reactor(qc->ch)); quic_unlock(qc); diff --git a/ssl/quic/quic_local.h b/ssl/quic/quic_local.h index 9aad63b742..4d6d18ae37 100644 --- a/ssl/quic/quic_local.h +++ b/ssl/quic/quic_local.h @@ -66,10 +66,10 @@ struct quic_conn_st { /* Initial peer L4 address. */ BIO_ADDR init_peer_addr; -#ifndef OPENSSL_NO_QUIC_THREAD_ASSIST +# ifndef OPENSSL_NO_QUIC_THREAD_ASSIST /* Manages thread for QUIC thread assisted mode. */ QUIC_THREAD_ASSIST thread_assist; -#endif +# endif /* If non-NULL, used instead of ossl_time_now(). Used for testing. */ OSSL_TIME (*override_now_cb)(void *arg); diff --git a/ssl/quic/quic_reactor.c b/ssl/quic/quic_reactor.c index b6490f2459..853d62f947 100644 --- a/ssl/quic/quic_reactor.c +++ b/ssl/quic/quic_reactor.c @@ -176,7 +176,7 @@ static int poll_two_fds(int rfd, int rfd_want_read, return 0; if (mutex != NULL) - CRYPTO_THREAD_unlock(mutex); + ossl_crypto_mutex_unlock(mutex); do { /* @@ -200,8 +200,8 @@ static int poll_two_fds(int rfd, int rfd_want_read, pres = select(maxfd + 1, &rfd_set, &wfd_set, &efd_set, ptv); } while (pres == -1 && get_last_socket_error_is_eintr()); - if (mutex != NULL && !CRYPTO_THREAD_write_lock(mutex)) - return 0; + if (mutex != NULL) + ossl_crypto_mutex_lock(mutex); return pres < 0 ? 0 : 1; #else @@ -233,7 +233,7 @@ static int poll_two_fds(int rfd, int rfd_want_read, return 0; if (mutex != NULL) - CRYPTO_THREAD_unlock(mutex); + ossl_crypto_mutex_unlock(mutex); do { if (ossl_time_is_infinite(deadline)) { @@ -247,8 +247,8 @@ static int poll_two_fds(int rfd, int rfd_want_read, pres = poll(pfds, npfd, timeout_ms); } while (pres == -1 && get_last_socket_error_is_eintr()); - if (mutex != NULL && !CRYPTO_THREAD_write_lock(mutex)) - return 0; + if (mutex != NULL) + ossl_crypto_mutex_lock(mutex); return pres < 0 ? 0 : 1; #endif diff --git a/ssl/quic/quic_tserver.c b/ssl/quic/quic_tserver.c index a0660458d4..a94a6b9dbe 100644 --- a/ssl/quic/quic_tserver.c +++ b/ssl/quic/quic_tserver.c @@ -70,7 +70,7 @@ QUIC_TSERVER *ossl_quic_tserver_new(const QUIC_TSERVER_ARGS *args, srv->args = *args; - if ((srv->mutex = CRYPTO_THREAD_lock_new()) == NULL) + if ((srv->mutex = ossl_crypto_mutex_new()) == NULL) goto err; srv->ctx = SSL_CTX_new_ex(srv->args.libctx, srv->args.propq, TLS_method()); @@ -111,8 +111,10 @@ QUIC_TSERVER *ossl_quic_tserver_new(const QUIC_TSERVER_ARGS *args, return srv; err: - if (srv != NULL) + if (srv != NULL) { ossl_quic_channel_free(srv->ch); + ossl_crypto_mutex_free(&srv->mutex); + } OPENSSL_free(srv); return NULL; @@ -128,7 +130,7 @@ void ossl_quic_tserver_free(QUIC_TSERVER *srv) BIO_free(srv->args.net_wbio); SSL_free(srv->tls); SSL_CTX_free(srv->ctx); - CRYPTO_THREAD_lock_free(srv->mutex); + ossl_crypto_mutex_free(&srv->mutex); OPENSSL_free(srv); } diff --git a/test/quic_tserver_test.c b/test/quic_tserver_test.c index 359b66c1b2..a385381716 100644 --- a/test/quic_tserver_test.c +++ b/test/quic_tserver_test.c @@ -374,29 +374,20 @@ err: static int test_tserver(int idx) { - int use_thread_assist, use_inject; + int thread_assisted, use_fake_time, use_inject; - use_thread_assist = idx % 2; + thread_assisted = idx % 2; idx /= 2; use_inject = idx % 2; + idx /= 2; - return test_tserver_actual(use_thread_assist, use_inject); -} + use_fake_time = idx % 2; -static int test_tserver_simple(void) -{ - return do_test(/*thread_assisted=*/0, /*fake_time=*/0, /*use_inject=*/0); -} + if (use_fake_time && !thread_assisted) + return 1; -static int test_tserver_thread(void) -{ - return do_test(/*thread_assisted=*/1, /*fake_time=*/0, /*use_inject=*/0); -} - -static int test_tserver_thread_fake_time(void) -{ - return do_test(/*thread_assisted=*/1, /*fake_time=*/1, /*use_inject=*/0); + return do_test(thread_assisted, use_fake_time, use_inject); } OPT_TEST_DECLARE_USAGE("certfile privkeyfile\n") @@ -415,8 +406,6 @@ int setup_tests(void) if ((fake_time_lock = CRYPTO_THREAD_lock_new()) == NULL) return 0; - ADD_TEST(test_tserver_simple); - ADD_TEST(test_tserver_thread); - ADD_TEST(test_tserver_thread_fake_time); + ADD_ALL_TESTS(test_tserver, 2 * 2 * 2); return 1; }