From 24d13009071b12ce38f3b9b5a4a82cc1812d56d1 Mon Sep 17 00:00:00 2001
From: Łukasz Burchard <lburchard@google.com>
Date: Wed, 16 Sep 2026 03:47:12 -0700
Subject: [PATCH] [variations] Do not trust unauthenticated Date header over HTTP retry

When an initial variations seed fetch over HTTPS fails, Chrome falls
back to an HTTP retry. HTTP responses lack transport security, so the
response headers cannot be authenticated and must not be trusted.

Previously, a 304 Not Modified response received over the HTTP retry
called RecordSuccessfulFetchSeedNotModified(), which updated the stored
kVariationsSeedDate using the unauthenticated Date header. An attacker
could exploit this by forcing HTTP fallback and spoofing a 304 response
with an arbitrary future Date header, manipulating date-gated field
trials or bypassing study expiry checks. Similarly, on an HTTP 200
response, the unauthenticated Date header was passed to StoreSeed().

This change resolves the vulnerability:
1. In OnSimpleLoaderComplete(), sets response_date to std::nullopt when
   last_request_was_http_retry_ is true, ignoring unauthenticated Date
   headers.
2. For HTTP 304 responses:
   - Records the last fetch time using local base::Time::Now() and
     notifies SafeSeedManager to reset failure streaks, preventing
     unnecessary retries and false safe mode triggers.
   - Updates the stored seed date in
     RecordSuccessfulFetchSeedNotModified() only when response_date
     has a value (received over HTTPS). Skipping this on insecure 304
     responses prevents date spoofing and avoids spurious day changes in
     Variations.SeedDateChange.
3. For HTTP 200 responses:
   - Passes response_date.value_or(base::Time()) to StoreSeed(), passing
     a null base::Time() when fetched over an HTTP retry.
4. For clients that only succeed over HTTP retry, kVariationsSeedDate
   remains un-updated. This causes
   VariationsSeedStore::GetTimeForStudyDateChecks() to fall back to
   base::GetBuildTime(), which is safe against remote manipulation.

TAG=agy
CONV=9f31a747-adcd-4fb7-9cd8-7957f219668b

Bug: 511820971
Change-Id: I7694358d46c688660059b6d8c66243ecb1a6c80f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7994437
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Commit-Queue: Łukasz Burchard <lburchard@google.com>
Reviewed-by: Andrzej Fiedukowicz <afie@google.com>
Cr-Commit-Position: refs/heads/main@{#1698815}
---
diff --git a/components/variations/service/variations_service.cc b/components/variations/service/variations_service.cc
index 192aa8f3..b2c3d46 100644
--- a/components/variations/service/variations_service.cc
+++ b/components/variations/service/variations_service.cc
@@ -1144,10 +1144,12 @@
   DCHECK(headers);
   DCHECK(response_body);
 
-  std::optional<base::Time> response_date = headers->GetDateValue();
+  // Ignore the Date header value from HTTP as it cannot be trusted.
+  const std::optional<base::Time> response_date =
+      last_request_was_http_retry_ ? std::nullopt : headers->GetDateValue();
   // If the seed was fetched securely, opportunistically update the network time
   // tracker with the headers time.
-  if (response_date && !last_request_was_http_retry_) {
+  if (response_date) {
     DCHECK(!response_date->is_null());
 
     const base::TimeDelta latency = now - last_request_started_time_;
@@ -1158,7 +1160,7 @@
 
   if (response_code == net::HTTP_NOT_MODIFIED) {
     // TODO(crbug.com/420652919): Reject responses without a date.
-    RecordSuccessfulFetchSeedNotModified(response_date.value_or(base::Time()));
+    RecordSuccessfulFetchSeedNotModified(response_date);
     return;
   }
 
@@ -1526,18 +1528,19 @@
 }
 
 void VariationsService::RecordSuccessfulFetchSeedNotModified(
-    base::Time response_date) {
+    std::optional<base::Time> response_date) {
   // Update the client-side fetch time to the current time.
   field_trial_creator_.seed_store()->RecordLastFetchTime(base::Time::Now());
   safe_seed_manager_.RecordSuccessfulFetch(field_trial_creator_.seed_store());
 
-  // Update the seed date value in local state (used for expiry check on
-  // next start up), since 304 is a successful response. Note that the
-  // serial number included in the request is always that of the latest
-  // seed, even when running in safe mode, so it's appropriate to always
-  // modify the latest seed's date.
-  field_trial_creator_.seed_store()->UpdateSeedDateAndLogDayChange(
-      response_date);
+  // Only treat a 304 as confirmation that the stored seed is current when it
+  // was received over HTTPS. Callers set `response_date` to std::nullopt for
+  // insecure HTTP retries because unauthenticated Date headers cannot be
+  // trusted.
+  if (response_date) {
+    field_trial_creator_.seed_store()->UpdateSeedDateAndLogDayChange(
+        response_date.value());
+  }
 }
 
 VariationsSeedStore* VariationsService::GetSeedStoreForTesting() {
diff --git a/components/variations/service/variations_service.h b/components/variations/service/variations_service.h
index 042c7b9..80aa691 100644
--- a/components/variations/service/variations_service.h
+++ b/components/variations/service/variations_service.h
@@ -378,10 +378,11 @@
   // streaks.
   void RecordSuccessfulFetchNewSeed();
 
-  // Like VariationsService::RecordSuccessfulFetchNewSeed(), but intended to be
-  // called for 304 responses from the variations server. Also, updates the seed
-  // date and client fetch time.
-  void RecordSuccessfulFetchSeedNotModified(base::Time response_date);
+  // Records a 304 response from the variations server. Updates the client fetch
+  // time and clears failure streaks. If `response_date` has a value (present on
+  // secure HTTPS responses), updates the stored seed date.
+  void RecordSuccessfulFetchSeedNotModified(
+      std::optional<base::Time> response_date);
 
   // Performs a simulation of the given `seed` to find any runtime mutable
   // changes that need to be applied to the current session, and apply them.
diff --git a/components/variations/service/variations_service_unittest.cc b/components/variations/service/variations_service_unittest.cc
index c6dccde..799909cc 100644
--- a/components/variations/service/variations_service_unittest.cc
+++ b/components/variations/service/variations_service_unittest.cc
@@ -192,6 +192,7 @@
   bool delta_compressed_seed() const { return delta_compressed_seed_; }
   bool gzip_compressed_seed() const { return gzip_compressed_seed_; }
   bool runtime_simulation_called() const { return runtime_simulation_called_; }
+  base::Time stored_date() const { return stored_date_; }
 
   bool CallMaybeRetryOverHTTP() { return CallMaybeRetryOverHTTPForTesting(); }
   void SimulateAndApplyRuntimeMutableChanges(
@@ -247,6 +248,7 @@
     stored_seed_data_ = seed_data;
     stored_country_ = country_code;
     stored_geo_level_ = geo_level1;
+    stored_date_ = date_fetched;
     delta_compressed_seed_ = is_delta_compressed;
     gzip_compressed_seed_ = is_gzip_compressed;
     OnSeedStoreResult(is_delta_compressed, seed_stores_succeed_,
@@ -277,6 +279,7 @@
   bool runtime_simulation_called_ = false;
   base::OnceClosure fetch_intercepted_callback_;
   std::string last_header_serial_number_;
+  base::Time stored_date_;
 };
 
 class TestVariationsServiceObserver : public VariationsService::Observer {
@@ -1095,6 +1098,88 @@
   EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsFailedToFetchSeedStreak));
 }
 
+TEST_F(VariationsServiceTest, NotModifiedOverHttpRetryDoesNotUpdateState) {
+  // Set a fixed past reference date (2024-01-01 00:00:00 UTC).
+  base::Time initial_time;
+  ASSERT_TRUE(
+      base::Time::FromUTCString("2024-01-01 00:00:00 UTC", &initial_time));
+  prefs_.SetInteger(prefs::kVariationsCrashStreak, 2);
+  prefs_.SetInteger(prefs::kVariationsFailedToFetchSeedStreak, 1);
+  prefs_.SetTime(prefs::kVariationsSeedDate, initial_time);
+  prefs_.SetTime(prefs::kVariationsLastFetchTime, initial_time);
+  VariationsService::EnableFetchForTesting();
+
+  TestVariationsService service(
+      std::make_unique<web_resource::TestRequestAllowedNotifier>(
+          &prefs_, network_tracker_),
+      &prefs_, GetMetricsStateManager(), /*use_secure_url=*/false);
+  service.set_intercepts_fetch(false);
+
+  std::string headers("HTTP/1.1 304 Not Modified\n\n");
+  auto head = network::mojom::URLResponseHead::New();
+  head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      net::HttpUtil::AssembleRawHeaders(headers));
+  // Set a spoofed Date header in the future relative to initial_time.
+  head->headers->SetHeader(
+      "Date", net::HttpUtil::TimeFormatHTTP(initial_time + base::Days(30)));
+  network::URLLoaderCompletionStatus status;
+  service.test_url_loader_factory()->AddResponse(service.interception_url(),
+                                                 std::move(head), "", status);
+
+  service.set_last_request_was_retry(false);
+  service.set_insecure_url(service.interception_url());
+  EXPECT_TRUE(service.CallMaybeRetryOverHTTP());
+  base::RunLoop().RunUntilIdle();
+
+  // A 304 received over the HTTP retry should not be treated as confirmation
+  // that the stored seed is current, so the seed date should not be updated.
+  // However, since we successfully reached the server, the last fetch time
+  // and failure streaks should be updated (using the local time, which cannot
+  // be spoofed).
+  EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsCrashStreak));
+  EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsFailedToFetchSeedStreak));
+  EXPECT_EQ(initial_time, prefs_.GetTime(prefs::kVariationsSeedDate));
+  EXPECT_NE(initial_time, prefs_.GetTime(prefs::kVariationsLastFetchTime));
+}
+
+TEST_F(VariationsServiceTest, SeedDateIgnoredOverHttpRetry) {
+  // Set a fixed past reference date (2024-01-01 00:00:00 UTC).
+  base::Time initial_time;
+  ASSERT_TRUE(
+      base::Time::FromUTCString("2024-01-01 00:00:00 UTC", &initial_time));
+  prefs_.SetTime(prefs::kVariationsSeedDate, initial_time);
+  prefs_.SetTime(prefs::kVariationsLastFetchTime, initial_time);
+  VariationsService::EnableFetchForTesting();
+
+  TestVariationsService service(
+      std::make_unique<web_resource::TestRequestAllowedNotifier>(
+          &prefs_, network_tracker_),
+      &prefs_, GetMetricsStateManager(), /*use_secure_url=*/false);
+  service.set_intercepts_fetch(false);
+
+  // Return 200 OK with a spoofed Date header in the future.
+  std::string headers("HTTP/1.1 200 OK\n\n");
+  auto head = network::mojom::URLResponseHead::New();
+  head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      net::HttpUtil::AssembleRawHeaders(headers));
+  // Set a spoofed Date header in the future relative to initial_time.
+  head->headers->SetHeader(
+      "Date", net::HttpUtil::TimeFormatHTTP(initial_time + base::Days(30)));
+  network::URLLoaderCompletionStatus status;
+  std::string seed_body = SerializeSeed(CreateTestSeed());
+  service.test_url_loader_factory()->AddResponse(
+      service.interception_url(), std::move(head), seed_body, status);
+
+  service.set_last_request_was_retry(false);
+  service.set_insecure_url(service.interception_url());
+  EXPECT_TRUE(service.CallMaybeRetryOverHTTP());
+  base::RunLoop().RunUntilIdle();
+
+  // The seed date should NOT be set to the future date. It should be
+  // base::Time() because the fetch was insecure.
+  EXPECT_EQ(base::Time(), service.stored_date());
+}
+
 TEST_F(VariationsServiceTest, FieldTrialCreatorInitializedCorrectly) {
   TestVariationsService service(
       std::make_unique<web_resource::TestRequestAllowedNotifier>(

diff --git a/components/variations/service/variations_service_unittest.cc b/components/variations/service/variations_service_unittest.cc
index c6dccde..799909cc 100644
--- a/components/variations/service/variations_service_unittest.cc
+++ b/components/variations/service/variations_service_unittest.cc
@@ -192,6 +192,7 @@
   bool delta_compressed_seed() const { return delta_compressed_seed_; }
   bool gzip_compressed_seed() const { return gzip_compressed_seed_; }
   bool runtime_simulation_called() const { return runtime_simulation_called_; }
+  base::Time stored_date() const { return stored_date_; }
 
   bool CallMaybeRetryOverHTTP() { return CallMaybeRetryOverHTTPForTesting(); }
   void SimulateAndApplyRuntimeMutableChanges(
@@ -247,6 +248,7 @@
     stored_seed_data_ = seed_data;
     stored_country_ = country_code;
     stored_geo_level_ = geo_level1;
+    stored_date_ = date_fetched;
     delta_compressed_seed_ = is_delta_compressed;
     gzip_compressed_seed_ = is_gzip_compressed;
     OnSeedStoreResult(is_delta_compressed, seed_stores_succeed_,
@@ -277,6 +279,7 @@
   bool runtime_simulation_called_ = false;
   base::OnceClosure fetch_intercepted_callback_;
   std::string last_header_serial_number_;
+  base::Time stored_date_;
 };
 
 class TestVariationsServiceObserver : public VariationsService::Observer {
@@ -1095,6 +1098,88 @@
   EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsFailedToFetchSeedStreak));
 }
 
+TEST_F(VariationsServiceTest, NotModifiedOverHttpRetryDoesNotUpdateState) {
+  // Set a fixed past reference date (2024-01-01 00:00:00 UTC).
+  base::Time initial_time;
+  ASSERT_TRUE(
+      base::Time::FromUTCString("2024-01-01 00:00:00 UTC", &initial_time));
+  prefs_.SetInteger(prefs::kVariationsCrashStreak, 2);
+  prefs_.SetInteger(prefs::kVariationsFailedToFetchSeedStreak, 1);
+  prefs_.SetTime(prefs::kVariationsSeedDate, initial_time);
+  prefs_.SetTime(prefs::kVariationsLastFetchTime, initial_time);
+  VariationsService::EnableFetchForTesting();
+
+  TestVariationsService service(
+      std::make_unique<web_resource::TestRequestAllowedNotifier>(
+          &prefs_, network_tracker_),
+      &prefs_, GetMetricsStateManager(), /*use_secure_url=*/false);
+  service.set_intercepts_fetch(false);
+
+  std::string headers("HTTP/1.1 304 Not Modified\n\n");
+  auto head = network::mojom::URLResponseHead::New();
+  head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      net::HttpUtil::AssembleRawHeaders(headers));
+  // Set a spoofed Date header in the future relative to initial_time.
+  head->headers->SetHeader(
+      "Date", net::HttpUtil::TimeFormatHTTP(initial_time + base::Days(30)));
+  network::URLLoaderCompletionStatus status;
+  service.test_url_loader_factory()->AddResponse(service.interception_url(),
+                                                 std::move(head), "", status);
+
+  service.set_last_request_was_retry(false);
+  service.set_insecure_url(service.interception_url());
+  EXPECT_TRUE(service.CallMaybeRetryOverHTTP());
+  base::RunLoop().RunUntilIdle();
+
+  // A 304 received over the HTTP retry should not be treated as confirmation
+  // that the stored seed is current, so the seed date should not be updated.
+  // However, since we successfully reached the server, the last fetch time
+  // and failure streaks should be updated (using the local time, which cannot
+  // be spoofed).
+  EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsCrashStreak));
+  EXPECT_EQ(0, prefs_.GetInteger(prefs::kVariationsFailedToFetchSeedStreak));
+  EXPECT_EQ(initial_time, prefs_.GetTime(prefs::kVariationsSeedDate));
+  EXPECT_NE(initial_time, prefs_.GetTime(prefs::kVariationsLastFetchTime));
+}
+
+TEST_F(VariationsServiceTest, SeedDateIgnoredOverHttpRetry) {
+  // Set a fixed past reference date (2024-01-01 00:00:00 UTC).
+  base::Time initial_time;
+  ASSERT_TRUE(
+      base::Time::FromUTCString("2024-01-01 00:00:00 UTC", &initial_time));
+  prefs_.SetTime(prefs::kVariationsSeedDate, initial_time);
+  prefs_.SetTime(prefs::kVariationsLastFetchTime, initial_time);
+  VariationsService::EnableFetchForTesting();
+
+  TestVariationsService service(
+      std::make_unique<web_resource::TestRequestAllowedNotifier>(
+          &prefs_, network_tracker_),
+      &prefs_, GetMetricsStateManager(), /*use_secure_url=*/false);
+  service.set_intercepts_fetch(false);
+
+  // Return 200 OK with a spoofed Date header in the future.
+  std::string headers("HTTP/1.1 200 OK\n\n");
+  auto head = network::mojom::URLResponseHead::New();
+  head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+      net::HttpUtil::AssembleRawHeaders(headers));
+  // Set a spoofed Date header in the future relative to initial_time.
+  head->headers->SetHeader(
+      "Date", net::HttpUtil::TimeFormatHTTP(initial_time + base::Days(30)));
+  network::URLLoaderCompletionStatus status;
+  std::string seed_body = SerializeSeed(CreateTestSeed());
+  service.test_url_loader_factory()->AddResponse(
+      service.interception_url(), std::move(head), seed_body, status);
+
+  service.set_last_request_was_retry(false);
+  service.set_insecure_url(service.interception_url());
+  EXPECT_TRUE(service.CallMaybeRetryOverHTTP());
+  base::RunLoop().RunUntilIdle();
+
+  // The seed date should NOT be set to the future date. It should be
+  // base::Time() because the fetch was insecure.
+  EXPECT_EQ(base::Time(), service.stored_date());
+}
+
 TEST_F(VariationsServiceTest, FieldTrialCreatorInitializedCorrectly) {
   TestVariationsService service(
       std::make_unique<web_resource::TestRequestAllowedNotifier>(