Discharge7443/brave-core
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Cookie expiry fixes (#14852)

Browse Source 
* Add cookie expiry browser tests
* Remove cookie store patch
* Add ModifyExpiration to account for JS cookie expiry
This commit is contained in:
Shivan Kaul Sahib
2022-09-01 10:57:39 -07:00
committed by GitHub
parent 60076161c1
commit 56de59362b
9 changed files with 256 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files
browser/brave_shields/cookie_expiry_browsertest.cc
+212
View File
@@ -0,0 +1,212 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <string>
#include "base/bind.h"
#include "base/test/bind.h"
#include "base/path_service.h"
#include "base/strings/stringprintf.h"
#include "brave/components/constants/brave_paths.h"
#include "build/build_config.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/content_mock_cert_verifier.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/default_handlers.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
namespace {
constexpr base::TimeDelta k4YearsInDays = base::Days(1461);
// There might be a gap of a few milliseconds between setting the cookie and it
// getting stored. To prevent flapping tests, set this margin to be large
// (but we're still testing what we want to test)
// See: net/cookies/canonical_cookie_unittest.cc
constexpr base::TimeDelta kMarginForTesting = base::Seconds(5);
} // namespace
class CookieExpirationTest : public InProcessBrowserTest {
public:
void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();
mock_cert_verifier_.mock_cert_verifier()->set_default_result(net::OK);
host_resolver()->AddRule("*", "127.0.0.1");
https_server_ = std::make_unique<net::EmbeddedTestServer>(
net::test_server::EmbeddedTestServer::TYPE_HTTPS);
RegisterDefaultHandlers(https_server_.get());
brave::RegisterPathProvider();
base::FilePath test_data_dir;
base::PathService::Get(brave::DIR_TEST_DATA, &test_data_dir);
https_server_->ServeFilesFromDirectory(test_data_dir);
ASSERT_TRUE(https_server_->Start());
}
void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
mock_cert_verifier_.SetUpCommandLine(command_line);
}
void SetUpInProcessBrowserTestFixture() override {
InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
mock_cert_verifier_.SetUpInProcessBrowserTestFixture();
}
void TearDownInProcessBrowserTestFixture() override {
InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
mock_cert_verifier_.TearDownInProcessBrowserTestFixture();
}
// Set a cookie with JavaScript.
void JSDocumentCookieWriteCookie(Browser* browser, std::string age) {
std::string cookie_string =
base::StringPrintf("document.cookie = 'name=Test; %s'", age.c_str());
ASSERT_TRUE(content::ExecJs(
browser->tab_strip_model()->GetActiveWebContents(), cookie_string));
}
void JSCookieStoreWriteCookie(Browser* browser, std::string expires_in_ms) {
ASSERT_TRUE(content::ExecJs(
browser->tab_strip_model()->GetActiveWebContents(),
base::StringPrintf("(async () => {"
"return await window.cookieStore.set("
" { name: 'name',"
" value: 'Good',"
" expires: Date.now() + %s,"
" });"
"})()",
expires_in_ms.c_str())));
}
std::vector<net::CanonicalCookie> GetAllCookiesDirect(Browser* browser) {
base::RunLoop run_loop;
std::vector<net::CanonicalCookie> cookies_out;
browser->tab_strip_model()
->GetActiveWebContents()
->GetBrowserContext()
->GetDefaultStoragePartition()
->GetCookieManagerForBrowserProcess()
->GetAllCookies(base::BindLambdaForTesting(
[&run_loop,
&cookies_out](const std::vector<net::CanonicalCookie>& cookies) {
cookies_out = cookies;
run_loop.Quit();
}));
run_loop.Run();
return cookies_out;
}
protected:
std::unique_ptr<net::EmbeddedTestServer> https_server_;
private:
content::ContentMockCertVerifier mock_cert_verifier_;
};
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForDocumentCookieLessThanMax) {
auto less_than_max = base::Days(2);
GURL url = https_server_->GetURL("a.com", "/simple.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
JSDocumentCookieWriteCookie(
browser(), "max-age=" + std::to_string(less_than_max.InSeconds()));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_LE((base::Time::Now() + less_than_max - cookie.ExpiryDate()),
kMarginForTesting);
}
}
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForDocumentCookieMoreThanMax) {
GURL url = https_server_->GetURL("a.com", "/simple.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
JSDocumentCookieWriteCookie(
browser(), "max-age=" + std::to_string(k4YearsInDays.InSeconds()));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_EQ((cookie.ExpiryDate() - cookie.CreationDate()).InDays(), 7);
}
}
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForCookieStoreLessThanMax) {
auto less_than_max = base::Days(2);
GURL url = https_server_->GetURL("a.com", "/simple.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
JSCookieStoreWriteCookie(browser(),
std::to_string(less_than_max.InMilliseconds()));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_LE((base::Time::Now() + less_than_max - cookie.ExpiryDate()),
kMarginForTesting);
}
}
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForCookieStoreMoreThanMax) {
GURL url = https_server_->GetURL("a.com", "/simple.html");
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
JSCookieStoreWriteCookie(browser(),
std::to_string(k4YearsInDays.InMilliseconds()));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_EQ((cookie.ExpiryDate() - cookie.CreationDate()).InDays(), 7);
}
}
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForHttpCookiesLessThanMax) {
auto less_than_max = base::Days(30);
std::string cookie_string = "/set-cookie?test=http;max-age=" +
std::to_string(less_than_max.InSeconds());
GURL url = https_server_->GetURL("a.com", cookie_string);
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_LE((base::Time::Now() + less_than_max - cookie.ExpiryDate()),
kMarginForTesting);
}
}
IN_PROC_BROWSER_TEST_F(CookieExpirationTest,
CheckExpiryForHttpCookiesMoreThanMax) {
std::string cookie_string =
"test=http;max-age=" + std::to_string(k4YearsInDays.InSeconds());
GURL url = https_server_->GetURL("a.com", "/set-cookie?" + cookie_string);
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
std::vector<net::CanonicalCookie> all_cookies =
GetAllCookiesDirect(browser());
EXPECT_EQ(1u, all_cookies.size());
for (const net::CanonicalCookie& cookie : all_cookies) {
EXPECT_EQ((cookie.ExpiryDate() - cookie.CreationDate()).InDays(), 180);
}
}
browser/brave_shields/sources.gni
+1
View File
@@ -21,6 +21,7 @@ brave_browser_brave_shields_deps = [
"//brave/components/brave_perf_predictor/browser",
"//brave/components/brave_shields/browser",
"//brave/components/brave_shields/common",
"//brave/components/constants",
"//chrome/browser/profiles:profile",
"//chrome/common",
"//components/content_settings/core/browser",
chromium_src/net/cookies/canonical_cookie.cc
+8 -8
View File
@@ -9,21 +9,21 @@
namespace {
// Javascript max expiration is handled by blink::CookieStore
constexpr base::TimeDelta kMaxCookieExpiration =
base::Days(30 * 6); // 6 months
base::Time BraveCanonExpiration(const base::Time& cookie_expires,
const base::Time& creation_time) {
const base::Time max_expiration = creation_time + kMaxCookieExpiration;
base::Time BraveCanonExpiration(const base::Time& expiry_date,
const base::Time& creation_date) {
const base::Time max_expiration = creation_date + kMaxCookieExpiration;
return std::min(cookie_expires, max_expiration);
return std::min(expiry_date, max_expiration);
}
} // namespace
#define BRAVE_CREATE \
cookie_expires = BraveCanonExpiration(cookie_expires, creation_time);
#define BRAVE_CANONICAL_COOKIE_VALIDATE_AND_ADJUST_EXPIRY_DATE \
if ((true)) \
return BraveCanonExpiration(expiry_date, fixed_creation_date);
#include "src/net/cookies/canonical_cookie.cc"
#undef BRAVE_CREATE
#undef BRAVE_CANONICAL_COOKIE_VALIDATE_AND_ADJUST_EXPIRY_DATE
chromium_src/services/network/restricted_cookie_manager.cc
+23
View File
@@ -24,14 +24,37 @@
// for components/content_settings/core/common/cookie_settings_base.{h,cc}.
#define IsFullCookieAccessAllowed IsEphemeralCookieAccessAllowed
#define FromStorage(NAME, VALUE, DOMAIN, PATH, CREATION, EXPIRY, LAST_ACCESS, \
LAST_UPDATE, SECURE, HTTP_ONLY, SAME_SITE, PRIORITY, \
SAME_PARTY, PARTITION, SOURCE_SCHEME, PORT) \
FromStorage(NAME, VALUE, DOMAIN, PATH, CREATION, \
ModifyExpiration(EXPIRY, CREATION), LAST_ACCESS, LAST_UPDATE, \
SECURE, HTTP_ONLY, SAME_SITE, PRIORITY, SAME_PARTY, PARTITION, \
SOURCE_SCHEME, PORT)
#include "src/services/network/restricted_cookie_manager.cc"
#undef IsFullCookieAccessAllowed
#undef AnnotateAndMoveUserBlockedCookies
#undef IsCookieAccessible
namespace {
constexpr base::TimeDelta kMaxCookieExpiration =
base::Days(7); // For JS cookies: CookieStore and document.cookie
} // namespace
namespace network {
base::Time RestrictedCookieManager::ModifyExpiration(
const base::Time& expiry_date,
const base::Time& creation_date) const {
const base::Time max_expiration = creation_date + kMaxCookieExpiration;
return std::min(expiry_date, max_expiration);
}
net::CookieOptions RestrictedCookieManager::MakeOptionsForSet(
mojom::RestrictedCookieManagerRole role,
const GURL& url,
chromium_src/services/network/restricted_cookie_manager.h
+2
View File
@@ -14,6 +14,8 @@
#define RemoveChangeListener \
NotUsed() const {} \
base::Time ModifyExpiration(const base::Time& expiry_date, \
const base::Time& creation_date) const; \
net::CookieOptions MakeOptionsForSet( \
mojom::RestrictedCookieManagerRole role, const GURL& url, \
const net::SiteForCookies& site_for_cookies, \
chromium_src/third_party/blink/renderer/modules/cookie_store/cookie_store.cc
Vendored
-23
View File
@@ -1,23 +0,0 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */
#include <algorithm>
#include "base/time/time.h"
namespace {
constexpr base::TimeDelta kJavascriptCookieExpiration = base::Days(7);
base::Time BraveCanonExpiration(const base::Time& current) {
// creation time is always now for new JS cookies
return std::min(base::Time::Now() + kJavascriptCookieExpiration, current);
}
} // namespace
#define BRAVE_TO_CANONICAL_COOKIE expires = BraveCanonExpiration(expires);
#include "src/third_party/blink/renderer/modules/cookie_store/cookie_store.cc"
#undef BRAVE_TO_CANONICAL_COOKIE
patches/net-cookies-canonical_cookie.cc.patch
+9 -9
View File
@@ -1,12 +1,12 @@
diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc
index 91d210fa1683da314922a15734d4fabbde8ab873..1123de9ee42317726e3e0d8f4a37bd71a54202d7 100644
index 91d210fa1683da314922a15734d4fabbde8ab873..20eb17aef84b0f3bf8182b62e6afe13b71aa8571 100644
--- a/net/cookies/canonical_cookie.cc
+++ b/net/cookies/canonical_cookie.cc
@@ -666,6 +666,7 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::Create(
// Get the port, this will get a default value if a port isn't provided.
int source_port = ValidateAndAdjustSourcePort(url.EffectiveIntPort());
+ BRAVE_CREATE
std::unique_ptr<CanonicalCookie> cc = base::WrapUnique(new CanonicalCookie(
parsed_cookie.Name(), parsed_cookie.Value(), cookie_domain, cookie_path,
creation_time, cookie_expires, creation_time,
@@ -552,6 +552,7 @@ base::Time CanonicalCookie::ValidateAndAdjustExpiryDate(
// * network_handler.cc::MakeCookieFromProtocolValues
fixed_creation_date = base::Time::Now();
}
+ BRAVE_CANONICAL_COOKIE_VALIDATE_AND_ADJUST_EXPIRY_DATE
if (base::FeatureList::IsEnabled(features::kClampCookieExpiryTo400Days)) {
base::Time maximum_expiry_date = fixed_creation_date + base::Days(400);
if (expiry_date > maximum_expiry_date)
patches/third_party-blink-renderer-modules-cookie_store-cookie_store.cc.patch
-12
View File
@@ -1,12 +0,0 @@
diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store.cc b/third_party/blink/renderer/modules/cookie_store/cookie_store.cc
index c4e35911f18c1690c1e212949a6ed86d5c517473..ee40afd0a3c91267e38f731ad518fd9d5dfe1015 100644
--- a/third_party/blink/renderer/modules/cookie_store/cookie_store.cc
+++ b/third_party/blink/renderer/modules/cookie_store/cookie_store.cc
@@ -85,6 +85,7 @@ std::unique_ptr<net::CanonicalCookie> ToCanonicalCookie(
base::Time expires = options->hasExpiresNonNull()
? base::Time::FromJavaTime(options->expiresNonNull())
: base::Time();
+ BRAVE_TO_CANONICAL_COOKIE
String cookie_url_host = cookie_url.Host();
String domain;
test/BUILD.gn
+1
View File
@@ -657,6 +657,7 @@ if (!is_android) {
"//brave/browser/brave_shields/ad_block_service_browsertest.cc",
"//brave/browser/brave_shields/ad_block_service_browsertest.h",
"//brave/browser/brave_shields/brave_shields_web_contents_observer_browsertest.cc",
"//brave/browser/brave_shields/cookie_expiry_browsertest.cc",
"//brave/browser/brave_shields/cookie_pref_service_browsertest.cc",
"//brave/browser/brave_shields/domain_block_page_browsertest.cc",
"//brave/browser/brave_shields/websockets_pool_limit_browsertest.cc",