Skip to content

GitLab

T
tangled
Commit 6e1d57f4 authored by Emily Stark's avatar Emily Stark Committed by Commit Bot
Browse files
Options

Handle blob URLs correctly for HTTP-Bad 

Previously, we were treating all blob: URLs as nonsecure for the purposes of
HTTP-Bad: namely, all blob: URLs would get a security level of
HTTP_SHOW_WARNING. Instead, we now use the origin of the blob: URL to determine
whether the security level should be HTTP_SHOW_WARNING (for nonsecure origins)
or NONE (for secure origins).

Bug: 797533
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I5400aaad91b55c3330d01c43bba5056b7351f723
Reviewed-on: https://chromium-review.googlesource.com/1031529
Commit-Queue: Emily Stark <estark@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554759}
parent 0e247539
Hide whitespace changes
Inline Side-by-side
Showing with 64 additions and 35 deletions
chrome/browser/ssl/security_state_tab_helper_browsertest.cc
View file @ 6e1d57f4
...@@ -76,6 +76,24 @@ ...@@ -76,6 +76,24 @@
namespace { namespace {
const char kCreateFilesystemUrlJavascript[] =
"window.webkitRequestFileSystem(window.TEMPORARY, 4096, function(fs) {"
" fs.root.getFile('test.html', {create: true}, function(fileEntry) {"
" fileEntry.createWriter(function(writer) {"
" writer.onwriteend = function(e) {"
" window.domAutomationController.send(fileEntry.toURL());"
" };"
" var blob = new Blob(['<html>hello</html>'], {type: 'text/html'});"
" writer.write(blob);"
" });"
" });"
"});";
const char kCreateBlobUrlJavascript[] =
"var blob = new Blob(['<html>hello</html>'],"
" {type: 'text/html'});"
"window.domAutomationController.send(URL.createObjectURL(blob));";
enum CertificateStatus { VALID_CERTIFICATE, INVALID_CERTIFICATE }; enum CertificateStatus { VALID_CERTIFICATE, INVALID_CERTIFICATE };
const base::FilePath::CharType kDocRoot[] = const base::FilePath::CharType kDocRoot[] =
...@@ -443,9 +461,11 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest { ...@@ -443,9 +461,11 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest {
} }
// Navigates to an empty page and runs |javascript| to create a URL with with // Navigates to an empty page and runs |javascript| to create a URL with with
// a scheme of |scheme|. Expects a security level of HTTP_SHOW_WARNING. // a scheme of |scheme|. Expects a security level of NONE if
// |use_secure_inner_origin| is true and HTTP_SHOW_WARNING otherwise.
void TestBlobOrFilesystemURL(const std::string& scheme, void TestBlobOrFilesystemURL(const std::string& scheme,
const std::string& javascript) { const std::string& javascript,
bool use_secure_inner_origin) {
content::WebContents* contents = content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents(); browser()->tab_strip_model()->GetActiveWebContents();
ASSERT_TRUE(contents); ASSERT_TRUE(contents);
...@@ -456,7 +476,9 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest { ...@@ -456,7 +476,9 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest {
ui_test_utils::NavigateToURL( ui_test_utils::NavigateToURL(
browser(), browser(),
GetURLWithNonLocalHostname(embedded_test_server(), "/empty.html")); GetURLWithNonLocalHostname(
use_secure_inner_origin ? &https_server_ : embedded_test_server(),
"/empty.html"));
// Create a URL and navigate to it. // Create a URL and navigate to it.
std::string blob_or_filesystem_url; std::string blob_or_filesystem_url;
...@@ -473,7 +495,9 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest { ...@@ -473,7 +495,9 @@ class SecurityStateTabHelperTest : public CertVerifierBrowserTest {
contents->GetController().GetVisibleEntry(); contents->GetController().GetVisibleEntry();
ASSERT_TRUE(entry); ASSERT_TRUE(entry);
EXPECT_EQ(security_state::HTTP_SHOW_WARNING, security_info.security_level); EXPECT_EQ(use_secure_inner_origin ? security_state::NONE
: security_state::HTTP_SHOW_WARNING,
security_info.security_level);
} }
net::EmbeddedTestServer https_server_; net::EmbeddedTestServer https_server_;
...@@ -1267,42 +1291,33 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, ...@@ -1267,42 +1291,33 @@ IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
// Tests the default security level on blob URLs. // Tests the default security level on blob URLs.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
DefaultSecurityLevelOnBlobUrl) { DefaultSecurityLevelOnBlobUrl) {
base::test::ScopedFeatureList scoped_feature_list; TestBlobOrFilesystemURL("blob", kCreateBlobUrlJavascript,
scoped_feature_list.InitAndEnableFeatureWithParameters( false /* use_secure_inner_origin */);
security_state::features::kMarkHttpAsFeature,
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterWarning}});
TestBlobOrFilesystemURL(
"blob",
"var blob = new Blob(['<html>hello</html>'],"
" {type: 'text/html'});"
"window.domAutomationController.send(URL.createObjectURL(blob));");
} }
// Same as DefaultSecurityLevelOnBlobUrl, but instead of a blob URL, // Same as DefaultSecurityLevelOnBlobUrl, but instead of a blob URL,
// this creates a filesystem URL. // this creates a filesystem URL.
IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest, IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
DefaultSecurityLevelOnFilesystemUrl) { DefaultSecurityLevelOnFilesystemUrl) {
base::test::ScopedFeatureList scoped_feature_list; TestBlobOrFilesystemURL("filesystem", kCreateFilesystemUrlJavascript,
scoped_feature_list.InitAndEnableFeatureWithParameters( false /* use_secure_inner_origin */);
security_state::features::kMarkHttpAsFeature, }
{{security_state::features::kMarkHttpAsFeatureParameterName,
security_state::features::kMarkHttpAsParameterWarning}}); // Tests the default security level on blob URLs with a secure inner origin.
TestBlobOrFilesystemURL( IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
"filesystem", DefaultSecurityLevelOnSecureBlobUrl) {
"window.webkitRequestFileSystem(window.TEMPORARY, 4096, function(fs) {" SetUpMockCertVerifierForHttpsServer(0, net::OK);
" fs.root.getFile('test.html', {create: true}, function(fileEntry) {" TestBlobOrFilesystemURL("blob", kCreateBlobUrlJavascript,
" fileEntry.createWriter(function(writer) {" true /* use_secure_inner_origin */);
" writer.onwriteend = function(e) {" }
" window.domAutomationController.send(fileEntry.toURL());"
" };" // Same as DefaultSecurityLevelOnBlobUrl, but instead of a blob URL,
" var blob =" // this creates a filesystem URL with a secure inner origin.
" new Blob(['<html>hello</html>']," IN_PROC_BROWSER_TEST_F(SecurityStateTabHelperTest,
" {type: 'text/html'});" DefaultSecurityLevelOnSecureFilesystemUrl) {
" writer.write(blob);" SetUpMockCertVerifierForHttpsServer(0, net::OK);
" });" TestBlobOrFilesystemURL("filesystem", kCreateFilesystemUrlJavascript,
" });" true /* use_secure_inner_origin */);
"});");
} }
// Tests that when an invisible password field is present on an HTTP page load, // Tests that when an invisible password field is present on an HTTP page load,
......
components/security_state/core/security_state.cc
View file @ 6e1d57f4
...@@ -19,6 +19,18 @@ namespace security_state { ...@@ -19,6 +19,18 @@ namespace security_state {
namespace { namespace {
// Returns true if |url| is a blob: URL and its path parses as a GURL with a
// nonsecure origin, and false otherwise. See
// https://url.spec.whatwg.org/#origin.
bool IsNonsecureBlobUrl(
const GURL& url,
const IsOriginSecureCallback& is_origin_secure_callback) {
if (!url.SchemeIs(url::kBlobScheme))
return false;
GURL inner_url(url.path());
return !is_origin_secure_callback.Run(inner_url);
}
// For nonsecure pages, sets |security_level| in |*security_info| based on the // For nonsecure pages, sets |security_level| in |*security_info| based on the
// provided information and the kMarkHttpAsFeature field trial. Also sets the // provided information and the kMarkHttpAsFeature field trial. Also sets the
// explanatory fields |incognito_downgraded_security_level| and // explanatory fields |incognito_downgraded_security_level| and
...@@ -152,7 +164,8 @@ void SetSecurityLevelAndRelatedFields( ...@@ -152,7 +164,8 @@ void SetSecurityLevelAndRelatedFields(
if (!is_cryptographic_with_certificate) { if (!is_cryptographic_with_certificate) {
if (!visible_security_state.is_error_page && if (!visible_security_state.is_error_page &&
!is_origin_secure_callback.Run(url) && !is_origin_secure_callback.Run(url) &&
(url.IsStandard() || url.SchemeIs(url::kBlobScheme))) { (url.IsStandard() ||
IsNonsecureBlobUrl(url, is_origin_secure_callback))) {
SetSecurityLevelAndRelatedFieldsForNonSecureFieldTrial( SetSecurityLevelAndRelatedFieldsForNonSecureFieldTrial(
visible_security_state.is_incognito, visible_security_state.is_incognito,
visible_security_state.is_error_page, visible_security_state.is_error_page,
......
testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
View file @ 6e1d57f4
...@@ -252,6 +252,7 @@ ...@@ -252,6 +252,7 @@
# Fails because of missing support to navigate to filesystem: URLs. # Fails because of missing support to navigate to filesystem: URLs.
# https://crbug.com/797292 # https://crbug.com/797292
-ProcessManagerBrowserTest.NestedURLNavigationsToExtensionBlocked -ProcessManagerBrowserTest.NestedURLNavigationsToExtensionBlocked
-SecurityStateTabHelperTest.DefaultSecurityLevelOnSecureFilesystemUrl
# Flaky in linux tests. # Flaky in linux tests.
# https://crbug.com/820995 # https://crbug.com/820995
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment