Feature policy seems totally broken for fullscreen

[bzbarsky]

Consider this testcase, loaded at toplevel from http://siteA.com:

<iframe src="http://siteB.com"></iframe>

with no specified feature policies of any kind. https://fullscreen.spec.whatwg.org/#security-and-privacy-considerations says:

To enable content in a nested browsing context to go fullscreen, it needs to be specifically allowed via feature policy, either through the allowfullscreen attribute of the HTML iframe element, or an appropriate declaration in the allow attribute of the HTML iframe element, or through a Feature-Policy HTTP header delivered with the document through which it is nested.

This prevents e.g. content from third parties to go fullscreen without explicit permission.

so presumably the desired behavior is that inside the subframe the "fullscreen" feature is not enabled. What is the actual specced behavior?

When the page in the iframe requests fullscreen on one of its elements, https://fullscreen.spec.whatwg.org/#fullscreen-element-ready-check calls into https://html.spec.whatwg.org/multipage/iframe-embed-object.html#allowed-to-use which calls into https://wicg.github.io/feature-policy/#is-feature-enabled and passes "fullscreen", the document inside the subframe, and the "http://siteB.com" origin.

We first look at the "inherited policy" of the framed document. This was set up via https://wicg.github.io/feature-policy/#initialize-from-response calling https://wicg.github.io/feature-policy/#initialize-for-document (not linked?) calling https://wicg.github.io/feature-policy/#define-inherited-policy calling https://wicg.github.io/feature-policy/#define-inherited-policy-in-container and passing in "fullscreen" and the iframe element.

In https://wicg.github.io/feature-policy/#define-inherited-policy-in-container we observe the following behavior, step by step:

1. parent is set to the toplevel document.
2. origin is set to "http://siteA.com".
3. container policy is an empty map.
4. skipped because the container policy is empty.
5. "fullscreen" is enabled in parent for "http://siteA.com" (see below), so "Enabled" is returned.

so the inherited policy is "Enabled". Going back to https://wicg.github.io/feature-policy/#is-feature-enabled we see that there is no declared policy, so step 3 is skipped. Per https://fullscreen.spec.whatwg.org/#feature-policy-integration the default allowlist is not '*', so step 4 is skipped. In step 5, the default allowlist is 'self', origin is identical to document's origin, so we return "Enabled". Which is wrong.

Side-note on why "fullscreen" is enabled in parent: In the toplevel document, the inherited policy is "Enabled" for all features per https://wicg.github.io/feature-policy/#define-inherited-policy step 2. There are no declared policies, so per the same steps as above you end up with "Enabled" for "fullscreen" in the parent.

@clelland Presumably some of these algorithms are not working on the right origins, but it's not clear to me offhand where the problem is. The resulting behavior, however, looks clearly wrong. Am I just missing something?

My best guess is that the issue is https://wicg.github.io/feature-policy/#is-feature-enabled step 5, which is doing what looks like a no-op origin compare... But it's not clear what it should really be comparing to what.

[bzbarsky]

@bakulf

[clelland]

Thanks @bzbarsky -- I think this is an error in the algorithms as written.

presumably the desired behavior is that inside the subframe the "fullscreen" feature is not enabled.

That's right -- across an origin boundary, fullscreen should not be enabled unless explicitly through feature policy, in one way or another.

We first look at the "inherited policy" of the framed document.

The inherited policy should be false in this case. Unless allowed by the parent in one way or another, the inherited policy for a feature with a default allowlist of 'self' (or 'none') in a cross-origin iframe should always be false.

I think that step 2 of Define an inherited policy for feature in container is wrong, and origin should be the origin of the Document in the browsing context container (Is this the context's active document at this point?)

In that case, we have this flow instead for that algorithm:

1. parent is set to the toplevel document.
2. origin is set to "http://siteB.com".
3. container policy is an empty map.
4. skipped because the container policy is empty.
5. skipped because "fullscreen" is not enabled in parent for "http://siteB.com"
6. "Disabled" is returned.

Then, going back to Is feature enabled in document for origin?, the algorithm should return "Disabled" at step 2.

In step 5, the default allowlist is 'self', origin is identical to document's origin, so we return "Enabled". Which is wrong.

I think that even if we had gotten this far, origin at that point would be http://siteB.com, as that is what was (correctly) passed to that algorithm.

not linked?

Will fix, thanks :)

[bzbarsky]

I think this is an error in the algorithms as written

I do too. ;)

The inherited policy should be false in this case.

That would make sense.

I think that step 2 of Define an inherited policy for feature in container is wrong, and origin should be the origin of the Document in the browsing context container (Is this the context's active document at this point?)

That's a good question. Using the active document would work if every single API that relies on this stuff is disabled in non-active documents. Otherwise it's slightly odd to do it, I think.

It looks like there are two callers of https://wicg.github.io/feature-policy/#define-inherited-policy-in-container. One is https://wicg.github.io/feature-policy/#observable-policy and should presumably work with the active document of the browsing context hosted by that node? The other is https://wicg.github.io/feature-policy/#define-inherited-policy which itself has only one caller: https://wicg.github.io/feature-policy/#initialize-for-document.

Given that, I think it would be clearest to pass the document involved to both https://wicg.github.io/feature-policy/#define-inherited-policy and https://wicg.github.io/feature-policy/#define-inherited-policy-in-container with https://wicg.github.io/feature-policy/#initialize-for-document passing document and https://wicg.github.io/feature-policy/#observable-policy passing in the active document thing.

That said, given that presumably in all the interesting cases https://wicg.github.io/feature-policy/#initialize-for-document runs when the document is active (is that true?) we could probably just do what you suggest. It just seems more fragile in the long run.

I agree that fixing step 2 fixes the problem I am seeing.

[clelland]

Thanks, @bzbarsky -- I'll make the changes as you suggest. I'm not sure if every feature will always be disabled in documents which are not the active document. Some of the proposed features, such as document-write and unsized-media, are for features which would normally be enabled in every document, whether the active document of some browsing context or not.

(Although I don't have a good intuition about what such documents can actually do, so maybe that statement isn't necessarily true. If you can't run scripts, for instance, then it doesn't matter whether JS APIs are enabled or not)

[bzbarsky]

You can run functions in a no-longer-active document just by holding on to them and then calling them after navigating away from it. At least in browsers that are not Edge.

[clelland]

I've made a pull request (#229) that should fix this

The origin is explicitly passed into the Define an inherited policy for feature in container at origin algorithm, and it comes either from the actual document's origin, or from the declared origin of the iframe element, in the case where we're using this to calculate the observable policy (since the actual document in the frame can't be observable from the parent).