Sanitize headers nominated by the Connection header

docs/root/intro/version_history.rst

@@ -9,6 +9,7 @@ Version history
 * build: official released binary is now built against libc++.
 * ext_authz: added :ref:`configurable ability<envoy_api_field_config.filter.http.ext_authz.v2.ExtAuthz.include_peer_certificate>` to send the :ref:`certificate<envoy_api_field_service.auth.v2.AttributeContext.Peer.certificate>` to the `ext_authz` service.
 * health check: gRPC health checker sets the gRPC deadline to the configured timeout duration.
+* http: added the ability to sanitize headers nominated by the Connection header. This new behavior is guarded by envoy.reloadable_features.connection_header_sanitization which defaults to true.
 * http: support :ref:`auto_host_rewrite_header<envoy_api_field_config.filter.http.dynamic_forward_proxy.v2alpha.PerRouteConfig.auto_host_rewrite_header>` in the dynamic forward proxy.
 * jwt_authn: added :ref:`bypass_cors_preflight<envoy_api_field_config.filter.http.jwt_authn.v2alpha.JwtAuthentication.bypass_cors_preflight>` to allow bypassing the CORS preflight request.
 * lb_subset_config: new fallback policy for selectors: :ref:`KEYS_SUBSET<envoy_api_enum_value_Cluster.LbSubsetConfig.LbSubsetSelector.LbSubsetSelectorFallbackPolicy.KEYS_SUBSET>`

source/common/http/http1/codec_impl.cc

@@ -360,7 +360,9 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& st
                      [&]() -> void { this->onAboveHighWatermark(); }),
       max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count),
       strict_header_validation_(
-          Runtime::runtimeFeatureEnabled("envoy.reloadable_features.strict_header_validation")) {
+          Runtime::runtimeFeatureEnabled("envoy.reloadable_features.strict_header_validation")),
+      connection_header_sanitization_(Runtime::runtimeFeatureEnabled(
+          "envoy.reloadable_features.connection_header_sanitization")) {
   output_buffer_.setWatermarks(connection.bufferLimit());
   http_parser_init(&parser_, type);
   parser_.data = this;
@@ -532,6 +534,15 @@ int ConnectionImpl::onHeadersCompleteBase() {
       ENVOY_CONN_LOG(trace, "codec entering upgrade mode.", connection_);
       handling_upgrade_ = true;
     }
+  } else if (connection_header_sanitization_ && current_header_map_->Connection()) {
+    // If we fail to sanitize the request, return a 400 to the client
+    if (!Utility::sanitizeConnectionHeader(*current_header_map_)) {
+      absl::string_view header_value = current_header_map_->Connection()->value().getStringView();
+      ENVOY_CONN_LOG(debug, "Invalid nominated headers in Connection: {}", connection_,
+                     header_value);
+      error_code_ = Http::Code::BadRequest;
+      sendProtocolError();
+    }
   }
 
   int rc = onHeadersComplete(std::move(current_header_map_));

source/common/http/http1/codec_impl.h

@@ -308,7 +308,8 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable<Log
   const uint32_t max_headers_kb_;
   const uint32_t max_headers_count_;
 
-  bool strict_header_validation_;
+  const bool strict_header_validation_;
+  const bool connection_header_sanitization_;
 };
 
 /**

source/common/http/utility.cc

@@ -24,6 +24,7 @@
 
 #include "absl/strings/match.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_split.h"
 
 namespace Envoy {
 namespace Http {
@@ -387,6 +388,129 @@ Utility::getLastAddressFromXFF(const Http::HeaderMap& request_headers, uint32_t
   }
 }
 
+bool Utility::sanitizeConnectionHeader(Http::HeaderMap& headers) {
+  static const size_t MAX_ALLOWED_NOMINATED_HEADERS = 10;
+  static const size_t MAX_ALLOWED_TE_VALUE_SIZE = 256;
+
+  // Remove any headers nominated by the Connection header. The TE header
+  // is sanitized and removed only if it's empty after removing unsupported values
+  // See https://github.com/envoyproxy/envoy/issues/8623
+  const auto& cv = Http::Headers::get().ConnectionValues;
+  const auto& connection_header_value = headers.Connection()->value();
+
+  StringUtil::CaseUnorderedSet headers_to_remove{};
+  std::vector<absl::string_view> connection_header_tokens =
+      StringUtil::splitToken(connection_header_value.getStringView(), ",", false);
+
+  // If we have 10 or more nominated headers, fail this request
+  if (connection_header_tokens.size() >= MAX_ALLOWED_NOMINATED_HEADERS) {
+    ENVOY_LOG_MISC(trace, "Too many nominated headers in request");
+    return false;
+  }
+
+  // Split the connection header and evaluate each nominated header
+  for (const auto& token : connection_header_tokens) {
+
+    const auto token_sv = StringUtil::trim(token);
+
+    // Build the LowerCaseString for header lookup
+    const LowerCaseString lcs_header_to_remove{std::string(token_sv)};
+
+    // If the Connection token value is not a nominated header, ignore it here since
+    // the connection header is removed elsewhere when the H1 request is upgraded to H2
+    if ((lcs_header_to_remove.get() == cv.Close) ||
+        (lcs_header_to_remove.get() == cv.Http2Settings) ||
+        (lcs_header_to_remove.get() == cv.KeepAlive) ||
+        (lcs_header_to_remove.get() == cv.Upgrade)) {
+      continue;
+    }
+
+    // By default we will remove any nominated headers
+    bool keep_header = false;
+
+    // Determine whether the nominated header contains invalid values
+    HeaderEntry* nominated_header = NULL;
+
+    if (lcs_header_to_remove == Http::Headers::get().Connection) {
+      // Remove the connection header from the nominated tokens if it's self nominated
+      // The connection header itself is *not removed*
+      ENVOY_LOG_MISC(trace, "Skipping self nominated header [{}]", token_sv);
+      keep_header = true;
+      headers_to_remove.emplace(token_sv);
+
+    } else if ((lcs_header_to_remove == Http::Headers::get().ForwardedFor) ||
+               (lcs_header_to_remove == Http::Headers::get().ForwardedHost) ||
+               (lcs_header_to_remove == Http::Headers::get().ForwardedProto) ||
+               !token_sv.find(':')) {
+
+      // An attacker could nominate an X-Forwarded* header, and its removal may mask
+      // the origin of the incoming request and potentially alter its handling.
+      // Additionally, pseudo headers should never be nominated. In both cases, we
+      // should fail the request.
+      // See: https://nathandavison.com/blog/abusing-http-hop-by-hop-request-headers
+
+      ENVOY_LOG_MISC(trace, "Invalid nomination of {} header", token_sv);
+      return false;
+    } else {
+      // Examine the value of all other nominated headers
+      nominated_header = headers.get(lcs_header_to_remove);
+    }
+
+    if (nominated_header) {
+      auto nominated_header_value_sv = nominated_header->value().getStringView();
+
+      const bool is_te_header = (lcs_header_to_remove == Http::Headers::get().TE);
+
+      // reject the request if the TE header is too large
+      if (is_te_header && (nominated_header_value_sv.size() >= MAX_ALLOWED_TE_VALUE_SIZE)) {
+        ENVOY_LOG_MISC(trace, "TE header contains a value that exceeds the allowable length");
+        return false;
+      }
+
+      if (is_te_header) {
+        for (const auto& header_value :
+             StringUtil::splitToken(nominated_header_value_sv, ",", false)) {
+
+          const absl::string_view header_sv = StringUtil::trim(header_value);
+
+          // If trailers exist in the TE value tokens, keep the header, removing any other values
+          // that may exist
+          if (StringUtil::CaseInsensitiveCompare()(header_sv,
+                                                   Http::Headers::get().TEValues.Trailers)) {
+            keep_header = true;
+            break;
+          }
+        }
+
+        if (keep_header) {
+          nominated_header->value().setCopy(Http::Headers::get().TEValues.Trailers.data(),
+                                            Http::Headers::get().TEValues.Trailers.size());
+        }
+      }
+    }
+
+    if (!keep_header) {
+      ENVOY_LOG_MISC(trace, "Removing nominated header [{}]", token_sv);
+      headers.remove(lcs_header_to_remove);
+      headers_to_remove.emplace(token_sv);
+    }
+  }
+
+  // Lastly remove extra nominated headers from the Connection header
+  if (!headers_to_remove.empty()) {
+    const std::string new_value = StringUtil::removeTokens(connection_header_value.getStringView(),
+                                                           ",", headers_to_remove, ",");
+
+    if (new_value.empty()) {
+      headers.removeConnection();
+    } else {
+      headers.Connection()->value(new_value);
+    }
+  }
+
+  return true;
+}
+
 const std::string& Utility::getProtocolString(const Protocol protocol) {
   switch (protocol) {
   case Protocol::Http10:
@@ -568,8 +692,8 @@ std::string Utility::PercentEncoding::encode(absl::string_view value) {
     // https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#responses.
     //
     // We do checking for each char in the string. If the current char is included in the defined
-    // escaping characters, we jump to "the slow path" (append the char [encoded or not encoded] to
-    // the returned string one by one) started from the current index.
+    // escaping characters, we jump to "the slow path" (append the char [encoded or not encoded]
+    // to the returned string one by one) started from the current index.
     if (ch < ' ' || ch >= '~' || ch == '%') {
       return PercentEncoding::encode(value, i);
     }

source/common/http/utility.h

@@ -231,6 +231,15 @@ struct GetLastAddressFromXffInfo {
 GetLastAddressFromXffInfo getLastAddressFromXFF(const Http::HeaderMap& request_headers,
                                                 uint32_t num_to_skip = 0);
 
+/**
+ * Remove any headers nominated by the Connection header
+ * Sanitize the TE header if it contains unsupported values
+ *
+ * @param headers the client request headers
+ * @return whether the headers were sanitized successfully
+ */
+bool sanitizeConnectionHeader(Http::HeaderMap& headers);
+
 /**
  * Get the string for the given http protocol.
  * @param protocol for which to return the string representation.

source/common/runtime/runtime_features.cc

@@ -29,6 +29,7 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.buffer_filter_populate_content_length",
     "envoy.reloadable_features.trusted_forwarded_proto",
     "envoy.reloadable_features.outlier_detection_support_for_grpc_status",
+    "envoy.reloadable_features.connection_header_sanitization",
 };
 
 // This is a list of configuration fields which are disallowed by default in Envoy