From 2bcd30b5f4c224a98d1394f4266335ac640f178d Mon Sep 17 00:00:00 2001
From: "data-plane-api(CircleCI)" <data-plane-api@users.noreply.github.com>
Date: Tue, 2 Jul 2019 16:35:14 +0000
Subject: [PATCH] router: add config to reject requests with invalid envoy
 headers (#7323)

Description:
Before this change, Envoy would silently ignore the `x-envoy-*` header if a
client specifies an invalid value for this header (e.g. `x-envoy-max-retries: 3.0`).
Introduce a `strict_check_headers` config option for `envoy.router` that adds
optional support to reject requests with invalid values for the following headers:
- x-envoy-upstream-rq-timeout-ms
- x-envoy-upstream-rq-per-try-timeout-ms
- x-envoy-max-retries
- x-envoy-retry-on
- x-envoy-retry-grpc-on

On rejection, Envoy responds with HTTP status 400 and sets a new response flag
`IH` to indicate the reason was due to an invalid header.

Risk Level: Low/medium

Testing: unit tests
- unit test: `FilterUtility::StrictHeaderChecker`
- test that router rejects request with HTTP status 400 + setting the `IH` response flag
- test that config validation rejects unsupported values
- manual end-to-end test `client -> envoy -> upstream server` to verify that
  Envoy returns a 400 and sets the response flag in the logs

Docs Changes:
- add inline docs to `router.proto` for `strict_check_headers`
- add inline docs to `accesslog.proto` for `IH` response flag

Release Notes: updated for router and accesslog
Fixes #6482

Signed-off-by: Xiao Yu <xyu@stripe.com>

Mirrored from https://github.com/envoyproxy/envoy @ ecd03a4eed07e1cfea9e9844e519b7fffada437a
---
 .../filter/accesslog/v2/accesslog.proto       |  3 +-
 .../config/filter/http/router/v2/router.proto | 28 +++++++++++++++++++
 envoy/data/accesslog/v2/accesslog.proto       |  4 +++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/envoy/config/filter/accesslog/v2/accesslog.proto b/envoy/config/filter/accesslog/v2/accesslog.proto
index fffd2251e..4eec60d74 100644
--- a/envoy/config/filter/accesslog/v2/accesslog.proto
+++ b/envoy/config/filter/accesslog/v2/accesslog.proto
@@ -191,7 +191,8 @@ message ResponseFlagFilter {
       "RLSE",
       "DC",
       "URX",
-      "SI"
+      "SI",
+      "IH"
     ]
   }];
 }
diff --git a/envoy/config/filter/http/router/v2/router.proto b/envoy/config/filter/http/router/v2/router.proto
index 02115cb81..e77675673 100644
--- a/envoy/config/filter/http/router/v2/router.proto
+++ b/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"
+    ]
+  }];
 }
diff --git a/envoy/data/accesslog/v2/accesslog.proto b/envoy/data/accesslog/v2/accesslog.proto
index 478d7b03b..1396c729f 100644
--- a/envoy/data/accesslog/v2/accesslog.proto
+++ b/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.