diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index a92d88f55707d..4cc773a334fdc 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -60,6 +60,7 @@ Bug Fixes * access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value. * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion `_. * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false. +* dynamic forward proxy: fixing a validation bug where san and sni checks were not applied setting :ref:`http_protocol_options ` via :ref:`typed_extension_protocol_options `. * ext_authz: fix the ext_authz filter to correctly merge multiple same headers using the ',' as separator in the check request to the external authorization service. * hcm: remove deprecation for :ref:`xff_num_trusted_hops ` and forbid mixing ip detection extensions with old related knobs. * http: limit use of deferred resets in the http2 codec to server-side connections. Use of deferred reset for client connections can result in incorrect behavior and performance problems. diff --git a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc index 4d7adefbd5d64..cc72fc392466a 100644 --- a/source/extensions/clusters/dynamic_forward_proxy/cluster.cc +++ b/source/extensions/clusters/dynamic_forward_proxy/cluster.cc @@ -6,6 +6,7 @@ #include "source/common/network/transport_socket_options_impl.h" #include "source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h" +#include "source/extensions/filters/network/common/utility.h" namespace Envoy { namespace Extensions { @@ -179,15 +180,10 @@ ClusterFactory::createClusterWithConfig( context.singletonManager(), context.dispatcher(), context.tls(), context.api(), context.runtime(), context.stats(), context.messageValidationVisitor()); envoy::config::cluster::v3::Cluster cluster_config = cluster; - if (cluster_config.has_upstream_http_protocol_options()) { - if (!proto_config.allow_insecure_cluster_options() && - (!cluster_config.upstream_http_protocol_options().auto_sni() || - !cluster_config.upstream_http_protocol_options().auto_san_validation())) { - throw EnvoyException( - "dynamic_forward_proxy cluster must have auto_sni and auto_san_validation true when " - "configured with upstream_http_protocol_options"); - } - } else { + if (!cluster_config.has_upstream_http_protocol_options()) { + // This sets defaults which will only apply if using old style http config. + // They will be a no-op if typed_extension_protocol_options are used for + // http config. cluster_config.mutable_upstream_http_protocol_options()->set_auto_sni(true); cluster_config.mutable_upstream_http_protocol_options()->set_auto_san_validation(true); } @@ -195,6 +191,18 @@ ClusterFactory::createClusterWithConfig( auto new_cluster = std::make_shared( cluster_config, proto_config, context.runtime(), cache_manager_factory, context.localInfo(), socket_factory_context, std::move(stats_scope), context.addedViaApi()); + + auto& options = new_cluster->info()->upstreamHttpProtocolOptions(); + + if (!proto_config.allow_insecure_cluster_options()) { + if (!options.has_value() || + (!options.value().auto_sni() || !options.value().auto_san_validation())) { + throw EnvoyException( + "dynamic_forward_proxy cluster must have auto_sni and auto_san_validation true unless " + "allow_insecure_cluster_options is set."); + } + } + auto lb = std::make_unique(*new_cluster); return std::make_pair(new_cluster, std::move(lb)); } diff --git a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc index 6f9a68db1dcef..db7407c4ed32b 100644 --- a/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc +++ b/test/extensions/clusters/dynamic_forward_proxy/cluster_test.cc @@ -249,8 +249,8 @@ upstream_http_protocol_options: {} EXPECT_THROW_WITH_MESSAGE( createCluster(yaml_config), EnvoyException, - "dynamic_forward_proxy cluster must have auto_sni and auto_san_validation true when " - "configured with upstream_http_protocol_options"); + "dynamic_forward_proxy cluster must have auto_sni and auto_san_validation true unless " + "allow_insecure_cluster_options is set."); } TEST_F(ClusterFactoryTest, InsecureUpstreamHttpProtocolOptions) { diff --git a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc index aea96c7aa5d85..6b6ff51010c10 100644 --- a/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc +++ b/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test.cc @@ -62,6 +62,12 @@ name: dynamic_forward_proxy cluster_.set_name("cluster_0"); cluster_.set_lb_policy(envoy::config::cluster::v3::Cluster::CLUSTER_PROVIDED); + ConfigHelper::HttpProtocolOptions protocol_options; + protocol_options.mutable_upstream_http_protocol_options()->set_auto_sni(true); + protocol_options.mutable_upstream_http_protocol_options()->set_auto_san_validation(true); + protocol_options.mutable_explicit_http_config()->mutable_http_protocol_options(); + ConfigHelper::setProtocolOptions(cluster_, protocol_options); + if (upstream_tls_) { envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; auto* validation_context =