Minor updates

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #21135)
openssl · Jul 16, 2023 · 96b7df6 · 96b7df6
1 parent 70cafc4
commit 96b7df6
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 26 deletions.
diff --git a/include/internal/quic_record_rx.h b/include/internal/quic_record_rx.h
@@ -319,9 +319,6 @@ uint64_t ossl_qrx_get_bytes_received(OSSL_QRX *qrx, int clear);
  *
  * Other packets in the same datagram will still be processed where possible.
  *
- * The intended use for this function is to allow validation of whether a PN is
- * a potential duplicate before spending CPU time decrypting the packet payload.
- *
  * The callback is optional and can be unset by passing NULL for cb.
  * cb_arg is an opaque value passed to cb.
  */

diff --git a/include/internal/quic_stream_map.h b/include/internal/quic_stream_map.h
@@ -604,7 +604,6 @@ void ossl_quic_stream_map_update_state(QUIC_STREAM_MAP *qsm, QUIC_STREAM *s);
  */
 void ossl_quic_stream_map_set_rr_stepping(QUIC_STREAM_MAP *qsm, size_t stepping);
 
-
 /*
  * Stream Send Part
  * ================
@@ -620,10 +619,10 @@ void ossl_quic_stream_map_set_rr_stepping(QUIC_STREAM_MAP *qsm, size_t stepping)
  * STREAM_DATA_BLOCKED) frame transmission for locally-initiated streams.
  *
  * Our implementation does not currently do this and we allocate stream IDs up
- * front, however we may revisit this in the future. Calling this ensures
- * represents a demand for a stream ID by the caller and ensures one has been
- * allocated to the stream, and causes us to transition to SEND if we are still
- * in the READY state.
+ * front, however we may revisit this in the future. Calling this represents a
+ * demand for a stream ID by the caller and ensures one has been allocated to
+ * the stream, and causes us to transition to SEND if we are still in the READY
+ * state.
  *
  * Returns 0 if there is no send part (caller error) and 1 otherwise.
  */
@@ -759,7 +758,7 @@ int ossl_quic_stream_map_stop_sending_recv_part(QUIC_STREAM_MAP *qsm,
 
 /*
  * Marks the stream as wanting a STOP_SENDING frame transmitted. It is not valid
- * to vall this if ossl_quic_stream_map_stop_sending_recv_part() has not been
+ * to call this if ossl_quic_stream_map_stop_sending_recv_part() has not been
  * called. For TXP use.
  */
 int ossl_quic_stream_map_schedule_stop_sending(QUIC_STREAM_MAP *qsm,

diff --git a/include/internal/quic_types.h b/include/internal/quic_types.h
@@ -102,8 +102,8 @@ static ossl_unused ossl_inline int ossl_quic_conn_id_eq(const QUIC_CONN_ID *a,
 #  define QUIC_STATELESS_RESET_TOKEN_LEN    16
 
 /*
- * An encoded preferred_addr transport parameter cannot be longer than this
- * number of bytes.
+ * An encoded preferred_addr transport parameter cannot be shorter or longer
+ * than these lengths in bytes.
  */
 #  define QUIC_MIN_ENCODED_PREFERRED_ADDR_LEN   41
 #  define QUIC_MAX_ENCODED_PREFERRED_ADDR_LEN   61

diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
@@ -1854,12 +1854,13 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
      *
      * We need to be a bit careful here as due to the BIO abstraction layer an
      * application is liable to be weird and lie to us about peer addresses.
-     * Only apply this check if we actually are using a real address and haven't
-     * been given AF_UNSPEC by the application.
+     * Only apply this check if we actually are using a real AF_INET or AF_INET6
+     * address.
      */
     if (!ch->is_server
         && ch->qrx_pkt->peer != NULL
-        && BIO_ADDR_family(&ch->cur_peer_addr) != AF_UNSPEC
+        && (BIO_ADDR_family(&ch->cur_peer_addr) == AF_INET
+            || BIO_ADDR_family(&ch->cur_peer_addr) == AF_INET6)
         && !bio_addr_eq(ch->qrx_pkt->peer, &ch->cur_peer_addr))
         return;
 
@@ -2984,7 +2985,6 @@ QUIC_STREAM *ossl_quic_channel_new_stream_local(QUIC_CHANNEL *ch, int is_uni)
     if ((qs = ossl_quic_stream_map_alloc(&ch->qsm, stream_id, type)) == NULL)
         return NULL;
 
-
     /* Locally-initiated stream, so we always want a send buffer. */
     if (!ch_init_new_stream(ch, qs, /*can_send=*/1, /*can_recv=*/!is_uni))
         goto err;

diff --git a/ssl/quic/quic_stream_map.c b/ssl/quic/quic_stream_map.c
@@ -495,7 +495,8 @@ int ossl_quic_stream_map_notify_reset_stream_acked(QUIC_STREAM_MAP *qsm,
     }
 }
 
-/* Stream Receive Part State Management
+/*
+ * Stream Receive Part State Management
  * ====================================
  */
 
@@ -511,7 +512,7 @@ int ossl_quic_stream_map_notify_size_known_recv_part(QUIC_STREAM_MAP *qsm,
         return 0;
 
     case QUIC_RSTREAM_STATE_RECV:
-        qs->recv_state      = QUIC_RSTREAM_STATE_SIZE_KNOWN;
+        qs->recv_state = QUIC_RSTREAM_STATE_SIZE_KNOWN;
         return 1;
     }
 }

diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
@@ -1888,7 +1888,8 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
         if (stream->want_reset_stream) {
             OSSL_QUIC_FRAME_RESET_STREAM f;
 
-            assert(stream->send_state == QUIC_SSTREAM_STATE_RESET_SENT);
+            if (!ossl_assert(stream->send_state == QUIC_SSTREAM_STATE_RESET_SENT))
+                return 0;
 
             wpkt = tx_helper_begin(h);
             if (wpkt == NULL)
@@ -1918,7 +1919,8 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
              * parties; if we happen to send a RESET_STREAM that consumes more
              * flow control credit, make sure we account for that.
              */
-            assert(f.final_size <= ossl_quic_txfc_get_swm(&stream->txfc));
+            if (!ossl_assert(f.final_size <= ossl_quic_txfc_get_swm(&stream->txfc)))
+                return 0;
 
             stream->txp_txfc_new_credit_consumed
                 = f.final_size - ossl_quic_txfc_get_swm(&stream->txfc);
@@ -1971,7 +1973,8 @@ static int txp_generate_stream_related(OSSL_QUIC_TX_PACKETISER *txp,
             && !ossl_quic_stream_send_is_reset(stream)) {
             int packet_full = 0, stream_drained = 0;
 
-            assert(!stream->want_reset_stream);
+            if (!ossl_assert(!stream->want_reset_stream))
+                return 0;
 
             if (!txp_generate_stream_frames(txp, h, pn_space, tpkt,
                                             stream->id, stream->sstream,

diff --git a/test/quic_multistream_test.c b/test/quic_multistream_test.c
@@ -2217,7 +2217,10 @@ static int script_21_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }
 
@@ -2296,7 +2299,10 @@ static int script_23_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }
 
@@ -2346,7 +2352,10 @@ static int script_24_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }
 
@@ -2457,7 +2466,10 @@ static int script_28_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }
 
@@ -2608,7 +2620,10 @@ static int script_32_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }
 
@@ -2788,7 +2803,10 @@ static int script_39_inject_plain(struct helper *h, QUIC_PKT_HDR *hdr,
 
     ok = 1;
 err:
-    WPACKET_finish(&wpkt);
+    if (ok)
+        WPACKET_finish(&wpkt);
+    else
+        WPACKET_cleanup(&wpkt);
     return ok;
 }