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/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

@@ -210,6 +210,10 @@ message ResponseFlags {
 
   // Indicates that the stream idle timeout was hit, resulting in a downstream 408.
   bool stream_idle_timeout = 17;
+
+  // Indicates that the request was rejected because an envoy request header failed strict
+  // validation.
+  bool invalid_envoy_request_headers = 18;
 }
 
 // Properties of a negotiated TLS connection.

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/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

@@ -70,8 +70,8 @@ RetryPolicyImpl::RetryPolicyImpl(const envoy::api::v2::route::RetryPolicy& retry
   per_try_timeout_ =
       std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(retry_policy, per_try_timeout, 0));
   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());
+  retry_on_ = RetryStateImpl::parseRetryOn(retry_policy.retry_on()).first;
+  retry_on_ |= RetryStateImpl::parseRetryGrpcOn(retry_policy.retry_on()).first;
 
   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

@@ -79,10 +79,11 @@ RetryStateImpl::RetryStateImpl(const RetryPolicy& route_policy, Http::HeaderMap&
 
   // Merge in the headers.
   if (request_headers.EnvoyRetryOn()) {
-    retry_on_ |= parseRetryOn(request_headers.EnvoyRetryOn()->value().getStringView());
+    retry_on_ |= parseRetryOn(request_headers.EnvoyRetryOn()->value().getStringView()).first;
   }
   if (request_headers.EnvoyRetryGrpcOn()) {
-    retry_on_ |= parseRetryGrpcOn(request_headers.EnvoyRetryGrpcOn()->value().getStringView());
+    retry_on_ |=
+        parseRetryGrpcOn(request_headers.EnvoyRetryGrpcOn()->value().getStringView()).first;
   }
   if (retry_on_ != 0 && request_headers.EnvoyMaxRetries()) {
     uint64_t temp;
@@ -113,8 +114,9 @@ void RetryStateImpl::enableBackoffTimer() {
   retry_timer_->enableTimer(std::chrono::milliseconds(backoff_strategy_->nextBackOffMs()));
 }
 
-uint32_t RetryStateImpl::parseRetryOn(absl::string_view config) {
+std::pair<uint32_t, bool> RetryStateImpl::parseRetryOn(absl::string_view config) {
   uint32_t ret = 0;
+  bool all_fields_valid = true;
   for (const auto retry_on : StringUtil::splitToken(config, ",")) {
     if (retry_on == Http::Headers::get().EnvoyRetryOnValues._5xx) {
       ret |= RetryPolicy::RETRY_ON_5XX;
@@ -128,14 +130,17 @@ 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 {
+      all_fields_valid = false;
     }
   }
 
-  return ret;
+  return {ret, all_fields_valid};
 }
 
-uint32_t RetryStateImpl::parseRetryGrpcOn(absl::string_view retry_grpc_on_header) {
+std::pair<uint32_t, bool> RetryStateImpl::parseRetryGrpcOn(absl::string_view retry_grpc_on_header) {
   uint32_t ret = 0;
+  bool all_fields_valid = true;
   for (const auto retry_on : StringUtil::splitToken(retry_grpc_on_header, ",")) {
     if (retry_on == Http::Headers::get().EnvoyRetryOnGrpcValues.Cancelled) {
       ret |= RetryPolicy::RETRY_ON_GRPC_CANCELLED;
@@ -147,10 +152,12 @@ 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 {
+      all_fields_valid = false;
     }
   }
 
-  return ret;
+  return {ret, all_fields_valid};
 }
 
 void RetryStateImpl::resetRetry() {

source/common/router/retry_state_impl.h

@@ -29,10 +29,23 @@ class RetryStateImpl : public RetryState {
                               Upstream::ResourcePriority priority);
   ~RetryStateImpl();
 
-  static uint32_t parseRetryOn(absl::string_view config);
-
-  // Returns the RetryPolicy extracted from the x-envoy-retry-grpc-on header.
-  static uint32_t parseRetryGrpcOn(absl::string_view retry_grpc_on_header);
+  /**
+   * Returns the RetryPolicy extracted from the x-envoy-retry-on header.
+   * @param config is the value of the header.
+   * @return std::pair<uint32_t, bool> the uint32_t is a bitset representing the
+   *         valid retry policies in @param config. The bool is TRUE iff all the
+   *         policies specified in @param config are valid.
+   */
+  static std::pair<uint32_t, bool> parseRetryOn(absl::string_view config);
+
+  /**
+   * Returns the RetryPolicy extracted from the x-envoy-retry-grpc-on header.
+   * @param config is the value of the header.
+   * @return std::pair<uint32_t, bool> the uint32_t is a bitset representing the
+   *         valid retry policies in @param config. The bool is TRUE iff all the
+   *         policies specified in @param config are valid.
+   */
+  static std::pair<uint32_t, bool> parseRetryGrpcOn(absl::string_view retry_grpc_on_header);
 
   // Router::RetryState
   bool enabled() override { return retry_on_ != 0; }

source/common/router/router.cc

@@ -222,6 +222,25 @@ Filter::~Filter() {
   ASSERT(!retry_state_);
 }
 
+const FilterUtility::StrictHeaderChecker::HeaderCheckResult
+FilterUtility::StrictHeaderChecker::checkHeader(Http::HeaderMap& headers,
+                                                const Http::LowerCaseString& target_header) {
+  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);
+  }
+  // Should only validate headers for which we have implemented a validator.
+  NOT_REACHED_GCOVR_EXCL_LINE
+}
+
 Stats::StatName Filter::upstreamZone(Upstream::HostDescriptionConstSharedPtr upstream_host) {
   return upstream_host ? upstream_host->localityZoneStatName() : config_.empty_stat_name_;
 }
@@ -381,6 +400,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) {
+    for (const auto& header : *config_.strict_check_headers_) {
+      const auto res = FilterUtility::StrictHeaderChecker::checkHeader(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,51 @@ class FilterUtility {
     bool hedge_on_per_try_timeout_;
   };
 
+  class StrictHeaderChecker {
+  public:
+    struct HeaderCheckResult {
+      bool valid_ = true;
+      const Http::HeaderEntry* entry_;
+    };
+
+    /**
+     * Determine whether a given header's value passes the strict validation
+     * defined for that header.
+     * @param headers supplies the headers from which to get the target header.
+     * @param target_header is the header to be validated.
+     * @return HeaderCheckResult containing the entry for @param target_header
+     *         and valid_ set to FALSE if @param target_header is set to an
+     *         invalid value. If @param target_header doesn't appear in
+     *         @param headers, return a result with valid_ set to TRUE.
+     */
+    static const HeaderCheckResult checkHeader(Http::HeaderMap& headers,
+                                               const Http::LowerCaseString& target_header);
+
+    using ParseRetryFlagsFunc = std::function<std::pair<uint32_t, bool>(absl::string_view)>;
+
+  private:
+    static HeaderCheckResult hasValidRetryFields(Http::HeaderEntry* header_entry,
+                                                 const ParseRetryFlagsFunc& parseFn) {
+      HeaderCheckResult r;
+      if (header_entry) {
+        const auto flags_and_validity = parseFn(header_entry->value().getStringView());
+        r.valid_ = flags_and_validity.second;
+        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 +160,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 +169,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 +185,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 HeaderVectorPtr = std::unique_ptr<HeaderVector>;
 
   ShadowWriter& shadowWriter() { return *shadow_writer_; }
   TimeSource& timeSource() { return time_source_; }
@@ -150,6 +206,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.
+  HeaderVectorPtr strict_check_headers_;
   std::list<AccessLog::InstanceSharedPtr> upstream_logs_;
   Http::Context& http_context_;
   Stats::StatNamePool stat_name_pool_;