Add checkbox for reporting invalid TLS/SSL cert chains

chrome/browser/ssl/ssl_blocking_page.cc

Index: chrome/browser/ssl/ssl_blocking_page.cc
diff --git a/chrome/browser/ssl/ssl_blocking_page.cc b/chrome/browser/ssl/ssl_blocking_page.cc
index ecf70291056c8b47e64f534c76b29a09aad97083..f77fc6540a1ab99c066c5f6ccb3b9dfa89eb54ea 100644
--- a/chrome/browser/ssl/ssl_blocking_page.cc
+++ b/chrome/browser/ssl/ssl_blocking_page.cc
@@ -10,6 +10,7 @@
 #include "base/i18n/time_formatting.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
+#include "base/prefs/pref_service.h"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
@@ -25,9 +26,11 @@
 #include "chrome/browser/ssl/ssl_error_classification.h"
 #include "chrome/browser/ssl/ssl_error_info.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/google/core/browser/google_util.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/cert_store.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
@@ -43,6 +46,9 @@
 #include "net/base/hash_value.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
+#include "net/url_request/fraudulent_certificate_reporter.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_WIN)
@@ -64,6 +70,7 @@
 
 using base::ASCIIToUTF16;
 using base::TimeTicks;
+using content::BrowserThread;
 using content::InterstitialPage;
 using content::NavigationController;
 using content::NavigationEntry;
@@ -209,6 +216,24 @@ bool IsErrorDueToBadClock(const base::Time& now, int error) {
       SSLErrorClassification::IsUserClockInTheFuture(now);
 }
 
+// A helper function that actually sends the cert collection report over
+// the network.
+void FinishCertCollectionInternal(
+    const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
+    const std::string& hostname,
+    const net::SSLInfo& ssl_info) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  net::FraudulentCertificateReporter* reporter =
+      request_context_getter->GetURLRequestContext()
+          ->fraudulent_certificate_reporter();
+  if (reporter) {
+    reporter->SendReport(
+        net::FraudulentCertificateReporter::REPORT_TYPE_EXTENDED_REPORTING,
+        hostname, ssl_info);
+  }
+}
+
 }  // namespace
 
 // static
@@ -430,6 +455,34 @@ void SSLBlockingPage::PopulateInterstitialStrings(
   std::vector<std::string> encoded_chain;
   ssl_info_.cert->GetPEMEncodedChain(&encoded_chain);
   load_time_data->SetString("pem", JoinString(encoded_chain, std::string()));
+
+  PopulateExtendedReportingOption(load_time_data);
+}
+
+void SSLBlockingPage::PopulateExtendedReportingOption(
+    base::DictionaryValue* load_time_data) {
+  // Only show the checkbox if not off-the-record and if the
+  // command-line option is set.
+  const bool show = !web_contents()->GetBrowserContext()->IsOffTheRecord() &&
+                    base::CommandLine::ForCurrentProcess()->HasSwitch(
+                        switches::kEnableInvalidCertCollection);
+
+  load_time_data->SetBoolean(interstitials::kDisplayCheckBox, show);
+  if (!show)
+    return;
+
+  load_time_data->SetBoolean(
+      interstitials::kBoxChecked,
+      IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingEnabled));
+
+  const std::string privacy_link = base::StringPrintf(
+      interstitials::kPrivacyLinkHtml,
+      l10n_util::GetStringUTF8(IDS_SAFE_BROWSING_PRIVACY_POLICY_PAGE).c_str());
+
+  load_time_data->SetString(
+      "optInLink",
+      l10n_util::GetStringFUTF16(IDS_SAFE_BROWSING_MALWARE_REPORTING_AGREE,
+                                 base::UTF8ToUTF16(privacy_link)));
 }
 
 void SSLBlockingPage::OverrideEntry(NavigationEntry* entry) {
@@ -462,6 +515,14 @@ void SSLBlockingPage::CommandReceived(const std::string& command) {
       }
       break;
     }
+    case CMD_DO_REPORT: {
+      SetReportingPreference(true);
+      break;
+    }
+    case CMD_DONT_REPORT: {
+      SetReportingPreference(false);
+      break;
+    }
     case CMD_MORE: {
       metrics_helper_->RecordUserInteraction(
           SecurityInterstitialMetricsHelper::SHOW_ADVANCED);
@@ -506,6 +567,11 @@ void SSLBlockingPage::OverrideRendererPrefs(
 void SSLBlockingPage::OnProceed() {
   metrics_helper_->RecordUserDecision(
       SecurityInterstitialMetricsHelper::PROCEED);
+
+  // Finish collection information about invalid certificates, if the
+  // user opted in to.
+  FinishCertCollection();
+
   RecordSSLExpirationPageEventState(
       expired_but_previously_allowed_, true, overridable_);
   // Accepting the certificate resumes the loading of the page.
@@ -515,6 +581,11 @@ void SSLBlockingPage::OnProceed() {
 void SSLBlockingPage::OnDontProceed() {
   metrics_helper_->RecordUserDecision(
       SecurityInterstitialMetricsHelper::DONT_PROCEED);
+
+  // Finish collection information about invalid certificates, if the
+  // user opted in to.
+  FinishCertCollection();
+
   RecordSSLExpirationPageEventState(
       expired_but_previously_allowed_, false, overridable_);
   NotifyDenyCertificate();
@@ -562,6 +633,25 @@ std::string SSLBlockingPage::GetSamplingEventName() const {
   return event_name;
 }
 
+void SSLBlockingPage::FinishCertCollection() {
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableInvalidCertCollection))
+    return;
+
+  const bool enabled =
+      IsPrefEnabled(prefs::kSafeBrowsingExtendedReportingEnabled);
+  UMA_HISTOGRAM_BOOLEAN("SB2.ExtendedReportingIsEnabled", enabled);

[estark, 2015/02/28 21:03:11]

I'm not quite sure what this is measuring -- should we be not running this line
when in incognito?

[felt, 2015/03/02 16:06:58]

That's meant to measure how many interstitials had a report done. So you're
right with  your guess, this should be moved inside the if-statement below.

[estark, 2015/03/02 16:26:10]

Ok! In that case I just moved the off-the-record check to the top of the
function which I think is a clearer place for it anyway.

+
+  if (enabled && !web_contents()->GetBrowserContext()->IsOffTheRecord()) {
+    scoped_refptr<net::URLRequestContextGetter> request_context_getter =
+        web_contents()->GetBrowserContext()->GetRequestContext();
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(FinishCertCollectionInternal, request_context_getter,
+                   request_url().host(), ssl_info_));
+  }
+}
+
 // static
 bool SSLBlockingPage::IsOptionsOverridable(int options_mask) {
   return (options_mask & SSLBlockingPage::OVERRIDABLE) &&