From 34e9a7eed133039008ea5f702a853a9a0279a3cd Mon Sep 17 00:00:00 2001 From: "data-plane-api(CircleCI)" Date: Thu, 16 Jul 2020 21:22:54 +0000 Subject: [PATCH] http: stream error on invalid messaging (#11748) This unifies HTTP/1.1 and HTTP/2 stream error on invalid messaging. Previously HTTP/1.1 defaulted permissive and HTTP/2 defaulted to strict. This defaults both to strict, resetting connections on invalid requests. This will have a major latency impact if downstream is sending a mix of valid and invalid requests over HTTP/1.1 Additional Description: This change is runtime guarded per default behavioral change rules. It can also be reverted by setting the default to permissive (for prior HTTP/1 behvior) then overriding HTTP/2 to struct (for prior HTTP/2 behavior). This works in conjunction with #11714, as the HTTP connection manager enforces the strictness, so the responses need to be sent via the HTTP connection manager to have strictness applied correctly. Risk Level: High (HCM changes) Testing: new unit tests, updated integration tests Docs Changes: n/a Release Notes: inline Runtime guard: envoy.reloadable_features.hcm_stream_error_on_invalid_message Fixes #9846 Signed-off-by: Alyssa Wilk Mirrored from https://github.com/envoyproxy/envoy @ 88dcb292817946510bb87f8f379a954962cece79 --- envoy/config/core/v3/protocol.proto | 21 +++++++++++++++++-- envoy/config/core/v4alpha/protocol.proto | 11 ++++++++-- .../v3/http_connection_manager.proto | 19 ++++++++++++++++- .../v4alpha/http_connection_manager.proto | 19 ++++++++++++++++- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/envoy/config/core/v3/protocol.proto b/envoy/config/core/v3/protocol.proto index 339601fe..0ab6289e 100644 --- a/envoy/config/core/v3/protocol.proto +++ b/envoy/config/core/v3/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.Http2ProtocolOptions"; @@ -280,8 +280,25 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This is overridden by HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // iff present. + // + // This is deprecated in favor of :ref:`override_stream_error_on_invalid_http_message + // ` + // + // See `RFC7540, sec. 8.1 `_ for details. + bool stream_error_on_invalid_http_messaging = 12 [deprecated = true]; + + // Allows invalid HTTP messaging and headers. When this option is disabled (default), then + // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, + // when this option is enabled, only the offending stream is terminated. + // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/envoy/config/core/v4alpha/protocol.proto b/envoy/config/core/v4alpha/protocol.proto index 2ec92441..955c2933 100644 --- a/envoy/config/core/v4alpha/protocol.proto +++ b/envoy/config/core/v4alpha/protocol.proto @@ -159,7 +159,7 @@ message Http1ProtocolOptions { bool enable_trailers = 5; } -// [#next-free-field: 14] +// [#next-free-field: 15] message Http2ProtocolOptions { option (udpa.annotations.versioning).previous_message_type = "envoy.config.core.v3.Http2ProtocolOptions"; @@ -180,6 +180,10 @@ message Http2ProtocolOptions { google.protobuf.UInt32Value value = 2 [(validate.rules).message = {required: true}]; } + reserved 12; + + reserved "stream_error_on_invalid_http_messaging"; + // `Maximum table size `_ // (in octets) that the encoder is permitted to use for the dynamic HPACK table. Valid values // range from 0 to 4294967295 (2^32 - 1) and defaults to 4096. 0 effectively disables header @@ -280,8 +284,11 @@ message Http2ProtocolOptions { // the whole HTTP/2 connection is terminated upon receiving invalid HEADERS frame. However, // when this option is enabled, only the offending stream is terminated. // + // This overrides any HCM :ref:`stream_error_on_invalid_http_messaging + // ` + // // See `RFC7540, sec. 8.1 `_ for details. - bool stream_error_on_invalid_http_messaging = 12; + google.protobuf.BoolValue override_stream_error_on_invalid_http_message = 14; // [#not-implemented-hide:] // Specifies SETTINGS frame parameters to be sent to the peer, with two exceptions: diff --git a/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto b/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto index f2a80959..a23fcc99 100644 --- a/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto +++ b/envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto @@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.config.filter.network.http_connection_manager.v2.HttpConnectionManager"; @@ -540,6 +540,23 @@ message HttpConnectionManager { // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // company-internal mesh) and false when receiving untrusted traffic (edge deployments). + // + // If different behaviors for invalid_http_message for HTTP/1 and HTTP/2 are + // desired, one *must* use the new HTTP/2 option + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; } // The configuration to customize local reply returned by Envoy. diff --git a/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto b/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto index aaf146e1..bdf3618b 100644 --- a/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto +++ b/envoy/extensions/filters/network/http_connection_manager/v4alpha/http_connection_manager.proto @@ -35,7 +35,7 @@ option (udpa.annotations.file_status).package_version_status = NEXT_MAJOR_VERSIO // HTTP connection manager :ref:`configuration overview `. // [#extension: envoy.filters.network.http_connection_manager] -// [#next-free-field: 40] +// [#next-free-field: 41] message HttpConnectionManager { option (udpa.annotations.versioning).previous_message_type = "envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; @@ -539,6 +539,23 @@ message HttpConnectionManager { // route with :ref:`domains` match set to `example`. Defaults to `false`. Note that port removal is not part // of `HTTP spec `_ and is provided for convenience. bool strip_matching_host_port = 39; + + // Governs Envoy's behavior when receiving invalid HTTP from downstream. + // If this option is false (default), Envoy will err on the conservative side handling HTTP + // errors, terminating both HTTP/1.1 and HTTP/2 connections when receiving an invalid request. + // If this option is set to true, Envoy will be more permissive, only resetting the invalid + // stream in the case of HTTP/2 and leaving the connection open where possible (if the entire + // request is read for HTTP/1.1) + // In general this should be true for deployments receiving trusted traffic (L2 Envoys, + // company-internal mesh) and false when receiving untrusted traffic (edge deployments). + // + // If different behaviors for invalid_http_message for HTTP/1 and HTTP/2 are + // desired, one *must* use the new HTTP/2 option + // :ref:`override_stream_error_on_invalid_http_message + // ` + // *not* the deprecated but similarly named :ref:`stream_error_on_invalid_http_messaging + // ` + google.protobuf.BoolValue stream_error_on_invalid_http_message = 40; } // The configuration to customize local reply returned by Envoy.