From: uazo Date: Mon, 9 Jan 2023 12:02:05 +0000 Subject: Partitioning all cookies by top frame domain Enables cookie partitioning by top frame etld, respecting the user's possible wish to disable all third-party cookies. Disabling the flag via the ui restores the normal mode, where samesite=none first-party cookies are sent in third-party contexts. License: GPL-2.0-or-later - https://spdx.org/licenses/GPL-2.0-or-later.html --- .../browser/net/profile_network_context_service.cc | 8 ++++---- .../core/common/cookie_settings_base.cc | 2 +- components/content_settings/core/common/features.cc | 4 ++-- components/permissions/features.cc | 4 ++-- content/public/common/content_features.cc | 4 ++-- net/base/features.cc | 12 ++++++------ net/cookies/canonical_cookie.cc | 12 +----------- net/cookies/cookie_deletion_info.cc | 3 ++- net/cookies/parsed_cookie.h | 7 ++++++- net/extras/sqlite/sqlite_persistent_cookie_store.cc | 10 ++++++++++ net/url_request/url_request_http_job.cc | 2 +- services/network/restricted_cookie_manager.cc | 3 +++ .../renderer/modules/cookie_store/cookie_init.idl | 2 +- .../renderer/modules/cookie_store/cookie_store.cc | 12 ++++++++++++ .../cookie_store/cookie_store_delete_options.idl | 2 +- ui/webui/webui_allowlist.cc | 1 + 16 files changed, 55 insertions(+), 33 deletions(-) diff --git a/chrome/browser/net/profile_network_context_service.cc b/chrome/browser/net/profile_network_context_service.cc --- a/chrome/browser/net/profile_network_context_service.cc +++ b/chrome/browser/net/profile_network_context_service.cc @@ -613,14 +613,14 @@ ProfileNetworkContextService::CreateCookieManagerParams( // UI to interact with SameSite cookies on accounts.google.com, which is used // for displaying a list of available accounts on the NTP // (chrome://new-tab-page), etc. - out->secure_origin_cookies_allowed_schemes.push_back( - content::kChromeUIScheme); + // out->secure_origin_cookies_allowed_schemes.push_back( + // content::kChromeUIScheme); #if BUILDFLAG(ENABLE_EXTENSIONS) // TODO(chlily): To be consistent with the content_settings version of // CookieSettings, we should probably also add kExtensionScheme to the list of // matching_scheme_cookies_allowed_schemes. - out->third_party_cookies_allowed_schemes.push_back( - extensions::kExtensionScheme); + // out->third_party_cookies_allowed_schemes.push_back( + // extensions::kExtensionScheme); #endif HostContentSettingsMap* host_content_settings_map = diff --git a/components/content_settings/core/common/cookie_settings_base.cc b/components/content_settings/core/common/cookie_settings_base.cc --- a/components/content_settings/core/common/cookie_settings_base.cc +++ b/components/content_settings/core/common/cookie_settings_base.cc @@ -318,7 +318,7 @@ CookieSettingsBase::GetCookieSettingInternal( absl::optional scope; if (block_third) { scope = IsAllowed(setting) - ? ThirdPartyBlockingScope::kUnpartitionedOnly + ? ThirdPartyBlockingScope::kUnpartitionedAndPartitioned : ThirdPartyBlockingScope::kUnpartitionedAndPartitioned; } return {block_third ? CONTENT_SETTING_BLOCK : setting, scope, diff --git a/components/content_settings/core/common/features.cc b/components/content_settings/core/common/features.cc --- a/components/content_settings/core/common/features.cc +++ b/components/content_settings/core/common/features.cc @@ -79,8 +79,8 @@ BASE_FEATURE(kImprovedSemanticsActivityIndicators, base::FEATURE_DISABLED_BY_DEFAULT); BASE_FEATURE(kTrackingProtection3pcd, - "TrackingProtection3pcd", - base::FEATURE_DISABLED_BY_DEFAULT); + "TrackingProtection3pcd", // disabled + base::FEATURE_DISABLED_BY_DEFAULT); // by default } // namespace features } // namespace content_settings diff --git a/components/permissions/features.cc b/components/permissions/features.cc --- a/components/permissions/features.cc +++ b/components/permissions/features.cc @@ -156,8 +156,8 @@ BASE_FEATURE(kMitigateUnpartitionedWebviewPermissions, // This includes enabling prompts, a new settings page and page info and // omnibox integration. BASE_FEATURE(kPermissionStorageAccessAPI, - "PermissionStorageAccessAPI", - base::FEATURE_DISABLED_BY_DEFAULT); + "PermissionStorageAccessAPI", // guard + base::FEATURE_DISABLED_BY_DEFAULT); // this // When enabled "window-placement" may be used as an alias for // "window-management". Additionally, reverse mappings (i.e. enum to string) diff --git a/content/public/common/content_features.cc b/content/public/common/content_features.cc --- a/content/public/common/content_features.cc +++ b/content/public/common/content_features.cc @@ -186,8 +186,8 @@ BASE_FEATURE(kCompositeBGColorAnimation, // server side testing without cookies. // (See https://developer.chrome.com/en/docs/privacy-sandbox/chrome-testing) BASE_FEATURE(kCookieDeprecationFacilitatedTesting, - "CookieDeprecationFacilitatedTesting", - base::FEATURE_DISABLED_BY_DEFAULT); + "CookieDeprecationFacilitatedTesting", // disabled + base::FEATURE_DISABLED_BY_DEFAULT); // by default // Set whether to enable cookie deprecation API for off-the-record profiles. const base::FeatureParam diff --git a/net/base/features.cc b/net/base/features.cc --- a/net/base/features.cc +++ b/net/base/features.cc @@ -260,8 +260,8 @@ BASE_FEATURE(kWaitForFirstPartySetsInit, base::FEATURE_DISABLED_BY_DEFAULT); BASE_FEATURE(kPartitionedCookies, - "PartitionedCookies", - base::FEATURE_ENABLED_BY_DEFAULT); + "PartitionedCookies", // guard this + base::FEATURE_ENABLED_BY_DEFAULT); // guard this BASE_FEATURE(kBlockTruncatedCookies, "BlockTruncatedCookies", @@ -272,8 +272,8 @@ BASE_FEATURE(kStaticKeyPinningEnforcement, base::FEATURE_ENABLED_BY_DEFAULT); BASE_FEATURE(kCookieDomainRejectNonASCII, - "CookieDomainRejectNonASCII", - base::FEATURE_DISABLED_BY_DEFAULT); + "CookieDomainRejectNonASCII", // guard this + base::FEATURE_ENABLED_BY_DEFAULT); // guard this // Enables partitioning of third party storage (IndexedDB, CacheStorage, etc.) // by the top level site to reduce fingerprinting. @@ -469,8 +469,8 @@ BASE_FEATURE(kDigestAuthEnableSecureAlgorithms, // are disabled by default. Partitioned storage will not be allowed if // third-party cookies are disabled due to a specific rule. BASE_FEATURE(kThirdPartyPartitionedStorageAllowedByDefault, - "ThirdPartyPartitionedStorageAllowedByDefault", - base::FEATURE_ENABLED_BY_DEFAULT); + "ThirdPartyPartitionedStorageAllowedByDefault", // must be + base::FEATURE_DISABLED_BY_DEFAULT); // disabled BASE_FEATURE(kPriorityHeader, "PriorityHeader", diff --git a/net/cookies/canonical_cookie.cc b/net/cookies/canonical_cookie.cc --- a/net/cookies/canonical_cookie.cc +++ b/net/cookies/canonical_cookie.cc @@ -1469,8 +1469,6 @@ bool CanonicalCookie::IsCanonicalForFromStorage() const { if (IsPartitioned()) { if (CookiePartitionKey::HasNonce(partition_key_)) return true; - if (!secure_) - return false; } return true; @@ -1727,15 +1725,7 @@ bool CanonicalCookie::IsCookiePartitionedValid(const GURL& url, bool secure, bool is_partitioned, bool partition_has_nonce) { - if (!is_partitioned) - return true; - if (partition_has_nonce) - return true; - CookieAccessScheme scheme = cookie_util::ProvisionalAccessScheme(url); - bool result = (scheme != CookieAccessScheme::kNonCryptographic) && secure; - DLOG_IF(WARNING, !result) - << "CanonicalCookie has invalid Partitioned attribute"; - return result; + return true; } CookieAndLineWithAccessResult::CookieAndLineWithAccessResult() = default; diff --git a/net/cookies/cookie_deletion_info.cc b/net/cookies/cookie_deletion_info.cc --- a/net/cookies/cookie_deletion_info.cc +++ b/net/cookies/cookie_deletion_info.cc @@ -131,7 +131,8 @@ bool CookieDeletionInfo::Matches(const CanonicalCookie& cookie, return false; } - if (cookie.IsPartitioned() && + // opened bug https://bugs.chromium.org/p/chromium/issues/detail?id=1405772 + if (cookie.IsPartitioned() && !cookie_partition_key_collection.IsEmpty() && !cookie_partition_key_collection.Contains(*cookie.PartitionKey())) { return false; } diff --git a/net/cookies/parsed_cookie.h b/net/cookies/parsed_cookie.h --- a/net/cookies/parsed_cookie.h +++ b/net/cookies/parsed_cookie.h @@ -11,6 +11,7 @@ #include #include +#include "net/base/features.h" #include "net/base/net_export.h" #include "net/cookies/cookie_constants.h" @@ -86,7 +87,11 @@ class NET_EXPORT ParsedCookie { CookieSameSiteString* samesite_string = nullptr) const; CookiePriority Priority() const; bool IsSameParty() const { return same_party_index_ != 0; } - bool IsPartitioned() const { return partitioned_index_ != 0; } + bool IsPartitioned() const { + if (base::FeatureList::IsEnabled(net::features::kPartitionedCookies)) + return true; + return partitioned_index_ != 0; + } bool HasInternalHtab() const { return internal_htab_; } TruncatingCharacterInCookieStringType GetTruncatingCharacterInCookieStringType() const { diff --git a/net/extras/sqlite/sqlite_persistent_cookie_store.cc b/net/extras/sqlite/sqlite_persistent_cookie_store.cc --- a/net/extras/sqlite/sqlite_persistent_cookie_store.cc +++ b/net/extras/sqlite/sqlite_persistent_cookie_store.cc @@ -814,6 +814,16 @@ bool SQLitePersistentCookieStore::Backend::DoInitializeDatabase() { if (!restore_old_session_cookies_) DeleteSessionCookiesOnStartup(); + // Since there is no automatic transition to partitioned cookies + // (the information would be missing), we clean the current ones + // present because they would otherwise be sent in third-party contexts + // even if the flag is active. + if (base::FeatureList::IsEnabled(features::kPartitionedCookies)) { + if (!db()->Execute("DELETE FROM cookies WHERE top_frame_site_key = ''")) { + LOG(WARNING) << "Unable to delete unpartitioned cookies."; + } + } + return true; } diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc --- a/net/url_request/url_request_http_job.cc +++ b/net/url_request/url_request_http_job.cc @@ -758,7 +758,7 @@ void URLRequestHttpJob::SetCookieHeaderAndStart( size_t n_partitioned_cookies = 0; bool may_set_sec_cookie_deprecation_header = - !request_->context()->cookie_deprecation_label().value_or("").empty(); + false; // TODO(crbug.com/1031664): Reduce the number of times the cookie list // is iterated over. Get metrics for every cookie which is included. diff --git a/services/network/restricted_cookie_manager.cc b/services/network/restricted_cookie_manager.cc --- a/services/network/restricted_cookie_manager.cc +++ b/services/network/restricted_cookie_manager.cc @@ -808,6 +808,9 @@ void RestrictedCookieManager::SetCookieFromString( std::move(callback).Run(site_for_cookies_ok, top_frame_origin_ok); callback = base::DoNothing(); + // https://bugs.chromium.org/p/chromium/issues/detail?id=911299 + if (!site_for_cookies_ok || !top_frame_origin_ok) return; + net::CookieInclusionStatus status; std::unique_ptr parsed_cookie = net::CanonicalCookie::Create( diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_init.idl b/third_party/blink/renderer/modules/cookie_store/cookie_init.idl --- a/third_party/blink/renderer/modules/cookie_store/cookie_init.idl +++ b/third_party/blink/renderer/modules/cookie_store/cookie_init.idl @@ -17,5 +17,5 @@ dictionary CookieInit { USVString path = "/"; DOMHighResTimeStamp? expires = null; CookieSameSite sameSite = "strict"; - [RuntimeEnabled=PartitionedCookies] boolean partitioned = false; + [RuntimeEnabled=PartitionedCookies] boolean partitioned = true; }; diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store.cc b/third_party/blink/renderer/modules/cookie_store/cookie_store.cc --- a/third_party/blink/renderer/modules/cookie_store/cookie_store.cc +++ b/third_party/blink/renderer/modules/cookie_store/cookie_store.cc @@ -321,6 +321,10 @@ ScriptPromise CookieStore::set(ScriptState* script_state, CookieInit* set_options = CookieInit::Create(); set_options->setName(name); set_options->setValue(value); + if (RuntimeEnabledFeatures::PartitionedCookiesEnabled( + CurrentExecutionContext(script_state->GetIsolate()))) { + set_options->setPartitioned(true); + } return set(script_state, set_options, exception_state); } @@ -343,6 +347,10 @@ ScriptPromise CookieStore::Delete(ScriptState* script_state, set_options->setName(name); set_options->setValue("deleted"); set_options->setExpires(0); + if (RuntimeEnabledFeatures::PartitionedCookiesEnabled( + CurrentExecutionContext(script_state->GetIsolate()))) { + set_options->setPartitioned(true); + } return DoWrite(script_state, set_options, exception_state); } @@ -357,6 +365,10 @@ ScriptPromise CookieStore::Delete(ScriptState* script_state, set_options->setPath(options->path()); set_options->setSameSite("strict"); set_options->setPartitioned(options->partitioned()); + if (RuntimeEnabledFeatures::PartitionedCookiesEnabled( + CurrentExecutionContext(script_state->GetIsolate()))) { + set_options->setPartitioned(true); + } return DoWrite(script_state, set_options, exception_state); } diff --git a/third_party/blink/renderer/modules/cookie_store/cookie_store_delete_options.idl b/third_party/blink/renderer/modules/cookie_store/cookie_store_delete_options.idl --- a/third_party/blink/renderer/modules/cookie_store/cookie_store_delete_options.idl +++ b/third_party/blink/renderer/modules/cookie_store/cookie_store_delete_options.idl @@ -8,5 +8,5 @@ dictionary CookieStoreDeleteOptions { required USVString name; USVString? domain = null; USVString path = "/"; - [RuntimeEnabled=PartitionedCookies] boolean partitioned = false; + [RuntimeEnabled=PartitionedCookies] boolean partitioned = true; }; diff --git a/ui/webui/webui_allowlist.cc b/ui/webui/webui_allowlist.cc --- a/ui/webui/webui_allowlist.cc +++ b/ui/webui/webui_allowlist.cc @@ -74,6 +74,7 @@ void WebUIAllowlist::RegisterAutoGrantedPermissions( void WebUIAllowlist::RegisterAutoGrantedThirdPartyCookies( const url::Origin& top_level_origin, const std::vector& origin_patterns) { + if ((true)) return; DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); -- 2.25.1