Add constructor to create SSLStatus from SSLInfo

chrome/browser/ssl/ssl_browser_tests.cc

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 465d7ab86bd8de73a92f5a000bc5d94a97a1a100..67726e0d8887d3d8e68f9e5f2cfc4c9a5bfcd311 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -47,6 +47,7 @@
 #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"
@@ -237,6 +238,26 @@ class SSLInterstitialTimerObserver {
   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
@@ -2354,6 +2375,84 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, BadCertFollowedByGoodCert) {
   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() {}