Include both certificate chains in invalid cert reporting

chrome/browser/net/chrome_fraudulent_certificate_reporter.cc

Index: chrome/browser/net/chrome_fraudulent_certificate_reporter.cc
diff --git a/chrome/browser/net/chrome_fraudulent_certificate_reporter.cc b/chrome/browser/net/chrome_fraudulent_certificate_reporter.cc
index 0e3a1d658533a4678f51c441ac05704cf2651547..24600a2e9a64af96bc2b666470dff0f2c5aaa02a 100644
--- a/chrome/browser/net/chrome_fraudulent_certificate_reporter.cc
+++ b/chrome/browser/net/chrome_fraudulent_certificate_reporter.cc
@@ -40,20 +40,33 @@ ChromeFraudulentCertificateReporter::~ChromeFraudulentCertificateReporter() {
   STLDeleteElements(&inflight_requests_);
 }
 
-static std::string BuildReport(const std::string& hostname,
-                               const net::SSLInfo& ssl_info) {
+// Helper function for |BuildReport|. Appends each PEM-encoded
+// certificate in the chain starting at |cert| to |chain_from_report|.
+static void AddCertChainToReport(const scoped_refptr<net::X509Certificate> cert,
+                                 std::string* chain_from_report) {
+  std::vector<std::string> pem_encoded_chain;
+  if (!cert || !cert->GetPEMEncodedChain(&pem_encoded_chain)) {

[felt, 2015/02/25 16:41:32]

is it possible and expected that this can be called with !cert? (maybe this can
happen for INVALID_CERT errors, but I am not sure; have you checked?) should
this be a DCHECK making sure that the cert has a value instead?

+    LOG(ERROR) << "Could not get PEM encoded chain.";

[felt, 2015/02/25 16:41:32]

should there also be a return here?

+  }
+  for (size_t i = 0; i < pem_encoded_chain.size(); ++i)
+    *chain_from_report += pem_encoded_chain[i];
+}
+
+std::string ChromeFraudulentCertificateReporter::BuildReport(
+    ChromeFraudulentCertificateReporter::ReportType type,
+    const std::string& hostname,
+    const net::SSLInfo& ssl_info) {
   CertLoggerRequest request;
   base::Time now = base::Time::Now();
   request.set_time_usec(now.ToInternalValue());
   request.set_hostname(hostname);
 
-  std::vector<std::string> pem_encoded_chain;
-  if (!ssl_info.cert->GetPEMEncodedChain(&pem_encoded_chain)) {
-    LOG(ERROR) << "Could not get PEM encoded chain.";
+  AddCertChainToReport(ssl_info.cert, request.mutable_cert_chain());
+  if (type ==
+      ChromeFraudulentCertificateReporter::REPORT_TYPE_EXTENDED_REPORTING) {
+    AddCertChainToReport(ssl_info.unverified_server_cert,
+                         request.mutable_unverified_server_cert_chain());
   }
-  std::string* cert_chain = request.mutable_cert_chain();
-  for (size_t i = 0; i < pem_encoded_chain.size(); ++i)
-    *cert_chain += pem_encoded_chain[i];
 
   request.add_pin(ssl_info.pinning_failure_log);
 
@@ -83,7 +96,7 @@ void ChromeFraudulentCertificateReporter::SendReport(
     // TODO(estark): Temporarily, since there is no upload endpoint, just log
     // the information.
     LOG(ERROR) << "SSL report for " << hostname << ":\n"
-               << BuildReport(hostname, ssl_info) << "\n\n";
+               << BuildReport(type, hostname, ssl_info) << "\n\n";
     return;
   }
 
@@ -93,7 +106,7 @@ void ChromeFraudulentCertificateReporter::SendReport(
     return;
   }
 
-  std::string report = BuildReport(hostname, ssl_info);
+  std::string report = BuildReport(type, hostname, ssl_info);
 
   scoped_ptr<net::URLRequest> url_request =
       CreateURLRequest(request_context_, pinning_violation_upload_url_);