Commit 5a1affdc authored by Steven Bingler's avatar Steven Bingler Committed by Commit Bot

Reland "Add field to SiteForCookies to indicate schemeful same-site of ancestors"

Fix and reland https://crrev.com/c/2133237 which was reverted in
https://crrev.com/c/2141207.

Add a new field to SiteforCookies which is used to indicate if |this|
would be the same when considered with schemeful same-site.

The field is updated in the browser and renderer when th computation for
a sub-frame's SiteForCookies is done.

This CL is groundwork for implementing schemeful same-site and thus
nothing is done with this new field yet.

Tbr:chlily@chromium.org,dcheng@chromium.org,morlovich@chromium.org

Bug: 1030938
Change-Id: Ie532e548a9842682170e8f97c7a44337013bbcbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142133
Commit-Queue: Steven Bingler <bingler@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Reviewed-by: default avatarLily Chen <chlily@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Reviewed-by: default avatarSteven Bingler <bingler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#757584}
parent e770b73c
......@@ -2304,6 +2304,7 @@ net::IsolationInfo RenderFrameHostImpl::ComputeIsolationInfoInternal(
return net::IsolationInfo::Create(redirect_mode, top_frame_origin,
frame_origin, net::SiteForCookies());
}
candidate_site_for_cookies.MarkIfCrossScheme(rfh->last_committed_origin_);
}
// If |redirect_mode| is kUpdateNothing, then IsolationInfo is being computed
......
......@@ -3315,6 +3315,84 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
SetBrowserClientForTesting(old_client);
}
// Test that when ancestor iframes differ in scheme that the SiteForCookies
// state is updated accordingly.
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
ComputeSiteForCookiesSchemefulIsSameForAncestorFrames) {
https_server()->ServeFilesFromSourceDirectory(GetTestDataFilePath());
https_server()->SetSSLConfig(net::EmbeddedTestServer::CERT_TEST_NAMES);
ASSERT_TRUE(https_server()->Start());
GURL url = https_server()->GetURL(
"a.test", "/cross_site_iframe_factory.html?a.test(a.test)");
GURL insecure_url = embedded_test_server()->GetURL(
"a.test", "/cross_site_iframe_factory.html?a.test(a.test(a.test))");
GURL other_url = https_server()->GetURL("c.test", "/");
EXPECT_TRUE(NavigateToURL(shell(), insecure_url));
{
WebContentsImpl* wc =
static_cast<WebContentsImpl*>(shell()->web_contents());
RenderFrameHostImpl* main_frame = wc->GetMainFrame();
EXPECT_EQ("a.test", main_frame->GetLastCommittedURL().host());
EXPECT_EQ("http", main_frame->frame_tree_node()->current_origin().scheme());
ASSERT_EQ(1u, main_frame->child_count());
FrameTreeNode* child = main_frame->child_at(0);
EXPECT_EQ("a.test", child->current_url().host());
EXPECT_EQ("http", child->current_origin().scheme());
ASSERT_EQ(1u, child->child_count());
FrameTreeNode* grandchild = child->child_at(0);
EXPECT_EQ("a.test", grandchild->current_url().host());
// Both the frames above grandchild are the same scheme, so
// SiteForCookies::schemefully_same() should indicate that.
EXPECT_TRUE(child->current_frame_host()
->ComputeSiteForCookiesForNavigation(other_url)
.schemefully_same());
EXPECT_EQ("a.test", child->current_frame_host()
->ComputeSiteForCookiesForNavigation(other_url)
.registrable_domain());
net::SiteForCookies grandchild_same_scheme =
grandchild->current_frame_host()->ComputeSiteForCookies();
EXPECT_TRUE(grandchild_same_scheme.schemefully_same());
EXPECT_EQ("a.test", grandchild_same_scheme.registrable_domain());
net::SiteForCookies grandchild_same_scheme_navigation =
grandchild->current_frame_host()->ComputeSiteForCookiesForNavigation(
other_url);
EXPECT_TRUE(grandchild_same_scheme_navigation.schemefully_same());
EXPECT_EQ("a.test", grandchild_same_scheme_navigation.registrable_domain());
// Navigate the middle child frame to https.
NavigateFrameToURL(child, url);
EXPECT_EQ("a.test", child->current_url().host());
EXPECT_EQ("https", child->current_origin().scheme());
EXPECT_EQ(1u, child->child_count());
grandchild = child->child_at(0);
// Now the frames above grandchild differ only in scheme. SiteForCookies
// should be the same except that schemefully_same() should be false.
net::SiteForCookies grandchild_cross_scheme =
grandchild->current_frame_host()->ComputeSiteForCookies();
EXPECT_FALSE(grandchild_cross_scheme.schemefully_same());
EXPECT_EQ("a.test", grandchild_cross_scheme.registrable_domain());
net::SiteForCookies grandchild_cross_scheme_navigation =
grandchild->current_frame_host()->ComputeSiteForCookiesForNavigation(
other_url);
EXPECT_FALSE(grandchild_cross_scheme_navigation.schemefully_same());
EXPECT_EQ("a.test",
grandchild_cross_scheme_navigation.registrable_domain());
// IsEquivalent() doesn't check schemefully_same.
EXPECT_TRUE(grandchild_cross_scheme.IsEquivalent(grandchild_same_scheme));
EXPECT_TRUE(grandchild_cross_scheme_navigation.IsEquivalent(
grandchild_same_scheme_navigation));
}
}
IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
ComputeSiteForCookiesForNavigationSandbox) {
// Test sandboxed subframe.
......
......@@ -20,7 +20,7 @@ std::string RegistrableDomainOrHost(const std::string& host) {
} // namespace
SiteForCookies::SiteForCookies() = default;
SiteForCookies::SiteForCookies() : schemefully_same_(false) {}
SiteForCookies::SiteForCookies(const SiteForCookies& other) = default;
SiteForCookies::SiteForCookies(SiteForCookies&& other) = default;
......@@ -35,6 +35,7 @@ SiteForCookies& SiteForCookies::operator=(SiteForCookies&& site_for_cookies) =
// static
bool SiteForCookies::FromWire(const std::string& scheme,
const std::string& registrable_domain,
bool schemefully_same,
SiteForCookies* out) {
// Make sure scheme meets precondition of methods like
// GURL::SchemeIsCryptographic.
......@@ -46,6 +47,8 @@ bool SiteForCookies::FromWire(const std::string& scheme,
if (registrable_domain != candidate.registrable_domain_)
return false;
candidate.schemefully_same_ = schemefully_same;
*out = std::move(candidate);
return true;
}
......@@ -65,8 +68,10 @@ SiteForCookies SiteForCookies::FromUrl(const GURL& url) {
}
std::string SiteForCookies::ToDebugString() const {
std::string same_scheme_string = schemefully_same_ ? "true" : "false";
return base::StrCat({"SiteForCookies: {scheme=", scheme_,
"; registrable_domain=", registrable_domain_, "}"});
"; registrable_domain=", registrable_domain_,
"; schemefully_same=", same_scheme_string, "}"});
}
bool SiteForCookies::IsFirstParty(const GURL& url) const {
......@@ -91,6 +96,40 @@ bool SiteForCookies::IsEquivalent(const SiteForCookies& other) const {
return registrable_domain_ == other.registrable_domain_;
}
void SiteForCookies::MarkIfCrossScheme(const url::Origin& other) {
// If |this| is IsNull() then |this| doesn't match anything which means that
// the scheme check is pointless. Also exit early if schemefully_same_ is
// already false.
if (IsNull() || !schemefully_same_)
return;
// Mark and return early if |other| is opaque. Opaque origins shouldn't match.
if (other.opaque()) {
schemefully_same_ = false;
return;
}
const std::string& other_scheme = other.scheme();
// Exact match case.
if (scheme_ == other_scheme)
return;
// ["https", "wss"] case.
if ((scheme_ == url::kHttpsScheme || scheme_ == url::kWssScheme) &&
(other_scheme == url::kHttpsScheme || other_scheme == url::kWssScheme)) {
return;
}
// ["http", "ws"] case.
if ((scheme_ == url::kHttpScheme || scheme_ == url::kWsScheme) &&
(other_scheme == url::kHttpScheme || other_scheme == url::kWsScheme)) {
return;
}
// The two are cross-scheme to each other.
schemefully_same_ = false;
}
GURL SiteForCookies::RepresentativeUrl() const {
if (IsNull())
return GURL();
......@@ -101,6 +140,8 @@ GURL SiteForCookies::RepresentativeUrl() const {
SiteForCookies::SiteForCookies(const std::string& scheme,
const std::string& host)
: scheme_(scheme), registrable_domain_(RegistrableDomainOrHost(host)) {}
: scheme_(scheme),
registrable_domain_(RegistrableDomainOrHost(host)),
schemefully_same_(!scheme.empty()) {}
} // namespace net
......@@ -23,12 +23,8 @@ namespace net {
// 2) They both have empty hostnames and equal schemes.
// Invalid URLs are not first party to anything.
//
// TODO(morlovich): It may make sense to require scheme to match in case (1)
// too, where the notion of matching makes http/https/ws/wss equivalent, but
// all other schemes are distinct.
//
// This should wait until SiteForCookies type is used everywhere relevant, so
// any changes are consistent.
// TODO(crbug.com/1030938): For case 1 the schemes must be "https" & "wss",
// "http" & "ws", or they must match exactly.
class NET_EXPORT SiteForCookies {
public:
// Matches nothing.
......@@ -50,6 +46,7 @@ class NET_EXPORT SiteForCookies {
// did not lie, merely that they are well-formed.
static bool FromWire(const std::string& scheme,
const std::string& registrable_domain,
bool schemefully_same,
SiteForCookies* out);
// If the origin is opaque, returns SiteForCookies that matches nothing.
......@@ -71,6 +68,13 @@ class NET_EXPORT SiteForCookies {
// as |this->IsFirstParty| (potentially none).
bool IsEquivalent(const SiteForCookies& other) const;
// Clears the schemefully_same_ flag if |other|'s scheme is cross-scheme to
// |this|.
// Two schemes are considered the same (not cross-scheme) if they exactly
// match, they are both in ["https", "wss"], or they are both in ["http",
// "ws"]. All other cases are cross-scheme.
void MarkIfCrossScheme(const url::Origin& other);
// Returns a URL that's first party to this SiteForCookies (an empty URL if
// none) --- that is, it has the property that
// site_for_cookies.IsEquivalent(
......@@ -87,6 +91,10 @@ class NET_EXPORT SiteForCookies {
const std::string& registrable_domain() const { return registrable_domain_; }
// Used for serialization/deserialization. This value is irrelevant if
// IsNull() is true.
bool schemefully_same() const { return schemefully_same_; }
// Returns true if this SiteForCookies matches nothing.
bool IsNull() const { return scheme_.empty(); }
......@@ -102,6 +110,25 @@ class NET_EXPORT SiteForCookies {
// just the bare hostname or IP, or an empty string if this represents
// something like file:///
std::string registrable_domain_;
// Used to indicate if the SiteForCookies would be the same if computed
// schemefully. A schemeful computation means to take the |scheme_| as well as
// the |registrable_domain_| into account when determining first-partyness.
// See MarkIfCrossScheme() for more information on scheme comparison.
//
// True means to treat |this| as-is while false means that |this| should be
// treated as if it matches nothing i.e. as if IsNull() returned true.
//
// This value is important in the case where the SiteForCookies is being used
// to assess the first-partyness of a sub-frame in a document.
//
// For a SiteForCookies with !scheme_.empty() this value starts as true and
// will only go false via MarkIfCrossScheme(), otherwise this value is
// irrelevant.
//
// TODO(https://crbug.com/1030938): Actually use this for decisions in other
// functions.
bool schemefully_same_;
};
} // namespace net
......
......@@ -70,8 +70,9 @@ TEST(SiteForCookiesTest, Default) {
SiteForCookies::FromOrigin(url::Origin())));
EXPECT_EQ("", should_match_none.scheme());
EXPECT_EQ("SiteForCookies: {scheme=; registrable_domain=}",
should_match_none.ToDebugString());
EXPECT_EQ(
"SiteForCookies: {scheme=; registrable_domain=; schemefully_same=false}",
should_match_none.ToDebugString());
}
TEST(SiteForCookiesTest, Basic) {
......@@ -134,8 +135,10 @@ TEST(SiteForCookiesTest, Blob) {
EXPECT_TRUE(from_blob.IsFirstParty(GURL("http://sub.example.org/resource")));
EXPECT_EQ("https", from_blob.scheme());
EXPECT_EQ("SiteForCookies: {scheme=https; registrable_domain=example.org}",
from_blob.ToDebugString());
EXPECT_EQ(
"SiteForCookies: {scheme=https; registrable_domain=example.org; "
"schemefully_same=true}",
from_blob.ToDebugString());
EXPECT_EQ("https://example.org/", from_blob.RepresentativeUrl().spec());
EXPECT_TRUE(from_blob.IsEquivalent(
SiteForCookies::FromUrl(GURL("http://www.example.org:631"))));
......@@ -145,28 +148,116 @@ TEST(SiteForCookiesTest, Wire) {
SiteForCookies out;
// Empty one.
EXPECT_TRUE(SiteForCookies::FromWire("", "", &out));
EXPECT_TRUE(SiteForCookies::FromWire("", "", false, &out));
EXPECT_TRUE(out.IsNull());
EXPECT_TRUE(SiteForCookies::FromWire("", "", true, &out));
EXPECT_TRUE(out.IsNull());
// Not a valid scheme.
EXPECT_FALSE(SiteForCookies::FromWire("aH", "example.com", &out));
EXPECT_FALSE(SiteForCookies::FromWire("aH", "example.com", false, &out));
EXPECT_TRUE(out.IsNull());
// Not a eTLD + 1 (or something hosty).
EXPECT_FALSE(SiteForCookies::FromWire("http", "sub.example.com", &out));
EXPECT_FALSE(
SiteForCookies::FromWire("http", "sub.example.com", false, &out));
EXPECT_TRUE(out.IsNull());
// This is fine, though.
EXPECT_TRUE(SiteForCookies::FromWire("https", "127.0.0.1", &out));
EXPECT_TRUE(SiteForCookies::FromWire("https", "127.0.0.1", true, &out));
EXPECT_FALSE(out.IsNull());
EXPECT_EQ(
"SiteForCookies: {scheme=https; registrable_domain=127.0.0.1; "
"schemefully_same=true}",
out.ToDebugString());
EXPECT_TRUE(SiteForCookies::FromWire("https", "127.0.0.1", false, &out));
EXPECT_FALSE(out.IsNull());
EXPECT_EQ("SiteForCookies: {scheme=https; registrable_domain=127.0.0.1}",
out.ToDebugString());
EXPECT_EQ(
"SiteForCookies: {scheme=https; registrable_domain=127.0.0.1; "
"schemefully_same=false}",
out.ToDebugString());
// As is actual eTLD+1.
EXPECT_TRUE(SiteForCookies::FromWire("wss", "example.com", &out));
EXPECT_TRUE(SiteForCookies::FromWire("wss", "example.com", true, &out));
EXPECT_FALSE(out.IsNull());
EXPECT_EQ("SiteForCookies: {scheme=wss; registrable_domain=example.com}",
out.ToDebugString());
EXPECT_EQ(
"SiteForCookies: {scheme=wss; registrable_domain=example.com; "
"schemefully_same=true}",
out.ToDebugString());
}
TEST(SiteForCookiesTest, SameScheme) {
struct TestCase {
const char* first;
const char* second;
bool expected_value;
};
const TestCase kTestCases[] = {
{"http://a.com", "http://a.com", true},
{"https://a.com", "https://a.com", true},
{"ws://a.com", "ws://a.com", true},
{"wss://a.com", "wss://a.com", true},
{"https://a.com", "wss://a.com", true},
{"wss://a.com", "https://a.com", true},
{"http://a.com", "ws://a.com", true},
{"ws://a.com", "http://a.com", true},
{"file://a.com", "file://a.com", true},
{"file://folder1/folder2/file.txt", "file://folder1/folder2/file.txt",
true},
{"ftp://a.com", "ftp://a.com", true},
{"http://a.com", "file://a.com", false},
{"ws://a.com", "wss://a.com", false},
{"wss://a.com", "ws://a.com", false},
{"https://a.com", "http://a.com", false},
{"file://a.com", "https://a.com", false},
{"https://a.com", "file://a.com", false},
{"file://a.com", "ftp://a.com", false},
{"ftp://a.com", "file://a.com", false},
};
for (const TestCase& t : kTestCases) {
SiteForCookies first = SiteForCookies::FromUrl(GURL(t.first));
url::Origin second = url::Origin::Create(GURL(t.second));
EXPECT_FALSE(first.IsNull());
first.MarkIfCrossScheme(second);
EXPECT_EQ(first.schemefully_same(), t.expected_value);
}
}
TEST(SiteForCookiesTest, SameSchemeOpaque) {
url::Origin not_opaque_secure =
url::Origin::Create(GURL("https://site.example"));
url::Origin not_opaque_nonsecure =
url::Origin::Create(GURL("http://site.example"));
// Check an opaque origin made from a triple origin and one from the default
// constructor.
const url::Origin kOpaqueOrigins[] = {
not_opaque_secure.DeriveNewOpaqueOrigin(),
not_opaque_nonsecure.DeriveNewOpaqueOrigin(), url::Origin()};
for (const url::Origin& origin : kOpaqueOrigins) {
SiteForCookies secure_sfc = SiteForCookies::FromOrigin(not_opaque_secure);
EXPECT_FALSE(secure_sfc.IsNull());
SiteForCookies nonsecure_sfc =
SiteForCookies::FromOrigin(not_opaque_nonsecure);
EXPECT_FALSE(nonsecure_sfc.IsNull());
EXPECT_TRUE(secure_sfc.schemefully_same());
secure_sfc.MarkIfCrossScheme(origin);
EXPECT_FALSE(secure_sfc.schemefully_same());
EXPECT_TRUE(nonsecure_sfc.schemefully_same());
nonsecure_sfc.MarkIfCrossScheme(origin);
EXPECT_FALSE(nonsecure_sfc.schemefully_same());
SiteForCookies opaque_sfc = SiteForCookies::FromOrigin(origin);
EXPECT_TRUE(opaque_sfc.IsNull());
// Slightly implementation detail specific as the value isn't relevant for
// null SFCs.
EXPECT_FALSE(nonsecure_sfc.schemefully_same());
}
}
} // namespace
......
......@@ -567,16 +567,21 @@ void ParamTraits<net::SiteForCookies>::Write(base::Pickle* m,
const param_type& p) {
WriteParam(m, p.scheme());
WriteParam(m, p.registrable_domain());
WriteParam(m, p.schemefully_same());
}
bool ParamTraits<net::SiteForCookies>::Read(const base::Pickle* m,
base::PickleIterator* iter,
param_type* r) {
std::string scheme, registrable_domain;
if (!ReadParam(m, iter, &scheme) || !ReadParam(m, iter, &registrable_domain))
bool schemefully_same;
if (!ReadParam(m, iter, &scheme) ||
!ReadParam(m, iter, &registrable_domain) ||
!ReadParam(m, iter, &schemefully_same))
return false;
return net::SiteForCookies::FromWire(scheme, registrable_domain, r);
return net::SiteForCookies::FromWire(scheme, registrable_domain,
schemefully_same, r);
}
void ParamTraits<net::SiteForCookies>::Log(const param_type& p,
......@@ -585,6 +590,8 @@ void ParamTraits<net::SiteForCookies>::Log(const param_type& p,
LogParam(p.scheme(), l);
l->append(",");
LogParam(p.registrable_domain(), l);
l->append(",");
LogParam(p.schemefully_same(), l);
l->append(")");
}
......
......@@ -16,7 +16,8 @@ bool StructTraits<network::mojom::SiteForCookiesDataView, net::SiteForCookies>::
if (!data.ReadRegistrableDomain(&registrable_domain))
return false;
return net::SiteForCookies::FromWire(scheme, registrable_domain, out);
return net::SiteForCookies::FromWire(scheme, registrable_domain,
data.schemefully_same(), out);
}
} // namespace mojo
......@@ -25,6 +25,10 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE)
return input.registrable_domain();
}
static bool schemefully_same(const net::SiteForCookies& input) {
return input.schemefully_same();
}
static bool Read(network::mojom::SiteForCookiesDataView data,
net::SiteForCookies* out);
};
......
......@@ -24,6 +24,7 @@ TEST(SiteForCookiesMojomTraitsTest, SerializeAndDeserialize) {
mojo::test::SerializeAndDeserialize<network::mojom::SiteForCookies>(
&original, &copied));
EXPECT_TRUE(original.IsEquivalent(copied));
EXPECT_EQ(original.schemefully_same(), copied.schemefully_same());
}
}
......
......@@ -10,4 +10,5 @@ struct SiteForCookies {
// net::SiteForCookies.
string scheme;
string registrable_domain;
bool schemefully_same;
};
......@@ -6146,12 +6146,13 @@ net::SiteForCookies Document::SiteForCookies() const {
const Frame* current_frame = GetFrame();
while (current_frame) {
const SecurityOrigin* cur_security_origin =
current_frame->GetSecurityContext()->GetSecurityOrigin();
if (!candidate.IsEquivalent(net::SiteForCookies::FromOrigin(
cur_security_origin->ToUrlOrigin()))) {
const url::Origin cur_security_origin =
current_frame->GetSecurityContext()->GetSecurityOrigin()->ToUrlOrigin();
if (!candidate.IsEquivalent(
net::SiteForCookies::FromOrigin(cur_security_origin))) {
return net::SiteForCookies();
}
candidate.MarkIfCrossScheme(cur_security_origin);
current_frame = current_frame->Tree().Parent();
}
......
......@@ -367,6 +367,15 @@ TEST_F(WebDocumentFirstPartyTest, NestedOriginA) {
OriginsEqual(g_nested_origin_a, NestedDocument()->TopFrameOrigin()));
}
TEST_F(WebDocumentFirstPartyTest, NestedOriginASchemefulSiteForCookies) {
Load(g_nested_origin_a);
// TopDocument is same scheme with itself so expect true.
ASSERT_TRUE(TopDocument()->SiteForCookies().schemefully_same());
// NestedDocument is same scheme with TopDocument so expect true.
ASSERT_TRUE(NestedDocument()->SiteForCookies().schemefully_same());
}
TEST_F(WebDocumentFirstPartyTest, NestedOriginSubA) {
Load(g_nested_origin_sub_a);
......@@ -395,6 +404,17 @@ TEST_F(WebDocumentFirstPartyTest, NestedOriginSecureA) {
NestedDocument()->TopFrameOrigin()));
}
TEST_F(WebDocumentFirstPartyTest, NestedOriginSecureASchemefulSiteForCookies) {
Load(g_nested_origin_secure_a);
// TopDocument is same scheme with itself so expect true.
ASSERT_TRUE(TopDocument()->SiteForCookies().schemefully_same());
// Since NestedDocument is secure, and the parent is insecure, the scheme will
// differ.
ASSERT_FALSE(NestedDocument()->SiteForCookies().schemefully_same());
}
TEST_F(WebDocumentFirstPartyTest, NestedOriginAInOriginA) {
Load(g_nested_origin_a_in_origin_a);
......
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