http: stream error on invalid messaging (envoyproxy#11748)

This unifies HTTP/1.1 and HTTP/2 stream error on invalid messaging. Previously HTTP/1.1 defaulted permissive and HTTP/2 defaulted to strict. This defaults both to strict, resetting connections on invalid requests. This will have a major latency impact if downstream is sending a mix of valid and invalid requests over HTTP/1.1

Additional Description:
This change is runtime guarded per default behavioral change rules. It can also be reverted by setting the default to permissive (for prior HTTP/1 behvior) then overriding HTTP/2 to struct (for prior HTTP/2 behavior).

This works in conjunction with envoyproxy#11714, as the HTTP connection manager enforces the strictness, so the responses need to be sent via the HTTP connection manager to have strictness applied correctly.

Risk Level: High (HCM changes)
Testing: new unit tests, updated integration tests
Docs Changes: n/a
Release Notes: inline
Runtime guard: envoy.reloadable_features.hcm_stream_error_on_invalid_message
Fixes envoyproxy#9846

Signed-off-by: Alyssa Wilk <[email protected]>
Signed-off-by: scheler <[email protected]>

api/envoy/config/core/v3/protocol.proto

@@ -159,7 +159,7 @@ message Http1ProtocolOptions {
   bool enable_trailers = 5;
 }
 
-// [#next-free-field: 14]
+// [#next-free-field: 15]
 message Http2ProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.Http2ProtocolOptions";
@@ -280,8 +280,25 @@ message Http2ProtocolOptions {
   // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However,
   // when this option is enabled, only the offending stream is terminated.
   //
+  // This is overridden by HCM :ref:`stream_error_on_invalid_http_messaging
+  // <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>`
+  // iff present.
+  //
+  // This is deprecated in favor of :ref:`override_stream_error_on_invalid_http_message
+  // <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>`
+  //
+  // See `RFC7540, sec. 8.1 <https://tools.ietf.org/html/rfc7540#section-8.1>`_ for details.
+  bool stream_error_on_invalid_http_messaging = 12 [deprecated = true];
+
+  // Allows invalid HTTP messaging and headers. When this option is disabled (default), then
+  // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However,
+  // when this option is enabled, only the offending stream is terminated.
+  //
+  // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging
+  // <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>`
+  //
   // See `RFC7540, sec. 8.1 <https://tools.ietf.org/html/rfc7540#section-8.1>`_ for details.
-  bool stream_error_on_invalid_http_messaging = 12;
+  google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14;
 
   // [#not-implemented-hide:]
   // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions:

api/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto

@@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // HTTP connection manager :ref:`configuration overview <config_http_conn_man>`.
 // [#extension: envoy.filters.network.http_connection_manager]
 
-// [#next-free-field: 40]
+// [#next-free-field: 41]
 message HttpConnectionManager {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager";
@@ -540,6 +540,23 @@ message HttpConnectionManager {
   // route with :ref:`domains<envoy_api_field_config.route.v3.VirtualHost.domains>` match set to `example`. Defaults to `false`. Note that port removal is not part
   // of `HTTP spec <https://tools.ietf.org/html/rfc3986>`_ and is provided for convenience.
   bool strip_matching_host_port = 39;
+
+  // Governs Envoy's behavior when receiving invalid HTTP from downstream.
+  // If this option is false (default), Envoy will err on the conservative side handling HTTP
+  // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request.
+  // If this option is set to true, Envoy will be more permissive, only resetting the invalid
+  // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire
+  // request is read for HTTP/1.1)
+  // In general this should be true for deployments receiving trusted traffic (L2 Envoys,
+  // company-internal mesh) and false when receiving untrusted traffic (edge deployments).
+  //
+  // If different behaviors for invalid_http_message for HTTP/1 and HTTP/2 are
+  // desired, one *must* use the new HTTP/2 option
+  // :ref:`override_stream_error_on_invalid_http_message
+  // <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>`
+  // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging
+  // <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.stream_error_on_invalid_http_messaging>`
+  google.protobuf.BoolValue stream_error_on_invalid_http_message = 40;
 }
 
 // The configuration to customize local reply returned by Envoy.

docs/root/configuration/best_practices/level_two.rst

@@ -5,14 +5,14 @@ Configuring Envoy as a level two proxy
 
 Envoy is a production-ready proxy, however, the default settings that are tailored for the
 edge use case may need to be adjusted when using Envoy in a multi-level deployment as a
-"level two" HTTP/2 proxy.
+"level two" proxy.
 
 .. image:: /_static/multilevel_deployment.svg
 
 **In summary, if you run level two Envoy version 1.11.1 or greater which terminates 
-HTTP/2, we strongly advise you to change the HTTP/2 configuration of your level 
+HTTP/2, we strongly advise you to change the HttpConnectionManager configuration of your level
 two Envoy, by setting its downstream**
-:ref:`validation of HTTP/2 messaging option <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.stream_error_on_invalid_http_messaging>`
+:ref:`validation of HTTP messaging option <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>`
 **to true.**
 
 If there is an invalid HTTP/2 request and this option is not set, the Envoy in 
@@ -29,9 +29,7 @@ user has insight into what traffic will bypass level one checks, they could spra
 “bad” traffic across the level one fleet, causing serious disruption to other users’ 
 traffic.
 
-Please note that the
-:ref:`validation of HTTP/2 messaging option <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.stream_error_on_invalid_http_messaging>`
-is planned to be deprecated and replaced with mandatory configuration in the HttpConnectionManager, to ensure
-that what is now an easily overlooked option would need to be configured, ideally
-appropriately for the given Envoy deployment. Please refer to the
-https://github.com/envoyproxy/envoy/issues/9285 for more information.
+This configuration option also has implications for invalid HTTP/1.1 though slightly less
+severe ones. For Envoy L1s, invalid HTTP/1 requests will also result in connection
+reset. If the option is set to true, and the request is completely read, the connection
+will persist and can be reused for a subsequent request.

docs/root/version_history/current.rst

@@ -10,6 +10,7 @@ Minor Behavior Changes
 *Changes that may cause incompatibilities for some users, but should not for most*
 
 * compressor: always insert `Vary` headers for compressible resources even if it's decided not to compress a response due to incompatible `Accept-Encoding` value. The `Vary` header needs to be inserted to let a caching proxy in front of Envoy know that the requested resource still can be served with compression applied.
+* http: added HCM level configuration of :ref:`error handling on invalid messaging <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` which substantially changes Envoy's behavior when encountering invalid HTTP/1.1 defaulting to closing the connection instead of allowing reuse. This can temporarily be reverted by setting `envoy.reloadable_features.hcm_stream_error_on_invalid_message` to false, or permanently reverted by setting the :ref:`HCM option <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.stream_error_on_invalid_http_message>` to true to restore prior HTTP/1.1 beavior and setting the *new* HTTP/2 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http2ProtocolOptions.override_stream_error_on_invalid_http_message>` to false to retain prior HTTP/2 behavior.
 * http: the per-stream FilterState maintained by the HTTP connection manager will now provide read/write access to the downstream connection FilterState. As such, code that relies on interacting with this might
   see a change in behavior.
 * logging: nghttp2 log messages no longer appear at trace level unless `ENVOY_NGHTTP2_TRACE` is set

source/common/http/BUILD

@@ -358,6 +358,7 @@ envoy_cc_library(
         "//source/common/json:json_loader_lib",
         "//source/common/network:utility_lib",
         "//source/common/protobuf:utility_lib",
+        "//source/common/runtime:runtime_features_lib",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )