router: add config to reject requests with invalid envoy headers

api/envoy/config/filter/accesslog/v2/accesslog.proto

@@ -191,7 +191,8 @@ message ResponseFlagFilter {
       "RLSE",
       "DC",
       "URX",
-      "SI"
+      "SI",
+      "IH"
     ]
   }];
 }

api/envoy/config/filter/http/router/v2/BUILD

@@ -1,4 +1,4 @@
-load("@envoy_api//bazel:api_build_system.bzl", "api_proto_library_internal")
+load("@envoy_api//bazel:api_build_system.bzl", "api_go_proto_library", "api_proto_library_internal")
 
 licenses(["notice"])  # Apache 2
 

api/envoy/config/filter/http/router/v2/router.proto

@@ -11,6 +11,8 @@ import "envoy/config/filter/accesslog/v2/accesslog.proto";
 
 import "google/protobuf/wrappers.proto";
 
+import "validate/validate.proto";
+
 // [#protodoc-title: Router]
 // Router :ref:`configuration overview <config_http_filters_router>`.
 
@@ -36,4 +38,30 @@ message Router {
   // <config_http_filters_router_headers_set>`, other Envoy filters and the HTTP
   // connection manager may continue to set *x-envoy-* headers.
   bool suppress_envoy_headers = 4;
+
+  // Specifies a list of HTTP headers to strictly validate. Envoy will reject a
+  // request and respond with HTTP status 400 if the request contains an invalid
+  // value for any of the headers listed in this field. Strict header checking
+  // is only supported for the following headers:
+  //
+  // Value must be a ','-delimited list (i.e. no spaces) of supported retry
+  // policy values:
+  //
+  // * :ref:`config_http_filters_router_x-envoy-retry-grpc-on`
+  // * :ref:`config_http_filters_router_x-envoy-retry-on`
+  //
+  // Value must be an integer:
+  //
+  // * :ref:`config_http_filters_router_x-envoy-max-retries`
+  // * :ref:`config_http_filters_router_x-envoy-upstream-rq-timeout-ms`
+  // * :ref:`config_http_filters_router_x-envoy-upstream-rq-per-try-timeout-ms`
+  repeated string strict_check_headers = 5 [(validate.rules).repeated .items.string = {
+    in: [
+      "x-envoy-upstream-rq-timeout-ms",
+      "x-envoy-upstream-rq-per-try-timeout-ms",
+      "x-envoy-max-retries",
+      "x-envoy-retry-grpc-on",
+      "x-envoy-retry-on"
+    ]
+  }];
 }

api/envoy/data/accesslog/v2/accesslog.proto

@@ -191,6 +191,9 @@ message ResponseFlags {
       REASON_UNSPECIFIED = 0;
       // The request was denied by the external authorization service.
       EXTERNAL_SERVICE = 1;
+      // The request was denied because a header value failed a :ref:`strict header check
+      // <envoy_api_field_config.filter.http.router.v2.Router.strict_check_headers>` failed.
+      STRICT_HEADER_CHECK_FAILED = 2;
     }
 
     Reason reason = 1;

docs/root/configuration/access_log.rst

@@ -218,6 +218,8 @@ The following command operators are supported:
     * **RL**: The request was ratelimited locally by the :ref:`HTTP rate limit filter <config_http_filters_rate_limit>` in addition to 429 response code.
     * **UAEX**: The request was denied by the external authorization service.
     * **RLSE**: The request was rejected because there was an error in rate limit service.
+    * **IH**: The request was rejected because it set an invalid value for a
+      :ref:`strictly-checked header <envoy_api_field_config.filter.http.router.v2.Router.strict_check_headers>` in addition to 400 response code.
     * **SI**: Stream idle timeout in addition to 408 response code.
 
 %RESPONSE_TX_DURATION%

docs/root/intro/version_history.rst

@@ -7,6 +7,7 @@ Version history
 * access log: added a new field for route name to file and gRPC access logger.
 * access log: added a new field for response code details in :ref:`file access logger<config_access_log_format_response_code_details>` and :ref:`gRPC access logger<envoy_api_field_data.accesslog.v2.HTTPResponseProperties.response_code_details>`.
 * access log: added several new variables for exposing information about the downstream TLS connection to :ref:`file access logger<config_access_log_format_response_code_details>` and :ref:`gRPC access logger<envoy_api_field_data.accesslog.v2.AccessLogCommon.tls_properties>`.
+* access log: added a new flag for request rejected due to failed strict header check
 * admin: the administration interface now includes a :ref:`/ready endpoint <operations_admin_interface>` for easier readiness checks.
 * admin: extend :ref:`/runtime_modify endpoint <operations_admin_interface_runtime_modify>` to support parameters within the request body.
 * admin: the :ref:`/listener endpoint <operations_admin_interface_listeners>` now returns :ref:`listeners.proto<envoy_api_msg_admin.v2alpha.Listeners>` which includes listener names and ports.
@@ -59,6 +60,8 @@ Version history
 * router: added :ref:`RouteAction's auto_host_rewrite_header <envoy_api_field_route.RouteAction.auto_host_rewrite_header>` to allow upstream host header substitution with some other header's value
 * router: added support for UPSTREAM_REMOTE_ADDRESS :ref:`header formatter
   <config_http_conn_man_headers_custom_request_headers>`.
+* router: add ability to reject a request that includes invalid values for
+  headers configured in :ref:`strict_check_headers <envoy_api_field_config.filter.http.router.v2.Router.strict_check_headers>`
 * runtime: added support for :ref:`flexible layering configuration
   <envoy_api_field_config.bootstrap.v2.Bootstrap.layered_runtime>`.
 * runtime: added support for statically :ref:`specifying the runtime in the bootstrap configuration

include/envoy/router/router.h

@@ -163,6 +163,7 @@ class RetryPolicy {
   static const uint32_t RETRY_ON_GRPC_UNAVAILABLE        = 0x100;
   static const uint32_t RETRY_ON_GRPC_INTERNAL           = 0x200;
   static const uint32_t RETRY_ON_RETRIABLE_STATUS_CODES  = 0x400;
+  static const uint32_t RETRY_UNKNOWN_FIELD_ERROR        = 0x800;
   // clang-format on
 
   virtual ~RetryPolicy() = default;

include/envoy/stream_info/stream_info.h

@@ -61,8 +61,10 @@ enum ResponseFlag {
   UpstreamRetryLimitExceeded = 0x8000,
   // Request hit the stream idle timeout, triggering a 408.
   StreamIdleTimeout = 0x10000,
+  // Request specified x-envoy-* header values that failed strict header checks.
+  InvalidEnvoyRequestHeaders = 0x20000,
   // ATTENTION: MAKE SURE THIS REMAINS EQUAL TO THE LAST FLAG.
-  LastFlag = StreamIdleTimeout
+  LastFlag = InvalidEnvoyRequestHeaders
 };
 
 /**
@@ -95,6 +97,8 @@ struct ResponseCodeDetailValues {
   const std::string MissingHost = "missing_host_header";
   // The request was rejected due to the request headers being larger than the configured limit.
   const std::string RequestHeadersTooLarge = "request_headers_too_large";
+  // The request was rejected due to x-envoy-* headers failing strict header validation.
+  const std::string InvalidEnvoyRequestHeaders = "request_headers_failed_strict_check";
   // The request was rejected due to the Path or :path header field missing.
   const std::string MissingPath = "missing_path_rejected";
   // The request was rejected due to using an absolute path on a route not supporting them.

source/common/http/async_client_impl.cc

@@ -36,9 +36,9 @@ AsyncClientImpl::AsyncClientImpl(Upstream::ClusterInfoConstSharedPtr cluster,
                                  Runtime::RandomGenerator& random,
                                  Router::ShadowWriterPtr&& shadow_writer,
                                  Http::Context& http_context)
-    : cluster_(cluster),
-      config_("http.async-client.", local_info, stats_store, cm, runtime, random,
-              std::move(shadow_writer), true, false, false, dispatcher.timeSource(), http_context),
+    : cluster_(cluster), config_("http.async-client.", local_info, stats_store, cm, runtime, random,
+                                 std::move(shadow_writer), true, false, false, {},
+                                 dispatcher.timeSource(), http_context),
       dispatcher_(dispatcher) {}
 
 AsyncClientImpl::~AsyncClientImpl() {

source/common/router/config_impl.cc

@@ -72,6 +72,12 @@ RetryPolicyImpl::RetryPolicyImpl(const envoy::api::v2::route::RetryPolicy& retry
   num_retries_ = PROTOBUF_GET_WRAPPED_OR_DEFAULT(retry_policy, num_retries, 1);
   retry_on_ = RetryStateImpl::parseRetryOn(retry_policy.retry_on());
   retry_on_ |= RetryStateImpl::parseRetryGrpcOn(retry_policy.retry_on());
+  // NB: parseRetryOn and parseRetryGrpcOn will return a value with
+  // RetryPolicy::RETRY_UNKNOWN_FIELD_ERROR set unless the input only contains
+  // supported values for `x-envoy-retry-on` and `x-envoy-retry-grpc-on`,
+  // respectively. Clear this flag here, since the `retry_on` config option is
+  // intended to contain retry policies from both.
+  retry_on_ &= ~RetryPolicy::RETRY_UNKNOWN_FIELD_ERROR;
 
   for (const auto& host_predicate : retry_policy.retry_host_predicate()) {
     auto& factory = Envoy::Config::Utility::getAndCheckFactory<Upstream::RetryHostPredicateFactory>(

source/common/router/retry_state_impl.cc

@@ -128,6 +128,8 @@ uint32_t RetryStateImpl::parseRetryOn(absl::string_view config) {
       ret |= RetryPolicy::RETRY_ON_REFUSED_STREAM;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.RetriableStatusCodes) {
       ret |= RetryPolicy::RETRY_ON_RETRIABLE_STATUS_CODES;
+    } else {
+      ret |= RetryPolicy::RETRY_UNKNOWN_FIELD_ERROR;
     }
   }
 
@@ -147,6 +149,8 @@ uint32_t RetryStateImpl::parseRetryGrpcOn(absl::string_view retry_grpc_on_header
       ret |= RetryPolicy::RETRY_ON_GRPC_UNAVAILABLE;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnGrpcValues.Internal) {
       ret |= RetryPolicy::RETRY_ON_GRPC_INTERNAL;
+    } else {
+      ret |= RetryPolicy::RETRY_UNKNOWN_FIELD_ERROR;
     }
   }
 

source/common/router/retry_state_impl.h

@@ -81,6 +81,9 @@ class RetryStateImpl : public RetryState {
   bool wouldRetryFromReset(const Http::StreamResetReason reset_reason);
   RetryStatus shouldRetry(bool would_retry, DoRetryCallback callback);
 
+  static uint32_t parseRetryOn_(absl::string_view config, bool flag_parse_failures);
+  static uint32_t parseRetryGrpcOn_(absl::string_view config, bool flag_parse_failures);
+
   const Upstream::ClusterInfo& cluster_;
   Runtime::Loader& runtime_;
   Runtime::RandomGenerator& random_;

source/common/router/router.cc

@@ -222,6 +222,29 @@ Filter::~Filter() {
   ASSERT(!retry_state_);
 }
 
+const FilterUtility::StrictHeaderChecker::HeaderCheckResult
+FilterUtility::StrictHeaderChecker::test(Http::HeaderMap& headers,
+                                         const Http::LowerCaseString& target_header) {
+  HeaderCheckResult r;
+  if (target_header == Http::Headers::get().EnvoyUpstreamRequestTimeoutMs) {
+    return isInteger(headers.EnvoyUpstreamRequestTimeoutMs());
+  } else if (target_header == Http::Headers::get().EnvoyUpstreamRequestPerTryTimeoutMs) {
+    return isInteger(headers.EnvoyUpstreamRequestPerTryTimeoutMs());
+  } else if (target_header == Http::Headers::get().EnvoyMaxRetries) {
+    return isInteger(headers.EnvoyMaxRetries());
+  } else if (target_header == Http::Headers::get().EnvoyRetryOn) {
+    return hasValidRetryFields(headers.EnvoyRetryOn(), &Router::RetryStateImpl::parseRetryOn);
+  } else if (target_header == Http::Headers::get().EnvoyRetryGrpcOn) {
+    return hasValidRetryFields(headers.EnvoyRetryGrpcOn(),
+                               &Router::RetryStateImpl::parseRetryGrpcOn);
+  } else {
+    // Should only validate headers for which we have implemented a validator
+    ASSERT(false);
+  }
+
+  return r;
+}
+
 Stats::StatName Filter::upstreamZone(Upstream::HostDescriptionConstSharedPtr upstream_host) {
   return upstream_host ? upstream_host->localityZoneStatName() : config_.empty_stat_name_;
 }
@@ -381,6 +404,24 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool e
   ENVOY_STREAM_LOG(debug, "cluster '{}' match for URL '{}'", *callbacks_,
                    route_entry_->clusterName(), headers.Path()->value().getStringView());
 
+  if (config_.strict_check_headers_ != nullptr && !config_.strict_check_headers_->empty()) {
+    for (const auto& header : *config_.strict_check_headers_) {
+      const auto res = FilterUtility::StrictHeaderChecker::test(headers, header);
+      if (!res.valid_) {
+        callbacks_->streamInfo().setResponseFlag(
+            StreamInfo::ResponseFlag::InvalidEnvoyRequestHeaders);
+        const std::string body = fmt::format("invalid header '{}' with value '{}'",
+                                             std::string(res.entry_->key().getStringView()),
+                                             std::string(res.entry_->value().getStringView()));
+        const std::string details =
+            absl::StrCat(StreamInfo::ResponseCodeDetails::get().InvalidEnvoyRequestHeaders, "{",
+                         res.entry_->key().getStringView(), "}");
+        callbacks_->sendLocalReply(Http::Code::BadRequest, body, nullptr, absl::nullopt, details);
+        return Http::FilterHeadersStatus::StopIteration;
+      }
+    }
+  }
+
   const Http::HeaderEntry* request_alt_name = headers.EnvoyUpstreamAltStatName();
   if (request_alt_name) {
     // TODO(#7003): converting this header value into a StatName requires

source/common/router/router.h

@@ -67,6 +67,41 @@ class FilterUtility {
     bool hedge_on_per_try_timeout_;
   };
 
+  class StrictHeaderChecker {
+  public:
+    struct HeaderCheckResult {
+      bool valid_ = true;
+      const Http::HeaderEntry* entry_;
+    };
+
+    static const HeaderCheckResult test(Http::HeaderMap& headers,
+                                        const Http::LowerCaseString& target_header);
+
+    using ParseRetryFlagsFunc = std::function<uint32_t(absl::string_view)>;
+
+  private:
+    static HeaderCheckResult hasValidRetryFields(Http::HeaderEntry* header_entry,
+                                                 const ParseRetryFlagsFunc& parseFn) {
+      HeaderCheckResult r;
+      if (header_entry) {
+        const auto flags = parseFn(header_entry->value().getStringView());
+        r.valid_ = (flags & RetryPolicy::RETRY_UNKNOWN_FIELD_ERROR) == 0;
+        r.entry_ = header_entry;
+      }
+      return r;
+    }
+
+    static HeaderCheckResult isInteger(Http::HeaderEntry* header_entry) {
+      HeaderCheckResult r;
+      if (header_entry) {
+        uint64_t out;
+        r.valid_ = absl::SimpleAtoi(header_entry->value().getStringView(), &out);
+        r.entry_ = header_entry;
+      }
+      return r;
+    }
+  };
+
   /**
    * Set the :scheme header based on the properties of the upstream cluster.
    */
@@ -115,6 +150,7 @@ class FilterConfig {
                Stats::Scope& scope, Upstream::ClusterManager& cm, Runtime::Loader& runtime,
                Runtime::RandomGenerator& random, ShadowWriterPtr&& shadow_writer,
                bool emit_dynamic_stats, bool start_child_span, bool suppress_envoy_headers,
+               const Protobuf::RepeatedPtrField<std::string>& strict_check_headers,
                TimeSource& time_source, Http::Context& http_context)
       : scope_(scope), local_info_(local_info), cm_(cm), runtime_(runtime),
         random_(random), stats_{ALL_ROUTER_STATS(POOL_COUNTER_PREFIX(scope, stat_prefix))},
@@ -123,7 +159,14 @@ class FilterConfig {
         stat_name_pool_(scope_.symbolTable()), retry_(stat_name_pool_.add("retry")),
         zone_name_(stat_name_pool_.add(local_info_.zoneName())),
         empty_stat_name_(stat_name_pool_.add("")), shadow_writer_(std::move(shadow_writer)),
-        time_source_(time_source) {}
+        time_source_(time_source) {
+    if (!strict_check_headers.empty()) {
+      strict_check_headers_ = std::make_unique<HeaderVector>();
+      for (const auto& header : strict_check_headers) {
+        strict_check_headers_->emplace_back(Http::LowerCaseString(header));
+      }
+    }
+  }
 
   FilterConfig(const std::string& stat_prefix, Server::Configuration::FactoryContext& context,
                ShadowWriterPtr&& shadow_writer,
@@ -132,11 +175,14 @@ class FilterConfig {
                      context.runtime(), context.random(), std::move(shadow_writer),
                      PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, dynamic_stats, true),
                      config.start_child_span(), config.suppress_envoy_headers(),
-                     context.api().timeSource(), context.httpContext()) {
+                     config.strict_check_headers(), context.api().timeSource(),
+                     context.httpContext()) {
     for (const auto& upstream_log : config.upstream_log()) {
       upstream_logs_.push_back(AccessLog::AccessLogFactory::fromProto(upstream_log, context));
     }
   }
+  using HeaderVector = std::vector<Http::LowerCaseString>;
+  using StrictCheckHeadersPtr = std::unique_ptr<HeaderVector>;
 
   ShadowWriter& shadowWriter() { return *shadow_writer_; }
   TimeSource& timeSource() { return time_source_; }
@@ -150,6 +196,8 @@ class FilterConfig {
   const bool emit_dynamic_stats_;
   const bool start_child_span_;
   const bool suppress_envoy_headers_;
+  // TODO(xyu-stripe): Make this a bitset to keep cluster memory footprint down
+  StrictCheckHeadersPtr strict_check_headers_;
   std::list<AccessLog::InstanceSharedPtr> upstream_logs_;
   Http::Context& http_context_;
   Stats::StatNamePool stat_name_pool_;

source/common/stream_info/utility.cc

@@ -23,6 +23,7 @@ const std::string ResponseFlagUtils::RATE_LIMITED = "RL";
 const std::string ResponseFlagUtils::UNAUTHORIZED_EXTERNAL_SERVICE = "UAEX";
 const std::string ResponseFlagUtils::RATELIMIT_SERVICE_ERROR = "RLSE";
 const std::string ResponseFlagUtils::STREAM_IDLE_TIMEOUT = "SI";
+const std::string ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS = "IH";
 
 void ResponseFlagUtils::appendString(std::string& result, const std::string& append) {
   if (result.empty()) {
@@ -35,7 +36,7 @@ void ResponseFlagUtils::appendString(std::string& result, const std::string& app
 const std::string ResponseFlagUtils::toShortString(const StreamInfo& stream_info) {
   std::string result;
 
-  static_assert(ResponseFlag::LastFlag == 0x10000, "A flag has been added. Fix this code.");
+  static_assert(ResponseFlag::LastFlag == 0x20000, "A flag has been added. Fix this code.");
 
   if (stream_info.hasResponseFlag(ResponseFlag::FailedLocalHealthCheck)) {
     appendString(result, FAILED_LOCAL_HEALTH_CHECK);
@@ -105,6 +106,10 @@ const std::string ResponseFlagUtils::toShortString(const StreamInfo& stream_info
     appendString(result, STREAM_IDLE_TIMEOUT);
   }
 
+  if (stream_info.hasResponseFlag(ResponseFlag::InvalidEnvoyRequestHeaders)) {
+    appendString(result, INVALID_ENVOY_REQUEST_HEADERS);
+  }
+
   return result.empty() ? NONE : result;
 }
 
@@ -129,6 +134,7 @@ absl::optional<ResponseFlag> ResponseFlagUtils::toResponseFlag(const std::string
        ResponseFlag::DownstreamConnectionTermination},
       {ResponseFlagUtils::UPSTREAM_RETRY_LIMIT_EXCEEDED, ResponseFlag::UpstreamRetryLimitExceeded},
       {ResponseFlagUtils::STREAM_IDLE_TIMEOUT, ResponseFlag::StreamIdleTimeout},
+      {ResponseFlagUtils::INVALID_ENVOY_REQUEST_HEADERS, ResponseFlag::InvalidEnvoyRequestHeaders},
   };
   const auto& it = map.find(flag);
   if (it != map.end()) {

source/common/stream_info/utility.h

@@ -38,6 +38,7 @@ class ResponseFlagUtils {
   const static std::string UNAUTHORIZED_EXTERNAL_SERVICE;
   const static std::string RATELIMIT_SERVICE_ERROR;
   const static std::string STREAM_IDLE_TIMEOUT;
+  const static std::string INVALID_ENVOY_REQUEST_HEADERS;
 };
 
 /**

source/extensions/access_loggers/http_grpc/grpc_access_log_impl.cc

@@ -108,7 +108,7 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags(
     envoy::data::accesslog::v2::AccessLogCommon& common_access_log,
     const StreamInfo::StreamInfo& stream_info) {
 
-  static_assert(StreamInfo::ResponseFlag::LastFlag == 0x10000,
+  static_assert(StreamInfo::ResponseFlag::LastFlag == 0x20000,
                 "A flag has been added. Fix this code.");
 
   if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::FailedLocalHealthCheck)) {
@@ -165,6 +165,12 @@ void HttpGrpcAccessLog::responseFlagsToAccessLogResponseFlags(
             ResponseFlags_Unauthorized_Reason_EXTERNAL_SERVICE);
   }
 
+  if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::InvalidEnvoyRequestHeaders)) {
+    common_access_log.mutable_response_flags()->mutable_unauthorized_details()->set_reason(
+        envoy::data::accesslog::v2::ResponseFlags_Unauthorized_Reason::
+            ResponseFlags_Unauthorized_Reason_STRICT_HEADER_CHECK_FAILED);
+  }
+
   if (stream_info.hasResponseFlag(StreamInfo::ResponseFlag::RateLimitServiceError)) {
     common_access_log.mutable_response_flags()->set_rate_limit_service_error(true);
   }