http: processing of headers with underscores in names (#10611)

api/envoy/api/v2/core/protocol.proto

@@ -26,7 +26,28 @@ message UpstreamHttpProtocolOptions {
   bool auto_sni = 1;
 }
 
+// [#next-free-field: 6]
 message HttpProtocolOptions {
+  // Action to take when Envoy receives client request with header names containing underscore
+  // characters.
+  // Underscore character is allowed in header names by the RFC-7230 and this behavior is implemented
+  // as a security measure due to systems that treat '_' and '-' as interchangeable. Envoy by default allows client request headers with underscore
+  // characters.
+  enum HeadersWithUnderscoresAction {
+    // Allow headers with underscores. This is the default behavior.
+    ALLOW = 0;
+
+    // Reject client request. HTTP/1 requests are rejected with the 400 status. HTTP/2 requests
+    // end with the stream reset. The "httpN.requests_rejected_with_underscores_in_headers" counter
+    // is incremented for each rejected request.
+    REJECT_REQUEST = 1;
+
+    // Drop the header with name containing underscores. The header is dropped before the filter chain is
+    // invoked and as such filters will not see dropped headers. The
+    // "httpN.dropped_headers_with_underscores" is incremented for each dropped header.
+    DROP_HEADER = 2;
+  }
+
   // The idle timeout for connections. The idle timeout is defined as the
   // period in which there are no active requests. If not set, there is no idle timeout. When the
   // idle timeout is reached the connection will be closed. If the connection is an HTTP/2
@@ -53,6 +74,11 @@ message HttpProtocolOptions {
   // maximum number of request headers allowed is 100. Requests that exceed this limit will receive
   // a 431 response for HTTP/1.x and cause a stream reset for HTTP/2.
   google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];
+
+  // Action to take when a client request with a header name containing underscore characters is received.
+  // If this setting is not specified, the value defaults to ALLOW.
+  // Note: upstream responses are not affected by this setting.
+  HeadersWithUnderscoresAction headers_with_underscores_action = 5;
 }
 
 // [#next-free-field: 6]

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

@@ -31,10 +31,31 @@ message UpstreamHttpProtocolOptions {
   bool auto_sni = 1;
 }
 
+// [#next-free-field: 6]
 message HttpProtocolOptions {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.api.v2.core.HttpProtocolOptions";
 
+  // Action to take when Envoy receives client request with header names containing underscore
+  // characters.
+  // Underscore character is allowed in header names by the RFC-7230 and this behavior is implemented
+  // as a security measure due to systems that treat '_' and '-' as interchangeable. Envoy by default allows client request headers with underscore
+  // characters.
+  enum HeadersWithUnderscoresAction {
+    // Allow headers with underscores. This is the default behavior.
+    ALLOW = 0;
+
+    // Reject client request. HTTP/1 requests are rejected with the 400 status. HTTP/2 requests
+    // end with the stream reset. The "httpN.requests_rejected_with_underscores_in_headers" counter
+    // is incremented for each rejected request.
+    REJECT_REQUEST = 1;
+
+    // Drop the header with name containing underscores. The header is dropped before the filter chain is
+    // invoked and as such filters will not see dropped headers. The
+    // "httpN.dropped_headers_with_underscores" is incremented for each dropped header.
+    DROP_HEADER = 2;
+  }
+
   // The idle timeout for connections. The idle timeout is defined as the
   // period in which there are no active requests. If not set, there is no idle timeout. When the
   // idle timeout is reached the connection will be closed. If the connection is an HTTP/2
@@ -61,6 +82,11 @@ message HttpProtocolOptions {
   // maximum number of request headers allowed is 100. Requests that exceed this limit will receive
   // a 431 response for HTTP/1.x and cause a stream reset for HTTP/2.
   google.protobuf.UInt32Value max_headers_count = 2 [(validate.rules).uint32 = {gte: 1}];
+
+  // Action to take when a client request with a header name containing underscore characters is received.
+  // If this setting is not specified, the value defaults to ALLOW.
+  // Note: upstream responses are not affected by this setting.
+  HeadersWithUnderscoresAction headers_with_underscores_action = 5;
 }
 
 // [#next-free-field: 6]

docs/root/configuration/http/http_conn_man/stats.rst

@@ -110,8 +110,10 @@ All http1 statistics are rooted at *http1.*
    :header: Name, Type, Description
    :widths: 1, 1, 2
 
+   dropped_headers_with_underscores, Counter, Total number of dropped headers with names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>`.
    metadata_not_supported_error, Counter, Total number of metadata dropped during HTTP/1 encoding
    response_flood, Counter, Total number of connections closed due to response flooding
+   requests_rejected_with_underscores_in_headers, Counter, Total numbers of rejected requests due to header names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>`.
 
 Http2 codec statistics
 ~~~~~~~~~~~~~~~~~~~~~~
@@ -122,13 +124,15 @@ All http2 statistics are rooted at *http2.*
    :header: Name, Type, Description
    :widths: 1, 1, 2
 
+   dropped_headers_with_underscores, Counter, Total number of dropped headers with names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>`.
    header_overflow, Counter, Total number of connections reset due to the headers being larger than the :ref:`configured value <envoy_api_field_config.filter.network.http_connection_manager.v2.HttpConnectionManager.max_request_headers_kb>`.
    headers_cb_no_stream, Counter, Total number of errors where a header callback is called without an associated stream. This tracks an unexpected occurrence due to an as yet undiagnosed bug
    inbound_empty_frames_flood, Counter, Total number of connections terminated for exceeding the limit on consecutive inbound frames with an empty payload and no end stream flag. The limit is configured by setting the :ref:`max_consecutive_inbound_frames_with_empty_payload config setting <envoy_api_field_core.Http2ProtocolOptions.max_consecutive_inbound_frames_with_empty_payload>`.
    inbound_priority_frames_flood, Counter, Total number of connections terminated for exceeding the limit on inbound frames of type PRIORITY. The limit is configured by setting the :ref:`max_inbound_priority_frames_per_stream config setting <envoy_api_field_core.Http2ProtocolOptions.max_inbound_priority_frames_per_stream>`.
    inbound_window_update_frames_flood, Counter, Total number of connections terminated for exceeding the limit on inbound frames of type WINDOW_UPDATE. The limit is configured by setting the :ref:`max_inbound_window_updateframes_per_data_frame_sent config setting <envoy_api_field_core.Http2ProtocolOptions.max_inbound_window_update_frames_per_data_frame_sent>`.
    outbound_flood, Counter, Total number of connections terminated for exceeding the limit on outbound frames of all types. The limit is configured by setting the :ref:`max_outbound_frames config setting <envoy_api_field_core.Http2ProtocolOptions.max_outbound_frames>`.
    outbound_control_flood, Counter, "Total number of connections terminated for exceeding the limit on outbound frames of types PING, SETTINGS and RST_STREAM. The limit is configured by setting the :ref:`max_outbound_control_frames config setting <envoy_api_field_core.Http2ProtocolOptions.max_outbound_control_frames>`."
+   requests_rejected_with_underscores_in_headers, Counter, Total numbers of rejected requests due to header names containing underscores. This action is configured by setting the :ref:`headers_with_underscores_action config setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>`.
    rx_messaging_error, Counter, Total number of invalid received frames that violated `section 8 <https://tools.ietf.org/html/rfc7540#section-8>`_ of the HTTP/2 spec. This will result in a *tx_reset*
    rx_reset, Counter, Total number of reset stream frames received by Envoy
    too_many_header_frames, Counter, Total number of times an HTTP2 connection is reset due to receiving too many headers frames. Envoy currently supports proxying at most one header frame for 100-Continue one non-100 response code header frame and one frame with trailers

docs/root/intro/version_history.rst

@@ -1,6 +1,10 @@
 Version history
 ---------------
 
+1.13.2 (Pending)
+================
+* http: added :ref:`headers_with_underscores_action setting <envoy_api_field_core.HttpProtocolOptions.headers_with_underscores_action>` to control how client requests with header names containing underscore characters are handled. The options are to allow such headers, reject request or drop headers. The default is to allow headers, preserving existing behavior.
+
 1.13.1 (March 3, 2020)
 ======================
 * buffer: force copy when appending small slices to OwnedImpl buffer to avoid fragmentation.

source/common/http/conn_manager_config.h

@@ -391,6 +391,13 @@ class ConnectionManagerConfig {
    * one.
    */
   virtual bool shouldMergeSlashes() const PURE;
+
+  /**
+   * @return the action HttpConnectionManager should take when receiving client request
+   * headers containing underscore characters.
+   */
+  virtual envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
+  headersWithUnderscoresAction() const PURE;
 };
 } // namespace Http
 } // namespace Envoy

source/common/http/conn_manager_utility.cc

@@ -44,15 +44,17 @@ ServerConnectionPtr ConnectionManagerUtility::autoCreateCodec(
     Network::Connection& connection, const Buffer::Instance& data,
     ServerConnectionCallbacks& callbacks, Stats::Scope& scope, const Http1Settings& http1_settings,
     const Http2Settings& http2_settings, uint32_t max_request_headers_kb,
-    uint32_t max_request_headers_count) {
+    uint32_t max_request_headers_count,
+    envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
+        headers_with_underscores_action) {
   if (determineNextProtocol(connection, data) == Http2::ALPN_STRING) {
-    return std::make_unique<Http2::ServerConnectionImpl>(connection, callbacks, scope,
-                                                         http2_settings, max_request_headers_kb,
-                                                         max_request_headers_count);
+    return std::make_unique<Http2::ServerConnectionImpl>(
+        connection, callbacks, scope, http2_settings, max_request_headers_kb,
+        max_request_headers_count, headers_with_underscores_action);
   } else {
-    return std::make_unique<Http1::ServerConnectionImpl>(connection, scope, callbacks,
-                                                         http1_settings, max_request_headers_kb,
-                                                         max_request_headers_count);
+    return std::make_unique<Http1::ServerConnectionImpl>(
+        connection, scope, callbacks, http1_settings, max_request_headers_kb,
+        max_request_headers_count, headers_with_underscores_action);
   }
 }
 

source/common/http/conn_manager_utility.h

@@ -37,7 +37,9 @@ class ConnectionManagerUtility {
   autoCreateCodec(Network::Connection& connection, const Buffer::Instance& data,
                   ServerConnectionCallbacks& callbacks, Stats::Scope& scope,
                   const Http1Settings& http1_settings, const Http2Settings& http2_settings,
-                  uint32_t max_request_headers_kb, uint32_t max_request_headers_count);
+                  uint32_t max_request_headers_kb, uint32_t max_request_headers_count,
+                  envoy::config::core::v3::HttpProtocolOptions::HeadersWithUnderscoresAction
+                      headers_with_underscores_action);
 
   /**
    * Mutates request headers in various ways. This functionality is broken out because of its

source/common/http/header_utility.cc

@@ -143,11 +143,15 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, const HeaderD
   return match != header_data.invert_match_;
 }
 
-bool HeaderUtility::headerIsValid(const absl::string_view header_value) {
+bool HeaderUtility::headerValueIsValid(const absl::string_view header_value) {
   return nghttp2_check_header_value(reinterpret_cast<const uint8_t*>(header_value.data()),
                                     header_value.size()) != 0;
 }
 
+bool HeaderUtility::headerNameContainsUnderscore(const absl::string_view header_name) {
+  return header_name.find('_') != absl::string_view::npos;
+}
+
 bool HeaderUtility::authorityIsValid(const absl::string_view header_value) {
   return nghttp2_check_authority(reinterpret_cast<const uint8_t*>(header_value.data()),
                                  header_value.size()) != 0;

source/common/http/header_utility.h

@@ -94,7 +94,16 @@ class HeaderUtility {
    * http://tools.ietf.org/html/rfc7230#section-3.2
    * @return bool true if the header values are valid, according to the aforementioned RFC.
    */
-  static bool headerIsValid(const absl::string_view header_value);
+  static bool headerValueIsValid(const absl::string_view header_value);
+
+  /**
+   * Checks if header name contains underscore characters.
+   * Underscore character is allowed in header names by the RFC-7230 and this check is implemented
+   * as a security measure due to systems that treat '_' and '-' as interchangeable. Envoy by
+   * default allows headers with underscore characters.
+   * @return bool true if header name contains underscore characters.
+   */
+  static bool headerNameContainsUnderscore(const absl::string_view header_name);
 
   /**
    * Validates that the characters in the authority are valid.

source/common/http/http1/BUILD

@@ -40,6 +40,7 @@ envoy_cc_library(
         "//source/common/http:utility_lib",
         "//source/common/http/http1:header_formatter_lib",
         "//source/common/runtime:runtime_lib",
+        "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )