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

Having both incumbentNavigationOrigin and the source browsing context's active document's origin seems bad #6514

Closed
domenic opened this issue Mar 19, 2021 · 4 comments · Fixed by #6315
Labels
security/privacy There are security or privacy implications topic: navigation

Comments

@domenic
Copy link
Member

domenic commented Mar 19, 2021

Spinning off from #1130 and #2591, my concern is that even if we applied tricks similar to #6497 and #6512, we might end up in a bad place, because we have two notions of "initiator origin" for a navigation.

The spec uses:

When will these two concepts differ? Well, assuming we fix source browsing context's active document's origin to be computed synchronously instead of lazily, then the differences arise mainly when cross-global scripting is involved. For example, consider:

  • https://a.example.com/ which embeds https://b.example.com/ which embeds https://c.example.com/.
  • All three of them use document.domain so that they can synchronously script each other. (You knew it was coming!!)
  • In the outermost frame I navigate the innermost frame by doing frames[0].contentDocument.querySelector("iframe").src = "https://whatever.example/"

Then, the incumbentNavigationOrigin is the outermost https://a.example.com/ where the script is running, whereas the source browsing context's active document's origin is https://b.example.com/. (The same thing happens for the initial navigation if we changed this example to insert the innermost iframe, instead of modify it, BTW.)

You can construct similar terrible situations with window.open(), where the source browsing context is derived from the entry global, which differs from incumbent in fun ways. (On the other hand, location.href and other setters always sets the source browsing context to the incumbent, so it cannot diverge there.) Other scenarios I haven't dug into are inserting <meta http-equiv="refresh"> or performing a non-bfcache history.back() navigation.

Anyway, this difference seems bad. Security checks for javascript: URLs and COOP should probably use the same security checks as Sec-Fetch-Site.

So, which origin do we care about? I guess this is testable in browsers, so probably I should write some tests and report back...

@domenic domenic added topic: navigation security/privacy There are security or privacy implications labels Mar 19, 2021
@annevk
Copy link
Member

annevk commented Mar 21, 2021

Note that you might not need document.domain as you can assign to location across sites. (I would actually expect setting src to behave differently and use the element's node document as source of authority always.)

@domenic
Copy link
Member Author

domenic commented Apr 1, 2021

In what follows:

  • INO = incumbentNavigationOrigin
  • SBCADO - source browsing context's active document's origin

In web-platform-tests/wpt#28347 I tested three scenarios:

  1. javascript: URL navigation
  2. about:blank navigation
    • The current spec creates the new about:blank document with the response's origin = INO
    • Chrome and Firefox both implement this
  3. Sec-Fetch-Site
    • The current spec checks the request's origin = SBCADO against the destination URL
    • Chrome seems to do this incorrectly in a strange way: it mishandles the location.href case which should give same-origin because there SBCADO = INO = outer frame origin = destination URL origin. Instead it gives same-site, seemingly consulting the intermediate iframe's origin? I'll see if I can get people to comment...
    • (Nobody else implements fetch metadata headers at this point.)

I did not yet test COOP enforcement as I figured that's a new feature and might be flexible. Also, I'm waiting on Safari results for the first two.


I'm not sure what to conclude from all this.

  • It seems like browsers definitely use INO for about:blank so we can't get rid of it. (Despite generally wanting to remove usage of the incumbent concept: Minimize the usage of the incumbent concept #1430.)

  • Aligning javascript: on INO seems somewhat reasonable and might help Firefox with compat. Making it the same as about:blank makes some sense to me; both are generating "from scratch" documents in a similar fashion, and if we check that the iframe's current origin is equal to the INO then that means the generated document from the javascript: URL will also have the INO as its origin, which is somewhat symmetrical.

  • But, SBCADO is pretty tightly wound into the spec (because it's the request's origin), so does that mean we'll have to have both concepts forever?

@domenic
Copy link
Member Author

domenic commented Apr 1, 2021

I guess I should also add a test for CORS since that's based on request's origin = SBCADO currently.

This does not work; CORS does not apply to navigations. CORP should work though.

No, CORP is for "no-cors" only; it doesn't apply to "navigate" it appears

@domenic
Copy link
Member Author

domenic commented Apr 2, 2021

I added another test to the test PR:

  1. Origin for form submissions
    • The specs derive this from the request's origin, which is derived from the SBCADO
    • The current spec entirely fails to set a source browsing context for form submissions though (plan to navigate step 4).
    • Chrome and Firefox seem to use the origin of the <form> element, not the origin of the script that submits the form.

In the meantime, I managed to engage @anforowicz on this question. His intuition is that in general the INO is more correct; in particular he's pretty sure we have a bug in both the spec and implementation on Sec-Fetch-Site. I'm unsure what he thinks about the form submission case though.

It would be nice if we could move everything to INO per @anforowicz's intuition. The way to do this would be to change the request's client to be determined from the incumbent settings object, instead of the source browsing context's active document's relevant settings object. (And we'd probably then also align javascript: URLs and COOP enforcement in the same way.) However this would depart from at least Chrome and Firefox in the form submission Origin header case, so maybe it's not that simple.

@domenic domenic mentioned this issue Apr 23, 2021
3 tasks
domenic added a commit that referenced this issue Jun 23, 2021
This makes it non-racy by taking the origin snapshotted at the top of the navigation algorithm. Fixes #2591. Helps with #1130. See #6514 for related investigation.
domenic added a commit that referenced this issue Jun 25, 2021
This makes it non-racy by taking the origin snapshotted at the top of the navigation algorithm. It also switches to same origin-domain, which is more reasonable since if you have synchronous access to the document then you can just run JavaScript directly in it anyway. Fixes #2591. Helps with #1130. See #6514 for related investigation.
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
This makes it non-racy by taking the origin snapshotted at the top of the navigation algorithm. It also switches to same origin-domain, which is more reasonable since if you have synchronous access to the document then you can just run JavaScript directly in it anyway. Fixes whatwg#2591. Helps with whatwg#1130. See whatwg#6514 for related investigation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security/privacy There are security or privacy implications topic: navigation
Development

Successfully merging a pull request may close this issue.

2 participants