1631859 - webRequest's frameAncestors is incomplete when Fission enabled (subresource loads)

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P1)

Description

[Rob Wu [:robwu]]

When Fission is enabled, the frameAncestors property of webRequest events is not properly set any more.

STR:

1. Start Firefox with Fission enabled, i.e. fission.autostart=true and gfx.webrender.all=true
2. Load an extension that has the webRequest permission and host permissions.
3. Visit about:debugging, inspect the extension and run browser.webRequest.onBeforeRequest.addListener(console.log, {urls:['*://*/*']});
4. Load a page that has a frame from a different origin, which in turn loads a resource, e.g. https://jsfiddle.net/876o01eg/1/
   (this example has <img src="https://www.mozilla.org/favicon.ico?loadedfromtheframe">)
5. Look at the console for the devtools from step 2.

Expected:

• The request for the subresource (https://www.mozilla.org/favicon.ico?loadedfromtheframe) should have information on the parent frame: frameAncestors: [{frameId: 0, url: "https://jsfiddle.net/876o01eg/1/"}]

Actual:

• The request for the subresource has an empty list: frameAncestors: []

This works without Fission. We do currently not have test coverage for this scenario, so the least that we should do for now is to add a unit test that loads nested frames from different origins, and check that the frameAncestors event detail has the expected values.

Comment 1

[Chris Peterson [:cpeterson]]

We need to fix this because this API is used by ad blockers and NoScript.

Anny says she'll investigate. She fixed a related bug for document loads (bug 1594529). This bug is about subresource loads.

Comment 2

[Anny Gakhokidze [:annyG]]

While writing a test case for this (having a document load an OOP iframe that embeds an image in it) I also noticed that there a few other things that are broken:

1. parentFrameId is incorrect for images loaded in an OOP iframe, so I will be fixing it in this bug as well.
2. frameId for the OOP iframe and the image it embeds are different (it should be the same according to the description for onBeforeRequest listener). To fix this, we should not use outer Window ID's in LoadInfo objects at all and should replace those with corresponding browsing context IDs. Most usages of outer window IDs (mOuterWindowID, mFrameOuterWindowID, mParentOuterWindowID) within LoadInfo object can be removed easily, except for mTopOuterWindowID.

Devtool's network observer uses outer window ID's from NetworkObserver filters (which are window objects) and compares them to LoadInfo's mTopOuterWindowID. If we add mTopBrowsingContextID to LoadInfo object, then network-observer could be changed to use browsing context IDs for NetworkObserver filters and compare them with mTopOuterWindowID instead.

Comment 3

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1631859 - Part 1: Fill out ancestor principals and outer window IDs for LoadInfo only in the parent, r=kmag!

Keeping a list of ancestor principals in a LoadInfo object, that, at times,
exists in the content process, is not secure. Since ancestor principals are
only ever needed to create a list of frameAncestors, which, in turn, are only
ever accessed from the parent process, we can assemble lists of ancestor
principals and outer windowIDs whenever we are in the parent process and are
either 1) creating a LoadInfo object or 2) deserializing a LoadInfoArgs struct,
received from content process, into a LoadInfo object.

Comment 4

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1631859 - Part 2: Remove ancestor data from LoadInfoArgs, r=kmag!

Depends on D78406

Comment 5

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1631859 - Part 3: Remove ancestor principals and outer windowIDs from Document and nsDocShell, r=kmag!

Depends on D78407

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe4de87e7c60
Part 1: Fill out ancestor principals and outer window IDs for LoadInfo only in the parent, r=kmag,extension-reviewers
https://hg.mozilla.org/integration/autoland/rev/173abb1ddd8b
Part 2: Remove ancestor data from LoadInfoArgs, r=kmag
https://hg.mozilla.org/integration/autoland/rev/befaa5e5b3e6
Part 3: Remove ancestor principals and outer windowIDs from Document and nsDocShell, r=kmag

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/fe4de87e7c60
https://hg.mozilla.org/mozilla-central/rev/173abb1ddd8b
https://hg.mozilla.org/mozilla-central/rev/befaa5e5b3e6

Comment 8

[Anny Gakhokidze [:annyG]]

I did not fix parentFrameId for subresources in this bug as it is easier to fix it in bug 1642468.