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..773ef65d4ab9b8fc5271e4062d92db9880e441cb 100644
--- a/net/cert/ct_signed_certificate_timestamp_log_param.cc
+++ b/net/cert/ct_signed_certificate_timestamp_log_param.cc
@@ -62,10 +62,12 @@ std::unique_ptr<base::DictionaryValue> SCTToDictionary(
 // Given a list of SCTs, return a ListValue instance where each item in the
 // list is a dictionary created by SCTToDictionary.

[estark, 2016/08/09 20:02:04]

nit: perhaps update this comment to note that we only output SCTs matching
|match_status|

[Eran Messeri, 2016/08/10 22:05:07]

Done.

 std::unique_ptr<base::ListValue> SCTListToPrintableValues(
-    const ct::SCTList& sct_list) {
+    const SignedCertificateTimestampAndStatusList& sct_and_status_list,
+    ct::SCTVerifyStatus match_status) {
   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)
+    if (sct_and_status.status == match_status)
+      output_scts->Append(SCTToDictionary(*(sct_and_status.sct.get())));
 
   return output_scts;
 }
@@ -78,13 +80,14 @@ std::unique_ptr<base::Value> NetLogSignedCertificateTimestampCallback(
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
 
   dict->Set("verified_scts",
-            SCTListToPrintableValues(ct_result->verified_scts));
+            SCTListToPrintableValues(ct_result->scts, ct::SCT_STATUS_OK));

[eroman, 2016/08/10 19:00:40]

Is there a need to preserve the old format?

AFAICT there isn't a consumer that relies on this structure (just getting dumped
into NetLog right)?

In that case, can you re-structure it to match the code organization?

For instance add an "scts" parameter that is an array of all the SCTs.

This should be more informative when debugging issues (in case order ends up
mattering), and also means if a new veryfstatus is added don't need to remember
to update this.

If you do not make that change, then please add a breadcrumb to inform callers
that this needs to be updated. Something like this would suffice:

static_assert(ct::SCT_STATUS_MAX == 4, "When adding new verify status add it to
the NetLog here");

[Eran Messeri, 2016/08/10 22:05:07]

Re-structured as suggested. Now the status is passed all the way to
SCTToDictionary and is added to the dictionary as an integer value there.

Note this affects the MultiLogCTVerifier unit test.

 
   dict->Set("invalid_scts",
-            SCTListToPrintableValues(ct_result->invalid_scts));
+            SCTListToPrintableValues(ct_result->scts, ct::SCT_STATUS_INVALID));
 
-  dict->Set("unknown_logs_scts",
-            SCTListToPrintableValues(ct_result->unknown_logs_scts));
+  dict->Set(
+      "unknown_logs_scts",
+      SCTListToPrintableValues(ct_result->scts, ct::SCT_STATUS_LOG_UNKNOWN));
 
   return std::move(dict);
 }