Add histograms for DNS rcodes resulting from CT-over-DNS lookups

These histograms can be compared to "AsyncDNS.Rcode" to determine
whether there is an unusual distribution of rcodes relative to normal
DNS lookups (i.e. A/AAAA lookups). I'm interested in particular to see
what rcodes map to DNS_SERVER_FAILED (besides ServFail).

Bug: 506227
Change-Id: I5bbdc6c52ef7b017f0d38c1329f450ed346f6e4a
Reviewed-on: https://chromium-review.googlesource.com/1152972
Commit-Queue: Rob Percival <robpercival@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581419}

components/certificate_transparency/log_dns_client.cc

@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -43,6 +44,20 @@ void LogQueryDuration(net::Error error, const base::TimeDelta& duration) {
   }
 }
 
+void LogQueryResult(const std::string& name,
+                    net::Error error,
+                    const net::DnsResponse* response) {
+  base::UmaHistogramSparse(
+      base::StrCat({"Net.CertificateTransparency.DnsQuery", name, "Error"}),
+      -error);
+
+  if (response) {
+    base::UmaHistogramSparse(
+        base::StrCat({"Net.CertificateTransparency.DnsQuery", name, "Rcode"}),
+        response->rcode());
+  }
+}
+
 // Returns an EDNS option that disables the client subnet extension, as
 // described in https://tools.ietf.org/html/rfc7871. This is to avoid the
 // privacy issues caused by this extension being enabled in recursive resolvers
@@ -250,6 +265,7 @@ AuditProofQueryImpl::AuditProofQueryImpl(net::DnsClient* dns_client,
     : next_state_(State::NONE),
       domain_for_log_(domain_for_log),
       dns_client_(dns_client),
+      last_dns_response_(nullptr),
       net_log_(net_log),
       weak_ptr_factory_(this) {
   DCHECK(dns_client_);
@@ -297,10 +313,8 @@ net::Error AuditProofQueryImpl::DoLoop(net::Error result) {
         break;
       case State::REQUEST_LEAF_INDEX_COMPLETE:
         result = RequestLeafIndexComplete(result);
-        if (result == net::OK) {
-          base::UmaHistogramSparse(
-              "Net.CertificateTransparency.DnsQueryLeafIndexError", net::OK);
-        }
+        if (result == net::OK)
+          LogQueryResult("LeafIndex", net::OK, last_dns_response_);
         break;
       case State::REQUEST_AUDIT_PROOF_NODES:
         result = RequestAuditProofNodes();
@@ -321,14 +335,12 @@ net::Error AuditProofQueryImpl::DoLoop(net::Error result) {
       case State::REQUEST_LEAF_INDEX:
       case State::REQUEST_LEAF_INDEX_COMPLETE:
         // An error must have occurred if the query completed in this state.
-        base::UmaHistogramSparse(
-            "Net.CertificateTransparency.DnsQueryLeafIndexError", -result);
+        LogQueryResult("LeafIndex", result, last_dns_response_);
         break;
       case State::REQUEST_AUDIT_PROOF_NODES:
       case State::REQUEST_AUDIT_PROOF_NODES_COMPLETE:
         // The query may have completed successfully.
-        base::UmaHistogramSparse(
-            "Net.CertificateTransparency.DnsQueryAuditProofError", -result);
+        LogQueryResult("AuditProof", result, last_dns_response_);
         break;
       case State::NONE:
         NOTREACHED();
@@ -453,6 +465,7 @@ bool AuditProofQueryImpl::StartDnsTransaction(const std::string& qname) {
     return false;
   }
 
+  last_dns_response_ = nullptr;
   current_dns_transaction_ = factory->CreateTransaction(
       qname, net::dns_protocol::kTypeTXT,
       base::BindOnce(&AuditProofQueryImpl::OnDnsTransactionComplete,

tools/metrics/histograms/histograms.xml

@@ -49649,6 +49649,17 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Net.CertificateTransparency.DnsQueryAuditProofRcode"
+    enum="AsyncDNSRcode">
+  <owner>robpercival@chromium.org</owner>
+  <summary>
+    Counts of specific DNS response codes returned by LogDnsClient at the end of
+    an attempt to obtain an inclusion proof for a certificate from a Certificate
+    Transparency log. The response codes (rcodes) and meanings are listed on
+    https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6.
+  </summary>
+</histogram>
+
 <histogram name="Net.CertificateTransparency.DnsQueryDuration" units="ms">
   <owner>robpercival@chromium.org</owner>
   <summary>
@@ -49678,6 +49689,17 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Net.CertificateTransparency.DnsQueryLeafIndexRcode"
+    enum="AsyncDNSRcode">
+  <owner>robpercival@chromium.org</owner>
+  <summary>
+    Counts of specific DNS response codes returned by LogDnsClient at the end of
+    an attempt to obtain a leaf index for a certificate from a Certificate
+    Transparency log. The response codes (rcodes) and meanings are listed on
+    https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6.
+  </summary>
+</histogram>
+
 <histogram name="Net.CertificateTransparency.DnsQueryStatus"
     enum="CertificateTransparencyDnsQueryStatus" expires_after="M67">
   <obsolete>