quic: use sds for upstream http/3

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.
+        if (dynamic_cast<Quic::QuicClientTransportSocketFactory*>(
+                &pool->host()->transportSocketFactory())
+                ->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);
 
@@ -86,6 +86,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

@@ -97,9 +97,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.
@@ -119,10 +117,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

@@ -811,7 +811,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/alpn_integration_test.cc

@@ -32,11 +32,10 @@ class AlpnIntegrationTest : public testing::TestWithParam<Network::Address::IpVe
   }
   void createUpstreams() override {
     for (uint32_t i = 0; i < fake_upstreams_count_; ++i) {
-      setUpstreamProtocol(protocols_[i]);
-      Network::TransportSocketFactoryPtr factory = createUpstreamTlsContext();
-      auto endpoint = upstream_address_fn_(i);
       auto config = upstreamConfig();
       config.upstream_protocol_ = protocols_[i];
+      Network::TransportSocketFactoryPtr factory = createUpstreamTlsContext(config);
+      auto endpoint = upstream_address_fn_(i);
       fake_upstreams_.emplace_back(new AutonomousUpstream(std::move(factory), endpoint, config,
                                                           autonomous_allow_incomplete_streams_));
     }

test/integration/base_integration_test.cc

@@ -105,7 +105,8 @@ void BaseIntegrationTest::initialize() {
   createEnvoy();
 }
 
-Network::TransportSocketFactoryPtr BaseIntegrationTest::createUpstreamTlsContext() {
+Network::TransportSocketFactoryPtr
+BaseIntegrationTest::createUpstreamTlsContext(const FakeUpstreamConfig& upstream_config) {
   envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
   const std::string yaml = absl::StrFormat(
       R"EOF(
@@ -120,12 +121,12 @@ Network::TransportSocketFactoryPtr BaseIntegrationTest::createUpstreamTlsContext
       TestEnvironment::runfilesPath("test/config/integration/certs/upstreamkey.pem"),
       TestEnvironment::runfilesPath("test/config/integration/certs/cacert.pem"));
   TestUtility::loadFromYaml(yaml, tls_context);
-  if (upstream_config_.upstream_protocol_ == FakeHttpConnection::Type::HTTP2) {
+  if (upstream_config.upstream_protocol_ == FakeHttpConnection::Type::HTTP2) {
     tls_context.mutable_common_tls_context()->add_alpn_protocols("h2");
-  } else if (upstream_config_.upstream_protocol_ == FakeHttpConnection::Type::HTTP1) {
+  } else if (upstream_config.upstream_protocol_ == FakeHttpConnection::Type::HTTP1) {
     tls_context.mutable_common_tls_context()->add_alpn_protocols("http/1.1");
   }
-  if (upstream_config_.upstream_protocol_ != FakeHttpConnection::Type::HTTP3) {
+  if (upstream_config.upstream_protocol_ != FakeHttpConnection::Type::HTTP3) {
     auto cfg = std::make_unique<Extensions::TransportSockets::Tls::ServerContextConfigImpl>(
         tls_context, factory_context_);
     static Stats::Scope* upstream_stats_store = new Stats::IsolatedStoreImpl();
@@ -146,7 +147,8 @@ Network::TransportSocketFactoryPtr BaseIntegrationTest::createUpstreamTlsContext
 void BaseIntegrationTest::createUpstreams() {
   for (uint32_t i = 0; i < fake_upstreams_count_; ++i) {
     Network::TransportSocketFactoryPtr factory =
-        upstream_tls_ ? createUpstreamTlsContext() : Network::Test::createRawBufferSocketFactory();
+        upstream_tls_ ? createUpstreamTlsContext(upstreamConfig())
+                      : Network::Test::createRawBufferSocketFactory();
     auto endpoint = upstream_address_fn_(i);
     if (autonomous_upstream_) {
       fake_upstreams_.emplace_back(new AutonomousUpstream(

test/integration/base_integration_test.h

@@ -307,17 +307,24 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
                                                  *dispatcher_, std::move(transport_socket));
   }
 
-  // Add a fake upstream bound to INADDR_ANY and there is no specified port.
-  FakeUpstream& addFakeUpstream(FakeHttpConnection::Type type) {
+  FakeUpstreamConfig configWithType(FakeHttpConnection::Type type) const {
     FakeUpstreamConfig config = upstream_config_;
     config.upstream_protocol_ = type;
+    if (type != FakeHttpConnection::Type::HTTP3) {
+      config.udp_fake_upstream_ = absl::nullopt;
+    }
+    return config;
+  }
+
+  FakeUpstream& addFakeUpstream(FakeHttpConnection::Type type) {
+    auto config = configWithType(type);
     fake_upstreams_.emplace_back(std::make_unique<FakeUpstream>(0, version_, config));
     return *fake_upstreams_.back();
   }
+
   FakeUpstream& addFakeUpstream(Network::TransportSocketFactoryPtr&& transport_socket_factory,
                                 FakeHttpConnection::Type type) {
-    FakeUpstreamConfig config = upstream_config_;
-    config.upstream_protocol_ = type;
+    auto config = configWithType(type);
     fake_upstreams_.emplace_back(
         std::make_unique<FakeUpstream>(std::move(transport_socket_factory), 0, version_, config));
     return *fake_upstreams_.back();
@@ -394,7 +401,8 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
   bool use_lds_{true}; // Use the integration framework's LDS set up.
   bool upstream_tls_{false};
 
-  Network::TransportSocketFactoryPtr createUpstreamTlsContext();
+  Network::TransportSocketFactoryPtr
+  createUpstreamTlsContext(const FakeUpstreamConfig& upstream_config);
   testing::NiceMock<Server::Configuration::MockTransportSocketFactoryContext> factory_context_;
   Extensions::TransportSockets::Tls::ContextManagerImpl context_manager_{timeSystem()};