From 5e33f306aebb96f6373d344a79b3df5d18c69797 Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Thu, 20 Nov 2025 10:07:19 -0500 Subject: [PATCH] Various fixups for SSL_listen_ex (readability/error unwinding) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Various review fixups to clarify meaning of variables and fix unwinding of operations should we encounter errors in some operations. Reviewed-by: Saša Nedvědický Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/27397) --- include/internal/quic_port.h | 8 ++++---- ssl/quic/quic_channel.c | 6 ++++-- ssl/quic/quic_impl.c | 16 ++++++++-------- ssl/quic/quic_port.c | 12 ++++++++---- ssl/quic/quic_port_local.h | 2 +- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/include/internal/quic_port.h b/include/internal/quic_port.h index bf451a6108a..9c8f703468c 100644 --- a/include/internal/quic_port.h +++ b/include/internal/quic_port.h @@ -159,14 +159,14 @@ size_t ossl_quic_port_get_num_incoming_channels(const QUIC_PORT *port); /* Sets if incoming connections should currently be allowed. */ void ossl_quic_port_set_allow_incoming(QUIC_PORT *port, int allow_incoming); -#define PEELOFF_LISTEN -1 -#define PEELOFF_ACCEPT 1 -#define PEELOFF_UNSET 0 +# define PEELOFF_LISTEN -1 +# define PEELOFF_ACCEPT 1 +# define PEELOFF_UNSET 0 /* * Sets flag to indicate we are using SSL_listen_ex to get connections * returns 1 if set was successful, or 0 if the set fails */ -int ossl_quic_port_set_using_peeloff(QUIC_PORT *port, int using_peeloff); +int ossl_quic_port_test_and_set_peeloff(QUIC_PORT *port, int using_peeloff); /* Returns 1 if we are using addressed mode on the read side. */ int ossl_quic_port_is_addressed_r(const QUIC_PORT *port); diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c index ab5334d2f22..21f28ab3b45 100644 --- a/ssl/quic/quic_channel.c +++ b/ssl/quic/quic_channel.c @@ -567,11 +567,13 @@ void ossl_quic_channel_set0_tls(QUIC_CHANNEL *ch, SSL *ssl) ch->tls = ssl; #ifndef OPENSSL_NO_QLOG /* - * If we're using qlog, make sure the tls get further configured properly + * If we're using qlog, make sure the tls gets further configured properly */ ch->use_qlog = 1; - if (ch->tls->ctx->qlog_title != NULL) + if (ch->tls->ctx->qlog_title != NULL) { + OPENSSL_free(ch->qlog_title); ch->qlog_title = OPENSSL_strdup(ch->tls->ctx->qlog_title); + } #endif } diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c index df96011d6ea..32a9b414bd3 100644 --- a/ssl/quic/quic_impl.c +++ b/ssl/quic/quic_impl.c @@ -4652,15 +4652,20 @@ int ossl_quic_peeloff_conn(SSL *listener, SSL *new_conn) qctx_lock_for_io(&lctx); - if (!ossl_quic_port_set_using_peeloff(lctx.ql->port, PEELOFF_LISTEN)) { + if (!ossl_quic_port_test_and_set_peeloff(lctx.ql->port, PEELOFF_LISTEN)) { QUIC_RAISE_NON_NORMAL_ERROR(NULL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED, "This listener is using SSL_accept_connection"); ret = -1; goto out; } - + new_ch = ossl_quic_port_pop_incoming(lctx.ql->port); if (new_ch != NULL) { + tls = ossl_ssl_connection_new_int(ossl_quic_port_get_channel_ctx(lctx.ql->port), + new_conn, TLS_method()); + if (tls == NULL) + goto out; + qc = cctx.qc; ql = lctx.ql; ossl_quic_channel_free(qc->ch); @@ -4675,10 +4680,6 @@ int ossl_quic_peeloff_conn(SSL *listener, SSL *new_conn) qc->mutex = ql->mutex; #endif qc->ch = new_ch; - tls = ossl_ssl_connection_new_int(ossl_quic_port_get_channel_ctx(lctx.ql->port), - new_conn, TLS_method()); - if (tls == NULL) - goto out; SSL_free(qc->tls); ossl_quic_channel_set0_tls(new_ch, tls); qc->tls = tls; @@ -4734,11 +4735,10 @@ SSL *ossl_quic_accept_connection(SSL *ssl, uint64_t flags) if (!ql_listen(ctx.ql)) goto out; - if (!ossl_quic_port_set_using_peeloff(ctx.ql->port, PEELOFF_ACCEPT)) { + if (!ossl_quic_port_test_and_set_peeloff(ctx.ql->port, PEELOFF_ACCEPT)) { QUIC_RAISE_NON_NORMAL_ERROR(NULL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED, "This listener is using SSL_listen_ex"); goto out; - } /* Wait for an incoming connection if needed. */ diff --git a/ssl/quic/quic_port.c b/ssl/quic/quic_port.c index 57cee322571..e1464c9c285 100644 --- a/ssl/quic/quic_port.c +++ b/ssl/quic/quic_port.c @@ -536,11 +536,15 @@ static QUIC_CHANNEL *port_make_channel(QUIC_PORT *port, SSL *tls, OSSL_QRX *qrx, if (tls != NULL) { ch->tls = tls; } else { - if (ossl_quic_port_set_using_peeloff(port, PEELOFF_ACCEPT)) { + if (ossl_quic_port_test_and_set_peeloff(port, PEELOFF_ACCEPT)) { /* * We're using the normal SSL_accept_connection_path */ ch->tls = port_new_handshake_layer(port, ch); + if (ch->tls == NULL) { + ossl_quic_channel_free(ch); + return NULL; + } } else { /* * We're deferring user ssl creation until SSL_listen_ex is called @@ -653,7 +657,7 @@ void ossl_quic_port_set_allow_incoming(QUIC_PORT *port, int allow_incoming) port->allow_incoming = allow_incoming; } -int ossl_quic_port_set_using_peeloff(QUIC_PORT *port, int using_peeloff) +int ossl_quic_port_test_and_set_peeloff(QUIC_PORT *port, int using_peeloff) { /* @@ -668,9 +672,9 @@ int ossl_quic_port_set_using_peeloff(QUIC_PORT *port, int using_peeloff) * i.e. this is a trapdoor, once we set using_peeloff to LISTEN or ACCEPT * Then the only thing we can set that port too in the future is the same value. */ - if (port->using_peeloff != using_peeloff && port->using_peeloff != PEELOFF_UNSET) + if (port->peeloff_mode != using_peeloff && port->peeloff_mode != PEELOFF_UNSET) return 0; - port->using_peeloff = using_peeloff; + port->peeloff_mode = using_peeloff; return 1; } diff --git a/ssl/quic/quic_port_local.h b/ssl/quic/quic_port_local.h index 1fdb0f62a4b..d2c9d858f99 100644 --- a/ssl/quic/quic_port_local.h +++ b/ssl/quic/quic_port_local.h @@ -115,7 +115,7 @@ struct quic_port_st { unsigned int bio_changed : 1; /* Are we using SSL_listen_ex to peeloff connections */ - int using_peeloff; + int peeloff_mode; /* AES-256 GCM context for token encryption */ EVP_CIPHER_CTX *token_ctx; -- 2.47.3