Add DNS cache entries with transient NIKs to net-export logs.

Bug: 997049
Change-Id: I2b6bd94a72110094914e0962c34f8bb893a19ef4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154726
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762002}

components/cronet/host_cache_persistence_manager.cc

@@ -84,7 +84,8 @@ void HostCachePersistenceManager::WriteToDisk() {
 
   net_log_.AddEvent(net::NetLogEventType::HOST_CACHE_PREF_WRITE);
   base::ListValue value;
-  cache_->GetAsListValue(&value, false);
+  cache_->GetAsListValue(&value, false,
+                         net::HostCache::SerializationType::kRestorable);
   writing_pref_ = true;
   pref_service_->Set(pref_name_, value);
   writing_pref_ = false;

components/cronet/host_cache_persistence_manager_unittest.cc

@@ -85,7 +85,8 @@ class HostCachePersistenceManagerTest : public testing::Test {
                    base::TimeDelta::FromSeconds(1));
 
     base::ListValue value;
-    temp_cache.GetAsListValue(&value, false);
+    temp_cache.GetAsListValue(&value, false /* include_stale */,
+                              net::HostCache::SerializationType::kRestorable);
     pref_service_->Set(kPrefName, value);
   }
 

net/dns/host_cache.cc

@@ -694,7 +694,8 @@ void HostCache::ClearForHosts(
 }
 
 void HostCache::GetAsListValue(base::ListValue* entry_list,
-                               bool include_staleness) const {
+                               bool include_staleness,
+                               SerializationType serialization_type) const {
   DCHECK(entry_list);
   entry_list->Clear();
 
@@ -703,9 +704,17 @@ void HostCache::GetAsListValue(base::ListValue* entry_list,
     const Entry& entry = pair.second;
 
     base::Value network_isolation_key_value;
-    // Don't save entries associated with ephemeral NetworkIsolationKeys.
-    if (!key.network_isolation_key.ToValue(&network_isolation_key_value))
-      continue;
+    if (serialization_type == SerializationType::kRestorable) {
+      // Don't save entries associated with ephemeral NetworkIsolationKeys.
+      if (!key.network_isolation_key.ToValue(&network_isolation_key_value))
+        continue;
+    } else {
+      // ToValue() fails for transient NIKs, since they should never be
+      // serialized to disk in a restorable format, so use ToDebugString() when
+      // serializing for debugging instead of for restoring from disk.
+      network_isolation_key_value =
+          base::Value(key.network_isolation_key.ToDebugString());
+    }
 
     auto entry_dict = std::make_unique<base::DictionaryValue>(
         entry.GetAsValue(include_staleness));
@@ -776,6 +785,7 @@ bool HostCache::RestoreFromListValue(const base::ListValue& old_cache) {
         entry_dict->FindKey(kNetworkIsolationKeyKey);
     NetworkIsolationKey network_isolation_key;
     if (!network_isolation_key_value ||
+        network_isolation_key_value->type() == base::Value::Type::STRING ||
         !NetworkIsolationKey::FromValue(*network_isolation_key_value,
                                         &network_isolation_key)) {
       return false;

net/dns/host_cache.h

@@ -280,6 +280,18 @@ class NET_EXPORT HostCache {
 
   using EntryMap = std::map<Key, Entry>;
 
+  // The two ways to serialize the cache to a value.
+  enum class SerializationType {
+    // Entries with transient NetworkIsolationKeys are not serialized, and
+    // RestoreFromListValue() can load the returned value.
+    kRestorable,
+    // Entries with transient NetworkIsolationKeys are serialized, and
+    // RestoreFromListValue() cannot load the returned value, since the debug
+    // serialization of NetworkIsolationKeys is used instead of the
+    // deserializable representation.
+    kDebug,
+  };
+
   // A HostCache::EntryStaleness representing a non-stale (fresh) cache entry.
   static const HostCache::EntryStaleness kNotStale;
 
@@ -341,15 +353,10 @@ class NET_EXPORT HostCache {
 
   // Fills the provided base::ListValue with the contents of the cache for
   // serialization. |entry_list| must be non-null and will be cleared before
-  // adding the cache contents. Entries with ephemeral NetworkIsolationKeys will
-  // not be written to the resulting list.
-  //
-  // TODO(mmenke): This is used both in combination with RestoreFromListValue()
-  // and for NetLog. Update the NetLogViewer's display to handle
-  // NetworkIsolationKeys, and add some way for to get a result with ephemeral
-  // NIKs included.
+  // adding the cache contents.
   void GetAsListValue(base::ListValue* entry_list,
-                      bool include_staleness) const;
+                      bool include_staleness,
+                      SerializationType serialization_type) const;
   // Takes a base::ListValue representing cache entries and stores them in the
   // cache, skipping any that already have entries. Returns true on success,
   // false on failure.

net/dns/host_cache_unittest.cc

@@ -908,7 +908,8 @@ TEST(HostCacheTest, SerializeAndDeserialize) {
   now += base::TimeDelta::FromSeconds(7);
 
   base::ListValue serialized_cache;
-  cache.GetAsListValue(&serialized_cache, /*include_staleness=*/false);
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kRestorable);
   HostCache restored_cache(kMaxCacheEntries);
 
   // Add entries for "foobar3.com" and "foobar4.com" to the cache before
@@ -1018,7 +1019,8 @@ TEST(HostCacheTest, SerializeAndDeserializeWithNetworkIsolationKey) {
   EXPECT_EQ(2u, cache.size());
 
   base::ListValue serialized_cache;
-  cache.GetAsListValue(&serialized_cache, /*include_staleness=*/false);
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kRestorable);
   HostCache restored_cache(kMaxCacheEntries);
   EXPECT_TRUE(restored_cache.RestoreFromListValue(serialized_cache));
   EXPECT_EQ(1u, restored_cache.size());
@@ -1034,6 +1036,43 @@ TEST(HostCacheTest, SerializeAndDeserializeWithNetworkIsolationKey) {
   EXPECT_FALSE(restored_cache.Lookup(key2, now));
 }
 
+TEST(HostCacheTest, SerializeForDebugging) {
+  const char kHostname[] = "hostname.test";
+  const base::TimeDelta kTTL = base::TimeDelta::FromSeconds(10);
+  const NetworkIsolationKey kNetworkIsolationKey =
+      NetworkIsolationKey::CreateTransient();
+
+  HostCache::Key key(kHostname, DnsQueryType::UNSPECIFIED, 0,
+                     HostResolverSource::ANY, kNetworkIsolationKey);
+  IPEndPoint endpoint(IPAddress(1, 2, 3, 4), 0);
+
+  HostCache::Entry entry = HostCache::Entry(OK, AddressList(endpoint),
+                                            HostCache::Entry::SOURCE_UNKNOWN);
+
+  base::TimeTicks now;
+  HostCache cache(kMaxCacheEntries);
+
+  cache.Set(key, entry, now, kTTL);
+
+  EXPECT_TRUE(cache.Lookup(key, now));
+  EXPECT_EQ(kNetworkIsolationKey,
+            cache.Lookup(key, now)->first.network_isolation_key);
+  EXPECT_EQ(1u, cache.size());
+
+  base::ListValue serialized_cache;
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kDebug);
+  HostCache restored_cache(kMaxCacheEntries);
+  EXPECT_FALSE(restored_cache.RestoreFromListValue(serialized_cache));
+
+  base::Value::ListView list = serialized_cache.GetList();
+  ASSERT_EQ(1u, list.size());
+  ASSERT_TRUE(list[0].is_dict());
+  base::Value* nik_value = list[0].FindPath("network_isolation_key");
+  ASSERT_TRUE(nik_value);
+  ASSERT_EQ(base::Value(kNetworkIsolationKey.ToDebugString()), *nik_value);
+}
+
 TEST(HostCacheTest, SerializeAndDeserialize_Text) {
   base::TimeTicks now;
 
@@ -1050,7 +1089,8 @@ TEST(HostCacheTest, SerializeAndDeserialize_Text) {
   EXPECT_EQ(1u, cache.size());
 
   base::ListValue serialized_cache;
-  cache.GetAsListValue(&serialized_cache, false /* include_staleness */);
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kRestorable);
   HostCache restored_cache(kMaxCacheEntries);
   restored_cache.RestoreFromListValue(serialized_cache);
 
@@ -1089,7 +1129,8 @@ TEST(HostCacheTest, SerializeAndDeserialize_Esni) {
   EXPECT_EQ(1u, cache.size());
 
   base::ListValue serialized_cache;
-  cache.GetAsListValue(&serialized_cache, false /* include_staleness */);
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kRestorable);
   HostCache restored_cache(kMaxCacheEntries);
   restored_cache.RestoreFromListValue(serialized_cache);
 
@@ -1132,7 +1173,8 @@ class HostCacheMalformedEsniSerializationTest : public ::testing::Test {
     HostCache cache(kMaxCacheEntries);
     cache.Set(key, entry, now, ttl);
     EXPECT_EQ(1u, cache.size());
-    cache.GetAsListValue(&serialized_cache_, true /* include_staleness */);
+    cache.GetAsListValue(&serialized_cache_, true /* include_staleness */,
+                         HostCache::SerializationType::kRestorable);
   }
 
   base::ListValue serialized_cache_;
@@ -1179,7 +1221,8 @@ TEST(HostCacheTest, SerializeAndDeserialize_Hostname) {
   EXPECT_EQ(1u, cache.size());
 
   base::ListValue serialized_cache;
-  cache.GetAsListValue(&serialized_cache, false /* include_staleness */);
+  cache.GetAsListValue(&serialized_cache, false /* include_staleness */,
+                       HostCache::SerializationType::kRestorable);
   HostCache restored_cache(kMaxCacheEntries);
   restored_cache.RestoreFromListValue(serialized_cache);
 

net/log/net_log_util.cc

@@ -381,7 +381,8 @@ NET_EXPORT std::unique_ptr<base::DictionaryValue> GetNetInfo(
       cache_info_dict->SetInteger("network_changes", cache->network_changes());
 
       cache->GetAsListValue(cache_contents_list.get(),
-                            /*include_staleness=*/true);
+                            true /* include_staleness */,
+                            HostCache::SerializationType::kDebug);
       cache_info_dict->Set("entries", std::move(cache_contents_list));
 
       dict->Set("cache", std::move(cache_info_dict));