Fenced frame: introduce `allow` attribute
This CL introduces a new `allow` attribute for fenced frames. This is
based off of the attribute of the same name for iframes, and has the
same syntax for building up the permissions policies.
The attribute is sent through a new fenced frame mojo call,
`DidChangeFramePolicy`, where it is read into the fenced frame's frame
tree node to be processed during navigation.
`NavigationRequest::CheckPermissionsPoliciesForFencedFrames` now
includes checks for the fenced frame's frame policy on top of the checks
currently in place for its parent's policies.
When the fenced frame configuration code is in place, the k-anonymous
permissions needed for the fenced frame to load will be checked against
the allowed permissions which includes the "allow" attribute. Until
then, this uses the same restrictions that are currently in place for
the parent's permissions policies of a fenced frame. That is, if a
permissions policy is explicitly specified with the `allow` attribute,
attribution reporting and shared storage must be enabled for the frame's
origin.
See the "Permission policy for API backed fenced frames" explainer:
https://github.com/WICG/fenced-frame/blob/master/explainer/permissions_policy_for_API_backed_fenced_frames.md
Change-Id: I9b0e35a41edaa0dab5f7e573c0a5c4ff2c1afa61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3999825
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Dominic Farolino <dom@chromium.org>
Commit-Queue: Liam Brady <lbrady@google.com>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1078585}
diff --git a/content/browser/fenced_frame/fenced_frame.cc b/content/browser/fenced_frame/fenced_frame.cc
index cbb10ae..0441b5fb 100644
--- a/content/browser/fenced_frame/fenced_frame.cc
+++ b/content/browser/fenced_frame/fenced_frame.cc
@@ -277,4 +277,13 @@
void FencedFrame::UpdateOverridingUserAgent() {}
+void FencedFrame::DidChangeFramePolicy(const blink::FramePolicy& frame_policy) {
+ FrameTreeNode* inner_root = frame_tree_->root();
+ const blink::FramePolicy& current_frame_policy =
+ inner_root->pending_frame_policy();
+ inner_root->SetPendingFramePolicy(blink::FramePolicy(
+ current_frame_policy.sandbox_flags, frame_policy.container_policy,
+ current_frame_policy.required_document_policy));
+}
+
} // namespace content
diff --git a/content/browser/fenced_frame/fenced_frame.h b/content/browser/fenced_frame/fenced_frame.h
index 69a08d7..6c24d58 100644
--- a/content/browser/fenced_frame/fenced_frame.h
+++ b/content/browser/fenced_frame/fenced_frame.h
@@ -57,6 +57,7 @@
// blink::mojom::FencedFrameOwnerHost implementation.
void Navigate(const GURL& url,
base::TimeTicks navigation_start_time) override;
+ void DidChangeFramePolicy(const blink::FramePolicy& frame_policy) override;
// FrameTree::Delegate.
void DidStartLoading(FrameTreeNode* frame_tree_node,
diff --git a/content/browser/fenced_frame/fenced_frame_browsertest.cc b/content/browser/fenced_frame/fenced_frame_browsertest.cc
index ba69ce62..d20610e 100644
--- a/content/browser/fenced_frame/fenced_frame_browsertest.cc
+++ b/content/browser/fenced_frame/fenced_frame_browsertest.cc
@@ -379,6 +379,10 @@
fenced_frame_->Navigate(url, navigation_start_time);
}
+ void DidChangeFramePolicy(const blink::FramePolicy& frame_policy) override {
+ fenced_frame_->DidChangeFramePolicy(frame_policy);
+ }
+
private:
mojo::AssociatedRemote<blink::mojom::FencedFrameOwnerHost> original_remote_;
mojo::AssociatedReceiver<blink::mojom::FencedFrameOwnerHost> receiver_{
diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc
index c6bafa8..24ceaf2 100644
--- a/content/browser/renderer_host/frame_tree_node.cc
+++ b/content/browser/renderer_host/frame_tree_node.cc
@@ -480,6 +480,15 @@
pending_frame_policy_.required_document_policy =
frame_policy.required_document_policy;
}
+
+ // Fenced frame roots do not have a parent, so add an extra check here to
+ // still allow a fenced frame to properly set its container policy. The
+ // required document policy and sandbox flags should stay unmodified.
+ if (IsFencedFrameRoot()) {
+ DCHECK(pending_frame_policy_.required_document_policy.empty());
+ DCHECK_EQ(pending_frame_policy_.sandbox_flags, frame_policy.sandbox_flags);
+ pending_frame_policy_.container_policy = frame_policy.container_policy;
+ }
}
void FrameTreeNode::SetAttributes(
diff --git a/content/browser/renderer_host/frame_tree_node.h b/content/browser/renderer_host/frame_tree_node.h
index cc6507f..e678bffa 100644
--- a/content/browser/renderer_host/frame_tree_node.h
+++ b/content/browser/renderer_host/frame_tree_node.h
@@ -228,8 +228,10 @@
// this frame. This includes flags inherited from parent frames and the latest
// flags from the <iframe> element hosting this frame. The returned policies
// may not yet have taken effect, since "sandbox" and "allow" attribute
- // updates in an <iframe> element take effect on next navigation. To retrieve
- // the currently active policy for this frame, use effective_frame_policy().
+ // updates in an <iframe> element take effect on next navigation. For
+ // <fencedframe> elements, not everything in the frame policy might actually
+ // take effect after the navigation. To retrieve the currently active policy
+ // for this frame, use effective_frame_policy().
const blink::FramePolicy& pending_frame_policy() const {
return pending_frame_policy_;
}
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 20e4532a..6865af82 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -154,6 +154,7 @@
#include "third_party/blink/public/common/navigation/navigation_params_mojom_traits.h"
#include "third_party/blink/public/common/navigation/navigation_policy.h"
#include "third_party/blink/public/common/permissions_policy/document_policy.h"
+#include "third_party/blink/public/common/permissions_policy/permissions_policy_features.h"
#include "third_party/blink/public/common/permissions_policy/policy_helper_public.h"
#include "third_party/blink/public/common/renderer_preferences/renderer_preferences.h"
#include "third_party/blink/public/common/security/address_space_feature.h"
@@ -7653,27 +7654,62 @@
return true;
}
+bool NavigationRequest::IsFencedFrameRequiredPolicyFeatureAllowed(
+ const url::Origin& origin,
+ const blink::mojom::PermissionsPolicyFeature feature) {
+ const blink::PermissionsPolicyFeatureList& feature_list =
+ blink::GetPermissionsPolicyFeatureList();
+
+ // Check if the outer document's permissions policies allow all of the
+ // required policies for `origin`.
+ if (GetParentFrameOrOuterDocument()
+ ->permissions_policy()
+ ->GetAllowlistForFeatureIfExists(feature) &&
+ !GetParentFrameOrOuterDocument()
+ ->permissions_policy()
+ ->IsFeatureEnabledForOrigin(feature, origin)) {
+ return false;
+ }
+
+ // Check if the container policies to be committed allow all of the required
+ // policies for `origin`. This means that the policy must be either
+ // explicitly enabled for `origin`, or the policy must by default
+ // be enabled for all origins. Note: because the policies have not been
+ // read into a RenderFrameHost's permissions_policy_ yet, we need to check
+ // the ParsedPermissionsPolicyDeclaration object directly.
+ auto policy_iter = std::find_if(
+ commit_params_->frame_policy.container_policy.begin(),
+ commit_params_->frame_policy.container_policy.end(),
+ [feature](const blink::ParsedPermissionsPolicyDeclaration& d) {
+ return d.feature == feature;
+ });
+ if (policy_iter == commit_params_->frame_policy.container_policy.end()) {
+ return feature_list.at(feature) ==
+ blink::PermissionsPolicyFeatureDefault::EnableForAll;
+ }
+
+ return policy_iter->Contains(origin);
+}
+
bool NavigationRequest::CheckPermissionsPoliciesForFencedFrames(
const url::Origin& origin) {
// These checks only apply to fenced frames.
if (!frame_tree_node_->IsFencedFrameRoot())
return true;
+ // Check that all of the required policies for a new document with origin
+ // `origin` in the fenced frame are allowed. This looks at the outer
+ // document's policies and the "allow" attribute. Note that the document will
+ // eventually only use the required policies without policy inheritance, so
+ // extra policies defined in the outer document/"allow" attribute won't have
+ // any effect.
for (const blink::mojom::PermissionsPolicyFeature feature :
blink::kFencedFrameOpaqueAdsDefaultAllowedFeatures) {
- // Only check if the feature is enabled for this origin if
- // a policy was explicitly specified.
- if (GetParentFrameOrOuterDocument()
- ->permissions_policy()
- ->GetAllowlistForFeatureIfExists(feature) &&
- !GetParentFrameOrOuterDocument()
- ->permissions_policy()
- ->IsFeatureEnabledForOrigin(feature, origin)) {
+ if (!IsFencedFrameRequiredPolicyFeatureAllowed(origin, feature)) {
const blink::PermissionsPolicyFeatureToNameMap& feature_to_name_map =
blink::GetPermissionsPolicyFeatureToNameMap();
- const std::string feature_string(
- (feature_to_name_map.find(feature))->second);
- AddDeferredConsoleMessage(
+ const std::string feature_string(feature_to_name_map.at(feature));
+ frame_tree_node_->current_frame_host()->AddMessageToConsole(
blink::mojom::ConsoleMessageLevel::kError,
base::StringPrintf(
"Refused to frame '%s' as a fenced frame because permissions "
diff --git a/content/browser/renderer_host/navigation_request.h b/content/browser/renderer_host/navigation_request.h
index d04636e..cdc2aeac 100644
--- a/content/browser/renderer_host/navigation_request.h
+++ b/content/browser/renderer_host/navigation_request.h
@@ -1500,11 +1500,17 @@
// If they aren't, this returns false and emits a crash report.
bool CoopCoepSanityCheck();
- // Checks if all of the permissions policies that a fenced frame requires to
- // be enabled for its origin are enabled. If not, it logs a console message
- // and returns false.
+ // Checks that, given an origin to be committed, all of the permissions
+ // policies that a fenced frame requires to be enabled are enabled. If not, it
+ // logs a console message and returns false.
bool CheckPermissionsPoliciesForFencedFrames(const url::Origin&);
+ // Helper function that determines if a given required permissions policy
+ // feature is properly enabled for a given origin to be committed.
+ bool IsFencedFrameRequiredPolicyFeatureAllowed(
+ const url::Origin&,
+ const blink::mojom::PermissionsPolicyFeature feature);
+
// Returns the user-agent override, or an empty string if one isn't set.
std::string GetUserAgentOverride();
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 9576c68..72b9361 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -636,8 +636,11 @@
const url::Origin& subframe_origin) {
std::unique_ptr<blink::PermissionsPolicy> subframe_policy;
if (frame->IsNestedWithinFencedFrame()) {
- // In Fenced Frames, all permission policy gated features must be disabled
- // for privacy reasons.
+ // Fenced frames have a list of required permission policies to load and
+ // can't be granted extra policies, so use the required policies instead of
+ // inheriting from its parent. Note that the parent policies must allow the
+ // required policies, which is checked separately in
+ // NavigationRequest::CheckPermissionsPoliciesForFencedFrames.
subframe_policy = blink::PermissionsPolicy::CreateForFencedFrame(
subframe_origin,
frame->frame_tree_node()->GetFencedFrameMode().value());
@@ -10195,8 +10198,11 @@
void RenderFrameHostImpl::ResetPermissionsPolicy() {
if (IsNestedWithinFencedFrame()) {
- // In Fenced Frames, all permission policy gated features must be disabled
- // for privacy reasons.
+ // Fenced frames have a list of required permission policies to load and
+ // can't be granted extra policies, so use the required policies instead of
+ // inheriting from its parent. Note that the parent policies must allow the
+ // required policies, which is checked separately in
+ // NavigationRequest::CheckPermissionsPoliciesForFencedFrames.
permissions_policy_ = blink::PermissionsPolicy::CreateForFencedFrame(
last_committed_origin_,
frame_tree_node()->GetFencedFrameMode().value());
