Skip to content

GitLab

T
tangled
Commit 5034ff12 authored by Maks Orlovich's avatar Maks Orlovich Committed by Commit Bot
Browse files
Options

HttpAuthRelationTypeOf: fix misunderstanding of null site_for_cookies, and simplify 

A null site_for_cookies means the containing frame is already cross-site, not that
it's top-level. (And it's impossible to see from it whether it's same-site with
a resource --- for purposes of cookies everything below is third-party)

Also simplify the function to not distinguish 5 cases when it's only used
to tell 2 apart.

Bug: 1006328
Change-Id: I41d46e990fe9afe78ba502a76b85621dce74d8b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1932088Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarCarlos IL <carlosil@chromium.org>
Commit-Queue: Maksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719374}
parent b5fb1575
Hide whitespace changes
Inline Side-by-side
Showing with 74 additions and 24 deletions
chrome/browser/ui/login/login_handler_browsertest.cc
View file @ 5034ff12
...@@ -905,6 +905,64 @@ IN_PROC_BROWSER_TEST_P(LoginPromptBrowserTest, ...@@ -905,6 +905,64 @@ IN_PROC_BROWSER_TEST_P(LoginPromptBrowserTest,
EXPECT_EQ(1, observer.auth_needed_count()); EXPECT_EQ(1, observer.auth_needed_count());
} }
// Deep cross-domain image login prompting should be blocked, too.
IN_PROC_BROWSER_TEST_P(LoginPromptBrowserTest,
BlockDeepCrossdomainPromptForSubresources) {
const char kTestPage[] = "/iframe_login_load_img_from_b.html";
ASSERT_TRUE(embedded_test_server()->Start());
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();
NavigationController* controller = &contents->GetController();
LoginPromptBrowserTestObserver observer;
observer.Register(content::Source<NavigationController>(controller));
// b.com is iframe'd under 127.0.0.1 and includes an image. This is still
// cross-domain.
{
GURL test_page = embedded_test_server()->GetURL(kTestPage);
ASSERT_EQ("127.0.0.1", test_page.host());
WindowedLoadStopObserver load_stop_waiter(controller, 1);
browser()->OpenURL(OpenURLParams(test_page, Referrer(),
WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, false));
load_stop_waiter.Wait();
}
EXPECT_EQ(0, observer.auth_needed_count());
// b.com iframe'd under b.com and includes an image.
{
GURL test_page = embedded_test_server()->GetURL(kTestPage);
ASSERT_EQ("127.0.0.1", test_page.host());
// Change the host from 127.0.0.1 to www.b.com so that when the
// page tries to load from b, it will be same-origin.
GURL::Replacements replacements;
replacements.SetHostStr("www.b.com");
test_page = test_page.ReplaceComponents(replacements);
WindowedAuthNeededObserver auth_needed_waiter(controller);
browser()->OpenURL(OpenURLParams(test_page, Referrer(),
WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_TYPED, false));
auth_needed_waiter.Wait();
ASSERT_EQ(1u, observer.handlers().size());
while (!observer.handlers().empty()) {
WindowedAuthCancelledObserver auth_cancelled_waiter(controller);
LoginHandler* handler = *observer.handlers().begin();
ASSERT_TRUE(handler);
handler->CancelAuth();
auth_cancelled_waiter.Wait();
}
}
EXPECT_EQ(1, observer.auth_needed_count());
}
// Block same domain image resource if the top level frame is HTTPS and the // Block same domain image resource if the top level frame is HTTPS and the
// image resource is HTTP. // image resource is HTTP.
// E.g. Top level: https://example.com, Image resource: http://example.com/image // E.g. Top level: https://example.com, Image resource: http://example.com/image
......
chrome/test/data/iframe_login_load_img_from_b.html 0 → 100644
View file @ 5034ff12
<html><head><title>Loads an iframe from b.com that then loads an image</title></head>
<body>
<script>
var f = document.createElement("iframe");
f.src = "http://www.b.com:" + location.port + "/login/load_img_from_b.html";
document.body.appendChild(f);
</script>
</body></html>
content/renderer/loader/web_url_loader_impl.cc
View file @ 5034ff12
...@@ -297,24 +297,11 @@ void SetSecurityStyleAndDetails(const GURL& url, ...@@ -297,24 +297,11 @@ void SetSecurityStyleAndDetails(const GURL& url,
response->SetSecurityDetails(webSecurityDetails); response->SetSecurityDetails(webSecurityDetails);
} }
// Relationship of resource being authenticated with the top level page. bool IsBannedCrossSiteAuth(network::ResourceRequest* resource_request,
enum HttpAuthRelationType { const WebURLRequest& request) {
HTTP_AUTH_RELATION_TOP, // Top-level page itself
HTTP_AUTH_RELATION_SAME_DOMAIN, // Sub-content from same domain
HTTP_AUTH_RELATION_BLOCKED_CROSS, // Blocked Sub-content from cross domain
HTTP_AUTH_RELATION_ALLOWED_CROSS, // Allowed Sub-content per command line
HTTP_AUTH_RELATION_LAST
};
HttpAuthRelationType HttpAuthRelationTypeOf(
network::ResourceRequest* resource_request,
const WebURLRequest& request) {
auto& request_url = resource_request->url; auto& request_url = resource_request->url;
auto& first_party = resource_request->site_for_cookies; auto& first_party = resource_request->site_for_cookies;
if (!first_party.is_valid())
return HTTP_AUTH_RELATION_TOP;
bool allow_cross_origin_auth_prompt = false; bool allow_cross_origin_auth_prompt = false;
if (request.GetExtraData()) { if (request.GetExtraData()) {
RequestExtraData* extra_data = RequestExtraData* extra_data =
...@@ -323,22 +310,20 @@ HttpAuthRelationType HttpAuthRelationTypeOf( ...@@ -323,22 +310,20 @@ HttpAuthRelationType HttpAuthRelationTypeOf(
extra_data->allow_cross_origin_auth_prompt(); extra_data->allow_cross_origin_auth_prompt();
} }
if (net::registry_controlled_domains::SameDomainOrHost( if (first_party.is_valid() && // empty site_for_cookies means cross-site.
net::registry_controlled_domains::SameDomainOrHost(
first_party, request_url, first_party, request_url,
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) { net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
// If the first party is secure but the subresource is not, this is // If the first party is secure but the subresource is not, this is
// mixed-content. Do not allow the image. // mixed-content. Do not allow the image.
if (!allow_cross_origin_auth_prompt && IsOriginSecure(first_party) && if (!allow_cross_origin_auth_prompt && IsOriginSecure(first_party) &&
!IsOriginSecure(request_url)) { !IsOriginSecure(request_url)) {
return HTTP_AUTH_RELATION_BLOCKED_CROSS; return true;
} }
return HTTP_AUTH_RELATION_SAME_DOMAIN; return false;
} }
if (allow_cross_origin_auth_prompt) return !allow_cross_origin_auth_prompt;
return HTTP_AUTH_RELATION_ALLOWED_CROSS;
return HTTP_AUTH_RELATION_BLOCKED_CROSS;
} }
} // namespace } // namespace
...@@ -721,8 +706,7 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request, ...@@ -721,8 +706,7 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request,
if (resource_request->resource_type == if (resource_request->resource_type ==
static_cast<int>(ResourceType::kImage) && static_cast<int>(ResourceType::kImage) &&
HTTP_AUTH_RELATION_BLOCKED_CROSS == IsBannedCrossSiteAuth(resource_request.get(), request)) {
HttpAuthRelationTypeOf(resource_request.get(), request)) {
// Prevent third-party image content from prompting for login, as this // Prevent third-party image content from prompting for login, as this
// is often a scam to extract credentials for another domain from the // is often a scam to extract credentials for another domain from the
// user. Only block image loads, as the attack applies largely to the // user. Only block image loads, as the attack applies largely to the
......
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