net: add DNS certificate revocation experiment.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 871f99eb734c9258e7a9898c2c0dcf34379ccf81..8d0010a03232750639ef0c250d65b5a99176dac2 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -4,6 +4,7 @@
 
 #include "net/url_request/url_request_http_job.h"
 
+#include "base/base64.h"
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -18,6 +19,8 @@
 #include "base/string_util.h"
 #include "base/time.h"
 #include "net/base/cert_status_flags.h"
+#include "net/base/dns_util.h"
+#include "net/base/dnsrr_resolver.h"
 #include "net/base/filter.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
@@ -25,6 +28,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/network_delegate.h"
+#include "net/base/public_key_hashes.h"
 #include "net/base/sdch_manager.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/base/ssl_config_service.h"
@@ -48,6 +52,68 @@ static const char kAvailDictionaryHeader[] = "Avail-Dictionary";
 
 namespace net {
 
+namespace {
+
+const char* const kComodoCerts[] = {
+  kSPKIHash_AAACertificateServices,
+  kSPKIHash_AddTrustClass1CARoot,
+  kSPKIHash_AddTrustExternalCARoot,
+  kSPKIHash_AddTrustPublicCARoot,
+  kSPKIHash_AddTrustQualifiedCARoot,
+  kSPKIHash_COMODOCertificationAuthority,
+  kSPKIHash_SecureCertificateServices,
+  kSPKIHash_TrustedCertificateServices,
+  kSPKIHash_UTNDATACorpSGC,
+  kSPKIHash_UTNUSERFirstClientAuthenticationandEmail,
+  kSPKIHash_UTNUSERFirstHardware,
+  kSPKIHash_UTNUSERFirstObject,
+};
+
+// IsComodoCertificate returns true if a known Comodo public key appears in
+// |public_key_hashes|.
+// TODO(agl): remove once experiment is complete, by July 2012.
+static bool IsComodoCertificate(
+    const std::vector<SHA1Fingerprint>& public_key_hashes) {
+  for (std::vector<SHA1Fingerprint>::const_iterator
+       i = public_key_hashes.begin(); i != public_key_hashes.end(); i++) {
+    std::string base64_hash;
+    base::Base64Encode(base::StringPiece(
+        reinterpret_cast<const char*>(i->data), sizeof(i->data)), &base64_hash);

[Ryan Sleevi, 2012/03/19 18:58:15]

nit: Use arraysize() rather than sizeof() - functionally equivalent, but
arraysize() will complain more if SHA1Fingerprint changes, so it's easier to
catch.

[agl, 2012/03/19 20:16:48]

Done.

+    base64_hash = "sha1/" + base64_hash;
+
+    for (size_t j = 0; j < arraysize(kComodoCerts); j++) {
+      if (base64_hash == kComodoCerts[j])
+        return true;
+    }
+  }
+
+  return false;
+}
+
+// RecordComodoDNSResult is a callback from a DNS resolution. It records the
+// elapsed time in a histogram.
+// TODO(agl): remove once experiment is complete, by July 2012.
+static void RecordComodoDNSResult(RRResponse* response,
+                                  base::TimeTicks start_time,
+                                  int result) {
+  base::TimeDelta total_time = base::TimeTicks::Now() - start_time;
+  if (total_time.InMilliseconds() > 10) {
+    // If the reply took > 10 ms there's a good chance that it didn't come from
+    // a local DNS cache.
+    if (result == OK &&
+        response->rrdatas.size() &&
+        response->rrdatas[0].find("wibble") != std::string::npos) {
+      UMA_HISTOGRAM_TIMES("Net.ComodoDNSExperimentSuccessTime", total_time);
+    } else {
+      UMA_HISTOGRAM_TIMES("Net.ComodoDNSExperimentFailureTime", total_time);
+    }
+  }
+
+  delete response;
+}
+
+}  // namespace
+
 class URLRequestHttpJob::HttpFilterContext : public FilterContext {
  public:
   explicit HttpFilterContext(URLRequestHttpJob* job);
@@ -690,6 +756,29 @@ void URLRequestHttpJob::OnStartCompleted(int result) {
     }
   }
 
+  // This is a temporary experiment, in conjuction with Comodo, to measure the
+  // effectiveness of a possible DNS-based revocation mechanism.
+  // TODO(agl): remove once experiment is complete, by July 2012.

[Ryan Sleevi, 2012/03/19 18:58:15]

Can this be implemented as a FieldTrial (base/metrics/FieldTrial.h), if
necessarily plumbed through the SSLConfigService as a static variable (as we
previously did DNS RR resolver trials and currently do the cached info
extension)?

The benefit here is that you can set a firm expiration in the event someone
falls off the update bandwagon.

[agl, 2012/03/19 20:16:48]

Yes, I like the idea of a date-based cutoff if all else fails. Have gated it on
a FieldTrial.

+  if (result == OK && transaction_->GetResponseInfo() != NULL) {

[willchan no longer on Chromium, 2012/03/19 19:11:01]

Can we move all this code into ChromeNetworkDelegate::OnResponseStarted()? I
know it's at a later point, so we lose some data points, but I suspect you don't
care. The reason to do it there is to keep net/ clean and also prevent this
behavior leaking to a bunch of our other consumers. This code is actually used
quite widely in contexts that you wouldn't necessarily expect.
ChromeNetworkDelegate makes it so that it's only used for normal Chrome
browsing.

That way we don't need to create a DnsRRResolver for each URLRequestContext
either, we can just stash it into ChromeNetworkDelegate.

[agl, 2012/03/19 20:16:48]

Done.

+    const HttpResponseInfo* response_info = transaction_->GetResponseInfo();
+    const SSLInfo& ssl_info = response_info->ssl_info;
+    if (response_info->was_cached == false &&
+        ssl_info.is_valid() &&
+        ssl_info.is_issued_by_known_root &&
+        IsComodoCertificate(ssl_info.public_key_hashes)) {
+      // The Comodo DNS record has a 20 minute TTL, so we won't actually

[willchan no longer on Chromium, 2012/03/19 19:11:01]

What happens if Comodo screws up their TTL? Do we have throttling at some other
level? If not, then I'm not OK with this behavior moving beyond canary.

[agl, 2012/03/19 20:16:48]

Have added a rate limit of once every ten minutes.

+      // request it more than three times an hour because it'll be in the
+      // DnsRRResolver's cache.
+      DnsRRResolver* dnsrr_resolver(context_->dnsrr_resolver());
+      RRResponse* response(new(RRResponse));

[Ryan Sleevi, 2012/03/19 18:58:15]

drive by: 
RRResponse* response = new RRResponse;

Alternatively, make it clear you're not doing placement new with:
RRResponse* response(new RRResponse);

However, this syntax is less common for naked pointers - it's more commonly used
for scoped_ptr<> construction. Naked pointers historically seem to fit "Foo foo*
= new Foo"

[agl, 2012/03/19 20:16:48]

Yep, that's better. Thanks.

+      base::TimeTicks start_time(base::TimeTicks::Now());
+      dnsrr_resolver->Resolve("wibble.comodoca.com", kDNS_TXT, 0 /* flags */,
+                              Bind(RecordComodoDNSResult, response, start_time),
+                              response, 0 /* priority */,

[willchan no longer on Chromium, 2012/03/19 19:11:01]

how about using base::Owned(response) so that on shutdown we don't leak?
valgrind shutdown leaks are annoying.

[agl, 2012/03/19 20:16:48]

Done.

+                              request_->net_log());
+    }
+  }
+
   if (result == OK) {
     scoped_refptr<HttpResponseHeaders> headers = GetResponseHeaders();
     if (request_->context() && request_->context()->network_delegate()) {