From 278ce7ebc29fc41a2957cd91c9ac46c1340baa40 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Tue, 9 Nov 2021 22:00:00 +0000 Subject: [PATCH 01/52] Auto-detect proxy protocol Signed-off-by: Kevin Dorosh --- .../proxy_protocol/v3/proxy_protocol.proto | 5 ++++ docs/root/version_history/current.rst | 1 + .../listener/proxy_protocol/proxy_protocol.cc | 24 ++++++++++++++----- .../listener/proxy_protocol/proxy_protocol.h | 13 ++++++++-- .../proxy_protocol/proxy_protocol_test.cc | 13 ++++++++++ 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index fb8047d391e9..136792e89e85 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -40,4 +40,9 @@ message ProxyProtocol { // The list of rules to apply to requests. repeated Rule rules = 1; + + // NOTICE: only enable if ALL traffic to the listener comes from a trusted source. + // Defaults to false. If true, attempt to detect proxy protocol if present, and allow + // requests through if proxy protocol is not used on the connection. + bool detect_proxy_protocol = 2; } diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 0ccdb71c4b8a..8892ebd460eb 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -64,6 +64,7 @@ New Features * listener: added support for :ref:`MPTCP ` (multipath TCP). * oauth filter: added :ref:`cookie_names ` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter. * oauth filter: setting IdToken and RefreshToken cookies if they are provided by Identity provider along with AccessToken. +* proxy protocol: added support for auto-detecting proxy protocol on the listener from trusted downstreams as an opt-in flag. * tcp: added a :ref:`FilterState ` :ref:`hash policy `, used by :ref:`TCP proxy ` to allow hashing load balancer algorithms to hash on objects in filter state. * tcp_proxy: added support to populate upstream http connect header values from stream info. * thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index ee9e0c53f025..12b228c84719 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -47,6 +47,7 @@ Config::Config( Stats::Scope& scope, const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config) : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} { + detect_proxy_protocol_ = proto_config.detect_proxy_protocol(); for (const auto& rule : proto_config.rules()) { tlv_types_[0xFF & rule.tlv_type()] = rule.on_tlv_present(); } @@ -63,6 +64,8 @@ const KeyValuePair* Config::isTlvTypeNeeded(uint8_t type) const { size_t Config::numberOfNeededTlvTypes() const { return tlv_types_.size(); } +bool Config::detectProxyProtocol() const { return detect_proxy_protocol_; } + Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { ENVOY_LOG(debug, "proxy_protocol: new connection accepted"); Network::ConnectionSocket& socket = cb.socket(); @@ -77,11 +80,20 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { return Network::FilterStatus::StopIteration; } +ReadOrParseState Filter::resetAndContinue(Network::IoHandle& io_handle) { + // Release the file event so that we do not interfere with the connection read events. + io_handle.resetFileEvents(); + cb_->continueFilterChain(true); + return ReadOrParseState::Done; +} + void Filter::onRead() { const ReadOrParseState read_state = onReadWorker(); if (read_state == ReadOrParseState::Error) { config_->stats_.downstream_cx_proxy_proto_error_.inc(); cb_->continueFilterChain(false); + } else if (read_state == ReadOrParseState::SkipFilterError) { + resetAndContinue(cb_->socket().ioHandle()); } } @@ -135,10 +147,7 @@ ReadOrParseState Filter::onReadWorker() { proxy_protocol_header_.value().remote_address_); } - // Release the file event so that we do not interfere with the connection read events. - socket.ioHandle().resetFileEvents(); - cb_->continueFilterChain(true); - return ReadOrParseState::Done; + return resetAndContinue(socket.ioHandle()); } absl::optional Filter::lenV2Address(char* buf) { @@ -309,7 +318,7 @@ ReadOrParseState Filter::parseExtensions(Network::IoHandle& io_handle, uint8_t* const auto recv_result = io_handle.recv(buf, to_read, 0); if (!recv_result.ok()) { if (recv_result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) { - return ReadOrParseState::TryAgainLater; + return ReadOrParseState::TryAgainLaterError; } ENVOY_LOG(debug, "failed to read proxy protocol (no bytes avail)"); return ReadOrParseState::Error; @@ -424,7 +433,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (!result.ok()) { if (result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) { - return ReadOrParseState::TryAgainLater; + return ReadOrParseState::TryAgainLaterError; } ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; @@ -434,6 +443,9 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (nread < 1) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; + } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->detectProxyProtocol()) { + ENVOY_LOG(debug, "need more bytes to read proxy protocol"); + return ReadOrParseState::SkipFilterError; } if (buf_off_ + nread >= PROXY_PROTO_V2_HEADER_LEN) { diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index 469c838f9b83..0b6f9e76a6c1 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -60,15 +60,22 @@ class Config : public Logger::Loggable { */ size_t numberOfNeededTlvTypes() const; + /** + * Filter configuration that determines if we should pass-through failed proxy protocol + * requests. Should only be configured to true for trusted downstreams. + */ + bool detectProxyProtocol() const; + private: absl::flat_hash_map tlv_types_; + bool detect_proxy_protocol_{}; }; using ConfigSharedPtr = std::shared_ptr; enum ProxyProtocolVersion { Unknown = -1, InProgress = -2, V1 = 1, V2 = 2 }; -enum class ReadOrParseState { Done, TryAgainLater, Error }; +enum class ReadOrParseState { Done, TryAgainLaterError, Error, SkipFilterError }; /** * Implementation the PROXY Protocol listener filter @@ -98,7 +105,7 @@ class Filter : public Network::ListenerFilter, Logger::Loggable lenV2Address(char* buf); + ReadOrParseState resetAndContinue(Network::IoHandle& io_handle); + Network::ListenerFilterCallbacks* cb_{}; // The offset in buf_ that has been fully read diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 6a7b0634a8bb..8a0b05433b09 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -222,6 +222,19 @@ TEST_P(ProxyProtocolTest, V1Basic) { disconnect(); } +TEST_P(ProxyProtocolTest, DetectNoProxyProtocol) { + + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_detect_proxy_protocol(true); + connect(true, &proto_config); + + write("more data"); + + expectData("more data"); + + disconnect(); +} + TEST_P(ProxyProtocolTest, V1Minimal) { connect(); write("PROXY UNKNOWN\r\nmore data"); From 121e87d085fc29c4cff0bf4ec16da53b7cd4799c Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 03:07:39 +0000 Subject: [PATCH 02/52] Fix format Signed-off-by: Kevin Dorosh --- source/extensions/extensions_build_config.bzl | 306 ++++++++---------- 1 file changed, 137 insertions(+), 169 deletions(-) diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index f9acb724d964..d883e80299a5 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -3,317 +3,285 @@ EXTENSIONS = { # # Access loggers # - - "envoy.access_loggers.file": "//source/extensions/access_loggers/file:config", - "envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config", - "envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config", - "envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config", - "envoy.access_loggers.stream": "//source/extensions/access_loggers/stream:config", - "envoy.access_loggers.wasm": "//source/extensions/access_loggers/wasm:config", + "envoy.access_loggers.file": "//source/extensions/access_loggers/file:config", + "envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config", + "envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config", + "envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config", + "envoy.access_loggers.stream": "//source/extensions/access_loggers/stream:config", + "envoy.access_loggers.wasm": "//source/extensions/access_loggers/wasm:config", # # Clusters # - - "envoy.clusters.aggregate": "//source/extensions/clusters/aggregate:cluster", - "envoy.clusters.dynamic_forward_proxy": "//source/extensions/clusters/dynamic_forward_proxy:cluster", - "envoy.clusters.redis": "//source/extensions/clusters/redis:redis_cluster", + "envoy.clusters.aggregate": "//source/extensions/clusters/aggregate:cluster", + "envoy.clusters.dynamic_forward_proxy": "//source/extensions/clusters/dynamic_forward_proxy:cluster", + "envoy.clusters.redis": "//source/extensions/clusters/redis:redis_cluster", # # Compression # - - "envoy.compression.gzip.compressor": "//source/extensions/compression/gzip/compressor:config", - "envoy.compression.gzip.decompressor": "//source/extensions/compression/gzip/decompressor:config", - "envoy.compression.brotli.compressor": "//source/extensions/compression/brotli/compressor:config", - "envoy.compression.brotli.decompressor": "//source/extensions/compression/brotli/decompressor:config", + "envoy.compression.gzip.compressor": "//source/extensions/compression/gzip/compressor:config", + "envoy.compression.gzip.decompressor": "//source/extensions/compression/gzip/decompressor:config", + "envoy.compression.brotli.compressor": "//source/extensions/compression/brotli/compressor:config", + "envoy.compression.brotli.decompressor": "//source/extensions/compression/brotli/decompressor:config", # # gRPC Credentials Plugins # - - "envoy.grpc_credentials.file_based_metadata": "//source/extensions/grpc_credentials/file_based_metadata:config", - "envoy.grpc_credentials.aws_iam": "//source/extensions/grpc_credentials/aws_iam:config", + "envoy.grpc_credentials.file_based_metadata": "//source/extensions/grpc_credentials/file_based_metadata:config", + "envoy.grpc_credentials.aws_iam": "//source/extensions/grpc_credentials/aws_iam:config", # # WASM # - - "envoy.bootstrap.wasm": "//source/extensions/bootstrap/wasm:config", + "envoy.bootstrap.wasm": "//source/extensions/bootstrap/wasm:config", # # Health checkers # - - "envoy.health_checkers.redis": "//source/extensions/health_checkers/redis:config", + "envoy.health_checkers.redis": "//source/extensions/health_checkers/redis:config", # # Input Matchers # - - "envoy.matching.input_matchers.consistent_hashing": "//source/extensions/matching/input_matchers/consistent_hashing:config", - "envoy.matching.input_matchers.ip": "//source/extensions/matching/input_matchers/ip:config", + "envoy.matching.input_matchers.consistent_hashing": "//source/extensions/matching/input_matchers/consistent_hashing:config", + "envoy.matching.input_matchers.ip": "//source/extensions/matching/input_matchers/ip:config", # # Generic Inputs # - - "envoy.matching.common_inputs.environment_variable": "//source/extensions/matching/common_inputs/environment_variable:config", + "envoy.matching.common_inputs.environment_variable": "//source/extensions/matching/common_inputs/environment_variable:config", # # HTTP filters # - - "envoy.filters.http.adaptive_concurrency": "//source/extensions/filters/http/adaptive_concurrency:config", - "envoy.filters.http.admission_control": "//source/extensions/filters/http/admission_control:config", - "envoy.filters.http.alternate_protocols_cache": "//source/extensions/filters/http/alternate_protocols_cache:config", - "envoy.filters.http.aws_lambda": "//source/extensions/filters/http/aws_lambda:config", - "envoy.filters.http.aws_request_signing": "//source/extensions/filters/http/aws_request_signing:config", - "envoy.filters.http.bandwidth_limit": "//source/extensions/filters/http/bandwidth_limit:config", - "envoy.filters.http.buffer": "//source/extensions/filters/http/buffer:config", - "envoy.filters.http.cache": "//source/extensions/filters/http/cache:config", - "envoy.filters.http.cdn_loop": "//source/extensions/filters/http/cdn_loop:config", - "envoy.filters.http.compressor": "//source/extensions/filters/http/compressor:config", - "envoy.filters.http.cors": "//source/extensions/filters/http/cors:config", - "envoy.filters.http.composite": "//source/extensions/filters/http/composite:config", - "envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config", - "envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config", - "envoy.filters.http.dynamic_forward_proxy": "//source/extensions/filters/http/dynamic_forward_proxy:config", - "envoy.filters.http.dynamo": "//source/extensions/filters/http/dynamo:config", - "envoy.filters.http.ext_authz": "//source/extensions/filters/http/ext_authz:config", - "envoy.filters.http.ext_proc": "//source/extensions/filters/http/ext_proc:config", - "envoy.filters.http.fault": "//source/extensions/filters/http/fault:config", - "envoy.filters.http.grpc_http1_bridge": "//source/extensions/filters/http/grpc_http1_bridge:config", - "envoy.filters.http.grpc_http1_reverse_bridge": "//source/extensions/filters/http/grpc_http1_reverse_bridge:config", - "envoy.filters.http.grpc_json_transcoder": "//source/extensions/filters/http/grpc_json_transcoder:config", - "envoy.filters.http.grpc_stats": "//source/extensions/filters/http/grpc_stats:config", - "envoy.filters.http.grpc_web": "//source/extensions/filters/http/grpc_web:config", - "envoy.filters.http.header_to_metadata": "//source/extensions/filters/http/header_to_metadata:config", - "envoy.filters.http.health_check": "//source/extensions/filters/http/health_check:config", - "envoy.filters.http.ip_tagging": "//source/extensions/filters/http/ip_tagging:config", - "envoy.filters.http.jwt_authn": "//source/extensions/filters/http/jwt_authn:config", + "envoy.filters.http.adaptive_concurrency": "//source/extensions/filters/http/adaptive_concurrency:config", + "envoy.filters.http.admission_control": "//source/extensions/filters/http/admission_control:config", + "envoy.filters.http.alternate_protocols_cache": "//source/extensions/filters/http/alternate_protocols_cache:config", + "envoy.filters.http.aws_lambda": "//source/extensions/filters/http/aws_lambda:config", + "envoy.filters.http.aws_request_signing": "//source/extensions/filters/http/aws_request_signing:config", + "envoy.filters.http.bandwidth_limit": "//source/extensions/filters/http/bandwidth_limit:config", + "envoy.filters.http.buffer": "//source/extensions/filters/http/buffer:config", + "envoy.filters.http.cache": "//source/extensions/filters/http/cache:config", + "envoy.filters.http.cdn_loop": "//source/extensions/filters/http/cdn_loop:config", + "envoy.filters.http.compressor": "//source/extensions/filters/http/compressor:config", + "envoy.filters.http.cors": "//source/extensions/filters/http/cors:config", + "envoy.filters.http.composite": "//source/extensions/filters/http/composite:config", + "envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config", + "envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config", + "envoy.filters.http.dynamic_forward_proxy": "//source/extensions/filters/http/dynamic_forward_proxy:config", + "envoy.filters.http.dynamo": "//source/extensions/filters/http/dynamo:config", + "envoy.filters.http.ext_authz": "//source/extensions/filters/http/ext_authz:config", + "envoy.filters.http.ext_proc": "//source/extensions/filters/http/ext_proc:config", + "envoy.filters.http.fault": "//source/extensions/filters/http/fault:config", + "envoy.filters.http.grpc_http1_bridge": "//source/extensions/filters/http/grpc_http1_bridge:config", + "envoy.filters.http.grpc_http1_reverse_bridge": "//source/extensions/filters/http/grpc_http1_reverse_bridge:config", + "envoy.filters.http.grpc_json_transcoder": "//source/extensions/filters/http/grpc_json_transcoder:config", + "envoy.filters.http.grpc_stats": "//source/extensions/filters/http/grpc_stats:config", + "envoy.filters.http.grpc_web": "//source/extensions/filters/http/grpc_web:config", + "envoy.filters.http.header_to_metadata": "//source/extensions/filters/http/header_to_metadata:config", + "envoy.filters.http.health_check": "//source/extensions/filters/http/health_check:config", + "envoy.filters.http.ip_tagging": "//source/extensions/filters/http/ip_tagging:config", + "envoy.filters.http.jwt_authn": "//source/extensions/filters/http/jwt_authn:config", # Disabled by default - "envoy.filters.http.kill_request": "//source/extensions/filters/http/kill_request:kill_request_config", - "envoy.filters.http.local_ratelimit": "//source/extensions/filters/http/local_ratelimit:config", - "envoy.filters.http.lua": "//source/extensions/filters/http/lua:config", - "envoy.filters.http.oauth2": "//source/extensions/filters/http/oauth2:config", - "envoy.filters.http.on_demand": "//source/extensions/filters/http/on_demand:config", - "envoy.filters.http.original_src": "//source/extensions/filters/http/original_src:config", - "envoy.filters.http.ratelimit": "//source/extensions/filters/http/ratelimit:config", - "envoy.filters.http.rbac": "//source/extensions/filters/http/rbac:config", - "envoy.filters.http.router": "//source/extensions/filters/http/router:config", - "envoy.filters.http.set_metadata": "//source/extensions/filters/http/set_metadata:config", - "envoy.filters.http.tap": "//source/extensions/filters/http/tap:config", - "envoy.filters.http.wasm": "//source/extensions/filters/http/wasm:config", + "envoy.filters.http.kill_request": "//source/extensions/filters/http/kill_request:kill_request_config", + "envoy.filters.http.local_ratelimit": "//source/extensions/filters/http/local_ratelimit:config", + "envoy.filters.http.lua": "//source/extensions/filters/http/lua:config", + "envoy.filters.http.oauth2": "//source/extensions/filters/http/oauth2:config", + "envoy.filters.http.on_demand": "//source/extensions/filters/http/on_demand:config", + "envoy.filters.http.original_src": "//source/extensions/filters/http/original_src:config", + "envoy.filters.http.ratelimit": "//source/extensions/filters/http/ratelimit:config", + "envoy.filters.http.rbac": "//source/extensions/filters/http/rbac:config", + "envoy.filters.http.router": "//source/extensions/filters/http/router:config", + "envoy.filters.http.set_metadata": "//source/extensions/filters/http/set_metadata:config", + "envoy.filters.http.tap": "//source/extensions/filters/http/tap:config", + "envoy.filters.http.wasm": "//source/extensions/filters/http/wasm:config", # # Listener filters # - - "envoy.filters.listener.http_inspector": "//source/extensions/filters/listener/http_inspector:config", + "envoy.filters.listener.http_inspector": "//source/extensions/filters/listener/http_inspector:config", # NOTE: The original_dst filter is implicitly loaded if original_dst functionality is # configured on the listener. Do not remove it in that case or configs will fail to load. - "envoy.filters.listener.original_dst": "//source/extensions/filters/listener/original_dst:config", - "envoy.filters.listener.original_src": "//source/extensions/filters/listener/original_src:config", + "envoy.filters.listener.original_dst": "//source/extensions/filters/listener/original_dst:config", + "envoy.filters.listener.original_src": "//source/extensions/filters/listener/original_src:config", # NOTE: The proxy_protocol filter is implicitly loaded if proxy_protocol functionality is # configured on the listener. Do not remove it in that case or configs will fail to load. - "envoy.filters.listener.proxy_protocol": "//source/extensions/filters/listener/proxy_protocol:config", - "envoy.filters.listener.tls_inspector": "//source/extensions/filters/listener/tls_inspector:config", + "envoy.filters.listener.proxy_protocol": "//source/extensions/filters/listener/proxy_protocol:config", + "envoy.filters.listener.tls_inspector": "//source/extensions/filters/listener/tls_inspector:config", # # Network filters # - - "envoy.filters.network.client_ssl_auth": "//source/extensions/filters/network/client_ssl_auth:config", - "envoy.filters.network.connection_limit": "//source/extensions/filters/network/connection_limit:config", - "envoy.filters.network.direct_response": "//source/extensions/filters/network/direct_response:config", - "envoy.filters.network.dubbo_proxy": "//source/extensions/filters/network/dubbo_proxy:config", - "envoy.filters.network.echo": "//source/extensions/filters/network/echo:config", - "envoy.filters.network.ext_authz": "//source/extensions/filters/network/ext_authz:config", - "envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config", - "envoy.filters.network.local_ratelimit": "//source/extensions/filters/network/local_ratelimit:config", - "envoy.filters.network.mongo_proxy": "//source/extensions/filters/network/mongo_proxy:config", - "envoy.filters.network.ratelimit": "//source/extensions/filters/network/ratelimit:config", - "envoy.filters.network.rbac": "//source/extensions/filters/network/rbac:config", - "envoy.filters.network.redis_proxy": "//source/extensions/filters/network/redis_proxy:config", - "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", - "envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", - "envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", - "envoy.filters.network.sni_dynamic_forward_proxy": "//source/extensions/filters/network/sni_dynamic_forward_proxy:config", - "envoy.filters.network.wasm": "//source/extensions/filters/network/wasm:config", - "envoy.filters.network.zookeeper_proxy": "//source/extensions/filters/network/zookeeper_proxy:config", + "envoy.filters.network.client_ssl_auth": "//source/extensions/filters/network/client_ssl_auth:config", + "envoy.filters.network.connection_limit": "//source/extensions/filters/network/connection_limit:config", + "envoy.filters.network.direct_response": "//source/extensions/filters/network/direct_response:config", + "envoy.filters.network.dubbo_proxy": "//source/extensions/filters/network/dubbo_proxy:config", + "envoy.filters.network.echo": "//source/extensions/filters/network/echo:config", + "envoy.filters.network.ext_authz": "//source/extensions/filters/network/ext_authz:config", + "envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config", + "envoy.filters.network.local_ratelimit": "//source/extensions/filters/network/local_ratelimit:config", + "envoy.filters.network.mongo_proxy": "//source/extensions/filters/network/mongo_proxy:config", + "envoy.filters.network.ratelimit": "//source/extensions/filters/network/ratelimit:config", + "envoy.filters.network.rbac": "//source/extensions/filters/network/rbac:config", + "envoy.filters.network.redis_proxy": "//source/extensions/filters/network/redis_proxy:config", + "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", + "envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", + "envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", + "envoy.filters.network.sni_dynamic_forward_proxy": "//source/extensions/filters/network/sni_dynamic_forward_proxy:config", + "envoy.filters.network.wasm": "//source/extensions/filters/network/wasm:config", + "envoy.filters.network.zookeeper_proxy": "//source/extensions/filters/network/zookeeper_proxy:config", # # UDP filters # - - "envoy.filters.udp_listener.dns_filter": "//source/extensions/filters/udp/dns_filter:config", - "envoy.filters.udp_listener.udp_proxy": "//source/extensions/filters/udp/udp_proxy:config", + "envoy.filters.udp_listener.dns_filter": "//source/extensions/filters/udp/dns_filter:config", + "envoy.filters.udp_listener.udp_proxy": "//source/extensions/filters/udp/udp_proxy:config", # # Resource monitors # - - "envoy.resource_monitors.fixed_heap": "//source/extensions/resource_monitors/fixed_heap:config", - "envoy.resource_monitors.injected_resource": "//source/extensions/resource_monitors/injected_resource:config", + "envoy.resource_monitors.fixed_heap": "//source/extensions/resource_monitors/fixed_heap:config", + "envoy.resource_monitors.injected_resource": "//source/extensions/resource_monitors/injected_resource:config", # # Stat sinks # - - "envoy.stat_sinks.dog_statsd": "//source/extensions/stat_sinks/dog_statsd:config", - "envoy.stat_sinks.graphite_statsd": "//source/extensions/stat_sinks/graphite_statsd:config", - "envoy.stat_sinks.hystrix": "//source/extensions/stat_sinks/hystrix:config", - "envoy.stat_sinks.metrics_service": "//source/extensions/stat_sinks/metrics_service:config", - "envoy.stat_sinks.statsd": "//source/extensions/stat_sinks/statsd:config", - "envoy.stat_sinks.wasm": "//source/extensions/stat_sinks/wasm:config", + "envoy.stat_sinks.dog_statsd": "//source/extensions/stat_sinks/dog_statsd:config", + "envoy.stat_sinks.graphite_statsd": "//source/extensions/stat_sinks/graphite_statsd:config", + "envoy.stat_sinks.hystrix": "//source/extensions/stat_sinks/hystrix:config", + "envoy.stat_sinks.metrics_service": "//source/extensions/stat_sinks/metrics_service:config", + "envoy.stat_sinks.statsd": "//source/extensions/stat_sinks/statsd:config", + "envoy.stat_sinks.wasm": "//source/extensions/stat_sinks/wasm:config", # # Thrift filters # - - "envoy.filters.thrift.router": "//source/extensions/filters/network/thrift_proxy/router:config", - "envoy.filters.thrift.ratelimit": "//source/extensions/filters/network/thrift_proxy/filters/ratelimit:config", + "envoy.filters.thrift.router": "//source/extensions/filters/network/thrift_proxy/router:config", + "envoy.filters.thrift.ratelimit": "//source/extensions/filters/network/thrift_proxy/filters/ratelimit:config", # # Tracers # - - "envoy.tracers.dynamic_ot": "//source/extensions/tracers/dynamic_ot:config", - "envoy.tracers.lightstep": "//source/extensions/tracers/lightstep:config", - "envoy.tracers.datadog": "//source/extensions/tracers/datadog:config", - "envoy.tracers.zipkin": "//source/extensions/tracers/zipkin:config", - "envoy.tracers.opencensus": "//source/extensions/tracers/opencensus:config", - "envoy.tracers.xray": "//source/extensions/tracers/xray:config", - "envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config", + "envoy.tracers.dynamic_ot": "//source/extensions/tracers/dynamic_ot:config", + "envoy.tracers.lightstep": "//source/extensions/tracers/lightstep:config", + "envoy.tracers.datadog": "//source/extensions/tracers/datadog:config", + "envoy.tracers.zipkin": "//source/extensions/tracers/zipkin:config", + "envoy.tracers.opencensus": "//source/extensions/tracers/opencensus:config", + "envoy.tracers.xray": "//source/extensions/tracers/xray:config", + "envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config", # # Transport sockets # - - "envoy.transport_sockets.alts": "//source/extensions/transport_sockets/alts:config", - "envoy.transport_sockets.upstream_proxy_protocol": "//source/extensions/transport_sockets/proxy_protocol:upstream_config", - "envoy.transport_sockets.raw_buffer": "//source/extensions/transport_sockets/raw_buffer:config", - "envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config", - "envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config", - "envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config", + "envoy.transport_sockets.alts": "//source/extensions/transport_sockets/alts:config", + "envoy.transport_sockets.upstream_proxy_protocol": "//source/extensions/transport_sockets/proxy_protocol:upstream_config", + "envoy.transport_sockets.raw_buffer": "//source/extensions/transport_sockets/raw_buffer:config", + "envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config", + "envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config", + "envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config", # # Retry host predicates # - - "envoy.retry_host_predicates.previous_hosts": "//source/extensions/retry/host/previous_hosts:config", - "envoy.retry_host_predicates.omit_canary_hosts": "//source/extensions/retry/host/omit_canary_hosts:config", - "envoy.retry_host_predicates.omit_host_metadata": "//source/extensions/retry/host/omit_host_metadata:config", + "envoy.retry_host_predicates.previous_hosts": "//source/extensions/retry/host/previous_hosts:config", + "envoy.retry_host_predicates.omit_canary_hosts": "//source/extensions/retry/host/omit_canary_hosts:config", + "envoy.retry_host_predicates.omit_host_metadata": "//source/extensions/retry/host/omit_host_metadata:config", # # Retry priorities # - - "envoy.retry_priorities.previous_priorities": "//source/extensions/retry/priority/previous_priorities:config", + "envoy.retry_priorities.previous_priorities": "//source/extensions/retry/priority/previous_priorities:config", # # CacheFilter plugins # - "envoy.cache.simple_http_cache": "//source/extensions/filters/http/cache/simple_http_cache:config", + "envoy.cache.simple_http_cache": "//source/extensions/filters/http/cache/simple_http_cache:config", # # Internal redirect predicates # - "envoy.internal_redirect_predicates.allow_listed_routes": "//source/extensions/internal_redirect/allow_listed_routes:config", - "envoy.internal_redirect_predicates.previous_routes": "//source/extensions/internal_redirect/previous_routes:config", - "envoy.internal_redirect_predicates.safe_cross_scheme": "//source/extensions/internal_redirect/safe_cross_scheme:config", + "envoy.internal_redirect_predicates.previous_routes": "//source/extensions/internal_redirect/previous_routes:config", + "envoy.internal_redirect_predicates.safe_cross_scheme": "//source/extensions/internal_redirect/safe_cross_scheme:config", # # Http Upstreams (excepting envoy.upstreams.http.generic which is hard-coded into the build so not registered here) # - - "envoy.upstreams.http.http": "//source/extensions/upstreams/http/http:config", - "envoy.upstreams.http.tcp": "//source/extensions/upstreams/http/tcp:config", + "envoy.upstreams.http.http": "//source/extensions/upstreams/http/http:config", + "envoy.upstreams.http.tcp": "//source/extensions/upstreams/http/tcp:config", # # Watchdog actions # - - "envoy.watchdog.profile_action": "//source/extensions/watchdog/profile_action:config", + "envoy.watchdog.profile_action": "//source/extensions/watchdog/profile_action:config", # # WebAssembly runtimes # - - "envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config", - "envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config", - "envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config", - "envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config", - "envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config", + "envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config", + "envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config", + "envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config", + "envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config", + "envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config", # # Rate limit descriptors # - - "envoy.rate_limit_descriptors.expr": "//source/extensions/rate_limit_descriptors/expr:config", + "envoy.rate_limit_descriptors.expr": "//source/extensions/rate_limit_descriptors/expr:config", # # IO socket # - - "envoy.io_socket.user_space": "//source/extensions/io_socket/user_space:config", + "envoy.io_socket.user_space": "//source/extensions/io_socket/user_space:config", # # TLS peer certification validators # - - "envoy.tls.cert_validator.spiffe": "//source/extensions/transport_sockets/tls/cert_validator/spiffe:config", + "envoy.tls.cert_validator.spiffe": "//source/extensions/transport_sockets/tls/cert_validator/spiffe:config", # # HTTP header formatters # - - "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter", + "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter", # # Original IP detection # - - "envoy.http.original_ip_detection.custom_header": "//source/extensions/http/original_ip_detection/custom_header:config", - "envoy.http.original_ip_detection.xff": "//source/extensions/http/original_ip_detection/xff:config", + "envoy.http.original_ip_detection.custom_header": "//source/extensions/http/original_ip_detection/custom_header:config", + "envoy.http.original_ip_detection.xff": "//source/extensions/http/original_ip_detection/xff:config", # # Quic extensions # - - "envoy.quic.crypto_stream.server.quiche": "//source/extensions/quic/crypto_stream:envoy_quic_default_crypto_server_stream", - "envoy.quic.proof_source.filter_chain": "//source/extensions/quic/proof_source:envoy_quic_default_proof_source", + "envoy.quic.crypto_stream.server.quiche": "//source/extensions/quic/crypto_stream:envoy_quic_default_crypto_server_stream", + "envoy.quic.proof_source.filter_chain": "//source/extensions/quic/proof_source:envoy_quic_default_proof_source", # # Formatter # - - "envoy.formatter.metadata": "//source/extensions/formatter/metadata:config", - "envoy.formatter.req_without_query": "//source/extensions/formatter/req_without_query:config", + "envoy.formatter.metadata": "//source/extensions/formatter/metadata:config", + "envoy.formatter.req_without_query": "//source/extensions/formatter/req_without_query:config", # # Key value store # - - "envoy.key_value.file_based": "//source/extensions/key_value/file_based:config_lib", + "envoy.key_value.file_based": "//source/extensions/key_value/file_based:config_lib", # # RBAC matchers # - - "envoy.rbac.matchers.upstream_ip_port": "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", + "envoy.rbac.matchers.upstream_ip_port": "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", # # DNS Resolver # # c-ares DNS resolver extension is recommended to be enabled to maintain the legacy DNS resolving behavior. - "envoy.network.dns_resolver.cares": "//source/extensions/network/dns_resolver/cares:config", + "envoy.network.dns_resolver.cares": "//source/extensions/network/dns_resolver/cares:config", # apple DNS resolver extension is only needed in MacOS build plus one want to use apple library for DNS resolving. - "envoy.network.dns_resolver.apple": "//source/extensions/network/dns_resolver/apple:config", + "envoy.network.dns_resolver.apple": "//source/extensions/network/dns_resolver/apple:config", } # These can be changed to ["//visibility:public"], for downstream builds which From de6b7f2a216d799016c5b617c2d91f268f491658 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 03:10:27 +0000 Subject: [PATCH 03/52] Fix format Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 8a0b05433b09..3e2d97165e1f 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -227,7 +227,7 @@ TEST_P(ProxyProtocolTest, DetectNoProxyProtocol) { envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_detect_proxy_protocol(true); connect(true, &proto_config); - + write("more data"); expectData("more data"); From 409f94c7b34165b1dec6aeadd6c36606d1f9ee80 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 03:15:23 +0000 Subject: [PATCH 04/52] Fix merge Signed-off-by: Kevin Dorosh --- source/extensions/extensions_build_config.bzl | 306 ++++++++++-------- 1 file changed, 166 insertions(+), 140 deletions(-) diff --git a/source/extensions/extensions_build_config.bzl b/source/extensions/extensions_build_config.bzl index 73fd6008166b..cd6a62d3d5c4 100644 --- a/source/extensions/extensions_build_config.bzl +++ b/source/extensions/extensions_build_config.bzl @@ -3,292 +3,318 @@ EXTENSIONS = { # # Access loggers # - "envoy.access_loggers.file": "//source/extensions/access_loggers/file:config", - "envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config", - "envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config", - "envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config", - "envoy.access_loggers.stream": "//source/extensions/access_loggers/stream:config", - "envoy.access_loggers.wasm": "//source/extensions/access_loggers/wasm:config", + + "envoy.access_loggers.file": "//source/extensions/access_loggers/file:config", + "envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config", + "envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config", + "envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config", + "envoy.access_loggers.stream": "//source/extensions/access_loggers/stream:config", + "envoy.access_loggers.wasm": "//source/extensions/access_loggers/wasm:config", # # Clusters # - "envoy.clusters.aggregate": "//source/extensions/clusters/aggregate:cluster", - "envoy.clusters.dynamic_forward_proxy": "//source/extensions/clusters/dynamic_forward_proxy:cluster", - "envoy.clusters.redis": "//source/extensions/clusters/redis:redis_cluster", + + "envoy.clusters.aggregate": "//source/extensions/clusters/aggregate:cluster", + "envoy.clusters.dynamic_forward_proxy": "//source/extensions/clusters/dynamic_forward_proxy:cluster", + "envoy.clusters.redis": "//source/extensions/clusters/redis:redis_cluster", # # Compression # - "envoy.compression.gzip.compressor": "//source/extensions/compression/gzip/compressor:config", - "envoy.compression.gzip.decompressor": "//source/extensions/compression/gzip/decompressor:config", - "envoy.compression.brotli.compressor": "//source/extensions/compression/brotli/compressor:config", - "envoy.compression.brotli.decompressor": "//source/extensions/compression/brotli/decompressor:config", + + "envoy.compression.gzip.compressor": "//source/extensions/compression/gzip/compressor:config", + "envoy.compression.gzip.decompressor": "//source/extensions/compression/gzip/decompressor:config", + "envoy.compression.brotli.compressor": "//source/extensions/compression/brotli/compressor:config", + "envoy.compression.brotli.decompressor": "//source/extensions/compression/brotli/decompressor:config", # # gRPC Credentials Plugins # - "envoy.grpc_credentials.file_based_metadata": "//source/extensions/grpc_credentials/file_based_metadata:config", - "envoy.grpc_credentials.aws_iam": "//source/extensions/grpc_credentials/aws_iam:config", + + "envoy.grpc_credentials.file_based_metadata": "//source/extensions/grpc_credentials/file_based_metadata:config", + "envoy.grpc_credentials.aws_iam": "//source/extensions/grpc_credentials/aws_iam:config", # # WASM # - "envoy.bootstrap.wasm": "//source/extensions/bootstrap/wasm:config", + + "envoy.bootstrap.wasm": "//source/extensions/bootstrap/wasm:config", # # Health checkers # - "envoy.health_checkers.redis": "//source/extensions/health_checkers/redis:config", + + "envoy.health_checkers.redis": "//source/extensions/health_checkers/redis:config", # # Input Matchers # - "envoy.matching.input_matchers.consistent_hashing": "//source/extensions/matching/input_matchers/consistent_hashing:config", - "envoy.matching.input_matchers.ip": "//source/extensions/matching/input_matchers/ip:config", + + "envoy.matching.input_matchers.consistent_hashing": "//source/extensions/matching/input_matchers/consistent_hashing:config", + "envoy.matching.input_matchers.ip": "//source/extensions/matching/input_matchers/ip:config", # # Generic Inputs # - "envoy.matching.common_inputs.environment_variable": "//source/extensions/matching/common_inputs/environment_variable:config", + + "envoy.matching.common_inputs.environment_variable": "//source/extensions/matching/common_inputs/environment_variable:config", # # HTTP filters # - "envoy.filters.http.adaptive_concurrency": "//source/extensions/filters/http/adaptive_concurrency:config", - "envoy.filters.http.admission_control": "//source/extensions/filters/http/admission_control:config", - "envoy.filters.http.alternate_protocols_cache": "//source/extensions/filters/http/alternate_protocols_cache:config", - "envoy.filters.http.aws_lambda": "//source/extensions/filters/http/aws_lambda:config", - "envoy.filters.http.aws_request_signing": "//source/extensions/filters/http/aws_request_signing:config", - "envoy.filters.http.bandwidth_limit": "//source/extensions/filters/http/bandwidth_limit:config", - "envoy.filters.http.buffer": "//source/extensions/filters/http/buffer:config", - "envoy.filters.http.cache": "//source/extensions/filters/http/cache:config", - "envoy.filters.http.cdn_loop": "//source/extensions/filters/http/cdn_loop:config", - "envoy.filters.http.compressor": "//source/extensions/filters/http/compressor:config", - "envoy.filters.http.cors": "//source/extensions/filters/http/cors:config", - "envoy.filters.http.composite": "//source/extensions/filters/http/composite:config", - "envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config", - "envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config", - "envoy.filters.http.dynamic_forward_proxy": "//source/extensions/filters/http/dynamic_forward_proxy:config", - "envoy.filters.http.dynamo": "//source/extensions/filters/http/dynamo:config", - "envoy.filters.http.ext_authz": "//source/extensions/filters/http/ext_authz:config", - "envoy.filters.http.ext_proc": "//source/extensions/filters/http/ext_proc:config", - "envoy.filters.http.fault": "//source/extensions/filters/http/fault:config", - "envoy.filters.http.grpc_http1_bridge": "//source/extensions/filters/http/grpc_http1_bridge:config", - "envoy.filters.http.grpc_http1_reverse_bridge": "//source/extensions/filters/http/grpc_http1_reverse_bridge:config", - "envoy.filters.http.grpc_json_transcoder": "//source/extensions/filters/http/grpc_json_transcoder:config", - "envoy.filters.http.grpc_stats": "//source/extensions/filters/http/grpc_stats:config", - "envoy.filters.http.grpc_web": "//source/extensions/filters/http/grpc_web:config", - "envoy.filters.http.header_to_metadata": "//source/extensions/filters/http/header_to_metadata:config", - "envoy.filters.http.health_check": "//source/extensions/filters/http/health_check:config", - "envoy.filters.http.ip_tagging": "//source/extensions/filters/http/ip_tagging:config", - "envoy.filters.http.jwt_authn": "//source/extensions/filters/http/jwt_authn:config", + + "envoy.filters.http.adaptive_concurrency": "//source/extensions/filters/http/adaptive_concurrency:config", + "envoy.filters.http.admission_control": "//source/extensions/filters/http/admission_control:config", + "envoy.filters.http.alternate_protocols_cache": "//source/extensions/filters/http/alternate_protocols_cache:config", + "envoy.filters.http.aws_lambda": "//source/extensions/filters/http/aws_lambda:config", + "envoy.filters.http.aws_request_signing": "//source/extensions/filters/http/aws_request_signing:config", + "envoy.filters.http.bandwidth_limit": "//source/extensions/filters/http/bandwidth_limit:config", + "envoy.filters.http.buffer": "//source/extensions/filters/http/buffer:config", + "envoy.filters.http.cache": "//source/extensions/filters/http/cache:config", + "envoy.filters.http.cdn_loop": "//source/extensions/filters/http/cdn_loop:config", + "envoy.filters.http.compressor": "//source/extensions/filters/http/compressor:config", + "envoy.filters.http.cors": "//source/extensions/filters/http/cors:config", + "envoy.filters.http.composite": "//source/extensions/filters/http/composite:config", + "envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config", + "envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config", + "envoy.filters.http.dynamic_forward_proxy": "//source/extensions/filters/http/dynamic_forward_proxy:config", + "envoy.filters.http.dynamo": "//source/extensions/filters/http/dynamo:config", + "envoy.filters.http.ext_authz": "//source/extensions/filters/http/ext_authz:config", + "envoy.filters.http.ext_proc": "//source/extensions/filters/http/ext_proc:config", + "envoy.filters.http.fault": "//source/extensions/filters/http/fault:config", + "envoy.filters.http.grpc_http1_bridge": "//source/extensions/filters/http/grpc_http1_bridge:config", + "envoy.filters.http.grpc_http1_reverse_bridge": "//source/extensions/filters/http/grpc_http1_reverse_bridge:config", + "envoy.filters.http.grpc_json_transcoder": "//source/extensions/filters/http/grpc_json_transcoder:config", + "envoy.filters.http.grpc_stats": "//source/extensions/filters/http/grpc_stats:config", + "envoy.filters.http.grpc_web": "//source/extensions/filters/http/grpc_web:config", + "envoy.filters.http.header_to_metadata": "//source/extensions/filters/http/header_to_metadata:config", + "envoy.filters.http.health_check": "//source/extensions/filters/http/health_check:config", + "envoy.filters.http.ip_tagging": "//source/extensions/filters/http/ip_tagging:config", + "envoy.filters.http.jwt_authn": "//source/extensions/filters/http/jwt_authn:config", # Disabled by default - "envoy.filters.http.kill_request": "//source/extensions/filters/http/kill_request:kill_request_config", - "envoy.filters.http.local_ratelimit": "//source/extensions/filters/http/local_ratelimit:config", - "envoy.filters.http.lua": "//source/extensions/filters/http/lua:config", - "envoy.filters.http.oauth2": "//source/extensions/filters/http/oauth2:config", - "envoy.filters.http.on_demand": "//source/extensions/filters/http/on_demand:config", - "envoy.filters.http.original_src": "//source/extensions/filters/http/original_src:config", - "envoy.filters.http.ratelimit": "//source/extensions/filters/http/ratelimit:config", - "envoy.filters.http.rbac": "//source/extensions/filters/http/rbac:config", - "envoy.filters.http.router": "//source/extensions/filters/http/router:config", - "envoy.filters.http.set_metadata": "//source/extensions/filters/http/set_metadata:config", - "envoy.filters.http.tap": "//source/extensions/filters/http/tap:config", - "envoy.filters.http.wasm": "//source/extensions/filters/http/wasm:config", + "envoy.filters.http.kill_request": "//source/extensions/filters/http/kill_request:kill_request_config", + "envoy.filters.http.local_ratelimit": "//source/extensions/filters/http/local_ratelimit:config", + "envoy.filters.http.lua": "//source/extensions/filters/http/lua:config", + "envoy.filters.http.oauth2": "//source/extensions/filters/http/oauth2:config", + "envoy.filters.http.on_demand": "//source/extensions/filters/http/on_demand:config", + "envoy.filters.http.original_src": "//source/extensions/filters/http/original_src:config", + "envoy.filters.http.ratelimit": "//source/extensions/filters/http/ratelimit:config", + "envoy.filters.http.rbac": "//source/extensions/filters/http/rbac:config", + "envoy.filters.http.router": "//source/extensions/filters/http/router:config", + "envoy.filters.http.set_metadata": "//source/extensions/filters/http/set_metadata:config", + "envoy.filters.http.tap": "//source/extensions/filters/http/tap:config", + "envoy.filters.http.wasm": "//source/extensions/filters/http/wasm:config", # # Listener filters # - "envoy.filters.listener.http_inspector": "//source/extensions/filters/listener/http_inspector:config", + + "envoy.filters.listener.http_inspector": "//source/extensions/filters/listener/http_inspector:config", # NOTE: The original_dst filter is implicitly loaded if original_dst functionality is # configured on the listener. Do not remove it in that case or configs will fail to load. - "envoy.filters.listener.original_dst": "//source/extensions/filters/listener/original_dst:config", - "envoy.filters.listener.original_src": "//source/extensions/filters/listener/original_src:config", + "envoy.filters.listener.original_dst": "//source/extensions/filters/listener/original_dst:config", + "envoy.filters.listener.original_src": "//source/extensions/filters/listener/original_src:config", # NOTE: The proxy_protocol filter is implicitly loaded if proxy_protocol functionality is # configured on the listener. Do not remove it in that case or configs will fail to load. - "envoy.filters.listener.proxy_protocol": "//source/extensions/filters/listener/proxy_protocol:config", - "envoy.filters.listener.tls_inspector": "//source/extensions/filters/listener/tls_inspector:config", + "envoy.filters.listener.proxy_protocol": "//source/extensions/filters/listener/proxy_protocol:config", + "envoy.filters.listener.tls_inspector": "//source/extensions/filters/listener/tls_inspector:config", # # Network filters # - "envoy.filters.network.client_ssl_auth": "//source/extensions/filters/network/client_ssl_auth:config", - "envoy.filters.network.connection_limit": "//source/extensions/filters/network/connection_limit:config", - "envoy.filters.network.direct_response": "//source/extensions/filters/network/direct_response:config", - "envoy.filters.network.dubbo_proxy": "//source/extensions/filters/network/dubbo_proxy:config", - "envoy.filters.network.echo": "//source/extensions/filters/network/echo:config", - "envoy.filters.network.ext_authz": "//source/extensions/filters/network/ext_authz:config", - "envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config", - "envoy.filters.network.local_ratelimit": "//source/extensions/filters/network/local_ratelimit:config", - "envoy.filters.network.mongo_proxy": "//source/extensions/filters/network/mongo_proxy:config", - "envoy.filters.network.ratelimit": "//source/extensions/filters/network/ratelimit:config", - "envoy.filters.network.rbac": "//source/extensions/filters/network/rbac:config", - "envoy.filters.network.redis_proxy": "//source/extensions/filters/network/redis_proxy:config", - "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", - "envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", - "envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", - "envoy.filters.network.sni_dynamic_forward_proxy": "//source/extensions/filters/network/sni_dynamic_forward_proxy:config", - "envoy.filters.network.wasm": "//source/extensions/filters/network/wasm:config", - "envoy.filters.network.zookeeper_proxy": "//source/extensions/filters/network/zookeeper_proxy:config", + + "envoy.filters.network.client_ssl_auth": "//source/extensions/filters/network/client_ssl_auth:config", + "envoy.filters.network.connection_limit": "//source/extensions/filters/network/connection_limit:config", + "envoy.filters.network.direct_response": "//source/extensions/filters/network/direct_response:config", + "envoy.filters.network.dubbo_proxy": "//source/extensions/filters/network/dubbo_proxy:config", + "envoy.filters.network.echo": "//source/extensions/filters/network/echo:config", + "envoy.filters.network.ext_authz": "//source/extensions/filters/network/ext_authz:config", + "envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config", + "envoy.filters.network.local_ratelimit": "//source/extensions/filters/network/local_ratelimit:config", + "envoy.filters.network.mongo_proxy": "//source/extensions/filters/network/mongo_proxy:config", + "envoy.filters.network.ratelimit": "//source/extensions/filters/network/ratelimit:config", + "envoy.filters.network.rbac": "//source/extensions/filters/network/rbac:config", + "envoy.filters.network.redis_proxy": "//source/extensions/filters/network/redis_proxy:config", + "envoy.filters.network.tcp_proxy": "//source/extensions/filters/network/tcp_proxy:config", + "envoy.filters.network.thrift_proxy": "//source/extensions/filters/network/thrift_proxy:config", + "envoy.filters.network.sni_cluster": "//source/extensions/filters/network/sni_cluster:config", + "envoy.filters.network.sni_dynamic_forward_proxy": "//source/extensions/filters/network/sni_dynamic_forward_proxy:config", + "envoy.filters.network.wasm": "//source/extensions/filters/network/wasm:config", + "envoy.filters.network.zookeeper_proxy": "//source/extensions/filters/network/zookeeper_proxy:config", # # UDP filters # - "envoy.filters.udp_listener.dns_filter": "//source/extensions/filters/udp/dns_filter:config", - "envoy.filters.udp_listener.udp_proxy": "//source/extensions/filters/udp/udp_proxy:config", + + "envoy.filters.udp_listener.dns_filter": "//source/extensions/filters/udp/dns_filter:config", + "envoy.filters.udp_listener.udp_proxy": "//source/extensions/filters/udp/udp_proxy:config", # # Resource monitors # - "envoy.resource_monitors.fixed_heap": "//source/extensions/resource_monitors/fixed_heap:config", - "envoy.resource_monitors.injected_resource": "//source/extensions/resource_monitors/injected_resource:config", + + "envoy.resource_monitors.fixed_heap": "//source/extensions/resource_monitors/fixed_heap:config", + "envoy.resource_monitors.injected_resource": "//source/extensions/resource_monitors/injected_resource:config", # # Stat sinks # - "envoy.stat_sinks.dog_statsd": "//source/extensions/stat_sinks/dog_statsd:config", - "envoy.stat_sinks.graphite_statsd": "//source/extensions/stat_sinks/graphite_statsd:config", - "envoy.stat_sinks.hystrix": "//source/extensions/stat_sinks/hystrix:config", - "envoy.stat_sinks.metrics_service": "//source/extensions/stat_sinks/metrics_service:config", - "envoy.stat_sinks.statsd": "//source/extensions/stat_sinks/statsd:config", - "envoy.stat_sinks.wasm": "//source/extensions/stat_sinks/wasm:config", + + "envoy.stat_sinks.dog_statsd": "//source/extensions/stat_sinks/dog_statsd:config", + "envoy.stat_sinks.graphite_statsd": "//source/extensions/stat_sinks/graphite_statsd:config", + "envoy.stat_sinks.hystrix": "//source/extensions/stat_sinks/hystrix:config", + "envoy.stat_sinks.metrics_service": "//source/extensions/stat_sinks/metrics_service:config", + "envoy.stat_sinks.statsd": "//source/extensions/stat_sinks/statsd:config", + "envoy.stat_sinks.wasm": "//source/extensions/stat_sinks/wasm:config", # # Thrift filters # -<<<<<<< HEAD - "envoy.filters.thrift.router": "//source/extensions/filters/network/thrift_proxy/router:config", - "envoy.filters.thrift.ratelimit": "//source/extensions/filters/network/thrift_proxy/filters/ratelimit:config", -======= "envoy.filters.thrift.router": "//source/extensions/filters/network/thrift_proxy/router:config", "envoy.filters.thrift.header_to_metadata": "//source/extensions/filters/network/thrift_proxy/filters/header_to_metadata:config", "envoy.filters.thrift.ratelimit": "//source/extensions/filters/network/thrift_proxy/filters/ratelimit:config", ->>>>>>> main # # Tracers # - "envoy.tracers.dynamic_ot": "//source/extensions/tracers/dynamic_ot:config", - "envoy.tracers.lightstep": "//source/extensions/tracers/lightstep:config", - "envoy.tracers.datadog": "//source/extensions/tracers/datadog:config", - "envoy.tracers.zipkin": "//source/extensions/tracers/zipkin:config", - "envoy.tracers.opencensus": "//source/extensions/tracers/opencensus:config", - "envoy.tracers.xray": "//source/extensions/tracers/xray:config", - "envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config", + + "envoy.tracers.dynamic_ot": "//source/extensions/tracers/dynamic_ot:config", + "envoy.tracers.lightstep": "//source/extensions/tracers/lightstep:config", + "envoy.tracers.datadog": "//source/extensions/tracers/datadog:config", + "envoy.tracers.zipkin": "//source/extensions/tracers/zipkin:config", + "envoy.tracers.opencensus": "//source/extensions/tracers/opencensus:config", + "envoy.tracers.xray": "//source/extensions/tracers/xray:config", + "envoy.tracers.skywalking": "//source/extensions/tracers/skywalking:config", # # Transport sockets # - "envoy.transport_sockets.alts": "//source/extensions/transport_sockets/alts:config", - "envoy.transport_sockets.upstream_proxy_protocol": "//source/extensions/transport_sockets/proxy_protocol:upstream_config", - "envoy.transport_sockets.raw_buffer": "//source/extensions/transport_sockets/raw_buffer:config", - "envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config", - "envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config", - "envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config", + + "envoy.transport_sockets.alts": "//source/extensions/transport_sockets/alts:config", + "envoy.transport_sockets.upstream_proxy_protocol": "//source/extensions/transport_sockets/proxy_protocol:upstream_config", + "envoy.transport_sockets.raw_buffer": "//source/extensions/transport_sockets/raw_buffer:config", + "envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config", + "envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config", + "envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config", # # Retry host predicates # - "envoy.retry_host_predicates.previous_hosts": "//source/extensions/retry/host/previous_hosts:config", - "envoy.retry_host_predicates.omit_canary_hosts": "//source/extensions/retry/host/omit_canary_hosts:config", - "envoy.retry_host_predicates.omit_host_metadata": "//source/extensions/retry/host/omit_host_metadata:config", + + "envoy.retry_host_predicates.previous_hosts": "//source/extensions/retry/host/previous_hosts:config", + "envoy.retry_host_predicates.omit_canary_hosts": "//source/extensions/retry/host/omit_canary_hosts:config", + "envoy.retry_host_predicates.omit_host_metadata": "//source/extensions/retry/host/omit_host_metadata:config", # # Retry priorities # - "envoy.retry_priorities.previous_priorities": "//source/extensions/retry/priority/previous_priorities:config", + + "envoy.retry_priorities.previous_priorities": "//source/extensions/retry/priority/previous_priorities:config", # # CacheFilter plugins # - "envoy.cache.simple_http_cache": "//source/extensions/filters/http/cache/simple_http_cache:config", + "envoy.cache.simple_http_cache": "//source/extensions/filters/http/cache/simple_http_cache:config", # # Internal redirect predicates # + "envoy.internal_redirect_predicates.allow_listed_routes": "//source/extensions/internal_redirect/allow_listed_routes:config", - "envoy.internal_redirect_predicates.previous_routes": "//source/extensions/internal_redirect/previous_routes:config", - "envoy.internal_redirect_predicates.safe_cross_scheme": "//source/extensions/internal_redirect/safe_cross_scheme:config", + "envoy.internal_redirect_predicates.previous_routes": "//source/extensions/internal_redirect/previous_routes:config", + "envoy.internal_redirect_predicates.safe_cross_scheme": "//source/extensions/internal_redirect/safe_cross_scheme:config", # # Http Upstreams (excepting envoy.upstreams.http.generic which is hard-coded into the build so not registered here) # - "envoy.upstreams.http.http": "//source/extensions/upstreams/http/http:config", - "envoy.upstreams.http.tcp": "//source/extensions/upstreams/http/tcp:config", + + "envoy.upstreams.http.http": "//source/extensions/upstreams/http/http:config", + "envoy.upstreams.http.tcp": "//source/extensions/upstreams/http/tcp:config", # # Watchdog actions # - "envoy.watchdog.profile_action": "//source/extensions/watchdog/profile_action:config", + + "envoy.watchdog.profile_action": "//source/extensions/watchdog/profile_action:config", # # WebAssembly runtimes # - "envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config", - "envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config", - "envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config", - "envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config", - "envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config", + + "envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config", + "envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config", + "envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config", + "envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config", + "envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config", # # Rate limit descriptors # - "envoy.rate_limit_descriptors.expr": "//source/extensions/rate_limit_descriptors/expr:config", + + "envoy.rate_limit_descriptors.expr": "//source/extensions/rate_limit_descriptors/expr:config", # # IO socket # - "envoy.io_socket.user_space": "//source/extensions/io_socket/user_space:config", + + "envoy.io_socket.user_space": "//source/extensions/io_socket/user_space:config", # # TLS peer certification validators # - "envoy.tls.cert_validator.spiffe": "//source/extensions/transport_sockets/tls/cert_validator/spiffe:config", + + "envoy.tls.cert_validator.spiffe": "//source/extensions/transport_sockets/tls/cert_validator/spiffe:config", # # HTTP header formatters # - "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter", + + "envoy.http.stateful_header_formatters.preserve_case": "//source/extensions/http/header_formatters/preserve_case:preserve_case_formatter", # # Original IP detection # - "envoy.http.original_ip_detection.custom_header": "//source/extensions/http/original_ip_detection/custom_header:config", - "envoy.http.original_ip_detection.xff": "//source/extensions/http/original_ip_detection/xff:config", + + "envoy.http.original_ip_detection.custom_header": "//source/extensions/http/original_ip_detection/custom_header:config", + "envoy.http.original_ip_detection.xff": "//source/extensions/http/original_ip_detection/xff:config", # # Quic extensions # - "envoy.quic.crypto_stream.server.quiche": "//source/extensions/quic/crypto_stream:envoy_quic_default_crypto_server_stream", - "envoy.quic.proof_source.filter_chain": "//source/extensions/quic/proof_source:envoy_quic_default_proof_source", + + "envoy.quic.crypto_stream.server.quiche": "//source/extensions/quic/crypto_stream:envoy_quic_default_crypto_server_stream", + "envoy.quic.proof_source.filter_chain": "//source/extensions/quic/proof_source:envoy_quic_default_proof_source", # # Formatter # - "envoy.formatter.metadata": "//source/extensions/formatter/metadata:config", - "envoy.formatter.req_without_query": "//source/extensions/formatter/req_without_query:config", + + "envoy.formatter.metadata": "//source/extensions/formatter/metadata:config", + "envoy.formatter.req_without_query": "//source/extensions/formatter/req_without_query:config", # # Key value store # - "envoy.key_value.file_based": "//source/extensions/key_value/file_based:config_lib", + + "envoy.key_value.file_based": "//source/extensions/key_value/file_based:config_lib", # # RBAC matchers # - "envoy.rbac.matchers.upstream_ip_port": "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", + + "envoy.rbac.matchers.upstream_ip_port": "//source/extensions/filters/common/rbac/matchers:upstream_ip_port_lib", # # DNS Resolver # # c-ares DNS resolver extension is recommended to be enabled to maintain the legacy DNS resolving behavior. - "envoy.network.dns_resolver.cares": "//source/extensions/network/dns_resolver/cares:config", + "envoy.network.dns_resolver.cares": "//source/extensions/network/dns_resolver/cares:config", # apple DNS resolver extension is only needed in MacOS build plus one want to use apple library for DNS resolving. - "envoy.network.dns_resolver.apple": "//source/extensions/network/dns_resolver/apple:config", + "envoy.network.dns_resolver.apple": "//source/extensions/network/dns_resolver/apple:config", } # These can be changed to ["//visibility:public"], for downstream builds which From ae19e1f20c34873b533fdf3da04e53e11451888f Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 03:21:28 +0000 Subject: [PATCH 05/52] Clearer error message Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 12b228c84719..4329c35d618d 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -444,7 +444,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->detectProxyProtocol()) { - ENVOY_LOG(debug, "need more bytes to read proxy protocol"); + ENVOY_LOG(debug, "need more bytes to detect proxy protocol header"); return ReadOrParseState::SkipFilterError; } From ed4fb5468772e1acd744b7d188813fca188d37dc Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 14:46:28 +0000 Subject: [PATCH 06/52] Add unit test for partial v1 protocol Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 6 +++-- .../proxy_protocol/proxy_protocol_test.cc | 26 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 4329c35d618d..5fe69650f77f 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -444,8 +444,10 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->detectProxyProtocol()) { - ENVOY_LOG(debug, "need more bytes to detect proxy protocol header"); - return ReadOrParseState::SkipFilterError; + if (nread < PROXY_PROTO_V1_SIGNATURE_LEN || memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + ENVOY_LOG(debug, "need more bytes to detect proxy protocol header"); + return ReadOrParseState::SkipFilterError; + } } if (buf_off_ + nread >= PROXY_PROTO_V2_HEADER_LEN) { diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 3e2d97165e1f..56ea524bee67 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -222,7 +222,7 @@ TEST_P(ProxyProtocolTest, V1Basic) { disconnect(); } -TEST_P(ProxyProtocolTest, DetectNoProxyProtocol) { +TEST_P(ProxyProtocolTest, AllowNoProxyProtocol) { envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_detect_proxy_protocol(true); @@ -940,6 +940,30 @@ TEST_P(ProxyProtocolTest, PartialRead) { disconnect(); } +TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { + + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_detect_proxy_protocol(true); + connect(true, &proto_config); + + write("PROXY TCP4"); + + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + write(" 254.254.2"); + write("54.254 1.2"); + write(".3.4 65535"); + write(" 1234\r\n..."); + + expectData("..."); + + EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), + "254.254.254.254"); + EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); + + disconnect(); +} + TEST_P(ProxyProtocolTest, V2PartialRead) { // A well-formed ipv4/tcp header, delivered with part of the signature, // part of the header, rest of header + body From bcc7aef0b4ca204ff6d318661825a4475092d41b Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 14:57:26 +0000 Subject: [PATCH 07/52] Rename new field to Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/v3/proxy_protocol.proto | 7 ++++--- .../filters/listener/proxy_protocol/proxy_protocol.cc | 8 ++++---- .../filters/listener/proxy_protocol/proxy_protocol.h | 8 ++++---- .../listener/proxy_protocol/proxy_protocol_test.cc | 4 ++-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index 136792e89e85..76f532c25559 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -42,7 +42,8 @@ message ProxyProtocol { repeated Rule rules = 1; // NOTICE: only enable if ALL traffic to the listener comes from a trusted source. - // Defaults to false. If true, attempt to detect proxy protocol if present, and allow - // requests through if proxy protocol is not used on the connection. - bool detect_proxy_protocol = 2; + // Defaults to false. If true, allow requests through that don't use proxy protocol. + // For more information on the security implications of this feature, see + // https://www.haproxy.org/download/1.9/doc/proxy-protocol.txt + bool allow_requests_without_proxy_protocol = 2; } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 5fe69650f77f..93d11c2b4886 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -47,7 +47,7 @@ Config::Config( Stats::Scope& scope, const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config) : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} { - detect_proxy_protocol_ = proto_config.detect_proxy_protocol(); + allow_requests_without_proxy_protocol_ = proto_config.allow_requests_without_proxy_protocol(); for (const auto& rule : proto_config.rules()) { tlv_types_[0xFF & rule.tlv_type()] = rule.on_tlv_present(); } @@ -64,7 +64,7 @@ const KeyValuePair* Config::isTlvTypeNeeded(uint8_t type) const { size_t Config::numberOfNeededTlvTypes() const { return tlv_types_.size(); } -bool Config::detectProxyProtocol() const { return detect_proxy_protocol_; } +bool Config::allowRequestsWithoutProxyProtocol() const { return allow_requests_without_proxy_protocol_; } Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { ENVOY_LOG(debug, "proxy_protocol: new connection accepted"); @@ -443,9 +443,9 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (nread < 1) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; - } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->detectProxyProtocol()) { + } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->allowRequestsWithoutProxyProtocol()) { if (nread < PROXY_PROTO_V1_SIGNATURE_LEN || memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { - ENVOY_LOG(debug, "need more bytes to detect proxy protocol header"); + ENVOY_LOG(debug, "need more bytes to determine if we have a proxy protocol header"); return ReadOrParseState::SkipFilterError; } } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index 0b6f9e76a6c1..e5c3ea021819 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -61,14 +61,14 @@ class Config : public Logger::Loggable { size_t numberOfNeededTlvTypes() const; /** - * Filter configuration that determines if we should pass-through failed proxy protocol - * requests. Should only be configured to true for trusted downstreams. + * Filter configuration that determines if we should pass-through requests without + * proxy protocol. Should only be configured to true for trusted downstreams. */ - bool detectProxyProtocol() const; + bool allowRequestsWithoutProxyProtocol() const; private: absl::flat_hash_map tlv_types_; - bool detect_proxy_protocol_{}; + bool allow_requests_without_proxy_protocol_{}; }; using ConfigSharedPtr = std::shared_ptr; diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 56ea524bee67..43a63fc64a54 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -225,7 +225,7 @@ TEST_P(ProxyProtocolTest, V1Basic) { TEST_P(ProxyProtocolTest, AllowNoProxyProtocol) { envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; - proto_config.set_detect_proxy_protocol(true); + proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); write("more data"); @@ -943,7 +943,7 @@ TEST_P(ProxyProtocolTest, PartialRead) { TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; - proto_config.set_detect_proxy_protocol(true); + proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); write("PROXY TCP4"); From 8c87bcd75a89ba47045d3cf72b5c114ecf8923dc Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:00:47 +0000 Subject: [PATCH 08/52] Fix format Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 93d11c2b4886..a2b690ec5b32 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -64,7 +64,9 @@ const KeyValuePair* Config::isTlvTypeNeeded(uint8_t type) const { size_t Config::numberOfNeededTlvTypes() const { return tlv_types_.size(); } -bool Config::allowRequestsWithoutProxyProtocol() const { return allow_requests_without_proxy_protocol_; } +bool Config::allowRequestsWithoutProxyProtocol() const { + return allow_requests_without_proxy_protocol_; +} Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { ENVOY_LOG(debug, "proxy_protocol: new connection accepted"); @@ -443,8 +445,10 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (nread < 1) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; - } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->allowRequestsWithoutProxyProtocol()) { - if (nread < PROXY_PROTO_V1_SIGNATURE_LEN || memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + } else if (nread < PROXY_PROTO_V2_HEADER_LEN && + config_.get()->allowRequestsWithoutProxyProtocol()) { + if (nread < PROXY_PROTO_V1_SIGNATURE_LEN || + memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { ENVOY_LOG(debug, "need more bytes to determine if we have a proxy protocol header"); return ReadOrParseState::SkipFilterError; } From 4aa520e57dd33a53903dafe3419b69da53e96663 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:06:10 +0000 Subject: [PATCH 09/52] Minor tweak to version history note Signed-off-by: Kevin Dorosh --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 48f407976792..cbe73afd8cb4 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -65,7 +65,7 @@ New Features * listener: added support for :ref:`MPTCP ` (multipath TCP). * oauth filter: added :ref:`cookie_names ` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter. * oauth filter: setting IdToken and RefreshToken cookies if they are provided by Identity provider along with AccessToken. -* proxy protocol: added support for auto-detecting proxy protocol on the listener from trusted downstreams as an opt-in flag. +* proxy protocol: added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. * tcp: added a :ref:`FilterState ` :ref:`hash policy `, used by :ref:`TCP proxy ` to allow hashing load balancer algorithms to hash on objects in filter state. * tcp_proxy: added support to populate upstream http connect header values from stream info. * thrift_proxy: add header to metadata filter for turning headers into dynamic metadata. From 91c9a1b8571e6056de9292ff4470f51020213732 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:20:18 +0000 Subject: [PATCH 10/52] Use attention clause in API comment Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/v3/proxy_protocol.proto | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index 76f532c25559..ecad2145fadf 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -41,9 +41,13 @@ message ProxyProtocol { // The list of rules to apply to requests. repeated Rule rules = 1; - // NOTICE: only enable if ALL traffic to the listener comes from a trusted source. - // Defaults to false. If true, allow requests through that don't use proxy protocol. - // For more information on the security implications of this feature, see - // https://www.haproxy.org/download/1.9/doc/proxy-protocol.txt + // Allow requests through that don't use proxy protocol. Defaults to false. + // + // .. attention:: + // + // Only enable if ALL traffic to the listener comes from a trusted source. + // For more information on the security implications of this feature, see + // https://www.haproxy.org/download/1.9/doc/proxy-protocol.txt + // bool allow_requests_without_proxy_protocol = 2; } From c45ecc77d2bce6f794ac104056d4555cdbb5c221 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:24:37 +0000 Subject: [PATCH 11/52] Set in ctor initialization list Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index a2b690ec5b32..badc09316e6b 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -46,8 +46,8 @@ namespace ProxyProtocol { Config::Config( Stats::Scope& scope, const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config) - : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} { - allow_requests_without_proxy_protocol_ = proto_config.allow_requests_without_proxy_protocol(); + : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} + , allow_requests_without_proxy_protocol_(proto_config.allow_requests_without_proxy_protocol()) { for (const auto& rule : proto_config.rules()) { tlv_types_[0xFF & rule.tlv_type()] = rule.on_tlv_present(); } From 4259f1ae4d2bef34b4b80daa48fb4f3516165ad7 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:25:59 +0000 Subject: [PATCH 12/52] Add comment to clarify test Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 43a63fc64a54..931ad1920fd8 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -223,7 +223,7 @@ TEST_P(ProxyProtocolTest, V1Basic) { } TEST_P(ProxyProtocolTest, AllowNoProxyProtocol) { - + // allows request through even though it doesn't use proxy protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); From 2b4a89a9fb037465fbd322a116e140d54837ea2e Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:28:26 +0000 Subject: [PATCH 13/52] Return value was only used in one of two return paths, tiny refactor Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 6 +++--- .../filters/listener/proxy_protocol/proxy_protocol.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index badc09316e6b..26c97032face 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -82,11 +82,10 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { return Network::FilterStatus::StopIteration; } -ReadOrParseState Filter::resetAndContinue(Network::IoHandle& io_handle) { +void Filter::resetAndContinue(Network::IoHandle& io_handle) { // Release the file event so that we do not interfere with the connection read events. io_handle.resetFileEvents(); cb_->continueFilterChain(true); - return ReadOrParseState::Done; } void Filter::onRead() { @@ -149,7 +148,8 @@ ReadOrParseState Filter::onReadWorker() { proxy_protocol_header_.value().remote_address_); } - return resetAndContinue(socket.ioHandle()); + resetAndContinue(socket.ioHandle()); + return ReadOrParseState::Done; } absl::optional Filter::lenV2Address(char* buf) { diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index e5c3ea021819..42c7c8c558b7 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -125,7 +125,7 @@ class Filter : public Network::ListenerFilter, Logger::Loggable lenV2Address(char* buf); - ReadOrParseState resetAndContinue(Network::IoHandle& io_handle); + void resetAndContinue(Network::IoHandle& io_handle); Network::ListenerFilterCallbacks* cb_{}; From f105c3dae4fff85a7d0b699a410435ec06d7ae0a Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 15:29:28 +0000 Subject: [PATCH 14/52] Fix format Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 26c97032face..29cb335e58e8 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -46,8 +46,8 @@ namespace ProxyProtocol { Config::Config( Stats::Scope& scope, const envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol& proto_config) - : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))} - , allow_requests_without_proxy_protocol_(proto_config.allow_requests_without_proxy_protocol()) { + : stats_{ALL_PROXY_PROTOCOL_STATS(POOL_COUNTER(scope))}, + allow_requests_without_proxy_protocol_(proto_config.allow_requests_without_proxy_protocol()) { for (const auto& rule : proto_config.rules()) { tlv_types_[0xFF & rule.tlv_type()] = rule.on_tlv_present(); } From 6a0d49bc4b1bfaca07ac3132243f2d42dfff5fb7 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 17:03:50 +0000 Subject: [PATCH 15/52] Test case with partial signature, just char Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 14 ++++++++--- .../proxy_protocol/proxy_protocol_test.cc | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 29cb335e58e8..b43bb1df5e23 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -447,9 +447,17 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { return ReadOrParseState::Error; } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->allowRequestsWithoutProxyProtocol()) { - if (nread < PROXY_PROTO_V1_SIGNATURE_LEN || - memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { - ENVOY_LOG(debug, "need more bytes to determine if we have a proxy protocol header"); + ssize_t to_compare_v1 = buf_off_ + nread; + if (to_compare_v1 > PROXY_PROTO_V1_SIGNATURE_LEN) { + to_compare_v1 = PROXY_PROTO_V1_SIGNATURE_LEN; + } + ssize_t to_compare_v2 = buf_off_ + nread; + if (to_compare_v2 > PROXY_PROTO_V2_SIGNATURE_LEN) { + to_compare_v2 = PROXY_PROTO_V2_SIGNATURE_LEN; + } + if (memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, to_compare_v1) && + memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, to_compare_v2)) { + ENVOY_LOG(debug, "we don't have v1 or v2 header"); return ReadOrParseState::SkipFilterError; } } diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 931ad1920fd8..e60f23a36a11 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -964,6 +964,31 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { disconnect(); } +TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { + + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + write("P"); + + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + write("ROXY TCP4"); + write(" 254.254.2"); + write("54.254 1.2"); + write(".3.4 65535"); + write(" 1234\r\n..."); + + expectData("..."); + + EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), + "254.254.254.254"); + EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); + + disconnect(); +} + TEST_P(ProxyProtocolTest, V2PartialRead) { // A well-formed ipv4/tcp header, delivered with part of the signature, // part of the header, rest of header + body From f47232fd6dc3b448d7ba1c8002cdfecd27084831 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 17:22:44 +0000 Subject: [PATCH 16/52] Use standard library for min Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index b43bb1df5e23..c48de1ff6ea6 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -447,17 +447,13 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { return ReadOrParseState::Error; } else if (nread < PROXY_PROTO_V2_HEADER_LEN && config_.get()->allowRequestsWithoutProxyProtocol()) { - ssize_t to_compare_v1 = buf_off_ + nread; - if (to_compare_v1 > PROXY_PROTO_V1_SIGNATURE_LEN) { - to_compare_v1 = PROXY_PROTO_V1_SIGNATURE_LEN; - } - ssize_t to_compare_v2 = buf_off_ + nread; - if (to_compare_v2 > PROXY_PROTO_V2_SIGNATURE_LEN) { - to_compare_v2 = PROXY_PROTO_V2_SIGNATURE_LEN; - } - if (memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, to_compare_v1) && - memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, to_compare_v2)) { - ENVOY_LOG(debug, "we don't have v1 or v2 header"); + if (memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, + std::min(buf_off_ + nread, PROXY_PROTO_V1_SIGNATURE_LEN)) && + memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, + std::min(buf_off_ + nread, PROXY_PROTO_V2_SIGNATURE_LEN))) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilterError; } } From fa9746a917e6f13ed74eac94c578958e05428e23 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 18:27:40 +0000 Subject: [PATCH 17/52] PR comments; fix bug with large requests being allowed Signed-off-by: Kevin Dorosh --- .../proxy_protocol/v3/proxy_protocol.proto | 3 ++- .../listener/proxy_protocol/proxy_protocol.cc | 11 ++++----- .../listener/proxy_protocol/proxy_protocol.h | 2 +- .../proxy_protocol/proxy_protocol_test.cc | 23 +++++++++++++++---- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index ecad2145fadf..a245b7e51620 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -45,9 +45,10 @@ message ProxyProtocol { // // .. attention:: // + // This breaks conformance with the specification. // Only enable if ALL traffic to the listener comes from a trusted source. // For more information on the security implications of this feature, see - // https://www.haproxy.org/download/1.9/doc/proxy-protocol.txt + // https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt // bool allow_requests_without_proxy_protocol = 2; } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index c48de1ff6ea6..167b3fc7a1ca 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -93,7 +93,7 @@ void Filter::onRead() { if (read_state == ReadOrParseState::Error) { config_->stats_.downstream_cx_proxy_proto_error_.inc(); cb_->continueFilterChain(false); - } else if (read_state == ReadOrParseState::SkipFilterError) { + } else if (read_state == ReadOrParseState::SkipFilter) { resetAndContinue(cb_->socket().ioHandle()); } } @@ -320,7 +320,7 @@ ReadOrParseState Filter::parseExtensions(Network::IoHandle& io_handle, uint8_t* const auto recv_result = io_handle.recv(buf, to_read, 0); if (!recv_result.ok()) { if (recv_result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) { - return ReadOrParseState::TryAgainLaterError; + return ReadOrParseState::TryAgainLater; } ENVOY_LOG(debug, "failed to read proxy protocol (no bytes avail)"); return ReadOrParseState::Error; @@ -435,7 +435,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (!result.ok()) { if (result.err_->getErrorCode() == Api::IoError::IoErrorCode::Again) { - return ReadOrParseState::TryAgainLaterError; + return ReadOrParseState::TryAgainLater; } ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; @@ -445,8 +445,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (nread < 1) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; - } else if (nread < PROXY_PROTO_V2_HEADER_LEN && - config_.get()->allowRequestsWithoutProxyProtocol()) { + } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { if (memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, std::min(buf_off_ + nread, PROXY_PROTO_V1_SIGNATURE_LEN)) && memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, @@ -454,7 +453,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilterError; + return ReadOrParseState::SkipFilter; } } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index 42c7c8c558b7..de55925fcf12 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -75,7 +75,7 @@ using ConfigSharedPtr = std::shared_ptr; enum ProxyProtocolVersion { Unknown = -1, InProgress = -2, V1 = 1, V2 = 2 }; -enum class ReadOrParseState { Done, TryAgainLaterError, Error, SkipFilterError }; +enum class ReadOrParseState { Done, TryAgainLater, Error, SkipFilter }; /** * Implementation the PROXY Protocol listener filter diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index e60f23a36a11..20df1de54f4f 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -222,15 +222,30 @@ TEST_P(ProxyProtocolTest, V1Basic) { disconnect(); } -TEST_P(ProxyProtocolTest, AllowNoProxyProtocol) { - // allows request through even though it doesn't use proxy protocol +TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { + // allows a small request (less bytes than v1/v2 header) through even though it doesn't use proxy + // protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("more data"); + write("data"); - expectData("more data"); + expectData("data"); + + disconnect(); +} + +TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { + // allows a large request (more bytes than v1/v2 header) through even though it doesn't use proxy + // protocol + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + write("more data more data more data"); + + expectData("more data more data more data"); disconnect(); } From a37ad43cd7cf2cedb4491a1700da4515896b27bb Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 19:51:42 +0000 Subject: [PATCH 18/52] Assert msg size in tests in addition to noting in comment Signed-off-by: Kevin Dorosh --- .../proxy_protocol/proxy_protocol_test.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 20df1de54f4f..6f60e866381e 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -30,6 +30,8 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V1_SIGNATURE_LEN; +using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V2_HEADER_LEN; using testing::_; using testing::AnyNumber; using testing::AtLeast; @@ -229,9 +231,13 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("data"); + std::string msg = "data"; + EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); + EXPECT_GT(PROXY_PROTO_V2_HEADER_LEN, msg.length()); - expectData("data"); + write(msg); + + expectData(msg); disconnect(); } @@ -243,9 +249,13 @@ TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("more data more data more data"); + std::string msg = "more data more data more data"; + EXPECT_GT(msg.length(), PROXY_PROTO_V1_SIGNATURE_LEN); + EXPECT_GT(msg.length(), PROXY_PROTO_V2_HEADER_LEN); + + write(msg); - expectData("more data more data more data"); + expectData(msg); disconnect(); } From 6f19883279fa64bbcc107f0a7d21da1cedc71da0 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 10 Nov 2021 20:31:39 +0000 Subject: [PATCH 19/52] Do not allow any false positives Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 14 ++++++++--- .../proxy_protocol/proxy_protocol_test.cc | 25 ------------------- 2 files changed, 10 insertions(+), 29 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 167b3fc7a1ca..47608a807b1c 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -446,10 +446,16 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { - if (memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, - std::min(buf_off_ + nread, PROXY_PROTO_V1_SIGNATURE_LEN)) && - memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, - std::min(buf_off_ + nread, PROXY_PROTO_V2_SIGNATURE_LEN))) { + + if (nread < PROXY_PROTO_V1_SIGNATURE_LEN) { + ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " + "protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + + if ((nread < PROXY_PROTO_V2_SIGNATURE_LEN || + memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && + memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 6f60e866381e..be0452709d7b 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -989,31 +989,6 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { disconnect(); } -TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { - - envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; - proto_config.set_allow_requests_without_proxy_protocol(true); - connect(true, &proto_config); - - write("P"); - - dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - - write("ROXY TCP4"); - write(" 254.254.2"); - write("54.254 1.2"); - write(".3.4 65535"); - write(" 1234\r\n..."); - - expectData("..."); - - EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), - "254.254.254.254"); - EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); - - disconnect(); -} - TEST_P(ProxyProtocolTest, V2PartialRead) { // A well-formed ipv4/tcp header, delivered with part of the signature, // part of the header, rest of header + body From 0927000776a5a1f3e590ecb810324fbf6df3c9ac Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 11 Nov 2021 05:06:00 +0000 Subject: [PATCH 20/52] Only check if proxy protocol on initial MSG_PEEK Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 2 +- .../proxy_protocol/proxy_protocol_test.cc | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 47608a807b1c..d6bab4a5ffe2 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -445,7 +445,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if (nread < 1) { ENVOY_LOG(debug, "failed to read proxy protocol (no bytes read)"); return ReadOrParseState::Error; - } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { + } else if (config_.get()->allowRequestsWithoutProxyProtocol() && buf_off_ == 0) { if (nread < PROXY_PROTO_V1_SIGNATURE_LEN) { ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index be0452709d7b..a5ceaaf7813a 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -989,6 +989,34 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { disconnect(); } +TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { + + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + write("PROXY TCP4"); + + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + write(" 25"); // intentionally smaller than the size of v1/v2 header + + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + + write("4.254.2"); + write("54.254 1.2"); + write(".3.4 65535"); + write(" 1234\r\n..."); + + expectData("..."); + + EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), + "254.254.254.254"); + EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); + + disconnect(); +} + TEST_P(ProxyProtocolTest, V2PartialRead) { // A well-formed ipv4/tcp header, delivered with part of the signature, // part of the header, rest of header + body From b5cbfc18d12f9f7fd61cf68467cf78612014fcd9 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 11 Nov 2021 14:42:22 +0000 Subject: [PATCH 21/52] Don't check too many bytes on v1 Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index d6bab4a5ffe2..f41eaa1138b0 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -455,7 +455,8 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if ((nread < PROXY_PROTO_V2_SIGNATURE_LEN || memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && - memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, + std::min(nread, PROXY_PROTO_V1_SIGNATURE_LEN))) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); From a7ef68f0f041ae0070c3ad6787b3b55a3b71a6b3 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 11 Nov 2021 14:46:09 +0000 Subject: [PATCH 22/52] Revert "Don't check too many bytes on v1" This reverts commit b5cbfc18d12f9f7fd61cf68467cf78612014fcd9. Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index f41eaa1138b0..d6bab4a5ffe2 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -455,8 +455,7 @@ ReadOrParseState Filter::readProxyHeader(Network::IoHandle& io_handle) { if ((nread < PROXY_PROTO_V2_SIGNATURE_LEN || memcmp(buf_, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && - memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, - std::min(nread, PROXY_PROTO_V1_SIGNATURE_LEN))) { + memcmp(buf_, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); From ac2982f00fad77280a67477ef99763f8f6c89c8c Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 28 Apr 2022 21:33:36 +0000 Subject: [PATCH 23/52] Fix bad merge on current.rst Signed-off-by: Kevin Dorosh --- docs/root/version_history/current.rst | 51 +-------------------------- 1 file changed, 1 insertion(+), 50 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 1b793ea085a7..8edd5a9ced4e 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -33,59 +33,10 @@ Removed Config or Runtime New Features ------------ -* access log: added :ref:`grpc_stream_retry_policy ` to the gRPC logger to reconnect when a connection fails to be established. -* access_log: added :ref:`METADATA` token to handle all types of metadata (DYNAMIC, CLUSTER, ROUTE). -* access_log: added a CEL extension filter to enable filtering of access logs based on Envoy attribute expressions. -* access_log: added new access_log command operator ``%UPSTREAM_REQUEST_ATTEMPT_COUNT%`` to retrieve the number of times given request got attempted upstream. -* access_log: added new access_log command operator ``%VIRTUAL_CLUSTER_NAME%`` to retrieve the matched Virtual Cluster name. -* api: added support for *xds.type.v3.TypedStruct* in addition to the now-deprecated *udpa.type.v1.TypedStruct* proto message, which is a wrapper proto used to encode typed JSON data in a *google.protobuf.Any* field. -* aws_request_signing_filter: added :ref:`match_excluded_headers ` to the signing filter to optionally exclude request headers from signing. -* bootstrap: added :ref:`typed_dns_resolver_config ` in the bootstrap to support DNS resolver as an extension. -* cluster: added :ref:`typed_dns_resolver_config ` in the cluster to support DNS resolver as an extension. -* config: added :ref:`environment_variable ` to the :ref:`DataSource `. -* dns: added :ref:`ALL ` option to return both IPv4 and IPv6 addresses. -* dns_cache: added :ref:`typed_dns_resolver_config ` in the dns_cache to support DNS resolver as an extension. -* dns_filter: added :ref:`typed_dns_resolver_config ` in the dns_filter to support DNS resolver as an extension. -* dns_resolver: added :ref:`CaresDnsResolverConfig` to support c-ares DNS resolver as an extension. -* dns_resolver: added :ref:`use_resolvers_as_fallback` to the c-ares DNS resolver. -* dns_resolver: added :ref:`AppleDnsResolverConfig` to support apple DNS resolver as an extension. -* ext_authz: added :ref:`query_parameters_to_set ` and :ref:`query_parameters_to_remove ` for adding and removing query string parameters when using a gRPC authorization server. -* grpc_json_transcoder: added support for matching unregistered custom verb :ref:`match_unregistered_custom_verb `. -* http: added support for %REQUESTED_SERVER_NAME% to extract SNI as a custom header. -* http: added support for %VIRTUAL_CLUSTER_NAME% to extract the matched Virtual Cluster name as a custom header. -* http: added support for :ref:`retriable health check status codes `. -* http: added timing information about upstream connection and encryption establishment to stream info. These can currently be accessed via custom access loggers. -* http: added support for :ref:`forwarding HTTP1 reason phrase `. -* listener: added API for extensions to access :ref:`typed_filter_metadata ` configured in the listener's :ref:`metadata ` field. -* listener: added support for :ref:`MPTCP ` (multipath TCP). -* listener: added support for opting out listeners from the globally set downstream connection limit via :ref:`ignore_global_conn_limit `. -* oauth filter: added :ref:`cookie_names ` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter. -* oauth filter: setting IdToken and RefreshToken cookies if they are provided by Identity provider along with AccessToken. -* proxy protocol: added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. -* perf: added support for [Perfetto](https://perfetto.dev) performance tracing. -* router: added support for the :ref:`config_http_conn_man_headers_x-forwarded-host` header. -* stats: added text_readouts query parameter to prometheus stats to return gauges made from text readouts. -* tcp: added a :ref:`FilterState ` :ref:`hash policy `, used by :ref:`TCP proxy ` to allow hashing load balancer algorithms to hash on objects in filter state. -* tcp_proxy: added support to populate upstream http connect header values from stream info. -* thrift_proxy: add header to metadata filter for turning headers into dynamic metadata. -* thrift_proxy: add upstream response zone metrics in the form ``cluster.cluster_name.zone.local_zone.upstream_zone.thrift.upstream_resp_success``. -* thrift_proxy: add upstream metrics to show decoding errors and whether exception is from local or remote, e.g. ``cluster.cluster_name.thrift.upstream_resp_exception_remote``. -* thrift_proxy: add host level success/error metrics where success is a reply of type success and error is any other response to a call. -* thrift_proxy: support header flags. -* thrift_proxy: support subset lb when using request or route metadata. -* tls: added support for :ref:`match_typed_subject_alt_names ` for subject alternative names to enforce specifying the subject alternative name type for the matcher to prevent matching against an unintended type in the certificate. -* tls: added support for only verifying the leaf CRL in the certificate chain with :ref:`only_verify_leaf_cert_crl `. -* tls: support loading certificate chain and private key via :ref:`pkcs12 `. -* tls_inspector filter: added :ref:`enable_ja3_fingerprinting ` to create JA3 fingerprint hash from Client Hello message. -* transport_socket: added :ref:`envoy.transport_sockets.tcp_stats ` which generates additional statistics gathered from the OS TCP stack. -* udp: add support for multiple listener filters. -* udp_proxy: added :ref:`use_per_packet_load_balancing ` option to enable per packet load balancing (selection of upstream host on each data chunk). -* upstream: added the ability to :ref:`configure max connection duration ` for upstream clusters. -* vcl_socket_interface: added VCL socket interface extension for fd.io VPP integration to :ref:`contrib images `. This can be enabled via :ref:`VCL ` configuration. -* xds: re-introduced unified delta and sotw xDS multiplexers that share most of the implementation. Added a new runtime config ``envoy.reloadable_features.unified_mux`` (disabled by default) that when enabled, switches xDS to use unified multiplexers. * access_log: added new access_log command operators to retrieve upstream connection information: ``%UPSTREAM_PROTOCOL%``, ``%UPSTREAM_PEER_SUBJECT%``, ``%UPSTREAM_PEER_ISSUER%``, ``%UPSTREAM_TLS_SESSION_ID%``, ``%UPSTREAM_TLS_CIPHER%``, ``%UPSTREAM_TLS_VERSION%``, ``%UPSTREAM_PEER_CERT_V_START%``, ``%UPSTREAM_PEER_CERT_V_END%`` and ``%UPSTREAM_PEER_CERT%``. * dns_resolver: added :ref:`include_unroutable_families` to the Apple DNS resolver. * ext_proc: added support for per-route :ref:`grpc_service `. +* proxy protocol: added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. * thrift: added flag to router to control downstream local close. :ref:`close_downstream_on_upstream_error `. Deprecated From 2a66a01dedb6d04aabbcb1986e25ce06e9b623bf Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 28 Apr 2022 22:02:38 +0000 Subject: [PATCH 24/52] Formatting Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 5548fd2ba4ff..c13c6cebe177 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -409,12 +409,12 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) if (config_.get()->allowRequestsWithoutProxyProtocol()) { if (raw_slice.len_ < PROXY_PROTO_V1_SIGNATURE_LEN) { ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " - "protocol, forwarding as is"); + "protocol, forwarding as is"); return ReadOrParseState::SkipFilter; } if ((raw_slice.len_ < PROXY_PROTO_V2_SIGNATURE_LEN || - memcmp(buf, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && + memcmp(buf, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit From c9e086f46eab0afce06b41ae8301642b2a9b4c04 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 16:24:57 +0000 Subject: [PATCH 25/52] Wait for more bytes if we can't tell Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index c13c6cebe177..ca8faafc5ff8 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -407,20 +407,21 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) const char* buf = static_cast(raw_slice.mem_); if (config_.get()->allowRequestsWithoutProxyProtocol()) { - if (raw_slice.len_ < PROXY_PROTO_V1_SIGNATURE_LEN) { - ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " - "protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } - if ((raw_slice.len_ < PROXY_PROTO_V2_SIGNATURE_LEN || - memcmp(buf, PROXY_PROTO_V2_SIGNATURE, PROXY_PROTO_V2_SIGNATURE_LEN)) && - memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + if (memcmp(buf, PROXY_PROTO_V2_SIGNATURE, std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)) && + memcmp(buf, PROXY_PROTO_V1_SIGNATURE, std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_))) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; } + + if (raw_slice.len_ < PROXY_PROTO_V2_SIGNATURE_LEN) { + ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " + "protocol, waiting for more bytes"); + return ReadOrParseState::TryAgainLater; + } + } if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { From af6f88dbe5810571e63eca591a1517ec8de3f3be Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 16:33:22 +0000 Subject: [PATCH 26/52] Update API comment to reflect waiting behavior Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/v3/proxy_protocol.proto | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index 81bdc39e206e..f7d89570288e 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -51,5 +51,10 @@ message ProxyProtocol { // For more information on the security implications of this feature, see // https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt // + // While incredibily rare, requests of less than 12 bytes that match the + // beginning of the proxy protocol v2 format and requests of less than 6 + // bytes that match the beginning of proxy protocol v1 format will timeout. + // (envoy is unable to differentiate these requests from incomplete proxy + // protocol requests) bool allow_requests_without_proxy_protocol = 2; } From 7f1766035d9966ad99b60090f25beff494b723ac Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 16:36:00 +0000 Subject: [PATCH 27/52] Formatting Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index ca8faafc5ff8..97468110118e 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -408,8 +408,10 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) if (config_.get()->allowRequestsWithoutProxyProtocol()) { - if (memcmp(buf, PROXY_PROTO_V2_SIGNATURE, std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)) && - memcmp(buf, PROXY_PROTO_V1_SIGNATURE, std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_))) { + if (memcmp(buf, PROXY_PROTO_V2_SIGNATURE, + std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)) && + memcmp(buf, PROXY_PROTO_V1_SIGNATURE, + std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_))) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); @@ -421,7 +423,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) "protocol, waiting for more bytes"); return ReadOrParseState::TryAgainLater; } - } if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { From 59206daa3f6c403e8230f8203df513db589f706e Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 16:57:14 +0000 Subject: [PATCH 28/52] ReadOrParseState is not always an error if not Done Signed-off-by: Kevin Dorosh --- .../extensions/filters/listener/proxy_protocol/proxy_protocol.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index 72ade1758a50..d41fac692eec 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -107,7 +107,7 @@ class Filter : public Network::ListenerFilter, Logger::Loggable Date: Fri, 29 Apr 2022 17:05:42 +0000 Subject: [PATCH 29/52] Improve comments Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 10757c14bd4b..291fc244b6fb 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -32,7 +32,7 @@ #include "gtest/gtest.h" using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V1_SIGNATURE_LEN; -using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V2_HEADER_LEN; +using Envoy::Extensions::Common::ProxyProtocol::PROXY_PROTO_V2_SIGNATURE_LEN; using testing::_; using testing::AnyNumber; using testing::AtLeast; @@ -233,7 +233,7 @@ TEST_P(ProxyProtocolTest, V1Basic) { } TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { - // allows a small request (less bytes than v1/v2 header) through even though it doesn't use proxy + // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use proxy // protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); @@ -241,7 +241,7 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { std::string msg = "data"; EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); - EXPECT_GT(PROXY_PROTO_V2_HEADER_LEN, msg.length()); + EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); @@ -251,7 +251,7 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { } TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { - // allows a large request (more bytes than v1/v2 header) through even though it doesn't use proxy + // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use proxy // protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); @@ -259,7 +259,7 @@ TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { std::string msg = "more data more data more data"; EXPECT_GT(msg.length(), PROXY_PROTO_V1_SIGNATURE_LEN); - EXPECT_GT(msg.length(), PROXY_PROTO_V2_HEADER_LEN); + EXPECT_GT(msg.length(), PROXY_PROTO_V2_SIGNATURE_LEN); write(msg); From 4e4638a878c9cf91b29d00f3eff58a1fad4971ad Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 17:07:15 +0000 Subject: [PATCH 30/52] Formatting Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 291fc244b6fb..3008bb55bd17 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -233,8 +233,8 @@ TEST_P(ProxyProtocolTest, V1Basic) { } TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { - // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use proxy - // protocol + // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use + // proxy protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); @@ -251,8 +251,8 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { } TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { - // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use proxy - // protocol + // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use + // proxy protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); From 354a3653e7231a814225d311deb5ccbcd31d2649 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 17:16:37 +0000 Subject: [PATCH 31/52] Better test for waiting for more bytes Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 3008bb55bd17..2b21745a843e 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -1103,11 +1103,11 @@ TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PROXY TCP4"); + write("PRO"); // intentionally smaller than the size of v1/v2 header dispatcher_->run(Event::Dispatcher::RunType::NonBlock); - write(" 25"); // intentionally smaller than the size of v1/v2 header + write("XY TCP4 25"); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); From 86ad288a4fb4f598b7d4160400e18d8bdbf2d03f Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 17:20:09 +0000 Subject: [PATCH 32/52] More accurate comment Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 2b21745a843e..7d1b8a463963 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -1103,7 +1103,7 @@ TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PRO"); // intentionally smaller than the size of v1/v2 header + write("PRO"); // intentionally smaller than the size of v1/v2 signature dispatcher_->run(Event::Dispatcher::RunType::NonBlock); From 899f199da60704de0f0cadf0fc2998cf1eb49e3c Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Fri, 29 Apr 2022 17:49:06 +0000 Subject: [PATCH 33/52] We can wait for less bytes for v1 proxy protocol Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 97468110118e..2f2830d4d38f 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -408,17 +408,20 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) if (config_.get()->allowRequestsWithoutProxyProtocol()) { - if (memcmp(buf, PROXY_PROTO_V2_SIGNATURE, - std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)) && - memcmp(buf, PROXY_PROTO_V1_SIGNATURE, - std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_))) { + auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, + std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); + auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, + std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); + + if (!matchv2 && !matchv1) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; } - if (raw_slice.len_ < PROXY_PROTO_V2_SIGNATURE_LEN) { + auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; + if (raw_slice.len_ < sig_len) { ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " "protocol, waiting for more bytes"); return ReadOrParseState::TryAgainLater; From 887d0bc55e917bec5c6b3d024275b60c1c3e5bd5 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Sat, 30 Apr 2022 02:10:52 +0000 Subject: [PATCH 34/52] Fix spelling error Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/v3/proxy_protocol.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index f7d89570288e..4edeb4f9066c 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -51,7 +51,7 @@ message ProxyProtocol { // For more information on the security implications of this feature, see // https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt // - // While incredibily rare, requests of less than 12 bytes that match the + // While incredibly rare, requests of less than 12 bytes that match the // beginning of the proxy protocol v2 format and requests of less than 6 // bytes that match the beginning of proxy protocol v1 format will timeout. // (envoy is unable to differentiate these requests from incomplete proxy From a23ac5f1e2613be72526f5dcb70a76b13deef126 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Sat, 30 Apr 2022 04:04:41 +0000 Subject: [PATCH 35/52] Reuse previous parsing logic; split checks on allowRequestsWithoutProxyProtocol Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 2f2830d4d38f..56b6b79ff129 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -405,34 +405,17 @@ ReadOrParseState Filter::readExtensions(Network::ListenerFilterBuffer& buffer) { ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) { auto raw_slice = buffer.rawSlice(); const char* buf = static_cast(raw_slice.mem_); - - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - - auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, - std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); - auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, - std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); - - if (!matchv2 && !matchv1) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } - - auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; - if (raw_slice.len_ < sig_len) { - ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " - "protocol, waiting for more bytes"); - return ReadOrParseState::TryAgainLater; - } - } - if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { const char* sig = PROXY_PROTO_V2_SIGNATURE; if (!memcmp(buf, sig, PROXY_PROTO_V2_SIGNATURE_LEN)) { header_version_ = V2; } else if (memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } // It is not v2, and can't be v1, so no sense hanging around: it is invalid ENVOY_LOG(debug, "failed to read proxy protocol (exceed max v1 header len)"); return ReadOrParseState::Error; @@ -485,6 +468,17 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } } + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, + std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); + if (!matchv1) { + // the bytes we have seen so far do not match v1 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + } + if (search_index_ > MAX_PROXY_PROTO_LEN_V1) { return ReadOrParseState::Error; } From 6075b676d7deba6559c3eee3be4f43b02d778f86 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Sat, 30 Apr 2022 04:08:49 +0000 Subject: [PATCH 36/52] Revert "Reuse previous parsing logic; split checks on allowRequestsWithoutProxyProtocol" This reverts commit a23ac5f1e2613be72526f5dcb70a76b13deef126. Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 40 +++++++++++-------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 56b6b79ff129..2f2830d4d38f 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -405,17 +405,34 @@ ReadOrParseState Filter::readExtensions(Network::ListenerFilterBuffer& buffer) { ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) { auto raw_slice = buffer.rawSlice(); const char* buf = static_cast(raw_slice.mem_); + + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + + auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, + std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); + auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, + std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); + + if (!matchv2 && !matchv1) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + + auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; + if (raw_slice.len_ < sig_len) { + ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " + "protocol, waiting for more bytes"); + return ReadOrParseState::TryAgainLater; + } + } + if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { const char* sig = PROXY_PROTO_V2_SIGNATURE; if (!memcmp(buf, sig, PROXY_PROTO_V2_SIGNATURE_LEN)) { header_version_ = V2; } else if (memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } // It is not v2, and can't be v1, so no sense hanging around: it is invalid ENVOY_LOG(debug, "failed to read proxy protocol (exceed max v1 header len)"); return ReadOrParseState::Error; @@ -468,17 +485,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } } - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, - std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); - if (!matchv1) { - // the bytes we have seen so far do not match v1 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } - } - if (search_index_ > MAX_PROXY_PROTO_LEN_V1) { return ReadOrParseState::Error; } From 4d6f75f61c088f703679ae405ecff66269fc3557 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Sat, 30 Apr 2022 18:06:04 +0000 Subject: [PATCH 37/52] Add v2 partial tests Signed-off-by: Kevin Dorosh --- .../proxy_protocol/proxy_protocol_test.cc | 66 ++++++++++++++++++- 1 file changed, 64 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 7d1b8a463963..e8beb2ae37b5 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -1079,7 +1079,7 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PROXY TCP4"); + write("PROXY TCP4"); // intentionally larger than the size of v1 proxy protocol signature dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -1103,7 +1103,7 @@ TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PRO"); // intentionally smaller than the size of v1/v2 signature + write("PRO"); // intentionally smaller than the size of v1 proxy protocol signature dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -1150,6 +1150,68 @@ TEST_P(ProxyProtocolTest, V2PartialRead) { disconnect(); } +TEST_P(ProxyProtocolTest, PartialV2ReadWithAllowNoProxyProtocol) { + // A well-formed ipv4/tcp header, delivered with part of the signature, + // part of the header, rest of header + body + constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, + 0x49, 0x54, 0x0a, 0x21, 0x11, 0x00, 0x0c, 0x01, 0x02, + 0x03, 0x04, 0x00, 0x01, 0x01, 0x02, 0x03, 0x05, 0x00, + 0x02, 'm', 'o', 'r', 'e', 'd', 'a', 't', 'a'}; + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + // using 18 intentionally as it is larger than v2 signature length and divides evenly into + // len(buffer) + auto buffer_incr_size = 18; + EXPECT_LT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); + for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) { + write(&buffer[i], buffer_incr_size); + if (i == 0) { + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + } + } + + expectData("moredata"); + + EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), + "1.2.3.4"); + EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); + + disconnect(); +} + +TEST_P(ProxyProtocolTest, TinyPartialV2ReadWithAllowNoProxyProtocol) { + // A well-formed ipv4/tcp header, delivered with part of the signature, + // part of the header, rest of header + body + constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, + 0x49, 0x54, 0x0a, 0x21, 0x11, 0x00, 0x0c, 0x01, 0x02, + 0x03, 0x04, 0x00, 0x01, 0x01, 0x02, 0x03, 0x05, 0x00, + 0x02, 'm', 'o', 'r', 'e', 'd', 'a', 't', 'a'}; + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + // using 3 intentionally as it is smaller than v2 signature length and divides evenly into + // len(buffer) + auto buffer_incr_size = 3; + EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); + for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) { + write(&buffer[i], buffer_incr_size); + if (i == 0) { + dispatcher_->run(Event::Dispatcher::RunType::NonBlock); + } + } + + expectData("moredata"); + + EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), + "1.2.3.4"); + EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); + + disconnect(); +} + const std::string ProxyProtocol = "envoy.filters.listener.proxy_protocol"; TEST_P(ProxyProtocolTest, V2ParseExtensionsLargeThanInitMaxReadBytes) { From 2cb45268cf06d44c96c68c604ecc7d585aeb9a51 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 02:59:55 +0000 Subject: [PATCH 38/52] Reuse existing parsing logic Signed-off-by: Kevin Dorosh --- .../proxy_protocol/v3/proxy_protocol.proto | 9 ++-- .../listener/proxy_protocol/proxy_protocol.cc | 46 ++++++++++--------- .../listener/proxy_protocol/proxy_protocol.h | 3 ++ 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index 4edeb4f9066c..bad45f9f9bed 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -51,10 +51,9 @@ message ProxyProtocol { // For more information on the security implications of this feature, see // https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt // - // While incredibly rare, requests of less than 12 bytes that match the - // beginning of the proxy protocol v2 format and requests of less than 6 - // bytes that match the beginning of proxy protocol v1 format will timeout. - // (envoy is unable to differentiate these requests from incomplete proxy - // protocol requests) + // While incredibly rare, requests of 12 or fewer bytes that match the + // proxy protocol v2 signature and requests of 6 or fewer bytes that match + // the proxy protocol v1 signature will timeout. (envoy is unable to + // differentiate these requests from incomplete proxy protocol requests) bool allow_requests_without_proxy_protocol = 2; } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 2f2830d4d38f..67227874d300 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -406,33 +406,17 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) auto raw_slice = buffer.rawSlice(); const char* buf = static_cast(raw_slice.mem_); - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - - auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, - std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); - auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, - std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); - - if (!matchv2 && !matchv1) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } - - auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; - if (raw_slice.len_ < sig_len) { - ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " - "protocol, waiting for more bytes"); - return ReadOrParseState::TryAgainLater; - } - } - if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { const char* sig = PROXY_PROTO_V2_SIGNATURE; if (!memcmp(buf, sig, PROXY_PROTO_V2_SIGNATURE_LEN)) { header_version_ = V2; } else if (memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } // It is not v2, and can't be v1, so no sense hanging around: it is invalid ENVOY_LOG(debug, "failed to read proxy protocol (exceed max v1 header len)"); return ReadOrParseState::Error; @@ -472,6 +456,24 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } else { // continue searching buffer from where we left off for (; search_index_ < raw_slice.len_; search_index_++) { + + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN && + buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) { + possibly_v1_ = false; + } + if (search_index_ < PROXY_PROTO_V2_SIGNATURE_LEN && + buf[search_index_] != PROXY_PROTO_V2_SIGNATURE[search_index_]) { + possibly_v2_ = false; + } + if (!possibly_v1_ && !possibly_v2_) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + } + if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { if (search_index_ == 1) { // There is not enough data to determine if it contains the v2 protocol signature, so wait diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index d41fac692eec..1f84fe7716ab 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -127,6 +127,9 @@ class Filter : public Network::ListenerFilter, Logger::Loggable Date: Mon, 2 May 2022 12:58:02 +0000 Subject: [PATCH 39/52] Remove whitespace in test Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index e8beb2ae37b5..066101f1a2cb 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -1074,7 +1074,6 @@ TEST_P(ProxyProtocolTest, PartialRead) { } TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { - envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); @@ -1098,7 +1097,6 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { } TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { - envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); From a0c977f6ed37cea7349d611cf97a687921c62bd2 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 13:11:09 +0000 Subject: [PATCH 40/52] Move check to else statement Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 25 ++++++++----------- .../proxy_protocol/proxy_protocol_test.cc | 18 +++++++++++++ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 67227874d300..a4768a0372db 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -456,8 +456,17 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } else { // continue searching buffer from where we left off for (; search_index_ < raw_slice.len_; search_index_++) { - - if (config_.get()->allowRequestsWithoutProxyProtocol()) { + if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { + if (search_index_ == 1) { + // There is not enough data to determine if it contains the v2 protocol signature, so wait + // for more data. + break; + } else { + header_version_ = V1; + search_index_++; + } + break; + } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN && buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) { possibly_v1_ = false; @@ -473,18 +482,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) return ReadOrParseState::SkipFilter; } } - - if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { - if (search_index_ == 1) { - // There is not enough data to determine if it contains the v2 protocol signature, so wait - // for more data. - break; - } else { - header_version_ = V1; - search_index_++; - } - break; - } } if (search_index_ > MAX_PROXY_PROTO_LEN_V1) { diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 066101f1a2cb..79e7ff79aafe 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -250,6 +250,24 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { disconnect(); } +TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatch) { + // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use + // proxy protocol + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(true, &proto_config); + + std::string msg = "PR\r\n"; // partial v1 proxy protocol signature match + EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); + EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); + + write(msg); + + expectData(msg); + + disconnect(); +} + TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use // proxy protocol From 85706e635a4d5c0a4d4ef8ba008b3c431245d95b Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 14:40:51 +0000 Subject: [PATCH 41/52] Clarify what tests are covering Signed-off-by: Kevin Dorosh --- .../proxy_protocol/proxy_protocol_test.cc | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 79e7ff79aafe..f88d27e36d76 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -240,7 +240,8 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { connect(true, &proto_config); std::string msg = "data"; - EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); + EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, + msg.length()); // ensure we attempt parsing byte by byte using `search_index_` EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); @@ -250,14 +251,17 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { disconnect(); } -TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatch) { +TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatches) { // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use - // proxy protocol + // proxy protocol v1/v2 (but it does match parts of both signatures) envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - std::string msg = "PR\r\n"; // partial v1 proxy protocol signature match + // first two bytes proxy protocol v1, second two bytes proxy protocol v2. + // this ensures our byte by byte parsing (search_index_) has persistence built-in to + // remember whether the previous bytes were also valid for the signature + std::string msg = "PR\r\n"; EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); @@ -276,8 +280,9 @@ TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { connect(true, &proto_config); std::string msg = "more data more data more data"; - EXPECT_GT(msg.length(), PROXY_PROTO_V1_SIGNATURE_LEN); - EXPECT_GT(msg.length(), PROXY_PROTO_V2_SIGNATURE_LEN); + EXPECT_GT(msg.length(), + PROXY_PROTO_V2_HEADER_LEN); // ensure we attempt parsing as v2 proxy protocol up front + // rather than parsing byte by byte using `search_index_` write(msg); From c86edef7feb35cd3d268f73c1020ba2b713c6d93 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 15:27:36 +0000 Subject: [PATCH 42/52] Add another test and fix for v2 followed by v1 signature bytes Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 19 +++++++++------ .../listener/proxy_protocol/proxy_protocol.h | 3 --- .../proxy_protocol/proxy_protocol_test.cc | 23 ++++++++++++++++++- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index a4768a0372db..a73f83df1d1d 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -458,6 +458,18 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) for (; search_index_ < raw_slice.len_; search_index_++) { if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { if (search_index_ == 1) { + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + // we need to check if what we have already could be v2 proxy protocol; + // if it cannot be, then we might as well forward now + auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, + std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); + if (!matchv2) { + // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can + // safely short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + } // There is not enough data to determine if it contains the v2 protocol signature, so wait // for more data. break; @@ -469,13 +481,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN && buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) { - possibly_v1_ = false; - } - if (search_index_ < PROXY_PROTO_V2_SIGNATURE_LEN && - buf[search_index_] != PROXY_PROTO_V2_SIGNATURE[search_index_]) { - possibly_v2_ = false; - } - if (!possibly_v1_ && !possibly_v2_) { // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index 1f84fe7716ab..d41fac692eec 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -127,9 +127,6 @@ class Filter : public Network::ListenerFilter, Logger::Loggable Date: Mon, 2 May 2022 15:56:27 +0000 Subject: [PATCH 43/52] Add tests to ensure we disconnect invalid requests after valid signature Signed-off-by: Kevin Dorosh --- .../proxy_protocol/proxy_protocol_test.cc | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 0781282bb779..4390554e965c 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -608,6 +608,19 @@ TEST_P(ProxyProtocolTest, V2ShortV4) { expectProxyProtoError(); } +TEST_P(ProxyProtocolTest, V2ShortV4WithAllowNoProxyProtocol) { + // An ipv4/tcp connection that has incorrect addr-len encoded + constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49, + 0x54, 0x0a, 0x21, 0x21, 0x00, 0x04, 0x00, 0x08, 0x00, 0x02, + 'm', 'o', 'r', 'e', ' ', 'd', 'a', 't', 'a'}; + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(false, &proto_config); + + write(buffer, sizeof(buffer)); + expectProxyProtoError(); +} + TEST_P(ProxyProtocolTest, V2ShortAddrV4) { // An ipv4/tcp connection that has insufficient header-length encoded constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49, @@ -691,6 +704,18 @@ TEST_P(ProxyProtocolTest, V1TooLong) { expectProxyProtoError(); } +TEST_P(ProxyProtocolTest, V1TooLongWithAllowNoProxyProtocol) { + constexpr uint8_t buffer[] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '}; + envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; + proto_config.set_allow_requests_without_proxy_protocol(true); + connect(false, &proto_config); + write("PROXY TCP4 1.2.3.4 2.3.4.5 100 100"); + for (size_t i = 0; i < 256; i += sizeof(buffer)) { + write(buffer, sizeof(buffer)); + } + expectProxyProtoError(); +} + TEST_P(ProxyProtocolTest, V2ParseExtensions) { // A well-formed ipv4/tcp with a pair of TLV extensions is accepted constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49, From a09bb1bd608c8ac25c86abbb8e5702389443d1d9 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 18:13:40 +0000 Subject: [PATCH 44/52] Use capital letters Signed-off-by: Kevin Dorosh --- .../proxy_protocol/v3/proxy_protocol.proto | 4 +-- .../proxy_protocol/proxy_protocol_test.cc | 28 +++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index bad45f9f9bed..f11607853427 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -53,7 +53,7 @@ message ProxyProtocol { // // While incredibly rare, requests of 12 or fewer bytes that match the // proxy protocol v2 signature and requests of 6 or fewer bytes that match - // the proxy protocol v1 signature will timeout. (envoy is unable to - // differentiate these requests from incomplete proxy protocol requests) + // the proxy protocol v1 signature will timeout (Envoy is unable to + // differentiate these requests from incomplete proxy protocol requests). bool allow_requests_without_proxy_protocol = 2; } diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 4390554e965c..2b67e05a3e0f 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -233,7 +233,7 @@ TEST_P(ProxyProtocolTest, V1Basic) { } TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { - // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use + // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use // proxy protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); @@ -241,7 +241,7 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { std::string msg = "data"; EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, - msg.length()); // ensure we attempt parsing byte by byte using `search_index_` + msg.length()); // Ensure we attempt parsing byte by byte using `search_index_` EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); @@ -252,14 +252,14 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { } TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV1First) { - // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use + // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use // proxy protocol v1/v2 (but it does match parts of both signatures) envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - // first two bytes proxy protocol v1, second two bytes proxy protocol v2. - // this ensures our byte by byte parsing (search_index_) has persistence built-in to + // First two bytes are proxy protocol v1, second two bytes are proxy protocol v2. + // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to // remember whether the previous bytes were also valid for the signature std::string msg = "PR\r\n"; EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); @@ -273,14 +273,14 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV1First) { } TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV2First) { - // allows a small request (less bytes than v1/v2 signature) through even though it doesn't use + // Allows a small request (less bytes than v1/v2 signature) through even though it doesn't use // proxy protocol v1/v2 (but it does match parts of both signatures) envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - // first two bytes proxy protocol v2, second two bytes proxy protocol v1. - // this ensures our byte by byte parsing (search_index_) has persistence built-in to + // First two bytes are proxy protocol v2, second two bytes are proxy protocol v1. + // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to // remember whether the previous bytes were also valid for the signature std::string msg = "\r\nOX"; EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); @@ -294,7 +294,7 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV2First) { } TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { - // allows a large request (more bytes than v1/v2 signature) through even though it doesn't use + // Allows a large request (more bytes than v1/v2 signature) through even though it doesn't use // proxy protocol envoy::extensions::filters::listener::proxy_protocol::v3::ProxyProtocol proto_config; proto_config.set_allow_requests_without_proxy_protocol(true); @@ -302,7 +302,7 @@ TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { std::string msg = "more data more data more data"; EXPECT_GT(msg.length(), - PROXY_PROTO_V2_HEADER_LEN); // ensure we attempt parsing as v2 proxy protocol up front + PROXY_PROTO_V2_HEADER_LEN); // Ensure we attempt parsing as v2 proxy protocol up front // rather than parsing byte by byte using `search_index_` write(msg); @@ -1147,7 +1147,7 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PROXY TCP4"); // intentionally larger than the size of v1 proxy protocol signature + write("PROXY TCP4"); // Intentionally larger than the size of v1 proxy protocol signature dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -1170,7 +1170,7 @@ TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - write("PRO"); // intentionally smaller than the size of v1 proxy protocol signature + write("PRO"); // Intentionally smaller than the size of v1 proxy protocol signature dispatcher_->run(Event::Dispatcher::RunType::NonBlock); @@ -1228,7 +1228,7 @@ TEST_P(ProxyProtocolTest, PartialV2ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - // using 18 intentionally as it is larger than v2 signature length and divides evenly into + // Using 18 intentionally as it is larger than v2 signature length and divides evenly into // len(buffer) auto buffer_incr_size = 18; EXPECT_LT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); @@ -1259,7 +1259,7 @@ TEST_P(ProxyProtocolTest, TinyPartialV2ReadWithAllowNoProxyProtocol) { proto_config.set_allow_requests_without_proxy_protocol(true); connect(true, &proto_config); - // using 3 intentionally as it is smaller than v2 signature length and divides evenly into + // Using 3 intentionally as it is smaller than v2 signature length and divides evenly into // len(buffer) auto buffer_incr_size = 3; EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); From cfeda5a69bfdfc0cefce68fe48ffe5764b4cd248 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 18:16:33 +0000 Subject: [PATCH 45/52] Use capital letters Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index a73f83df1d1d..8e405dbcc467 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -412,7 +412,7 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) header_version_ = V2; } else if (memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { if (config_.get()->allowRequestsWithoutProxyProtocol()) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; @@ -443,7 +443,7 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) hdr_addr_len, addr_len); return ReadOrParseState::Error; } - // waiting for more data if there is not enough data for address. + // waiting for more data if there is no enough data for address. if (raw_slice.len_ >= static_cast(PROXY_PROTO_V2_HEADER_LEN + addr_len)) { // The TLV remain, they are parsed in `parseTlvs()` which is called from the // parent (if needed). @@ -459,12 +459,12 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { if (search_index_ == 1) { if (config_.get()->allowRequestsWithoutProxyProtocol()) { - // we need to check if what we have already could be v2 proxy protocol; + // We need to check if what we have already could be v2 proxy protocol; // if it cannot be, then we might as well forward now auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); if (!matchv2) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can + // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can // safely short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; @@ -481,7 +481,7 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN && buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) { - // the bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; From 5a660a0425ae3713077711138a63ef2d9b72dcee Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 18:20:19 +0000 Subject: [PATCH 46/52] Use const Signed-off-by: Kevin Dorosh --- .../extensions/filters/listener/proxy_protocol/proxy_protocol.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index d41fac692eec..e374196b8121 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -68,7 +68,7 @@ class Config : public Logger::Loggable { private: absl::flat_hash_map tlv_types_; - bool allow_requests_without_proxy_protocol_{}; + const bool allow_requests_without_proxy_protocol_{}; }; using ConfigSharedPtr = std::shared_ptr; From e92a8a493fd5f190600597ebf55cabeca5cbe975 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 18:22:50 +0000 Subject: [PATCH 47/52] Do all checking in one place up front Signed-off-by: Kevin Dorosh --- .../listener/proxy_protocol/proxy_protocol.cc | 45 ++++++++----------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 8e405dbcc467..f91e9c9ac067 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -406,17 +406,30 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) auto raw_slice = buffer.rawSlice(); const char* buf = static_cast(raw_slice.mem_); + if (config_.get()->allowRequestsWithoutProxyProtocol()) { + auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, + std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); + auto matchv1 = !memcmp(buf, PROXY_PROTO_V1_SIGNATURE, + std::min(PROXY_PROTO_V1_SIGNATURE_LEN, raw_slice.len_)); + if (!matchv2 && !matchv1) { + // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely + // short-circuit + ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + return ReadOrParseState::SkipFilter; + } + auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; + if (raw_slice.len_ < sig_len) { + ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " + "protocol, waiting for more bytes"); + return ReadOrParseState::TryAgainLater; + } + } + if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { const char* sig = PROXY_PROTO_V2_SIGNATURE; if (!memcmp(buf, sig, PROXY_PROTO_V2_SIGNATURE_LEN)) { header_version_ = V2; } else if (memcmp(buf, PROXY_PROTO_V1_SIGNATURE, PROXY_PROTO_V1_SIGNATURE_LEN)) { - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } // It is not v2, and can't be v1, so no sense hanging around: it is invalid ENVOY_LOG(debug, "failed to read proxy protocol (exceed max v1 header len)"); return ReadOrParseState::Error; @@ -458,18 +471,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) for (; search_index_ < raw_slice.len_; search_index_++) { if (buf[search_index_] == '\n' && buf[search_index_ - 1] == '\r') { if (search_index_ == 1) { - if (config_.get()->allowRequestsWithoutProxyProtocol()) { - // We need to check if what we have already could be v2 proxy protocol; - // if it cannot be, then we might as well forward now - auto matchv2 = !memcmp(buf, PROXY_PROTO_V2_SIGNATURE, - std::min(PROXY_PROTO_V2_SIGNATURE_LEN, raw_slice.len_)); - if (!matchv2) { - // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can - // safely short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } - } // There is not enough data to determine if it contains the v2 protocol signature, so wait // for more data. break; @@ -478,14 +479,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) search_index_++; } break; - } else if (config_.get()->allowRequestsWithoutProxyProtocol()) { - if (search_index_ < PROXY_PROTO_V1_SIGNATURE_LEN && - buf[search_index_] != PROXY_PROTO_V1_SIGNATURE[search_index_]) { - // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely - // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); - return ReadOrParseState::SkipFilter; - } } } From 84cdc3284671370faf9d3eb5baee2ae6356d3972 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Mon, 2 May 2022 19:58:23 +0000 Subject: [PATCH 48/52] Do all checking in one place up front; remove unneeded check Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index f91e9c9ac067..050b4184ab32 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -417,12 +417,6 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; } - auto sig_len = matchv1 ? PROXY_PROTO_V1_SIGNATURE_LEN : PROXY_PROTO_V2_SIGNATURE_LEN; - if (raw_slice.len_ < sig_len) { - ENVOY_LOG(debug, "request does not have enough bytes to determine if v1 or v2 proxy " - "protocol, waiting for more bytes"); - return ReadOrParseState::TryAgainLater; - } } if (raw_slice.len_ >= PROXY_PROTO_V2_HEADER_LEN) { From 056f74f02706cc6f686ba1916200bfedf436e045 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Tue, 3 May 2022 19:20:05 +0000 Subject: [PATCH 49/52] Add changelog Signed-off-by: Kevin Dorosh --- changelogs/1.23.0.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelogs/1.23.0.yaml b/changelogs/1.23.0.yaml index c63c0b33b13c..673413353e83 100644 --- a/changelogs/1.23.0.yaml +++ b/changelogs/1.23.0.yaml @@ -69,5 +69,8 @@ new_features: - area: on_demand change: | :ref:`OnDemand ` got extended to hold configuration for on-demand cluster discovery. A similar message for :ref:`per-route configuration ` is also added. +- area: proxy_protcol + change: | + added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. deprecated: From 39e7b50492bfcf004b0ae8b31dd2a73e9cf206e7 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Tue, 3 May 2022 19:23:36 +0000 Subject: [PATCH 50/52] Remove current.rst file that was added in bad merge Signed-off-by: Kevin Dorosh --- docs/root/version_history/current.rst | 52 --------------------------- 1 file changed, 52 deletions(-) delete mode 100644 docs/root/version_history/current.rst diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst deleted file mode 100644 index c11264a9f7ad..000000000000 --- a/docs/root/version_history/current.rst +++ /dev/null @@ -1,52 +0,0 @@ -1.23.0 (Pending) -================ - -Incompatible Behavior Changes ------------------------------ -*Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required* - -* tls-inspector: the listener filter tls inspector's stats ``connection_closed`` and ``read_error`` are removed. The new stats are introduced for listener, ``downstream_peek_remote_close`` and ``read_error`` :ref:`listener stats `. - -Minor Behavior Changes ----------------------- -*Changes that may cause incompatibilities for some users, but should not for most* - -* http: the behavior of the :ref:`timeout ` - field has been modified to extend the timeout when *any* frame is received on the owning HTTP/2 - connection. This negates the effect of head-of-line (HOL) blocking for slow connections. If - any frame is received the assumption is that the connection is working. This behavior change - can be reverted by setting the ``envoy.reloadable_features.http2_delay_keepalive_timeout`` runtime - flag to false. -* lua: export symbols of LuaJit by default on Linux. This is useful in cases where you have a lua script - that loads shared object libraries, such as those installed via luarocks. -* thrift: add validate_clusters in :ref:`RouteConfiguration ` to override the default behavior of cluster validation. -* tls: if both :ref:`match_subject_alt_names ` and :ref:`match_typed_subject_alt_names ` are specified, the former (deprecated) field is ignored. Previously, setting both fields would result in an error. -* tls: removed SHA-1 cipher suites from the server-side defaults. - -Bug Fixes ---------- -*Changes expected to improve the state of the world and are unlikely to have negative effects* - -Removed Config or Runtime -------------------------- -*Normally occurs at the end of the* :ref:`deprecation period ` - -* compressor: removed ``envoy.reloadable_features.fix_added_trailers`` and legacy code paths. -* conn pool: removed ``envoy.reloadable_features.conn_pool_delete_when_idle`` and legacy code paths. -* dns: removed ``envoy.reloadable_features.use_dns_ttl`` and legacy code paths. -* ext_authz: removed ``envoy.reloadable_features.http_ext_authz_do_not_skip_direct_response_and_redirect`` runtime guard and legacy code paths. -* http: deprecated ``envoy.reloadable_features.correct_scheme_and_xfp`` and legacy code paths. -* http: deprecated ``envoy.reloadable_features.validate_connect`` and legacy code paths. -* tcp_proxy: removed ``envoy.reloadable_features.new_tcp_connection_pool`` and legacy code paths. - -New Features ------------- -* access_log: added new access_log command operators to retrieve upstream connection information: ``%UPSTREAM_PROTOCOL%``, ``%UPSTREAM_PEER_SUBJECT%``, ``%UPSTREAM_PEER_ISSUER%``, ``%UPSTREAM_TLS_SESSION_ID%``, ``%UPSTREAM_TLS_CIPHER%``, ``%UPSTREAM_TLS_VERSION%``, ``%UPSTREAM_PEER_CERT_V_START%``, ``%UPSTREAM_PEER_CERT_V_END%`` and ``%UPSTREAM_PEER_CERT%``. -* dns_resolver: added :ref:`include_unroutable_families` to the Apple DNS resolver. -* ext_proc: added support for per-route :ref:`grpc_service `. -* on_demand: :ref:`OnDemand ` got extended to hold configuration for on-demand cluster discovery. A similar message for :ref:`per-route configuration ` is also added. -* proxy protocol: added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. -* thrift: added flag to router to control downstream local close. :ref:`close_downstream_on_upstream_error `. - -Deprecated ----------- From 392124e56a162d6cbc0052626d3c1c43930aba48 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Wed, 4 May 2022 16:05:22 +0000 Subject: [PATCH 51/52] PR comments Signed-off-by: Kevin Dorosh --- .../proxy_protocol/v3/proxy_protocol.proto | 10 +++--- changelogs/1.23.0.yaml | 2 +- .../listener/proxy_protocol/proxy_protocol.cc | 11 +++--- .../listener/proxy_protocol/proxy_protocol.h | 2 +- .../proxy_protocol/proxy_protocol_test.cc | 36 ++++++------------- 5 files changed, 25 insertions(+), 36 deletions(-) diff --git a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto index f11607853427..50472e568830 100644 --- a/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto +++ b/api/envoy/extensions/filters/listener/proxy_protocol/v3/proxy_protocol.proto @@ -51,9 +51,11 @@ message ProxyProtocol { // For more information on the security implications of this feature, see // https://www.haproxy.org/download/2.1/doc/proxy-protocol.txt // - // While incredibly rare, requests of 12 or fewer bytes that match the - // proxy protocol v2 signature and requests of 6 or fewer bytes that match - // the proxy protocol v1 signature will timeout (Envoy is unable to - // differentiate these requests from incomplete proxy protocol requests). + // .. attention:: + // + // Requests of 12 or fewer bytes that match the proxy protocol v2 signature + // and requests of 6 or fewer bytes that match the proxy protocol v1 + // signature will timeout (Envoy is unable to differentiate these requests + // from incomplete proxy protocol requests). bool allow_requests_without_proxy_protocol = 2; } diff --git a/changelogs/1.23.0.yaml b/changelogs/1.23.0.yaml index 673413353e83..4fe8bc24c67f 100644 --- a/changelogs/1.23.0.yaml +++ b/changelogs/1.23.0.yaml @@ -71,6 +71,6 @@ new_features: :ref:`OnDemand ` got extended to hold configuration for on-demand cluster discovery. A similar message for :ref:`per-route configuration ` is also added. - area: proxy_protcol change: | - added support for allowing requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. + added :ref:`allow_requests_without_proxy_protocol` to allow requests without proxy protocol on the listener from trusted downstreams as an opt-in flag. deprecated: diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 050b4184ab32..244fe58d15ea 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -77,13 +77,16 @@ Network::FilterStatus Filter::onAccept(Network::ListenerFilterCallbacks& cb) { Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) { const ReadOrParseState read_state = parseBuffer(buffer); - if (read_state == ReadOrParseState::Error) { + switch (read_state) { + case ReadOrParseState::Error: config_->stats_.downstream_cx_proxy_proto_error_.inc(); cb_->socket().ioHandle().close(); return Network::FilterStatus::StopIteration; - } else if (read_state == ReadOrParseState::TryAgainLater) { + case ReadOrParseState::TryAgainLater: return Network::FilterStatus::StopIteration; - } else if (read_state == ReadOrParseState::SkipFilter) { + case ReadOrParseState::SkipFilter: + return Network::FilterStatus::Continue; + default: return Network::FilterStatus::Continue; } return Network::FilterStatus::Continue; @@ -414,7 +417,7 @@ ReadOrParseState Filter::readProxyHeader(Network::ListenerFilterBuffer& buffer) if (!matchv2 && !matchv1) { // The bytes we have seen so far do not match v1 or v2 proxy protocol, so we can safely // short-circuit - ENVOY_LOG(debug, "request does not use v1 or v2 proxy protocol, forwarding as is"); + ENVOY_LOG(trace, "request does not use v1 or v2 proxy protocol, forwarding as is"); return ReadOrParseState::SkipFilter; } } diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h index e374196b8121..4d594244755f 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.h @@ -68,7 +68,7 @@ class Config : public Logger::Loggable { private: absl::flat_hash_map tlv_types_; - const bool allow_requests_without_proxy_protocol_{}; + const bool allow_requests_without_proxy_protocol_; }; using ConfigSharedPtr = std::shared_ptr; diff --git a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc index 2b67e05a3e0f..afeba9505b0a 100644 --- a/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/filters/listener/proxy_protocol/proxy_protocol_test.cc @@ -240,14 +240,12 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocol) { connect(true, &proto_config); std::string msg = "data"; - EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, + ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); // Ensure we attempt parsing byte by byte using `search_index_` - EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); + ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); - expectData(msg); - disconnect(); } @@ -262,13 +260,11 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV1First) { // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to // remember whether the previous bytes were also valid for the signature std::string msg = "PR\r\n"; - EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); - EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); + ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); + ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); - expectData(msg); - disconnect(); } @@ -283,13 +279,11 @@ TEST_P(ProxyProtocolTest, AllowTinyNoProxyProtocolPartialMatchesV2First) { // This ensures our byte by byte parsing (`search_index_`) has persistence built-in to // remember whether the previous bytes were also valid for the signature std::string msg = "\r\nOX"; - EXPECT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); - EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); + ASSERT_GT(PROXY_PROTO_V1_SIGNATURE_LEN, msg.length()); + ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, msg.length()); write(msg); - expectData(msg); - disconnect(); } @@ -301,14 +295,12 @@ TEST_P(ProxyProtocolTest, AllowLargeNoProxyProtocol) { connect(true, &proto_config); std::string msg = "more data more data more data"; - EXPECT_GT(msg.length(), + ASSERT_GT(msg.length(), PROXY_PROTO_V2_HEADER_LEN); // Ensure we attempt parsing as v2 proxy protocol up front // rather than parsing byte by byte using `search_index_` write(msg); - expectData(msg); - disconnect(); } @@ -609,7 +601,7 @@ TEST_P(ProxyProtocolTest, V2ShortV4) { } TEST_P(ProxyProtocolTest, V2ShortV4WithAllowNoProxyProtocol) { - // An ipv4/tcp connection that has incorrect addr-len encoded + // An ipv4/tcp PROXY header that has incorrect addr-len encoded constexpr uint8_t buffer[] = {0x0d, 0x0a, 0x0d, 0x0a, 0x00, 0x0d, 0x0a, 0x51, 0x55, 0x49, 0x54, 0x0a, 0x21, 0x21, 0x00, 0x04, 0x00, 0x08, 0x00, 0x02, 'm', 'o', 'r', 'e', ' ', 'd', 'a', 't', 'a'}; @@ -1157,11 +1149,9 @@ TEST_P(ProxyProtocolTest, PartialV1ReadWithAllowNoProxyProtocol) { write(" 1234\r\n..."); expectData("..."); - EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), "254.254.254.254"); EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); - disconnect(); } @@ -1184,11 +1174,9 @@ TEST_P(ProxyProtocolTest, TinyPartialV1ReadWithAllowNoProxyProtocol) { write(" 1234\r\n..."); expectData("..."); - EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), "254.254.254.254"); EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); - disconnect(); } @@ -1231,7 +1219,7 @@ TEST_P(ProxyProtocolTest, PartialV2ReadWithAllowNoProxyProtocol) { // Using 18 intentionally as it is larger than v2 signature length and divides evenly into // len(buffer) auto buffer_incr_size = 18; - EXPECT_LT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); + ASSERT_LT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) { write(&buffer[i], buffer_incr_size); if (i == 0) { @@ -1240,11 +1228,9 @@ TEST_P(ProxyProtocolTest, PartialV2ReadWithAllowNoProxyProtocol) { } expectData("moredata"); - EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), "1.2.3.4"); EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); - disconnect(); } @@ -1262,7 +1248,7 @@ TEST_P(ProxyProtocolTest, TinyPartialV2ReadWithAllowNoProxyProtocol) { // Using 3 intentionally as it is smaller than v2 signature length and divides evenly into // len(buffer) auto buffer_incr_size = 3; - EXPECT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); + ASSERT_GT(PROXY_PROTO_V2_SIGNATURE_LEN, buffer_incr_size); for (size_t i = 0; i < sizeof(buffer); i += buffer_incr_size) { write(&buffer[i], buffer_incr_size); if (i == 0) { @@ -1271,11 +1257,9 @@ TEST_P(ProxyProtocolTest, TinyPartialV2ReadWithAllowNoProxyProtocol) { } expectData("moredata"); - EXPECT_EQ(server_connection_->connectionInfoProvider().remoteAddress()->ip()->addressAsString(), "1.2.3.4"); EXPECT_TRUE(server_connection_->connectionInfoProvider().localAddressRestored()); - disconnect(); } From 5c50e234f843f03e47eef8efda689899608ff139 Mon Sep 17 00:00:00 2001 From: Kevin Dorosh Date: Thu, 5 May 2022 15:41:16 +0000 Subject: [PATCH 52/52] Ensure every case is handled in switch statement Signed-off-by: Kevin Dorosh --- .../filters/listener/proxy_protocol/proxy_protocol.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc index 244fe58d15ea..22267d1e9c37 100644 --- a/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc +++ b/source/extensions/filters/listener/proxy_protocol/proxy_protocol.cc @@ -86,7 +86,7 @@ Network::FilterStatus Filter::onData(Network::ListenerFilterBuffer& buffer) { return Network::FilterStatus::StopIteration; case ReadOrParseState::SkipFilter: return Network::FilterStatus::Continue; - default: + case ReadOrParseState::Done: return Network::FilterStatus::Continue; } return Network::FilterStatus::Continue;