MEDIUM: h3: do not close conn on unaligned non DATA frame

MEDIUM: h3: do not close conn on unaligned non DATA frame

Until now, H3 parser rejected a non aligned frame, except for DATA. The
whole connection was closed with error H3_EXCESSIVE_LOAD, which can be
seen as a medium defect given that the client did not produce anything
invalid or even suspect.

In fact, most frame parsers are already able to support wrapping
content, as they deal with a struct buffer representation. This is also
the case for QPACK uni streams. The real limitation is for HEADERS frame
which rely on qpack_decode_fs() which can only work on aligned content.

This patch removes the connection closure on unaligned content. For H3
HEADERS frames, the buffer content is realigned by performing two
memmove() calls into a temporarily allocated alternative area. The
current parsing iteration will continue in this new aligned storage.

A future optimisation would be to support wrapping content in
qpack_decode_fs() to remove realign operation.

This can be considered as a bug, as previous state prevents to handle
valid H3 content. As no user complains about it, it may be better to
refrain from backporting the patch, unless there is some explicit demand
about it.

Note that original Rx buffer is not realigned, as this would mangle the
underlying MUX ncbuf storage. An alternative approach was implemented
first, performing the realign at the MUX level, however it was probably
less efficient as it required to copy the whole ncbuf. For the record,
this patch is attached here with its original message.

---
Subject: [PATCH] MEDIUM: quic/h3: support parsing of unaligned non DATA H3
 frame

Until now, H3 parser was not able to parse a non DATA frame which wraps
in QCS Rx buffer. When this happened, the whole connection was closed
with error H3_EXCESSIVE_LOAD, which can be seen as a medium defect given
that the client did not produce anything invalid or even suspect.

This patch finally implements a method to deal with unaligned non DATA
H3 frames. When such frame is detected by H3 parser, rcv_buf callback is
interrupted. MUX will react to a null returned value and detect that the
buffer data is wrapping. In this case, the new function
qcs_realign_rx_data() is used to perform the realign operation using
b_slow_realign() helper. Callback rcv_buf is then reinvoked immediately.

This can be considered as a bug, as previous state prevents to handle
valid H3 content. As no user complains about it, it may be better to
refrain from backporting the patch, unless there is some explicit demand
about it.
---
 src/h3.c       |  8 ++------
 src/mux_quic.c | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/src/h3.c b/src/h3.c
index 4750ff69f..1ed4a4d8b 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -2068,13 +2068,9 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
 				goto err;
 			}

-			/* TODO extend parser to support the realignment of a frame. */
 			if (b_head(b) + b_data(b) > b_wrap(b)) {
-				TRACE_ERROR("cannot parse unaligned data frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-				qcc_set_error(qcs->qcc, H3_ERR_EXCESSIVE_LOAD, 1,
-				              muxc_tevt_type_other_err);
-				qcc_report_glitch(qcs->qcc, 1);
-				goto err;
+				TRACE_PROTO("pause parsing on unaligned payload", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+				break;
 			}

 			/* Only parse full HTTP/3 frames. */
diff --git a/src/mux_quic.c b/src/mux_quic.c
index de443a568..70a6cad2d 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1255,6 +1255,24 @@ static inline struct buffer qcs_b_dup(const struct qc_stream_rxbuf *b)
 	}
 }

+/* Realign buffer content in <rxbuf>. This should be called when parsing is
+ * blocked due to wrapping content.
+ */
+static void qcs_realign_rx_data(struct qc_stream_rxbuf *rxbuf)
+{
+	struct ncbuf *ncb = &rxbuf->ncb;
+	struct buffer old;
+
+	/* Convert ncbuf to a full buffer type. This forces b_slow_realign()
+	 * will copy every non contiguous blocks plus first ncb block header.
+	 */
+	old = b_make(ncb_orig(ncb), ncb->size, ncb->head, ncb->size);
+	b_slow_realign(&old, trash.area, 0);
+
+	/* ncb area now realigned - ensure head is resetted as well. */
+	ncb->head = 0;
+}
+
 /* Transfer data into <qcs> stream <rxbuf> current Rx buffer from its directly
  * following buffer. This is useful when parsing was interrupted due to partial
  * data. If following buffer does not exists, nothing is done.
@@ -1508,9 +1526,20 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs)
 		 * at a Rx buffer boundary. Try to realign data if possible and
 		 * restart decoding.
 		 */
-		if (!ret && rxbuf && !(qcs->flags & QC_SF_DEM_FULL) &&
-		    qcs->rx.offset + ncb_data(&rxbuf->ncb, 0) == rxbuf->off_end) {
-			if (!qcs_transfer_rx_data(qcs, rxbuf)) {
+		if (!ret && rxbuf && !(qcs->flags & QC_SF_DEM_FULL)) {
+			int need_restart = 0;
+
+			if (ncb_head(&rxbuf->ncb) + ncb_data(&rxbuf->ncb, 0) > ncb_wrap(&rxbuf->ncb)) {
+				qcs_realign_rx_data(rxbuf);
+				need_restart = 1;
+			}
+
+			if (qcs->rx.offset + ncb_data(&rxbuf->ncb, 0) == rxbuf->off_end) {
+				if (!qcs_transfer_rx_data(qcs, rxbuf))
+					need_restart = 1;
+			}
+
+			if (need_restart) {
 				TRACE_DEVEL("restart parsing after data realignment", QMUX_EV_QCS_RECV, qcc->conn, qcs);
 				goto restart;
 			}
--
2.56.0