diff --git a/.bazelrc b/.bazelrc index 7533ffc8a62f..a14ceec398de 100644 --- a/.bazelrc +++ b/.bazelrc @@ -492,16 +492,18 @@ build:rbe-engflow --remote_timeout=3600s build:rbe-engflow --bes_timeout=3600s build:rbe-engflow --bes_upload_mode=fully_async -build:rbe-envoy-engflow --google_default_credentials=false -build:rbe-envoy-engflow --remote_cache=grpcs://morganite.cluster.engflow.com +build:cache-envoy-engflow --google_default_credentials=false +build:cache-envoy-engflow --remote_cache=grpcs://morganite.cluster.engflow.com +build:cache-envoy-engflow --remote_timeout=3600s +build:cache-envoy-engflow --credential_helper=*.engflow.com=%workspace%/bazel/engflow-bazel-credential-helper.sh +build:cache-envoy-engflow --grpc_keepalive_time=30s +build:bes-envoy-engflow --bes_backend=grpcs://morganite.cluster.engflow.com/ +build:bes-envoy-engflow --bes_results_url=https://morganite.cluster.engflow.com/invocation/ +build:bes-envoy-engflow --bes_timeout=3600s +build:bes-envoy-engflow --bes_upload_mode=fully_async +build:rbe-envoy-engflow --config=cache-envoy-engflow +build:rbe-envoy-engflow --config=bes-envoy-engflow build:rbe-envoy-engflow --remote_executor=grpcs://morganite.cluster.engflow.com -build:rbe-envoy-engflow --bes_backend=grpcs://morganite.cluster.engflow.com/ -build:rbe-envoy-engflow --bes_results_url=https://morganite.cluster.engflow.com/invocation/ -build:rbe-envoy-engflow --credential_helper=*.engflow.com=%workspace%/bazel/engflow-bazel-credential-helper.sh -build:rbe-envoy-engflow --grpc_keepalive_time=30s -build:rbe-envoy-engflow --remote_timeout=3600s -build:rbe-envoy-engflow --bes_timeout=3600s -build:rbe-envoy-engflow --bes_upload_mode=fully_async build:rbe-envoy-engflow --remote_default_exec_properties=container-image=docker://docker.io/envoyproxy/envoy-build-ubuntu:fdd65c6270a8507a18d5acd6cf19a18cb695e4fa@sha256:3c8a3ce6f90dcfb5d09dc8f79bb01404d3526d420061f9a176e0a8e91e1e573e ############################################################################# diff --git a/.github/config.yml b/.github/config.yml index 50a28027053d..a9dde4d03896 100644 --- a/.github/config.yml +++ b/.github/config.yml @@ -80,27 +80,8 @@ checks: - publish - verify required: true - windows: - name: Envoy/Windows - required: true - on-run: - - build-windows run: - build-windows: - paths: - - .bazelrc - - .bazelversion - - .github/config.yml - - api/**/* - - bazel/**/* - - ci/**/* - - configs/**/* - - contrib/**/* - - envoy/**/* - - source/**/* - - test/**/* - - VERSION.txt build-macos: paths: - .bazelrc diff --git a/VERSION.txt b/VERSION.txt index 3bae5204be9e..d6201580edb4 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -1 +1 @@ -1.27.3 +1.27.4 diff --git a/bazel/foreign_cc/nghttp2.patch b/bazel/foreign_cc/nghttp2.patch index d1cbab6356e5..511e2a2e4b29 100644 --- a/bazel/foreign_cc/nghttp2.patch +++ b/bazel/foreign_cc/nghttp2.patch @@ -14,3 +14,171 @@ diff -u -r a/CMakeLists.txt b/CMakeLists.txt endif() # AC_TYPE_UINT8_T # AC_TYPE_UINT16_T +diff --git a/doc/Makefile.am b/doc/Makefile.am +index 7d7f31c6..ce50d89e 100644 +--- a/doc/Makefile.am ++++ b/doc/Makefile.am +@@ -74,6 +74,7 @@ APIDOCS= \ + nghttp2_option_set_peer_max_concurrent_streams.rst \ + nghttp2_option_set_server_fallback_rfc7540_priorities.rst \ + nghttp2_option_set_user_recv_extension_type.rst \ ++ nghttp2_option_set_max_continuations.rst \ + nghttp2_option_set_max_outbound_ack.rst \ + nghttp2_option_set_max_settings.rst \ + nghttp2_option_set_stream_reset_rate_limit.rst \ +diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h +index 7910db23..a54efbfd 100644 +--- a/lib/includes/nghttp2/nghttp2.h ++++ b/lib/includes/nghttp2/nghttp2.h +@@ -440,7 +440,12 @@ typedef enum { + * exhaustion on server side to send these frames forever and does + * not read network. + */ +- NGHTTP2_ERR_FLOODED = -904 ++ NGHTTP2_ERR_FLOODED = -904, ++ /** ++ * When a local endpoint receives too many CONTINUATION frames ++ * following a HEADER frame. ++ */ ++ NGHTTP2_ERR_TOO_MANY_CONTINUATIONS = -905, + } nghttp2_error; + + /** +@@ -2773,6 +2778,17 @@ NGHTTP2_EXTERN void + nghttp2_option_set_stream_reset_rate_limit(nghttp2_option *option, + uint64_t burst, uint64_t rate); + ++/** ++ * @function ++ * ++ * This function sets the maximum number of CONTINUATION frames ++ * following an incoming HEADER frame. If more than those frames are ++ * received, the remote endpoint is considered to be misbehaving and ++ * session will be closed. The default value is 8. ++ */ ++NGHTTP2_EXTERN void nghttp2_option_set_max_continuations(nghttp2_option *option, ++ size_t val); ++ + /** + * @function + * +diff --git a/lib/nghttp2_helper.c b/lib/nghttp2_helper.c +index 93dd4754..b3563d98 100644 +--- a/lib/nghttp2_helper.c ++++ b/lib/nghttp2_helper.c +@@ -336,6 +336,8 @@ const char *nghttp2_strerror(int error_code) { + "closed"; + case NGHTTP2_ERR_TOO_MANY_SETTINGS: + return "SETTINGS frame contained more than the maximum allowed entries"; ++ case NGHTTP2_ERR_TOO_MANY_CONTINUATIONS: ++ return "Too many CONTINUATION frames following a HEADER frame"; + default: + return "Unknown error code"; + } +diff --git a/lib/nghttp2_option.c b/lib/nghttp2_option.c +index 43d4e952..53144b9b 100644 +--- a/lib/nghttp2_option.c ++++ b/lib/nghttp2_option.c +@@ -150,3 +150,8 @@ void nghttp2_option_set_stream_reset_rate_limit(nghttp2_option *option, + option->stream_reset_burst = burst; + option->stream_reset_rate = rate; + } ++ ++void nghttp2_option_set_max_continuations(nghttp2_option *option, size_t val) { ++ option->opt_set_mask |= NGHTTP2_OPT_MAX_CONTINUATIONS; ++ option->max_continuations = val; ++} +diff --git a/lib/nghttp2_option.h b/lib/nghttp2_option.h +index 2259e184..c89cb97f 100644 +--- a/lib/nghttp2_option.h ++++ b/lib/nghttp2_option.h +@@ -71,6 +71,7 @@ typedef enum { + NGHTTP2_OPT_SERVER_FALLBACK_RFC7540_PRIORITIES = 1 << 13, + NGHTTP2_OPT_NO_RFC9113_LEADING_AND_TRAILING_WS_VALIDATION = 1 << 14, + NGHTTP2_OPT_STREAM_RESET_RATE_LIMIT = 1 << 15, ++ NGHTTP2_OPT_MAX_CONTINUATIONS = 1 << 16, + } nghttp2_option_flag; + + /** +@@ -98,6 +99,10 @@ struct nghttp2_option { + * NGHTTP2_OPT_MAX_SETTINGS + */ + size_t max_settings; ++ /** ++ * NGHTTP2_OPT_MAX_CONTINUATIONS ++ */ ++ size_t max_continuations; + /** + * Bitwise OR of nghttp2_option_flag to determine that which fields + * are specified. +diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c +index ce21caf9..18949528 100644 +--- a/lib/nghttp2_session.c ++++ b/lib/nghttp2_session.c +@@ -496,6 +496,7 @@ static int session_new(nghttp2_session **session_ptr, + (*session_ptr)->max_send_header_block_length = NGHTTP2_MAX_HEADERSLEN; + (*session_ptr)->max_outbound_ack = NGHTTP2_DEFAULT_MAX_OBQ_FLOOD_ITEM; + (*session_ptr)->max_settings = NGHTTP2_DEFAULT_MAX_SETTINGS; ++ (*session_ptr)->max_continuations = NGHTTP2_DEFAULT_MAX_CONTINUATIONS; + + if (option) { + if ((option->opt_set_mask & NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE) && +@@ -584,6 +585,10 @@ static int session_new(nghttp2_session **session_ptr, + option->stream_reset_burst, + option->stream_reset_rate); + } ++ ++ if (option->opt_set_mask & NGHTTP2_OPT_MAX_CONTINUATIONS) { ++ (*session_ptr)->max_continuations = option->max_continuations; ++ } + } + + rv = nghttp2_hd_deflate_init2(&(*session_ptr)->hd_deflater, +@@ -6778,6 +6783,8 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, + } + } + session_inbound_frame_reset(session); ++ ++ session->num_continuations = 0; + } + break; + } +@@ -6899,6 +6906,10 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, + } + #endif /* DEBUGBUILD */ + ++ if (++session->num_continuations > session->max_continuations) { ++ return NGHTTP2_ERR_TOO_MANY_CONTINUATIONS; ++ } ++ + readlen = inbound_frame_buf_read(iframe, in, last); + in += readlen; + +diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h +index b119329a..ef8f7b27 100644 +--- a/lib/nghttp2_session.h ++++ b/lib/nghttp2_session.h +@@ -110,6 +110,10 @@ typedef struct { + #define NGHTTP2_DEFAULT_STREAM_RESET_BURST 1000 + #define NGHTTP2_DEFAULT_STREAM_RESET_RATE 33 + ++/* The default max number of CONTINUATION frames following an incoming ++ HEADER frame. */ ++#define NGHTTP2_DEFAULT_MAX_CONTINUATIONS 8 ++ + /* Internal state when receiving incoming frame */ + typedef enum { + /* Receiving frame header */ +@@ -290,6 +294,12 @@ struct nghttp2_session { + size_t max_send_header_block_length; + /* The maximum number of settings accepted per SETTINGS frame. */ + size_t max_settings; ++ /* The maximum number of CONTINUATION frames following an incoming ++ HEADER frame. */ ++ size_t max_continuations; ++ /* The number of CONTINUATION frames following an incoming HEADER ++ frame. This variable is reset when END_HEADERS flag is seen. */ ++ size_t num_continuations; + /* Next Stream ID. Made unsigned int to detect >= (1 << 31). */ + uint32_t next_stream_id; + /* The last stream ID this session initiated. For client session, diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 0fff5cd49b19..4554b7af10cb 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -456,12 +456,12 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "Nghttp2", project_desc = "Implementation of HTTP/2 and its header compression algorithm HPACK in C", project_url = "https://nghttp2.org", - version = "1.57.0", - sha256 = "1e3258453784d3b7e6cc48d0be087b168f8360b5d588c66bfeda05d07ad39ffd", + version = "1.59.0", + sha256 = "90fd27685120404544e96a60ed40398a3457102840c38e7215dc6dec8684470f", strip_prefix = "nghttp2-{version}", urls = ["https://github.com/nghttp2/nghttp2/releases/download/v{version}/nghttp2-{version}.tar.gz"], use_category = ["controlplane", "dataplane_core"], - release_date = "2023-10-10", + release_date = "2024-01-21", cpe = "cpe:2.3:a:nghttp2:nghttp2:*", license = "MIT", license_url = "https://github.com/nghttp2/nghttp2/blob/v{version}/LICENSE", diff --git a/changelogs/1.26.8.yaml b/changelogs/1.26.8.yaml new file mode 100644 index 000000000000..a59f0acb0ad0 --- /dev/null +++ b/changelogs/1.26.8.yaml @@ -0,0 +1,13 @@ +date: April 4, 2024 + +bug_fixes: +- area: http2 + change: | + Update nghttp2 to resolve CVE-2024-30255 (https://github.com/envoyproxy/envoy/security/advisories/GHSA-j654-3ccm-vfmm). + +new_features: +- area: google_grpc + change: | + Added an off-by-default runtime flag + ``envoy.reloadable_features.google_grpc_disable_tls_13`` to disable TLSv1.3 + usage by gRPC SDK for ``google_grpc`` services. diff --git a/changelogs/1.27.3.yaml b/changelogs/1.27.3.yaml new file mode 100644 index 000000000000..a67d0b6cabb2 --- /dev/null +++ b/changelogs/1.27.3.yaml @@ -0,0 +1,52 @@ +date: February 9, 2024 + +minor_behavior_changes: +- area: access_log + change: | + When emitting grpc logs, only downstream filter state was used. Now, both downstream and upstream filter states will be tried + to find the keys configured in filter_state_objects_to_log. + +bug_fixes: +- area: buffer + change: | + Fixed a bug (https://github.com/envoyproxy/envoy/issues/28760) that the internal listener causes an undefined + behavior due to the unintended release of the buffer memory. +- area: http + change: | + Fixed recursion when HTTP connection is disconnected due to a high number of premature resets. +- area: grpc + change: | + Fixed a bug in gRPC async client cache which intermittently causes CPU spikes due to busy loop in timer expiration. +- area: tracing + change: | + Fixed a bug where Datadog spans tagged as errors would not have the appropriate error property set. +- area: tracing + change: | + Fixed a bug where child spans produced by the Datadog tracer would have an incorrect operation name. +- area: tracing + change: | + Fixed a bug that caused the Datadog tracing extension to drop traces that + should be kept on account of an extracted sampling decision. +- area: proxy protocol + change: | + fixed a crash when Envoy is configured for PROXY protocol on both a listener and cluster, and the listener receives + a PROXY protocol header with address type LOCAL (typically used for health checks). +- area: proxy_protocol + change: | + Fix crash due to uncaught exception when the operating system does not support an address type (such as IPv6) that is + received in a proxy protocol header. Connections will instead be dropped/reset. +- area: proxy_protocol + change: | + Fixed a bug where TLVs with non utf8 characters were inserted as protobuf values into filter metadata circumventing + ext_authz checks when ``failure_mode_allow`` is set to ``true``. +- area: tls + change: | + Fix crash due to uncaught exception when the operating system does not support an address type (such as IPv6) that is + received in an mTLS client cert IP SAN. These SANs will be ignored. This applies only when using formatter + ``%DOWNSTREAM_PEER_IP_SAN%``. +- area: http + change: | + Fixed crash when HTTP request idle and per try timeouts occurs within backoff interval. +- area: url matching + change: | + Fixed excessive CPU utilization when using regex URL template matcher. diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 396e36b1d9a4..8ce8e7ad5ca9 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -1,17 +1,19 @@ -date: February 9, 2024 +date: April 4, 2024 -minor_behavior_changes: -- area: access_log +behavior_changes: +- area: http2 change: | - When emitting grpc logs, only downstream filter state was used. Now, both downstream and upstream filter states will be tried - to find the keys configured in filter_state_objects_to_log. + Discard the ``Host`` header if the ``:authority`` header was received to bring Envoy into compliance with + https://www.rfc-editor.org/rfc/rfc9113#section-8.3.1 This behavioral change can be reverted by setting runtime flag + ``envoy.reloadable_features.http2_discard_host_header`` to false. bug_fixes: -- area: buffer +- area: http2 change: | - Fixed a bug (https://github.com/envoyproxy/envoy/issues/28760) that the internal listener causes an undefined - behavior due to the unintended release of the buffer memory. -- area: http + Update nghttp2 to resolve CVE-2024-30255 (https://github.com/envoyproxy/envoy/security/advisories/GHSA-j654-3ccm-vfmm). + +new_features: +- area: google_grpc change: | Fixed recursion when HTTP connection is disconnected due to a high number of premature resets. - area: grpc @@ -102,3 +104,6 @@ new_features: config APIs to enable sending and receiving attributes from/to the external processing server. deprecated: + Added an off-by-default runtime flag + ``envoy.reloadable_features.google_grpc_disable_tls_13`` to disable TLSv1.3 + usage by gRPC SDK for ``google_grpc`` services. diff --git a/ci/Dockerfile-envoy b/ci/Dockerfile-envoy index 7f6f0bd13911..396e1fb1d770 100644 --- a/ci/Dockerfile-envoy +++ b/ci/Dockerfile-envoy @@ -1,5 +1,5 @@ ARG BUILD_OS=ubuntu -ARG BUILD_TAG=20.04@sha256:bb1c41682308d7040f74d103022816d41c50d7b0c89e9d706a74b4e548636e54 +ARG BUILD_TAG=20.04@sha256:80ef4a44043dec4490506e6cc4289eeda2d106a70148b74b5ae91ee670e9c35d ARG ENVOY_VRP_BASE_IMAGE=envoy-base @@ -58,7 +58,7 @@ COPY --chown=0:0 --chmod=755 \ # STAGE: envoy-distroless -FROM gcr.io/distroless/base-nossl-debian12:nonroot@sha256:51ab103bb161fdf8fee4c6311a2d41f484effc409d4f4c58342ab68b2da7ccc2 AS envoy-distroless +FROM gcr.io/distroless/base-nossl-debian12:nonroot@sha256:0cf184cfdb9ac2878822b15b8917fae5d42fba26da654cd75ab3ed34add0737f AS envoy-distroless EXPOSE 10000 ENTRYPOINT ["/usr/local/bin/envoy"] CMD ["-c", "/etc/envoy/envoy.yaml"] diff --git a/docs/inventories/v1.26/objects.inv b/docs/inventories/v1.26/objects.inv index 26b06951df94..e8c220301cff 100644 Binary files a/docs/inventories/v1.26/objects.inv and b/docs/inventories/v1.26/objects.inv differ diff --git a/docs/inventories/v1.27/objects.inv b/docs/inventories/v1.27/objects.inv index 0b0acf5c7b42..4eee783c0fd4 100644 Binary files a/docs/inventories/v1.27/objects.inv and b/docs/inventories/v1.27/objects.inv differ diff --git a/docs/versions.yaml b/docs/versions.yaml index 91b80a86fe85..4be885fd2e54 100644 --- a/docs/versions.yaml +++ b/docs/versions.yaml @@ -19,5 +19,5 @@ "1.23": 1.23.12 "1.24": 1.24.12 "1.25": 1.25.11 -"1.26": 1.26.7 -"1.27": 1.27.2 +"1.26": 1.26.8 +"1.27": 1.27.3 diff --git a/source/common/grpc/BUILD b/source/common/grpc/BUILD index 96d1ed06353d..d662c6863e4b 100644 --- a/source/common/grpc/BUILD +++ b/source/common/grpc/BUILD @@ -210,6 +210,7 @@ envoy_cc_library( "//envoy/grpc:google_grpc_creds_interface", "//envoy/registry", "//source/common/config:datasource_lib", + "//source/common/runtime:runtime_lib", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], alwayslink = LEGACY_ALWAYSLINK, diff --git a/source/common/grpc/google_grpc_creds_impl.cc b/source/common/grpc/google_grpc_creds_impl.cc index ae49d3257a7f..5aa2ea91fd8a 100644 --- a/source/common/grpc/google_grpc_creds_impl.cc +++ b/source/common/grpc/google_grpc_creds_impl.cc @@ -4,6 +4,9 @@ #include "envoy/grpc/google_grpc_creds.h" #include "source/common/config/datasource.h" +#include "source/common/runtime/runtime_features.h" + +#include "grpcpp/security/tls_certificate_provider.h" namespace Envoy { namespace Grpc { @@ -15,12 +18,29 @@ std::shared_ptr CredsUtility::getChannelCredentials( case envoy::config::core::v3::GrpcService::GoogleGrpc::ChannelCredentials:: CredentialSpecifierCase::kSslCredentials: { const auto& ssl_credentials = google_grpc.channel_credentials().ssl_credentials(); - const grpc::SslCredentialsOptions ssl_credentials_options = { - Config::DataSource::read(ssl_credentials.root_certs(), true, api), - Config::DataSource::read(ssl_credentials.private_key(), true, api), - Config::DataSource::read(ssl_credentials.cert_chain(), true, api), - }; - return grpc::SslCredentials(ssl_credentials_options); + const auto root_certs = Config::DataSource::read(ssl_credentials.root_certs(), true, api); + const auto private_key = Config::DataSource::read(ssl_credentials.private_key(), true, api); + const auto cert_chain = Config::DataSource::read(ssl_credentials.cert_chain(), true, api); + grpc::experimental::TlsChannelCredentialsOptions options; + if (!private_key.empty() || !cert_chain.empty()) { + options.set_certificate_provider( + std::make_shared( + root_certs, + std::vector{{private_key, cert_chain}})); + } else if (!root_certs.empty()) { + options.set_certificate_provider( + std::make_shared(root_certs)); + } + if (!root_certs.empty()) { + options.watch_root_certs(); + } + if (!private_key.empty() || !cert_chain.empty()) { + options.watch_identity_key_cert_pairs(); + } + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.google_grpc_disable_tls_13")) { + options.set_max_tls_version(grpc_tls_version::TLS1_2); + } + return grpc::experimental::TlsCredentials(options); } case envoy::config::core::v3::GrpcService::GoogleGrpc::ChannelCredentials:: CredentialSpecifierCase::kLocalCredentials: { @@ -43,7 +63,11 @@ std::shared_ptr CredsUtility::defaultSslChannelCredent if (creds != nullptr) { return creds; } - return grpc::SslCredentials({}); + grpc::experimental::TlsChannelCredentialsOptions options; + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.google_grpc_disable_tls_13")) { + options.set_max_tls_version(grpc_tls_version::TLS1_2); + } + return grpc::experimental::TlsCredentials(options); } std::vector> diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index ad7c29b423dc..ab4d5d3b9184 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -1812,6 +1812,11 @@ ConnectionImpl::Http2Options::Http2Options( // on this mitigation, set back to the old 10K number to avoid any changes in the HTTP/2 codec // behavior. nghttp2_option_set_max_outbound_ack(options_, 10000); + + // nghttp2 REQUIRES setting max number of CONTINUATION frames. + // 1024 is chosen to accommodate Envoy's 8Mb max limit of max_request_headers_kb + // in both headers and trailers + nghttp2_option_set_max_continuations(options_, 1024); } ConnectionImpl::Http2Options::~Http2Options() { nghttp2_option_del(options_); } @@ -1826,6 +1831,11 @@ ConnectionImpl::ClientHttp2Options::ClientHttp2Options( // TODO(PiotrSikora): remove this once multiple upstream connections or queuing are implemented. nghttp2_option_set_peer_max_concurrent_streams( options_, ::Envoy::Http2::Utility::OptionsLimits::DEFAULT_MAX_CONCURRENT_STREAMS); + + // nghttp2 REQUIRES setting max number of CONTINUATION frames. + // 1024 is chosen to accommodate Envoy's 8Mb max limit of max_request_headers_kb + // in both headers and trailers + nghttp2_option_set_max_continuations(options_, 1024); } void ConnectionImpl::dumpState(std::ostream& os, int indent_level) const { @@ -2110,6 +2120,18 @@ int ServerConnectionImpl::onHeader(const nghttp2_frame* frame, HeaderString&& na // For a server connection, we should never get push promise frames. ASSERT(frame->hd.type == NGHTTP2_HEADERS); ASSERT(frame->headers.cat == NGHTTP2_HCAT_REQUEST || frame->headers.cat == NGHTTP2_HCAT_HEADERS); + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.http2_discard_host_header")) { + StreamImpl* stream = getStreamUnchecked(frame->hd.stream_id); + if (stream && name == static_cast(Http::Headers::get().HostLegacy)) { + // Check if there is already the :authority header + const auto result = stream->headers().get(Http::Headers::get().Host); + if (!result.empty()) { + // Discard the host header value + return 0; + } + // Otherwise use host value as :authority + } + } return saveHeader(frame, std::move(name), std::move(value)); } diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 60d594c96431..c1d28ac747d4 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -47,6 +47,7 @@ RUNTIME_GUARD(envoy_reloadable_features_format_ports_as_numbers); RUNTIME_GUARD(envoy_reloadable_features_handle_uppercase_scheme); RUNTIME_GUARD(envoy_reloadable_features_http1_allow_codec_error_response_after_1xx_headers); RUNTIME_GUARD(envoy_reloadable_features_http2_decode_metadata_with_quiche); +RUNTIME_GUARD(envoy_reloadable_features_http2_discard_host_header); RUNTIME_GUARD(envoy_reloadable_features_http2_validate_authority_with_quiche); RUNTIME_GUARD(envoy_reloadable_features_http_allow_partial_urls_in_referer); RUNTIME_GUARD(envoy_reloadable_features_http_ext_auth_failure_mode_allow_header_add); @@ -122,6 +123,10 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_refresh_rtt_after_request); // TODO(danzh) false deprecate it once QUICHE has its own enable/disable flag. FALSE_RUNTIME_GUARD(envoy_reloadable_features_quic_reject_all); +// A flag to set the maximum TLS version for google_grpc client to TLS1.2, when needed for +// compliance restrictions. +FALSE_RUNTIME_GUARD(envoy_reloadable_features_google_grpc_disable_tls_13); + // Block of non-boolean flags. Use of int flags is deprecated. Do not add more. ABSL_FLAG(uint64_t, re2_max_program_size_error_level, 100, ""); // NOLINT ABSL_FLAG(uint64_t, re2_max_program_size_warn_level, // NOLINT diff --git a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc index 2b74663df397..f1f593a83aa6 100644 --- a/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc +++ b/source/extensions/stat_sinks/metrics_service/grpc_metrics_service_impl.cc @@ -135,6 +135,7 @@ void MetricsFlusher::flushSummary(io::prometheus::client::MetricFamily& metrics_ quantile->set_value(hist_stats.computedQuantiles()[i]); } summary->set_sample_count(hist_stats.sampleCount()); + summary->set_sample_sum(hist_stats.sampleSum()); } io::prometheus::client::Metric* diff --git a/test/common/grpc/BUILD b/test/common/grpc/BUILD index 01458712de95..890b2ba26d61 100644 --- a/test/common/grpc/BUILD +++ b/test/common/grpc/BUILD @@ -178,6 +178,7 @@ envoy_cc_test( ":grpc_client_integration_test_harness_lib", "//source/common/grpc:async_client_lib", "//source/extensions/grpc_credentials/example:config", + "//test/test_common:test_runtime_lib", ] + envoy_select_google_grpc(["//source/common/grpc:google_async_client_lib"]), ) diff --git a/test/common/grpc/grpc_client_integration_test.cc b/test/common/grpc/grpc_client_integration_test.cc index 3fd32c96d8fc..099d6fa5704c 100644 --- a/test/common/grpc/grpc_client_integration_test.cc +++ b/test/common/grpc/grpc_client_integration_test.cc @@ -5,6 +5,7 @@ #endif +#include "test/test_common/test_runtime.h" #include "test/common/grpc/grpc_client_integration_test_harness.h" using testing::Eq; @@ -409,6 +410,29 @@ TEST_P(GrpcSslClientIntegrationTest, BasicSslRequestWithClientCert) { dispatcher_helper_.runDispatcher(); } +// Validate TLS version mismatch between the client and the server. +TEST_P(GrpcSslClientIntegrationTest, BasicSslRequestHandshakeFailure) { + SKIP_IF_GRPC_CLIENT(ClientType::EnvoyGrpc); + TestScopedRuntime scoped_runtime; + scoped_runtime.mergeValues({{"envoy.reloadable_features.google_grpc_disable_tls_13", "true"}}); + use_server_tls_13_ = true; + initialize(); + auto request = createRequest(empty_metadata_, false); + EXPECT_CALL(*request->child_span_, setTag(Eq(Tracing::Tags::get().GrpcStatusCode), Eq("13"))); + EXPECT_CALL(*request->child_span_, + setTag(Eq(Tracing::Tags::get().Error), Eq(Tracing::Tags::get().True))); + EXPECT_CALL(*request, onFailure(Status::Internal, "", _)).WillOnce(InvokeWithoutArgs([this]() { + dispatcher_helper_.dispatcher_.exit(); + })); + EXPECT_CALL(*request->child_span_, finishSpan()); + FakeRawConnectionPtr fake_connection; + ASSERT_TRUE(fake_upstream_->waitForRawConnection(fake_connection)); + if (fake_connection->connected()) { + ASSERT_TRUE(fake_connection->waitForDisconnect()); + } + dispatcher_helper_.dispatcher_.run(Event::Dispatcher::RunType::Block); +} + #ifdef ENVOY_GOOGLE_GRPC // AccessToken credential validation tests. class GrpcAccessTokenClientIntegrationTest : public GrpcSslClientIntegrationTest { diff --git a/test/common/grpc/grpc_client_integration_test_harness.h b/test/common/grpc/grpc_client_integration_test_harness.h index a30067cb190a..93990a8982ab 100644 --- a/test/common/grpc/grpc_client_integration_test_harness.h +++ b/test/common/grpc/grpc_client_integration_test_harness.h @@ -365,7 +365,8 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { virtual void expectExtraHeaders(FakeStream&) {} - HelloworldRequestPtr createRequest(const TestMetadata& initial_metadata) { + HelloworldRequestPtr createRequest(const TestMetadata& initial_metadata, + bool expect_upstream_request = true) { auto request = std::make_unique(dispatcher_helper_); EXPECT_CALL(*request, onCreateInitialMetadata(_)) .WillOnce(Invoke([&initial_metadata](Http::HeaderMap& headers) { @@ -392,6 +393,10 @@ class GrpcClientIntegrationTest : public GrpcClientIntegrationParamTest { active_span, Http::AsyncClient::RequestOptions()); EXPECT_NE(request->grpc_request_, nullptr); + if (!expect_upstream_request) { + return request; + } + if (!fake_connection_) { AssertionResult result = fake_upstream_->waitForHttpConnection(*dispatcher_, fake_connection_); @@ -526,6 +531,7 @@ class GrpcSslClientIntegrationTest : public GrpcClientIntegrationTest { tls_cert->mutable_private_key()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/clientkey.pem")); } + auto cfg = std::make_unique( tls_context, factory_context_); @@ -557,6 +563,13 @@ class GrpcSslClientIntegrationTest : public GrpcClientIntegrationTest { validation_context->mutable_trusted_ca()->set_filename( TestEnvironment::runfilesPath("test/config/integration/certs/cacert.pem")); } + if (use_server_tls_13_) { + auto* tls_params = common_tls_context->mutable_tls_params(); + tls_params->set_tls_minimum_protocol_version( + envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3); + tls_params->set_tls_maximum_protocol_version( + envoy::extensions::transport_sockets::tls::v3::TlsParameters::TLSv1_3); + } auto cfg = std::make_unique( tls_context, factory_context_); @@ -568,6 +581,7 @@ class GrpcSslClientIntegrationTest : public GrpcClientIntegrationTest { } bool use_client_cert_{}; + bool use_server_tls_13_{false}; testing::NiceMock factory_context_; }; diff --git a/test/common/http/http2/http2_frame.h b/test/common/http/http2/http2_frame.h index 6349f5d94d84..0789fc5bf11a 100644 --- a/test/common/http/http2/http2_frame.h +++ b/test/common/http/http2/http2_frame.h @@ -260,6 +260,9 @@ class Http2Frame { ASSERT(size() >= HeaderSize); setPayloadSize(size() - HeaderSize); } + // Headers are directly encoded + void appendStaticHeader(StaticHeaderIndex index); + void appendHeaderWithoutIndexing(StaticHeaderIndex index, absl::string_view value); private: void buildHeader(Type type, uint32_t payload_size = 0, uint8_t flags = 0, uint32_t stream_id = 0); @@ -277,9 +280,6 @@ class Http2Frame { std::copy(data.begin(), data.end(), data_.begin() + 9); } - // Headers are directly encoded - void appendStaticHeader(StaticHeaderIndex index); - void appendHeaderWithoutIndexing(StaticHeaderIndex index, absl::string_view value); void appendEmptyHeader(); DataContainer data_; diff --git a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc index 6e7dc7425ce0..59ceb061fbd4 100644 --- a/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc +++ b/test/extensions/stats_sinks/metrics_service/grpc_metrics_service_impl_test.cc @@ -344,6 +344,7 @@ TEST_F(MetricsServiceSinkTest, HistogramEmitModeBoth) { const auto& metric1 = (*metrics)[0].metric(0); EXPECT_TRUE(metric1.has_summary()); + EXPECT_TRUE(metric1.summary().has_sample_sum()); const auto& metric2 = (*metrics)[1].metric(0); EXPECT_TRUE(metric2.has_histogram()); })); @@ -364,6 +365,7 @@ TEST_F(MetricsServiceSinkTest, HistogramEmitModeSummary) { const auto& metric1 = (*metrics)[0].metric(0); EXPECT_TRUE(metric1.has_summary()); + EXPECT_TRUE(metric1.summary().has_sample_sum()); })); sink.flush(snapshot_); } diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index f2e7df6b2b98..c2c9dc5e5486 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -2129,7 +2129,84 @@ TEST_P(Http2FrameIntegrationTest, AccessLogOfWireBytesIfResponseSizeGreaterThanW // Cleanup. tcp_client_->close(); } +TEST_P(Http2FrameIntegrationTest, HostDifferentFromAuthority) { + beginSession(); + + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(Http2Frame::makeClientStreamId(request_idx), + "one.example.com", "/path", {{"host", "two.example.com"}}); + sendFrame(request); + + waitForNextUpstreamRequest(); + EXPECT_EQ(upstream_request_->headers().getHostValue(), "one.example.com"); + upstream_request_->encodeHeaders(default_response_headers_, true); + auto frame = readFrame(); + EXPECT_EQ(Http2Frame::Type::Headers, frame.type()); + EXPECT_EQ(Http2Frame::ResponseStatus::Ok, frame.responseStatus()); + tcp_client_->close(); +} + +TEST_P(Http2FrameIntegrationTest, HostSameAsAuthority) { + beginSession(); + + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(Http2Frame::makeClientStreamId(request_idx), + "one.example.com", "/path", {{"host", "one.example.com"}}); + sendFrame(request); + + waitForNextUpstreamRequest(); + EXPECT_EQ(upstream_request_->headers().getHostValue(), "one.example.com"); + upstream_request_->encodeHeaders(default_response_headers_, true); + auto frame = readFrame(); + EXPECT_EQ(Http2Frame::Type::Headers, frame.type()); + EXPECT_EQ(Http2Frame::ResponseStatus::Ok, frame.responseStatus()); + tcp_client_->close(); +} + +TEST_P(Http2FrameIntegrationTest, HostConcatenatedWithAuthorityWithOverride) { + config_helper_.addRuntimeOverride("envoy.reloadable_features.http2_discard_host_header", "false"); + beginSession(); + + uint32_t request_idx = 0; + auto request = Http2Frame::makeRequest(Http2Frame::makeClientStreamId(request_idx), + "one.example.com", "/path", {{"host", "two.example.com"}}); + sendFrame(request); + + waitForNextUpstreamRequest(); + EXPECT_EQ(upstream_request_->headers().getHostValue(), "one.example.com,two.example.com"); + upstream_request_->encodeHeaders(default_response_headers_, true); + auto frame = readFrame(); + EXPECT_EQ(Http2Frame::Type::Headers, frame.type()); + EXPECT_EQ(Http2Frame::ResponseStatus::Ok, frame.responseStatus()); + tcp_client_->close(); +} + +// All HTTP/2 static headers must be before non-static headers. +// Verify that codecs validate this. +TEST_P(Http2FrameIntegrationTest, HostBeforeAuthorityIsRejected) { +#ifdef ENVOY_ENABLE_UHV + // TODO(yanavlasov): fix this check for oghttp2 in UHV mode. + return; +#endif + beginSession(); + Http2Frame request = Http2Frame::makeEmptyHeadersFrame(Http2Frame::makeClientStreamId(0), + Http2Frame::HeadersFlags::EndHeaders); + request.appendStaticHeader(Http2Frame::StaticHeaderIndex::MethodPost); + request.appendStaticHeader(Http2Frame::StaticHeaderIndex::SchemeHttps); + request.appendHeaderWithoutIndexing(Http2Frame::StaticHeaderIndex::Path, "/path"); + // Add the `host` header before `:authority` + request.appendHeaderWithoutIndexing({"host", "two.example.com"}); + request.appendHeaderWithoutIndexing(Http2Frame::StaticHeaderIndex::Authority, "one.example.com"); + request.adjustPayloadSize(); + + sendFrame(request); + + // By default codec treats stream errors as protocol errors and closes the connection. + tcp_client_->waitForDisconnect(); + tcp_client_->close(); + EXPECT_EQ(1, test_server_->counter("http.config_test.downstream_cx_protocol_error")->value()); +} TEST_P(Http2FrameIntegrationTest, MultipleHeaderOnlyRequests) { const int kRequestsSentPerIOCycle = 20; autonomous_upstream_ = true;