Make SSLClientSessionCache take a struct for a key.

When client certificate preferences change, we should clear all session
cache entries for the host. This allows SSLClientSessionCache to
determine the host portion of each key.

Note this changes it from a std::unordered_map-backed MRUCache to a
std::map-backed one. I'm assuming that, given the cache is bounded to
1024 entries, it doesn't matter either way.

Bug: 951205
Change-Id: Ia6cb77a7331fa8cfc8aaa86f366b42f845940cd3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1707035
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683789}

net/socket/ssl_client_socket_impl.cc

@@ -48,7 +48,6 @@
 #include "net/log/net_log_values.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
-#include "net/ssl/ssl_client_session_cache.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_handshake_details.h"
 #include "net/ssl/ssl_info.h"
@@ -1669,25 +1668,17 @@ void SSLClientSocketImpl::AddCTInfoToSSLInfo(SSLInfo* ssl_info) const {
   ssl_info->UpdateCertificateTransparencyInfo(ct_verify_result_);
 }
 
-std::string SSLClientSocketImpl::GetSessionCacheKey(
+SSLClientSessionCache::Key SSLClientSocketImpl::GetSessionCacheKey(
     base::Optional<IPAddress> dest_ip_addr) const {
-  std::string ret;
-  if (dest_ip_addr) {
-    ret += dest_ip_addr->ToString();
-  }
-  ret.push_back('/');
-  ret += host_and_port_.ToString();
-  ret.push_back('/');
-  if (ssl_config_.privacy_mode == PRIVACY_MODE_ENABLED) {
-    ret.push_back('1');
-  } else {
-    ret.push_back('0');
-  }
+  SSLClientSessionCache::Key key;
+  key.server = host_and_port_;
+  key.dest_ip_addr = dest_ip_addr;
   if (base::FeatureList::IsEnabled(
           features::kPartitionSSLSessionsByNetworkIsolationKey)) {
-    ret += '/' + ssl_config_.network_isolation_key.ToString();
+    key.network_isolation_key = ssl_config_.network_isolation_key;
   }
-  return ret;
+  key.privacy_mode = ssl_config_.privacy_mode;
+  return key;
 }
 
 bool SSLClientSocketImpl::IsRenegotiationAllowed() const {

net/socket/ssl_client_socket_impl.h

@@ -30,6 +30,7 @@
 #include "net/socket/stream_socket.h"
 #include "net/ssl/openssl_ssl_util.h"
 #include "net/ssl/ssl_client_cert_type.h"
+#include "net/ssl/ssl_client_session_cache.h"
 #include "net/ssl/ssl_config.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "third_party/boringssl/src/include/openssl/base.h"
@@ -162,8 +163,9 @@ class SSLClientSocketImpl : public SSLClientSocket,
   // the |ssl_info|.signed_certificate_timestamps list.
   void AddCTInfoToSSLInfo(SSLInfo* ssl_info) const;
 
-  // Returns a unique key string for the SSL session cache for this socket.
-  std::string GetSessionCacheKey(base::Optional<IPAddress> dest_ip_addr) const;
+  // Returns a session cache key for this socket.
+  SSLClientSessionCache::Key GetSessionCacheKey(
+      base::Optional<IPAddress> dest_ip_addr) const;
 
   // Returns true if renegotiations are allowed.
   bool IsRenegotiationAllowed() const;

net/ssl/ssl_client_session_cache.cc

@@ -4,6 +4,7 @@
 
 #include "net/ssl/ssl_client_session_cache.h"
 
+#include <tuple>
 #include <utility>
 
 #include "base/containers/flat_set.h"
@@ -24,6 +25,27 @@ bool IsTLS13(const SSL_SESSION* session) {
 
 }  // namespace
 
+SSLClientSessionCache::Key::Key() = default;
+SSLClientSessionCache::Key::Key(const Key& other) = default;
+SSLClientSessionCache::Key::Key(Key&& other) = default;
+SSLClientSessionCache::Key::~Key() = default;
+SSLClientSessionCache::Key& SSLClientSessionCache::Key::operator=(
+    const Key& other) = default;
+SSLClientSessionCache::Key& SSLClientSessionCache::Key::operator=(Key&& other) =
+    default;
+
+bool SSLClientSessionCache::Key::operator==(const Key& other) const {
+  return std::tie(server, dest_ip_addr, network_isolation_key, privacy_mode) ==
+         std::tie(other.server, other.dest_ip_addr, other.network_isolation_key,
+                  other.privacy_mode);
+}
+
+bool SSLClientSessionCache::Key::operator<(const Key& other) const {
+  return std::tie(server, dest_ip_addr, network_isolation_key, privacy_mode) <
+         std::tie(other.server, other.dest_ip_addr, other.network_isolation_key,
+                  other.privacy_mode);
+}
+
 SSLClientSessionCache::SSLClientSessionCache(const Config& config)
     : clock_(base::DefaultClock::GetInstance()),
       config_(config),
@@ -42,7 +64,7 @@ size_t SSLClientSessionCache::size() const {
 }
 
 bssl::UniquePtr<SSL_SESSION> SSLClientSessionCache::Lookup(
-    const std::string& cache_key) {
+    const Key& cache_key) {
   // Expire stale sessions.
   lookups_since_flush_++;
   if (lookups_since_flush_ >= config_.expiration_check_count) {
@@ -73,7 +95,7 @@ bssl::UniquePtr<SSL_SESSION> SSLClientSessionCache::Lookup(
   return session;
 }
 
-void SSLClientSessionCache::Insert(const std::string& cache_key,
+void SSLClientSessionCache::Insert(const Key& cache_key,
                                    bssl::UniquePtr<SSL_SESSION> session) {
   if (IsTLS13(session.get())) {
     base::TimeDelta lifetime =

net/ssl/ssl_client_session_cache.h

@@ -15,9 +15,14 @@
 #include "base/containers/mru_cache.h"
 #include "base/macros.h"
 #include "base/memory/memory_pressure_monitor.h"
+#include "base/optional.h"
 #include "base/time/time.h"
 #include "base/trace_event/memory_dump_provider.h"
+#include "net/base/host_port_pair.h"
+#include "net/base/ip_address.h"
 #include "net/base/net_export.h"
+#include "net/base/network_isolation_key.h"
+#include "net/base/privacy_mode.h"
 #include "third_party/boringssl/src/include/openssl/base.h"
 
 namespace base {
@@ -38,6 +43,23 @@ class NET_EXPORT SSLClientSessionCache {
     size_t expiration_check_count = 256;
   };
 
+  struct NET_EXPORT Key {
+    Key();
+    Key(const Key& other);
+    Key(Key&& other);
+    ~Key();
+    Key& operator=(const Key& other);
+    Key& operator=(Key&& other);
+
+    bool operator==(const Key& other) const;
+    bool operator<(const Key& other) const;
+
+    HostPortPair server;
+    base::Optional<IPAddress> dest_ip_addr;
+    NetworkIsolationKey network_isolation_key;
+    PrivacyMode privacy_mode = PRIVACY_MODE_DISABLED;
+  };
+
   explicit SSLClientSessionCache(const Config& config);
   ~SSLClientSessionCache();
 
@@ -48,13 +70,12 @@ class NET_EXPORT SSLClientSessionCache {
 
   // Returns the session associated with |cache_key| and moves it to the front
   // of the MRU list. Returns nullptr if there is none.
-  bssl::UniquePtr<SSL_SESSION> Lookup(const std::string& cache_key);
+  bssl::UniquePtr<SSL_SESSION> Lookup(const Key& cache_key);
 
   // Inserts |session| into the cache at |cache_key|. If there is an existing
   // one, it is released. Every |expiration_check_count| calls, the cache is
   // checked for stale entries.
-  void Insert(const std::string& cache_key,
-              bssl::UniquePtr<SSL_SESSION> session);
+  void Insert(const Key& cache_key, bssl::UniquePtr<SSL_SESSION> session);
 
   // Removes all entries from the cache.
   void Flush();
@@ -96,7 +117,7 @@ class NET_EXPORT SSLClientSessionCache {
 
   base::Clock* clock_;
   Config config_;
-  base::HashingMRUCache<std::string, Entry> cache_;
+  base::MRUCache<Key, Entry> cache_;
   size_t lookups_since_flush_;
   std::unique_ptr<base::MemoryPressureListener> memory_pressure_listener_;