Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2679)

Unified Diff: chrome/browser/ssl/ssl_browser_tests.cc

Issue 2620203003: Add initial version of captive portal list checking. (Closed)
Patch Set: More tests and rebase Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ssl/ssl_browser_tests.cc
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 9bbf09a3a2d08454c0f0df78ea52cd223036300d..552c663774459a3f6aba3c607e408b0ba9cd05d5 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -18,6 +18,7 @@
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_clock.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_clock.h"
@@ -38,6 +39,7 @@
#include "chrome/browser/ssl/common_name_mismatch_handler.h"
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/ssl/ssl_blocking_page.h"
+#include "chrome/browser/ssl/ssl_error_assistant.pb.h"
#include "chrome/browser/ssl/ssl_error_handler.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_commands.h"
@@ -110,6 +112,10 @@
#include "net/cert/nss_cert_database.h"
#endif // defined(USE_NSS_CERTS)
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+#include "chrome/browser/ssl/captive_portal_blocking_page.h"
+#endif
+
using base::ASCIIToUTF16;
using chrome_browser_interstitials::SecurityInterstitialIDNTest;
using content::InterstitialPage;
@@ -124,6 +130,20 @@ const base::FilePath::CharType kDocRoot[] =
namespace {
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+// Sha256 fingerprint of okay.pem's Subject Public Key Information.
Ryan Sleevi 2017/02/01 22:06:06 s/okay.pem/ok_cert.pem/ Hardcoding to ok_cert.pem
meacer 2017/02/02 01:56:05 estark mentioned this too. I'm now computing this
+// Compute the hash as follows:
+// openssl x509 -noout -in net/data/ssl/certificates/ok_cert.pem -pubkey | \
+// openssl asn1parse -noout -inform pem -out public.key; \
+// openssl dgst -sha256 -binary public.key | openssl enc -base64
+const char kOkayPemSPKI[] =
+ "sha256/2zCMVDKgnKec0721Sp1zVh2yiHeW/LJK4STkNnEa1og=";
+
+// A hostname that will trigger a captive portal interstitial. The actual value
+// isn't important.
+const char kCaptivePortalHostname[] = "captive-portal.test";
+#endif
+
enum ProceedDecision {
SSL_INTERSTITIAL_PROCEED,
SSL_INTERSTITIAL_DO_NOT_PROCEED
@@ -226,12 +246,17 @@ class SSLInterstitialTimerObserver {
// Waits until the interstitial delay timer in SSLErrorHandler is started.
void WaitForTimerStarted() { message_loop_runner_->Run(); }
+ // Returns true if the interstitial delay timer has been started.
+ bool timer_started() const { return timer_started_; }
+
private:
void OnTimerStarted(content::WebContents* web_contents) {
+ timer_started_ = true;
if (web_contents_ == web_contents)
message_loop_runner_->Quit();
}
+ bool timer_started_ = false;
const content::WebContents* web_contents_;
SSLErrorHandler::TimerStartedCallback callback_;
@@ -308,6 +333,11 @@ class SSLUITest : public InProcessBrowserTest {
"https", "localhost", std::move(interceptor));
}
+ void SetUp() override {
+ InProcessBrowserTest::SetUp();
+ SSLErrorHandler::ResetConfigForTesting();
+ }
+
void SetUpCommandLine(base::CommandLine* command_line) override {
// Browser will both run and display insecure content.
command_line->AppendSwitch(switches::kAllowRunningInsecureContent);
@@ -3885,6 +3915,256 @@ IN_PROC_BROWSER_TEST_F(SSLUITestIgnoreLocalhostCertErrors,
ASSERT_TRUE(content::ExecuteScript(tab, "window.open()"));
}
+#if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+
+IN_PROC_BROWSER_TEST_F(SSLUITest, CaptivePortalCertificateList_Disabled) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitFromCommandLine(
+ "" /* enabled */, "CaptivePortalCertificateList" /* disabled */);
+
+ ASSERT_TRUE(https_server_mismatched_.Start());
+ base::HistogramTester histograms;
+
+ // Mark the server's cert as a captive portal cert.
+ chrome_browser_ssl::SSLErrorAssistantConfig config_proto;
+ config_proto.add_captive_portal_cert()->set_sha256_hash(kOkayPemSPKI);
+ SSLErrorHandler::SetErrorAssistantProtoForTesting(config_proto);
+
+ // Navigate to an unsafe page on the server. A normal SSL interstitial should
+ // be displayed since CaptivePortalCertificateList feature is disabled.
+ WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+ SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+ ui_test_utils::NavigateToURL(
+ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html"));
+ content::WaitForInterstitialAttach(tab);
+
+ InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+ ASSERT_EQ(SSLBlockingPage::kTypeForTesting,
+ interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+ EXPECT_TRUE(interstitial_timer_observer.timer_started());
+
+ // Check that the histogram for the SSL interstitial was recorded.
+ histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::HANDLE_ALL, 1);
+ histograms.ExpectBucketCount(
+ SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 0);
+}
+
+IN_PROC_BROWSER_TEST_F(SSLUITest,
+ CaptivePortalCertificateList_Enabled_FromProto) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitFromCommandLine(
+ "CaptivePortalCertificateList" /* enabled */, "" /* disabled */);
+
+ ASSERT_TRUE(https_server_mismatched_.Start());
+ base::HistogramTester histograms;
+
+ // Mark the server's cert as a captive portal cert.
+ chrome_browser_ssl::SSLErrorAssistantConfig config_proto;
+ config_proto.add_captive_portal_cert()->set_sha256_hash(kOkayPemSPKI);
+ SSLErrorHandler::SetErrorAssistantProtoForTesting(config_proto);
+
+ // Navigate to an unsafe page on the server. The captive portal interstitial
+ // should be displayed since CaptivePortalCertificateList feature is enabled.
+ WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+ SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+ ui_test_utils::NavigateToURL(
+ browser(), https_server_mismatched_.GetURL("/ssl/blank_page.html"));
+ content::WaitForInterstitialAttach(tab);
+
+ InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+ ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
+ interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+ EXPECT_FALSE(interstitial_timer_observer.timer_started());
+
+ // Check that the histogram for the captive portal cert was recorded.
+ histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 3);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::HANDLE_ALL, 1);
+ histograms.ExpectBucketCount(
+ SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
+}
+
+namespace {
+
+// Test class that mimics a URL request with a certificate whose SPKI hash is in
+// ssl_error_assistant.asciipb resource. A better way of testing the SPKI hashes
+// inside the resource bundle would be to serve the actual certificate from the
+// embeded test server, but the test server can only serve a limited number of
+// predefined certificates.
+template <int kNetError>
+class SSLUICaptivePortalTest : public InProcessBrowserTest {
+ public:
+ SSLUICaptivePortalTest() : InProcessBrowserTest(), cert_(nullptr) {}
+
+ void SetUpOnMainThread() override {
+ cert_ =
+ net::ImportCertFromFile(net::GetTestCertsDirectory(), "ok_cert.pem");
+ ASSERT_TRUE(cert_);
+ content::BrowserThread::PostTask(
+ content::BrowserThread::IO, FROM_HERE,
+ base::Bind(&SSLUICaptivePortalTest::AddUrlHandler, cert_));
+ }
+
+ static void AddUrlHandler(scoped_refptr<net::X509Certificate> cert) {
+ net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+ filter->AddHostnameInterceptor("https", kCaptivePortalHostname,
+ std::unique_ptr<net::URLRequestInterceptor>(
+ new CaptivePortalInterceptor(cert)));
+ }
+
+ private:
+ // A URLRequestJob that mocks a TLS connection with a certificate whose SPKI
+ // hash is marked as a known captive portal certificate SPKI.
+ class CaptivePortalURLRequestJob : public net::URLRequestJob {
+ public:
+ CaptivePortalURLRequestJob(net::URLRequest* request,
+ net::NetworkDelegate* network_delegate,
+ scoped_refptr<net::X509Certificate> cert)
+ : net::URLRequestJob(request, network_delegate),
+ cert_(std::move(cert)),
+ weak_factory_(this) {}
+
+ // net::URLRequestJob method:
+ void Start() override {
+ net::SSLInfo ssl_info;
+ ssl_info.SetCertError(kNetError);
+ ssl_info.cert = cert_;
+
+ // Set the SPKI hash to captive-portal.badssl.com leaf certificate. This
+ // doesn't match the actual cert (ok_cert.pem) but is good enough for
+ // testing.
+ net::HashValue hash;
+ ASSERT_TRUE(hash.FromString(
+ "sha256/fjZPHewEHTrMDX3I1ecEIeoy3WFxHyGplOLv28kIbtI="));
+ ssl_info.public_key_hashes.push_back(hash);
+
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::Bind(&CaptivePortalURLRequestJob::NotifySSLCertificateError,
+ weak_factory_.GetWeakPtr(), ssl_info, false));
+ }
+
+ protected:
+ ~CaptivePortalURLRequestJob() override {}
+
+ private:
+ const scoped_refptr<net::X509Certificate> cert_;
+ base::WeakPtrFactory<CaptivePortalURLRequestJob> weak_factory_;
+
+ DISALLOW_COPY_AND_ASSIGN(CaptivePortalURLRequestJob);
+ };
+
+ // A URLRequestInterceptor that handles requests with
+ // CaptivePortalURLRequestJob jobs.
+ class CaptivePortalInterceptor : public net::URLRequestInterceptor {
+ public:
+ CaptivePortalInterceptor(scoped_refptr<net::X509Certificate> cert)
+ : cert_(std::move(cert)) {}
+
+ ~CaptivePortalInterceptor() override {}
+
+ // net::URLRequestInterceptor method:
+ net::URLRequestJob* MaybeInterceptRequest(
+ net::URLRequest* request,
+ net::NetworkDelegate* network_delegate) const override {
+ return new CaptivePortalURLRequestJob(request, network_delegate, cert_);
+ }
+
+ private:
+ const scoped_refptr<net::X509Certificate> cert_;
+
+ DISALLOW_COPY_AND_ASSIGN(CaptivePortalInterceptor);
+ };
+
+ scoped_refptr<net::X509Certificate> cert_;
+};
+
+using SSLUICaptivePortalNameMismatchTest =
+ SSLUICaptivePortalTest<net::ERR_CERT_COMMON_NAME_INVALID>;
+using SSLUICaptivePortalAuthorityInvalidTest =
+ SSLUICaptivePortalTest<net::ERR_CERT_AUTHORITY_INVALID>;
+
+} // namespace
+
+// Same as CaptivePortalCertificateList_Enabled_FromProto, but this time the
+// cert's SPKI hash is listed in ssl_error_assistant.asciipb.
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalNameMismatchTest,
+ CaptivePortalCertificateList_Enabled_FromResource) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitFromCommandLine(
+ "CaptivePortalCertificateList" /* enabled */, "" /* disabled */);
+
+ base::HistogramTester histograms;
+
+ // Navigate to an unsafe page on the server. The captive portal interstitial
+ // should be displayed since CaptivePortalCertificateList feature is enabled.
+ WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+ SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+ ui_test_utils::NavigateToURL(
+ browser(), GURL(std::string("https://") + kCaptivePortalHostname));
+ content::WaitForInterstitialAttach(tab);
+
+ InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+ ASSERT_EQ(CaptivePortalBlockingPage::kTypeForTesting,
+ interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+ EXPECT_FALSE(interstitial_timer_observer.timer_started());
+
+ // Check that the histogram for the captive portal cert was recorded.
+ histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 3);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::HANDLE_ALL, 1);
+ histograms.ExpectBucketCount(
+ SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::SHOW_CAPTIVE_PORTAL_INTERSTITIAL_OVERRIDABLE, 1);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::CAPTIVE_PORTAL_CERT_FOUND, 1);
+}
+
+// Same as SSLUICaptivePortalNameMismatchTest, but this time the error is
+// authority-invalid. Captive portal interstitial should not be shown.
+IN_PROC_BROWSER_TEST_F(SSLUICaptivePortalAuthorityInvalidTest,
+ CaptivePortalCertificateList_Enabled_FromResource) {
+ base::test::ScopedFeatureList scoped_feature_list;
+ scoped_feature_list.InitFromCommandLine(
+ "CaptivePortalCertificateList" /* enabled */, "" /* disabled */);
+
+ // Set interstitial delay to zero.
+ SSLErrorHandler::SetInterstitialDelayForTesting(base::TimeDelta());
+
+ base::HistogramTester histograms;
+ // Navigate to an unsafe page on the server. CaptivePortalCertificateList
+ // feature is enabled but the error is not a name mismatch, so a generic SSL
+ // interstitial should be displayed.
+ WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+ SSLInterstitialTimerObserver interstitial_timer_observer(tab);
+ ui_test_utils::NavigateToURL(
+ browser(), GURL(std::string("https://") + kCaptivePortalHostname));
+ content::WaitForInterstitialAttach(tab);
+
+ InterstitialPage* interstitial_page = tab->GetInterstitialPage();
+ ASSERT_EQ(SSLBlockingPage::kTypeForTesting,
+ interstitial_page->GetDelegateForTesting()->GetTypeForTesting());
+ EXPECT_TRUE(interstitial_timer_observer.timer_started());
+
+ // Check that the histogram for the captive portal cert was recorded.
+ histograms.ExpectTotalCount(SSLErrorHandler::GetHistogramNameForTesting(), 2);
+ histograms.ExpectBucketCount(SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::HANDLE_ALL, 1);
+ histograms.ExpectBucketCount(
+ SSLErrorHandler::GetHistogramNameForTesting(),
+ SSLErrorHandler::SHOW_SSL_INTERSTITIAL_OVERRIDABLE, 1);
+}
+
+#endif // BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+
// TODO(jcampan): more tests to do below.
// Visit a page over https that contains a frame with a redirect.

Powered by Google App Engine
This is Rietveld 408576698