Hook up NetworkIsolationKey to QuicCryptoClientConfig.

The class has a per-server cache that could be used for fingerprinting.
As it's defined outside of net/, rather than make QuicCryptoClientConfig
NetworkIsolationKey-aware, this CL has QuicStreamFactory create one
QuicCryptoClientConfig per NetworkIsolationKey, as needed. Once there
are no live QuicSessions/QuicStreamFactory::[.*]Jobs using a particular
QuicCryptoClientConfig, the QuicCryptoClientConfig is added to an MRU
cache of recently used configs, and reused/evicted as needed.

This CL also passes in a NetworkIsolationKey to the
HttpServerProperties::*QuicServerInfo calls, to finish hooking up
NetworkIsolationKeys to HttpServerProperties. It's needed in this CL
because the only integration tests for QuicCryptoClientConfig rely
on those methods.

Bug: 1003423, 969890
Change-Id: Id1823012104b84677f6684205bb58fa0227825b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1809601Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698665}

net/BUILD.gn

@@ -1057,6 +1057,8 @@ component("net") {
       "quic/quic_connection_logger.h",
       "quic/quic_connectivity_probing_manager.cc",
       "quic/quic_connectivity_probing_manager.h",
+      "quic/quic_crypto_client_config_handle.cc",
+      "quic/quic_crypto_client_config_handle.h",
       "quic/quic_crypto_client_stream_factory.cc",
       "quic/quic_crypto_client_stream_factory.h",
       "quic/quic_flags_list.h",
@@ -5326,6 +5328,8 @@ test("net_unittests") {
     "quic/quic_test_packet_maker.cc",
     "quic/quic_test_packet_maker.h",
     "quic/quic_utils_chromium_test.cc",
+    "quic/test_quic_crypto_client_config_handle.cc",
+    "quic/test_quic_crypto_client_config_handle.h",
     "socket/client_socket_pool_base_unittest.cc",
     "socket/client_socket_pool_unittest.cc",
     "socket/connect_job_test_util.cc",

net/quic/bidirectional_stream_quic_impl_unittest.cc

@@ -30,11 +30,13 @@
 #include "net/quic/quic_chromium_connection_helper.h"
 #include "net/quic/quic_chromium_packet_reader.h"
 #include "net/quic/quic_chromium_packet_writer.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/quic/quic_server_info.h"
 #include "net/quic/quic_stream_factory.h"
 #include "net/quic/quic_test_packet_maker.h"
 #include "net/quic/quic_test_packet_printer.h"
+#include "net/quic/test_quic_crypto_client_config_handle.h"
 #include "net/quic/test_task_runner.h"
 #include "net/socket/socket_test_util.h"
 #include "net/test/gtest_util.h"
@@ -534,8 +536,9 @@ class BidirectionalStreamQuicImplTest
             kQuicYieldAfterDurationMilliseconds),
         /*go_away_on_path_degrading*/ false,
         client_headers_include_h2_stream_dependency_, /*cert_verify_flags=*/0,
-        quic::test::DefaultQuicConfig(), &crypto_config_, "CONNECTION_UNKNOWN",
-        dns_start, dns_end, &push_promise_index_, nullptr,
+        quic::test::DefaultQuicConfig(),
+        std::make_unique<TestQuicCryptoClientConfigHandle>(&crypto_config_),
+        "CONNECTION_UNKNOWN", dns_start, dns_end, &push_promise_index_, nullptr,
         base::DefaultTickClock::GetInstance(),
         base::ThreadTaskRunnerHandle::Get().get(),
         /*socket_performance_watcher=*/nullptr, net_log().bound().net_log()));

net/quic/properties_based_quic_server_info.cc

@@ -25,8 +25,10 @@ namespace net {
 
 PropertiesBasedQuicServerInfo::PropertiesBasedQuicServerInfo(
     const quic::QuicServerId& server_id,
+    const NetworkIsolationKey& network_isolation_key,
     HttpServerProperties* http_server_properties)
     : QuicServerInfo(server_id),
+      network_isolation_key_(network_isolation_key),
       http_server_properties_(http_server_properties) {
   DCHECK(http_server_properties_);
 }
@@ -35,7 +37,7 @@ PropertiesBasedQuicServerInfo::~PropertiesBasedQuicServerInfo() {}
 
 bool PropertiesBasedQuicServerInfo::Load() {
   const string* data = http_server_properties_->GetQuicServerInfo(
-      server_id_, NetworkIsolationKey());
+      server_id_, network_isolation_key_);
   string decoded;
   if (!data) {
     RecordQuicServerInfoFailure(PARSE_NO_DATA_FAILURE);
@@ -55,7 +57,7 @@ bool PropertiesBasedQuicServerInfo::Load() {
 void PropertiesBasedQuicServerInfo::Persist() {
   string encoded;
   base::Base64Encode(Serialize(), &encoded);
-  http_server_properties_->SetQuicServerInfo(server_id_, NetworkIsolationKey(),
+  http_server_properties_->SetQuicServerInfo(server_id_, network_isolation_key_,
                                              encoded);
 }
 

net/quic/properties_based_quic_server_info.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "net/base/network_isolation_key.h"
 #include "net/quic/quic_server_info.h"
 #include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
 
@@ -23,8 +24,10 @@ class HttpServerProperties;
 class QUIC_EXPORT_PRIVATE PropertiesBasedQuicServerInfo
     : public QuicServerInfo {
  public:
-  PropertiesBasedQuicServerInfo(const quic::QuicServerId& server_id,
-                                HttpServerProperties* http_server_properties);
+  PropertiesBasedQuicServerInfo(
+      const quic::QuicServerId& server_id,
+      const NetworkIsolationKey& network_isolation_key,
+      HttpServerProperties* http_server_properties);
   ~PropertiesBasedQuicServerInfo() override;
 
   // QuicServerInfo implementation.
@@ -33,7 +36,8 @@ class QUIC_EXPORT_PRIVATE PropertiesBasedQuicServerInfo
   size_t EstimateMemoryUsage() const override;
 
  private:
-  HttpServerProperties* http_server_properties_;
+  const NetworkIsolationKey network_isolation_key_;
+  HttpServerProperties* const http_server_properties_;
 
   DISALLOW_COPY_AND_ASSIGN(PropertiesBasedQuicServerInfo);
 };

net/quic/properties_based_quic_server_info_test.cc

@@ -32,7 +32,9 @@ class PropertiesBasedQuicServerInfoTest : public ::testing::Test {
  protected:
   PropertiesBasedQuicServerInfoTest()
       : server_id_("www.google.com", 443, PRIVACY_MODE_DISABLED),
-        server_info_(server_id_, &http_server_properties_) {}
+        server_info_(server_id_,
+                     NetworkIsolationKey(),
+                     &http_server_properties_) {}
 
   // Initialize |server_info_| object and persist it.
   void InitializeAndPersist() {
@@ -68,7 +70,7 @@ TEST_F(PropertiesBasedQuicServerInfoTest, Update) {
   InitializeAndPersist();
 
   // Read the persisted data and verify we have read the data correctly.
-  PropertiesBasedQuicServerInfo server_info1(server_id_,
+  PropertiesBasedQuicServerInfo server_info1(server_id_, NetworkIsolationKey(),
                                              &http_server_properties_);
   EXPECT_TRUE(server_info1.Load());
 
@@ -83,7 +85,7 @@ TEST_F(PropertiesBasedQuicServerInfoTest, Update) {
   server_info1.Persist();
 
   // Read the persisted data and verify we have read the data correctly.
-  PropertiesBasedQuicServerInfo server_info2(server_id_,
+  PropertiesBasedQuicServerInfo server_info2(server_id_, NetworkIsolationKey(),
                                              &http_server_properties_);
   EXPECT_TRUE(server_info2.Load());
 

net/quic/quic_chromium_client_session.cc

@@ -713,7 +713,7 @@ QuicChromiumClientSession::QuicChromiumClientSession(
     bool headers_include_h2_stream_dependency,
     int cert_verify_flags,
     const quic::QuicConfig& config,
-    quic::QuicCryptoClientConfig* crypto_config,
+    std::unique_ptr<QuicCryptoClientConfigHandle> crypto_config,
     const char* const connection_description,
     base::TimeTicks dns_resolution_start_time,
     base::TimeTicks dns_resolution_end_time,
@@ -752,6 +752,7 @@ QuicChromiumClientSession::QuicChromiumClientSession(
       most_recent_stream_close_time_(tick_clock_->NowTicks()),
       most_recent_write_error_(0),
       most_recent_write_error_timestamp_(base::TimeTicks()),
+      crypto_config_(std::move(crypto_config)),
       stream_factory_(stream_factory),
       transport_security_state_(transport_security_state),
       ssl_config_service_(ssl_config_service),
@@ -798,7 +799,7 @@ QuicChromiumClientSession::QuicChromiumClientSession(
           session_key.server_id(), this,
           std::make_unique<ProofVerifyContextChromium>(cert_verify_flags,
                                                        net_log_),
-          crypto_config));
+          crypto_config_->GetConfig()));
   connection->set_debug_visitor(logger_.get());
   connection->set_creator_debug_delegate(logger_.get());
   migrate_back_to_default_timer_.SetTaskRunner(task_runner_);

net/quic/quic_chromium_client_session.h

@@ -34,6 +34,7 @@
 #include "net/quic/quic_chromium_packet_writer.h"
 #include "net/quic/quic_connection_logger.h"
 #include "net/quic/quic_connectivity_probing_manager.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_session_key.h"
 #include "net/socket/socket_performance_watcher.h"
 #include "net/spdy/http2_priority_dependencies.h"
@@ -402,7 +403,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
       bool headers_include_h2_stream_dependency,
       int cert_verify_flags,
       const quic::QuicConfig& config,
-      quic::QuicCryptoClientConfig* crypto_config,
+      std::unique_ptr<QuicCryptoClientConfigHandle> crypto_config,
       const char* const connection_description,
       base::TimeTicks dns_resolution_start_time,
       base::TimeTicks dns_resolution_end_time,
@@ -798,6 +799,8 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession
   int most_recent_write_error_;
   base::TimeTicks most_recent_write_error_timestamp_;
 
+  std::unique_ptr<QuicCryptoClientConfigHandle> crypto_config_;
+
   std::unique_ptr<quic::QuicCryptoClientStream> crypto_stream_;
   QuicStreamFactory* stream_factory_;
   std::vector<std::unique_ptr<DatagramClientSocket>> sockets_;

net/quic/quic_chromium_client_session_test.cc

@@ -32,10 +32,12 @@
 #include "net/quic/quic_chromium_connection_helper.h"
 #include "net/quic/quic_chromium_packet_reader.h"
 #include "net/quic/quic_chromium_packet_writer.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_crypto_client_stream_factory.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/quic/quic_server_info.h"
 #include "net/quic/quic_test_packet_maker.h"
+#include "net/quic/test_quic_crypto_client_config_handle.h"
 #include "net/socket/datagram_client_socket.h"
 #include "net/socket/socket_test_util.h"
 #include "net/spdy/spdy_test_util_common.h"
@@ -165,9 +167,11 @@ class QuicChromiumClientSessionTest
             kQuicYieldAfterDurationMilliseconds),
         /*cert_verify_flags=*/0, /*go_away_on_path_degrading*/ false,
         client_headers_include_h2_stream_dependency_,
-        quic::test::DefaultQuicConfig(), &crypto_config_, "CONNECTION_UNKNOWN",
-        base::TimeTicks::Now(), base::TimeTicks::Now(), &push_promise_index_,
-        &test_push_delegate_, base::DefaultTickClock::GetInstance(),
+        quic::test::DefaultQuicConfig(),
+        std::make_unique<TestQuicCryptoClientConfigHandle>(&crypto_config_),
+        "CONNECTION_UNKNOWN", base::TimeTicks::Now(), base::TimeTicks::Now(),
+        &push_promise_index_, &test_push_delegate_,
+        base::DefaultTickClock::GetInstance(),
         base::ThreadTaskRunnerHandle::Get().get(),
         /*socket_performance_watcher=*/nullptr, &net_log_));
 

net/quic/quic_crypto_client_config_handle.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/quic/quic_crypto_client_config_handle.h"
+
+namespace net {
+
+QuicCryptoClientConfigHandle::~QuicCryptoClientConfigHandle() = default;
+QuicCryptoClientConfigHandle::QuicCryptoClientConfigHandle() = default;
+
+}  // namespace net

net/quic/quic_crypto_client_config_handle.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_QUIC_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_
+#define NET_QUIC_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_
+
+#include "base/macros.h"
+#include "net/base/net_export.h"
+
+namespace quic {
+class QuicCryptoClientConfig;
+}  // namespace quic
+
+namespace net {
+
+// Class that allows consumers to access a quic::QuicCryptoClientConfig, while
+// ensuring that the QuciStreamFactory that owns it keeps it alive. Once a
+// QuicCryptoClientConfigHandle is destroyed, the underlying
+// QuicCryptoClientConfig object may be destroyed as well. All
+// QuicCryptoClientConfigHandle must be destroyed before the end of the
+// QuciStreamFactory's destructor.
+//
+// This ownership model is used instead of refcounting for stronger safety
+// guarantees, and because the underlying QuicCryptoClientConfig depends on
+// other network objects that may be deleted after the QuicStreamFactory.
+class NET_EXPORT_PRIVATE QuicCryptoClientConfigHandle {
+ public:
+  virtual ~QuicCryptoClientConfigHandle();
+  virtual quic::QuicCryptoClientConfig* GetConfig() const = 0;
+
+ protected:
+  QuicCryptoClientConfigHandle();
+
+ private:
+  DISALLOW_ASSIGN(QuicCryptoClientConfigHandle);
+};
+
+}  // namespace net
+
+#endif  // NET_QUIC_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_

net/quic/quic_http_stream_test.cc

@@ -38,11 +38,13 @@
 #include "net/quic/quic_chromium_connection_helper.h"
 #include "net/quic/quic_chromium_packet_reader.h"
 #include "net/quic/quic_chromium_packet_writer.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/quic/quic_server_info.h"
 #include "net/quic/quic_stream_factory.h"
 #include "net/quic/quic_test_packet_maker.h"
 #include "net/quic/quic_test_packet_printer.h"
+#include "net/quic/test_quic_crypto_client_config_handle.h"
 #include "net/quic/test_task_runner.h"
 #include "net/socket/socket_performance_watcher.h"
 #include "net/socket/socket_test_util.h"
@@ -345,8 +347,9 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<
             kQuicYieldAfterDurationMilliseconds),
         /*go_away_on_path_degrading*/ false,
         client_headers_include_h2_stream_dependency_, /*cert_verify_flags=*/0,
-        quic::test::DefaultQuicConfig(), &crypto_config_, "CONNECTION_UNKNOWN",
-        dns_start, dns_end, &push_promise_index_, nullptr,
+        quic::test::DefaultQuicConfig(),
+        std::make_unique<TestQuicCryptoClientConfigHandle>(&crypto_config_),
+        "CONNECTION_UNKNOWN", dns_start, dns_end, &push_promise_index_, nullptr,
         base::DefaultTickClock::GetInstance(),
         base::ThreadTaskRunnerHandle::Get().get(),
         /*socket_performance_watcher=*/nullptr, net_log_.bound().net_log()));

net/quic/quic_proxy_client_socket_unittest.cc

@@ -30,10 +30,12 @@
 #include "net/quic/quic_chromium_client_session.h"
 #include "net/quic/quic_chromium_connection_helper.h"
 #include "net/quic/quic_chromium_packet_writer.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_http_utils.h"
 #include "net/quic/quic_server_info.h"
 #include "net/quic/quic_stream_factory.h"
 #include "net/quic/quic_test_packet_maker.h"
+#include "net/quic/test_quic_crypto_client_config_handle.h"
 #include "net/quic/test_task_runner.h"
 #include "net/socket/socket_test_util.h"
 #include "net/test/cert_test_util.h"
@@ -236,8 +238,9 @@ class QuicProxyClientSocketTest
             kQuicYieldAfterDurationMilliseconds),
         /*go_away_on_path_degrading*/ false,
         client_headers_include_h2_stream_dependency_, /*cert_verify_flags=*/0,
-        quic::test::DefaultQuicConfig(), &crypto_config_, "CONNECTION_UNKNOWN",
-        dns_start, dns_end, &push_promise_index_, nullptr,
+        quic::test::DefaultQuicConfig(),
+        std::make_unique<TestQuicCryptoClientConfigHandle>(&crypto_config_),
+        "CONNECTION_UNKNOWN", dns_start, dns_end, &push_promise_index_, nullptr,
         base::DefaultTickClock::GetInstance(),
         base::ThreadTaskRunnerHandle::Get().get(),
         /*socket_performance_watcher=*/nullptr, net_log_.bound().net_log()));

net/quic/quic_stream_factory.h

@@ -10,10 +10,12 @@
 
 #include <list>
 #include <map>
+#include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
+#include "base/containers/mru_cache.h"
 #include "base/gtest_prod_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ -33,6 +35,7 @@
 #include "net/quic/network_connection.h"
 #include "net/quic/quic_chromium_client_session.h"
 #include "net/quic/quic_clock_skew_detector.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
 #include "net/quic/quic_session_key.h"
 #include "net/socket/client_socket_pool.h"
 #include "net/ssl/ssl_config_service.h"
@@ -106,6 +109,15 @@ const int64_t kMaxMigrationsToNonDefaultNetworkOnWriteError = 5;
 // degrading per network.
 const int64_t kMaxMigrationsToNonDefaultNetworkOnPathDegrading = 5;
 
+// Maximum number of not currently in use QuicCryptoClientConfig that can be
+// stored in |recent_crypto_config_map_|.
+//
+// TODO(mmenke): Should figure out a reasonable value of this, using field
+// trials. The optimal value may increase over time, as QUIC becomes more
+// prevalent. Whether or not NetworkIsolationKeys end up including subframe URLs
+// will also influence the ideal value.
+const int kMaxRecentCryptoConfigs = 100;
+
 // Structure containing simple configuration options and experiments for QUIC.
 struct NET_EXPORT QuicParams {
   QuicParams();
@@ -492,6 +504,8 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
  private:
   class Job;
   class CertVerifierJob;
+  class QuicCryptoClientConfigOwner;
+  class CryptoClientConfigHandle;
   friend class test::QuicStreamFactoryPeer;
 
   typedef std::map<QuicSessionKey, QuicChromiumClientSession*> SessionMap;
@@ -505,6 +519,9 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   typedef std::map<QuicSessionKey, std::unique_ptr<Job>> JobMap;
   typedef std::map<quic::QuicServerId, std::unique_ptr<CertVerifierJob>>
       CertVerifierJobMap;
+  using QuicCryptoClientConfigMap =
+      std::map<NetworkIsolationKey,
+               std::unique_ptr<QuicCryptoClientConfigOwner>>;
 
   bool HasMatchingIpSession(const QuicSessionAliasKey& key,
                             const AddressList& address_list);
@@ -549,14 +566,23 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // Helper methods.
   bool WasQuicRecentlyBroken(const QuicSessionKey& session_key) const;
 
-  bool CryptoConfigCacheIsEmpty(const quic::QuicServerId& server_id);
+  bool CryptoConfigCacheIsEmpty(
+      const quic::QuicServerId& server_id,
+      const NetworkIsolationKey& network_isolation_key);
 
   // Starts an asynchronous job for cert verification if
   // |params_.race_cert_verification| is enabled and if there are cached certs
   // for the given |server_id|.
-  quic::QuicAsyncStatus StartCertVerifyJob(const quic::QuicServerId& server_id,
-                                           int cert_verify_flags,
-                                           const NetLogWithSource& net_log);
+  //
+  // Takes a constant reference to a CryptoClientConfigHandle instead of a
+  // NetworkIsolationKey to force the caller to keep the corresponding
+  // QuicCryptoClientConfig alive. There's no guarantee it won't be garbage
+  // collected beyond when this method completes, otherwise.
+  quic::QuicAsyncStatus StartCertVerifyJob(
+      const CryptoClientConfigHandle& crypto_config_handle,
+      const quic::QuicServerId& server_id,
+      int cert_verify_flags,
+      const NetLogWithSource& net_log);
 
   // Helper method to initialize the following migration options and check
   // pre-requisites:
@@ -572,6 +598,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // |connection_id| with the next server designated connection id,
   // if any, and otherwise leaves it unchanged.
   void InitializeCachedStateInCryptoConfig(
+      const CryptoClientConfigHandle& crypto_config_handle,
       const quic::QuicServerId& server_id,
       const std::unique_ptr<QuicServerInfo>& server_info,
       quic::QuicConnectionId* connection_id);
@@ -580,6 +607,29 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
                                const quic::QuicServerId& server_id,
                                bool was_session_active);
 
+  // Creates a CreateCryptoConfigHandle for the specified NetworkIsolationKey.
+  // If there's already a corresponding entry in |active_crypto_config_map_|,
+  // reuses it. If there's a corresponding entry in |recent_crypto_config_map_|,
+  // promotes it to |active_crypto_config_map_| and then reuses it. Otherwise,
+  // creates a new entry in |active_crypto_config_map_|.
+  std::unique_ptr<CryptoClientConfigHandle> CreateCryptoConfigHandle(
+      const NetworkIsolationKey& network_isolation_key);
+
+  // Salled when the indicated member of |active_crypto_config_map_| has no
+  // outstanding references. The QuicCryptoClientConfigOwner is then moved to
+  // |recent_crypto_config_map_|, an MRU cache.
+  void OnAllCryptoClientRefReleased(
+      QuicCryptoClientConfigMap::iterator& map_iterator);
+
+  std::unique_ptr<QuicCryptoClientConfigHandle> GetCryptoConfigForTesting(
+      const NetworkIsolationKey& network_isolation_key);
+
+  quic::QuicAsyncStatus StartCertVerifyJobForTesting(
+      const quic::QuicServerId& server_id,
+      const NetworkIsolationKey& network_isolation_key,
+      int cert_verify_flags,
+      const NetLogWithSource& net_log);
+
   // Whether QUIC is known to work on current network. This is true when QUIC is
   // expected to work in general, rather than whether QUIC was broken / recently
   // broken when used with a particular server. That information is stored in
@@ -591,8 +641,10 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   ClientSocketFactory* client_socket_factory_;
   HttpServerProperties* http_server_properties_;
   ServerPushDelegate* push_delegate_;
-  TransportSecurityState* transport_security_state_;
-  CTVerifier* cert_transparency_verifier_;
+  CertVerifier* const cert_verifier_;
+  CTPolicyEnforcer* const ct_policy_enforcer_;
+  TransportSecurityState* const transport_security_state_;
+  CTVerifier* const cert_transparency_verifier_;
   QuicCryptoClientStreamFactory* quic_crypto_client_stream_factory_;
   quic::QuicRandom* random_generator_;  // Unowned.
   quic::QuicClock* clock_;              // Unowned.
@@ -625,8 +677,19 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // Origins which have gone away recently.
   AliasSet gone_away_aliases_;
 
+  // When a QuicCryptoClientConfig is in use, it has one or more live
+  // CryptoClientConfigHandles, and is stored in |active_crypto_config_map_|.
+  // Once all the handles are deleted, it's moved to
+  // |recent_crypto_config_map_|. If reused before it is evicted from MRUCache,
+  // it will be removed from the cache and return to the active config map.
+  // These two maps should never both have entries with the same
+  // NetworkIsolationKey.
+  QuicCryptoClientConfigMap active_crypto_config_map_;
+  base::MRUCache<NetworkIsolationKey,
+                 std::unique_ptr<QuicCryptoClientConfigOwner>>
+      recent_crypto_config_map_;
+
   const quic::QuicConfig config_;
-  quic::QuicCryptoClientConfig crypto_config_;
 
   JobMap active_jobs_;
 
@@ -656,6 +719,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // True if we need to check HttpServerProperties if QUIC was supported last
   // time.
   bool need_to_check_persisted_supports_quic_;
+  bool prefer_aes_gcm_recorded_;
 
   NetworkConnection network_connection_;
 
@@ -669,6 +733,16 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
 
   SSLConfigService* const ssl_config_service_;
 
+  // Whether NetworkIsolationKeys should be used for
+  // |active_crypto_config_map_|. If false, there will just be one config with
+  // an empty NetworkIsolationKey. Whether QuicSessionAliasKeys all have an
+  // empty NIK is based on whether socket pools are respecting NIKs, but whether
+  // those NIKs are also used when accessing |active_crypto_config_map_| is also
+  // gated this, which is set based on whether HttpServerProperties is
+  // respecting NIKs, as that data is fed into the crypto config map using the
+  // corresponding NIK.
+  const bool use_network_isolation_key_for_crypto_configs_;
+
   base::WeakPtrFactory<QuicStreamFactory> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(QuicStreamFactory);

net/quic/quic_stream_factory_peer.cc

@@ -27,9 +27,11 @@ const quic::QuicConfig* QuicStreamFactoryPeer::GetConfig(
   return &factory->config_;
 }
 
-quic::QuicCryptoClientConfig* QuicStreamFactoryPeer::GetCryptoConfig(
-    QuicStreamFactory* factory) {
-  return &factory->crypto_config_;
+std::unique_ptr<QuicCryptoClientConfigHandle>
+QuicStreamFactoryPeer::GetCryptoConfig(
+    QuicStreamFactory* factory,
+    const NetworkIsolationKey& network_isolation_key) {
+  return factory->GetCryptoConfigForTesting(network_isolation_key);
 }
 
 bool QuicStreamFactoryPeer::HasActiveSession(
@@ -128,9 +130,11 @@ void QuicStreamFactoryPeer::SetRaceCertVerification(
 quic::QuicAsyncStatus QuicStreamFactoryPeer::StartCertVerifyJob(
     QuicStreamFactory* factory,
     const quic::QuicServerId& server_id,
+    const NetworkIsolationKey& network_isolation_key,
     int cert_verify_flags,
     const NetLogWithSource& net_log) {
-  return factory->StartCertVerifyJob(server_id, cert_verify_flags, net_log);
+  return factory->StartCertVerifyJobForTesting(server_id, network_isolation_key,
+                                               cert_verify_flags, net_log);
 }
 
 void QuicStreamFactoryPeer::SetYieldAfterPackets(QuicStreamFactory* factory,
@@ -146,13 +150,16 @@ void QuicStreamFactoryPeer::SetYieldAfterDuration(
 
 bool QuicStreamFactoryPeer::CryptoConfigCacheIsEmpty(
     QuicStreamFactory* factory,
-    const quic::QuicServerId& quic_server_id) {
-  return factory->CryptoConfigCacheIsEmpty(quic_server_id);
+    const quic::QuicServerId& quic_server_id,
+    const NetworkIsolationKey& network_isolation_key) {
+  return factory->CryptoConfigCacheIsEmpty(quic_server_id,
+                                           network_isolation_key);
 }
 
 void QuicStreamFactoryPeer::CacheDummyServerConfig(
     QuicStreamFactory* factory,
-    const quic::QuicServerId& quic_server_id) {
+    const quic::QuicServerId& quic_server_id,
+    const NetworkIsolationKey& network_isolation_key) {
   // Minimum SCFG that passes config validation checks.
   const char scfg[] = {// SCFG
                        0x53, 0x43, 0x46, 0x47,
@@ -178,9 +185,10 @@ void QuicStreamFactoryPeer::CacheDummyServerConfig(
   DCHECK(cert);
   certs.emplace_back(x509_util::CryptoBufferAsStringPiece(cert->cert_buffer()));
 
-  quic::QuicCryptoClientConfig* crypto_config = &factory->crypto_config_;
+  std::unique_ptr<QuicCryptoClientConfigHandle> crypto_config_handle =
+      GetCryptoConfig(factory, network_isolation_key);
   quic::QuicCryptoClientConfig::CachedState* cached =
-      crypto_config->LookupOrCreate(quic_server_id);
+      crypto_config_handle->GetConfig()->LookupOrCreate(quic_server_id);
   quic::QuicChromiumClock clock;
   cached->Initialize(server_config, source_address_token, certs, "", "",
                      signature, clock.WallNow(), quic::QuicWallTime::Zero());

net/quic/quic_stream_factory_peer.h

@@ -8,6 +8,8 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <memory>
+
 #include "base/macros.h"
 #include "base/sequenced_task_runner.h"
 #include "base/time/tick_clock.h"
@@ -22,13 +24,13 @@ namespace quic {
 class QuicAlarmFactory;
 class QuicClientPushPromiseIndex;
 class QuicConfig;
-class QuicCryptoClientConfig;
 }  // namespace quic
 
 namespace net {
 
 class NetLogWithSource;
 class QuicChromiumClientSession;
+class QuicCryptoClientConfigHandle;
 class QuicStreamFactory;
 
 namespace test {
@@ -37,8 +39,9 @@ class QuicStreamFactoryPeer {
  public:
   static const quic::QuicConfig* GetConfig(QuicStreamFactory* factory);
 
-  static quic::QuicCryptoClientConfig* GetCryptoConfig(
-      QuicStreamFactory* factory);
+  static std::unique_ptr<QuicCryptoClientConfigHandle> GetCryptoConfig(
+      QuicStreamFactory* factory,
+      const NetworkIsolationKey& network_isolation_key);
 
   static bool HasActiveSession(
       QuicStreamFactory* factory,
@@ -81,9 +84,13 @@ class QuicStreamFactoryPeer {
   static void SetRaceCertVerification(QuicStreamFactory* factory,
                                       bool race_cert_verification);
 
+  // When using this method, the caller should be holding onto a live
+  // NetworkIsolationKey, if it wants the results to stay alive in the
+  // per-NetworkIsolationKey cache.
   static quic::QuicAsyncStatus StartCertVerifyJob(
       QuicStreamFactory* factory,
       const quic::QuicServerId& server_id,
+      const NetworkIsolationKey& network_isolation_key,
       int cert_verify_flags,
       const NetLogWithSource& net_log);
 
@@ -98,11 +105,16 @@ class QuicStreamFactoryPeer {
 
   static bool CryptoConfigCacheIsEmpty(
       QuicStreamFactory* factory,
-      const quic::QuicServerId& quic_server_id);
+      const quic::QuicServerId& quic_server_id,
+      const NetworkIsolationKey& network_isolation_key);
 
-  // Creates a dummy QUIC server config and caches it.
-  static void CacheDummyServerConfig(QuicStreamFactory* factory,
-                                     const quic::QuicServerId& quic_server_id);
+  // Creates a dummy QUIC server config and caches it. Caller must be holding
+  // onto a QuicCryptoClientConfigHandle for the corresponding
+  // |network_isolation_key|.
+  static void CacheDummyServerConfig(
+      QuicStreamFactory* factory,
+      const quic::QuicServerId& quic_server_id,
+      const NetworkIsolationKey& network_isolation_key);
 
   static quic::QuicClientPushPromiseIndex* GetPushPromiseIndex(
       QuicStreamFactory* factory);

net/quic/test_quic_crypto_client_config_handle.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/quic/test_quic_crypto_client_config_handle.h"
+
+namespace net {
+
+TestQuicCryptoClientConfigHandle::TestQuicCryptoClientConfigHandle(
+    quic::QuicCryptoClientConfig* crypto_config)
+    : crypto_config_(crypto_config) {}
+
+TestQuicCryptoClientConfigHandle::~TestQuicCryptoClientConfigHandle() = default;
+
+quic::QuicCryptoClientConfig* TestQuicCryptoClientConfigHandle::GetConfig()
+    const {
+  return crypto_config_;
+}
+
+}  // namespace net

net/quic/test_quic_crypto_client_config_handle.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_QUIC_TEST_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_
+#define NET_QUIC_TEST_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_
+
+#include "base/macros.h"
+#include "net/quic/quic_crypto_client_config_handle.h"
+
+namespace quic {
+class QuicCryptoClientConfig;
+}  // namespace quic
+
+namespace net {
+
+// Test implementation of QuicCryptoClientConfigHandle. Wraps a passed in
+// QuicCryptoClientConfig and returns it as needed. Does nothing on destruction.
+class TestQuicCryptoClientConfigHandle : public QuicCryptoClientConfigHandle {
+ public:
+  TestQuicCryptoClientConfigHandle(quic::QuicCryptoClientConfig* crypto_config);
+  ~TestQuicCryptoClientConfigHandle() override;
+
+  quic::QuicCryptoClientConfig* GetConfig() const override;
+
+ private:
+  quic::QuicCryptoClientConfig* const crypto_config_;
+
+  DISALLOW_ASSIGN(TestQuicCryptoClientConfigHandle);
+};
+
+}  // namespace net
+
+#endif  // NET_QUIC_TEST_QUIC_CRYPTO_CLIENT_CONFIG_HANDLE_H_