diff options
author | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-15 00:38:36 +0000 |
---|---|---|
committer | pkasting@chromium.org <pkasting@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-05-15 00:38:36 +0000 |
commit | 18de157f6c4c5eee14ad84e037def47a7f52a3a8 (patch) | |
tree | ade764c9a40927c1856a759d0f92e6456bf7f2ea | |
parent | 429e1ec2b634ce856c43f077ae6d18d537fc86f2 (diff) | |
download | chromium_src-18de157f6c4c5eee14ad84e037def47a7f52a3a8.zip chromium_src-18de157f6c4c5eee14ad84e037def47a7f52a3a8.tar.gz chromium_src-18de157f6c4c5eee14ad84e037def47a7f52a3a8.tar.bz2 |
(Original patch reviewed at http://codereview.chromium.org/2067003 )
Track "display" and "run" separately for mixed content, and make the latter downgrade the SSL state to "authentication broken".
Make the "display" state only affect the current tab (not the entire host).
Fix an SSL browser test by supplying the appropriate SiteInstance*.
Move a test from "disabled" to "flaky" since it at least passes for me.
Make the SSLManager header and .cc files put functions in the same order, and make that order somewhat saner.
BUG=15072, 18626, 40932, 42758
TEST=Covered by browser tests
Review URL: http://codereview.chromium.org/2063008
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47347 0039d316-1c4b-4281-b951-d872f2087c98
23 files changed, 452 insertions, 368 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 5bfb3fd..6cc3378 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -5050,8 +5050,11 @@ Keep your key file in a safe place. You will need it to create new versions of y <message name="IDS_PAGE_INFO_SECURITY_TAB_NOT_ENCRYPTED_CONNECTION_TEXT" desc="The text of the connection section when the connection is not encrypted."> Your connection to <ph name="DOMAIN">$1<ex>www.google.com</ex></ph> is not encrypted. </message> - <message name="IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_MIXED_CONTENT_WARNING" desc="Some extra text of the connection section when the connection is encrypted and there is some mixed content on the page."> - However, this page includes other resources which are not secure. These resources can be viewed by others while in transit, and can be modified by an attacker to change the look or behavior of the page. + <message name="IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_MIXED_CONTENT_WARNING" desc="Some extra text of the connection section when the connection is encrypted and the page contains mixed content which has been displayed (e.g. images, CSS)."> + However, this page includes other resources which are not secure. These resources can be viewed by others while in transit, and can be modified by an attacker to change the look of the page. + </message> + <message name="IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_MIXED_CONTENT_ERROR" desc="Some extra text of the connection section when the connection is encrypted and the page contains mixed content which has been run (e.g. script)."> + However, this page includes other resources which are not secure. These resources can be viewed by others while in transit, and can be modified by an attacker to change the behavior of the page. </message> <message name="IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_SENTENCE_LINK" desc="Linking 2 sentences in 1 paragraph."> <ph name="SENTENCE1">$1<ex>Your connection is encrypted.</ex></ph> <ph name="SENTENCE2">$2<ex>However, this page includes resources from other pages whose identity cannot be verified.</ex></ph> diff --git a/chrome/browser/external_tab_container.cc b/chrome/browser/external_tab_container.cc index 0833a6c..c651892 100644 --- a/chrome/browser/external_tab_container.cc +++ b/chrome/browser/external_tab_container.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -750,7 +750,8 @@ bool ExternalTabContainer::InitNavigationInfo(IPC::NavigationInfo* nav_info, nav_info->title = UTF8ToWide(nav_info->url.spec()); nav_info->security_style = entry->ssl().security_style(); - nav_info->has_mixed_content = entry->ssl().has_mixed_content(); + nav_info->displayed_mixed_content = entry->ssl().displayed_mixed_content(); + nav_info->ran_mixed_content = entry->ssl().ran_mixed_content(); return true; } diff --git a/chrome/browser/page_info_model.cc b/chrome/browser/page_info_model.cc index 890c3dfb..381008c 100644 --- a/chrome/browser/page_info_model.cc +++ b/chrome/browser/page_info_model.cc @@ -138,13 +138,14 @@ PageInfoModel::PageInfoModel(Profile* profile, IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_CONNECTION_TEXT, subject_name, IntToString16(ssl.security_bits()))); - if (ssl.has_mixed_content()) { + if (ssl.displayed_mixed_content() || ssl.ran_mixed_content()) { state = false; description.assign( l10n_util::GetStringFUTF16( IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_SENTENCE_LINK, description, - l10n_util::GetStringUTF16( + l10n_util::GetStringUTF16(ssl.ran_mixed_content() ? + IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_MIXED_CONTENT_ERROR : IDS_PAGE_INFO_SECURITY_TAB_ENCRYPTED_MIXED_CONTENT_WARNING))); } } diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc index a2fa261b..74d9dbe 100644 --- a/chrome/browser/ssl/ssl_browser_tests.cc +++ b/chrome/browser/ssl/ssl_browser_tests.cc @@ -34,13 +34,14 @@ class SSLUITest : public InProcessBrowserTest { } void CheckAuthenticatedState(TabContents* tab, - bool mixed_content) { + bool displayed_mixed_content) { NavigationEntry* entry = tab->controller().GetActiveEntry(); ASSERT_TRUE(entry); EXPECT_EQ(NavigationEntry::NORMAL_PAGE, entry->page_type()); EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, entry->ssl().security_style()); EXPECT_EQ(0, entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS); - EXPECT_EQ(mixed_content, entry->ssl().has_mixed_content()); + EXPECT_EQ(displayed_mixed_content, entry->ssl().displayed_mixed_content()); + EXPECT_FALSE(entry->ssl().ran_mixed_content()); } void CheckUnauthenticatedState(TabContents* tab) { @@ -49,11 +50,13 @@ class SSLUITest : public InProcessBrowserTest { EXPECT_EQ(NavigationEntry::NORMAL_PAGE, entry->page_type()); EXPECT_EQ(SECURITY_STYLE_UNAUTHENTICATED, entry->ssl().security_style()); EXPECT_EQ(0, entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS); - EXPECT_FALSE(entry->ssl().has_mixed_content()); + EXPECT_FALSE(entry->ssl().displayed_mixed_content()); + EXPECT_FALSE(entry->ssl().ran_mixed_content()); } void CheckAuthenticationBrokenState(TabContents* tab, int error, + bool ran_mixed_content, bool interstitial) { NavigationEntry* entry = tab->controller().GetActiveEntry(); ASSERT_TRUE(entry); @@ -66,7 +69,8 @@ class SSLUITest : public InProcessBrowserTest { // to SECURITY_STYLE_AUTHENTICATION_BROKEN. ASSERT_NE(net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION, error); EXPECT_EQ(error, entry->ssl().cert_status() & net::CERT_STATUS_ALL_ERRORS); - EXPECT_FALSE(entry->ssl().has_mixed_content()); + EXPECT_FALSE(entry->ssl().displayed_mixed_content()); + EXPECT_EQ(ran_mixed_content, entry->ssl().ran_mixed_content()); } void CheckWorkerLoadResult(TabContents* tab, bool expectLoaded) { @@ -160,12 +164,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndProceed) { bad_https_server->TestServerPage("files/ssl/google.html")); TabContents* tab = browser()->GetSelectedTabContents(); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing ProceedThroughInterstitial(tab); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No interstitial showing } @@ -204,7 +208,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestHTTPSExpiredCertAndDontProceed) { // An interstitial should be showing. CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID, - true); // Interstitial showing. + false, true); // Simulate user clicking "Take me back". InterstitialPage* interstitial_page = tab->interstitial_page(); @@ -237,7 +241,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSExpiredCertAndGoBackViaButton) { // Now go to a bad HTTPS page that shows an interstitial. ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/google.html")); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing // Simulate user clicking on back button (crbug.com/39248). @@ -266,7 +270,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestHTTPSExpiredCertAndGoBackViaMenu) { // Now go to a bad HTTPS page that shows an interstitial. ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/google.html")); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing // Simulate user clicking and holding on back button (crbug.com/37215). @@ -306,7 +310,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestHTTPSExpiredCertAndGoForward) { // Now go to a bad HTTPS page that shows an interstitial. ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/google.html")); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing // Simulate user clicking and holding on forward button. @@ -368,34 +372,34 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestHTTPSErrorWithNoNavEntry) { // Mixed contents // -// Visits a page with mixed content. -IN_PROC_BROWSER_TEST_F(SSLUITest, TestMixedContents) { +// Visits a page that displays mixed content. +IN_PROC_BROWSER_TEST_F(SSLUITest, TestDisplaysMixedContent) { scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); ASSERT_TRUE(https_server.get() != NULL); scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); - // Load a page with mixed-content, the default behavior is to show the mixed - // content. + // Load a page that displays mixed content. ui_test_utils::NavigateToURL(browser(), https_server->TestServerPage( - "files/ssl/page_with_mixed_contents.html")); + "files/ssl/page_displays_mixed_content.html")); CheckAuthenticatedState(browser()->GetSelectedTabContents(), true); } -// Visits a page with an http script that tries to suppress our mixed content -// warnings by randomize location.hash. +// Visits a page that runs mixed content and tries to suppress the mixed content +// warnings by randomizing location.hash. // Based on http://crbug.com/8706 -IN_PROC_BROWSER_TEST_F(SSLUITest, TestMixedContentsRandomizeHash) { +IN_PROC_BROWSER_TEST_F(SSLUITest, TestRunsMixedContentRandomizeHash) { scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); ASSERT_TRUE(https_server.get() != NULL); scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); ui_test_utils::NavigateToURL(browser(), - https_server->TestServerPage("files/ssl/page_with_http_script.html")); + https_server->TestServerPage("files/ssl/page_runs_mixed_content.html")); - CheckAuthenticatedState(browser()->GetSelectedTabContents(), true); + CheckAuthenticationBrokenState(browser()->GetSelectedTabContents(), 0, true, + false); } // Visits a page with unsafe content and make sure that: @@ -434,13 +438,13 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestUnsafeContents) { bool js_result = false; EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool( - tab->render_view_host(), L"", + tab->render_view_host(), std::wstring(), L"window.domAutomationController.send(IsFooSet());", &js_result)); EXPECT_FALSE(js_result); } // Visits a page with mixed content loaded by JS (after the initial page load). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) { +IN_PROC_BROWSER_TEST_F(SSLUITest, TestDisplaysMixedContentLoadedFromJS) { scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); ASSERT_TRUE(https_server.get() != NULL); scoped_refptr<HTTPTestServer> http_server = PlainServer(); @@ -462,11 +466,10 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestMixedContentsLoadedFromJS) { CheckAuthenticatedState(tab, true); } -// Visits two pages from the same origin: one with mixed content and one -// without. The test checks that we propagate the mixed content state from one -// to the other. -// TODO(jcampan): http://crbug.com/15072 this test fails. -IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestMixedContentsTwoTabs) { +// Visits two pages from the same origin: one that displays mixed content and +// one that doesn't. The test checks that we do not propagate the mixed content +// state from one to the other. +IN_PROC_BROWSER_TEST_F(SSLUITest, TestDisplaysMixedContentTwoTabs) { scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); ASSERT_TRUE(https_server.get() != NULL); scoped_refptr<HTTPTestServer> http_server = PlainServer(); @@ -481,41 +484,95 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestMixedContentsTwoTabs) { CheckAuthenticatedState(tab1, false); // Create a new tab. - GURL url = - https_server->TestServerPage("files/ssl/page_with_http_script.html"); + GURL url = https_server->TestServerPage( + "files/ssl/page_displays_mixed_content.html"); TabContents* tab2 = browser()->AddTabWithURL(url, GURL(), - PageTransition::TYPED, 0, Browser::ADD_SELECTED, NULL, std::string()); + PageTransition::TYPED, 0, Browser::ADD_SELECTED, tab1->GetSiteInstance(), + std::string()); ui_test_utils::WaitForNavigation(&(tab2->controller())); // The new tab has mixed content. CheckAuthenticatedState(tab2, true); + // The original tab should not be contaminated. + CheckAuthenticatedState(tab1, false); +} + +// Visits two pages from the same origin: one that runs mixed content and one +// that doesn't. The test checks that we propagate the mixed content state from +// one to the other. +IN_PROC_BROWSER_TEST_F(SSLUITest, TestRunsMixedContentTwoTabs) { + scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); + ASSERT_TRUE(https_server.get() != NULL); + scoped_refptr<HTTPTestServer> http_server = PlainServer(); + ASSERT_TRUE(http_server.get() != NULL); + + ui_test_utils::NavigateToURL(browser(), + https_server->TestServerPage("files/ssl/blank_page.html")); + + TabContents* tab1 = browser()->GetSelectedTabContents(); + + // This tab should be fine. + CheckAuthenticatedState(tab1, false); + + // Create a new tab. + GURL url = + https_server->TestServerPage("files/ssl/page_runs_mixed_content.html"); + TabContents* tab2 = browser()->AddTabWithURL(url, GURL(), + PageTransition::TYPED, 0, Browser::ADD_SELECTED, tab1->GetSiteInstance(), + std::string()); + ui_test_utils::WaitForNavigation(&(tab2->controller())); + + // The new tab has mixed content. + CheckAuthenticationBrokenState(tab2, 0, true, false); + // Which means the origin for the first tab has also been contaminated with // mixed content. - CheckAuthenticatedState(tab1, true); + CheckAuthenticationBrokenState(tab1, 0, true, false); } // Visits a page with an image over http. Visits another page over https // referencing that same image over http (hoping it is coming from the webcore // memory cache). -IN_PROC_BROWSER_TEST_F(SSLUITest, TestCachedMixedContents) { +IN_PROC_BROWSER_TEST_F(SSLUITest, TestDisplaysCachedMixedContent) { scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); ASSERT_TRUE(https_server.get() != NULL); scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); ui_test_utils::NavigateToURL(browser(), http_server->TestServerPage( - "files/ssl/page_with_mixed_contents.html")); + "files/ssl/page_displays_mixed_content.html")); TabContents* tab = browser()->GetSelectedTabContents(); CheckUnauthenticatedState(tab); - // Load again but over SSL. It should have mixed-contents (even though the - // image comes from the WebCore memory cache). + // Load again but over SSL. It should be marked as displaying mixed content + // (even though the image comes from the WebCore memory cache). ui_test_utils::NavigateToURL(browser(), https_server->TestServerPage( - "files/ssl/page_with_mixed_contents.html")); + "files/ssl/page_displays_mixed_content.html")); CheckAuthenticatedState(tab, true); } +// Visits a page with script over http. Visits another page over https +// referencing that same script over http (hoping it is coming from the webcore +// memory cache). +IN_PROC_BROWSER_TEST_F(SSLUITest, TestRunsCachedMixedContent) { + scoped_refptr<HTTPSTestServer> https_server = GoodCertServer(); + ASSERT_TRUE(https_server.get() != NULL); + scoped_refptr<HTTPTestServer> http_server = PlainServer(); + ASSERT_TRUE(http_server.get() != NULL); + + ui_test_utils::NavigateToURL(browser(), + http_server->TestServerPage("files/ssl/page_runs_mixed_content.html")); + TabContents* tab = browser()->GetSelectedTabContents(); + CheckUnauthenticatedState(tab); + + // Load again but over SSL. It should be marked as displaying mixed content + // (even though the image comes from the WebCore memory cache). + ui_test_utils::NavigateToURL(browser(), + https_server->TestServerPage("files/ssl/page_runs_mixed_content.html")); + CheckAuthenticationBrokenState(tab, 0, true, false); +} + // This test ensures the CN invalid status does not 'stick' to a certificate // (see bug #1044942) and that it depends on the host-name. IN_PROC_BROWSER_TEST_F(SSLUITest, TestCNInvalidStickiness) { @@ -532,12 +589,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestCNInvalidStickiness) { // We get an interstitial page as a result. TabContents* tab = browser()->GetSelectedTabContents(); CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID, - true); // Interstitial showing. + false, true); // Interstitial showing. ProceedThroughInterstitial(tab); CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID, - false); // No interstitial showing. + false, false); // No interstitial showing. // Now we try again with the right host name this time. @@ -561,7 +618,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestCNInvalidStickiness) { // Since we OKed the interstitial last time, we get right to the page. CheckAuthenticationBrokenState(tab, net::CERT_STATUS_COMMON_NAME_INVALID, - false); // No interstitial showing. + false, false); // No interstitial showing. } // Test that navigating to a #ref does not change a bad security state. @@ -573,12 +630,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestRefNavigation) { bad_https_server->TestServerPage("files/ssl/page_with_refs.html")); TabContents* tab = browser()->GetSelectedTabContents(); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing. ProceedThroughInterstitial(tab); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No interstitial showing. // Now navigate to a ref in the page, the security state should not have @@ -586,7 +643,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestRefNavigation) { ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/page_with_refs.html#jp")); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No interstitial showing. } @@ -642,7 +699,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestRedirectBadToGoodHTTPS) { TabContents* tab = browser()->GetSelectedTabContents(); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing. ProceedThroughInterstitial(tab); @@ -664,12 +721,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestRedirectGoodToBadHTTPS) { ui_test_utils::NavigateToURL(browser(), GURL(url1.spec() + url2.spec())); TabContents* tab = browser()->GetSelectedTabContents(); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing. ProceedThroughInterstitial(tab); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No interstitial showing. } @@ -706,12 +763,12 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestRedirectHTTPToBadHTTPS) { bad_https_server->TestServerPage("files/ssl/google.html"); ui_test_utils::NavigateToURL(browser(), GURL(http_url.spec() + bad_https_url.spec())); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing. ProceedThroughInterstitial(tab); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No interstitial showing. } @@ -752,7 +809,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestConnectToBadPort) { // - navigate to a bad HTTPS (expect unsafe content and filtered frame), then // back // - navigate to HTTP (expect mixed content), then back -IN_PROC_BROWSER_TEST_F(SSLUITest, DISABLED_TestGoodFrameNavigation) { +IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestGoodFrameNavigation) { scoped_refptr<HTTPTestServer> http_server = PlainServer(); ASSERT_TRUE(http_server.get() != NULL); scoped_refptr<HTTPSTestServer> good_https_server = GoodCertServer(); @@ -833,7 +890,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestBadFrameNavigation) { TabContents* tab = browser()->GetSelectedTabContents(); ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/top_frame.html")); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing ProceedThroughInterstitial(tab); @@ -848,7 +905,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestBadFrameNavigation) { ui_test_utils::WaitForNavigation(&tab->controller()); // We should still be authentication broken. - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false); + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, + false); } // From an HTTP top frame, navigate to good and bad HTTPS (security state should @@ -932,18 +990,19 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, FLAKY_TestUnsafeContentsInWorker) { ui_test_utils::NavigateToURL(browser(), bad_https_server->TestServerPage("files/ssl/blank_page.html")); TabContents* tab = browser()->GetSelectedTabContents(); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, true); // Interstitial showing ProceedThroughInterstitial(tab); - CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, + CheckAuthenticationBrokenState(tab, net::CERT_STATUS_DATE_INVALID, false, false); // No Interstitial // Navigate to safe page that has Worker loading unsafe content. - // Expect content to load but 'mixed' indicators show up. + // Expect content to load but be marked as auth broken due to running mixed + // content. ui_test_utils::NavigateToURL(browser(), good_https_server->TestServerPage( "files/ssl/page_with_unsafe_worker.html")); CheckWorkerLoadResult(tab, true); // Worker loads mixed content - CheckAuthenticatedState(tab, true); + CheckAuthenticationBrokenState(tab, 0, true, false); } // TODO(jcampan): more tests to do below. diff --git a/chrome/browser/ssl/ssl_host_state.cc b/chrome/browser/ssl/ssl_host_state.cc index 7e6d3cc..e12c458 100644 --- a/chrome/browser/ssl/ssl_host_state.cc +++ b/chrome/browser/ssl/ssl_host_state.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -23,22 +23,21 @@ SSLHostState::SSLHostState() { SSLHostState::~SSLHostState() { } -void SSLHostState::MarkHostAsBroken(const std::string& host, int pid) { +void SSLHostState::HostRanInsecureContent(const std::string& host, int pid) { DCHECK(CalledOnValidThread()); - - broken_hosts_.insert(BrokenHostEntry(host, pid)); + ran_insecure_content_hosts_.insert(BrokenHostEntry(host, pid)); } -bool SSLHostState::DidMarkHostAsBroken(const std::string& host, int pid) { +bool SSLHostState::DidHostRunInsecureContent(const std::string& host, + int pid) const { DCHECK(CalledOnValidThread()); - // CAs issue certificate for intranet hosts to everyone. Therefore, we always - // treat intranet hosts as broken. + // CAs issue certificates for intranet hosts to everyone. Therefore, we + // always treat intranet hosts as having run insecure content. if (IsIntranetHost(host)) return true; - return (broken_hosts_.find( - BrokenHostEntry(host, pid)) != broken_hosts_.end()); + return !!ran_insecure_content_hosts_.count(BrokenHostEntry(host, pid)); } void SSLHostState::DenyCertForHost(net::X509Certificate* cert, diff --git a/chrome/browser/ssl/ssl_host_state.h b/chrome/browser/ssl/ssl_host_state.h index 5a8d774..3c87900 100644 --- a/chrome/browser/ssl/ssl_host_state.h +++ b/chrome/browser/ssl/ssl_host_state.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -27,14 +27,11 @@ class SSLHostState : public NonThreadSafe { SSLHostState(); ~SSLHostState(); - // Records that a host is "broken" in a particular render process. That is, - // the origin for that host has been contaminated with insecure content, - // either via HTTP or via HTTPS with a bad certificate. - void MarkHostAsBroken(const std::string& host, int pid); + // Records that a host has run insecure content. + void HostRanInsecureContent(const std::string& host, int pid); - // Returns whether the specified host was marked as broken in a particular - // render process. - bool DidMarkHostAsBroken(const std::string& host, int pid); + // Returns whether the specified host ran insecure content. + bool DidHostRunInsecureContent(const std::string& host, int pid) const; // Records that |cert| is permitted to be used for |host| in the future. void DenyCertForHost(net::X509Certificate* cert, const std::string& host); @@ -54,7 +51,7 @@ class SSLHostState : public NonThreadSafe { // Hosts which have been contaminated with insecure content in the // specified process. Note that insecure content can travel between // same-origin frames in one processs but cannot jump between processes. - std::set<BrokenHostEntry> broken_hosts_; + std::set<BrokenHostEntry> ran_insecure_content_hosts_; // Certificate policies for each host. std::map<std::string, net::X509Certificate::Policy> cert_policy_for_host_; diff --git a/chrome/browser/ssl/ssl_host_state_unittest.cc b/chrome/browser/ssl/ssl_host_state_unittest.cc index 2a3db46..09db79b 100644 --- a/chrome/browser/ssl/ssl_host_state_unittest.cc +++ b/chrome/browser/ssl/ssl_host_state_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -91,24 +91,24 @@ unsigned char google_der[] = { class SSLHostStateTest : public testing::Test { }; -TEST_F(SSLHostStateTest, MarkHostAsBroken) { +TEST_F(SSLHostStateTest, DidHostRunInsecureContent) { SSLHostState state; - EXPECT_FALSE(state.DidMarkHostAsBroken("www.google.com", 42)); - EXPECT_FALSE(state.DidMarkHostAsBroken("www.google.com", 191)); - EXPECT_FALSE(state.DidMarkHostAsBroken("example.com", 42)); + EXPECT_FALSE(state.DidHostRunInsecureContent("www.google.com", 42)); + EXPECT_FALSE(state.DidHostRunInsecureContent("www.google.com", 191)); + EXPECT_FALSE(state.DidHostRunInsecureContent("example.com", 42)); - state.MarkHostAsBroken("www.google.com", 42); + state.HostRanInsecureContent("www.google.com", 42); - EXPECT_TRUE(state.DidMarkHostAsBroken("www.google.com", 42)); - EXPECT_FALSE(state.DidMarkHostAsBroken("www.google.com", 191)); - EXPECT_FALSE(state.DidMarkHostAsBroken("example.com", 42)); + EXPECT_TRUE(state.DidHostRunInsecureContent("www.google.com", 42)); + EXPECT_FALSE(state.DidHostRunInsecureContent("www.google.com", 191)); + EXPECT_FALSE(state.DidHostRunInsecureContent("example.com", 42)); - state.MarkHostAsBroken("example.com", 42); + state.HostRanInsecureContent("example.com", 42); - EXPECT_TRUE(state.DidMarkHostAsBroken("www.google.com", 42)); - EXPECT_FALSE(state.DidMarkHostAsBroken("www.google.com", 191)); - EXPECT_TRUE(state.DidMarkHostAsBroken("example.com", 42)); + EXPECT_TRUE(state.DidHostRunInsecureContent("www.google.com", 42)); + EXPECT_FALSE(state.DidHostRunInsecureContent("www.google.com", 191)); + EXPECT_TRUE(state.DidHostRunInsecureContent("example.com", 42)); } TEST_F(SSLHostStateTest, QueryPolicy) { diff --git a/chrome/browser/ssl/ssl_manager.cc b/chrome/browser/ssl/ssl_manager.cc index 7683221..99729b6 100644 --- a/chrome/browser/ssl/ssl_manager.cc +++ b/chrome/browser/ssl/ssl_manager.cc @@ -28,38 +28,6 @@ void SSLManager::RegisterUserPrefs(PrefService* prefs) { FilterPolicy::DONT_FILTER); } -SSLManager::SSLManager(NavigationController* controller) - : backend_(controller), - policy_(new SSLPolicy(&backend_)), - controller_(controller) { - DCHECK(controller_); - - // Subscribe to various notifications. - registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR, - Source<NavigationController>(controller_)); - registrar_.Add(this, NotificationType::RESOURCE_RESPONSE_STARTED, - Source<NavigationController>(controller_)); - registrar_.Add(this, NotificationType::RESOURCE_RECEIVED_REDIRECT, - Source<NavigationController>(controller_)); - registrar_.Add(this, NotificationType::LOAD_FROM_MEMORY_CACHE, - Source<NavigationController>(controller_)); - registrar_.Add(this, NotificationType::SSL_INTERNAL_STATE_CHANGED, - NotificationService::AllSources()); -} - -SSLManager::~SSLManager() { -} - -bool SSLManager::ProcessedSSLErrorFromRequest() const { - NavigationEntry* entry = controller_->GetActiveEntry(); - if (!entry) { - NOTREACHED(); - return false; - } - - return net::IsCertStatusError(entry->ssl().cert_status()); -} - // static void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh, URLRequest* request, @@ -86,8 +54,109 @@ void SSLManager::OnSSLCertificateError(ResourceDispatcherHost* rdh, &SSLCertErrorHandler::Dispatch)); } -void SSLManager::DidDisplayInsecureContent() { - policy()->DidDisplayInsecureContent(controller_->GetActiveEntry()); +// static +void SSLManager::NotifySSLInternalStateChanged() { + NotificationService::current()->Notify( + NotificationType::SSL_INTERNAL_STATE_CHANGED, + NotificationService::AllSources(), + NotificationService::NoDetails()); +} + +// static +std::string SSLManager::SerializeSecurityInfo(int cert_id, + int cert_status, + int security_bits) { + Pickle pickle; + pickle.WriteInt(cert_id); + pickle.WriteInt(cert_status); + pickle.WriteInt(security_bits); + return std::string(static_cast<const char*>(pickle.data()), pickle.size()); +} + +// static +bool SSLManager::DeserializeSecurityInfo(const std::string& state, + int* cert_id, + int* cert_status, + int* security_bits) { + DCHECK(cert_id && cert_status && security_bits); + if (state.empty()) { + // No SSL used. + *cert_id = 0; + *cert_status = 0; + *security_bits = -1; + return false; + } + + Pickle pickle(state.data(), static_cast<int>(state.size())); + void * iter = NULL; + return pickle.ReadInt(&iter, cert_id) && + pickle.ReadInt(&iter, cert_status) && + pickle.ReadInt(&iter, security_bits); +} + +// static +std::wstring SSLManager::GetEVCertName(const net::X509Certificate& cert) { + // EV are required to have an organization name and country. + if (cert.subject().organization_names.empty() || + cert.subject().country_name.empty()) { + NOTREACHED(); + return std::wstring(); + } + + return l10n_util::GetStringF(IDS_SECURE_CONNECTION_EV, + UTF8ToWide(cert.subject().organization_names[0]), + UTF8ToWide(cert.subject().country_name)); +} + +SSLManager::SSLManager(NavigationController* controller) + : backend_(controller), + policy_(new SSLPolicy(&backend_)), + controller_(controller) { + DCHECK(controller_); + + // Subscribe to various notifications. + registrar_.Add(this, NotificationType::FAIL_PROVISIONAL_LOAD_WITH_ERROR, + Source<NavigationController>(controller_)); + registrar_.Add(this, NotificationType::RESOURCE_RESPONSE_STARTED, + Source<NavigationController>(controller_)); + registrar_.Add(this, NotificationType::RESOURCE_RECEIVED_REDIRECT, + Source<NavigationController>(controller_)); + registrar_.Add(this, NotificationType::LOAD_FROM_MEMORY_CACHE, + Source<NavigationController>(controller_)); + registrar_.Add(this, NotificationType::SSL_INTERNAL_STATE_CHANGED, + NotificationService::AllSources()); +} + +SSLManager::~SSLManager() { +} + +void SSLManager::DidCommitProvisionalLoad( + const NotificationDetails& in_details) { + NavigationController::LoadCommittedDetails* details = + Details<NavigationController::LoadCommittedDetails>(in_details).ptr(); + + NavigationEntry* entry = controller_->GetActiveEntry(); + + if (details->is_main_frame) { + if (entry) { + // Decode the security details. + int ssl_cert_id, ssl_cert_status, ssl_security_bits; + DeserializeSecurityInfo(details->serialized_security_info, + &ssl_cert_id, + &ssl_cert_status, + &ssl_security_bits); + + // We may not have an entry if this is a navigation to an initial blank + // page. Reset the SSL information and add the new data we have. + entry->ssl() = NavigationEntry::SSLStatus(); + entry->ssl().set_cert_id(ssl_cert_id); + entry->ssl().set_cert_status(ssl_cert_status); + entry->ssl().set_security_bits(ssl_security_bits); + } + backend_.ShowPendingMessages(); + } + + UpdateEntry(entry); } void SSLManager::DidRunInsecureContent(const std::string& security_origin) { @@ -95,6 +164,16 @@ void SSLManager::DidRunInsecureContent(const std::string& security_origin) { security_origin); } +bool SSLManager::ProcessedSSLErrorFromRequest() const { + NavigationEntry* entry = controller_->GetActiveEntry(); + if (!entry) { + NOTREACHED(); + return false; + } + + return net::IsCertStatusError(entry->ssl().cert_status()); +} + void SSLManager::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { @@ -123,27 +202,6 @@ void SSLManager::Observe(NotificationType type, } } -void SSLManager::DispatchSSLVisibleStateChanged() { - NotificationService::current()->Notify( - NotificationType::SSL_VISIBLE_STATE_CHANGED, - Source<NavigationController>(controller_), - NotificationService::NoDetails()); -} - -void SSLManager::UpdateEntry(NavigationEntry* entry) { - // We don't always have a navigation entry to update, for example in the - // case of the Web Inspector. - if (!entry) - return; - - NavigationEntry::SSLStatus original_ssl_status = entry->ssl(); // Copy! - - policy()->UpdateEntry(entry); - - if (!entry->ssl().Equals(original_ssl_status)) - DispatchSSLVisibleStateChanged(); -} - void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) { DCHECK(details); @@ -166,35 +224,6 @@ void SSLManager::DidLoadFromMemoryCache(LoadFromMemoryCacheDetails* details) { policy()->OnRequestStarted(info.get()); } -void SSLManager::DidCommitProvisionalLoad( - const NotificationDetails& in_details) { - NavigationController::LoadCommittedDetails* details = - Details<NavigationController::LoadCommittedDetails>(in_details).ptr(); - - NavigationEntry* entry = controller_->GetActiveEntry(); - - if (details->is_main_frame) { - if (entry) { - // Decode the security details. - int ssl_cert_id, ssl_cert_status, ssl_security_bits; - DeserializeSecurityInfo(details->serialized_security_info, - &ssl_cert_id, - &ssl_cert_status, - &ssl_security_bits); - - // We may not have an entry if this is a navigation to an initial blank - // page. Reset the SSL information and add the new data we have. - entry->ssl() = NavigationEntry::SSLStatus(); - entry->ssl().set_cert_id(ssl_cert_id); - entry->ssl().set_cert_status(ssl_cert_status); - entry->ssl().set_security_bits(ssl_security_bits); - } - backend_.ShowPendingMessages(); - } - - UpdateEntry(entry); -} - void SSLManager::DidFailProvisionalLoadWithError( ProvisionalLoadDetails* details) { DCHECK(details); @@ -238,48 +267,20 @@ void SSLManager::DidChangeSSLInternalState() { UpdateEntry(controller_->GetActiveEntry()); } -// static -std::string SSLManager::SerializeSecurityInfo(int cert_id, - int cert_status, - int security_bits) { - Pickle pickle; - pickle.WriteInt(cert_id); - pickle.WriteInt(cert_status); - pickle.WriteInt(security_bits); - return std::string(static_cast<const char*>(pickle.data()), pickle.size()); -} +void SSLManager::UpdateEntry(NavigationEntry* entry) { + // We don't always have a navigation entry to update, for example in the + // case of the Web Inspector. + if (!entry) + return; -// static -bool SSLManager::DeserializeSecurityInfo(const std::string& state, - int* cert_id, - int* cert_status, - int* security_bits) { - DCHECK(cert_id && cert_status && security_bits); - if (state.empty()) { - // No SSL used. - *cert_id = 0; - *cert_status = 0; - *security_bits = -1; - return false; - } + NavigationEntry::SSLStatus original_ssl_status = entry->ssl(); // Copy! - Pickle pickle(state.data(), static_cast<int>(state.size())); - void * iter = NULL; - return pickle.ReadInt(&iter, cert_id) && - pickle.ReadInt(&iter, cert_status) && - pickle.ReadInt(&iter, security_bits); -} + policy()->UpdateEntry(entry, controller_->tab_contents()); -// static -std::wstring SSLManager::GetEVCertName(const net::X509Certificate& cert) { - // EV are required to have an organization name and country. - if (cert.subject().organization_names.empty() || - cert.subject().country_name.empty()) { - NOTREACHED(); - return std::wstring(); + if (!entry->ssl().Equals(original_ssl_status)) { + NotificationService::current()->Notify( + NotificationType::SSL_VISIBLE_STATE_CHANGED, + Source<NavigationController>(controller_), + NotificationService::NoDetails()); } - - return l10n_util::GetStringF(IDS_SECURE_CONNECTION_EV, - UTF8ToWide(cert.subject().organization_names[0]), - UTF8ToWide(cert.subject().country_name)); } diff --git a/chrome/browser/ssl/ssl_manager.h b/chrome/browser/ssl/ssl_manager.h index f9e1685..7d6c134 100644 --- a/chrome/browser/ssl/ssl_manager.h +++ b/chrome/browser/ssl/ssl_manager.h @@ -38,18 +38,6 @@ class URLRequest; class SSLManager : public NotificationObserver { public: - // Construct an SSLManager for the specified tab. - // If |delegate| is NULL, SSLPolicy::GetDefaultPolicy() is used. - explicit SSLManager(NavigationController* controller); - ~SSLManager(); - - SSLPolicy* policy() { return policy_.get(); } - SSLPolicyBackend* backend() { return &backend_; } - - // The navigation controller associated with this SSLManager. The - // NavigationController is guaranteed to outlive the SSLManager. - NavigationController* controller() { return controller_; } - static void RegisterUserPrefs(PrefService* prefs); // Entry point for SSLCertificateErrors. This function begins the process @@ -63,25 +51,9 @@ class SSLManager : public NotificationObserver { int cert_error, net::X509Certificate* cert); - // Mixed content entry points. - void DidDisplayInsecureContent(); - void DidRunInsecureContent(const std::string& security_origin); - - // Entry point for navigation. This function begins the process of updating - // the security UI when the main frame navigates to a new URL. - // - // Called on the UI thread. - virtual void Observe(NotificationType type, - const NotificationSource& source, - const NotificationDetails& details); - - // This entry point is called directly (instead of via the notification - // service) because we need more precise control of the order in which folks - // are notified of this event. - void DidCommitProvisionalLoad(const NotificationDetails& details); - - // Called to determine if there were any processed SSL errors from request. - bool ProcessedSSLErrorFromRequest() const; + // Called when SSL state for a host or tab changes. Broadcasts the + // SSL_INTERNAL_STATE_CHANGED notification. + static void NotifySSLInternalStateChanged(); // Convenience methods for serializing/deserializing the security info. static std::string SerializeSecurityInfo(int cert_id, @@ -95,6 +67,37 @@ class SSLManager : public NotificationObserver { // Returns "<organization_name> [<country>]". static std::wstring GetEVCertName(const net::X509Certificate& cert); + // Construct an SSLManager for the specified tab. + // If |delegate| is NULL, SSLPolicy::GetDefaultPolicy() is used. + explicit SSLManager(NavigationController* controller); + ~SSLManager(); + + SSLPolicy* policy() { return policy_.get(); } + SSLPolicyBackend* backend() { return &backend_; } + + // The navigation controller associated with this SSLManager. The + // NavigationController is guaranteed to outlive the SSLManager. + NavigationController* controller() { return controller_; } + + // This entry point is called directly (instead of via the notification + // service) because we need more precise control of the order in which folks + // are notified of this event. + void DidCommitProvisionalLoad(const NotificationDetails& details); + + // Mixed content entry point. + void DidRunInsecureContent(const std::string& security_origin); + + // Called to determine if there were any processed SSL errors from request. + bool ProcessedSSLErrorFromRequest() const; + + // Entry point for navigation. This function begins the process of updating + // the security UI when the main frame navigates to a new URL. + // + // Called on the UI thread. + virtual void Observe(NotificationType type, + const NotificationSource& source, + const NotificationDetails& details); + private: // SSLMessageInfo contains the information necessary for displaying a message // in an info-bar. @@ -130,9 +133,6 @@ class SSLManager : public NotificationObserver { void DidReceiveResourceRedirect(ResourceRedirectDetails* details); void DidChangeSSLInternalState(); - // Dispatch NotificationType::SSL_VISIBLE_STATE_CHANGED notification. - void DispatchSSLVisibleStateChanged(); - // Update the NavigationEntry with our current state. void UpdateEntry(NavigationEntry* entry); diff --git a/chrome/browser/ssl/ssl_policy.cc b/chrome/browser/ssl/ssl_policy.cc index 7335abc..4405972 100644 --- a/chrome/browser/ssl/ssl_policy.cc +++ b/chrome/browser/ssl/ssl_policy.cc @@ -83,15 +83,6 @@ void SSLPolicy::OnCertError(SSLCertErrorHandler* handler) { } } -void SSLPolicy::DidDisplayInsecureContent(NavigationEntry* entry) { - if (!entry) - return; - - // TODO(abarth): We don't actually need to break the whole origin here, - // but we can handle that in a later patch. - DidRunInsecureContent(entry, entry->url().spec()); -} - void SSLPolicy::DidRunInsecureContent(NavigationEntry* entry, const std::string& security_origin) { if (!entry) @@ -101,16 +92,52 @@ void SSLPolicy::DidRunInsecureContent(NavigationEntry* entry, if (!site_instance) return; - backend_->MarkHostAsBroken(GURL(security_origin).host(), - site_instance->GetProcess()->id()); + backend_->HostRanInsecureContent(GURL(security_origin).host(), + site_instance->GetProcess()->id()); } void SSLPolicy::OnRequestStarted(SSLRequestInfo* info) { - if (net::IsCertStatusError(info->ssl_cert_status())) - UpdateStateForUnsafeContent(info); + // TODO(abarth): This mechanism is wrong. What we should be doing is sending + // this information back through WebKit and out some FrameLoaderClient + // methods. + // + // The behavior for HTTPS resources with cert errors should be as follows: + // 1) If we don't know anything about this host (the one hosting the + // resource), the resource load just fails. + // 2) If the user has previously approved the same certificate error for + // this host in a full-page interstitial, then we'll proceed with the + // load. + // 3) If we proceed with the load, we should treat the resources as if they + // were loaded over HTTP, w.r.t. the display vs. run distinction above. + // + // However, right now we don't have the proper context to understand where + // these resources will be used. Consequently, we're conservative and treat + // them all like DidRunInsecureContent(). + + if (net::IsCertStatusError(info->ssl_cert_status())) { + backend_->HostRanInsecureContent(info->url().host(), info->child_id()); + + // TODO(abarth): We should eventually remove the main_frame_origin and + // frame_origin properties. First, not every resource load is associated + // with a frame, so they don't always make sense. Second, the + // main_frame_origin is computed from the first_party_for_cookies, which has + // been hacked to death to support third-party cookie blocking. + + if (info->resource_type() != ResourceType::MAIN_FRAME && + info->resource_type() != ResourceType::SUB_FRAME) { + // The frame's origin now contains mixed content and therefore is broken. + OriginRanInsecureContent(info->frame_origin(), info->child_id()); + } + + if (info->resource_type() != ResourceType::MAIN_FRAME) { + // The main frame now contains a frame with mixed content. Therefore, we + // mark the main frame's origin as broken too. + OriginRanInsecureContent(info->main_frame_origin(), info->child_id()); + } + } } -void SSLPolicy::UpdateEntry(NavigationEntry* entry) { +void SSLPolicy::UpdateEntry(NavigationEntry* entry, TabContents* tab_contents) { DCHECK(entry); InitializeEntryIfNeeded(entry); @@ -140,9 +167,15 @@ void SSLPolicy::UpdateEntry(NavigationEntry* entry) { // necessarily have site instances. Without a process, the entry can't // possibly have mixed content. See bug http://crbug.com/12423. if (site_instance && - backend_->DidMarkHostAsBroken(entry->url().host(), - site_instance->GetProcess()->id())) - entry->ssl().set_has_mixed_content(); + backend_->DidHostRunInsecureContent(entry->url().host(), + site_instance->GetProcess()->id())) { + entry->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); + entry->ssl().set_ran_mixed_content(); + return; + } + + if (tab_contents->displayed_insecure_content()) + entry->ssl().set_displayed_mixed_content(); } //////////////////////////////////////////////////////////////////////////////// @@ -207,33 +240,8 @@ void SSLPolicy::InitializeEntryIfNeeded(NavigationEntry* entry) { SECURITY_STYLE_AUTHENTICATED : SECURITY_STYLE_UNAUTHENTICATED); } -void SSLPolicy::MarkOriginAsBroken(const std::string& origin, int pid) { +void SSLPolicy::OriginRanInsecureContent(const std::string& origin, int pid) { GURL parsed_origin(origin); - if (!parsed_origin.SchemeIsSecure()) - return; - - backend_->MarkHostAsBroken(parsed_origin.host(), pid); -} - -void SSLPolicy::UpdateStateForMixedContent(SSLRequestInfo* info) { - // TODO(abarth): This function isn't right because we need to remove - // info->main_frame_origin(). - - if (info->resource_type() != ResourceType::MAIN_FRAME && - info->resource_type() != ResourceType::SUB_FRAME) { - // The frame's origin now contains mixed content and therefore is broken. - MarkOriginAsBroken(info->frame_origin(), info->child_id()); - } - - if (info->resource_type() != ResourceType::MAIN_FRAME) { - // The main frame now contains a frame with mixed content. Therefore, we - // mark the main frame's origin as broken too. - MarkOriginAsBroken(info->main_frame_origin(), info->child_id()); - } -} - -void SSLPolicy::UpdateStateForUnsafeContent(SSLRequestInfo* info) { - // This request as a broken cert, which means its host is broken. - backend_->MarkHostAsBroken(info->url().host(), info->child_id()); - UpdateStateForMixedContent(info); + if (parsed_origin.SchemeIsSecure()) + backend_->HostRanInsecureContent(parsed_origin.host(), pid); } diff --git a/chrome/browser/ssl/ssl_policy.h b/chrome/browser/ssl/ssl_policy.h index adb574f..8c8a5f8 100644 --- a/chrome/browser/ssl/ssl_policy.h +++ b/chrome/browser/ssl/ssl_policy.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -29,7 +29,6 @@ class SSLPolicy : public SSLBlockingPage::Delegate { // An error occurred with the certificate in an SSL connection. void OnCertError(SSLCertErrorHandler* handler); - void DidDisplayInsecureContent(NavigationEntry* entry); void DidRunInsecureContent(NavigationEntry* entry, const std::string& security_origin); @@ -37,7 +36,8 @@ class SSLPolicy : public SSLBlockingPage::Delegate { void OnRequestStarted(SSLRequestInfo* info); // Update the SSL information in |entry| to match the current state. - void UpdateEntry(NavigationEntry* entry); + // |tab_contents| is the TabContents associated with this entry. + void UpdateEntry(NavigationEntry* entry, TabContents* tab_contents); SSLPolicyBackend* backend() const { return backend_; } @@ -59,16 +59,8 @@ class SSLPolicy : public SSLBlockingPage::Delegate { // it with the default style for its URL. void InitializeEntryIfNeeded(NavigationEntry* entry); - // Mark |origin| as containing insecure content in the process with ID |pid|. - void MarkOriginAsBroken(const std::string& origin, int pid); - - // Called after we've decided that |info| represents a request for mixed - // content. Updates our internal state to reflect that we've loaded |info|. - void UpdateStateForMixedContent(SSLRequestInfo* info); - - // Called after we've decided that |info| represents a request for unsafe - // content. Updates our internal state to reflect that we've loaded |info|. - void UpdateStateForUnsafeContent(SSLRequestInfo* info); + // Mark |origin| as having run insecure content in the process with ID |pid|. + void OriginRanInsecureContent(const std::string& origin, int pid); // The backend we use to enact our decisions. SSLPolicyBackend* backend_; diff --git a/chrome/browser/ssl/ssl_policy_backend.cc b/chrome/browser/ssl/ssl_policy_backend.cc index db030ea..7674fca 100644 --- a/chrome/browser/ssl/ssl_policy_backend.cc +++ b/chrome/browser/ssl/ssl_policy_backend.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -11,7 +11,6 @@ #include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/navigation_entry.h" #include "chrome/browser/tab_contents/tab_contents.h" -#include "chrome/common/notification_service.h" #include "grit/generated_resources.h" #include "grit/theme_resources.h" @@ -103,14 +102,15 @@ void SSLPolicyBackend::ShowMessageWithLink(const std::wstring& msg, } } -void SSLPolicyBackend::MarkHostAsBroken(const std::string& host, int id) { - ssl_host_state_->MarkHostAsBroken(host, id); - DispatchSSLInternalStateChanged(); +void SSLPolicyBackend::HostRanInsecureContent(const std::string& host, + int id) { + ssl_host_state_->HostRanInsecureContent(host, id); + SSLManager::NotifySSLInternalStateChanged(); } -bool SSLPolicyBackend::DidMarkHostAsBroken(const std::string& host, - int pid) const { - return ssl_host_state_->DidMarkHostAsBroken(host, pid); +bool SSLPolicyBackend::DidHostRunInsecureContent(const std::string& host, + int pid) const { + return ssl_host_state_->DidHostRunInsecureContent(host, pid); } void SSLPolicyBackend::DenyCertForHost(net::X509Certificate* cert, @@ -128,13 +128,6 @@ net::X509Certificate::Policy::Judgment SSLPolicyBackend::QueryPolicy( return ssl_host_state_->QueryPolicy(cert, host); } -void SSLPolicyBackend::DispatchSSLInternalStateChanged() { - NotificationService::current()->Notify( - NotificationType::SSL_INTERNAL_STATE_CHANGED, - Source<NavigationController>(controller_), - NotificationService::NoDetails()); -} - void SSLPolicyBackend::ShowPendingMessages() { std::vector<SSLMessageInfo>::const_iterator iter; for (iter = pending_messages_.begin(); diff --git a/chrome/browser/ssl/ssl_policy_backend.h b/chrome/browser/ssl/ssl_policy_backend.h index 059534b..b795314 100644 --- a/chrome/browser/ssl/ssl_policy_backend.h +++ b/chrome/browser/ssl/ssl_policy_backend.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -31,13 +31,11 @@ class SSLPolicyBackend { const std::wstring& link_text, Task* task); - // Records that a host is "broken," that is, the origin for that host has been - // contaminated with insecure content, either via HTTP or via HTTPS with a - // bad certificate. - void MarkHostAsBroken(const std::string& host, int pid); + // Records that a host has run insecure content. + void HostRanInsecureContent(const std::string& host, int pid); - // Returns whether the specified host was marked as broken. - bool DidMarkHostAsBroken(const std::string& host, int pid) const; + // Returns whether the specified host ran insecure content. + bool DidHostRunInsecureContent(const std::string& host, int pid) const; // Records that |cert| is permitted to be used for |host| in the future. void DenyCertForHost(net::X509Certificate* cert, const std::string& host); @@ -80,9 +78,6 @@ class SSLPolicyBackend { Task* action; }; - // Dispatch NotificationType::SSL_INTERNAL_STATE_CHANGED notification. - void DispatchSSLInternalStateChanged(); - // The NavigationController that owns this SSLManager. We are responsible // for the security UI of this tab. NavigationController* controller_; diff --git a/chrome/browser/tab_contents/navigation_entry.h b/chrome/browser/tab_contents/navigation_entry.h index 7aaa5e0..833107c 100644 --- a/chrome/browser/tab_contents/navigation_entry.h +++ b/chrome/browser/tab_contents/navigation_entry.h @@ -37,8 +37,17 @@ class NavigationEntry { public: // Flags used for the page security content status. enum ContentStatusFlags { - NORMAL_CONTENT = 0, // No mixed content. - MIXED_CONTENT = 1 << 0, // https page containing http resources. + // HTTP page, or HTTPS page with no insecure content. + NORMAL_CONTENT = 0, + + // HTTPS page containing "displayed" HTTP resources (e.g. images, CSS). + DISPLAYED_MIXED_CONTENT = 1 << 0, + + // HTTPS page containing "executed" HTTP resources (i.e. script). + // Also currently used for HTTPS page containing broken-HTTPS resources; + // this is wrong and should be fixed (see comments in + // SSLPolicy::OnRequestStarted()). + RAN_MIXED_CONTENT = 1 << 1, }; SSLStatus(); @@ -79,13 +88,18 @@ class NavigationEntry { return security_bits_; } - // Mixed content means that this page which is served over https contains - // http sub-resources. - void set_has_mixed_content() { - content_status_ |= MIXED_CONTENT; + void set_displayed_mixed_content() { + content_status_ |= DISPLAYED_MIXED_CONTENT; + } + bool displayed_mixed_content() const { + return (content_status_ & DISPLAYED_MIXED_CONTENT) != 0; + } + + void set_ran_mixed_content() { + content_status_ |= RAN_MIXED_CONTENT; } - bool has_mixed_content() const { - return (content_status_ & MIXED_CONTENT) != 0; + bool ran_mixed_content() const { + return (content_status_ & RAN_MIXED_CONTENT) != 0; } // Raw accessors for all the content status flags. This contains a diff --git a/chrome/browser/tab_contents/navigation_entry_unittest.cc b/chrome/browser/tab_contents/navigation_entry_unittest.cc index 8fde535..ef37b11 100644 --- a/chrome/browser/tab_contents/navigation_entry_unittest.cc +++ b/chrome/browser/tab_contents/navigation_entry_unittest.cc @@ -101,23 +101,27 @@ TEST_F(NavigationEntryTest, NavigationEntrySSLStatus) { EXPECT_EQ(0, entry1_.get()->ssl().cert_id()); EXPECT_EQ(0, entry1_.get()->ssl().cert_status()); EXPECT_EQ(-1, entry1_.get()->ssl().security_bits()); - EXPECT_FALSE(entry1_.get()->ssl().has_mixed_content()); + EXPECT_FALSE(entry1_.get()->ssl().displayed_mixed_content()); + EXPECT_FALSE(entry1_.get()->ssl().ran_mixed_content()); // Change from the defaults entry2_.get()->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATED); entry2_.get()->ssl().set_cert_id(4); entry2_.get()->ssl().set_cert_status(1); entry2_.get()->ssl().set_security_bits(0); + entry2_.get()->ssl().set_displayed_mixed_content(); EXPECT_EQ(SECURITY_STYLE_AUTHENTICATED, entry2_.get()->ssl().security_style()); EXPECT_EQ(4, entry2_.get()->ssl().cert_id()); EXPECT_EQ(1, entry2_.get()->ssl().cert_status()); EXPECT_EQ(0, entry2_.get()->ssl().security_bits()); + EXPECT_TRUE(entry2_.get()->ssl().displayed_mixed_content()); - // Mixed content unaffected by unsafe content - EXPECT_FALSE(entry2_.get()->ssl().has_mixed_content()); - entry2_.get()->ssl().set_has_mixed_content(); - EXPECT_TRUE(entry2_.get()->ssl().has_mixed_content()); + entry2_.get()->ssl().set_security_style(SECURITY_STYLE_AUTHENTICATION_BROKEN); + entry2_.get()->ssl().set_ran_mixed_content(); + EXPECT_EQ(SECURITY_STYLE_AUTHENTICATION_BROKEN, + entry2_.get()->ssl().security_style()); + EXPECT_TRUE(entry2_.get()->ssl().ran_mixed_content()); } // Test other basic accessors diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc index 8b29480..1f7dd3d 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -270,6 +270,7 @@ TabContents::TabContents(Profile* profile, encoding_(), blocked_popups_(NULL), dont_notify_render_view_(false), + displayed_insecure_content_(false), infobar_delegates_(), find_ui_active_(false), find_op_aborted_(false), @@ -1582,6 +1583,10 @@ void TabContents::DidNavigateMainFramePostCommit( geolocation_settings_state_.DidNavigate(details); if (delegate_) delegate_->OnContentSettingsChange(this); + + // Once the main frame is navigated, we're no longer considered to have + // displayed insecure content. + displayed_insecure_content_ = false; } // Close constrained windows if necessary. @@ -2051,7 +2056,8 @@ void TabContents::DidLoadResourceFromMemoryCache( } void TabContents::DidDisplayInsecureContent() { - controller_.ssl_manager()->DidDisplayInsecureContent(); + displayed_insecure_content_ = true; + SSLManager::NotifySSLInternalStateChanged(); } void TabContents::DidRunInsecureContent(const std::string& security_origin) { diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 78465ec..c657d55 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -298,6 +298,10 @@ class TabContents : public PageNavigator, // navigation state change to trigger repaint of title. void SetAppIcon(const SkBitmap& app_icon); + bool displayed_insecure_content() const { + return displayed_insecure_content_; + } + // Internal state ------------------------------------------------------------ // This flag indicates whether the tab contents is currently being @@ -1135,6 +1139,9 @@ class TabContents : public PageNavigator, // Stores which content setting types actually have blocked content. bool content_blocked_[CONTENT_SETTINGS_NUM_TYPES]; + // True if this is a secure page which displayed insecure content. + bool displayed_insecure_content_; + // Data for shelves and stuff ------------------------------------------------ // Delegates for InfoBars associated with this TabContents. diff --git a/chrome/browser/toolbar_model.cc b/chrome/browser/toolbar_model.cc index c641539..61c121f 100644 --- a/chrome/browser/toolbar_model.cc +++ b/chrome/browser/toolbar_model.cc @@ -79,7 +79,7 @@ ToolbarModel::SecurityLevel ToolbarModel::GetSecurityLevel() const { return SECURITY_ERROR; case SECURITY_STYLE_AUTHENTICATED: - if (ssl.has_mixed_content()) + if (ssl.displayed_mixed_content()) return SECURITY_WARNING; if (net::IsCertStatusError(ssl.cert_status())) { DCHECK_EQ(ssl.cert_status() & net::CERT_STATUS_ALL_ERRORS, diff --git a/chrome/common/notification_type.h b/chrome/common/notification_type.h index ec2a858..f6fc5f1 100644 --- a/chrome/common/notification_type.h +++ b/chrome/common/notification_type.h @@ -131,16 +131,15 @@ class NotificationType { // Updating the SSL security indicators (the lock icon and such) proceeds // in two phases: // - // 1) An SSLManager changes the SSLHostState (which hangs off the profile - // object). When this happens, the SSLManager broadcasts an - // SSL_INTERNAL_STATE_CHANGED notification. + // 1) The internal SSL state for a host or tab changes. When this happens, + // the SSLManager broadcasts an SSL_INTERNAL_STATE_CHANGED notification. // // 2) The SSLManager for each tab receives this notification and might or // might not update the navigation entry for its tab, depending on - // whether the change in SSLHostState affects that tab. If the - // SSLManager does change the navigation entry, then the SSLManager - // broadcasts an SSL_VISIBLE_STATE_CHANGED notification to the user - // interface can redraw properly. + // whether the change in state affects that tab. If the SSLManager does + // change the navigation entry, then the SSLManager broadcasts an + // SSL_VISIBLE_STATE_CHANGED notification to the user interface can + // redraw properly. // The SSL state of a page has changed in some visible way. For example, // if an insecure resource is loaded on a secure page. Note that a @@ -149,9 +148,7 @@ class NotificationType { // case. Listen to this notification if you need to refresh SSL-related UI // elements. // - // The source will be the navigation controller associated with the load. - // There are no details. The entry changed will be the active entry of the - // controller. + // There is no source or details. SSL_VISIBLE_STATE_CHANGED, // The SSL state of the browser has changed in some internal way. For diff --git a/chrome/test/automation/automation_messages.h b/chrome/test/automation/automation_messages.h index ddd1c83..d2fff4b 100644 --- a/chrome/test/automation/automation_messages.h +++ b/chrome/test/automation/automation_messages.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -452,7 +452,8 @@ struct NavigationInfo { std::wstring title; GURL url; SecurityStyle security_style; - bool has_mixed_content; + bool displayed_mixed_content; + bool ran_mixed_content; }; // Traits for NavigationInfo structure to pack/unpack. @@ -466,7 +467,8 @@ struct ParamTraits<NavigationInfo> { WriteParam(m, p.title); WriteParam(m, p.url); WriteParam(m, p.security_style); - WriteParam(m, p.has_mixed_content); + WriteParam(m, p.displayed_mixed_content); + WriteParam(m, p.ran_mixed_content); } static bool Read(const Message* m, void** iter, param_type* p) { return ReadParam(m, iter, &p->navigation_type) && @@ -475,7 +477,8 @@ struct ParamTraits<NavigationInfo> { ReadParam(m, iter, &p->title) && ReadParam(m, iter, &p->url) && ReadParam(m, iter, &p->security_style) && - ReadParam(m, iter, &p->has_mixed_content); + ReadParam(m, iter, &p->displayed_mixed_content) && + ReadParam(m, iter, &p->ran_mixed_content); } static void Log(const param_type& p, std::wstring* l) { l->append(L"("); @@ -491,7 +494,9 @@ struct ParamTraits<NavigationInfo> { l->append(L", "); LogParam(p.security_style, l); l->append(L", "); - LogParam(p.has_mixed_content, l); + LogParam(p.displayed_mixed_content, l); + l->append(L", "); + LogParam(p.ran_mixed_content, l); l->append(L")"); } }; diff --git a/chrome/test/data/ssl/page_with_mixed_contents.html b/chrome/test/data/ssl/page_displays_mixed_content.html index b19730a..479fb3b 100644 --- a/chrome/test/data/ssl/page_with_mixed_contents.html +++ b/chrome/test/data/ssl/page_displays_mixed_content.html @@ -1,5 +1,5 @@ <html> -<head><title>Page with mixed contents</title> +<head><title>Page that displays mixed content</title> <script> function ImageWidth() { return document.getElementById("bad_image").width; diff --git a/chrome/test/data/ssl/page_with_http_script.html b/chrome/test/data/ssl/page_runs_mixed_content.html index d0719c4..9e8f18e 100644 --- a/chrome/test/data/ssl/page_with_http_script.html +++ b/chrome/test/data/ssl/page_runs_mixed_content.html @@ -1,5 +1,5 @@ <html> -<head><title>Page with http script</title></head> +<head><title>Page that runs mixed content</title></head> <body> This page contains an script which is served over an http connection, causing mixed contents (when this page is loaded over https).<br> diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc index aa8b07a..d289c54 100644 --- a/chrome_frame/chrome_active_document.cc +++ b/chrome_frame/chrome_active_document.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -642,8 +642,10 @@ void ChromeActiveDocument::UpdateNavigationState( bool is_title_changed = (navigation_info_.title != new_navigation_info.title); bool is_ssl_state_changed = (navigation_info_.security_style != new_navigation_info.security_style) || - (navigation_info_.has_mixed_content != - new_navigation_info.has_mixed_content); + (navigation_info_.displayed_mixed_content != + new_navigation_info.displayed_mixed_content) || + (navigation_info_.ran_mixed_content != + new_navigation_info.ran_mixed_content); if (is_ssl_state_changed) { int lock_status = SECURELOCK_SET_UNSECURE; @@ -652,7 +654,7 @@ void ChromeActiveDocument::UpdateNavigationState( lock_status = SECURELOCK_SET_SECUREUNKNOWNBIT; break; case SECURITY_STYLE_AUTHENTICATED: - lock_status = new_navigation_info.has_mixed_content ? + lock_status = new_navigation_info.displayed_mixed_content ? SECURELOCK_SET_MIXED : SECURELOCK_SET_SECUREUNKNOWNBIT; break; default: |