From e415ddae9e64dcd0fec6fb0fa645d5beca0fbded Mon Sep 17 00:00:00 2001
From: Ari Chivukula <arichiv@chromium.org>
Date: Wed, 02 Sep 2026 14:26:00 -0700
Subject: [PATCH] [TLS Cert] Allow updates when user bypasses error notice

Previously, in https://crrev.com/c/8164692, we made the call to block
client-hints storage when a TLS error was present. After discussion on
https://crrev.com/c/8307253 about the same issue for the clear-site-data
header we made the call to allow this and update the security.md file to
reflect this permissiveness for items besides the page cache (as, once
loaded, durable storage like cookies can be written anyway).

Fixed: 540046516, 553283471
Bug: 540681571
Change-Id: I32646e3340ba78cafb8847fecc93ec20c93b03da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8315500
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Auto-Submit: Ari Chivukula <arichiv@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1691100}
---
diff --git a/content/SECURITY.md b/content/SECURITY.md
index 62c2af5..f5e3402 100644
--- a/content/SECURITY.md
+++ b/content/SECURITY.md
@@ -149,6 +149,14 @@
 * **Safe Browsing:** Safe Browsing is not designed to be protected against a
   compromised renderer process; we can assume that Safe Browsing already failed
   to do its job if a renderer process is compromised.
+* **TLS Certificate Error:** If the user encounters a TLS certificate error
+  interstitial and allows the page load to move forward, it is generally
+  permissable for the HTTP Headers or JavaScript of that page to cause writes
+  (i.e., Set-Cookie, document.cookie, Clear-Site-Data, Accept-CH, localStorage).
+  Data not directly manageable by the origin va HTTP Headers or JavaScript may
+  be withheld due to sensitivity (i.e., passwords or autofill) or to prevent
+  caching of insecure resources (i.e., HTTP Cache), and is still in-scope.
+
 
 ## Mitigating Factors
 
diff --git a/content/browser/client_hints/client_hints.cc b/content/browser/client_hints/client_hints.cc
index ca7e3fd..b1778eb 100644
--- a/content/browser/client_hints/client_hints.cc
+++ b/content/browser/client_hints/client_hints.cc
@@ -980,15 +980,9 @@
       container_policy, /*ftn_for_devtools_override=*/frame_tree_node);
 }
 
-// Allow Android WebView tests to disable checking the cert status, which fails
-// when tests remap origins to localhost.
-BASE_FEATURE(kBypassClientHintsSSLCertStatusForTest,
-             base::FEATURE_DISABLED_BY_DEFAULT);
-
 std::optional<std::vector<WebClientHintsType>>
 ParseAndPersistAcceptCHForNavigation(
     const url::Origin& origin,
-    const std::optional<net::SSLInfo> ssl_info,
     const network::mojom::ParsedHeadersPtr& parsed_headers,
     const net::HttpResponseHeaders* response_headers,
     BrowserContext* context,
@@ -1004,11 +998,6 @@
   if (!IsValidURLForClientHints(origin))
     return std::nullopt;
 
-  if (ssl_info && net::IsCertStatusError(ssl_info->cert_status) &&
-      !base::FeatureList::IsEnabled(kBypassClientHintsSSLCertStatusForTest)) {
-    return std::nullopt;
-  }
-
   // Client hints should only be enabled when JavaScript is enabled. Platforms
   // which enable/disable JavaScript on a per-origin basis should implement
   // IsJavaScriptAllowed to check a given origin. Other platforms (Android
diff --git a/content/browser/client_hints/client_hints.h b/content/browser/client_hints/client_hints.h
index 41f3e37..d811d027 100644
--- a/content/browser/client_hints/client_hints.h
+++ b/content/browser/client_hints/client_hints.h
@@ -138,13 +138,9 @@
 // processes, but here, we just pass the HttpRequestHeaders to the
 // TrialTokenValidator library.  There is precedent for calling the
 // TrialTokenValidator from the browser process, see crrev.com/c/2142580.
-//
-// `ssl_info` is used to check for certificate errors. If there are any errors,
-// hints are not written or read.
 CONTENT_EXPORT std::optional<std::vector<network::mojom::WebClientHintsType>>
 ParseAndPersistAcceptCHForNavigation(
     const url::Origin& origin,
-    const std::optional<net::SSLInfo> ssl_info,
     const network::mojom::ParsedHeadersPtr& parsed_headers,
     const net::HttpResponseHeaders* response_headers,
     BrowserContext* context,
diff --git a/content/browser/client_hints/critical_client_hints_throttle.cc b/content/browser/client_hints/critical_client_hints_throttle.cc
index ecfd3f2..648a6381 100644
--- a/content/browser/client_hints/critical_client_hints_throttle.cc
+++ b/content/browser/client_hints/critical_client_hints_throttle.cc
@@ -146,7 +146,7 @@
     return;
   }
 
-  ParseAndPersistAcceptCHForNavigation(response_origin, response_head.ssl_info,
+  ParseAndPersistAcceptCHForNavigation(response_origin,
                                        response_head.parsed_headers,
                                        response_head.headers.get(), context_,
                                        client_hint_delegate_, frame_tree_node);
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index f611f87..b252d37a 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -6413,7 +6413,7 @@
     const network::mojom::URLResponseHead* response_head =
         commit_params_->redirect_params.back()->response_head.get();
     ParseAndPersistAcceptCHForNavigation(
-        source_origin, response_head->ssl_info, response_head->parsed_headers,
+        source_origin, response_head->parsed_headers,
         response_head->headers.get(), browser_context, client_hints_delegate,
         frame_tree_node_);
 
@@ -7044,9 +7044,9 @@
         opt_in_hints_from_response;
     if (response()) {
       opt_in_hints_from_response = ParseAndPersistAcceptCHForNavigation(
-          url::Origin::Create(common_params_->url), response()->ssl_info,
-          response()->parsed_headers, response()->headers.get(),
-          browser_context, client_hints_delegate, frame_tree_node_);
+          url::Origin::Create(common_params_->url), response()->parsed_headers,
+          response()->headers.get(), browser_context, client_hints_delegate,
+          frame_tree_node_);
     }
     commit_params_->enabled_client_hints =
         LookupAcceptCHForCommit(origin_to_commit, client_hints_delegate,
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
index 66b300d..b16dc969 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
@@ -96,10 +96,7 @@
     @Test
     @SmallTest
     @Feature({"AndroidWebView"})
-    @CommandLineFlags.Add({
-        "enable-features=BypassClientHintsSSLCertStatusForTest",
-        ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1"
-    })
+    @CommandLineFlags.Add({ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1"})
     public void testClientHintsDefault() throws Throwable {
         final TestAwContentsClient contentsClient = new TestAwContentsClient();
         final AwContents contents =
diff --git a/content/browser/client_hints/client_hints_unittest.cc b/content/browser/client_hints/client_hints_unittest.cc
index 12583451..f5607003 100644
--- a/content/browser/client_hints/client_hints_unittest.cc
+++ b/content/browser/client_hints/client_hints_unittest.cc
@@ -93,18 +93,15 @@
 
   std::optional<ClientHintsVector> ParseAndPersist(
       const GURL& url,
-      net::CertStatus cert_status,
       const net::HttpResponseHeaders* response_header,
       const std::string& accept_ch_str,
       FrameTreeNode* frame_tree_node,
       MockClientHintsControllerDelegate* delegate) {
     auto parsed_headers = network::mojom::ParsedHeaders::New();
     parsed_headers->accept_ch = network::ParseClientHintsHeader(accept_ch_str);
-    net::SSLInfo ssl_info;
-    ssl_info.cert_status = cert_status;
 
     return ParseAndPersistAcceptCHForNavigation(
-        url::Origin::Create(url), ssl_info, parsed_headers, response_header,
+        url::Origin::Create(url), parsed_headers, response_header,
         browser_context(), delegate, frame_tree_node);
   }
 
@@ -271,38 +268,25 @@
     std::string description;
     std::string accept_ch_str;
     raw_ptr<FrameTreeNode> frame_tree_node;
-    net::CertStatus cert_status;
     std::optional<ClientHintsVector> expect_hints;
     ClientHintsVector expect_commit_hints;
   } tests[] = {
-      {"Fail to persist due to cert status error",
-       "sec-ch-ua-platform, sec-ch-ua-bitness", main_frame_node,
-       net::CERT_STATUS_ALL_ERRORS, std::nullopt, ClientHintsVector{}},
       {"Persist hints for main frame", "sec-ch-ua-platform, sec-ch-ua-bitness",
        main_frame_node,
-       /*cert_status=*/0,
        std::make_optional(ClientHintsVector{WebClientHintsType::kUAPlatform,
                                             WebClientHintsType::kUABitness}),
        ClientHintsVector{WebClientHintsType::kUAPlatform,
                          WebClientHintsType::kUABitness}},
-      {"Fail to persist hints for sub frame",
-       "sec-ch-ua-platform, sec-ch-ua-bitness", sub_frame_node,
-       /*cert_status=*/0, std::nullopt,
-       ClientHintsVector{WebClientHintsType::kUAPlatform,
-                         WebClientHintsType::kUABitness}},
-      {"Fail to update due to cert status error",
-       all_non_origin_trial_hints_pair.first, main_frame_node,
-       net::CERT_STATUS_ALL_ERRORS, std::nullopt,
+      {"No persist hints for sub frame",
+       "sec-ch-ua-platform, sec-ch-ua-bitness", sub_frame_node, std::nullopt,
        ClientHintsVector{WebClientHintsType::kUAPlatform,
                          WebClientHintsType::kUABitness}},
       {"All client hints for main frame", all_non_origin_trial_hints_pair.first,
        main_frame_node,
-       /*cert_status=*/0,
        std::make_optional(all_non_origin_trial_hints_pair.second),
        all_non_origin_trial_hints_pair.second},
       {"All client hints for sub frame", all_non_origin_trial_hints_pair.first,
-       sub_frame_node, /*cert_status=*/0, std::nullopt,
-       all_non_origin_trial_hints_pair.second},
+       sub_frame_node, std::nullopt, all_non_origin_trial_hints_pair.second},
   };
 
   for (const auto& test : tests) {
@@ -310,8 +294,8 @@
         base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK\n");
 
     auto actual_hints =
-        ParseAndPersist(url, test.cert_status, response_headers.get(),
-                        test.accept_ch_str, test.frame_tree_node, &delegate);
+        ParseAndPersist(url, response_headers.get(), test.accept_ch_str,
+                        test.frame_tree_node, &delegate);
     EXPECT_EQ(test.expect_hints, actual_hints)
         << "Test case [" << test.description << "]: expected hints "
         << HintsToString(test.expect_hints) << " but got "
@@ -350,9 +334,8 @@
 
   // We shouldn't parse accept-ch in subframe, it should not overwrite existing
   // hints.
-  auto actual_updated_hints =
-      ParseAndPersist(url, /*cert_status=*/0, response_headers.get(),
-                      accept_ch_str, sub_frame_node, &delegate);
+  auto actual_updated_hints = ParseAndPersist(
+      url, response_headers.get(), accept_ch_str, sub_frame_node, &delegate);
 
   EXPECT_EQ(std::nullopt, actual_updated_hints);
   blink::EnabledClientHints current_hints;
@@ -384,9 +367,8 @@
 
   // We shouldn't parse accept-ch in fenced frame, it should not overwrite
   // existing hints.
-  auto actual_updated_hints =
-      ParseAndPersist(url, /*cert_status=*/0, response_headers.get(),
-                      accept_ch_str, fenced_frame_node, &delegate);
+  auto actual_updated_hints = ParseAndPersist(
+      url, response_headers.get(), accept_ch_str, fenced_frame_node, &delegate);
 
   EXPECT_EQ(std::nullopt, actual_updated_hints);
   blink::EnabledClientHints current_hints;

diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
index 66b300d..b16dc969 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/ClientHintsTest.java
@@ -96,10 +96,7 @@
     @Test
     @SmallTest
     @Feature({"AndroidWebView"})
-    @CommandLineFlags.Add({
-        "enable-features=BypassClientHintsSSLCertStatusForTest",
-        ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1"
-    })
+    @CommandLineFlags.Add({ContentSwitches.HOST_RESOLVER_RULES + "=MAP * 127.0.0.1"})
     public void testClientHintsDefault() throws Throwable {
         final TestAwContentsClient contentsClient = new TestAwContentsClient();
         final AwContents contents =
diff --git a/content/browser/client_hints/client_hints_unittest.cc b/content/browser/client_hints/client_hints_unittest.cc
index 12583451..f5607003 100644
--- a/content/browser/client_hints/client_hints_unittest.cc
+++ b/content/browser/client_hints/client_hints_unittest.cc
@@ -93,18 +93,15 @@
 
   std::optional<ClientHintsVector> ParseAndPersist(
       const GURL& url,
-      net::CertStatus cert_status,
       const net::HttpResponseHeaders* response_header,
       const std::string& accept_ch_str,
       FrameTreeNode* frame_tree_node,
       MockClientHintsControllerDelegate* delegate) {
     auto parsed_headers = network::mojom::ParsedHeaders::New();
     parsed_headers->accept_ch = network::ParseClientHintsHeader(accept_ch_str);
-    net::SSLInfo ssl_info;
-    ssl_info.cert_status = cert_status;
 
     return ParseAndPersistAcceptCHForNavigation(
-        url::Origin::Create(url), ssl_info, parsed_headers, response_header,
+        url::Origin::Create(url), parsed_headers, response_header,
         browser_context(), delegate, frame_tree_node);
   }
 
@@ -271,38 +268,25 @@
     std::string description;
     std::string accept_ch_str;
     raw_ptr<FrameTreeNode> frame_tree_node;
-    net::CertStatus cert_status;
     std::optional<ClientHintsVector> expect_hints;
     ClientHintsVector expect_commit_hints;
   } tests[] = {
-      {"Fail to persist due to cert status error",
-       "sec-ch-ua-platform, sec-ch-ua-bitness", main_frame_node,
-       net::CERT_STATUS_ALL_ERRORS, std::nullopt, ClientHintsVector{}},
       {"Persist hints for main frame", "sec-ch-ua-platform, sec-ch-ua-bitness",
        main_frame_node,
-       /*cert_status=*/0,
        std::make_optional(ClientHintsVector{WebClientHintsType::kUAPlatform,
                                             WebClientHintsType::kUABitness}),
        ClientHintsVector{WebClientHintsType::kUAPlatform,
                          WebClientHintsType::kUABitness}},
-      {"Fail to persist hints for sub frame",
-       "sec-ch-ua-platform, sec-ch-ua-bitness", sub_frame_node,
-       /*cert_status=*/0, std::nullopt,
-       ClientHintsVector{WebClientHintsType::kUAPlatform,
-                         WebClientHintsType::kUABitness}},
-      {"Fail to update due to cert status error",
-       all_non_origin_trial_hints_pair.first, main_frame_node,
-       net::CERT_STATUS_ALL_ERRORS, std::nullopt,
+      {"No persist hints for sub frame",
+       "sec-ch-ua-platform, sec-ch-ua-bitness", sub_frame_node, std::nullopt,
        ClientHintsVector{WebClientHintsType::kUAPlatform,
                          WebClientHintsType::kUABitness}},
       {"All client hints for main frame", all_non_origin_trial_hints_pair.first,
        main_frame_node,
-       /*cert_status=*/0,
        std::make_optional(all_non_origin_trial_hints_pair.second),
        all_non_origin_trial_hints_pair.second},
       {"All client hints for sub frame", all_non_origin_trial_hints_pair.first,
-       sub_frame_node, /*cert_status=*/0, std::nullopt,
-       all_non_origin_trial_hints_pair.second},
+       sub_frame_node, std::nullopt, all_non_origin_trial_hints_pair.second},
   };
 
   for (const auto& test : tests) {
@@ -310,8 +294,8 @@
         base::MakeRefCounted<net::HttpResponseHeaders>("HTTP/1.1 200 OK\n");
 
     auto actual_hints =
-        ParseAndPersist(url, test.cert_status, response_headers.get(),
-                        test.accept_ch_str, test.frame_tree_node, &delegate);
+        ParseAndPersist(url, response_headers.get(), test.accept_ch_str,
+                        test.frame_tree_node, &delegate);
     EXPECT_EQ(test.expect_hints, actual_hints)
         << "Test case [" << test.description << "]: expected hints "
         << HintsToString(test.expect_hints) << " but got "
@@ -350,9 +334,8 @@
 
   // We shouldn't parse accept-ch in subframe, it should not overwrite existing
   // hints.
-  auto actual_updated_hints =
-      ParseAndPersist(url, /*cert_status=*/0, response_headers.get(),
-                      accept_ch_str, sub_frame_node, &delegate);
+  auto actual_updated_hints = ParseAndPersist(
+      url, response_headers.get(), accept_ch_str, sub_frame_node, &delegate);
 
   EXPECT_EQ(std::nullopt, actual_updated_hints);
   blink::EnabledClientHints current_hints;
@@ -384,9 +367,8 @@
 
   // We shouldn't parse accept-ch in fenced frame, it should not overwrite
   // existing hints.
-  auto actual_updated_hints =
-      ParseAndPersist(url, /*cert_status=*/0, response_headers.get(),
-                      accept_ch_str, fenced_frame_node, &delegate);
+  auto actual_updated_hints = ParseAndPersist(
+      url, response_headers.get(), accept_ch_str, fenced_frame_node, &delegate);
 
   EXPECT_EQ(std::nullopt, actual_updated_hints);
   blink::EnabledClientHints current_hints;