Add Certificate Transparency logs auditing

components/certificate_transparency/tree_state_tracker.cc

Index: components/certificate_transparency/tree_state_tracker.cc
diff --git a/components/certificate_transparency/tree_state_tracker.cc b/components/certificate_transparency/tree_state_tracker.cc
index 118cec0fedb94f4f149281c106a09ee730534cf7..79875c2b610442d4d6f5fef46ae14bf65b772a4b 100644
--- a/components/certificate_transparency/tree_state_tracker.cc
+++ b/components/certificate_transparency/tree_state_tracker.cc
@@ -4,23 +4,49 @@
 
 #include "components/certificate_transparency/tree_state_tracker.h"
 
+#include "base/feature_list.h"
+#include "base/memory/ptr_util.h"
+#include "components/certificate_transparency/log_dns_client.h"
 #include "components/certificate_transparency/single_tree_tracker.h"
+#include "net/base/network_change_notifier.h"
 #include "net/cert/ct_log_verifier.h"
 #include "net/cert/signed_certificate_timestamp.h"
 #include "net/cert/signed_tree_head.h"
 #include "net/cert/x509_certificate.h"
+#include "net/dns/dns_client.h"
+#include "net/dns/dns_config_service.h"
+#include "net/log/net_log.h"
 
 using net::X509Certificate;
 using net::CTLogVerifier;
 using net::ct::SignedCertificateTimestamp;
 using net::ct::SignedTreeHead;
 
+namespace {
+const size_t kMaxConcurrentDnsQueries = 1;
+}
+
 namespace certificate_transparency {
 
+// Enables or disables auditing Certificate Transparency logs over DNS.
+const base::Feature kCTLogAuditing = {"CertificateTransparencyLogAuditing",
+                                      base::FEATURE_DISABLED_BY_DEFAULT};
+
 TreeStateTracker::TreeStateTracker(
     std::vector<scoped_refptr<const CTLogVerifier>> ct_logs) {
-  for (const auto& log : ct_logs)
-    tree_trackers_[log->key_id()].reset(new SingleTreeTracker(log));
+  if (!base::FeatureList::IsEnabled(kCTLogAuditing))
+    return;
+
+  net::NetLogWithSource net_log;

[Ryan Sleevi, 2017/01/10 03:15:54]

I'm a little squicky about this and whether it's OK.

eroman: Can you offer thoughts on this?

[eroman, 2017/01/10 21:23:10]

This is technically a supported pattern, in that it has the effect of passing a
null NetLog dependency.

The consequence is that any NetLog events emitted by this DnsClient and
LogDnsClient are sent to oblivion.

This is not generally something we want to do.

I haven't read the surrounding code to determine whether in this instance this
is reasonable.

Without knowing more, my recommendation is to forward the NetLog dependency so
this DNS activity shows up in our ChromeNetLog, or justify why we don't need to
log it.

Cheers.

[Eran Messeri, 2017/01/17 11:37:57]

I can fix it the following way:
* Pass in a NetLog from the IOThread when creating the TreeStateTracker:
In
https://cs.chromium.org/chromium/src/chrome/browser/io_thread.cc?q=TreeStateT...
, where the TreeStateTracker is created, there's already an initialized
ChromeNetLog instance (net_log_).

So a NetLog instance would be passed into the TreeStateTracker c'tor.
Here, a NetLogWithSource will be created from the NetLog instance (I would add a
new source type in net_log_source_type_list.h, to indicate all dns queries are
related to CT inclusion proof fetching).

Would that address your concern, Ryan?
(I'd rather do it in a follow-up CL to avoid increasing the scope of this CL,
since such a change would touch more net/ code, the io_thread and
profile_io_data).

[eroman, 2017/01/20 00:03:49]

That works for me.
Stick a TODO by this line and you can follow-up with me separately.

[Eran Messeri, 2017/01/20 09:49:05]

Done.

+  std::unique_ptr<net::DnsClient> dns_client =
+      net::DnsClient::CreateClient(net_log.net_log());
+  dns_client_ = base::MakeUnique<LogDnsClient>(std::move(dns_client), net_log,
+                                               kMaxConcurrentDnsQueries);
+
+  for (const auto& log : ct_logs) {
+    tree_trackers_[log->key_id()].reset(
+        new SingleTreeTracker(log, dns_client_.get()));
+  }
 }
 
 TreeStateTracker::~TreeStateTracker() {}