Avoid reparsing Content-Security-Policy headers in AncestorThrottle

The logic checking the X-Frame-Options header in the AncestorThrottle
was manually parsing the Content-Security-Policy headers for finding
out whether the 'frame-ancestors' directive was used (since the
X-Frame-Options header should be ignored if the 'frame-ancestors' CSP
directive is being used).

With this change we use instead the already parsed
Content-Security-Policy in parsed_headers for checking that. This
avoids unnecessary additional parsings of the CSP headers in the
AncestorThrottle.

Bug: 1032139
Change-Id: Ibe0c64cd7eb1e6f8ff0fa915a81db6248399da1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2292051Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: Antonio Sartori <antoniosartori@chromium.org>
Cr-Commit-Position: refs/heads/master@{#789919}

content/browser/BUILD.gn

@@ -49,6 +49,7 @@ jumbo_source_set("browser") {
     "//base:base_static",
     "//base:clang_profiling_buildflags",
     "//base/third_party/dynamic_annotations",
+    "//base/util/ranges:ranges",
     "//build:branding_buildflags",
     "//build:lacros_buildflags",
     "//cc",

content/browser/frame_host/ancestor_throttle.cc

@@ -8,6 +8,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/util/ranges/algorithm.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/navigation_request.h"
@@ -79,27 +80,15 @@ void RecordXFrameOptionsUsage(XFrameOptionsHistogram usage) {
       XFrameOptionsHistogram::XFRAMEOPTIONS_HISTOGRAM_MAX);
 }
 
-bool HeadersContainFrameAncestorsCSP(const net::HttpResponseHeaders* headers,
-                                     bool include_report_only) {
-  std::vector<std::string> header_names = {"content-security-policy"};
-  if (include_report_only)
-    header_names.push_back("content-security-policy-report-only");
-  for (const auto& header : header_names) {
-    size_t iter = 0;
-    std::string value;
-    while (headers->EnumerateHeader(&iter, header, &value)) {
-      // A content-security-policy is a semicolon-separated list of directives.
-      for (const auto& directive : base::SplitStringPiece(
-               value, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) {
-        // The trailing " " is intentional; we'd otherwise match
-        // "frame-ancestors-is-not-this-directive".
-        if (base::StartsWith(directive, "frame-ancestors ",
-                             base::CompareCase::INSENSITIVE_ASCII))
-          return true;
-      }
-    }
-  }
-  return false;
+bool HeadersContainFrameAncestorsCSP(
+    const network::mojom::ParsedHeadersPtr& headers) {
+  return util::ranges::any_of(
+      headers->content_security_policy, [](const auto& csp) {
+        return csp->header->type ==
+                   network::mojom::ContentSecurityPolicyType::kEnforce &&
+               csp->directives.count(
+                   network::mojom::CSPDirectiveName::FrameAncestors);
+      });
 }
 
 class FrameAncestorCSPContext : public network::CSPContext {
@@ -285,6 +274,16 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateXFrameOptions(
   HeaderDisposition disposition =
       ParseXFrameOptionsHeader(request->GetResponseHeaders(), &header_value);
 
+  // If 'X-Frame-Options' would potentially block the response, check whether
+  // the 'frame-ancestors' CSP directive should take effect instead. See
+  // https://www.w3.org/TR/CSP/#frame-ancestors-and-frame-options
+  if (disposition != HeaderDisposition::NONE &&
+      disposition != HeaderDisposition::ALLOWALL &&
+      HeadersContainFrameAncestorsCSP(request->response()->parsed_headers)) {
+    RecordXFrameOptionsUsage(XFrameOptionsHistogram::BYPASS);
+    return CheckResult::PROCEED;
+  }
+
   switch (disposition) {
     case HeaderDisposition::CONFLICT:
       if (logging == LoggingDisposition::LOG_TO_CONSOLE)
@@ -339,9 +338,6 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateXFrameOptions(
     case HeaderDisposition::NONE:
       RecordXFrameOptionsUsage(XFrameOptionsHistogram::NONE);
       return CheckResult::PROCEED;
-    case HeaderDisposition::BYPASS:
-      RecordXFrameOptionsUsage(XFrameOptionsHistogram::BYPASS);
-      return CheckResult::PROCEED;
     case HeaderDisposition::ALLOWALL:
       RecordXFrameOptionsUsage(XFrameOptionsHistogram::ALLOWALL);
       return CheckResult::PROCEED;
@@ -524,18 +520,6 @@ AncestorThrottle::HeaderDisposition AncestorThrottle::ParseXFrameOptionsHeader(
       result = HeaderDisposition::CONFLICT;
   }
 
-  // If 'X-Frame-Options' would potentially block the response, check whether
-  // the 'frame-ancestors' CSP directive should take effect instead. See
-  // https://www.w3.org/TR/CSP/#frame-ancestors-and-frame-options
-  if (result != HeaderDisposition::NONE &&
-      result != HeaderDisposition::ALLOWALL &&
-      // TODO(antoniosartori): Use the already parsed CSP header instead of the
-      // raw headers here as soon as we remove
-      // network::features::kOutOfBlinkFrameAncestors
-      HeadersContainFrameAncestorsCSP(headers, false)) {
-    return HeaderDisposition::BYPASS;
-  }
-
   return result;
 }
 

content/browser/frame_host/ancestor_throttle.h

@@ -37,8 +37,7 @@ class CONTENT_EXPORT AncestorThrottle : public NavigationThrottle {
     SAMEORIGIN,
     ALLOWALL,
     INVALID,
-    CONFLICT,
-    BYPASS
+    CONFLICT
   };
 
   static std::unique_ptr<NavigationThrottle> MaybeCreateThrottleFor(