Skip to content

Commit

Permalink
router: add config to reject requests with invalid envoy headers (#7323)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
data-plane-api(CircleCI) committed Jul 2, 2019
1 parent 60e2e69 commit 2bcd30b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 1 deletion.
3 changes: 2 additions & 1 deletion envoy/config/filter/accesslog/v2/accesslog.proto
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ message ResponseFlagFilter {
"RLSE",
"DC",
"URX",
"SI"
"SI",
"IH"
]
}];
}
Expand Down
28 changes: 28 additions & 0 deletions envoy/config/filter/http/router/v2/router.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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>`.

Expand All @@ -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"
]
}];
}
4 changes: 4 additions & 0 deletions envoy/data/accesslog/v2/accesslog.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 2bcd30b

Please sign in to comment.