Require-Document-Policy header no longer affect current document

Previously if a http response has both Document-Policy and
Require-Document-Policy header and they are incompatible, the document
will be blocked, which is not the correct behavior. This CL corrects
the behavior of document policy under such condition.

Bug: 993790
Change-Id: I5fc6b5c213fa7d56d473a8ea7e10235dd418ba42
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015443
Commit-Queue: Charlie Hu <chenleihu@google.com>
Reviewed-by: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736886}

third_party/blink/renderer/core/frame/frame.h

@@ -31,7 +31,7 @@
 
 #include "base/optional.h"
 #include "base/unguessable_token.h"
-#include "third_party/blink/public/common/frame/frame_policy.h"
+#include "third_party/blink/public/common/feature_policy/document_policy.h"
 #include "third_party/blink/public/common/frame/user_activation_state.h"
 #include "third_party/blink/public/common/frame/user_activation_update_source.h"
 #include "third_party/blink/public/mojom/ad_tagging/ad_frame.mojom-blink.h"
@@ -244,10 +244,13 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
     opener_feature_state_ = state;
   }
 
-  const FramePolicy& GetFramePolicy() const { return frame_policy_; }
+  const DocumentPolicy::FeatureState& GetRequiredDocumentPolicy() const {
+    return required_document_policy_;
+  }
 
-  void SetFramePolicy(const FramePolicy& frame_policy) {
-    frame_policy_ = frame_policy;
+  void SetRequiredDocumentPolicy(
+      const DocumentPolicy::FeatureState& required_document_policy) {
+    required_document_policy_ = required_document_policy;
   }
 
   WindowAgentFactory& window_agent_factory() const {
@@ -328,10 +331,12 @@ class CORE_EXPORT Frame : public GarbageCollected<Frame> {
   // frames.
   FeaturePolicy::FeatureState opener_feature_state_;
 
-  // Frame policy of current frame. This can be different to
-  // Owner()->GetFramePolicy(), as the document hosted in the frame can also
-  // further specify frame policy.
-  FramePolicy frame_policy_;
+  // The required document policy for any subframes of this frame.
+  // Note: current frame's document policy might not conform to
+  // |required_document_policy_| here, as the Require-Document-Policy HTTP
+  // header can specify required document policy which only takes effect for
+  // subtree frames.
+  DocumentPolicy::FeatureState required_document_policy_;
 
   Member<WindowAgentFactory> window_agent_factory_;
 

third_party/blink/renderer/core/html/html_frame_owner_element.cc

@@ -316,8 +316,7 @@ void HTMLFrameOwnerElement::UpdateRequiredPolicy() {
   DCHECK(frame);
   frame_policy_.required_document_policy = DocumentPolicy::MergeFeatureState(
       self_required_policy,
-      frame->GetFramePolicy()
-          .required_document_policy /* parent required policy */);
+      frame->GetRequiredDocumentPolicy() /* parent required policy */);
   if (ContentFrame()) {
     frame->Client()->DidChangeFramePolicy(ContentFrame(), frame_policy_);
   }

third_party/blink/renderer/core/loader/document_loader.cc

@@ -176,10 +176,22 @@ DocumentLoader::DocumentLoader(
   force_fetch_cache_mode_ = params_->force_fetch_cache_mode;
   response_ = params_->response.ToResourceResponse();
   frame_policy_ = params_->frame_policy.value_or(FramePolicy());
+
   document_policy_ = CreateDocumentPolicy();
-  // Initialize |frame_policy_| in frame after the update to
-  // |frame_policy_.required_document_policy| in CreateDocumentPolicy.
-  frame_->SetFramePolicy(frame_policy_);
+  // If the document is blocked by document policy, there won't be content
+  // in the sub-frametree, thus no need to initialize required_policy for
+  // subtree.
+  if (!was_blocked_by_document_policy_) {
+    // Require-Document-Policy header only affects subtree of current document,
+    // but not the current document.
+    const DocumentPolicy::FeatureState header_required_policy =
+        DocumentPolicy::Parse(
+            response_.HttpHeaderField(http_names::kRequireDocumentPolicy)
+                .Ascii())
+            .value_or(DocumentPolicy::FeatureState{});
+    frame_->SetRequiredDocumentPolicy(DocumentPolicy::MergeFeatureState(
+        frame_policy_.required_document_policy, header_required_policy));
+  }
 
   WebNavigationTimings& timings = params_->navigation_timings;
   if (!timings.input_start.is_null())
@@ -806,19 +818,11 @@ DocumentPolicy::FeatureState DocumentLoader::CreateDocumentPolicy() {
   if (!RuntimeEnabledFeatures::DocumentPolicyEnabled())
     return DocumentPolicy::FeatureState{};
 
-  const DocumentPolicy::FeatureState header_policy =
+  DocumentPolicy::FeatureState header_policy =
       DocumentPolicy::Parse(
           response_.HttpHeaderField(http_names::kDocumentPolicy).Ascii())
           .value_or(DocumentPolicy::FeatureState{});
 
-  const DocumentPolicy::FeatureState header_required_policy =
-      DocumentPolicy::Parse(
-          response_.HttpHeaderField(http_names::kRequireDocumentPolicy).Ascii())
-          .value_or(DocumentPolicy::FeatureState{});
-
-  frame_policy_.required_document_policy = DocumentPolicy::MergeFeatureState(
-      frame_policy_.required_document_policy, header_required_policy);
-
   if (!DocumentPolicy::IsPolicyCompatible(
           frame_policy_.required_document_policy, header_policy)) {
     was_blocked_by_document_policy_ = true;
@@ -826,6 +830,7 @@ DocumentPolicy::FeatureState DocumentLoader::CreateDocumentPolicy() {
     // policy to initialize document policy for the document.
     return frame_policy_.required_document_policy;
   }
+
   return header_policy;
 }
 

third_party/blink/renderer/core/loader/document_loader_test.cc

@@ -296,6 +296,23 @@ TEST_F(DocumentLoaderSimTest, ReportErrorWhenDocumentPolicyIncompatible) {
   EXPECT_EQ(child_document->Url(), SecurityOrigin::UrlWithUniqueOpaqueOrigin());
 }
 
+// HTTP header Require-Document-Policy should only take effect on subtree of
+// current document, but not on current document.
+TEST_F(DocumentLoaderSimTest,
+       RequireDocumentPolicyHeaderShouldNotAffectCurrentDocument) {
+  blink::ScopedDocumentPolicyForTest sdp(true);
+  SimRequest::Params params;
+  params.response_http_headers = {
+      {"Require-Document-Policy", "unoptimized-lossless-images;bpp=1.0"},
+      {"Document-Policy", "unoptimized-lossless-images;bpp=1.1"}};
+
+  SimRequest main_resource("https://example.com", "text/html", params);
+  LoadURL("https://example.com");
+  // If document is blocked by document policy because of incompatible document
+  // policy, this test will fail by crashing here.
+  main_resource.Finish();
+}
+
 TEST_F(DocumentLoaderTest, CommitsDeferredOnSameOriginNavigation) {
   const KURL& requestor_url =
       KURL(NullURL(), "https://www.example.com/foo.html");