From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 30 Jun 2022 07:30:23 +0000 (+0200)
Subject: MINOR: qpack: properly handle invalid dynamic table references
X-Git-Tag: v2.7-dev2~173
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a7a4c80adefe8c8934093a874f5557d61f0c995d;p=thirdparty%2Fhaproxy.git

MINOR: qpack: properly handle invalid dynamic table references

Return QPACK_DECOMPRESSION_FAILED error code when dealing with dynamic
table references. This is justified as for now haproxy does not
implement dynamic table support and advertizes a zero-sized table.

The H3 calling function will thus reuse this code in a CONNECTION_CLOSE
frame, in conformance with the QPACK RFC.

This proper error management allows to remove obsolete ABORT_NOW guards.
---

diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index 8fa19b1724..aa30121e47 100644
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -220,12 +220,7 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
 
 		if (efl_type == QPACK_LFL_WPBNM) {
 			/* Literal field line with post-base name reference
-			 *
-			 * TODO not implemented
-			 *
-			 * For the moment, this should never happen as
-			 * currently we do not support dynamic table insertion
-			 * and specify an empty table size.
+			 * TODO adjust this when dynamic table support is implemented.
 			 */
 #if 0
 			uint64_t index __maybe_unused, length;
@@ -260,16 +255,20 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
 			raw += length;
 			len -= length;
 #endif
-			ABORT_NOW(); /* dynamic table not supported */
+
+			/* RFC9204 2.2.3 Invalid References
+			 *
+			 * If the decoder encounters a reference in a field line representation
+			 * to a dynamic table entry that has already been evicted or that has an
+			 * absolute index greater than or equal to the declared Required Insert
+			 * Count (Section 4.5.1), it MUST treat this as a connection error of
+			 * type QPACK_DECOMPRESSION_FAILED.
+			 */
+			return -QPACK_DECOMPRESSION_FAILED;
 		}
 		else if (efl_type == QPACK_IFL_WPBI) {
 			/* Indexed field line with post-base index
-			 *
-			 * TODO not implemented
-			 *
-			 * For the moment, this should never happen as
-			 * currently we do not support dynamic table insertion
-			 * and specify an empty table size.
+			 * TODO adjust this when dynamic table support is implemented.
 			 */
 #if 0
 			uint64_t index __maybe_unused;
@@ -284,7 +283,16 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
 
 			qpack_debug_printf(stderr, " index=%llu", (unsigned long long)index);
 #endif
-			ABORT_NOW(); /* dynamic table not supported */
+
+			/* RFC9204 2.2.3 Invalid References
+			 *
+			 * If the decoder encounters a reference in a field line representation
+			 * to a dynamic table entry that has already been evicted or that has an
+			 * absolute index greater than or equal to the declared Required Insert
+			 * Count (Section 4.5.1), it MUST treat this as a connection error of
+			 * type QPACK_DECOMPRESSION_FAILED.
+			 */
+			return -QPACK_DECOMPRESSION_FAILED;
 		}
 		else if (efl_type & QPACK_IFL_BIT) {
 			/* Indexed field line */
@@ -305,13 +313,17 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
 				value = qpack_sht[index].v;
 			}
 			else {
-				/* TODO not implemented
+				/* RFC9204 2.2.3 Invalid References
+				 *
+				 * If the decoder encounters a reference in a field line representation
+				 * to a dynamic table entry that has already been evicted or that has an
+				 * absolute index greater than or equal to the declared Required Insert
+				 * Count (Section 4.5.1), it MUST treat this as a connection error of
+				 * type QPACK_DECOMPRESSION_FAILED.
 				 *
-				 * For the moment, this should never happen as
-				 * currently we do not support dynamic table insertion
-				 * and specify an empty table size.
+				 * TODO adjust this when dynamic table support is implemented.
 				 */
-				ABORT_NOW();
+				return -QPACK_DECOMPRESSION_FAILED;
 			}
 
 			qpack_debug_printf(stderr,  " t=%d index=%llu", !!static_tbl, (unsigned long long)index);
@@ -335,13 +347,17 @@ int qpack_decode_fs(const unsigned char *raw, uint64_t len, struct buffer *tmp,
 				name = qpack_sht[index].n;
 			}
 			else {
-				/* TODO not implemented
+				/* RFC9204 2.2.3 Invalid References
+				 *
+				 * If the decoder encounters a reference in a field line representation
+				 * to a dynamic table entry that has already been evicted or that has an
+				 * absolute index greater than or equal to the declared Required Insert
+				 * Count (Section 4.5.1), it MUST treat this as a connection error of
+				 * type QPACK_DECOMPRESSION_FAILED.
 				 *
-				 * For the moment, this should never happen as
-				 * currently we do not support dynamic table insertion
-				 * and specify an empty table size.
+				 * TODO adjust this when dynamic table support is implemented.
 				 */
-				ABORT_NOW();
+				return -QPACK_DECOMPRESSION_FAILED;
 			}
 
 			qpack_debug_printf(stderr, " n=%d t=%d index=%llu", !!n, !!static_tbl, (unsigned long long)index);