transport_sockets: fixing alpn for wrapped sockets

docs/root/version_history/current.rst

@@ -21,6 +21,7 @@ Minor Behavior Changes
 * listener: destroy per network filter chain stats when a network filter chain is removed during the listener in place update.
 * quic: add back the support for IETF draft 29 which is guarded via ``envoy.reloadable_features.FLAGS_quic_reloadable_flag_quic_disable_version_draft_29``. It is off by default so Envoy only supports RFCv1 without flipping this runtime guard explicitly. Draft 29 is not recommended for use.
 * router: take elapsed time into account when setting the x-envoy-expected-rq-timeout-ms header for retries, and never send a value that's longer than the request timeout. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.update_expected_rq_timeout_on_retry`` to false.
+* upstream: fixed a bug where auto_config didn't work for wrapped TLS sockets (e.g. if proxy proto were configured for TLS).
 
 Bug Fixes
 ---------

source/extensions/transport_sockets/common/passthrough.h

@@ -9,6 +9,26 @@ namespace Envoy {
 namespace Extensions {
 namespace TransportSockets {
 
+class PassthroughFactory : public Network::TransportSocketFactory {
+public:
+  PassthroughFactory(Network::TransportSocketFactoryPtr&& transport_socket_factory)
+      : transport_socket_factory_(std::move(transport_socket_factory)) {
+    ASSERT(transport_sockiet_factory_ != nullptr);
+  }
+
+  bool implementsSecureTransport() const override {
+    return transport_socket_factory_->implementsSecureTransport();
+  }
+  bool supportsAlpn() const override { return transport_socket_factory_->supportsAlpn(); }
+  bool usesProxyProtocolOptions() const override {
+    return transport_socket_factory_->usesProxyProtocolOptions();
+  }
+
+protected:
+  // The wrapped factory.
+  Network::TransportSocketFactoryPtr transport_socket_factory_;
+};
+
 class PassthroughSocket : public Network::TransportSocket {
 public:
   PassthroughSocket(Network::TransportSocketPtr&& transport_socket);

source/extensions/transport_sockets/proxy_protocol/proxy_protocol.cc

@@ -107,7 +107,7 @@ void UpstreamProxyProtocolSocket::onConnected() {
 
 UpstreamProxyProtocolSocketFactory::UpstreamProxyProtocolSocketFactory(
     Network::TransportSocketFactoryPtr transport_socket_factory, ProxyProtocolConfig config)
-    : transport_socket_factory_(std::move(transport_socket_factory)), config_(config) {}
+    : PassthroughFactory(std::move(transport_socket_factory)), config_(config) {}
 
 Network::TransportSocketPtr UpstreamProxyProtocolSocketFactory::createTransportSocket(
     Network::TransportSocketOptionsConstSharedPtr options) const {
@@ -119,10 +119,6 @@ Network::TransportSocketPtr UpstreamProxyProtocolSocketFactory::createTransportS
                                                        config_.version());
 }
 
-bool UpstreamProxyProtocolSocketFactory::implementsSecureTransport() const {
-  return transport_socket_factory_->implementsSecureTransport();
-}
-
 } // namespace ProxyProtocol
 } // namespace TransportSockets
 } // namespace Extensions

source/extensions/transport_sockets/proxy_protocol/proxy_protocol.h

@@ -39,19 +39,17 @@ class UpstreamProxyProtocolSocket : public TransportSockets::PassthroughSocket,
   ProxyProtocolConfig_Version version_{ProxyProtocolConfig_Version::ProxyProtocolConfig_Version_V1};
 };
 
-class UpstreamProxyProtocolSocketFactory : public Network::TransportSocketFactory {
+class UpstreamProxyProtocolSocketFactory : public PassthroughFactory {
 public:
   UpstreamProxyProtocolSocketFactory(Network::TransportSocketFactoryPtr transport_socket_factory,
                                      ProxyProtocolConfig config);
 
   // Network::TransportSocketFactory
   Network::TransportSocketPtr
   createTransportSocket(Network::TransportSocketOptionsConstSharedPtr options) const override;
-  bool implementsSecureTransport() const override;
   bool usesProxyProtocolOptions() const override { return true; }
 
 private:
-  Network::TransportSocketFactoryPtr transport_socket_factory_;
   ProxyProtocolConfig config_;
 };
 

source/extensions/transport_sockets/tap/tap.cc

@@ -54,22 +54,14 @@ TapSocketFactory::TapSocketFactory(
     Network::TransportSocketFactoryPtr&& transport_socket_factory)
     : ExtensionConfigBase(proto_config.common_config(), std::move(config_factory), admin,
                           singleton_manager, tls, main_thread_dispatcher),
-      transport_socket_factory_(std::move(transport_socket_factory)) {}
+      PassthroughFactory(std::move(transport_socket_factory)) {}
 
 Network::TransportSocketPtr TapSocketFactory::createTransportSocket(
     Network::TransportSocketOptionsConstSharedPtr options) const {
   return std::make_unique<TapSocket>(currentConfigHelper<SocketTapConfig>(),
                                      transport_socket_factory_->createTransportSocket(options));
 }
 
-bool TapSocketFactory::implementsSecureTransport() const {
-  return transport_socket_factory_->implementsSecureTransport();
-}
-
-bool TapSocketFactory::usesProxyProtocolOptions() const {
-  return transport_socket_factory_->usesProxyProtocolOptions();
-}
-
 } // namespace Tap
 } // namespace TransportSockets
 } // namespace Extensions

source/extensions/transport_sockets/tap/tap.h

@@ -28,8 +28,7 @@ class TapSocket : public TransportSockets::PassthroughSocket {
   PerSocketTapperPtr tapper_;
 };
 
-class TapSocketFactory : public Network::TransportSocketFactory,
-                         public Common::Tap::ExtensionConfigBase {
+class TapSocketFactory : public Common::Tap::ExtensionConfigBase, public PassthroughFactory {
 public:
   TapSocketFactory(const envoy::extensions::transport_sockets::tap::v3::Tap& proto_config,
                    Common::Tap::TapConfigFactoryPtr&& config_factory, Server::Admin& admin,
@@ -40,11 +39,6 @@ class TapSocketFactory : public Network::TransportSocketFactory,
   // Network::TransportSocketFactory
   Network::TransportSocketPtr
   createTransportSocket(Network::TransportSocketOptionsConstSharedPtr options) const override;
-  bool implementsSecureTransport() const override;
-  bool usesProxyProtocolOptions() const override;
-
-private:
-  Network::TransportSocketFactoryPtr transport_socket_factory_;
 };
 
 } // namespace Tap

source/extensions/transport_sockets/tcp_stats/config.cc

@@ -16,7 +16,7 @@ TcpStatsSocketFactory::TcpStatsSocketFactory(
     Server::Configuration::TransportSocketFactoryContext& context,
     const envoy::extensions::transport_sockets::tcp_stats::v3::Config& config,
     Network::TransportSocketFactoryPtr&& inner_factory)
-    : inner_factory_(std::move(inner_factory)) {
+    : PassthroughFactory(std::move(inner_factory)) {
 #if defined(__linux__)
   config_ = std::make_shared<Config>(config, context.scope());
 #else
@@ -29,7 +29,7 @@ TcpStatsSocketFactory::TcpStatsSocketFactory(
 Network::TransportSocketPtr TcpStatsSocketFactory::createTransportSocket(
     Network::TransportSocketOptionsConstSharedPtr options) const {
 #if defined(__linux__)
-  auto inner_socket = inner_factory_->createTransportSocket(options);
+  auto inner_socket = transport_socket_factory_->createTransportSocket(options);
   if (inner_socket == nullptr) {
     return nullptr;
   }
@@ -40,14 +40,6 @@ Network::TransportSocketPtr TcpStatsSocketFactory::createTransportSocket(
 #endif
 }
 
-bool TcpStatsSocketFactory::implementsSecureTransport() const {
-  return inner_factory_->implementsSecureTransport();
-}
-
-bool TcpStatsSocketFactory::usesProxyProtocolOptions() const {
-  return inner_factory_->usesProxyProtocolOptions();
-}
-
 class TcpStatsConfigFactory : public virtual Server::Configuration::TransportSocketConfigFactory {
 public:
   std::string name() const override { return "envoy.transport_sockets.tcp_stats"; }

source/extensions/transport_sockets/tcp_stats/config.h

@@ -3,26 +3,24 @@
 #include "envoy/extensions/transport_sockets/tcp_stats/v3/tcp_stats.pb.h"
 #include "envoy/server/transport_socket_config.h"
 
+#include "source/extensions/transport_sockets/common/passthrough.h"
 #include "source/extensions/transport_sockets/tcp_stats/tcp_stats.h"
 
 namespace Envoy {
 namespace Extensions {
 namespace TransportSockets {
 namespace TcpStats {
 
-class TcpStatsSocketFactory : public Network::TransportSocketFactory {
+class TcpStatsSocketFactory : public PassthroughFactory {
 public:
   TcpStatsSocketFactory(Server::Configuration::TransportSocketFactoryContext& context,
                         const envoy::extensions::transport_sockets::tcp_stats::v3::Config& config,
                         Network::TransportSocketFactoryPtr&& inner_factory);
 
   Network::TransportSocketPtr
   createTransportSocket(Network::TransportSocketOptionsConstSharedPtr options) const override;
-  bool implementsSecureTransport() const override;
-  bool usesProxyProtocolOptions() const override;
 
 private:
-  Network::TransportSocketFactoryPtr inner_factory_;
 #if defined(__linux__)
   ConfigConstSharedPtr config_;
 #endif

test/extensions/transport_sockets/common/passthrough_test.cc

@@ -88,6 +88,26 @@ TEST_F(PassthroughTest, FailOnStartSecureTransport) {
   EXPECT_FALSE(passthrough_socket_->startSecureTransport());
 }
 
+TEST(PassthroughFactoryTest, TestDelegation) {
+  auto inner_factory_ptr = std::make_unique<NiceMock<Network::MockTransportSocketFactory>>();
+  Network::MockTransportSocketFactory* inner_factory = inner_factory_ptr.get();
+  Network::TransportSocketFactoryPtr factory{std::move(inner_factory_ptr)};
+
+  {
+    EXPECT_CALL(*inner_factory, implementsSecureTransport());
+    factory->implementsSecureTransport();
+  }
+
+  {
+    EXPECT_CALL(*inner_factory, supportsAlpn());
+    factory->supportsAlpn();
+  }
+  {
+    EXPECT_CALL(*inner_factory, usesProxyProtocolOptions());
+    factory->usesProxyProtocolOptions();
+  }
+}
+
 } // namespace
 } // namespace TransportSockets
 } // namespace Extensions