diff --git a/include/envoy/network/transport_socket.h b/include/envoy/network/transport_socket.h index 95d07ac2e182..4244e6247767 100644 --- a/include/envoy/network/transport_socket.h +++ b/include/envoy/network/transport_socket.h @@ -180,7 +180,7 @@ class TransportSocketOptions { virtual const std::vector& applicationProtocolListOverride() const PURE; /** - * The application protocol to use when negotiating an upstream connection and no other + * The application protocol(s) to use when negotiating an upstream connection and no other * application protocol has been configured. Both * TransportSocketOptions::applicationProtocolListOverride and application protocols configured * in the CommonTlsContext on the Cluster will take precedence. @@ -188,10 +188,10 @@ class TransportSocketOptions { * Note that this option is intended for intermediate code (e.g. the HTTP connection pools) to * specify a default ALPN when no specific values are specified elsewhere. As such, providing a * value here might not make sense prior to load balancing. - * @return the optional fallback for application protocols, for when they are not specified in the - * TLS configuration. + * @return the optional fallback(s) for application protocols, for when they are not specified in + * the TLS configuration. */ - virtual const absl::optional& applicationProtocolFallback() const PURE; + virtual const std::vector& applicationProtocolFallback() const PURE; /** * @return optional PROXY protocol address information. diff --git a/source/common/http/conn_pool_base.cc b/source/common/http/conn_pool_base.cc index 06e61af75ff9..e785f87a7b0e 100644 --- a/source/common/http/conn_pool_base.cc +++ b/source/common/http/conn_pool_base.cc @@ -12,35 +12,37 @@ namespace Http { Network::TransportSocketOptionsSharedPtr wrapTransportSocketOptions(Network::TransportSocketOptionsSharedPtr transport_socket_options, - Protocol protocol) { + std::vector protocols) { if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.http_default_alpn")) { return transport_socket_options; } - // If configured to do so, we override the ALPN to use for the upstream connection to match the - // selected protocol. - std::string alpn; - switch (protocol) { - case Http::Protocol::Http10: - NOT_REACHED_GCOVR_EXCL_LINE; - case Http::Protocol::Http11: - alpn = Http::Utility::AlpnNames::get().Http11; - break; - case Http::Protocol::Http2: - alpn = Http::Utility::AlpnNames::get().Http2; - break; - case Http::Protocol::Http3: - // TODO(snowp): Add once HTTP/3 upstream support is added. - NOT_IMPLEMENTED_GCOVR_EXCL_LINE; - break; + std::vector fallbacks; + for (auto protocol : protocols) { + // If configured to do so, we override the ALPN to use for the upstream connection to match the + // selected protocol. + switch (protocol) { + case Http::Protocol::Http10: + NOT_REACHED_GCOVR_EXCL_LINE; + case Http::Protocol::Http11: + fallbacks.push_back(Http::Utility::AlpnNames::get().Http11); + break; + case Http::Protocol::Http2: + fallbacks.push_back(Http::Utility::AlpnNames::get().Http2); + break; + case Http::Protocol::Http3: + // TODO(snowp): Add once HTTP/3 upstream support is added. + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + break; + } } if (transport_socket_options) { return std::make_shared( - std::move(alpn), transport_socket_options); + std::move(fallbacks), transport_socket_options); } else { return std::make_shared( - "", std::vector{}, std::vector{}, std::move(alpn)); + "", std::vector{}, std::vector{}, std::move(fallbacks)); } } @@ -48,11 +50,18 @@ HttpConnPoolImplBase::HttpConnPoolImplBase( Upstream::HostConstSharedPtr host, Upstream::ResourcePriority priority, Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, - Random::RandomGenerator& random_generator, Http::Protocol protocol) + Random::RandomGenerator& random_generator, std::vector protocols) : Envoy::ConnectionPool::ConnPoolImplBase( host, priority, dispatcher, options, - wrapTransportSocketOptions(transport_socket_options, protocol)), - random_generator_(random_generator), protocol_(protocol) {} + wrapTransportSocketOptions(transport_socket_options, protocols)), + random_generator_(random_generator) { + ASSERT(!protocols.empty()); + // TODO(alyssawilk) the protocol function should probably be an optional and + // simply not set if there's more than one and ALPN has not been negotiated. + if (!protocols.empty()) { + protocol_ = protocols[0]; + } +} ConnectionPool::Cancellable* HttpConnPoolImplBase::newStream(Http::ResponseDecoder& response_decoder, diff --git a/source/common/http/conn_pool_base.h b/source/common/http/conn_pool_base.h index 10610ed96cac..16e4f39ac103 100644 --- a/source/common/http/conn_pool_base.h +++ b/source/common/http/conn_pool_base.h @@ -43,7 +43,8 @@ class HttpConnPoolImplBase : public Envoy::ConnectionPool::ConnPoolImplBase, Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options, - Random::RandomGenerator& random_generator, Http::Protocol protocol); + Random::RandomGenerator& random_generator, + std::vector protocol); // ConnectionPool::Instance void addDrainedCallback(DrainedCb cb) override { addDrainedCallbackImpl(cb); } diff --git a/source/common/http/http1/conn_pool.cc b/source/common/http/http1/conn_pool.cc index ce0c0fce1cd4..3aaf5aa2b572 100644 --- a/source/common/http/http1/conn_pool.cc +++ b/source/common/http/http1/conn_pool.cc @@ -28,7 +28,7 @@ ConnPoolImpl::ConnPoolImpl(Event::Dispatcher& dispatcher, Random::RandomGenerato const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options) : HttpConnPoolImplBase(std::move(host), std::move(priority), dispatcher, options, - transport_socket_options, random_generator, Protocol::Http11) {} + transport_socket_options, random_generator, {Protocol::Http11}) {} ConnPoolImpl::~ConnPoolImpl() { destructAllConnections(); } diff --git a/source/common/http/http2/conn_pool.cc b/source/common/http/http2/conn_pool.cc index 29f89bb60c32..b4e3be74a46f 100644 --- a/source/common/http/http2/conn_pool.cc +++ b/source/common/http/http2/conn_pool.cc @@ -21,7 +21,7 @@ ConnPoolImpl::ConnPoolImpl(Event::Dispatcher& dispatcher, Random::RandomGenerato const Network::ConnectionSocket::OptionsSharedPtr& options, const Network::TransportSocketOptionsSharedPtr& transport_socket_options) : HttpConnPoolImplBase(std::move(host), std::move(priority), dispatcher, options, - transport_socket_options, random_generator, Protocol::Http2) {} + transport_socket_options, random_generator, {Protocol::Http2}) {} ConnPoolImpl::~ConnPoolImpl() { destructAllConnections(); } diff --git a/source/common/network/transport_socket_options_impl.cc b/source/common/network/transport_socket_options_impl.cc index 66b139bba6e1..9f158f416bf0 100644 --- a/source/common/network/transport_socket_options_impl.cc +++ b/source/common/network/transport_socket_options_impl.cc @@ -38,8 +38,10 @@ void commonHashKey(const TransportSocketOptions& options, std::vector application_protocols; std::vector subject_alt_names; + std::vector alpn_fallback; absl::optional proxy_protocol_options; bool needs_transport_socket_options = false; @@ -100,8 +103,8 @@ TransportSocketOptionsUtility::fromFilterState(const StreamInfo::FilterState& fi if (needs_transport_socket_options) { return std::make_shared( - server_name, std::move(subject_alt_names), std::move(application_protocols), absl::nullopt, - proxy_protocol_options); + server_name, std::move(subject_alt_names), std::move(application_protocols), + std::move(alpn_fallback), proxy_protocol_options); } else { return nullptr; } diff --git a/source/common/network/transport_socket_options_impl.h b/source/common/network/transport_socket_options_impl.h index 4e08da3f292d..8a961e4fa33d 100644 --- a/source/common/network/transport_socket_options_impl.h +++ b/source/common/network/transport_socket_options_impl.h @@ -10,7 +10,7 @@ namespace Network { // A wrapper around another TransportSocketOptions that overrides the ALPN fallback. class AlpnDecoratingTransportSocketOptions : public TransportSocketOptions { public: - AlpnDecoratingTransportSocketOptions(std::string&& alpn, + AlpnDecoratingTransportSocketOptions(std::vector&& alpn, TransportSocketOptionsSharedPtr inner_options) : alpn_fallback_(std::move(alpn)), inner_options_(std::move(inner_options)) {} // Network::TransportSocketOptions @@ -23,7 +23,7 @@ class AlpnDecoratingTransportSocketOptions : public TransportSocketOptions { const std::vector& applicationProtocolListOverride() const override { return inner_options_->applicationProtocolListOverride(); } - const absl::optional& applicationProtocolFallback() const override { + const std::vector& applicationProtocolFallback() const override { return alpn_fallback_; } absl::optional proxyProtocolOptions() const override { @@ -33,7 +33,7 @@ class AlpnDecoratingTransportSocketOptions : public TransportSocketOptions { const Network::TransportSocketFactory& factory) const override; private: - const absl::optional alpn_fallback_; + const std::vector alpn_fallback_; const TransportSocketOptionsSharedPtr inner_options_; }; @@ -42,8 +42,7 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { TransportSocketOptionsImpl( absl::string_view override_server_name = "", std::vector&& override_verify_san_list = {}, - std::vector&& override_alpn = {}, - absl::optional&& fallback_alpn = {}, + std::vector&& override_alpn = {}, std::vector&& fallback_alpn = {}, absl::optional proxy_proto_options = absl::nullopt) : override_server_name_(override_server_name.empty() ? absl::nullopt @@ -62,7 +61,7 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { const std::vector& applicationProtocolListOverride() const override { return override_alpn_list_; } - const absl::optional& applicationProtocolFallback() const override { + const std::vector& applicationProtocolFallback() const override { return alpn_fallback_; } absl::optional proxyProtocolOptions() const override { @@ -75,7 +74,7 @@ class TransportSocketOptionsImpl : public TransportSocketOptions { const absl::optional override_server_name_; const std::vector override_verify_san_list_; const std::vector override_alpn_list_; - const absl::optional alpn_fallback_; + const std::vector alpn_fallback_; const absl::optional proxy_protocol_options_; }; diff --git a/source/extensions/transport_sockets/tls/context_impl.cc b/source/extensions/transport_sockets/tls/context_impl.cc index cfcf1d6ce16a..e163eddfc8b4 100644 --- a/source/extensions/transport_sockets/tls/context_impl.cc +++ b/source/extensions/transport_sockets/tls/context_impl.cc @@ -963,9 +963,9 @@ bssl::UniquePtr ClientContextImpl::newSsl(const Network::TransportSocketOpt has_alpn_defined |= parseAndSetAlpn(options->applicationProtocolListOverride(), *ssl_con); } - if (options && !has_alpn_defined && options->applicationProtocolFallback().has_value()) { + if (options && !has_alpn_defined && !options->applicationProtocolFallback().empty()) { // If ALPN hasn't already been set (either through TLS context or override), use the fallback. - parseAndSetAlpn({*options->applicationProtocolFallback()}, *ssl_con); + parseAndSetAlpn(options->applicationProtocolFallback(), *ssl_con); } if (allow_renegotiation_) { diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 7b19f269bbdd..ea256d23386d 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -397,6 +397,22 @@ envoy_cc_test( ], ) +envoy_cc_test( + name = "mixed_conn_pool_test", + srcs = ["mixed_conn_pool_test.cc"], + deps = [ + "//source/common/http:conn_pool_base_lib", + "//test/common/upstream:utility_lib", + "//test/mocks:common_lib", + "//test/mocks/buffer:buffer_mocks", + "//test/mocks/http:http_mocks", + "//test/mocks/local_info:local_info_mocks", + "//test/mocks/router:router_mocks", + "//test/mocks/runtime:runtime_mocks", + "//test/mocks/stats:stats_mocks", + ], +) + envoy_proto_library( name = "path_utility_fuzz_proto", srcs = ["path_utility_fuzz.proto"], diff --git a/test/common/http/http1/conn_pool_test.cc b/test/common/http/http1/conn_pool_test.cc index 072a1b807d1b..0d02cc51e36b 100644 --- a/test/common/http/http1/conn_pool_test.cc +++ b/test/common/http/http1/conn_pool_test.cc @@ -274,7 +274,7 @@ TEST_F(Http1ConnPoolImplTest, VerifyAlpnFallback) { .WillOnce(Invoke( [](Network::TransportSocketOptionsSharedPtr options) -> Network::TransportSocketPtr { EXPECT_TRUE(options != nullptr); - EXPECT_EQ(options->applicationProtocolFallback(), + EXPECT_EQ(options->applicationProtocolFallback()[0], Http::Utility::AlpnNames::get().Http11); return std::make_unique(); })); diff --git a/test/common/http/http2/conn_pool_test.cc b/test/common/http/http2/conn_pool_test.cc index dd5952b2a227..569991842e89 100644 --- a/test/common/http/http2/conn_pool_test.cc +++ b/test/common/http/http2/conn_pool_test.cc @@ -329,7 +329,7 @@ TEST_F(Http2ConnPoolImplTest, VerifyAlpnFallback) { .WillOnce(Invoke( [](Network::TransportSocketOptionsSharedPtr options) -> Network::TransportSocketPtr { EXPECT_TRUE(options != nullptr); - EXPECT_EQ(options->applicationProtocolFallback(), + EXPECT_EQ(options->applicationProtocolFallback()[0], Http::Utility::AlpnNames::get().Http2); return std::make_unique(); })); diff --git a/test/common/http/mixed_conn_pool_test.cc b/test/common/http/mixed_conn_pool_test.cc new file mode 100644 index 000000000000..f48b37afdcf7 --- /dev/null +++ b/test/common/http/mixed_conn_pool_test.cc @@ -0,0 +1,65 @@ +#include + +#include "common/http/conn_pool_base.h" +#include "common/http/utility.h" + +#include "test/common/upstream/utility.h" +#include "test/mocks/common.h" +#include "test/mocks/event/mocks.h" +#include "test/mocks/runtime/mocks.h" +#include "test/mocks/upstream/cluster_info.h" +#include "test/test_common/simulated_time_system.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Http { +namespace { + +// TODO(alyssawilk) replace this with the MixedConnectionPool once it lands. +class ConnPoolImplForTest : public HttpConnPoolImplBase { +public: + ConnPoolImplForTest(Event::MockDispatcher& dispatcher, Random::RandomGenerator& random, + Upstream::ClusterInfoConstSharedPtr cluster) + : HttpConnPoolImplBase(Upstream::makeTestHost(cluster, "tcp://127.0.0.1:9000"), + Upstream::ResourcePriority::Default, dispatcher, nullptr, nullptr, + random, {Http::Protocol::Http2, Http::Protocol::Http11}) {} + + Envoy::ConnectionPool::ActiveClientPtr instantiateActiveClient() override { return nullptr; } + Http::Protocol protocol() const override { return Http::Protocol::Http2; } + CodecClientPtr createCodecClient(Upstream::Host::CreateConnectionData&) override { + return nullptr; + } +}; + +/** + * Test fixture for a connection pool which can have HTTP/2 or HTTP/1.1 connections. + */ +class MixedConnPoolImplTest : public testing::Test { +public: + MixedConnPoolImplTest() + : conn_pool_(std::make_unique(dispatcher_, random_, cluster_)) {} + + ~MixedConnPoolImplTest() override { + EXPECT_EQ("", TestUtility::nonZeroedGauges(cluster_->stats_store_.gauges())); + } + + NiceMock dispatcher_; + std::shared_ptr cluster_{new NiceMock()}; + std::unique_ptr conn_pool_; + NiceMock runtime_; + NiceMock random_; +}; + +TEST_F(MixedConnPoolImplTest, AlpnTest) { + auto& fallback = conn_pool_->transportSocketOptions()->applicationProtocolFallback(); + ASSERT_EQ(2, fallback.size()); + EXPECT_EQ(fallback[0], Http::Utility::AlpnNames::get().Http2); + EXPECT_EQ(fallback[1], Http::Utility::AlpnNames::get().Http11); +} + +} // namespace +} // namespace Http +} // namespace Envoy diff --git a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc index 953e5999a5fb..16f56b4f9489 100644 --- a/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc +++ b/test/extensions/transport_sockets/proxy_protocol/proxy_protocol_test.cc @@ -256,7 +256,7 @@ TEST_F(ProxyProtocolTest, V1IPV4DownstreamAddresses) { new Network::Address::Ipv4Instance("174.2.2.222", 80)); Network::TransportSocketOptionsSharedPtr socket_options = std::make_shared( - "", std::vector{}, std::vector{}, absl::nullopt, + "", std::vector{}, std::vector{}, std::vector{}, absl::optional( Network::ProxyProtocolData{src_addr, dst_addr})); transport_callbacks_.connection_.local_address_ = @@ -288,7 +288,7 @@ TEST_F(ProxyProtocolTest, V1IPV6DownstreamAddresses) { Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv6Instance("a:b:c:d::", 80)); Network::TransportSocketOptionsSharedPtr socket_options = std::make_shared( - "", std::vector{}, std::vector{}, absl::nullopt, + "", std::vector{}, std::vector{}, std::vector{}, absl::optional( Network::ProxyProtocolData{src_addr, dst_addr})); transport_callbacks_.connection_.local_address_ = @@ -365,7 +365,7 @@ TEST_F(ProxyProtocolTest, V2IPV4DownstreamAddresses) { Network::Address::InstanceConstSharedPtr(new Network::Address::Ipv4Instance("0.1.1.2", 513)); Network::TransportSocketOptionsSharedPtr socket_options = std::make_shared( - "", std::vector{}, std::vector{}, absl::nullopt, + "", std::vector{}, std::vector{}, std::vector{}, absl::optional( Network::ProxyProtocolData{src_addr, dst_addr})); transport_callbacks_.connection_.local_address_ = @@ -397,7 +397,7 @@ TEST_F(ProxyProtocolTest, V2IPV6DownstreamAddresses) { new Network::Address::Ipv6Instance("1:100:200:3::", 2)); Network::TransportSocketOptionsSharedPtr socket_options = std::make_shared( - "", std::vector{}, std::vector{}, absl::nullopt, + "", std::vector{}, std::vector{}, std::vector{}, absl::optional( Network::ProxyProtocolData{src_addr, dst_addr})); transport_callbacks_.connection_.local_address_ = @@ -429,7 +429,7 @@ TEST_F(ProxyProtocolTest, OnConnectedCallsInnerOnConnected) { new Network::Address::Ipv6Instance("1:100:200:3::", 2)); Network::TransportSocketOptionsSharedPtr socket_options = std::make_shared( - "", std::vector{}, std::vector{}, absl::nullopt, + "", std::vector{}, std::vector{}, std::vector{}, absl::optional( Network::ProxyProtocolData{src_addr, dst_addr})); transport_callbacks_.connection_.local_address_ = @@ -473,4 +473,4 @@ TEST_F(ProxyProtocolSocketFactoryTest, ImplementsSecureTransportCallInnerFactory } // namespace ProxyProtocol } // namespace TransportSockets } // namespace Extensions -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 8aa58b0cdc76..b96fc6366330 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -4648,7 +4648,22 @@ TEST_P(SslSocketTest, OverrideApplicationProtocols) { // in the config. server_ctx->add_alpn_protocols("test"); transport_socket_options = std::make_shared( - "", std::vector{}, std::vector{}, "test"); + "", std::vector{}, std::vector{}, std::vector{"test"}); + testUtilV2(test_options.setExpectedALPNProtocol("test").setTransportSocketOptions( + transport_socket_options)); + + // With multiple fallbacks specified, a single match will match. + transport_socket_options = std::make_shared( + "", std::vector{}, std::vector{}, + std::vector{"foo", "test"}); + testUtilV2(test_options.setExpectedALPNProtocol("test").setTransportSocketOptions( + transport_socket_options)); + + // With multiple matching fallbacks specified, a single match will match. + server_ctx->add_alpn_protocols("foo"); + transport_socket_options = std::make_shared( + "", std::vector{}, std::vector{}, + std::vector{"foo", "test"}); testUtilV2(test_options.setExpectedALPNProtocol("test").setTransportSocketOptions( transport_socket_options));