Various fixups for SSL_listen_ex (readability/error unwinding)

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ý <sashan@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27397)

diff --git a/include/internal/quic_port.h b/include/internal/quic_port.h
index bf451a6108a9055680df19a4ec3129f09b121ff7..9c8f703468cf3ab6a7bdc89682849575beea8135 100644 (file)
--- 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 ab5334d2f2227c72e08f5e40a048ede53af5e267..21f28ab3b4502c517f9b58d6499344e4fbae708c 100644 (file)
--- 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 df96011d6ea8106a03b782c888a149337f4bc136..32a9b414bd3d5a242741c3019c27bbf049dfb9b4 100644 (file)
--- 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 57cee3225710f5efeda9430b48e7c1a057027bf8..e1464c9c285268b88259d46d88da228d41935fa3 100644 (file)
--- 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 1fdb0f62a4b6283025af495625b77126db1578d1..d2c9d858f99fb67ada0aa2517edbe92360bb8447 100644 (file)
--- 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;