http: stream error on invalid HTTP messaging for HTTP/1

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

@@ -100,7 +100,7 @@ message HttpProtocolOptions {
   HeadersWithUnderscoresAction headers_with_underscores_action = 5;
 }
 
-// [#next-free-field: 6]
+// [#next-free-field: 7]
 message Http1ProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.Http1ProtocolOptions";
@@ -157,6 +157,16 @@ message Http1ProtocolOptions {
   //   - Not a response to a HEAD request.
   //   - The content length header is not present.
   bool enable_trailers = 5;
+
+  // Allows invalid HTTP messaging. When this option is false, then Envoy will terminate
+  // HTTP/1.1 connections upon receiving an invalid HTTP message. However,
+  // when this option is true, then Envoy will leave the HTTP/1.1 connection
+  // open where possible.
+  // If set, 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>`;
+  // if not set, this is overridden by 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>`, which defaults to false.
+  google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 6;
 }
 
 // [#next-free-field: 15]

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

@@ -552,7 +552,8 @@ message HttpConnectionManager {
   // 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
+  // desired, one should use the new HTTP/1 option :ref:`override_stream_error_on_invalid_http_message
+  // <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>` or 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

docs/root/version_history/current.rst

@@ -14,7 +14,7 @@ Minor Behavior Changes
 * 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.
 * decompressor: headers-only requests were incorrectly not advertising accept-encoding when configured to do so. This is now fixed.
 * http: added :ref:`headers_to_add <envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.ResponseMapper.headers_to_add>` to :ref:`local reply mapper <config_http_conn_man_local_reply>` to allow its users to add/append/override response HTTP headers to local replies.
-* 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: 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 HTTP/1 configuration :ref:`override_stream_error_on_invalid_http_message <envoy_v3_api_field_config.core.v3.Http1ProtocolOptions.override_stream_error_on_invalid_http_message>` to true to restore prior HTTP/1.1 behavior (i.e. connection isn't terminated) and to retain prior HTTP/2 behavior (i.e. connection is terminated).
 * http: changed Envoy to send error headers and body when possible. This behavior may be temporarily reverted by setting `envoy.reloadable_features.allow_response_for_timeout` to false.
 * http: changed empty trailers encoding behavior by sending empty data with ``end_stream`` true (instead of sending empty trailers) for HTTP/2. This behavior can be reverted temporarily by setting runtime feature ``envoy.reloadable_features.http2_skip_encoding_empty_trailers`` to false.
 * http: clarified and enforced 1xx handling. Multiple 100-continue headers are coalesced when proxying. 1xx headers other than {100, 101} are dropped.

include/envoy/http/codec.h

@@ -142,6 +142,12 @@ class ResponseEncoder : public virtual StreamEncoder {
    * @param trailers supplies the trailers to encode.
    */
   virtual void encodeTrailers(const ResponseTrailerMap& trailers) PURE;
+
+  /**
+   * Indicates whether invalid HTTP messaging should be handled with a stream error or a connection
+   * error; if value is missing, then HCM drives behaviour on invalid HTTP messaging.
+   */
+  virtual absl::optional<bool> streamErrorOnInvalidHttpMessage() const PURE;
 };
 
 /**
@@ -386,6 +392,12 @@ struct Http1Settings {
 
   // How header keys should be formatted when serializing HTTP/1.1 headers.
   HeaderKeyFormat header_key_format_{HeaderKeyFormat::Default};
+
+  // Behaviour on invalid HTTP messaging:
+  // - if set and true, the HTTP/1.1 connection is left open (where possible)
+  // - if set and false, the HTTP/1.1 connection is terminated
+  // - if not set, fall back to the HCM behaviour on invalid HTTP messaging
+  absl::optional<bool> stream_error_on_invalid_http_message_;
 };
 
 /**

source/common/http/conn_manager_impl.cc

@@ -80,6 +80,16 @@ void recordLatestDataFilter(const typename FilterList<T>::iterator current_filte
   }
 }
 
+bool streamErrorOnInvalidHttpMessage(
+    bool hcm_stream_error_on_invalid_http_message,
+    const absl::optional<bool> override_stream_error_on_invalid_http_message) {
+  if (override_stream_error_on_invalid_http_message.has_value()) {
+    return override_stream_error_on_invalid_http_message.value();
+  } else {
+    return hcm_stream_error_on_invalid_http_message;
+  }
+}
+
 } // namespace
 
 ConnectionManagerStats ConnectionManagerImpl::generateStats(const std::string& prefix,
@@ -1578,8 +1588,10 @@ void ConnectionManagerImpl::ActiveStream::sendLocalReply(
   // The BadRequest error code indicates there has been a messaging error.
   if (Runtime::runtimeFeatureEnabled(
           "envoy.reloadable_features.hcm_stream_error_on_invalid_message") &&
-      !connection_manager_.config_.streamErrorOnInvalidHttpMessaging() &&
-      code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2) {
+      code == Http::Code::BadRequest && connection_manager_.codec_->protocol() < Protocol::Http2 &&
+      !streamErrorOnInvalidHttpMessage(
+          connection_manager_.config_.streamErrorOnInvalidHttpMessaging(),
+          response_encoder_->streamErrorOnInvalidHttpMessage())) {
     state_.saw_connection_close_ = true;
   }
 

source/common/http/http1/codec_impl.h

@@ -123,8 +123,10 @@ class StreamEncoderImpl : public virtual StreamEncoder,
  */
 class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
 public:
-  ResponseEncoderImpl(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
-      : StreamEncoderImpl(connection, header_key_formatter) {}
+  ResponseEncoderImpl(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter,
+                      absl::optional<bool>& stream_error_on_invalid_http_message)
+      : StreamEncoderImpl(connection, header_key_formatter),
+        stream_error_on_invalid_http_message_(stream_error_on_invalid_http_message) {}
 
   bool startedResponse() { return started_response_; }
 
@@ -133,8 +135,13 @@ class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
   void encodeHeaders(const ResponseHeaderMap& headers, bool end_stream) override;
   void encodeTrailers(const ResponseTrailerMap& trailers) override { encodeTrailersBase(trailers); }
 
+  absl::optional<bool> streamErrorOnInvalidHttpMessage() const override {
+    return stream_error_on_invalid_http_message_;
+  }
+
 private:
   bool started_response_{};
+  const absl::optional<bool> stream_error_on_invalid_http_message_;
 };
 
 /**
@@ -463,8 +470,9 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
    * An active HTTP/1.1 request.
    */
   struct ActiveRequest {
-    ActiveRequest(ConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
-        : response_encoder_(connection, header_key_formatter) {}
+    ActiveRequest(ServerConnectionImpl& connection, HeaderKeyFormatter* header_key_formatter)
+        : response_encoder_(connection, header_key_formatter,
+                            connection.codec_settings_.stream_error_on_invalid_http_message_) {}
 
     HeaderString request_url_;
     RequestDecoder* request_decoder_{};

source/common/http/http1/codec_impl_legacy.h

@@ -126,8 +126,10 @@ class StreamEncoderImpl : public virtual StreamEncoder,
 class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
 public:
   ResponseEncoderImpl(ConnectionImpl& connection,
-                      Http::Http1::HeaderKeyFormatter* header_key_formatter)
-      : StreamEncoderImpl(connection, header_key_formatter) {}
+                      Http::Http1::HeaderKeyFormatter* header_key_formatter,
+                      absl::optional<bool>& stream_error_on_invalid_http_message)
+      : StreamEncoderImpl(connection, header_key_formatter),
+        stream_error_on_invalid_http_message_(stream_error_on_invalid_http_message) {}
 
   bool startedResponse() { return started_response_; }
 
@@ -136,8 +138,13 @@ class ResponseEncoderImpl : public StreamEncoderImpl, public ResponseEncoder {
   void encodeHeaders(const ResponseHeaderMap& headers, bool end_stream) override;
   void encodeTrailers(const ResponseTrailerMap& trailers) override { encodeTrailersBase(trailers); }
 
+  absl::optional<bool> streamErrorOnInvalidHttpMessage() const override {
+    return stream_error_on_invalid_http_message_;
+  }
+
 private:
   bool started_response_{};
+  const absl::optional<bool> stream_error_on_invalid_http_message_;
 };
 
 /**
@@ -436,8 +443,10 @@ class ServerConnectionImpl : public ServerConnection, public ConnectionImpl {
    * An active HTTP/1.1 request.
    */
   struct ActiveRequest {
-    ActiveRequest(ConnectionImpl& connection, Http::Http1::HeaderKeyFormatter* header_key_formatter)
-        : response_encoder_(connection, header_key_formatter) {}
+    ActiveRequest(ServerConnectionImpl& connection,
+                  Http::Http1::HeaderKeyFormatter* header_key_formatter)
+        : response_encoder_(connection, header_key_formatter,
+                            connection.codec_settings_.stream_error_on_invalid_http_message_) {}
 
     HeaderString request_url_;
     RequestDecoder* request_decoder_{};

source/common/http/http2/codec_impl.h

@@ -391,6 +391,10 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
     RequestDecoder* request_decoder_{};
     absl::variant<RequestHeaderMapPtr, RequestTrailerMapPtr> headers_or_trailers_;
+
+    absl::optional<bool> streamErrorOnInvalidHttpMessage() const override {
+      return parent_.stream_error_on_invalid_http_messaging_;
+    }
   };
 
   using ServerStreamImplPtr = std::unique_ptr<ServerStreamImpl>;

source/common/http/http2/codec_impl_legacy.h

@@ -359,7 +359,9 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
    */
   struct ServerStreamImpl : public StreamImpl, public ResponseEncoder {
     ServerStreamImpl(ConnectionImpl& parent, uint32_t buffer_limit)
-        : StreamImpl(parent, buffer_limit), headers_or_trailers_(RequestHeaderMapImpl::create()) {}
+        : StreamImpl(parent, buffer_limit), headers_or_trailers_(RequestHeaderMapImpl::create()) {
+      stream_error_on_invalid_http_message_ = parent.stream_error_on_invalid_http_messaging_;
+    }
 
     // StreamImpl
     void submitHeaders(const std::vector<nghttp2_nv>& final_headers,
@@ -391,6 +393,11 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
 
     RequestDecoder* request_decoder_{};
     absl::variant<RequestHeaderMapPtr, RequestTrailerMapPtr> headers_or_trailers_;
+    absl::optional<bool> stream_error_on_invalid_http_message_;
+
+    absl::optional<bool> streamErrorOnInvalidHttpMessage() const override {
+      return stream_error_on_invalid_http_message_;
+    }
   };
 
   using ServerStreamImplPtr = std::unique_ptr<ServerStreamImpl>;

source/common/http/utility.cc

@@ -411,6 +411,11 @@ Utility::parseHttp1Settings(const envoy::config::core::v3::Http1ProtocolOptions&
     ret.header_key_format_ = Http1Settings::HeaderKeyFormat::Default;
   }
 
+  if (config.has_override_stream_error_on_invalid_http_message()) {
+    ret.stream_error_on_invalid_http_message_ =
+        config.override_stream_error_on_invalid_http_message().value();
+  }
+
   return ret;
 }
 

source/extensions/quic_listeners/quiche/envoy_quic_server_stream.h

@@ -36,6 +36,7 @@ class EnvoyQuicServerStream : public quic::QuicSpdyServerStreamBase,
   Http::Http1StreamEncoderOptionsOptRef http1StreamEncoderOptions() override {
     return absl::nullopt;
   }
+  absl::optional<bool> streamErrorOnInvalidHttpMessage() const override { return absl::nullopt; }
 
   // Http::Stream
   void resetStream(Http::StreamResetReason reason) override;