Add initial version of captive portal list checking.

chrome/browser/ssl/ssl_error_handler.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ssl/ssl_error_handler.h"
 
 #include <stdint.h>
+#include <unordered_set>
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/feature_list.h"
+#include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/non_thread_safe.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ssl/bad_clock_blocking_page.h"
 #include "chrome/browser/ssl/ssl_blocking_page.h"
 #include "chrome/browser/ssl/ssl_cert_reporter.h"
+#include "chrome/browser/ssl/tls_error_assistant.pb.h"
 #include "chrome/common/features.h"
+#include "chrome/grit/browser_resources.h"
 #include "components/network_time/network_time_tracker.h"
 #include "components/ssl_errors/error_classification.h"
 #include "components/ssl_errors/error_info.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/net_errors.h"
+#include "ui/base/resource/resource_bundle.h"
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
 #include "chrome/browser/captive_portal/captive_portal_service.h"
 #include "chrome/browser/captive_portal/captive_portal_service_factory.h"
 #include "chrome/browser/captive_portal/captive_portal_tab_helper.h"
 #include "chrome/browser/ssl/captive_portal_blocking_page.h"
 #endif
 
 namespace {
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
 const base::Feature kCaptivePortalInterstitial{
     "CaptivePortalInterstitial", base::FEATURE_ENABLED_BY_DEFAULT};
 #endif
 
 const base::Feature kSSLCommonNameMismatchHandling{
     "SSLCommonNameMismatchHandling", base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Default delay in milliseconds before displaying the SSL interstitial.
 // This can be changed in tests.
 // - If there is a name mismatch and a suggested URL available result arrives
 //   during this time, the user is redirected to the suggester URL.
 // - If a "captive portal detected" result arrives during this time,
 //   a captive portal interstitial is displayed.
 // - Otherwise, an SSL interstitial is displayed.
 const int64_t kInterstitialDelayInMilliseconds = 3000;
 
-// Events for UMA.
-enum SSLErrorHandlerEvent {
-  HANDLE_ALL,
-  SHOW_CAPTIVE_PORTAL_INTERSTITIAL_NONOVERRIDABLE,
-  SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE,
-  SHOW_SSL_INTERSTITIAL_NONOVERRIDABLE,
-  SHOW_SSL_INTERSTITIAL_OVERRIDABLE,
-  WWW_MISMATCH_FOUND,
-  WWW_MISMATCH_URL_AVAILABLE,
-  WWW_MISMATCH_URL_NOT_AVAILABLE,
-  SHOW_BAD_CLOCK,
-  SSL_ERROR_HANDLER_EVENT_COUNT
-};
+const char kHistogram[] = "interstitial.ssl_error_handler";
 
 // Adds a message to console after navigation commits and then, deletes itself.
 // Also deletes itself if the navigation is stopped.
 class CommonNameMismatchRedirectObserver
     : public content::WebContentsObserver,
       public content::WebContentsUserData<CommonNameMismatchRedirectObserver> {
  public:
   static void AddToConsoleAfterNavigation(
       content::WebContents* web_contents,
       const std::string& request_url_hostname,
@@ skipping 38 matching lines @@
     web_contents_->RemoveUserData(UserDataKey());
   }
 
   content::WebContents* web_contents_;
   const std::string request_url_hostname_;
   const std::string suggested_url_hostname_;
 
   DISALLOW_COPY_AND_ASSIGN(CommonNameMismatchRedirectObserver);
 };
 
-void RecordUMA(SSLErrorHandlerEvent event) {
-  UMA_HISTOGRAM_ENUMERATION("interstitial.ssl_error_handler", event,
-                            SSL_ERROR_HANDLER_EVENT_COUNT);
+void RecordUMA(SSLErrorHandler::UMAEvent event) {
+  UMA_HISTOGRAM_ENUMERATION(kHistogram, event,
+                            SSLErrorHandler::SSL_ERROR_HANDLER_EVENT_COUNT);
 }
 
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
 bool IsCaptivePortalInterstitialEnabled() {
   return base::FeatureList::IsEnabled(kCaptivePortalInterstitial);
 }
 #endif
 
 bool IsSSLCommonNameMismatchHandlingEnabled() {
   return base::FeatureList::IsEnabled(kSSLCommonNameMismatchHandling);
 }
 
+// Reads the TLS error assistant configuration from the resource bundle.
+void ReadErrorAssistantProtoFromResourceBundle(std::string* binary_pb) {
+  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  bundle.GetRawDataResource(IDR_TLS_ERROR_ASSISTANT_PB).CopyToString(binary_pb);
+}
+
+std::unique_ptr<std::unordered_set<std::string>> LoadCaptivePortalCertHashes(
+    const chrome_browser_ssl::TLSErrorAssistantConfig& proto) {
+  std::unique_ptr<std::unordered_set<std::string>> hashes(
+      new std::unordered_set<std::string>());
+  for (const chrome_browser_ssl::CaptivePortalCert& cert :
+       proto.captive_portal_cert()) {
+    hashes.get()->insert(cert.sha256_hash());
+  }
+  return hashes;
+}
+
 // Configuration for SSLErrorHandler.
 class ConfigSingleton : public base::NonThreadSafe {
  public:
   ConfigSingleton();
 
   base::TimeDelta interstitial_delay() const;
   SSLErrorHandler::TimerStartedCallback* timer_started_callback() const;
   base::Clock* clock() const;
   network_time::NetworkTimeTracker* network_time_tracker() const;
 
+  // Returns true if one of the cert hashes in |ssl_info| is of a captive portal

[estark, 2017/01/12 18:54:43]

Haven't read the implementation yet, but this makes it sound like we check every
cert in the chain against the captive portal list.

Is that intentional? I'm wondering if it's safer and equally effective to just
check the leaf cert. (In case we somehow accidentally were to end up with an
intermediate or root on the captive portal list.)

....


or after reading a little below, maybe this means that every different hash
algorithm of the leaf cert is checked in the captive portal list. So, plz
clarify?

[meacer, 2017/01/12 23:54:03]

It's intentional for this draft in the sense that I wanted to make sure leaf is
the first or the last cert in the chain. If we can guarantee, we should just
check the leaf.

Though I wonder if there are any intermediates that we can also mark as captive
portal (haven't seen any so far)
Yes, it is checking both hashes. This is another thing I wanted to clarify, do
we always have both hashes? If so, we can always check the sha256 version.

Also note that it's checking the serialized hashes via ToString(), rather than
the actual hash. The reason is code simplicity: Since the hashes are put in a
set, we'll have to pull in comparison operators for net::HashValue if we use the
actual hashes. Let me know if you prefer using std::unorderd_set<net::HashValue>
instead.

[estark, 2017/01/13 23:40:23]

Hmmm. From my reading of ssl_info.h, I would guess we can't assume anything
about the order of the hashes or what algorithms are present. Nor do I see any
nicely exposed way to calculate an SPKI hash. So I guess what you're doing is
the best option, we should just make sure to only add leaf certs to the captive
portal list. (Is there somewhere we can document that?)

(I think comparing the strings is fine.)

[meacer, 2017/01/20 21:29:59]

Added a note to tls_error_assistant.proto. In the future, we can also do some
validation while generating the binary version of that proto, but that will be
complicated (e.g. will need to add the full PEM).

+  // certificate. The set of captive portal hashes is loaded on first use.
+  bool IsKnownCaptivePortalCert(const net::SSLInfo& ssl_info);
+
+  // Testing methods:
   void SetInterstitialDelayForTesting(const base::TimeDelta& delay);
   void SetTimerStartedCallbackForTesting(
       SSLErrorHandler::TimerStartedCallback* callback);
   void SetClockForTesting(base::Clock* clock);
   void SetNetworkTimeTrackerForTesting(
       network_time::NetworkTimeTracker* tracker);
+  void SetErrorAssistantProtoForTesting(
+      const chrome_browser_ssl::TLSErrorAssistantConfig& error_assistant_proto);
 
  private:
   base::TimeDelta interstitial_delay_;
 
   // Callback to call when the interstitial timer is started. Used for
   // testing.
   SSLErrorHandler::TimerStartedCallback* timer_started_callback_ = nullptr;
 
   // The clock to use when deciding which error type to display. Used for
   // testing.
   base::Clock* testing_clock_ = nullptr;
 
   network_time::NetworkTimeTracker* network_time_tracker_ = nullptr;
+
+  // SPKI hashes belonging to certs treated as captive portal portals. Populated

[estark, 2017/01/12 18:54:43]

nit: extra "portal"

[meacer, 2017/01/20 21:29:59]

Done.

+  // the first time IsKnownCaptivePortalCert() or
+  // SetErrorAssistantProtoForTesting() is called.
+  std::unique_ptr<std::unordered_set<std::string>> captive_portal_spki_hashes_;
 };
 
 ConfigSingleton::ConfigSingleton()
     : interstitial_delay_(
           base::TimeDelta::FromMilliseconds(kInterstitialDelayInMilliseconds)) {
 }
 
 base::TimeDelta ConfigSingleton::interstitial_delay() const {
   return interstitial_delay_;
 }
@@ skipping 26 matching lines @@
 
 void ConfigSingleton::SetClockForTesting(base::Clock* clock) {
   testing_clock_ = clock;
 }
 
 void ConfigSingleton::SetNetworkTimeTrackerForTesting(
     network_time::NetworkTimeTracker* tracker) {
   network_time_tracker_ = tracker;
 }
 
+void ConfigSingleton::SetErrorAssistantProtoForTesting(
+    const chrome_browser_ssl::TLSErrorAssistantConfig& error_assistant_proto) {
+  DCHECK(!captive_portal_spki_hashes_);
+  captive_portal_spki_hashes_ =
+      LoadCaptivePortalCertHashes(error_assistant_proto);
+}
+
+bool ConfigSingleton::IsKnownCaptivePortalCert(const net::SSLInfo& ssl_info) {
+  if (!captive_portal_spki_hashes_) {
+    std::string binary_proto;
+    ReadErrorAssistantProtoFromResourceBundle(&binary_proto);
+    chrome_browser_ssl::TLSErrorAssistantConfig proto;
+    proto.ParseFromString(binary_proto);
+    captive_portal_spki_hashes_ = LoadCaptivePortalCertHashes(proto);
+  }
+
+  for (const net::HashValue& hash_value : ssl_info.public_key_hashes) {

[estark, 2017/01/12 18:54:43]

Ok, now that I got down here, it looks like we're comparing the hashes of
several algorithms for each cert in the chain. So I think the comment above
should be clarified (line 166), and also want to make sure you're intending to
check every cert in the chain. Is this mostly for convenience, so that we don't
have to calculate the SPKI hashes of the leaf cert here?

[meacer, 2017/01/12 23:54:03]

Can we not assume the first or the last two of the hashes belong to the leaf? If
yes I don't think we need to re-calculate the hashes, right? 

[meacer, 2017/01/20 21:29:59]

Added sha256 check and clarified comment in line 166.

+    if (captive_portal_spki_hashes_->find(hash_value.ToString()) !=
+        captive_portal_spki_hashes_->end()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 static base::LazyInstance<ConfigSingleton>::Leaky g_config =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(SSLErrorHandler);
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(CommonNameMismatchRedirectObserver);
 
 void SSLErrorHandler::HandleSSLError(
     content::WebContents* web_contents,
@@ skipping 28 matching lines @@
 void SSLErrorHandler::SetClockForTesting(base::Clock* testing_clock) {
   g_config.Pointer()->SetClockForTesting(testing_clock);
 }
 
 // static
 void SSLErrorHandler::SetNetworkTimeTrackerForTesting(
     network_time::NetworkTimeTracker* tracker) {
   g_config.Pointer()->SetNetworkTimeTrackerForTesting(tracker);
 }
 
+// static
+void SSLErrorHandler::SetErrorAssistantProtoForTesting(
+    const chrome_browser_ssl::TLSErrorAssistantConfig& config_proto) {
+  g_config.Pointer()->SetErrorAssistantProtoForTesting(config_proto);
+}
+
+// static
+std::string SSLErrorHandler::GetHistogramNameForTesting() {

[estark, 2017/01/12 18:54:43]

Ah, this is a good idea, I always end up repeating the histogram name literal in
the tests...

[meacer, 2017/01/12 23:54:03]

It always works until it doesn't.

+  return kHistogram;
+}
+
 SSLErrorHandler::SSLErrorHandler(
     content::WebContents* web_contents,
     int cert_error,
     const net::SSLInfo& ssl_info,
     const GURL& request_url,
     int options_mask,
     std::unique_ptr<SSLCertReporter> ssl_cert_reporter,
     const base::Callback<void(content::CertificateRequestResultType)>& callback)
     : content::WebContentsObserver(web_contents),
       web_contents_(web_contents),
@@ skipping 11 matching lines @@
 
 void SSLErrorHandler::StartHandlingError() {
   RecordUMA(HANDLE_ALL);
 
   if (ssl_errors::ErrorInfo::NetErrorToErrorType(cert_error_) ==
       ssl_errors::ErrorInfo::CERT_DATE_INVALID) {
     HandleCertDateInvalidError();
     return;
   }
 
+  if (cert_error_ == net::ERR_CERT_COMMON_NAME_INVALID &&
+      g_config.Pointer()->IsKnownCaptivePortalCert(ssl_info_)) {

[estark, 2017/01/12 18:54:43]

Maybe this should be Finchable?

It would be good to have a Finch feature as a kill switch, plus if we roll it
out 50/50 we could see via UMA the difference in captive portal interstitials
shown for users who are checking the captive portal list and those who aren't.

[meacer, 2017/01/12 23:54:03]

I was hoping to avoid another finch for this file, but being able to experiment
sounds like a good idea. Will do this.

+    RecordUMA(CAPTIVE_PORTAL_CERT_FOUND);
+    ShowCaptivePortalInterstitial(GURL());

[estark, 2017/01/12 18:54:43]

How will this work without an empty landing URL? Does that mean the Connect
button (or whatever it's called) won't go anywhere?

[meacer, 2017/01/12 23:54:03]

Empty landing URL takes the user to the captive portal ping URL
(www.gstatic.com/generate_204). If the captive portal is a proper one, it should
then show its own login page there.

[meacer, 2017/01/20 21:29:59]

Actually, this was buggy (the text was broken). I'm now setting the URL
explicitly.

+    return;
+  }
+
   std::vector<std::string> dns_names;
   ssl_info_.cert->GetDNSNames(&dns_names);
   DCHECK(!dns_names.empty());
   GURL suggested_url;
   if (IsSSLCommonNameMismatchHandlingEnabled() &&
       cert_error_ == net::ERR_CERT_COMMON_NAME_INVALID &&
       IsErrorOverridable() && GetSuggestedUrl(dns_names, &suggested_url)) {
     RecordUMA(WWW_MISMATCH_FOUND);
     net::CertStatus extra_cert_errors =
         ssl_info_.cert_status ^ net::CERT_STATUS_COMMON_NAME_INVALID;
@@ skipping 228 matching lines @@
   network_time::NetworkTimeTracker* tracker =
       g_config.Pointer()->network_time_tracker();
   ssl_errors::ClockState clock_state = ssl_errors::GetClockState(now, tracker);
   if (clock_state == ssl_errors::CLOCK_STATE_FUTURE ||
       clock_state == ssl_errors::CLOCK_STATE_PAST) {
     ShowBadClockInterstitial(now, clock_state);
     return;  // |this| is deleted after showing the interstitial.
   }
   ShowSSLInterstitial();
 }