Commit d8675a0e authored by Lily Chen's avatar Lily Chen Committed by Commit Bot

Add heuristic for detecting cookie pairs for SameSite=None compatibility

This attempts to detect SameSite cookie "compatibility pairs" and
tag them with a new CookieInclusionStatus::WarningReason.

These are pairs of two similar cookies used to work around incompatible
clients (browsers which do not support SameSite=None), where one has
SameSite=None and Secure and the other has no SameSite attribute
specified. The intention is that browsers which support SameSite=None
and enforce SameSite-Lax-by-default will drop the old-style cookie,
while browsers which do not support SameSite=None will drop the
new-style cookie.

The heuristic used to detect such pairs of cookies is:
 - The cookies cannot be equivalent (same name, domain, path).
 - One must have SameSite=None and Secure, and the other must have
   unspecified SameSite.
 - They must have the same domain, path, and value.
 - One must have a name that is a prefix or suffix of the other's name,
   and the shorter of the two names must have length at least 3.

Such pairs of cookies are tagged if they are included in the same
cross-site access attempt via HTTP request, HTTP response, or
document.cookie read. Setting cookies via writing to document.cookie
does not result in tagging of compatibility pairs.

Additionally, a cookie access in a non-HTTP (i.e. script) context will
not tag or compute cookie pairs from any cookie that has the HttpOnly
attribute.

Bug: 1095192
Change-Id: I43075cb851e6e02a5d2ef3e443e63e13fb21bd4a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2243255
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: default avatarMaksim Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779942}
parent 823fa7fb
...@@ -244,6 +244,8 @@ std::string CookieInclusionStatus::GetDebugString() const { ...@@ -244,6 +244,8 @@ std::string CookieInclusionStatus::GetDebugString() const {
base::StrAppend(&out, {"WARN_LAX_CROSS_DOWNGRADE_STRICT_SAMESITE, "}); base::StrAppend(&out, {"WARN_LAX_CROSS_DOWNGRADE_STRICT_SAMESITE, "});
if (HasWarningReason(WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE)) if (HasWarningReason(WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE))
base::StrAppend(&out, {"WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE, "}); base::StrAppend(&out, {"WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE, "});
if (HasWarningReason(WARN_SAMESITE_COMPAT_PAIR))
base::StrAppend(&out, {"WARN_SAMESITE_COMPAT_PAIR, "});
// Strip trailing comma and space. // Strip trailing comma and space.
out.erase(out.end() - 2, out.end()); out.erase(out.end() - 2, out.end());
......
...@@ -117,6 +117,24 @@ class NET_EXPORT CookieInclusionStatus { ...@@ -117,6 +117,24 @@ class NET_EXPORT CookieInclusionStatus {
// Lax to Cross-site downgrade for an effective SameSite=Lax cookie. // Lax to Cross-site downgrade for an effective SameSite=Lax cookie.
WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE = 7, WARN_LAX_CROSS_DOWNGRADE_LAX_SAMESITE = 7,
// This is applied to a cookie that may be part of a "double cookie" pair
// used for compatibility reasons. These pairs consist of one cookie that
// has "SameSite=None; Secure" and a duplicate cookie that leaves SameSite
// unspecified to maintain compatibility with browsers that do not support
// the "SameSite=None" attribute. This warning is applied to both
// members of the pair. See cookie_util::IsSameSiteCompatPair().
//
// If computing this for a cookie access attempt from a non-network context
// (i.e. script), this should not be applied if either member of the pair is
// HttpOnly, to avoid leaking information about the name and value of
// HttpOnly cookies to an untrusted renderer.
//
// This is only relevant if WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT is
// present on the same status or a status for a cookie accessed at the same
// time, so it may not be applied at other times (e.g. when the context is
// same-site).
WARN_SAMESITE_COMPAT_PAIR = 8,
// This should be kept last. // This should be kept last.
NUM_WARNING_REASONS NUM_WARNING_REASONS
}; };
......
...@@ -556,6 +556,48 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForSubresource( ...@@ -556,6 +556,48 @@ CookieOptions::SameSiteCookieContext ComputeSameSiteContextForSubresource(
return CookieOptions::SameSiteCookieContext::MakeInclusive(); return CookieOptions::SameSiteCookieContext::MakeInclusive();
} }
bool IsSameSiteCompatPair(const CanonicalCookie& c1,
const CanonicalCookie& c2,
const CookieOptions& options) {
if (options.exclude_httponly() && (c1.IsHttpOnly() || c2.IsHttpOnly()))
return false;
if (c1.IsEquivalent(c2))
return false;
// One of them is SameSite=None and Secure; the other one has unspecified
// SameSite.
bool same_site_attributes_ok =
c1.SameSite() == CookieSameSite::NO_RESTRICTION && c1.IsSecure() &&
c2.SameSite() == CookieSameSite::UNSPECIFIED;
same_site_attributes_ok =
same_site_attributes_ok ||
(c2.SameSite() == CookieSameSite::NO_RESTRICTION && c2.IsSecure() &&
c1.SameSite() == CookieSameSite::UNSPECIFIED);
if (!same_site_attributes_ok)
return false;
if (c1.Domain() != c2.Domain() || c1.Path() != c2.Path() ||
c1.Value() != c2.Value()) {
return false;
}
DCHECK(c1.Name() != c2.Name());
std::string shorter, longer;
std::tie(shorter, longer) = (c1.Name().length() < c2.Name().length())
? std::tie(c1.Name(), c2.Name())
: std::tie(c2.Name(), c1.Name());
// One of them has a name that is a prefix or suffix of the other and has
// length at least 3 characters.
if (shorter.length() < kMinCompatPairNameLength)
return false;
if (base::StartsWith(longer, shorter, base::CompareCase::SENSITIVE) ||
base::EndsWith(longer, shorter, base::CompareCase::SENSITIVE)) {
return true;
}
return false;
}
bool IsSameSiteByDefaultCookiesEnabled() { bool IsSameSiteByDefaultCookiesEnabled() {
return base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies); return base::FeatureList::IsEnabled(features::kSameSiteByDefaultCookies);
} }
......
...@@ -28,6 +28,10 @@ const int kVlogPerCookieMonster = 1; ...@@ -28,6 +28,10 @@ const int kVlogPerCookieMonster = 1;
const int kVlogSetCookies = 7; const int kVlogSetCookies = 7;
const int kVlogGarbageCollection = 5; const int kVlogGarbageCollection = 5;
// Minimum name length for SameSite compatibility pair heuristic (see
// IsSameSiteCompatPair() below.)
const int kMinCompatPairNameLength = 3;
// This enum must match the numbering for StorageAccessResult in // This enum must match the numbering for StorageAccessResult in
// histograms/enums.xml. Do not reorder or remove items, only add new items // histograms/enums.xml. Do not reorder or remove items, only add new items
// at the end. // at the end.
...@@ -186,6 +190,26 @@ ComputeSameSiteContextForSubresource(const GURL& url, ...@@ -186,6 +190,26 @@ ComputeSameSiteContextForSubresource(const GURL& url,
const SiteForCookies& site_for_cookies, const SiteForCookies& site_for_cookies,
bool force_ignore_site_for_cookies); bool force_ignore_site_for_cookies);
// Evaluates a heuristic to determine whether |c1| and |c2| are likely to be a
// "double cookie" pair used for SameSite=None compatibility reasons.
//
// This returns true if all of the following are true:
// 1. The cookies are not equivalent (i.e. same name, domain, and path).
// 2. One of them is SameSite=None and Secure; the other one has unspecified
// SameSite.
// 3. Their domains are equal.
// 4. Their paths are equal.
// 5. Their values are equal.
// 6. One of them has a name that is a prefix or suffix of the other and has
// length at least 3 characters.
//
// |options| is the CookieOptions object used to access (get/set) the cookies.
// If the CookieOptions indicate that HttpOnly cookies are not allowed, this
// will return false if either of |c1| or |c2| is HttpOnly.
NET_EXPORT bool IsSameSiteCompatPair(const CanonicalCookie& c1,
const CanonicalCookie& c2,
const CookieOptions& options);
// Returns whether the respective SameSite feature is enabled. // Returns whether the respective SameSite feature is enabled.
NET_EXPORT bool IsSameSiteByDefaultCookiesEnabled(); NET_EXPORT bool IsSameSiteByDefaultCookiesEnabled();
NET_EXPORT bool IsCookiesWithoutSameSiteMustBeSecureEnabled(); NET_EXPORT bool IsCookiesWithoutSameSiteMustBeSecureEnabled();
......
...@@ -1236,6 +1236,139 @@ TEST(CookieUtilTest, AdaptCookieInclusionStatusToBool) { ...@@ -1236,6 +1236,139 @@ TEST(CookieUtilTest, AdaptCookieInclusionStatusToBool) {
EXPECT_TRUE(result_out); EXPECT_TRUE(result_out);
} }
TEST(CookieUtilTest, IsSameSiteCompatPair) {
ASSERT_EQ(3, cookie_util::kMinCompatPairNameLength)
<< "This test assumes that SameSite compatibility pairs have cookie name "
"length at least 3.";
GURL url("https://www.site.example/path");
struct {
const char* cookie_line_1;
const char* cookie_line_2;
bool expected_is_same_site_compat_pair;
} kTestCases[] = {
// Matching cases
{"name=value; SameSite=None; Secure", "name_legacy=value", true},
{"uid=value; SameSite=None; Secure", "uid_old=value", true},
{"name=value; SameSite=None; Secure", "name2=value; Secure", true},
{"name_samesite=value; SameSite=None; Secure", "name=value", true},
{"__Secure-name=value; SameSite=None; Secure", "name=value", true},
{"__Secure-3Pname=value; SameSite=None; Secure", "name=value", true},
{"name=value; SameSite=None; Secure; HttpOnly", "name_legacy=value",
true},
{"name=value; SameSite=None; Secure; Domain=site.example",
"name_legacy=value; Secure; Domain=site.example", true},
// Fails because cookies are equivalent
{"name=value; SameSite=None; Secure", "name=value", false},
// Fails SameSite criterion
{"name=value", "name_legacy=value", false},
{"name=value; SameSite=None", "name_legacy=value", false},
{"name=value; SameSite=None; Secure", "name_legacy=value; SameSite=None",
false},
{"name=value; SameSite=None; Secure",
"name_legacy=value; SameSite=None; Secure", false},
// Fails Domain criterion
{"name=value; SameSite=None; Secure; Domain=site.example",
"name_legacy=value", false},
{"name=value; SameSite=None; Secure; Domain=www.site.example",
"name_legacy=value", false},
{"name=value; SameSite=None; Secure",
"name_legacy=value; Domain=site.example", false},
{"name=value; SameSite=None; Secure",
"name_legacy=value; Domain=www.site.example", false},
// Fails Path criterion
{"name=value; SameSite=None; Secure; Path=/path", "name_legacy=value",
false},
{"name=value; SameSite=None; Secure; Path=/path",
"name_legacy=value; Path=/", false},
{"name=value; SameSite=None; Secure; Path=/",
"name_legacy=value; Path=/path", false},
{"name=value; SameSite=None; Secure", "name_legacy=value; Path=/path",
false},
// Fails value criterion
{"name=value; SameSite=None; Secure", "name_legacy=foobar", false},
{"name=value; SameSite=None; Secure", "name_legacy=value2", false},
// Fails name length criterion
{"id=value; SameSite=None; Secure", "id_legacy=value", false},
{"id_samesite=value; SameSite=None; Secure", "id=value", false},
{"value; SameSite=None; Secure", "legacy=value", false},
// Fails suffix/prefix criterion
{"name_samesite=value; SameSite=None; Secure", "name_legacy=value",
false},
{"name1=value; SameSite=None; Secure", "name2=value", false},
};
for (const auto& test_case : kTestCases) {
auto cookie1 = CanonicalCookie::Create(url, test_case.cookie_line_1,
base::Time::Now(), base::nullopt);
auto cookie2 = CanonicalCookie::Create(url, test_case.cookie_line_2,
base::Time::Now(), base::nullopt);
ASSERT_TRUE(cookie1);
ASSERT_TRUE(cookie2);
EXPECT_EQ(test_case.expected_is_same_site_compat_pair,
cookie_util::IsSameSiteCompatPair(
*cookie1, *cookie2, CookieOptions::MakeAllInclusive()));
EXPECT_EQ(test_case.expected_is_same_site_compat_pair,
cookie_util::IsSameSiteCompatPair(
*cookie2, *cookie1, CookieOptions::MakeAllInclusive()));
}
}
TEST(CookieUtilTest, IsSameSiteCompatPair_HttpOnly) {
GURL url("https://www.site.example/path");
auto new_cookie =
CanonicalCookie::Create(url, "name=value; SameSite=None; Secure",
base::Time::Now(), base::nullopt);
auto legacy_cookie = CanonicalCookie::Create(
url, "name_legacy=value", base::Time::Now(), base::nullopt);
auto http_only_new_cookie = CanonicalCookie::Create(
url, "name=value; SameSite=None; Secure; HttpOnly", base::Time::Now(),
base::nullopt);
auto http_only_legacy_cookie = CanonicalCookie::Create(
url, "name_legacy=value; HttpOnly", base::Time::Now(), base::nullopt);
ASSERT_TRUE(new_cookie);
ASSERT_TRUE(legacy_cookie);
ASSERT_TRUE(http_only_new_cookie);
ASSERT_TRUE(http_only_legacy_cookie);
// Allows HttpOnly access.
CookieOptions inclusive_options = CookieOptions::MakeAllInclusive();
// Disallows HttpOnly access.
CookieOptions restrictive_options;
// Allows SameSite but not HttpOnly access. (SameSite shouldn't matter.)
CookieOptions same_site_options;
same_site_options.set_same_site_cookie_context(
CookieOptions::SameSiteCookieContext::MakeInclusive());
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(*new_cookie, *legacy_cookie,
inclusive_options));
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *legacy_cookie, inclusive_options));
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(
*new_cookie, *http_only_legacy_cookie, inclusive_options));
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *http_only_legacy_cookie, inclusive_options));
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(*new_cookie, *legacy_cookie,
restrictive_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *legacy_cookie, restrictive_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*new_cookie, *http_only_legacy_cookie, restrictive_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *http_only_legacy_cookie, restrictive_options));
EXPECT_TRUE(cookie_util::IsSameSiteCompatPair(*new_cookie, *legacy_cookie,
same_site_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *legacy_cookie, same_site_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*new_cookie, *http_only_legacy_cookie, same_site_options));
EXPECT_FALSE(cookie_util::IsSameSiteCompatPair(
*http_only_new_cookie, *http_only_legacy_cookie, same_site_options));
}
} // namespace } // namespace
} // namespace net } // namespace net
...@@ -160,6 +160,58 @@ void RecordCTHistograms(const net::SSLInfo& ssl_info) { ...@@ -160,6 +160,58 @@ void RecordCTHistograms(const net::SSLInfo& ssl_info) {
} }
} }
template <typename CookieWithMetadata>
bool ShouldMarkSameSiteCompatPairs(
const std::vector<CookieWithMetadata>& cookie_list,
const net::CookieOptions& options) {
// If the context is same-site then there cannot be any SameSite-by-default
// warnings, so the compat pair warning is irrelevant.
if (options.same_site_cookie_context().GetContextForCookieInclusion() >
net::CookieOptions::SameSiteCookieContext::ContextType::
SAME_SITE_LAX_METHOD_UNSAFE) {
return false;
}
return cookie_list.size() >= 2;
}
void MarkSameSiteCompatPairs(
std::vector<net::CookieWithAccessResult>& cookie_list,
const net::CookieOptions& options) {
for (size_t i = 0; i < cookie_list.size() - 1; ++i) {
const net::CanonicalCookie& c1 = cookie_list[i].cookie;
for (size_t j = i + 1; j < cookie_list.size(); ++j) {
const net::CanonicalCookie& c2 = cookie_list[j].cookie;
if (net::cookie_util::IsSameSiteCompatPair(c1, c2, options)) {
cookie_list[i].access_result.status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
cookie_list[j].access_result.status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
}
}
}
}
void MarkSameSiteCompatPairs(
std::vector<net::CookieAndLineWithStatus>& cookie_list,
const net::CookieOptions& options) {
for (size_t i = 0; i < cookie_list.size() - 1; ++i) {
if (!cookie_list[i].cookie.has_value())
continue;
const net::CanonicalCookie& c1 = cookie_list[i].cookie.value();
for (size_t j = i + 1; j < cookie_list.size(); ++j) {
if (!cookie_list[j].cookie.has_value())
continue;
const net::CanonicalCookie& c2 = cookie_list[j].cookie.value();
if (net::cookie_util::IsSameSiteCompatPair(c1, c2, options)) {
cookie_list[i].status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
cookie_list[j].status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
}
}
}
}
} // namespace } // namespace
namespace net { namespace net {
...@@ -637,6 +689,11 @@ void URLRequestHttpJob::SetCookieHeaderAndStart( ...@@ -637,6 +689,11 @@ void URLRequestHttpJob::SetCookieHeaderAndStart(
} }
} }
// Mark the CookieInclusionStatuses of items in |maybe_sent_cookies| if they
// are part of a presumed SameSite compatibility pair.
if (ShouldMarkSameSiteCompatPairs(maybe_sent_cookies, options))
MarkSameSiteCompatPairs(maybe_sent_cookies, options);
request_->set_maybe_sent_cookies(std::move(maybe_sent_cookies)); request_->set_maybe_sent_cookies(std::move(maybe_sent_cookies));
StartTransaction(); StartTransaction();
...@@ -735,8 +792,14 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) { ...@@ -735,8 +792,14 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {
// loop has been exited. // loop has been exited.
num_cookie_lines_left_--; num_cookie_lines_left_--;
if (num_cookie_lines_left_ == 0) if (num_cookie_lines_left_ == 0) {
// Mark the CookieInclusionStatuses of items in |set_cookie_status_list_| if
// they are part of a presumed SameSite compatibility pair.
if (ShouldMarkSameSiteCompatPairs(set_cookie_status_list_, options))
MarkSameSiteCompatPairs(set_cookie_status_list_, options);
NotifyHeadersComplete(); NotifyHeadersComplete();
}
} }
void URLRequestHttpJob::OnSetCookieResult( void URLRequestHttpJob::OnSetCookieResult(
...@@ -762,8 +825,14 @@ void URLRequestHttpJob::OnSetCookieResult( ...@@ -762,8 +825,14 @@ void URLRequestHttpJob::OnSetCookieResult(
// If all the cookie lines have been handled, |set_cookie_status_list_| now // If all the cookie lines have been handled, |set_cookie_status_list_| now
// reflects the result of all Set-Cookie lines, and the request can be // reflects the result of all Set-Cookie lines, and the request can be
// continued. // continued.
if (num_cookie_lines_left_ == 0) if (num_cookie_lines_left_ == 0) {
// Mark the CookieInclusionStatuses of items in |set_cookie_status_list_| if
// they are part of a presumed SameSite compatibility pair.
if (ShouldMarkSameSiteCompatPairs(set_cookie_status_list_, options))
MarkSameSiteCompatPairs(set_cookie_status_list_, options);
NotifyHeadersComplete(); NotifyHeadersComplete();
}
} }
void URLRequestHttpJob::ProcessStrictTransportSecurityHeader() { void URLRequestHttpJob::ProcessStrictTransportSecurityHeader() {
......
...@@ -6914,6 +6914,200 @@ TEST_F(URLRequestTest, ReportCookieActivity) { ...@@ -6914,6 +6914,200 @@ TEST_F(URLRequestTest, ReportCookieActivity) {
} }
} }
// Test that CookieInclusionStatus warnings for SameSite cookie compatibility
// pairs are applied correctly.
TEST_F(URLRequestTest, SameSiteCookieCompatPairWarnings) {
EmbeddedTestServer https_test_server(EmbeddedTestServer::TYPE_HTTPS);
RegisterDefaultHandlers(&https_test_server);
ASSERT_TRUE(https_test_server.Start());
GURL set_pair_url = https_test_server.GetURL(
"/set-cookie?name=value;SameSite=None;Secure&"
"name_legacy=value");
GURL set_httponly_pair_url = https_test_server.GetURL(
"/set-cookie?name=value;SameSite=None;Secure;HttpOnly&"
"name_legacy=value;HttpOnly");
GURL set_pair_and_other_url = https_test_server.GetURL(
"/set-cookie?name=value;SameSite=None;Secure&"
"name_legacy=value&"
"name2=value;SameSite=None;Secure");
GURL set_two_pairs_url = https_test_server.GetURL(
"/set-cookie?name=value;SameSite=None;Secure&"
"name_legacy=value&"
"name2=value;SameSite=None;Secure&"
"compat-name2=value");
GURL get_cookies_url = https_test_server.GetURL("/echoheader?Cookie");
struct TestCase {
// URL used to set cookies.
// Note: This test works because each URL uses a superset of the cookies
// used by URLs before it.
GURL set_cookies_url;
// Names of cookies and whether they are expected to be included for a
// cross-site get/set. (All are expected for a same-site request.)
std::map<std::string, bool> expected_cookies;
// Names of cookies expected to have a compat pair warning for a cross-site
// request.
std::set<std::string> expected_compat_warning_cookies;
// Whether all cookies should be HttpOnly.
bool expected_httponly = false;
} kTestCases[] = {
// Basic case with a single compat pair.
{set_pair_url,
{{"name", true}, {"name_legacy", false}},
{"name", "name_legacy"}},
// Compat pair with HttpOnly cookies (should not change behavior because
// this is an HTTP request).
{set_httponly_pair_url,
{{"name", true}, {"name_legacy", false}},
{"name", "name_legacy"},
true},
// Pair should be marked, but extra cookie (not part of a pair) should
// not.
{set_pair_and_other_url,
{{"name", true}, {"name_legacy", false}, {"name2", true}},
{"name", "name_legacy"}},
// Two separate pairs should all be marked.
{set_two_pairs_url,
{{"name", true},
{"name_legacy", false},
{"name2", true},
{"compat-name2", false}},
{"name", "name_legacy", "name2", "compat-name2"}}};
// For each test case, this exercises:
// 1. Set cookies in a cross-site context to trigger compat pair warnings.
// 2. Set cookies in a same-site context and check that no compat pair
// warnings are applied (and also make sure the cookies are actually
// present for the subsequent tests).
// 3. Get cookies in a same-site context and check that no compat pair
// warnings are applied.
// 4. Get cookies in a cross-site context to trigger compat pair warnings.
for (const auto& test : kTestCases) {
{
// Set cookies in a cross-site context.
TestDelegate d;
std::unique_ptr<URLRequest> req(default_context().CreateRequest(
test.set_cookies_url, DEFAULT_PRIORITY, &d,
TRAFFIC_ANNOTATION_FOR_TESTS));
req->Start();
d.RunUntilComplete();
ASSERT_EQ(test.expected_cookies.size(),
req->maybe_stored_cookies().size());
for (const auto& cookie : req->maybe_stored_cookies()) {
ASSERT_TRUE(cookie.cookie.has_value());
EXPECT_EQ(cookie.cookie->IsHttpOnly(), test.expected_httponly);
const std::string& cookie_name = cookie.cookie->Name();
auto it = test.expected_cookies.find(cookie_name);
ASSERT_NE(test.expected_cookies.end(), it);
bool included = cookie.status.IsInclude();
EXPECT_EQ(it->second, included);
std::vector<CookieInclusionStatus::ExclusionReason> exclusions;
std::vector<CookieInclusionStatus::WarningReason> warnings;
if (!included) {
exclusions.push_back(CookieInclusionStatus::
EXCLUDE_SAMESITE_UNSPECIFIED_TREATED_AS_LAX);
warnings.push_back(CookieInclusionStatus::
WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT);
}
if (base::Contains(test.expected_compat_warning_cookies, cookie_name)) {
warnings.push_back(CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
}
EXPECT_TRUE(
cookie.status.HasExactlyExclusionReasonsForTesting(exclusions));
EXPECT_TRUE(cookie.status.HasExactlyWarningReasonsForTesting(warnings));
}
}
{
// Set cookies in a same-site context.
TestDelegate d;
std::unique_ptr<URLRequest> req(default_context().CreateFirstPartyRequest(
test.set_cookies_url, DEFAULT_PRIORITY, &d,
TRAFFIC_ANNOTATION_FOR_TESTS));
req->Start();
d.RunUntilComplete();
ASSERT_EQ(test.expected_cookies.size(),
req->maybe_stored_cookies().size());
for (const auto& cookie : req->maybe_stored_cookies()) {
ASSERT_TRUE(cookie.cookie.has_value());
EXPECT_EQ(cookie.cookie->IsHttpOnly(), test.expected_httponly);
EXPECT_TRUE(
base::Contains(test.expected_cookies, cookie.cookie->Name()));
// Cookie was included and there are no warnings.
EXPECT_EQ(CookieInclusionStatus(), cookie.status);
}
}
{
// Get cookies in a same-site context.
TestDelegate d;
std::unique_ptr<URLRequest> req(default_context().CreateFirstPartyRequest(
get_cookies_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
req->Start();
d.RunUntilComplete();
ASSERT_EQ(test.expected_cookies.size(), req->maybe_sent_cookies().size());
for (const auto& cookie : req->maybe_sent_cookies()) {
EXPECT_EQ(cookie.cookie.IsHttpOnly(), test.expected_httponly);
EXPECT_TRUE(
base::Contains(test.expected_cookies, cookie.cookie.Name()));
EXPECT_THAT(d.data_received(),
::testing::HasSubstr(cookie.cookie.Name() + "=value"));
// Cookie was included and there are no warnings.
EXPECT_EQ(CookieInclusionStatus(), cookie.access_result.status);
}
}
{
// Get cookies in a cross-site context.
TestDelegate d;
std::unique_ptr<URLRequest> req(default_context().CreateRequest(
get_cookies_url, DEFAULT_PRIORITY, &d, TRAFFIC_ANNOTATION_FOR_TESTS));
req->Start();
d.RunUntilComplete();
ASSERT_EQ(test.expected_cookies.size(), req->maybe_sent_cookies().size());
for (const auto& cookie : req->maybe_sent_cookies()) {
EXPECT_EQ(cookie.cookie.IsHttpOnly(), test.expected_httponly);
const std::string& cookie_name = cookie.cookie.Name();
auto it = test.expected_cookies.find(cookie_name);
ASSERT_NE(test.expected_cookies.end(), it);
bool included = cookie.access_result.status.IsInclude();
EXPECT_EQ(it->second, included);
if (included) {
EXPECT_THAT(d.data_received(),
::testing::HasSubstr(cookie.cookie.Name() + "=value"));
} else {
EXPECT_THAT(d.data_received(), ::testing::Not(::testing::HasSubstr(
cookie.cookie.Name() + "=value")));
}
std::vector<CookieInclusionStatus::ExclusionReason> exclusions;
std::vector<CookieInclusionStatus::WarningReason> warnings;
if (!included) {
exclusions.push_back(CookieInclusionStatus::
EXCLUDE_SAMESITE_UNSPECIFIED_TREATED_AS_LAX);
warnings.push_back(CookieInclusionStatus::
WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT);
}
if (base::Contains(test.expected_compat_warning_cookies, cookie_name)) {
warnings.push_back(CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
}
EXPECT_TRUE(
cookie.access_result.status.HasExactlyExclusionReasonsForTesting(
exclusions));
EXPECT_TRUE(
cookie.access_result.status.HasExactlyWarningReasonsForTesting(
warnings));
}
}
}
}
// Test that the SameSite-by-default CookieInclusionStatus warnings do not get // Test that the SameSite-by-default CookieInclusionStatus warnings do not get
// set if the cookie would have been rejected for other reasons. // set if the cookie would have been rejected for other reasons.
// Regression test for https://crbug.com/1027318. // Regression test for https://crbug.com/1027318.
......
...@@ -84,6 +84,31 @@ net::CookieOptions MakeOptionsForGet( ...@@ -84,6 +84,31 @@ net::CookieOptions MakeOptionsForGet(
return options; return options;
} }
void MarkSameSiteCompatPairs(std::vector<net::CookieWithStatus>& cookie_list,
const net::CookieOptions& options) {
// If the context is same-site then there cannot be any SameSite-by-default
// warnings, so the compat pair warning is irrelevant.
if (options.same_site_cookie_context().GetContextForCookieInclusion() >
net::CookieOptions::SameSiteCookieContext::ContextType::
SAME_SITE_LAX_METHOD_UNSAFE) {
return;
}
if (cookie_list.size() < 2)
return;
for (size_t i = 0; i < cookie_list.size() - 1; ++i) {
const net::CanonicalCookie& c1 = cookie_list[i].cookie;
for (size_t j = i + 1; j < cookie_list.size(); ++j) {
const net::CanonicalCookie& c2 = cookie_list[j].cookie;
if (net::cookie_util::IsSameSiteCompatPair(c1, c2, options)) {
cookie_list[i].status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
cookie_list[j].status.AddWarningReason(
net::CookieInclusionStatus::WARN_SAMESITE_COMPAT_PAIR);
}
}
}
}
} // namespace } // namespace
class RestrictedCookieManager::Listener : public base::LinkNode<Listener> { class RestrictedCookieManager::Listener : public base::LinkNode<Listener> {
...@@ -248,6 +273,11 @@ void RestrictedCookieManager::CookieListToGetAllForUrlCallback( ...@@ -248,6 +273,11 @@ void RestrictedCookieManager::CookieListToGetAllForUrlCallback(
// TODO(https://crbug.com/977040): Remove once samesite tightening up is // TODO(https://crbug.com/977040): Remove once samesite tightening up is
// rolled out. // rolled out.
// |result_with_status| is populated with excluded cookies here based on
// warnings present before WARN_SAMESITE_COMPAT_PAIR can be applied by
// MarkSameSiteCompatPairs(). This is ok because WARN_SAMESITE_COMPAT_PAIR is
// irrelevant unless WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT is already
// present.
for (const auto& cookie_and_access_result : excluded_cookies) { for (const auto& cookie_and_access_result : excluded_cookies) {
if (cookie_and_access_result.access_result.status.ShouldWarn()) { if (cookie_and_access_result.access_result.status.ShouldWarn()) {
result_with_status.push_back( result_with_status.push_back(
...@@ -288,6 +318,10 @@ void RestrictedCookieManager::CookieListToGetAllForUrlCallback( ...@@ -288,6 +318,10 @@ void RestrictedCookieManager::CookieListToGetAllForUrlCallback(
} }
if (cookie_observer_) { if (cookie_observer_) {
// Mark the CookieInclusionStatuses of items in |result_with_status| if they
// are part of a presumed SameSite compatibility pair.
MarkSameSiteCompatPairs(result_with_status, net_options);
cookie_observer_->OnCookiesAccessed(mojom::CookieAccessDetails::New( cookie_observer_->OnCookiesAccessed(mojom::CookieAccessDetails::New(
mojom::CookieAccessDetails::Type::kRead, url, site_for_cookies, mojom::CookieAccessDetails::Type::kRead, url, site_for_cookies,
result_with_status, base::nullopt)); result_with_status, base::nullopt));
......
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