Split cert reporter class into report building/serializing and sending

chrome/browser/net/certificate_error_reporter.cc

Index: chrome/browser/net/certificate_error_reporter.cc
diff --git a/chrome/browser/net/certificate_error_reporter.cc b/chrome/browser/net/certificate_error_reporter.cc
index edff489f4ca0762dac6eb03bf554a1bc7e0adef2..32082711430bf2558f6a44c13ad524b61e8f3f04 100644
--- a/chrome/browser/net/certificate_error_reporter.cc
+++ b/chrome/browser/net/certificate_error_reporter.cc
@@ -7,9 +7,7 @@
 #include <set>
 
 #include "base/logging.h"
-#include "base/stl_util.h"
-#include "base/time/time.h"
-#include "chrome/browser/net/cert_logger.pb.h"
+#include "chrome/browser/net/encrypted_cert_logger.pb.h"
 
 #if defined(USE_OPENSSL)
 #include "crypto/aead_openssl.h"
@@ -22,8 +20,6 @@
 #include "net/base/load_flags.h"
 #include "net/base/request_priority.h"
 #include "net/base/upload_bytes_element_reader.h"
-#include "net/cert/x509_certificate.h"
-#include "net/ssl/ssl_info.h"
 #include "net/url_request/url_request_context.h"
 
 namespace {
@@ -114,25 +110,20 @@ CertificateErrorReporter::~CertificateErrorReporter() {
   STLDeleteElements(&inflight_requests_);
 }
 
-void CertificateErrorReporter::SendReport(ReportType type,
-                                          const std::string& hostname,
-                                          const net::SSLInfo& ssl_info) {
-  CertLoggerRequest request;
-  BuildReport(hostname, ssl_info, &request);
-
+void CertificateErrorReporter::SendReport(
+    ReportType type,
+    const std::string& serialized_report) {
   switch (type) {
     case REPORT_TYPE_PINNING_VIOLATION:
-      SendCertLoggerRequest(request);
+      SendSerializedRequest(serialized_report);
       break;
     case REPORT_TYPE_EXTENDED_REPORTING:
       if (upload_url_.SchemeIsCryptographic()) {

[eroman, 2015/05/12 00:27:51]

Is this uploading reports to Google? under what circumstances would the
upload_url_ not be https? (Not directly related to your refactor, but still
curious)

[estark, 2015/05/12 20:42:15]

Eventually, all uploads will be over HTTP, not HTTPS. Our goal is to collect
information about SSL warnings that users are seeing, and we very much want to
include data from users who might not be able to make any valid HTTPS
connections at all. At the moment we'll be sending reports (encrypted) over HTTP
on platforms where we have BoringSSL crypto, and we're sending them over HTTPS
on platforms where we don't yet have BoringSSL crypto.

(And yes, all the reports are being sent to Google servers.)

-        SendCertLoggerRequest(request);
+        SendSerializedRequest(serialized_report);
       } else {
         DCHECK(IsHttpUploadUrlSupported());
 #if defined(USE_OPENSSL)
         EncryptedCertLoggerRequest encrypted_report;
-        std::string serialized_report;
-        request.SerializeToString(&serialized_report);
         if (!EncryptSerializedReport(server_public_key_,
                                      server_public_key_version_,
                                      serialized_report, &encrypted_report)) {
@@ -191,7 +182,7 @@ bool CertificateErrorReporter::IsHttpUploadUrlSupported() {
 bool CertificateErrorReporter::DecryptCertificateErrorReport(
     const uint8 server_private_key[32],
     const EncryptedCertLoggerRequest& encrypted_report,
-    CertLoggerRequest* decrypted_report) {
+    std::string* decrypted_serialized_report) {
   uint8 shared_secret[crypto::curve25519::kBytes];
   crypto::curve25519::ScalarMult(
       server_private_key, (uint8*)encrypted_report.client_public_key().data(),
@@ -208,23 +199,11 @@ bool CertificateErrorReporter::DecryptCertificateErrorReport(
   // Use an all-zero nonce because the key is random per-message.
   std::string nonce(aead.NonceLength(), 0);
 
-  std::string plaintext;
-  if (!aead.Open(encrypted_report.encrypted_report(), nonce, "", &plaintext)) {
-    LOG(ERROR) << "Error opening certificate report";
-    return false;
-  }
-
-  return decrypted_report->ParseFromString(plaintext);
+  return aead.Open(encrypted_report.encrypted_report(), nonce, "",
+                   decrypted_serialized_report);
 }
 #endif
 
-void CertificateErrorReporter::SendCertLoggerRequest(
-    const CertLoggerRequest& request) {
-  std::string serialized_request;
-  request.SerializeToString(&serialized_request);
-  SendSerializedRequest(serialized_request);
-}
-
 void CertificateErrorReporter::SendSerializedRequest(
     const std::string& serialized_request) {
   scoped_ptr<net::URLRequest> url_request = CreateURLRequest(request_context_);
@@ -245,24 +224,6 @@ void CertificateErrorReporter::SendSerializedRequest(
   raw_url_request->Start();
 }
 
-void CertificateErrorReporter::BuildReport(const std::string& hostname,
-                                           const net::SSLInfo& ssl_info,
-                                           CertLoggerRequest* out_request) {
-  base::Time now = base::Time::Now();
-  out_request->set_time_usec(now.ToInternalValue());
-  out_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.";
-
-  std::string* cert_chain = out_request->mutable_cert_chain();
-  for (size_t i = 0; i < pem_encoded_chain.size(); ++i)
-    *cert_chain += pem_encoded_chain[i];
-
-  out_request->add_pin(ssl_info.pinning_failure_log);
-}
-
 void CertificateErrorReporter::RequestComplete(net::URLRequest* request) {
   std::set<net::URLRequest*>::iterator i = inflight_requests_.find(request);
   DCHECK(i != inflight_requests_.end());