[Insecure Cookies] Cap lifetimes of insecure cookies

This adds a new feature flag (kTimeLimitedInsecureCookies) that when
enabled, caps expiration of cookies set by http:// sites to 3 hours.
The new flag is only functional if kEnableSchemeBoundCookies is also
enabled.

Bug: 1510563

Change-Id: Idede0065e27c6e75c5d4778ff02f342af2df97cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4924269
Auto-Submit: Carlos IL <carlosil@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Reviewed-by: Steven Bingler <bingler@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1238808}

google_apis/gaia/oauth_multilogin_result.cc

@@ -13,6 +13,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
+#include "net/cookies/cookie_constants.h"
 
 namespace {
 
@@ -112,7 +113,8 @@ void OAuthMultiloginResult::TryParseCookiesFromValue(
     // Alternatly, if we were sure GAIA cookies wouldn't try to expire more
     // than 400 days in the future we wouldn't need this either.
     base::Time expiration = net::CanonicalCookie::ValidateAndAdjustExpiryDate(
-        now + base::Seconds(expiration_delta.value_or(0.0)), now);
+        now + base::Seconds(expiration_delta.value_or(0.0)), now,
+        net::CookieSourceScheme::kSecure);
     std::string cookie_domain = domain ? *domain : "";
     std::string cookie_host = host ? *host : "";
     if (cookie_domain.empty() && !cookie_host.empty() &&

net/base/features.cc

@@ -447,6 +447,10 @@ BASE_FEATURE(kEnableSchemeBoundCookies,
              "EnableSchemeBoundCookies",
              base::FEATURE_DISABLED_BY_DEFAULT);
 
+BASE_FEATURE(kTimeLimitedInsecureCookies,
+             "TimeLimitedInsecureCookies",
+             base::FEATURE_DISABLED_BY_DEFAULT);
+
 // Enable third-party cookie blocking from the command line.
 BASE_FEATURE(kForceThirdPartyCookieBlocking,
              "ForceThirdPartyCookieBlockingEnabled",

net/base/features.h

@@ -450,6 +450,10 @@ NET_EXPORT BASE_DECLARE_FEATURE(kEnablePortBoundCookies);
 // enables domain cookie shadowing protection.
 NET_EXPORT BASE_DECLARE_FEATURE(kEnableSchemeBoundCookies);
 
+// Enables expiration duration limit (3 hours) for cookies on insecure websites.
+// This feature is a no-op unless kEnableSchemeBoundCookies is enabled.
+NET_EXPORT BASE_DECLARE_FEATURE(kTimeLimitedInsecureCookies);
+
 // Enables enabling third-party cookie blocking from the command line.
 NET_EXPORT BASE_DECLARE_FEATURE(kForceThirdPartyCookieBlocking);
 

net/cookies/canonical_cookie.cc

@@ -498,7 +498,8 @@ Time CanonicalCookie::ParseExpiration(const ParsedCookie& pc,
 // static
 base::Time CanonicalCookie::ValidateAndAdjustExpiryDate(
     const base::Time& expiry_date,
-    const base::Time& creation_date) {
+    const base::Time& creation_date,
+    net::CookieSourceScheme scheme) {
   if (expiry_date.is_null())
     return expiry_date;
   base::Time fixed_creation_date = creation_date;
@@ -513,7 +514,13 @@ base::Time CanonicalCookie::ValidateAndAdjustExpiryDate(
     // * network_handler.cc::MakeCookieFromProtocolValues
     fixed_creation_date = base::Time::Now();
   }
-  base::Time maximum_expiry_date = fixed_creation_date + base::Days(400);
+  base::Time maximum_expiry_date;
+  if (!cookie_util::IsTimeLimitedInsecureCookiesEnabled() ||
+      scheme == net::CookieSourceScheme::kSecure) {
+    maximum_expiry_date = fixed_creation_date + base::Days(400);
+  } else {
+    maximum_expiry_date = fixed_creation_date + base::Hours(3);
+  }
   if (expiry_date > maximum_expiry_date) {
     return maximum_expiry_date;
   }
@@ -582,9 +589,6 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::Create(
     cookie_server_time = server_time.value();
 
   DCHECK(!creation_time.is_null());
-  Time cookie_expires = CanonicalCookie::ParseExpiration(
-      parsed_cookie, creation_time, cookie_server_time);
-  cookie_expires = ValidateAndAdjustExpiryDate(cookie_expires, creation_time);
 
   CookiePrefix prefix_case_sensitive =
       GetCookiePrefix(parsed_cookie.Name(), /*check_insensitively=*/false);
@@ -676,6 +680,11 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::Create(
   int source_port = CanonicalCookie::GetAndAdjustPortForTrustworthyUrls(
       url, parsed_cookie.IsSecure());
 
+  Time cookie_expires = CanonicalCookie::ParseExpiration(
+      parsed_cookie, creation_time, cookie_server_time);
+  cookie_expires =
+      ValidateAndAdjustExpiryDate(cookie_expires, creation_time, source_scheme);
+
   auto cc = std::make_unique<CanonicalCookie>(
       base::PassKey<CanonicalCookie>(), parsed_cookie.Name(),
       parsed_cookie.Value(), cookie_domain, cookie_path, creation_time,
@@ -888,7 +897,8 @@ std::unique_ptr<CanonicalCookie> CanonicalCookie::CreateSanitizedCookie(
     status->AddExclusionReason(
         net::CookieInclusionStatus::EXCLUDE_FAILURE_TO_STORE);
   }
-  expiration_time = ValidateAndAdjustExpiryDate(expiration_time, creation_time);
+  expiration_time = ValidateAndAdjustExpiryDate(expiration_time, creation_time,
+                                                source_scheme);
 
   if (!status->IsInclude())
     return nullptr;
@@ -1444,8 +1454,10 @@ bool CanonicalCookie::IsCanonical() const {
   // TODO(crbug.com/1264458): Eventually we should push this logic into
   // IsCanonicalForFromStorage, but for now we allow cookies already stored with
   // high expiration dates to be retrieved.
-  if (ValidateAndAdjustExpiryDate(expiry_date_, creation_date_) != expiry_date_)
+  if (ValidateAndAdjustExpiryDate(expiry_date_, creation_date_,
+                                  SourceScheme()) != expiry_date_) {
     return false;
+  }
 
   return IsCanonicalForFromStorage();
 }

net/cookies/canonical_cookie.h

@@ -463,9 +463,9 @@ class NET_EXPORT CanonicalCookie {
 
   // Per rfc6265bis the maximum expiry date is no further than 400 days in the
   // future.
-  static base::Time ValidateAndAdjustExpiryDate(
-      const base::Time& expiry_date,
-      const base::Time& creation_date);
+  static base::Time ValidateAndAdjustExpiryDate(const base::Time& expiry_date,
+                                                const base::Time& creation_date,
+                                                net::CookieSourceScheme scheme);
 
   // Cookie ordering methods.
 

net/cookies/canonical_cookie_unittest.cc

@@ -2739,6 +2739,47 @@ TEST(CanonicalCookieTest, IncludeForRequestURL_DomainCookiesPortMatch) {
   }
 }
 
+TEST(CanonicalCookieTest, InsecureCookiesExpiryTimeLimit) {
+  GURL url("http://www.example.com/test/foo.html");
+  base::Time creation_time = base::Time::Now();
+  base::Time future_date = creation_time + base::Days(1);
+  {
+    base::test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.InitWithFeatures(
+        {features::kEnableSchemeBoundCookies,
+         features::kTimeLimitedInsecureCookies},
+        {});
+    std::unique_ptr<CanonicalCookie> cookie = CanonicalCookie::Create(
+        url, "A=1; expires=" + HttpUtil::TimeFormatHTTP(future_date),
+        creation_time, /*server_time=*/absl::nullopt,
+        /*cookie_partition_key=*/absl::nullopt);
+    ASSERT_TRUE(cookie);
+    // With the feature enabled, expiration time should be limited to 3 hours
+    // after creation. Equality check needs to have a second margin due to
+    // microsecond rounding causing breakage.
+    EXPECT_TRUE(((creation_time + base::Hours(3)) - cookie->ExpiryDate())
+                    .FloorToMultiple(base::Seconds(1))
+                    .is_zero());
+  }
+  {
+    base::test::ScopedFeatureList scoped_feature_list;
+    scoped_feature_list.InitWithFeatures(
+        {features::kEnableSchemeBoundCookies},
+        {features::kTimeLimitedInsecureCookies});
+    std::unique_ptr<CanonicalCookie> cookie = CanonicalCookie::Create(
+        url, "A=1; expires=" + HttpUtil::TimeFormatHTTP(future_date),
+        creation_time, /*server_time=*/absl::nullopt,
+        /*cookie_partition_key=*/absl::nullopt);
+    ASSERT_TRUE(cookie);
+    // With the feature disabled, expiration time should not be limited.
+    // Equality check needs to have a second margin due to microsecond rounding
+    // causing breakage.
+    EXPECT_TRUE((future_date - cookie->ExpiryDate())
+                    .FloorToMultiple(base::Seconds(1))
+                    .is_zero());
+  }
+}
+
 TEST(CanonicalCookieTest, MultipleExclusionReasons) {
   GURL url("http://www.not-secure.com/foo");
   base::Time creation_time = base::Time::Now();

net/cookies/cookie_util.cc

@@ -890,6 +890,11 @@ bool IsSchemeBoundCookiesEnabled() {
   return base::FeatureList::IsEnabled(features::kEnableSchemeBoundCookies);
 }
 
+bool IsTimeLimitedInsecureCookiesEnabled() {
+  return IsSchemeBoundCookiesEnabled() &&
+         base::FeatureList::IsEnabled(features::kTimeLimitedInsecureCookies);
+}
+
 bool IsSchemefulSameSiteEnabled() {
   return base::FeatureList::IsEnabled(features::kSchemefulSameSite);
 }

net/cookies/cookie_util.h

@@ -284,6 +284,8 @@ NET_EXPORT bool IsPortBoundCookiesEnabled();
 
 NET_EXPORT bool IsSchemeBoundCookiesEnabled();
 
+NET_EXPORT bool IsTimeLimitedInsecureCookiesEnabled();
+
 // Returns whether the respective feature is enabled.
 NET_EXPORT bool IsSchemefulSameSiteEnabled();
 

services/network/cookie_manager.cc

@@ -133,8 +133,8 @@ void CookieManager::SetCanonicalCookie(const net::CanonicalCookie& cookie,
 
   auto cookie_ptr = std::make_unique<net::CanonicalCookie>(cookie);
   base::Time adjusted_expiry_date =
-      net::CanonicalCookie::ValidateAndAdjustExpiryDate(cookie.ExpiryDate(),
-                                                        cookie.CreationDate());
+      net::CanonicalCookie::ValidateAndAdjustExpiryDate(
+          cookie.ExpiryDate(), cookie.CreationDate(), cookie.SourceScheme());
   if (adjusted_expiry_date != cookie.ExpiryDate() || !cookie_partition_key) {
     cookie_ptr = net::CanonicalCookie::FromStorage(
         cookie.Name(), cookie.Value(), cookie.Domain(), cookie.Path(),