Certificate Transparency: Change CTVerifyResult to have a single list

net/cert/ct_signed_certificate_timestamp_log_param.cc

Index: net/cert/ct_signed_certificate_timestamp_log_param.cc
diff --git a/net/cert/ct_signed_certificate_timestamp_log_param.cc b/net/cert/ct_signed_certificate_timestamp_log_param.cc
index f829e56a8139ae1a24a40d5ce3f950af83f1b332..7e4bdc5d3c2ac854deccb3a15c92dac49c18773e 100644
--- a/net/cert/ct_signed_certificate_timestamp_log_param.cc
+++ b/net/cert/ct_signed_certificate_timestamp_log_param.cc
@@ -37,10 +37,12 @@ void SetBinaryData(
 // is this field's value in the SCT. This dictionary is meant to be used for
 // outputting a de-serialized SCT to the NetLog.
 std::unique_ptr<base::DictionaryValue> SCTToDictionary(
-    const ct::SignedCertificateTimestamp& sct) {
+    const ct::SignedCertificateTimestamp& sct,
+    ct::SCTVerifyStatus status) {
   std::unique_ptr<base::DictionaryValue> out(new base::DictionaryValue());
 
   out->SetString("origin", OriginToString(sct.origin));
+  out->SetInteger("verification_status", status);

[eroman, 2016/08/11 22:55:50]

[optional] Unless the integer code is going to be stable, I suggest turning it
into a string here (easier to then read the NetLog). There are other options
like doing the reverse mapping on the NetLog viewer UI that can be explored, but
unless the data size is going to be a problem I suggest just saving a string.

[Eran Messeri, 2016/08/13 21:53:26]

The integer could should be stable as the same enum (SCTVerifyStatus) is used
for UMA and its documentation explicitly states that the values should remain
stable.

I've simply turned this into a string as you suggested, though,  as there's
already a handy function to do this
(https://cs.chromium.org/chromium/src/net/cert/ct_sct_to_string.h?l=33).

As a side note, are there memory constraints around NetLog logging? the reason I
stuck with an integer is the lower memory consumption. Is that a concern when
logging to NetLog?

[eroman, 2016/08/15 17:48:07]

<background>....

Memory is always a concern. In the case of NetLog these are serialized to disk
so the important metric is filesize, not so much the amount of RAM (which is
short lived).

That said, there is also a tension with ease-of-use and revisioning. So unless
the status code is going to appear frequently, I discourage against it.

The NetLog format aims to be self-describing (by virtue of being JSON), and
independent of C++ side changes. This makes it easy on the C++ developers as I
don't want them to have to worry about refactoring changes breaking NetLogs
(case in point this CL).

Yet at the same time there is enough information in the log format for it to be
useful when consuming. String statuses are self-describing to the viewer so no
special visualization needs to be added for those events in the NetLog viewer
application.

There are some frequently used enums like load status and net error codes which
are encoded in the logs as integers, however the log file itself also carries
the mapping back to their symbolic name (i.e. like a symbol table) so this
really just serves as a dumb form of compression (effectively their string value
is still encoded in the log).

At some point in the future I hope to make NetLog's use a compressed file
format, so the duplication of string status wasting disk space will be less of a
concern.

   out->SetInteger("version", sct.version);
 
   SetBinaryData("log_id", sct.log_id, out.get());
@@ -59,13 +61,14 @@ std::unique_ptr<base::DictionaryValue> SCTToDictionary(
   return out;
 }
 
-// Given a list of SCTs, return a ListValue instance where each item in the
-// list is a dictionary created by SCTToDictionary.
+// Given a list of SCTs and their statuses, return a ListValue instance where
+// each item in the list is a dictionary created by SCTToDictionary.
 std::unique_ptr<base::ListValue> SCTListToPrintableValues(
-    const ct::SCTList& sct_list) {
+    const SignedCertificateTimestampAndStatusList& sct_and_status_list) {
   std::unique_ptr<base::ListValue> output_scts(new base::ListValue());
-  for (const auto& sct : sct_list)
-    output_scts->Append(SCTToDictionary(*(sct.get())));
+  for (const auto& sct_and_status : sct_and_status_list)
+    output_scts->Append(
+        SCTToDictionary(*(sct_and_status.sct.get()), sct_and_status.status));
 
   return output_scts;
 }
@@ -77,14 +80,7 @@ std::unique_ptr<base::Value> NetLogSignedCertificateTimestampCallback(
     NetLogCaptureMode capture_mode) {
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
 
-  dict->Set("verified_scts",
-            SCTListToPrintableValues(ct_result->verified_scts));
-
-  dict->Set("invalid_scts",
-            SCTListToPrintableValues(ct_result->invalid_scts));
-
-  dict->Set("unknown_logs_scts",
-            SCTListToPrintableValues(ct_result->unknown_logs_scts));
+  dict->Set("scts", SCTListToPrintableValues(ct_result->scts));
 
   return std::move(dict);
 }