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

I2I: support "disallowDocumentAccess" for same-origin-iframes #30824

Closed
dvoytenko opened this issue Oct 22, 2020 · 3 comments · Fixed by #31100
Closed

I2I: support "disallowDocumentAccess" for same-origin-iframes #30824

dvoytenko opened this issue Oct 22, 2020 · 3 comments · Fixed by #31100
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code

Comments

@dvoytenko
Copy link
Contributor

Summary

Support the new disallowDocumentAccess API and allow same-origin iframes in AMP.

The main impact of this proposal is the removal of AMP's Iframe origin policy.

Design document

The disallowDocumentAccess API prevents cross-context DOM access even for same-origin iframes.

There are two reasons why we currently disallow same-origin iframes:

  1. We cannot guarantee that they would be portable. I.e. we can't be certain that these iframes will run correctly both on origin and from Cache.
  2. We cannot assure AMP performance and layout guarantees. Even if the AMP page is fully valid statically, a same origin iframe can manipulate its content on Cache and break the gurantees.

The amp-iframe will automatically set disallowDocumentAccess and allow sandbox="allow-same-origin" iframes.

The main issue with this proposal is that disallowDocumentAccess API is currently only available in Chromium-based browsers. This means that AMP pages could possibly violate AMP guarantees on non-supporting browsers. This proposal relies on several nuances:

  1. We don't have issues on Cache since allow-same-origin only affects pages at origin.
  2. Chrome is a popular browser. A developer would be discouraged from deploying features that do not work on Chrome and/or do not work on Cache.
  3. Chrome DevTools is a popular tool. Many developers develop and debug with Chrome DevTools and thus any violation of the disallowDocumentAccess would be noticed sooner.

/cc @ampproject/wg-approvers
/cc @cramforce @dtapuska

@dvoytenko dvoytenko added the INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code label Oct 22, 2020
@cramforce
Copy link
Member

It was always possible to work-around via redirect, so this is not a security issue

@dvoytenko
Copy link
Contributor Author

Right. This is not a security feature, but rather a consistency and avoid-the-bugs issue mainly.

@dvoytenko
Copy link
Contributor Author

Unfortunately the spec change has not been approved in Chrome. There will be other attempts to solve same-origin iframe isolation, but it will be done in a different way with a different API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants