Skip to content

GitLab

T
tangled
Commit 2d2a3ba4 authored by Lukasz Anforowicz's avatar Lukasz Anforowicz Committed by Commit Bot
Browse files
Options

Cross-scheme initiator should result in `Sec-Fetch-Site: cross-site`. 

Before this CL, IsSameSite helper function in
services/network/sec_header_helpers.cc would only compare |initiator|'s
and |target_origin|'s hosts, which meant that a request with:
  - initiator = http://foo.example.com
  - target = https://foo.example.com
would be sent with:
  Sec-Fetch-Site: same-site

After this CL, IsSameSite also compares schemes (but not ports) to match
Site Isolation's notion of a site and so the request above will use:
  Sec-Fetch-Site: cross-site

Bug: 979257
Change-Id: I9b27a1118cdda8cfc02bf07ac697fc8d591d6b77
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1680096Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Reviewed-by: default avatarMike West <mkwst@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676649}
parent b00d4d15
Hide whitespace changes
Inline Side-by-side
Showing with 96 additions and 4 deletions
content/browser/frame_host/sec_fetch_browsertest.cc
View file @ 2d2a3ba4
......@@ -12,6 +12,7 @@
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/shell/browser/shell.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
......@@ -97,4 +98,34 @@ IN_PROC_BROWSER_TEST_F(SecFetchBrowserTest, TypedNavigation) {
}
}
// Verify that cross-port navigations are treated as same-site by
// Sec-Fetch-Site.
IN_PROC_BROWSER_TEST_F(SecFetchBrowserTest, CrossPortNavigation) {
net::EmbeddedTestServer server2(net::EmbeddedTestServer::TYPE_HTTPS);
server2.AddDefaultHandlers(GetTestDataFilePath());
server2.SetSSLConfig(net::EmbeddedTestServer::CERT_OK);
ASSERT_TRUE(server2.Start());
GURL initial_url = server2.GetURL("/title1.html");
GURL final_url = GetSecFetchUrl();
EXPECT_EQ(initial_url.scheme(), final_url.scheme());
EXPECT_EQ(initial_url.host(), final_url.host());
EXPECT_NE(initial_url.port(), final_url.port());
// Navigate to (paraphrasing): https://foo.com:port1/...
ASSERT_TRUE(NavigateToURL(shell(), initial_url));
// Navigate to (paraphrasing): https://foo.com:port2/... when the navigation
// is initiated from (paraphrasing): https://foo.com:port1/...
{
TestNavigationObserver nav_observer(shell()->web_contents());
ASSERT_TRUE(ExecJs(shell(), JsReplace("location = $1", final_url)));
nav_observer.Wait();
}
// Verify that https://foo.com:port1 is treated as same-site wrt
// https://foo.com:port2.
EXPECT_EQ("same-site", GetContent());
}
} // namespace content
services/network/sec_header_helpers.cc
View file @ 2d2a3ba4
......@@ -22,6 +22,11 @@ namespace {
bool IsSameSite(const url::Origin& initiator,
const url::Origin& target_origin) {
// Cross-scheme initiator should be considered cross-site (even if it's host
// is same-site with the target). See also https://crbug.com/979257.
if (initiator.scheme() != target_origin.scheme())
return false;
return net::registry_controlled_domains::SameDomainOrHost(
initiator, target_origin,
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
......
third_party/blink/web_tests/external/wpt/fetch/sec-metadata/fetch.tentative.sub.html 0 → 100644
View file @ 2d2a3ba4
<!DOCTYPE html>
<script src=/resources/testharness.js></script>
<script src=/resources/testharnessreport.js></script>
<script src=/fetch/sec-metadata/resources/helper.js></script>
<script>
// http -> https should see `Sec-Fetch-Site: cross-site`.
// This is a regression test for
// https://github.com/w3c/webappsec-fetch-metadata/issues/34
promise_test(t => {
assert_equals(location.protocol, "http:");
return fetch("https://{{host}}:{{ports[https][0]}}/fetch/sec-metadata/resources/echo-as-json.py")
.then(r => r.json())
.then(j => {
assert_header_equals(j, {
"dest": "empty",
"site": "cross-site",
"user": "",
"mode": "cors",
});
});
}, "http->https fetch (cross-scheme => cross-site)");
// http -> http should see no `Sec-Fetch-Site`.
promise_test(t => {
assert_equals(location.protocol, "http:");
return fetch("resources/echo-as-json.py")
.then(r => r.json())
.then(j => {
assert_header_equals(j, {
"dest": "",
"site": "",
"user": "",
"mode": "",
});
});
}, "http->http fetch (non-trustworthy destination => no sec-metadata)");
</script>
third_party/blink/web_tests/external/wpt/fetch/sec-metadata/iframe.tentative.sub.html
View file @ 2d2a3ba4
......@@ -60,4 +60,23 @@
document.body.appendChild(i);
}, "Non-secure cross-site iframe => No headers.");
async_test(t => {
let i = document.createElement('iframe');
i.src = "https://{{host}}:{{ports[https][0]}}/fetch/sec-metadata/resources/post-to-owner.py";
window.addEventListener('message', t.step_func(e => {
if (e.source != i.contentWindow)
return;
assert_header_equals(e.data, {
"dest": "nested-document",
"site": "cross-site",
"user": "",
"mode": "nested-navigate",
});
t.done();
}));
document.body.appendChild(i);
}, "Secure, cross-site (cross-scheme, same-host) iframe");
</script>
third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/multiple-redirect-https-downgrade-upgrade.tentative.sub.html
View file @ 2d2a3ba4
......@@ -19,7 +19,7 @@
<div id="fontTest">Downgraded then upgraded font</div>
<script>
let nonce = token();
let expected = { "dest": "", "site": "same-site", "user": "", "mode": "" };
let expected = { "dest": "", "site": "cross-site", "user": "", "mode": "" };
// Validate various scenarios handle a request that redirects from https => http
// correctly and avoids disclosure of any Sec- headers.
......@@ -52,7 +52,7 @@
// Note that we're using `undefined` here, as opposed to "" elsewhere because of the way
// that `image.py` encodes data.
"dest": undefined,
"site": "same-site",
"site": "cross-site",
"user": undefined,
"mode": undefined,
});
......
third_party/blink/web_tests/external/wpt/fetch/sec-metadata/redirect/redirect-http-upgrade.tentative.sub.html
View file @ 2d2a3ba4
......@@ -19,7 +19,7 @@
<div id="fontTest">Upgraded font</div>
<script>
let nonce = token();
let expected = { "dest": "", "site": "same-site", "user": "", "mode": "" };
let expected = { "dest": "", "site": "cross-site", "user": "", "mode": "" };
// Validate various scenarios handle a request that redirects from http => https correctly and add the proper Sec- headers.
RunCommonRedirectTests("Http upgrade", upgradeRedirectTo, expected);
......@@ -51,7 +51,7 @@
// Note that we're using `undefined` here, as opposed to "" elsewhere because of the way
// that `image.py` encodes data.
"dest": undefined,
"site": "same-site",
"site": "cross-site",
"user": undefined,
"mode": undefined,
});
......
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