Review request for Feature Policy

[clelland]

Hello TAG!

I'm requesting a TAG review of:

• Name: Feature Policy
• Specification URL: https://wicg.github.io/feature-policy/
• Explainer, Requirements Doc, or Example code: https://docs.google.com/document/d/1k0Ua-ZWlM_PsFCFdLMa8kaVTo32PeNZ4G7FFHqpFx4E/edit?usp=sharing
• Primary contacts: clelland, raymeskhoury, ojanvafai

Further details:

• I have read and filled out the Self-Review Questionnare on Security and Privacy. The assessment is here.
• I have reviewed the TAG's API Design Principles

You should also know that...

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 clelland, raymeskhoury, ojanvafai

[dbaron]

One initial comment, based on a quick look, is that I think it might be helpful if the Other and related mechanisms section were a bit clearer. It's not clear to me what some of the prose in it means. But, more importantly, as the number of feature restriction mechanisms increases, the need for clear guidance to future designers of platform features (spec authors and implementors) becomes more important.

I expect the TAG is going to need to add a section to our design principles document offering suggestions about which mechanisms should be used in which contexts; we've already had this sort of discussion a number of times, leading to w3ctag/design-principles#41 being filed, but I expect the issue will need to be even broader.

So I think it would probably be useful if the authors of this specification were to provide a clearer statement, probably in that section (at least for now, although we might need to put it someplace common eventually), of when they think future features should be added to feature-policy vs. CSP vs. values of the sandbox attribute vs. allow-* attributes on iframe.

[clelland]

Thanks, @dbaron -- I'll see about cleaning that section up. It's getting a bit dated now, and it certainly would be good to make the intentions clearer, especially wrt. sandbox attributes.

@mikewest or @ojanvafai can probably also comment on this, but I think that the eventual goal is to implement most of the sandbox attributes as policy-controlled features (possibly leaving them in sandbox for compatibility, but essentially being syntactic sugar for the same thing)

[ojanvafai]

Yup, I think we should expose all sandbox things to feature policy. But I also agree with dbaron than we should take it on ourselves to draft a one-pager that outlines the various ways that features can be controlled, with rough guidelines on when to use which, including things from WebIDL like SecureContext.

See related chromium thread: https://groups.google.com/a/chromium.org/forum/#!topic/platform-architecture-dev/z3eD8u_qgbk

[torgo]

Suggest the explainer doc should be in the same repo as the spec…

[dbaron]

Other questions that came up in another brief TAG discussion (some of which are just things we want to understand):

• how does this interact with CSP?
• how do the policies propagate into subframes -- is there a merging mechanism?
• are the name used here unified with names used by the permissions API?

We're going to try to do more in a subgroup.

[triblondon]

Punting to a subgroup disucssion. In particular:

• integration with perms API
• interplay of multiple feature policies
• whitelist vs blacklist reasoning
• fail closed / fail open

[slightlyoff]

Related: #147

[triblondon]

Thanks for bringing this to TAG review! We're collecting feedback here ahead of opening issues:

• Glad to see you have an explainer!

• The cascade effect of parent header > allow attribute > iframe header is confusing. For example, if a parent document defines vibrate: ["self"], the iframe has allow="vibrate", and the iframe policy header has vibrate:[], then we believe this will prohibit vibrate in the iframe, but since the allow attribute is more visible to the developer this may cause significant head scratching. We wonder how compelling are the use cases for the attribute, given the expressiveness of the header?

• If an iframe is permitted to use a feature but chooses to disable it in its own header, it presumably retains the power to re-enable it on an embedded iframe using an attribute, whereas if the feature was disabled by a policy in the parent document, it will not be allowed to re-enable it on a nested frame. Should there be a requirement for the UA to issue an expressive error if an allow attribute attempts to enable a feature that has been prohibited at a higher level?

• The allow attribute and the HTTP header have very different granularity. While the header takes a list of allowed origins, the attribute's syntax is essentially sugar for featurename:["*"], with no ability to be more granular. We recognise that this is similar to the relationship between the CSP header and the sandbox attribute, but it feels very inconsistent, and a higher than necessary learning curve.

• Under the default allowlists header, the phrase "The feature is allowed at the top level by default, and when allowed, is allowed by default to documents in child frames." is rather confusing, and suggests to us that perhaps the allow attribute is required in order to extend a feature-policy defined in the header, into child frames (which doesn't make a lot of sense because in that case there'd be no reason to have multiple origins listed in the header policy). Perhaps this is a hangover from a previous version of the spec in which there was a disable attribute as well as the enable (now allow) one?

• The default behaviour of new features added in the future must depend on the shipping status of the feature in the platform. For example, if a document.write policy is added it must "fail open" in order to avoid breaking existing websites, and therefore the only safe scenario in which a feature could be introduced with a closed default status if is if support for the feature in feature policy is a required part of shipping the feature itself. Is there a mechanism for tying together the introduction of a feature in the platform and the addition of a new token to this specification? (see also registry point)

• The explainer includes reference to the ability to disable, as well as enable, features, but the current spec appears to only support allowlists

• The terms used in the Explainer and the spec are not consistent. Some examples in the explainer use enable and disable attributes, whereas the draft spec includes only allow

• There appears to be some overlap between the sandbox and allow attributes. Have the designers considered migrating sandbox directives to Feature Policy? This might enable equivalence between these, and eventual deprecation of sandbox in favour of allow:

  <iframe sandbox="allow-forms allow-modals" allow="geolocation" ...>
<iframe sandbox allow="forms modals geolocation" ...>

  With the transitionary, compatible use of:

  <iframe sandbox="allow-forms allow-modals" allow="forms modals geolocation" ...>

• This spec introduces a new registry of names. What is the strategy for maintaining consistency between the Permissions API registry and naming and the feature names in this spec?

• Related, we acknowledge that registry maintainence is an unresolved issue in the platform. We're going look into development of a unified solution for many specs and will ping back here if we make progress.

[plinss]

Closed pending feedback