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

Same origin S&P requirement conflicts with Permissions Policy integration #133

Closed
rakuco opened this issue Jan 30, 2024 · 3 comments · Fixed by #136
Closed

Same origin S&P requirement conflicts with Permissions Policy integration #133

rakuco opened this issue Jan 30, 2024 · 3 comments · Fixed by #136
Labels

Comments

@rakuco
Copy link
Member

rakuco commented Jan 30, 2024

With the official integration of the Permissions Policy bits in #121, we now have contradicting requirements in the spec:

  • The presence of the Permissions Policy integration means usage of the Device Orientation API can be allowed in third-party iframes provided that the right tokens are in place (also subject to the requestPermission()/Permissions API requirements added in Add Permissions API integration, start requiring requestPermission() usage #123).
  • The "Security and privacy considerations" section contains the following excerpt: "fire events only on the top-level traversable's active window and child navigables' active windows whose relevant settings object's origin is same origin with the top-level traversable's active window's relevant settings object's origin", which is a complicated but more spec-compliant wording of the previous version, "fire events only on the top-level browsing context and same-origin nested browsing contexts".

The same-origin requirement was added back in 2016 with #25, and it was implemented by Gecko in https://bugzilla.mozilla.org/show_bug.cgi?id=1197901 (change mirrored in mozilla/gecko-dev@bdb1f6d).

As far as I can see, this requirement was never implemented in Blink, but it did add Permissions Policy integration in early 2018 (https://bugs.chromium.org/p/chromium/issues/detail?id=796894). When WebKit implemented this API a few years later, they did add Permissions Policy integration as well, so we have 2 engines implementing the Permissions Policy integration and 1 implementing the same-origin requirement.

My proposal is to remove that item from the S&P section and add a new item referring to the Permissions API and Permissions Policy integration requirements. The current Permissions Policy requirements (with features whose default allowlist is "self") acts as a superset of the original requirement -- AFAICS, an implementation that switches from one requirement to the other will continue allowing the exact same set of sites (and this is without taking the Permissions API integration into consideration).

@rakuco rakuco added the bug label Jan 30, 2024
@rakuco
Copy link
Member Author

rakuco commented Jan 30, 2024

cc @anssiko @reillyeon, I've noticed this after finally taking a look at #126.

@reillyeon
Copy link
Member

I see the "top-level and same-origin children" requirement as having been a stop-gap until the Permissions Policy controls were in place and the capability could be delegated to cross-origin children only when the top-level document opted-in.

I agree with your proposal.

rakuco added a commit that referenced this issue Jan 31, 2024
…olicy one

This addresses a conflict that was introduced in #121:

- The presence of the Permissions Policy integration means usage of the
  Device Orientation API can be allowed in third-party iframes provided that
  the right tokens are in place.
- The "Security and privacy considerations" section contains a requirement
  that events are fired only on child navigables that are same-origin with
  the top-level traversable.

The latter was introduced in #25 and served as a stop-gap measure before
Permissions Policy integration was added.

The current implementation status is:
- Blink never implemented the same-origin requirement, but added Permissions
  Policy integration in 2018.
- WebKit has always implemented Permissions Policy integration.
- Gecko implements the same-origin requirement (see Mozilla bug 1197901).

This means we can safely replace the same-origin requirement with a
requirement to support the Permissions Policy integration, as switching from
one to the other is transparent in the sense that the exact same set of
websites that worked before will continue to work with the change, as the
features we define have a default allowlist of "self".

Fixes #133
rakuco added a commit that referenced this issue Jan 31, 2024
…olicy one (#136)

This addresses a conflict that was introduced in #121:

- The presence of the Permissions Policy integration means usage of the
  Device Orientation API can be allowed in third-party iframes provided that
  the right tokens are in place.
- The "Security and privacy considerations" section contains a requirement
  that events are fired only on child navigables that are same-origin with
  the top-level traversable.

The latter was introduced in #25 and served as a stop-gap measure before
Permissions Policy integration was added.

The current implementation status is:
- Blink never implemented the same-origin requirement, but added Permissions
  Policy integration in 2018.
- WebKit has always implemented Permissions Policy integration.
- Gecko implements the same-origin requirement (see Mozilla bug 1197901).

This means we can safely replace the same-origin requirement with a
requirement to support the Permissions Policy integration, as switching from
one to the other is transparent in the sense that the exact same set of
websites that worked before will continue to work with the change, as the
features we define have a default allowlist of "self".

Fixes #133
@rakuco
Copy link
Member Author

rakuco commented Feb 5, 2024

I've just filed https://bugzilla.mozilla.org/show_bug.cgi?id=1878686 to let the Gecko developers know about these changes.

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

Successfully merging a pull request may close this issue.

2 participants