Skip to content

Commit

Permalink
Compute cookie partition key when ingesting cookies from the Set-Cookie
Browse files Browse the repository at this point in the history
header.

We only ingest partition keys if the --partitioned-cookies flag is set.
Otherwise we will treat the cookie as unpartitioned.

Bug: 1225444
Change-Id: I138a80d1bac0197c8268b56e7527aac0d757b8fa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3069119
Commit-Queue: Dylan Cutler <dylancutler@google.com>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Shuran Huang <shuuran@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910528}
  • Loading branch information
DCtheTall authored and Chromium LUCI CQ committed Aug 10, 2021
1 parent 40352f2 commit 39fa227
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 3 deletions.
15 changes: 15 additions & 0 deletions net/cookies/cookie_partition_key.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "net/cookies/cookie_partition_key.h"

#include "base/feature_list.h"
#include "net/base/features.h"
#include "net/cookies/cookie_constants.h"

namespace net {
Expand Down Expand Up @@ -65,4 +67,17 @@ bool CookiePartitionKey::Deserialize(const std::string& in,
return true;
}

absl::optional<CookiePartitionKey> CookiePartitionKey::FromNetworkIsolationKey(
const NetworkIsolationKey& network_isolation_key) {
if (!base::FeatureList::IsEnabled(features::kPartitionedCookies))
return absl::nullopt;
// TODO(crbug.com/1225444): Check if the top frame site is in a First-Party
// Set or if it is an extension URL.
absl::optional<net::SchemefulSite> top_frame_site =
network_isolation_key.GetTopFrameSite();
if (!top_frame_site)
return absl::nullopt;
return absl::make_optional(net::CookiePartitionKey(top_frame_site.value()));
}

} // namespace net
6 changes: 6 additions & 0 deletions net/cookies/cookie_partition_key.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>

#include "net/base/net_export.h"
#include "net/base/network_isolation_key.h"
#include "net/base/schemeful_site.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -63,6 +64,11 @@ class NET_EXPORT CookiePartitionKey {
return CookiePartitionKey(url);
}

// Create a cookie partition key from a request's NetworkIsolationKey.
//
static absl::optional<CookiePartitionKey> FromNetworkIsolationKey(
const NetworkIsolationKey& network_isolation_key);

// Temporary method, used to mark the places where we need to supply the
// cookie partition key to CanonicalCookie::Create.
static absl::optional<CookiePartitionKey> Todo() { return absl::nullopt; }
Expand Down
42 changes: 40 additions & 2 deletions net/cookies/cookie_partition_key_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,33 @@
#define NET_COOKIES_COOKIE_PARTITION_KEY_UNITTEST_H_

#include "net/cookies/cookie_partition_key.h"
#include "base/test/scoped_feature_list.h"
#include "net/base/features.h"
#include "net/cookies/cookie_constants.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace net {

TEST(CookiePartitionKeyTest, Serialization) {
class CookiePartitionKeyTest : public testing::TestWithParam<bool> {
protected:
// testing::Test
void SetUp() override {
if (PartitionedCookiesEnabled())
scoped_feature_list_.InitAndEnableFeature(features::kPartitionedCookies);
testing::TestWithParam<bool>::SetUp();
}

bool PartitionedCookiesEnabled() { return GetParam(); }

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

INSTANTIATE_TEST_SUITE_P(/* no label */,
CookiePartitionKeyTest,
testing::Bool());

TEST_P(CookiePartitionKeyTest, Serialization) {
struct {
absl::optional<CookiePartitionKey> input;
bool expected_ret;
Expand Down Expand Up @@ -47,7 +68,7 @@ TEST(CookiePartitionKeyTest, Serialization) {
}
}

TEST(CookiePartitionKeyTest, Deserialization) {
TEST_P(CookiePartitionKeyTest, Deserialization) {
struct {
std::string input;
bool expected_ret;
Expand All @@ -72,6 +93,23 @@ TEST(CookiePartitionKeyTest, Deserialization) {
}
}

TEST_P(CookiePartitionKeyTest, FromNetworkIsolationKey) {
EXPECT_FALSE(
CookiePartitionKey::FromNetworkIsolationKey(NetworkIsolationKey()));

SchemefulSite top_level_site =
SchemefulSite(GURL("https://toplevelsite.com"));
absl::optional<CookiePartitionKey> got =
CookiePartitionKey::FromNetworkIsolationKey(NetworkIsolationKey(
top_level_site, SchemefulSite(GURL("https://cookiesite.com"))));

bool partitioned_cookies_enabled = PartitionedCookiesEnabled();
EXPECT_EQ(partitioned_cookies_enabled, got.has_value());
if (partitioned_cookies_enabled) {
EXPECT_EQ(CookiePartitionKey(top_level_site), got.value());
}
}

} // namespace net

#endif // NET_COOKIES_COOKIE_PARTITION_KEY_UNITTEST_H_
4 changes: 3 additions & 1 deletion net/url_request/url_request_http_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,9 @@ void URLRequestHttpJob::SaveCookiesAndNotifyHeadersComplete(int result) {

std::unique_ptr<CanonicalCookie> cookie = net::CanonicalCookie::Create(
request_->url(), cookie_string, base::Time::Now(), server_time,
net::CookiePartitionKey::Todo(), &returned_status);
net::CookiePartitionKey::FromNetworkIsolationKey(
request_->isolation_info().network_isolation_key()),
&returned_status);

absl::optional<CanonicalCookie> cookie_to_return = absl::nullopt;
if (returned_status.IsInclude()) {
Expand Down
71 changes: 71 additions & 0 deletions net/url_request/url_request_http_job_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
#include "base/strings/string_split.h"
#include "base/test/bind.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
#include "build/build_config.h"
#include "net/base/auth.h"
#include "net/base/features.h"
#include "net/base/isolation_info.h"
#include "net/base/network_isolation_key.h"
#include "net/base/request_priority.h"
Expand Down Expand Up @@ -1910,4 +1912,73 @@ TEST_F(URLRequestHttpJobTest, IndividuallyBlockedCookies) {
MatchesCookieAccessResult(IsInclude(), _, _, _))));
}

class PartitionedCookiesURLRequestHttpJobTest
: public URLRequestHttpJobTest,
public testing::WithParamInterface<bool> {
protected:
// testing::Test
void SetUp() override {
if (PartitionedCookiesEnabled())
scoped_feature_list_.InitAndEnableFeature(features::kPartitionedCookies);
URLRequestHttpJobTest::SetUp();
}

bool PartitionedCookiesEnabled() { return GetParam(); }

private:
base::test::ScopedFeatureList scoped_feature_list_;
};

INSTANTIATE_TEST_SUITE_P(/* no label */,
PartitionedCookiesURLRequestHttpJobTest,
testing::Bool());

TEST_P(PartitionedCookiesURLRequestHttpJobTest, SetPartitionedCookie) {
EmbeddedTestServer https_test(EmbeddedTestServer::TYPE_HTTPS);
const url::Origin kTopFrameOrigin =
url::Origin::Create(GURL("https://www.toplevelsite.com"));
const IsolationInfo kTestIsolationInfo =
IsolationInfo::CreateForInternalRequest(kTopFrameOrigin);

https_test.AddDefaultHandlers(base::FilePath());
ASSERT_TRUE(https_test.Start());
TestURLRequestContext context;

CookieMonster cookie_monster(nullptr, nullptr);
context.set_cookie_store(&cookie_monster);

GURL test_url = https_test.GetURL(
"/set-cookie?__Host-foo=bar;SameSite=None;Secure;Path=/;Partitioned;");

TestDelegate delegate;
std::unique_ptr<URLRequest> request = context.CreateRequest(
test_url, DEFAULT_PRIORITY, &delegate, TRAFFIC_ANNOTATION_FOR_TESTS);

request->set_isolation_info(kTestIsolationInfo);
request->Start();
ASSERT_TRUE(request->is_pending());
delegate.RunUntilComplete();

base::RunLoop run_loop;
bool partitioned_cookies_enabled = PartitionedCookiesEnabled();
cookie_monster.GetAllCookiesAsync(base::BindLambdaForTesting(
[&partitioned_cookies_enabled, &run_loop](const CookieList& cookies) {
EXPECT_EQ(1u, cookies.size());
EXPECT_EQ(partitioned_cookies_enabled, cookies[0].IsPartitioned());
if (partitioned_cookies_enabled) {
EXPECT_EQ(CookiePartitionKey::FromURLForTesting(
GURL("https://toplevelsite.com")),
cookies[0].PartitionKey().value());
} else {
EXPECT_FALSE(cookies[0].PartitionKey());
}
run_loop.Quit();
}));
run_loop.Run();

// TODO(crbug.com/1225444) Test that the cookie is available in a cross-site
// context on a different top-level site only when partitioned cookies are
// disabled.
}

} // namespace net

0 comments on commit 39fa227

Please sign in to comment.