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

Send feature policy in sub-resource request headers #20

Closed
jkarlin opened this issue Jun 21, 2016 · 11 comments
Closed

Send feature policy in sub-resource request headers #20

jkarlin opened this issue Jun 21, 2016 · 11 comments
Milestone

Comments

@jkarlin
Copy link

jkarlin commented Jun 21, 2016

This seems to be intended, but it's not actually stated in the spec, that sub-resource requests should include the feature policy in its headers.

@clelland
Copy link
Collaborator

From the resolution of #7, and commits ed93b0c and bc67bf9, it looks like the resolution is actually that requests never include the policy; it's a response header only.

@ojanvafai
Copy link
Collaborator

I think we probably want some way to send these in request headers, but I'm concerned that if it's the default it will be a lot of wasted bytes on every network request.

@igrigorik
Copy link
Member

tl;dr: I think we can revert those changes; we should advertise the active policy.

A brief history...

Originally, the thought was that we should advertise the active policy and have the server reflect back ("ack") the policy in the response; absence of such reflected policy would then block the response. This was modeled after the embedded-csp proposal. Then, after discussing it some more, we realized that the "ack" is hard to implement for many + basically unnecesasry: we don't need to block the resource, we enforce the policy by restricting access to APIs. Given that, we dropped the header because it seemed unnecessary.

Later (through various conversations at BlinkOn) we identified that advertising the policy is, in fact, very useful after all, as it provides a signal to the embedee for what policies will be enforced on its content. So, I think we revert the earlier commits and drop the 'reflect' bits..

I think we probably want some way to send these in request headers, but I'm concerned that if it's the default it will be a lot of wasted bytes on every network request.

The FP request header would only be present on pages that specify a Feature-Policy; it's an opt-in header. I think this fine.

@ojanvafai
Copy link
Collaborator

I think we probably want some way to send these in request headers, but I'm concerned that if it's the default it will be a lot of wasted bytes on every network request.

The FP request header would only be present on pages that specify a Feature-Policy; it's an opt-in header. I think this fine.

Imagine a future world in which almost every page is using FP. Now it's not fine anymore. This isn't theoretical, this might actually happen.

Could we start with just exposing the active policy to the main HTML resource of each frame and to JS? Then pages can do what they want for subresources (e.g. put in a cookie, append as query parameters to URLs, etc).

@igrigorik
Copy link
Member

Then pages can do what they want for subresources (e.g. put in a cookie, append as query parameters to URLs, etc).

In which case they're worse off, since headers would be compressed via HPACK (h2), whereas their cookies and query string params would only make the problem worse... shrug

I'm not against this, but it feels like we're micro-optimizing.

@clelland
Copy link
Collaborator

Cookies are in the header, and should probably compress slightly better than the FP header, which isn't in the static header field list. The problem with them is that you shouldn't be able to set cookies for third-party content, so you have to fall back to URLs anyway.

@igrigorik igrigorik added this to the v1 milestone Jul 26, 2016
@igrigorik
Copy link
Member

Imagine a future world in which almost every page is using FP. Now it's not fine anymore. This isn't theoretical, this might actually happen.

Origin-Policy looks like a promising solution to address this: https://mikewest.github.io/origin-policy/

igrigorik added a commit that referenced this issue Aug 2, 2016
This reverts commit ed93b0c.

See discussion in #20.
@igrigorik
Copy link
Member

As I'm working on this, ran across an interesting case. Consider this...

Feature-Policy: {"disable":["webrtc"], "target":["https://example.com"]},
        {"enable":["vibrate"], "target":
          ["https://foo.com", "https://bar.com"]}

 <iframe src="https://foo.com/thing"></iframe>
  • Disable is always propagated down - e.g. if parent disables webrtc on example.com, then foo.com embedee automatically inherits same policy.
  • Should enable policy be propagated down as well?
    • Embedder delegates access to Vibrate API to foo.com
    • Embedder delegates access to Vibrate API to bar.com

Does the above mean that embedder enabled self -> foo.com -> bar.com? OR, does the enable policy only apply for the context that sets it and bar.com does not automagically propagate down to foo.com. In other words, can embedder delegate permission on behalf of the embedee? As currently written in 3e3acb6 this is allowed. Should it be?

/cc @raymeskhoury @noncombatant

@igrigorik
Copy link
Member

Based on a VC chat today: the initial take is that enable should not propagate down. So, in above example foo.com would have to explicitly (re)enable bar.com if it wants to delegate permission to it.

@igrigorik igrigorik modified the milestones: vNext, v1 Nov 23, 2016
@igrigorik
Copy link
Member

For v1 (see #43) we're targeting a small set of ~permissions features, which don't (strictly) need this advertisement. For v2+, once we tackle features that are default on and don't have a permissions prompt.. we'll need to (a) provide some JS mechanism to query status and (b) send the request header policy.

@annevk
Copy link
Member

annevk commented Apr 24, 2019

Closing this as this is tackled by newer issues around sandboxing.

@annevk annevk closed this as completed Apr 24, 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

No branches or pull requests

5 participants