quic: use sds for upstream http/3 (envoyproxy#16462)

Risk Level: Medium (some data plane refactors, mostly no-ops for !HTTP/3)
Testing: turned up HTTP/3 upstream SDS integration tests
Docs Changes: n/a
Release Notes: n/a
part of envoyproxy#14829

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>

source/common/conn_pool/conn_pool_base.cc

@@ -142,6 +142,10 @@ ConnPoolImplBase::tryCreateNewConnection(float global_preconnect_ratio) {
       (ready_clients_.empty() && busy_clients_.empty() && connecting_clients_.empty())) {
     ENVOY_LOG(debug, "creating a new connection");
     ActiveClientPtr client = instantiateActiveClient();
+    if (client.get() == nullptr) {
+      ENVOY_LOG(trace, "connection creation failed");
+      return ConnectionResult::FailedToCreateConnection;
+    }
     ASSERT(client->state() == ActiveClient::State::CONNECTING);
     ASSERT(std::numeric_limits<uint64_t>::max() - connecting_stream_capacity_ >=
            client->effectiveConcurrentStreamLimit());
@@ -249,9 +253,19 @@ ConnectionPool::Cancellable* ConnPoolImplBase::newStream(AttachContext& context)
     // increase capacity is if the connection limits are exceeded.
     ENVOY_BUG(pending_streams_.size() <= connecting_stream_capacity_ ||
                   connecting_stream_capacity_ > old_capacity ||
-                  result == ConnectionResult::NoConnectionRateLimited,
+                  (result == ConnectionResult::NoConnectionRateLimited ||
+                   result == ConnectionResult::FailedToCreateConnection),
               fmt::format("Failed to create expected connection: {}", *this));
-    return pending;
+    if (result == ConnectionResult::FailedToCreateConnection) {
+      // This currently only happens for HTTP/3 if secrets aren't yet loaded.
+      // Trigger connection failure.
+      pending->cancel(Envoy::ConnectionPool::CancelPolicy::CloseExcess);
+      onPoolFailure(nullptr, absl::string_view(), ConnectionPool::PoolFailureReason::Overflow,
+                    context);
+      return nullptr;
+    } else {
+      return pending;
+    }
   } else {
     ENVOY_LOG(debug, "max pending streams overflow");
     onPoolFailure(nullptr, absl::string_view(), ConnectionPool::PoolFailureReason::Overflow,

source/common/conn_pool/conn_pool_base.h

@@ -248,6 +248,7 @@ class ConnPoolImplBase : protected Logger::Loggable<Logger::Id::pool> {
   virtual void onConnected(Envoy::ConnectionPool::ActiveClient&) {}
 
   enum class ConnectionResult {
+    FailedToCreateConnection,
     CreatedNewConnection,
     ShouldNotConnect,
     NoConnectionRateLimited,

source/common/http/http3/conn_pool.cc

@@ -15,6 +15,7 @@
 #ifdef ENVOY_ENABLE_QUIC
 #include "common/quic/client_connection_factory_impl.h"
 #include "common/quic/envoy_quic_utils.h"
+#include "common/quic/quic_transport_socket_factory.h"
 #else
 #error "http3 conn pool should not be built with QUIC disabled"
 #endif
@@ -64,7 +65,13 @@ allocateConnPool(Event::Dispatcher& dispatcher, Random::RandomGenerator& random_
                  Upstream::ClusterConnectivityState& state, TimeSource& time_source) {
   return std::make_unique<Http3ConnPoolImpl>(
       host, priority, dispatcher, options, transport_socket_options, random_generator, state,
-      [](HttpConnPoolImplBase* pool) {
+      [](HttpConnPoolImplBase* pool) -> ::Envoy::ConnectionPool::ActiveClientPtr {
+        // If there's no ssl context, the secrets are not loaded. Fast-fail by returning null.
+        auto factory = &pool->host()->transportSocketFactory();
+        ASSERT(dynamic_cast<Quic::QuicClientTransportSocketFactory*>(factory) != nullptr);
+        if (static_cast<Quic::QuicClientTransportSocketFactory*>(factory)->sslCtx() == nullptr) {
+          return nullptr;
+        }
         Http3ConnPoolImpl* h3_pool = reinterpret_cast<Http3ConnPoolImpl*>(pool);
         Upstream::Host::CreateConnectionData data{};
         data.host_description_ = pool->host();

source/common/quic/client_connection_factory_impl.cc

@@ -19,19 +19,34 @@ getContext(Network::TransportSocketFactory& transport_socket_factory) {
   auto* quic_socket_factory =
       dynamic_cast<QuicClientTransportSocketFactory*>(&transport_socket_factory);
   ASSERT(quic_socket_factory != nullptr);
-  ASSERT(quic_socket_factory->sslCtx() != nullptr);
   return quic_socket_factory->sslCtx();
 }
 
+std::shared_ptr<quic::QuicCryptoClientConfig> PersistentQuicInfoImpl::cryptoConfig() {
+  auto context = getContext(transport_socket_factory_);
+  // If the secrets haven't been loaded, there is no crypto config.
+  if (context == nullptr) {
+    return nullptr;
+  }
+
+  // If the secret has been updated, update the proof source.
+  if (context.get() != client_context_.get()) {
+    client_context_ = context;
+    client_config_ = std::make_shared<quic::QuicCryptoClientConfig>(
+        std::make_unique<EnvoyQuicProofVerifier>(getContext(transport_socket_factory_)),
+        std::make_unique<EnvoyQuicSessionCache>((time_source_)));
+  }
+  // Return the latest client config.
+  return client_config_;
+}
+
 PersistentQuicInfoImpl::PersistentQuicInfoImpl(
     Event::Dispatcher& dispatcher, Network::TransportSocketFactory& transport_socket_factory,
     TimeSource& time_source, Network::Address::InstanceConstSharedPtr server_addr)
     : conn_helper_(dispatcher), alarm_factory_(dispatcher, *conn_helper_.GetClock()),
       server_id_{getConfig(transport_socket_factory).serverNameIndication(),
                  static_cast<uint16_t>(server_addr->ip()->port()), false},
-      crypto_config_(std::make_unique<quic::QuicCryptoClientConfig>(
-          std::make_unique<EnvoyQuicProofVerifier>(getContext(transport_socket_factory)),
-          std::make_unique<EnvoyQuicSessionCache>(time_source))) {
+      transport_socket_factory_(transport_socket_factory), time_source_(time_source) {
   quiche::FlagRegistry::getInstance();
 }
 
@@ -53,6 +68,10 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d
   // This flag fix a QUICHE issue which may crash Envoy during connection close.
   SetQuicReloadableFlag(quic_single_ack_in_packet2, true);
   PersistentQuicInfoImpl* info_impl = reinterpret_cast<PersistentQuicInfoImpl*>(&info);
+  auto config = info_impl->cryptoConfig();
+  if (config == nullptr) {
+    return nullptr; // no secrets available yet.
+  }
 
   auto connection = std::make_unique<EnvoyQuicClientConnection>(
       quic::QuicUtils::CreateRandomConnectionId(), server_addr, info_impl->conn_helper_,
@@ -66,8 +85,8 @@ createQuicNetworkConnection(Http::PersistentQuicInfo& info, Event::Dispatcher& d
   // send_buffer_limit instead of using 0.
   auto ret = std::make_unique<EnvoyQuicClientSession>(
       info_impl->quic_config_, info_impl->supported_versions_, std::move(connection),
-      info_impl->server_id_, info_impl->crypto_config_.get(), &static_info.push_promise_index_,
-      dispatcher, /*send_buffer_limit=*/0);
+      info_impl->server_id_, std::move(config), &static_info.push_promise_index_, dispatcher,
+      /*send_buffer_limit=*/0);
   return ret;
 }
 

source/common/quic/client_connection_factory_impl.h

@@ -22,15 +22,25 @@ struct PersistentQuicInfoImpl : public Http::PersistentQuicInfo {
                          TimeSource& time_source,
                          Network::Address::InstanceConstSharedPtr server_addr);
 
+  // Returns the most recent crypto config from transport_socket_factory_;
+  std::shared_ptr<quic::QuicCryptoClientConfig> cryptoConfig();
+
   EnvoyQuicConnectionHelper conn_helper_;
   EnvoyQuicAlarmFactory alarm_factory_;
-  // server-id and server address can change over the lifetime of Envoy but will be consistent for a
+  // server-id can change over the lifetime of Envoy but will be consistent for a
   // given connection pool.
   quic::QuicServerId server_id_;
-  quic::ParsedQuicVersionVector supported_versions_{quic::CurrentSupportedVersions()};
-  // TODO(danzh) move this into client transport socket factory so that it can
-  // be updated with SDS.
-  std::unique_ptr<quic::QuicCryptoClientConfig> crypto_config_;
+  // Latch the transport socket factory, to get the latest crypto config and the
+  // time source to create it.
+  Network::TransportSocketFactory& transport_socket_factory_;
+  TimeSource& time_source_;
+  // Latch the latest crypto config, to determine if it has updated since last
+  // checked.
+  Envoy::Ssl::ClientContextSharedPtr client_context_;
+  // If client context changes, client config will be updated as well.
+  std::shared_ptr<quic::QuicCryptoClientConfig> client_config_;
+  const quic::ParsedQuicVersionVector supported_versions_{quic::CurrentSupportedVersions()};
+  // TODO(alyssawilk) actually set this up properly.
   quic::QuicConfig quic_config_;
 };
 

source/common/quic/envoy_quic_client_session.cc

@@ -8,14 +8,14 @@ namespace Quic {
 EnvoyQuicClientSession::EnvoyQuicClientSession(
     const quic::QuicConfig& config, const quic::ParsedQuicVersionVector& supported_versions,
     std::unique_ptr<EnvoyQuicClientConnection> connection, const quic::QuicServerId& server_id,
-    quic::QuicCryptoClientConfig* crypto_config,
+    std::shared_ptr<quic::QuicCryptoClientConfig> crypto_config,
     quic::QuicClientPushPromiseIndex* push_promise_index, Event::Dispatcher& dispatcher,
     uint32_t send_buffer_limit)
     : QuicFilterManagerConnectionImpl(*connection, connection->connection_id(), dispatcher,
                                       send_buffer_limit),
       quic::QuicSpdyClientSession(config, supported_versions, connection.release(), server_id,
-                                  crypto_config, push_promise_index),
-      host_name_(server_id.host()) {}
+                                  crypto_config.get(), push_promise_index),
+      host_name_(server_id.host()), crypto_config_(crypto_config) {}
 
 EnvoyQuicClientSession::~EnvoyQuicClientSession() {
   ASSERT(!connection()->connected());

source/common/quic/envoy_quic_client_session.h

@@ -33,7 +33,7 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl,
                          const quic::ParsedQuicVersionVector& supported_versions,
                          std::unique_ptr<EnvoyQuicClientConnection> connection,
                          const quic::QuicServerId& server_id,
-                         quic::QuicCryptoClientConfig* crypto_config,
+                         std::shared_ptr<quic::QuicCryptoClientConfig> crypto_config,
                          quic::QuicClientPushPromiseIndex* push_promise_index,
                          Event::Dispatcher& dispatcher, uint32_t send_buffer_limit);
 
@@ -89,6 +89,7 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl,
   // them.
   Http::ConnectionCallbacks* http_connection_callbacks_{nullptr};
   const absl::string_view host_name_;
+  std::shared_ptr<quic::QuicCryptoClientConfig> crypto_config_;
 };
 
 } // namespace Quic

source/common/quic/quic_transport_socket_factory.h

@@ -96,9 +96,7 @@ class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase {
       Ssl::ClientContextConfigPtr config,
       Server::Configuration::TransportSocketFactoryContext& factory_context);
 
-  void initialize() override {
-    // TODO(14829) fallback_factory_ needs to call onSecretUpdated() upon SDS update.
-  }
+  void initialize() override {}
 
   // As documented above for QuicTransportSocketFactoryBase, the actual HTTP/3
   // code does not create transport sockets.
@@ -118,10 +116,8 @@ class QuicClientTransportSocketFactory : public QuicTransportSocketFactoryBase {
   }
 
 protected:
-  void onSecretUpdated() override {
-    // fallback_factory_ will update the stats.
-    // TODO(14829) Client transport socket factory may also need to update quic crypto.
-  }
+  // fallback_factory_ will update the context.
+  void onSecretUpdated() override {}
 
 private:
   // The QUIC client transport socket can create TLS sockets for fallback to TCP.

test/common/quic/envoy_quic_client_session_test.cc

@@ -80,7 +80,7 @@ class TestEnvoyQuicClientSession : public EnvoyQuicClientSession {
                              const quic::ParsedQuicVersionVector& supported_versions,
                              std::unique_ptr<EnvoyQuicClientConnection> connection,
                              const quic::QuicServerId& server_id,
-                             quic::QuicCryptoClientConfig* crypto_config,
+                             std::shared_ptr<quic::QuicCryptoClientConfig> crypto_config,
                              quic::QuicClientPushPromiseIndex* push_promise_index,
                              Event::Dispatcher& dispatcher, uint32_t send_buffer_limit)
       : EnvoyQuicClientSession(config, supported_versions, std::move(connection), server_id,
@@ -109,10 +109,11 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<bool> {
         quic_connection_(new TestEnvoyQuicClientConnection(
             quic::test::TestConnectionId(), connection_helper_, alarm_factory_, writer_,
             quic_version_, *dispatcher_, createConnectionSocket(peer_addr_, self_addr_, nullptr))),
-        crypto_config_(quic::test::crypto_test_utils::ProofVerifierForTesting()),
+        crypto_config_(std::make_shared<quic::QuicCryptoClientConfig>(
+            quic::test::crypto_test_utils::ProofVerifierForTesting())),
         envoy_quic_session_(quic_config_, quic_version_,
                             std::unique_ptr<TestEnvoyQuicClientConnection>(quic_connection_),
-                            quic::QuicServerId("example.com", 443, false), &crypto_config_, nullptr,
+                            quic::QuicServerId("example.com", 443, false), crypto_config_, nullptr,
                             *dispatcher_,
                             /*send_buffer_limit*/ 1024 * 1024),
         stats_({ALL_HTTP3_CODEC_STATS(POOL_COUNTER_PREFIX(scope_, "http3."),
@@ -173,7 +174,7 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<bool> {
   Network::Address::InstanceConstSharedPtr self_addr_;
   TestEnvoyQuicClientConnection* quic_connection_;
   quic::QuicConfig quic_config_;
-  quic::QuicCryptoClientConfig crypto_config_;
+  std::shared_ptr<quic::QuicCryptoClientConfig> crypto_config_;
   TestEnvoyQuicClientSession envoy_quic_session_;
   Network::MockConnectionCallbacks network_connection_callbacks_;
   Http::MockServerConnectionCallbacks http_connection_callbacks_;

test/config/utility.cc

@@ -768,7 +768,8 @@ void ConfigHelper::setProtocolOptions(envoy::config::cluster::v3::Cluster& clust
     HttpProtocolOptions old_options = MessageUtil::anyConvert<ConfigHelper::HttpProtocolOptions>(
         (*cluster.mutable_typed_extension_protocol_options())
             ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"]);
-    protocol_options.MergeFrom(old_options);
+    old_options.MergeFrom(protocol_options);
+    protocol_options.CopyFrom(old_options);
   }
   (*cluster.mutable_typed_extension_protocol_options())
       ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"]

test/integration/quic_http_integration_test.cc

@@ -113,7 +113,7 @@ class QuicHttpIntegrationTest : public HttpIntegrationTest, public QuicMultiVers
     auto& persistent_info = static_cast<PersistentQuicInfoImpl&>(*quic_connection_persistent_info_);
     auto session = std::make_unique<EnvoyQuicClientSession>(
         persistent_info.quic_config_, supported_versions_, std::move(connection),
-        persistent_info.server_id_, persistent_info.crypto_config_.get(), &push_promise_index_,
+        persistent_info.server_id_, persistent_info.cryptoConfig(), &push_promise_index_,
         *dispatcher_,
         // Use smaller window than the default one to have test coverage of client codec buffer
         // exceeding high watermark.