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

Navigation sourceDocument for browser-initiated navigations #9133

Open
domenic opened this issue Apr 7, 2023 · 3 comments
Open

Navigation sourceDocument for browser-initiated navigations #9133

domenic opened this issue Apr 7, 2023 · 3 comments

Comments

@domenic
Copy link
Member

domenic commented Apr 7, 2023

Currently the navigate algorithm assumes it is always passed a sourceDocument. This is used for:

  • Checking if the source is allowed to navigate the target by sandboxing
  • Setting the initiator origin for the navigation. This is used:
    • When deciding what the origin should be if the URL is about:blank
    • For displaying to the user if the navigation ends up triggering external software (e.g. some-external-app://foo)
    • For preventing cross-origin-domain javascript: URL navigation and creating the resulting document
  • Creating the source snapshot params, including:
    • "has transient activation", which affects Sec-Fetch-User;
    • "sandboxing flags", used as part of the allowed-to-navigate check;
    • "allows downloading", which affects whether navigating to a Content-Disposition: attachment will succeed;
    • "fetch client", which affects a number of things in the Fetch algorithm, e.g. referrer calculation; and
    • "source policy container", which is used as the default policy container for the resulting document for local URLs

However, for browser UI-initiated navigations, we don't set any sourceDocument. So, that's bad; many algorithms are actually broken.

You might think we could treat browser UI navigations as if the source document is navigating itself. This works in some cases, e.g., it bypasses the allowed-by-sandboxing check. But it doesn't work for others; e.g., if the user navigates to a Content-Disposition: attachment URL, it should work, even if the page currently being displayed has sandboxing flags that disallow downloads.

So I think we need to allow sourceDocument to be null for browser UI navigations, and figure out replacements for all of the things derived from it. Ideas:

  • Skip the allowed-by-sandboxing check
  • Initiator origin: ???
  • Source snapshot params

OK, so the hard ones are initiator origin and fetch client. Ideas:

  • initiator origin: I think we want a new opaque origin for top-level about:blanks or javascript:'a string'. We don't want to tell the user "an opaque origin is attempting to navigate you to some-external-app://foo" if the user themselves typed the URL in the URL bar, so we might need a bit of special casing here.

  • fetch client: maybe, leave the client as null and try to fill in the fetch fields directly? Per this section of Fetch, those would be:

    • origin: a new opaque origin?? Will this break Fetch and cause various security checks to fail?? Maybe we should pick the target URL's origin instead?
    • policy container: an empty policy container
    • service-workers mode: "all"
    • referrer: "no-referrer"

I'm willing to put up a PR for this, but I'd love some confirmation that I'm on the right track here before I do. Any thoughts from @annevk @smaug---- @domfarolino @jakearchibald ? Especially @annevk for the Fetch integration questions

@domenic
Copy link
Member Author

domenic commented Apr 7, 2023

  • initiator origin: I think we want a new opaque origin for top-level about:blanks or javascript:'a string'.

Here is a fun fact! If you navigate to javascript:'a string' (via a bookmark), the resulting page has self.origin equal to the origin of the page you just left! Whereas, self.origin === "null" for about:blank. So, I guess we need to special-case the javascript: case to actually pull the initiator origin from the currently-active document!

@domfarolino
Copy link
Member

Yeah I like the idea of making the initiator origin opaque in this case in general, but I'm curious what other browsers are doing here, and if it's observable in any other ways we're missing. But...

We don't want to tell the user "an opaque origin is attempting to navigate you to some-external-app://foo" if the user themselves typed the URL in the URL bar, so we might need a bit of special casing here.

...I can glean a little bit about what browsers are doing by looking at the external protocol case specifically. For example, when I navigate with browser UI to facetime:<number> in Chromium, I get "A website wants to open this application", whereas if I click a facetime: link on a website, I get a prompt that's specifically tailored to the initiator origin. At least Chromium is checking if the initiator origin is opaque and throwing up a more generic prompt in that case, which I think is exactly in the purview of a UA, so I personally think we won't need to special-case anything in the spec here. UAs already have to deal with opaque origins in this case if external protocols are accessed from other opaque origin contexts.

I'm not seeing any other interesting observable effects between using opaque origins vs origins derived in HTML, but I'd definitely like to know more about the Fetch implications. At least I don't think we have to worry about any "tainting", since it seems like the mode="navigate" case is handled specially for that, and I don't think we ever send the Origin header on normal GETs, so it seems fine to use an opaque origin there. Is it possible to trigger a POST (or any other request where we'd have to send the Origin header) with browser UI, as if there were no initiator document? My guess is no... from messing around on https://iframe-session-history.glitch.me/ (with POST requests specifically) I can't get the POST resubmission prompt to throw by clicking in the URL bar and hitting enter.


Fetch client being null and manually filling in the other bits sounds good to me. At least it won't mess with the fetch task queueing stuff since we're already using useParallelQueue = true, but I too would like to hear from @annevk.


So, I guess we need to special-case the javascript: case to actually pull the initiator origin from the currently-active document!

Great find. It's a little surprising that we literally just run the JS in the currently active document of the target navigable, but I guess procedurally that does make sense, and I suppose is fine from a security perspective since the user is making all of these actions.

@annevk
Copy link
Member

annevk commented Apr 28, 2023

I think I mentioned this at some point and you assured me there was always a document. 😊

Manually filling in bits makes sense to me, but Fetch might also need some changes as it doesn't really do well when there is no client. I'm also not sure about the origin field.

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

No branches or pull requests

3 participants