Count embedded pages that didn't explicitly opt-in.

This CL adds a new WebFeature metric for pages that are embedded via
<frame> or <iframe> by default, without explicitly setting an
X-Frame-Options header or frame-ancestors CSP directive.

https://github.com/mikewest/embedding-requires-opt-in

Bug: 1153274
Change-Id: I5bb45df3eb2aec0010f2ddd2dfe457551de9d40d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2623010
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#842597}

chrome/browser/chrome_web_platform_security_metrics_browsertest.cc

@@ -280,6 +280,72 @@ IN_PROC_BROWSER_TEST_F(ChromeWebPlatformSecurityMetricsBrowserTest,
   ExpectHistogramIncreasedBy(1);
 }
 
+// Check kEmbeddedCrossOriginFrameWithoutFrameAncestorsOrXFO feature usage.
+// This should increment in cases where a cross-origin frame is embedded which
+// does not assert either X-Frame-Options or CSP's frame-ancestors.
+IN_PROC_BROWSER_TEST_F(ChromeWebPlatformSecurityMetricsBrowserTest,
+                       EmbeddingOptIn) {
+  set_monitored_feature(
+      blink::mojom::WebFeature::
+          kEmbeddedCrossOriginFrameWithoutFrameAncestorsOrXFO);
+  GURL main_document_url = https_server().GetURL("a.com", "/title1.html");
+
+  struct TestCase {
+    const char* name;
+    const char* host;
+    const char* header;
+    bool expect_counter;
+  } cases[] = {{
+                   "Same-origin, no XFO, no frame-ancestors",
+                   "a.com",
+                   nullptr,
+                   false,
+               },
+               {
+                   "Cross-origin, no XFO, no frame-ancestors",
+                   "b.com",
+                   nullptr,
+                   true,
+               },
+               {
+                   "Same-origin, yes XFO, no frame-ancestors",
+                   "a.com",
+                   "X-Frame-Options: ALLOWALL",
+                   false,
+               },
+               {
+                   "Cross-origin, yes XFO, no frame-ancestors",
+                   "b.com",
+                   "X-Frame-Options: ALLOWALL",
+                   false,
+               },
+               {
+                   "Same-origin, no XFO, yes frame-ancestors",
+                   "a.com",
+                   "Content-Security-Policy: frame-ancestors *",
+                   false,
+               },
+               {
+                   "Cross-origin, no XFO, yes frame-ancestors",
+                   "b.com",
+                   "Content-Security-Policy: frame-ancestors *",
+                   false,
+               }};
+
+  for (auto test : cases) {
+    SCOPED_TRACE(test.name);
+    EXPECT_TRUE(content::NavigateToURL(web_contents(), main_document_url));
+
+    std::string path = "/set-header?";
+    if (test.header)
+      path += test.header;
+    GURL url = https_server().GetURL(test.host, path);
+    LoadIFrame(url);
+
+    ExpectHistogramIncreasedBy(test.expect_counter ? 1 : 0);
+  }
+}
+
 // TODO(arthursonzogni): Add basic test(s) for the WebFeatures:
 // - CrossOriginOpenerPolicySameOrigin
 // - CrossOriginOpenerPolicySameOriginAllowPopups

content/browser/renderer_host/ancestor_throttle.cc

@@ -23,10 +23,12 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
 #include "net/http/http_response_headers.h"
 #include "services/network/public/cpp/content_security_policy/csp_context.h"
 #include "third_party/blink/public/mojom/devtools/console_message.mojom.h"
+#include "third_party/blink/public/mojom/web_feature/web_feature.mojom.h"
 
 namespace content {
 
@@ -453,15 +455,11 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateXFrameOptions(
 
 AncestorThrottle::CheckResult AncestorThrottle::EvaluateEmbeddingOptIn(
     LoggingDisposition logging) {
-  if (!base::FeatureList::IsEnabled(features::kEmbeddingRequiresOptIn))
-    return CheckResult::PROCEED;
-
+  // If the proposal in https://github.com/mikewest/embedding-requires-opt-in is
+  // enabled, a response will be blocked unless it's explicitly opted-into
+  // being embeddable via 'X-Frame-Options'/'frame-ancestors', or is same-origin
+  // with its ancestors.
   NavigationRequest* request = NavigationRequest::From(navigation_handle());
-
-  // If embedding requires opt-in, then we check whether the response opted-into
-  // embedding via either an 'X-Frame-Options' header or a 'frame-ancestors'
-  // directive. If neither is present, the response will be blocked unless it is
-  // same-origin with its ancestor chain.
   if (request->response()->parsed_headers->xfo ==
           network::mojom::XFrameOptionsValue::kNone &&
       !HeadersContainFrameAncestorsCSP(request->response()->parsed_headers)) {
@@ -471,8 +469,16 @@ AncestorThrottle::CheckResult AncestorThrottle::EvaluateEmbeddingOptIn(
         url::Origin::Create(navigation_handle()->GetURL());
     while (parent) {
       if (!parent->GetLastCommittedOrigin().IsSameOriginWith(current_origin)) {
+        GetContentClient()->browser()->LogWebFeatureForCurrentPage(
+            parent, blink::mojom::WebFeature::
+                        kEmbeddedCrossOriginFrameWithoutFrameAncestorsOrXFO);
+
+        if (!base::FeatureList::IsEnabled(features::kEmbeddingRequiresOptIn))
+          return CheckResult::PROCEED;
+
         if (logging == LoggingDisposition::LOG_TO_CONSOLE)
           ConsoleErrorEmbeddingRequiresOptIn();
+
         return CheckResult::BLOCK;
       }
       parent = ParentOrOuterDelegate(parent);

third_party/blink/public/mojom/web_feature/web_feature.mojom

@@ -3070,6 +3070,7 @@ enum WebFeature {
   kEmbedElementWithoutTypeSrcChanged = 3749,
   kPaymentHandlerStandardizedPaymentMethodIdentifier = 3750,
   kWebCodecsAudioEncoder = 3751,
+  kEmbeddedCrossOriginFrameWithoutFrameAncestorsOrXFO = 3752,
 
   // Add new features immediately above this line. Don't change assigned
   // numbers of any item, and don't reuse removed slots.

tools/metrics/histograms/enums.xml

@@ -30841,6 +30841,7 @@ Called by update_use_counter_feature_enum.py.-->
   <int value="3749" label="EmbedElementWithoutTypeSrcChanged"/>
   <int value="3750" label="PaymentHandlerStandardizedPaymentMethodIdentifier"/>
   <int value="3751" label="WebCodecsAudioEncoder"/>
+  <int value="3752" label="EmbeddedCrossOriginFrameWithoutFrameAncestorsOrXFO"/>
 </enum>
 
 <enum name="FeaturePolicyAllowlistType">