Resolve some of the TODO(QUIC) items

For some of the items we add FUTURE/SERVER/TESTING/MULTIPATH
designation to indicate these do not need to be resolved
in QUIC MVP release.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21539)

12 files changed, 37 insertions, 39 deletions

diff --git a/ssl/quic/cc_newreno.c b/ssl/quic/cc_newreno.c
index de4c10c6a2..1fe37c276e 100644
--- a/ssl/quic/cc_newreno.c
+++ b/ssl/quic/cc_newreno.c
@@ -36,7 +36,7 @@ typedef struct ossl_cc_newreno_st {
 
 #define MIN_MAX_INIT_WND_SIZE    14720  /* RFC 9002 s. 7.2 */
 
-/* TODO(QUIC): Pacing support. */
+/* TODO(QUIC FUTURE): Pacing support. */
 
 static void newreno_set_max_dgram_size(OSSL_CC_NEWRENO *nr,
                                        size_t max_dgram_size);
diff --git a/ssl/quic/quic_ackm.c b/ssl/quic/quic_ackm.c
index 9e22bf2fc0..7c567eae77 100644
--- a/ssl/quic/quic_ackm.c
+++ b/ssl/quic/quic_ackm.c
@@ -927,7 +927,7 @@ static int ackm_set_loss_detection_timer(OSSL_ACKM *ackm)
 static int ackm_in_persistent_congestion(OSSL_ACKM *ackm,
                                          const OSSL_ACKM_TX_PKT *lpkt)
 {
-    /* TODO(QUIC): Persistent congestion not currently implemented. */
+    /* TODO(QUIC FUTURE): Persistent congestion not currently implemented. */
     return 0;
 }
 
@@ -1284,7 +1284,8 @@ static void ackm_queue_probe_anti_deadlock_initial(OSSL_ACKM *ackm)
 static void ackm_queue_probe(OSSL_ACKM *ackm, int pkt_space)
 {
     /*
-     * TODO(QUIC): We are allowed to send either one or two probe packets here.
+     * TODO(QUIC FUTURE): We are allowed to send either one or two probe
+     * packets here.
      * Determine a strategy for when we should send two probe packets.
      */
     ++ackm->pending_probe.pto[pkt_space];
diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 54ccf5d316..af3a1d051a 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -21,8 +21,8 @@
  * not suitable for network use. In particular, it does not implement address
  * validation, anti-amplification or retry logic.
  *
- * TODO(QUIC): Implement address validation and anti-amplification
- * TODO(QUIC): Implement retry logic
+ * TODO(QUIC SERVER): Implement address validation and anti-amplification
+ * TODO(QUIC SERVER): Implement retry logic
  */
 
 #define INIT_DCID_LEN           8
@@ -1380,7 +1380,7 @@ static int ch_on_transport_params(const unsigned char *params,
 
         case QUIC_TPARAM_PREFERRED_ADDR:
             {
-                /* TODO(QUIC): Handle preferred address. */
+                /* TODO(QUIC FUTURE): Handle preferred address. */
                 QUIC_PREFERRED_ADDR pfa;
 
                 /*
@@ -1684,7 +1684,7 @@ static void ch_tick(QUIC_TICK_RESULT *res, void *arg, uint32_t flags)
              * again because packets that were not previously processable and
              * were deferred might now be processable.
              *
-             * TODO(QUIC): Consider handling this in the yield_secret callback.
+             * TODO(QUIC FUTURE): Consider handling this in the yield_secret callback.
              */
         } while (ch->have_new_rx_secret);
     }
@@ -1985,7 +1985,7 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
             return;
 
         /*
-         * TODO(QUIC): Theoretically this should probably be in the QRX.
+         * TODO(QUIC FUTURE): Theoretically this should probably be in the QRX.
          * However because validation is dependent on context (namely the
          * client's initial DCID) we can't do this cleanly. In the future we
          * should probably add a callback to the QRX to let it call us (via
@@ -2011,8 +2011,8 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
             return;
 
         /*
-         * TODO(QUIC): Implement 0-RTT on the server side. We currently do
-         * not need to implement this as a client can only do 0-RTT if we
+         * TODO(QUIC 0RTT): Implement 0-RTT on the server side. We currently
+         * do not need to implement this as a client can only do 0-RTT if we
          * have given it permission to in a previous session.
          */
         break;
@@ -2124,7 +2124,7 @@ static void ch_default_packet_handler(QUIC_URXE *e, void *arg)
         case QUIC_VERSION_NONE:
         default:
             /* Unknown version or proactive version negotiation request, bail. */
-            /* TODO(QUIC): Handle version negotiation on server side */
+            /* TODO(QUIC SERVER): Handle version negotiation on server side */
             goto undesirable;
     }
 
diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 89475f3889..ca00fcd476 100644
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -375,7 +375,7 @@ SSL *ossl_quic_new(SSL_CTX *ctx)
         = (ssl_base->method == OSSL_QUIC_client_thread_method());
 #endif
 
-    qc->as_server       = 0; /* TODO(QUIC): server support */
+    qc->as_server       = 0; /* TODO(QUIC SERVER): add server support */
     qc->as_server_state = qc->as_server;
 
     qc->default_stream_mode     = SSL_DEFAULT_STREAM_MODE_AUTO_BIDI;
@@ -547,7 +547,7 @@ int ossl_quic_clear(SSL *s)
     if (!expect_quic(s, &ctx))
         return 0;
 
-    /* TODO(QUIC): Currently a no-op. */
+    /* TODO(QUIC FUTURE): Currently a no-op. */
     return 1;
 }
 
@@ -1172,7 +1172,6 @@ int ossl_quic_conn_shutdown(SSL *s, uint64_t flags,
         return -1;
 
     if (ctx.is_stream)
-        /* TODO(QUIC): Semantics currently undefined for QSSOs */
         return -1;
 
     quic_lock(ctx.qc);
@@ -1412,7 +1411,6 @@ static int quic_do_handshake(QCTX *ctx)
     }
 
     if (qc->as_server != qc->as_server_state) {
-        /* TODO(QUIC): Must match the method used to create the QCSO */
         QUIC_RAISE_NON_NORMAL_ERROR(ctx, ERR_R_PASSED_INVALID_ARGUMENT, NULL);
         return -1; /* Non-protocol error */
     }
@@ -1800,8 +1798,8 @@ static void quic_post_write(QUIC_XSO *xso, int did_append, int do_tick)
     /*
      * Try and send.
      *
-     * TODO(QUIC): It is probably inefficient to try and do this immediately,
-     * plus we should eventually consider Nagle's algorithm.
+     * TODO(QUIC FUTURE): It is probably inefficient to try and do this
+     * immediately, plus we should eventually consider Nagle's algorithm.
      */
     if (do_tick)
         ossl_quic_reactor_tick(ossl_quic_channel_get_reactor(xso->conn->ch), 0);
diff --git a/ssl/quic/quic_local.h b/ssl/quic/quic_local.h
index eb6d3e4e6f..d00a63d556 100644
--- a/ssl/quic/quic_local.h
+++ b/ssl/quic/quic_local.h
@@ -293,7 +293,7 @@ const SSL_METHOD *func_name(void)  \
                 ossl_quic_free, \
                 ossl_quic_reset, \
                 ossl_quic_init, \
-                ossl_quic_clear, \
+                NULL /* clear */, \
                 ossl_quic_deinit, \
                 q_accept, \
                 q_connect, \
diff --git a/ssl/quic/quic_reactor.c b/ssl/quic/quic_reactor.c
index ef447b78c3..f89337b38e 100644
--- a/ssl/quic/quic_reactor.c
+++ b/ssl/quic/quic_reactor.c
@@ -342,14 +342,14 @@ int ossl_quic_reactor_block_until_pred(QUIC_REACTOR *rtor,
              * things again. If poll_two_fds returns 0, this is some other
              * non-timeout failure and we should stop here.
              *
-             * TODO(QUIC): In the future we could avoid unnecessary syscalls by
-             * not retrying network I/O that isn't ready based on the result of
-             * the poll call. However this might be difficult because it
-             * requires we do the call to poll(2) or equivalent syscall
-             * ourselves, whereas in the general case the application does the
-             * polling and just calls SSL_handle_events(). Implementing this
-             * optimisation in the future will probably therefore require API
-             * changes.
+             * TODO(QUIC FUTURE): In the future we could avoid unnecessary
+             * syscalls by not retrying network I/O that isn't ready based
+             * on the result of the poll call. However this might be difficult
+             * because it requires we do the call to poll(2) or equivalent
+             * syscall ourselves, whereas in the general case the application
+             * does the polling and just calls SSL_handle_events().
+             * Implementing this optimisation in the future will probably
+             * therefore require API changes.
              */
             return 0;
     }
diff --git a/ssl/quic/quic_rstream.c b/ssl/quic/quic_rstream.c
index 0b3c870661..c51bc2014c 100644
--- a/ssl/quic/quic_rstream.c
+++ b/ssl/quic/quic_rstream.c
@@ -280,7 +280,6 @@ int ossl_quic_rstream_move_to_rbuf(QUIC_RSTREAM *qrs)
 
 int ossl_quic_rstream_resize_rbuf(QUIC_RSTREAM *qrs, size_t rbuf_size)
 {
-    /* TODO(QUIC): Do we need to distinguish different error conditions ? */
     if (ossl_sframe_list_is_head_locked(&qrs->fl))
         return 0;
 
diff --git a/ssl/quic/quic_rx_depack.c b/ssl/quic/quic_rx_depack.c
index 9e10a0a032..a20aac61bf 100644
--- a/ssl/quic/quic_rx_depack.c
+++ b/ssl/quic/quic_rx_depack.c
@@ -332,7 +332,7 @@ static int depack_do_frame_new_token(PACKET *pkt, QUIC_CHANNEL *ch,
         return 0;
     }
 
-    /* TODO(QUIC): ADD CODE to send |token| to the session manager */
+    /* TODO(QUIC FUTURE): ADD CODE to send |token| to the session manager */
 
     return 1;
 }
@@ -881,7 +881,7 @@ static int depack_do_frame_retire_conn_id(PACKET *pkt,
      * currently non-conformant and for internal testing use; simply handle it
      * as a no-op in this case.
      *
-     * TODO(QUIC): Revise and implement correctly for server support.
+     * TODO(QUIC SERVER): Revise and implement correctly for server support.
      */
     if (!ch->is_server) {
         ossl_quic_channel_raise_protocol_error(ch,
@@ -968,7 +968,7 @@ static int depack_do_frame_path_response(PACKET *pkt,
         return 0;
     }
 
-    /* TODO(QUIC): ADD CODE to send |frame_data| to the ch manager */
+    /* TODO(QUIC MULTIPATH): ADD CODE to send |frame_data| to the ch manager */
 
     return 1;
 }
@@ -1378,8 +1378,8 @@ int ossl_quic_handle_frames(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpacket)
     /* Initialize |ackm_data| (and reinitialize |ok|)*/
     memset(&ackm_data, 0, sizeof(ackm_data));
     /*
-     * TODO(QUIC): ASSUMPTION: All packets that aren't special case have a
-     * packet number
+     * ASSUMPTION: All packets that aren't special case have a
+     * packet number.
      */
     ackm_data.pkt_num = qpacket->pn;
     ackm_data.time = qpacket->time;
@@ -1413,7 +1413,7 @@ int ossl_quic_handle_frames(QUIC_CHANNEL *ch, OSSL_QRX_PKT *qpacket)
     ok = 1;
  end:
     /*
-     * TODO(QUIC): ASSUMPTION: If this function is called at all, |qpacket| is
+     * ASSUMPTION: If this function is called at all, |qpacket| is
      * a legitimate packet, even if its contents aren't.
      * Therefore, we call ossl_ackm_on_rx_packet() unconditionally, as long as
      * |ackm_data| has at least been initialized.
diff --git a/ssl/quic/quic_stream_map.c b/ssl/quic/quic_stream_map.c
index b23429a378..5d4354a2df 100644
--- a/ssl/quic/quic_stream_map.c
+++ b/ssl/quic/quic_stream_map.c
@@ -290,7 +290,7 @@ static ossl_unused int qsm_send_part_permits_gc(const QUIC_STREAM *qs)
 
 static int qsm_ready_for_gc(QUIC_STREAM_MAP *qsm, QUIC_STREAM *qs)
 {
-    int recv_stream_fully_drained = 0; /* TODO(QUIC): Optimisation */
+    int recv_stream_fully_drained = 0; /* TODO(QUIC FUTURE): Optimisation */
 
     /*
      * If sstream has no FIN, we auto-reset it at marked-for-deletion time, so
diff --git a/ssl/quic/quic_trace.c b/ssl/quic/quic_trace.c
index 5a57e675f9..d881140294 100644
--- a/ssl/quic/quic_trace.c
+++ b/ssl/quic/quic_trace.c
@@ -578,8 +578,8 @@ int ossl_quic_trace(int write_p, int version, int content_type,
                 return 0;
             /* Decode the packet header */
             /*
-             * TODO(QUIC): We need to query the short connection id len here,
-             *             e.g. via some API SSL_get_short_conn_id_len()
+             * TODO(QUIC SERVER): We need to query the short connection id len
+             * here, e.g. via some API SSL_get_short_conn_id_len()
              */
             if (ossl_quic_wire_decode_pkt_hdr(&pkt, 0, 0, 1, &hdr, NULL) != 1)
                 return 0;
diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index 4768cd856b..1107a0778d 100644
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -1276,7 +1276,7 @@ static int txp_should_try_staging(OSSL_QUIC_TX_PACKETISER *txp,
      * This is not a major concern for clients, since if a client has a 1-RTT EL
      * provisioned the server is guaranteed to also have a 1-RTT EL provisioned.
      *
-     * TODO(QUIC): Revisit this when server support is added.
+     * TODO(QUIC SERVER): Revisit this when server support is added.
      */
     if (*conn_close_enc_level > enc_level
         && *conn_close_enc_level != QUIC_ENC_LEVEL_1RTT)
@@ -1402,7 +1402,7 @@ static int sstream_is_pending(QUIC_SSTREAM *sstream)
 /* Determine how many bytes we should use for the encoded PN. */
 static size_t txp_determine_pn_len(OSSL_QUIC_TX_PACKETISER *txp)
 {
-    return 4; /* TODO(QUIC) */
+    return 4; /* TODO(QUIC FUTURE) */
 }
 
 /* Determine plaintext packet payload length from payload length. */
diff --git a/ssl/quic/quic_wire_pkt.c b/ssl/quic/quic_wire_pkt.c
index a96843aa9f..5185d7821b 100644
--- a/ssl/quic/quic_wire_pkt.c
+++ b/ssl/quic/quic_wire_pkt.c
@@ -854,7 +854,7 @@ int ossl_quic_calculate_retry_integrity_tag(OSSL_LIB_CTX *libctx,
         return 0;
 
     /* Create and initialise cipher context. */
-    /* TODO(QUIC): Cipher fetch caching. */
+    /* TODO(QUIC FUTURE): Cipher fetch caching. */
     if ((cipher = EVP_CIPHER_fetch(libctx, "AES-128-GCM", propq)) == NULL)
         goto err;