Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support propagating feature policy in popups. #259

Closed
wants to merge 1 commit into from

Conversation

clelland
Copy link
Collaborator

@clelland clelland commented Nov 30, 2018

This change accounts for the fact that not all top-level browsing
contexts should be treated as top-level documents by feature policy. In
particular, new windows created with window.open() and <a href="_blank">
have auxiliary browsing contexts, which should get an inherited policy
based on the document that created them.

<ol>
<li>Let <var>opener</var> be <var>context</var>'s [=opener browsing
context=]'s [=active document=].</li>
<li>If either <var>context</var> is not <a
Copy link
Contributor

@ehsan-karamad ehsan-karamad Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this suggestions means that a disowned auxiliary browsing context with a sandbox-ed opener (that has the sandbox flag to propagate sandbox to popups set) will not inherit feature policy state. This sounds good but is a different behavior from what sandbox currently does and hence I wonder if it will block implementing sandbox based on feature policy in its current form.

Should we instead special case sandbox policies here, or for now make feature policy follow sandbox guidelines when sandbox is present.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking another look I realized this instruction is suggesting what I mentioned above; to give an example,
Suppose feature sync-xhr is disabled inside a browsing context. Then the following table maps the combination of sandbox and rel to the feature state in the auxiliary context (i.e., results of document.policy.allowsFeature('sync-xhr')):

<iframe> has: \ window is: rel='noopener' rel=''
sandbox present false false
sandbox present, allow-popups-to-escape-sandbox present true true
sandbox not present true false

aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 19, 2019
This CL introduces the "inherited opener feature policies". This
includes the logic to propagate feature policy states from a browsing
context to the auxiliary browsing contexts.

As the first step (and hidden behind flag) all the feature policies
will be inherited by the auxiliary browsing context. The only exception
is when the original context is sandboxed but allows popups to escape
sandbox.

The inheritance model will be fine tuned in further work. Firstly, not
all features might follow this "sandbox-like" inheritance model. Also
possibly through introducing a new Feature Policy (that replicates
'allow-popups-to-escape-sandbox') and special casing "rel='noopener'"
there will be exit doors for the open contexts to *not* inherit the
policies.

These issues are currently publicly being tracked here:

w3c/webappsec-permissions-policy#264
w3c/webappsec-permissions-policy#252
w3c/webappsec-permissions-policy#259

Bug: 774620
Change-Id: Ic0b5ab8155c2e5d786bc51d3f9c3a601f7e4d8e9
Reviewed-on: https://chromium-review.googlesource.com/c/1384992
Reviewed-by: Ehsan Karamad <[email protected]>
Reviewed-by: Mike West <[email protected]>
Reviewed-by: Ian Clelland <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Ehsan Karamad <[email protected]>
Cr-Commit-Position: refs/heads/master@{#633452}
@clelland
Copy link
Collaborator Author

clelland commented Dec 6, 2019

After a number of discussions on this topic, I'm closing this; the consensus right now seems to be that the features which are best suited for feature policy are those which are reset in popups anyway -- permissions, powerful features, etc, for which any requests for access can be attributed to a new top-level origin, and visible window.

@clelland clelland closed this Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants