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

Feature policy control over sandbox features #339

Closed
3 of 5 tasks
clelland opened this issue Jan 29, 2019 · 13 comments
Closed
3 of 5 tasks

Feature policy control over sandbox features #339

clelland opened this issue Jan 29, 2019 · 13 comments
Assignees

Comments

@clelland
Copy link

clelland commented Jan 29, 2019

こんにちはTAG!

I'm requesting a TAG review of:

Further details (optional):

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@dbaron
Copy link
Member

dbaron commented Feb 4, 2019

Am I correct in my interpretation of the specification and explainer links that the material this review is about is covered by the linked explainer, but hasn't yet been edited into the spec?

@clelland
Copy link
Author

clelland commented Feb 4, 2019

Yes, that's correct; the spec hasn't been updated yet.

@dbaron dbaron self-assigned this Feb 5, 2019
@torgo torgo assigned slightlyoff, torgo and dbaron and unassigned dbaron Feb 5, 2019
@torgo torgo added this to the 2019-02-05-f2f milestone Feb 5, 2019
@slightlyoff
Copy link
Member

slightlyoff commented Feb 5, 2019

Hey @clelland,

Thanks for filing for a review. A few questions from today's F2F discussion with @torgo, @sideshowbarker, @ylafon, @torgo, and @alice.

This work appears to be a follow-up from our suggestions in #159 that the allow attribute superset the set of sandbox attributes. Thanks for doing this.

  • What's the interaction between a non-null sandbox policy and allow? E.g., if sandboxing prevents something, but allow enables it, who wins? The processing order doesn't seem to be specified.
  • Do you have data about the usage of sandbox attributes? On what timeframe would a deprecation be possible?
  • As a meta point, the explainer doesn't really address the question of why this solves an important problem for developers. Given that this might make controlling feature use easier and provide a single way to do it, that seems worth calling out.
  • An aside: it seems like there's some overlap between document.featurePolicy.allowsFeature() and the Permissions API. How is that going to be resolved? Are the semantics the same? Which one should a developer use? Are events sent to document.featurePolicy to note when things are relaxed or tightened?
  • What, if any mechanism, is being proposed to ensure that allow attributes continue to be a strict superset of sandbox policies? Are you asking the TAG to stay vigilant here? Should it go into our design guidance?

@torgo
Copy link
Member

torgo commented Feb 5, 2019

Just expanding on that meta-point, it would be good to also give an indication or example of what end-user benefit is derived, following on from the developer problem solved.

@torgo torgo removed the extra time label Feb 5, 2019
@clelland
Copy link
Author

clelland commented Feb 6, 2019

Hey @clelland,

Thanks for filing for a review. A few questions from today's F2F discussion with @torgo, @sideshowbarker, @ylafon, @torgo, and @alice.

This work appears to be a follow-up from our suggestions in #159 that the allow attribute superset the set of sandbox attributes. Thanks for doing this.

  • What's the interaction between a non-null sandbox policy and allow? E.g., if sandboxing prevents something, but allow enables it, who wins? The processing order doesn't seem to be specified.

Sandboxing only prevents things by not mentioning them -- an example of sandbox preventing modal dialogs, for instance, and FP enabling it would look like

<iframe sandbox="allow-scripts" allow="dialogs">

I believe in that case the developer would expect the feature to be allowed. The model, in my head at least, is that:

  1. The presence of the sandbox attribute disables a number of features in the frame
  2. The sandbox attributes are able to re-enable those features (classic behaviour)
  3. After this, the "allow" attribute is able to re-enable any still-disabled features, or further refine the set of origins where the features will be enabled (in the case of navigation in an allow-same-origin frame)

Does that make sense? (TAG input appreciated here, at least for ergonomics. This matches the way that FP handles the legacy allowfullscreen attribute, FWIW)

  • Do you have data about the usage of sandbox attributes? On what timeframe would a deprecation be possible?
  • As a meta point, the explainer doesn't really address the question of why this solves an important problem for developers. Given that this might make controlling feature use easier and provide a single way to do it, that seems worth calling out.

+1, will do.

  • An aside: it seems like there's some overlap between document.featurePolicy.allowsFeature() and the Permissions API. How is that going to be resolved? Are the semantics the same? Which one should a developer use? Are events sent to document.featurePolicy to note when things are relaxed or tightened?

They share some identifiers (permission-features are a subset of feature policy features), but the semantics are different -- the permissions API will only return true if the user has explicitly granted permission, while the JS API (#292) will return true if access the feature has been allowed by the browser / embedding page (and hence whether a permission prompt could even potentially be shown to a user.) I would expect developers to use the FP API to decide whether to even advertise that permission controlled features could be used -- for instance, deciding whether to include a "turn camera on" button on a page, based on the embedding context. If allowed by FP, then the permissions API would could be used to present the user with a prompt, and determine the user's response.

(This should probably be continued on #292, if there's more to discuss, or even on w3c/webappsec-permissions-policy#166)

  • What, if any mechanism, is being proposed to ensure that allow attributes continue to be a strict superset of sandbox policies? Are you asking the TAG to stay vigilant here? Should it go into our design guidance?

Good question -- unless we can enshrine it in specs, TAG vigilance might be the way we have to go -- any proposals for new sandbox flags should probably prompt the question "Should this just be a policy-controlled feature instead?", quickly followed by "Should we disable it by default in sandboxes?" I suspect that the answer to the first question is going to be yes in most cases.

@dbaron
Copy link
Member

dbaron commented Mar 26, 2019

That "TAG vigilance" may be related to w3ctag/design-principles#41, which we need to address at some point...

@torgo
Copy link
Member

torgo commented May 21, 2019

@clelland is the explainer available at a URL in w3c or w3c github space? If so can you update the issue above to reflect that location?

@dbaron
Copy link
Member

dbaron commented May 21, 2019

That "TAG vigilance" may be related to w3ctag/design-principles#41, which we need to address at some point...

Though based on our discussions this morning about trying to avoid the TAG being a bottleneck for others: perhaps this is work that the TAG isn't the most suitable body to drive. In particular, it seems like perhaps some folks in the webappsec WG could take the lead in drafting something that explains how these pieces (sandbox, allow, other things mentioned in that issue) fit together and how new things are meant to be added to them.

I think this is important because we'd like it to be clear how to add things to the platform -- and I think if others write such a document there are folks on the TAG who would be interested in reviewing it -- but maybe we should avoid giving you the impression that you should be waiting for us to write it!

@dbaron dbaron added the Review type: CG early review An early review of general direction from a Community Group label May 21, 2019
@dbaron
Copy link
Member

dbaron commented May 21, 2019

The answers in #339 (comment) seem reasonable to me -- I think the biggest concern about them is that these are complicated issues and those things are going to need to be clear to developers -- so something is probably going to need to do a better job of explaining them. I'm not sure whether that's the explainer (which may eventually drive other documentation like what ends up on MDN), or the spec, or something else. But it may at least be a good start to make those things clearer in the explainer if they're not clear already, and also making sure that the explainer is reachable from the spec (since it seems like this is an explainer for a specific addition to an existing spec, so people may not go looking for this explainer in particular to understand that part of the spec).

It's not clear to me whether we want to leave this issue open to remind us to look into that, or into my previous comment, or whether we should close this issue now (and maybe re-review later once there's a solid spec, if that's appropriate).

@dbaron
Copy link
Member

dbaron commented Sep 11, 2019

@torgo and I are just looking at this in a breakout at the TAG face-to-face meeting.

I'm curious if you had any reaction to my previous #339 (comment) -- just because I think the area of how to distinguish when things should be feature-policy flags, allow attribute values, sandbox features, or a few other things that I can't keep track of is something that I think most if not all TAG members are confused by -- so I think we need to get to a state where it's documented better for people adding features to the web platform. (See w3ctag/design-principles#41.) It would be great to get something started on that and have folks like you and others in webappsec and the HTML community who have the relevant expertise contribute to it.

@torgo
Copy link
Member

torgo commented Sep 11, 2019

Just picking this up at our Tokyo f2f. This issue has stalled. Is there additional work that has gone on, @clelland? If so, can you update us and let us know what specific feedback from the TAG might be most helpful?

Will this topic be discussed in TPAC?

@dbaron dbaron added the Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review label Sep 11, 2019
@clelland
Copy link
Author

Sorry for missing that, @dbaron -- let me see if I can get the TAG up to speed here:

The biggest news is certainly the evolution of feature policy, and splitting Document Policy off of it. I expect that document policy is the place that sandbox-y features would eventually end up. The explainer goes into what that would look like, but I can answer any questions you may have on that. I believe that the processing model for document policy, specifically around required document policies, matches the existing sandbox quite closely.

Re: #339 (comment), we've certainly given that a lot of thought. I would suggest that these models make sense for different kinds of features:

  1. Feature Policy / allow -- This is suitable for permission-gated features, and for features which are powerful enough that we want to support them on the top-level origin only, unavailable in third-party frames unless delegated. The key principle here is delegation, and granting access to frames which would not otherwise have it.

  2. Document Policy -- This is most suitable for restrictions, changing the way that a feature operates away from the default, or blocking an API that should generally be available to all content. This is where I think that new sandbox-like features should go.

  3. Traditional sandboxing / iframe sandbox attribute -- This is a set of features that are all turned off as a group with the 'sandbox' attribute, coupled with an opaque origin. In my ideal world, these features are all also controllable separately, through something like Document Policy (hence this TAG review request), and 'sandbox' represents a carefully thought-out subset of features which should almost always be disabled together in a sandbox. (Sandbox also does not require any kind of acknowledgement from the embedded site, so we need to ensure that disabling features doesn't introduce any potential vulnerabilities in embedded content)

As far as TAG feedback, I think that I'm looking for guidance in these areas:

  1. Is this a good idea (Policy control over individual features, even outside of a 'sandboxed' iframe)?

  2. When 'sandbox' and a newer attribute both try to control a feature, which should take precedence? I'm personally leaning towards "old attribute must win", to ensure consistent behaviour even in old browsers, but I'm not sure that makes sense in this case, since the way to disable features with the 'sandbox' attribute is to not mention them.

  3. For future sandbox-style features, we're considering requiring the embedded site to acknowledge the policy, to mitigate security risks, refusing to load the site if the policy is not accepted explicitly. This is clearly a huge barrier to adoption, and existing sandbox features don't require that. Does the TAG have any opinions on when that explicit acknowledgment might not be required?

And yes, I'll be at TPAC, and Feature Policy / Document policy / sandboxing are on the agenda for WebAppSec.

@alice alice removed this from the 2019-09-10-f2f-tokyo milestone Jan 27, 2020
@plinss plinss added Progress: in progress and removed Progress: external help requested Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review Progress: stalled labels Mar 5, 2020
@plinss plinss added this to the 2020-03-16-week milestone Mar 5, 2020
@torgo
Copy link
Member

torgo commented Mar 16, 2020

We discussed and agreed to close this as the real activity should be going on in w3ctag/design-principles#41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants