Add constructor to create SSLStatus from SSLInfo

chrome/browser/ssl/ssl_browser_tests.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/metrics/field_trial.h"
@@ skipping 29 matching lines @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/security_interstitials/core/metrics_helper.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/cert_store.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/security_style.h"
@@ skipping 170 matching lines @@
   }
 
   const content::WebContents* web_contents_;
   SSLErrorHandler::TimerStartedCallback callback_;
 
   scoped_ptr<base::RunLoop> message_loop_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(SSLInterstitialTimerObserver);
 };
 
+// Checks that two SSLStatuses will result in the same security UI: that
+// is, the cert ids can differ as long as they refer to the same cert,
+// and otherwise SSLStatus::Equals() must be true.
+void CheckSSLStatusesEquals(const content::SSLStatus& one,
+                            const content::SSLStatus& two) {
+  content::CertStore* cert_store = content::CertStore::GetInstance();
+  scoped_refptr<net::X509Certificate> cert1;
+  scoped_refptr<net::X509Certificate> cert2;
+  cert_store->RetrieveCert(one.cert_id, &cert1);
+  cert_store->RetrieveCert(two.cert_id, &cert2);
+  EXPECT_TRUE(cert1);

[meacer, 2015/09/10 17:55:22]

nit: EXPECT_TRUE(cert1 && cert2)?

[estark, 2015/09/11 05:22:52]

Done.

+  EXPECT_TRUE(cert1->Equals(cert2.get()));
+
+  SSLStatus one_without_cert_id = one;
+  one_without_cert_id.cert_id = 0;
+  SSLStatus two_without_cert_id = two;
+  two_without_cert_id.cert_id = 0;

[meacer, 2015/09/10 17:55:22]

nit: I'm wondering if these should be done in SSLStatus::Equals. You could
simply skip cert_id check there and add a comment explaining why.

Actually, you could go one step further and fix SSLStatus::Equals altogether by
moving the RetrieveCert code above to that function.

(I'm trying to patch your CL and see if doing that will break any tests, but
it's taking too long and I wanted to send my comments earlier)

[estark, 2015/09/10 17:59:28]

The bummer is that SSLStatus is in content/public/common, and CertStore only
exists in content/public/browser. :( So I don't think it would be possible to
expose this kind of equality test to the renderer, because the CertStore doesn't
exist in the renderer.

[meacer, 2015/09/10 18:02:23]

Ah, right. I suppose we could make SSLStatus::Equals pure virtual method, and
then derive our own ChromeSSLStatus and implement that and so on to make this
even more complicated.

Let's go with this one instead :)

+  EXPECT_TRUE(one_without_cert_id.Equals(two_without_cert_id));
+}
+
 }  // namespace
 
 class SSLUITest
     : public certificate_reporting_test_utils::CertificateReportingTest {
  public:
   SSLUITest()
       : https_server_(net::SpawnedTestServer::TYPE_HTTPS,
                       SSLOptions(SSLOptions::CERT_OK),
                       base::FilePath(kDocRoot)),
         https_server_expired_(net::SpawnedTestServer::TYPE_HTTPS,
@@ skipping 2097 matching lines @@
 
   ProceedThroughInterstitial(tab);
   EXPECT_TRUE(state->HasAllowException(https_server_host));
 
   ui_test_utils::NavigateToURL(browser(),
                                https_server_.GetURL("files/ssl/google.html"));
   ASSERT_FALSE(tab->GetInterstitialPage());
   EXPECT_FALSE(state->HasAllowException(https_server_host));
 }
 
+// Tests that the SSLStatus of a navigation entry for an SSL
+// interstitial matches the navigation entry once the interstitial is
+// clicked through. https://crbug.com/529456
+IN_PROC_BROWSER_TEST_F(SSLUITest,
+                       SSLStatusMatchesOnInterstitialAndAfterProceed) {
+  ASSERT_TRUE(https_server_expired_.Start());
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(tab);
+
+  ui_test_utils::NavigateToURL(
+      browser(), https_server_expired_.GetURL("files/ssl/google.html"));
+  content::WaitForInterstitialAttach(tab);
+  EXPECT_TRUE(tab->ShowingInterstitialPage());
+
+  content::NavigationEntry* entry = tab->GetController().GetActiveEntry();
+  ASSERT_TRUE(entry);
+  content::SSLStatus interstitial_ssl_status = entry->GetSSL();
+
+  ProceedThroughInterstitial(tab);
+  EXPECT_FALSE(tab->ShowingInterstitialPage());
+  entry = tab->GetController().GetActiveEntry();
+  ASSERT_TRUE(entry);
+
+  content::SSLStatus after_interstitial_ssl_status = entry->GetSSL();
+  ASSERT_NO_FATAL_FAILURE(CheckSSLStatusesEquals(after_interstitial_ssl_status,
+                                                 interstitial_ssl_status));
+}
+
+// As above, but for a bad clock interstitial. Tests that a clock
+// interstitial's SSLStatus matches the SSLStatus of the HTTPS page
+// after proceeding through a normal SSL interstitial.
+IN_PROC_BROWSER_TEST_F(SSLUITest,
+                       SSLStatusMatchesonClockInterstitialAndAfterProceed) {
+  ASSERT_TRUE(https_server_expired_.Start());
+  WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(tab);
+
+  // Set up the build and current clock times to be more than a year apart.
+  base::SimpleTestClock mock_clock;
+  mock_clock.SetNow(base::Time::NowFromSystemTime());
+  mock_clock.Advance(base::TimeDelta::FromDays(367));
+  SSLErrorHandler::SetClockForTest(&mock_clock);
+  SSLErrorClassification::SetBuildTimeForTesting(
+      base::Time::NowFromSystemTime());
+
+  ui_test_utils::NavigateToURL(browser(), https_server_expired_.GetURL("/"));
+  content::WaitForInterstitialAttach(tab);
+  InterstitialPage* clock_interstitial = tab->GetInterstitialPage();
+  ASSERT_TRUE(clock_interstitial);
+  EXPECT_EQ(BadClockBlockingPage::kTypeForTesting,
+            clock_interstitial->GetDelegateForTesting()->GetTypeForTesting());
+
+  // Grab the SSLStatus on the clock interstitial.
+  content::NavigationEntry* entry = tab->GetController().GetActiveEntry();
+  ASSERT_TRUE(entry);
+  content::SSLStatus clock_interstitial_ssl_status = entry->GetSSL();
+
+  // Put the clock back to normal, trigger a normal SSL interstitial,
+  // and proceed through it.
+  mock_clock.SetNow(base::Time::NowFromSystemTime());
+  ui_test_utils::NavigateToURL(browser(), https_server_expired_.GetURL("/"));
+  content::WaitForInterstitialAttach(tab);
+  InterstitialPage* ssl_interstitial = tab->GetInterstitialPage();
+  ASSERT_TRUE(ssl_interstitial);
+  EXPECT_EQ(SSLBlockingPage::kTypeForTesting,
+            ssl_interstitial->GetDelegateForTesting()->GetTypeForTesting());
+  ProceedThroughInterstitial(tab);
+  EXPECT_FALSE(tab->ShowingInterstitialPage());
+
+  // Grab the SSLStatus from the page and check that it is the same as
+  // on the clock interstitial.
+  entry = tab->GetController().GetActiveEntry();
+  ASSERT_TRUE(entry);
+  content::SSLStatus after_interstitial_ssl_status = entry->GetSSL();
+  ASSERT_NO_FATAL_FAILURE(CheckSSLStatusesEquals(
+      after_interstitial_ssl_status, clock_interstitial_ssl_status));
+}
+
 class CommonNameMismatchBrowserTest : public CertVerifierBrowserTest {
  public:
   CommonNameMismatchBrowserTest() : CertVerifierBrowserTest() {}
   ~CommonNameMismatchBrowserTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     // Enable finch experiment for SSL common name mismatch handling.
     command_line->AppendSwitchASCII(switches::kForceFieldTrials,
                                     "SSLCommonNameMismatchHandling/Enabled/");
   }
@@ skipping 367 matching lines @@
 
 // Visit a page over https that contains a frame with a redirect.
 
 // XMLHttpRequest insecure content in synchronous mode.
 
 // XMLHttpRequest insecure content in asynchronous mode.
 
 // XMLHttpRequest over bad ssl in synchronous mode.
 
 // XMLHttpRequest over OK ssl in synchronous mode.