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

Add support for cross-origin navigation redirects #23037

Open
jdm opened this issue Mar 14, 2019 · 5 comments
Open

Add support for cross-origin navigation redirects #23037

jdm opened this issue Mar 14, 2019 · 5 comments
Labels
A-constellation Involves the constellation A-content/script Related to the script thread A-security

Comments

@jdm
Copy link
Member

jdm commented Mar 14, 2019

https://www.joshmatthews.net/mlredir2.html has a 301 server redirect to https://threejs.org/examples/webgl_custom_attributes_points3.html. This yields a black screen and SecurityError: The operation is insecure., while loading the threejs example directly yields a functioning webgl page. I suspect the underlying problem is that we create a script thread for the initial origin, then any redirected content is still loaded in that script thread and incorrect URLs are stored in various places that are later checked.

@jdm jdm added A-content/script Related to the script thread A-constellation Involves the constellation labels Mar 14, 2019
@jdm
Copy link
Member Author

jdm commented Mar 14, 2019

This happens even when clicking a cross-origin link to load the example. That's... very bad!

@gterzian
Copy link
Member

gterzian commented Jul 17, 2019

I think the SecurityError: The operation is insecure. is a good thing actually, because what I think is happening is completely insecure(and it could be a way to break process isolation).

Just from reading the code, which should be taken with a grain of salt, here what I think happens:

  1. Starting with a navigation to https://www.joshmatthews.net/mlredir2.html.
  2. Constellation creates a new pipeline, and either starts a new script-thread, or re-uses one, based on the the joshmatthews.net domain. (here)
  3. The new pipeline either sends the AttachLayout to an existing script-thread, or creates a new script-thread(via ScriptThreadFactory).
  4. In both cases, this calls into ScriptThread.pre_page_load.
  5. There, the script-thread stores an IncompleteLoad using joshmatthews.net as origin.
  6. It then sends a ScriptMsg::InitiateNavigateRequest to the constellation.
  7. It get's interesting in how the NetworkListener handles the redirect here, it looks it simply re-initates a fetch for the redirect.
  8. Eventually, the metadata for the redirect comes in, and is forwarded by the listener to the script-thread, which handles it at handle_fetch_metadata
  9. This calls into process_response of the ParserContext.
  10. This calls into ScriptThread::page_headers_available
  11. At this point, the script-thread calls self.load using the metadat of the re-direct, and the IncompleteLoad stored at 5 above.
  12. load creates a document using the origin of the IncompletLoad, which is going to be joshmatthews.net, but the url of the document is using the metadata of the re-direct, which is something like https://threejs.org/examples/webgl_custom_attributes_points3.html
  13. Note that the origin of the window on the other hand is set to threejs.org/ here.

So I think we're lucky the document raises SecurityError: The operation is insecure. upon some attribute access(I guess), because otherwise we would not have noticed.

The problem here is really that we're running a page from threejs.org/ in the a script-thread which is stored by the constellation under the joshmatthews.net host.

So if joshmatthews.net would have a link to https://www.joshmatthews.net/mlredir2.html using target=_blank, you'de get a cross-site auxiliary running threejs.org executing in the same process at joshmatthews.net, defeating process isolation.

And even if you just load the redirect in an existing script-thread where joshmatthews.net was previously running, you would also have broken process isolation since the redirect could still perform a Spectre attack on the memory previously used in the same process by a different site.

I even think threejs.org could then load an iframe running joshmatthews.net (probably just looking at it's referrer header or some other header telling you about where the re-direct originated from), and get it to load in the same process, since the constellation thinks the script-thread running threejs.org is running joshmatthews.net.


I think the fix for this could be something like:

  1. When the NetworkListener hits a redirect, from site A to site B, it should send a message to script-thread A to remove it's IncompleteLoad, and create a new pipeline/script-thread to handle the redirect via new_pipeline

@mrobinson
Copy link
Member

Just bumped into while moving the NetworkListener into the ScriptThread entirely. I think that's fine, but the ScriptThread will have to respond properly to a cross origin redirect by sending new messages to the Constellation.

@mrobinson mrobinson changed the title Cross-origin redirection during initial page load messes up origin checks Add support for cross-origin redirects Jan 8, 2025
@mrobinson
Copy link
Member

I've repurposed this bug to track the addition of the cross-origin redirects feature -- a pretty big missing piece of the web puzzle!

@mrobinson mrobinson changed the title Add support for cross-origin redirects Add support for cross-origin navigation redirects Jan 8, 2025
mrobinson added a commit to mrobinson/servo that referenced this issue Jan 9, 2025
This allows reusing the asynchrnous fetch mechanism that we use for page
resources and is likely a step toward removing the `FetchThread`.

Benefits:
 - Reduces IPC traffic during navigation. Now instead of bouncing
   between the constellation and the `ScriptThread` responses are sent
   directly to the `ScriptThread`.
 - Allows cancelling loads after redirects, which was not possible
   before.

There is the question of what to do when a redirect is cross-origin
(servo#23037). This currently isn't handled properly as the `Constellation`
sends data to the same `Pipeline` that initiated the load. This change
doesn't fix this issue, but does make it more possible for the
`ScriptThread` to shut down the pipeline and ask the `Constellation` to
replace it with a new one.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
@gterzian
Copy link
Member

gterzian commented Jan 10, 2025

As per #34918 (comment) I think we may be able to get some inspiration with how message ports was done, because that also involved IPC and with a kind of cross-origin "redirect" because ports are transferable inside message on ports, including across origins. Which is why the below was added, which incidentally also avoided creating a new IPC route for each port.

/// State representing whether this global is currently managing messageports.

mrobinson added a commit to mrobinson/servo that referenced this issue Jan 10, 2025
This allows reusing the asynchrnous fetch mechanism that we use for page
resources and is likely a step toward removing the `FetchThread`.

Benefits:
 - Reduces IPC traffic during navigation. Now instead of bouncing
   between the constellation and the `ScriptThread` responses are sent
   directly to the `ScriptThread`.
 - Allows cancelling loads after redirects, which was not possible
   before.

There is the question of what to do when a redirect is cross-origin
(servo#23037). This currently isn't handled properly as the `Constellation`
sends data to the same `Pipeline` that initiated the load. This change
doesn't fix this issue, but does make it more possible for the
`ScriptThread` to shut down the pipeline and ask the `Constellation` to
replace it with a new one.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
github-merge-queue bot pushed a commit that referenced this issue Jan 10, 2025
This allows reusing the asynchrnous fetch mechanism that we use for page
resources and is likely a step toward removing the `FetchThread`.

Benefits:
 - Reduces IPC traffic during navigation. Now instead of bouncing
   between the constellation and the `ScriptThread` responses are sent
   directly to the `ScriptThread`.
 - Allows cancelling loads after redirects, which was not possible
   before.

There is the question of what to do when a redirect is cross-origin
(#23037). This currently isn't handled properly as the `Constellation`
sends data to the same `Pipeline` that initiated the load. This change
doesn't fix this issue, but does make it more possible for the
`ScriptThread` to shut down the pipeline and ask the `Constellation` to
replace it with a new one.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-constellation Involves the constellation A-content/script Related to the script thread A-security
Projects
None yet
Development

No branches or pull requests

3 participants