Fixing HCM fuzzer ContinueAndEndStream with end_stream set (envoyprox…

…y#11497)

Fixing the HCM fuzzer to avoid cases where its decodeHeaders method returns FilterHeadersStatus::ContinueAndEndStream when called with end_stream=true.
This assert was added in envoyproxy#4885, to avoid misuse of the FilterHeadersStatus::ContinueAndEndStream value.

Risk Level: Low - tests only
Testing: Changed HCM fuzzer test code
Fixes oss fuzz issue 21971

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>

docs/root/faq/extensions/contract.rst

@@ -0,0 +1,48 @@
+.. _faq_filter_contract:
+
+Is there a contract my HTTP filter must adhere to?
+--------------------------------------------------
+
+* Headers encoding/decoding
+
+  * During encoding/decoding of headers if a filter returns ``FilterHeadersStatus::StopIteration``,
+    the processing can be resumed if ``encodeData()``/``decodeData()`` return
+    ``FilterDataStatus::Continue`` or by explicitly calling
+    ``continueEncoding()``/``continueDecoding()``.
+
+  * During encoding/decoding of headers if a filter returns
+    ``FilterHeadersStatus::StopAllIterationAndBuffer`` or
+    ``FilterHeadersStatus::StopAllIterationAndWatermark``, the processing can be resumed by calling
+    ``continueEncoding()``/``continueDecoding()``.
+
+  * A filter's ``decodeHeaders()`` implementation must not return
+    ``FilterHeadersStatus::ContinueAndEndStream`` when called with ``end_stream`` set to *true*. In this case
+    ``FilterHeadersStatus::Continue`` should be returned.
+
+  * A filter's ``encode100ContinueHeaders()`` must return ``FilterHeadersStatus::Continue`` or
+    ``FilterHeadersStatus::StopIteration``.
+
+* Data encoding/decoding
+
+  * During encoding/decoding of data if a filter returns
+    ``FilterDataStatus::StopIterationAndBuffer``, ``FilterDataStatus::StopIterationAndWatermark``,
+    or ``FilterDataStatus::StopIterationNoBuffer``, the processing can be resumed if
+    ``encodeData()``/``decodeData()`` return ``FilterDataStatus::Continue`` or by explicitly
+    calling ``continueEncoding()``/``continueDecoding()``.
+
+* Trailers encoding/decoding
+
+  * During encoding/decoding of trailers if a filter returns ``FilterTrailersStatus::StopIteration``,
+    the processing can be resumed by explicitly calling ``continueEncoding()``/``continueDecoding()``.
+
+Are there well-known headers that will appear in the given headers map of ``decodeHeaders()``?
+----------------------------------------------------------------------------------------------
+
+The first filter of the decoding filter chain will have the following headers in the map:
+
+* ``Host``
+* ``Path`` (this might be omitted for CONNECT requests).
+
+Although these headers may be omitted by one of the filters on the decoding filter chain,
+they should be reinserted before the terminal filter is triggered.
+

docs/root/faq/overview.rst

@@ -72,3 +72,11 @@ Load balancing
   load_balancing/disable_circuit_breaking
   load_balancing/transient_failures
   load_balancing/region_failover
+
+Extensions
+----------
+
+.. toctree::
+  :maxdepth: 2
+
+  extensions/contract

include/envoy/http/filter.h

@@ -34,6 +34,7 @@ enum class FilterHeadersStatus {
   StopIteration,
   // Continue iteration to remaining filters, but ignore any subsequent data or trailers. This
   // results in creating a header only request/response.
+  // This status MUST NOT be returned by decodeHeaders() when end_stream is set to true.
   ContinueAndEndStream,
   // Do not iterate for headers as well as data and trailers for the current filter and the filters
   // following, and buffer body data for later dispatching. ContinueDecoding() MUST

test/common/http/conn_manager_impl_fuzz_test.cc

@@ -284,11 +284,16 @@ class FuzzStream {
           return Http::okStatus();
         }));
     ON_CALL(*decoder_filter_, decodeHeaders(_, _))
-        .WillByDefault(
-            InvokeWithoutArgs([this, decode_header_status]() -> Http::FilterHeadersStatus {
-              header_status_ = fromHeaderStatus(decode_header_status);
-              return *header_status_;
-            }));
+        .WillByDefault(InvokeWithoutArgs([this, decode_header_status,
+                                          end_stream]() -> Http::FilterHeadersStatus {
+          header_status_ = fromHeaderStatus(decode_header_status);
+          // When a filter should not return ContinueAndEndStream when send with end_stream set
+          // (see https://github.com/envoyproxy/envoy/pull/4885#discussion_r232176826)
+          if (end_stream && (*header_status_ == Http::FilterHeadersStatus::ContinueAndEndStream)) {
+            *header_status_ = Http::FilterHeadersStatus::Continue;
+          }
+          return *header_status_;
+        }));
     fakeOnData();
     FUZZ_ASSERT(testing::Mock::VerifyAndClearExpectations(config_.codec_));
   }