Closed Bug 1578405 Opened 5 years ago Closed 3 years ago

Deprecate Cross-Origin requests from content scripts in Manifest v3

Categories

(WebExtensions :: General, task, P2)

Product:
WebExtensions
Component:
General
Type:
task
Points:
3

Tracking

(Fission Milestone:Future, firefox101 fixed)

Status:
RESOLVED FIXED
Milestone:
101 Branch
Project Flags:
Fission Milestone Future
Tracking Flags:
Tracking Status
firefox101 --- fixed

People

(Reporter: zombie, Assigned: robwu)

References

(Blocks 4 open bugs,
URL
)

Details

(Keywords: dev-doc-complete, Whiteboard: mv3:m2 [mv3-m2])

Attachments

(1 file)

Bug 1578405 - Use page's fetch/XHR/WebSocket in MV3 content scripts
48 bytes, text/x-phabricator-request
Details | Review

Currently, content scripts can issue fetch and XHR requests to any domain the extension has host permissions for. This change is coming with manifest v3, and is also related to Fission (content processes should generally be able to issue network requests only to the same domain).

To help developers prepare for the change, we probably want to implement this behind a pref, that would make it easy to test ahead.

Chrome is doing something similar, deprecating for all but select few whitelisted extensions ahead of v3 rollout.

Google's doc with details:
https://www.chromium.org/Home/chromium-security/extension-content-script-fetches

URL: https://www.chromium.org/Home/chromiu...
Priority: -- → P2
Depends on: 1581608
No longer depends on: 1581608

Firefox supports Contextual Identities. But I cannot find an easy way to configure an background fetch / xhr using certain cookieStoreId. (Maybe I'm missing something?) As the result, moving xhr from content script to background would be much harder than other browsers, imo. If this proposal is intend to be implemented, I would like to see some mechanism to make the using of Contextual Identities in background fetch / xhr easier.

Hey Andrea, do you perhaps have some thoughts about how we might be able to solve the use-case from comment 2?

Perhaps something like a cookieStoreId property added to fetch requests, only available to chrome/extensions?

Flags: needinfo?(amarchesini)

(In reply to Tian, Sheng from comment #2)

Firefox supports Contextual Identities. But I cannot find an easy way to configure an background fetch / xhr using certain cookieStoreId. (Maybe I'm missing something?)

I don't know if this solution would fit here, but you can create a sandbox, and force a particular origin-attribute.
So far this has been done in C++, but I'm quite sure we have everything we need to do it in JS, if needed.
An example is here: https://searchfox.org/mozilla-central/rev/6866d3a650c826f1cabd123663e84b95ee743701/dom/reporting/ReportDeliver.cpp#87-249

Flags: needinfo?(amarchesini)

Wouldn't we just need to change what we're doing in the content script sandbox? I figured we'd just need remove the xhr/fetch logic here to make it use the xhr/fetch from the content page itself.

https://searchfox.org/mozilla-central/rev/17756e2a5c180d980a4b08d99f8cc0c97290ae8d/toolkit/components/extensions/ExtensionContent.jsm#844-853

Oh, the origin-attribute item seems like a different bug than this one.

Will content scripts still be able to load data from the extension via fetch/xhr? For example I'm using this to load some necessary json files.

That shouldn't be affected, since those are not "cross origin" from the extension perspective.

This raises a (in my opinion) rather big privacy concern regarding extensions, as you can see with bug 1380812 the split permission is not supported by firefox at this time. This means that extension background pages can not properly differentiate the source from requests through the api or properly route requests through the correct session.

@creesch.r The split feature does not offer enough isolation to resolve privacy issues. When users use container tabs, requests shouldn't be shared with the main browsing context either.

This does show that if we were to remove cross-origin requests from content scripts (or even if we don't), that we should consider introducing an API to help extension developers to create requests with the correct settings (cookieStoreId, private). I don't think that we can expect extensions to be aware of all potential ways to isolate requests.

(In reply to Rob Wu [:robwu] from comment #11)

@creesch.r The split feature does not offer enough isolation to resolve privacy issues. When users use container tabs, requests shouldn't be shared with the main browsing context either.

Why wouldn't it? The way chrome handles it each "container" gets their own dedicated background page as they should so there is no mixing it.

As far as sharing requests with the main browser context goes, I am not sure what you mean by that.

This does show that if we were to remove cross-origin requests from content scripts (or even if we don't), that we should consider introducing an API to help extension developers to create requests with the correct settings (cookieStoreId, private). I don't think that we can expect extensions to be aware of all potential ways to isolate requests.

I am having trouble understanding where introducing a new API (which will not be supported by chrome or edge, at least initially if we are optimistic) is the proper way to handle this. It seems to me that if firefox would implement the split method this already is the way the webextension framework should handle this.

(In reply to creesch.r from comment #12)

(In reply to Rob Wu [:robwu] from comment #11)

@creesch.r The split feature does not offer enough isolation to resolve privacy issues. When users use container tabs, requests shouldn't be shared with the main browsing context either.

Why wouldn't it? The way chrome handles it each "container" gets their own dedicated background page as they should so there is no mixing it.

As far as sharing requests with the main browser context goes, I am not sure what you mean by that.

You're thinking of incognito / private browsing. If that is the only kind of isolation, then yes, incognito: "split" could work. However, I am talking about container tabs / contextual identities. This is a feature unique to Firefox that offers more control over user privacy. See https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Work_with_contextual_identities

There are even more isolations, depending on user preferences: (dynamic) first-party isolation is another one (bug 1625228).

To preserve the isolation, extensions should not mix requests from container tabs with requests from other contexts, such as the background page. "split": is not a good base to build upon for further isolation, because its design scales poorly (the memory usage would be excessive if every container would have their own add-on instances).

This does show that if we were to remove cross-origin requests from content scripts (or even if we don't), that we should consider introducing an API to help extension developers to create requests with the correct settings (cookieStoreId, private). I don't think that we can expect extensions to be aware of all potential ways to isolate requests.

I am having trouble understanding where introducing a new API (which will not be supported by chrome or edge, at least initially if we are optimistic) is the proper way to handle this. It seems to me that if firefox would implement the split method this already is the way the webextension framework should handle this.

This comment exactly proves my point. You didn't know what I referred to at first, so if incognito:"split" were to be supported, you would rely on it and mistakenly believe that your extension is privacy-friendly when in fact it is not (due to information leakage across supposedly isolated contexts). A dedicated API for this purpose would allow extensions to respect privacy without requiring extension developers to closely follow advances in isolation/APIs. For example, beyond what we have today, I could imagine the introduction of a new kind of isolation to support add-ons in Custom Tabs of Firefox for Android. Without a dedicated API, extensions would not have a reasonable way of making requests that are guaranteed to respect the user expectations on privacy/isolation.

(In reply to Rob Wu [:robwu] from comment #13)

(In reply to creesch.r from comment #12)

(In reply to Rob Wu [:robwu] from comment #11)

@creesch.r The split feature does not offer enough isolation to resolve privacy issues. When users use container tabs, requests shouldn't be shared with the main browsing context either.

Why wouldn't it? The way chrome handles it each "container" gets their own dedicated background page as they should so there is no mixing it.

As far as sharing requests with the main browser context goes, I am not sure what you mean by that.

You're thinking of incognito / private browsing. If that is the only kind of isolation, then yes, incognito: "split" could work. However, I am talking about container tabs / contextual identities. This is a feature unique to Firefox that offers more control over user privacy. See https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Work_with_contextual_identities

Frankly, container tabs could be handled with split as well as far as I am concerned.

This comment exactly proves my point. You didn't know what I referred to at first, so if incognito:"split" were to be supported, you would rely on it and mistakenly believe that your extension is privacy-friendly

No.... I rather have it that split is implemented for those case where tabs are truly containers (as is the case with tab containers and incognito mode) so I don't have to bother with an entire separate API as at least for those tabs I know all data is contained within that tab and the corresponding background page.

Currently requests can be done freely from content_page scripts, meaning that whatever context we are in is automatically respected. When the above change is implemented to follow what chrome does this is no longer possible for requests across subdomains. At that time extensions will need to move requests to the background page like recommended by the chrome devs, when doing that the option I have as extension developer for the things we discussed is to either go with the default which means that in the background page content from different containers is freely mixed (resulting in very odd behavior) or I specifically don't allow my extension to load in a private tab to somewhat protect my users and prevent confusion as they are seeing data from sessions in their non private tabs.

First party isolation is an entirely different principle and doesn't really seem relevant to my concerns.

At the same time I do appreciate you actually discussing this with me and not dismissing the entire thing entirely. bug 1380812 has been open for three years and I raised my concerns about a year ago there. This is the first time someone is actually discussing about possible solutions, I still rather have a solution that also works with split as that makes cross browser extension development a lot easier. But if a separate API is need I'd work with that as well. As long as there is a solution for this as currently there is none and so far it didn't seem anyone was interested in creating or even discussing a proper solution anyway.

(In reply to creesch.r from comment #14)

This comment exactly proves my point. You didn't know what I referred to at first, so if incognito:"split" were to be supported, you would rely on it and mistakenly believe that your extension is privacy-friendly

No.... I rather have it that split is implemented for those case where tabs are truly containers (as is the case with tab containers and incognito mode) so I don't have to bother with an entire separate API as at least for those tabs I know all data is contained within that tab and the corresponding background page.

As said, it doesn't scale well. With incognito/private browsing mode, there would be at most two instances of the same extension. With more kinds of isolation, there may be many more.

extensions will need to move requests to the background page like recommended by the chrome devs, when doing that the option I have as extension developer for the things we discussed is to either go with the default which means that in the background page content from different containers is freely mixed (resulting in very odd behavior) or I specifically don't allow my extension to load in a private tab to somewhat protect my users and prevent confusion as they are seeing data from sessions in their non private tabs.

The described options are not the only ones. The issue that you've shown in comment 10 is very valid, but the proposed remediation is suboptimal. incognito:"split" offers good isolation of DOM APIs such as fetch and localStorage, but does little against information leakage through extension APIs (e.g. storage / messaging) (this is what was being referred to in comment 5 of bug 1380812). So while there is some isolation, it is not complete and also has other drawbacks (memory usage, latency due to extension startup).

Regardless of an extension's support, users already have the ability to toggle extensions in private browsing mode (both in Firefox and Chrome). We are also exploring ways to offer even more granular user control, follow bug 1365019 and bug 1497075 if you're interested.

First party isolation is an entirely different principle and doesn't really seem relevant to my concerns.

For this discussion, (dynamic) first-party isolation (FPI) is equally relevant. With (d)FPI, requests from two different tabs can be isolated. Extensions that ignore this characteristic can potentially weaken the privacy of users, for example by (inadvertently) mixing network requests on behalf of those tabs. The idea of split could bluntly be applied here, but then every unique top-level site will have their own extension instance.

I still rather have a solution that also works with split as that makes cross browser extension development a lot easier. But if a separate API is need I'd work with that as well. As long as there is a solution for this as currently there is none and so far it didn't seem anyone was interested in creating or even discussing a proper solution anyway.

I'm not outright rejecting compatibility with split mode-code, but if the isolation/privacy problem is not addressed by chosing that path, then there would not be much point in implementing it. This problem should be resolved before turning cross-origin requests are turned off. I guess that the latter won't happen this year, so we are not in a rush to make hasty decisions. I do expect to file a new bug to track the identified issue, after discussing this with the rest of my team.

Whiteboard: webext?

(In reply to Rob Wu [:robwu] from comment #15)

As said, it doesn't scale well. With incognito/private browsing mode, there would be at most two instances of the same extension. With more kinds of isolation, there may be many more.

That is a fair point, though I am not sure how bad it would be. More precise, at what point does it become a real concern regarding performance?

The described options are not the only ones. The issue that you've shown in comment 10 is very valid, but the proposed remediation is suboptimal. incognito:"split" offers good isolation of DOM APIs such as fetch and localStorage, but does little against information leakage through extension APIs (e.g. storage / messaging) (this is what was being referred to in comment 5 of bug 1380812). So while there is some isolation, it is not complete and also has other drawbacks (memory usage, latency due to extension startup).

At this point fetch/XHR isolation is the only thing I am personally concerned about as without split and an extension enabled in private browsing they aren't isolated at all as sessions carry over where there isn't just a local risk but also a risk of data leaking into online services.

In fact, I'd argue that extension storage should be a toggle as many users would expect their extension settings to carry over to private browsing though I do realize that is just as much a real concern of incognito data leaking out.

Regardless of an extension's support, users already have the ability to toggle extensions in private browsing mode (both in Firefox and Chrome). We are also exploring ways to offer even more granular user control, follow bug 1365019 and bug 1497075 if you're interested.

Which at this point means no isolation at all.

For this discussion, (dynamic) first-party isolation (FPI) is equally relevant. With (d)FPI, requests from two different tabs can be isolated. Extensions that ignore this characteristic can potentially weaken the privacy of users, for example by (inadvertently) mixing network requests on behalf of those tabs. The idea of split could bluntly be applied here, but then every unique top-level site will have their own extension instance.

I see your point but frankly I still think they are different exactly because of how they are applied and as such there might not be a good solution that works for both the earlier discussed things and first-party isolation.

I'm not outright rejecting compatibility with split mode-code, but if the isolation/privacy problem is not addressed by chosing that path, then there would not be much point in implementing it. This problem should be resolved before turning cross-origin requests are turned off. I guess that the latter won't happen this year, so we are not in a rush to make hasty decisions. I do expect to file a new bug to track the identified issue, after discussing this with the rest of my team.

The point would be at the very least compatibility with chrome and edge which I realize isn't maybe something that mozilla internally cares about but is a concern for extension devs. It is also why I am not entirely a fan of yet another API which will not be very likely to implement in chromium based webextension frameworks.

Depends on: 1670278

Marking as dependency on bug 1605197, because the ability to perform regular CORS requests is necessary to help extension authors with transitioning off the extension's host permissions.

Depends on: 1605197

Tomislav says this bug doesn't need to block Fission MVP.

Fission Milestone: --- → Future

(In reply to Chris Peterson [:cpeterson] from comment #18)

Tomislav says this bug doesn't need to block Fission MVP.

With all due respect, I very much suspect that this is also the reason why in webextension development we keep encountering issues with firefox in these sorts of areas as after a certain MVP is reached priorities shift en the details don't get filled in properly or considered specifically regarding how things will impact webextensions. In fact the entire situation here and bug 1380812 is as far as I can tell pretty much the result of such a situation.

Now if it really doesn't need to be blocking for Fission then that is absolutely fine but as an extension developer I fear that it eventually will end up causing more issues for extension in this area if it is simply brushed aside for fission.

Anyway, I just wanted to voice my concern here and make sure that the functionality of webextensions isn't (even more) adversely affected here.

Thanks for your feedback creeschr.

Removal of cross-origin requests is a breaking change. To not unnecessarily break extensions in a backwards-incompatible way, we're making this change as part of manifest v3 instead of modifying the behavior for existing manifest v2-extensions. This gives extension authors about 1-2 years to migrate, and users will not unexpectedly encounter broken extensions.

That timeline for removal and deprecation is similar to Chromium's (at https://www.chromium.org/Home/chromium-security/extension-content-script-fetches ).

We also recognize that Chromium's suggestion of just moving requests to the background page is not going to cover all use cases (in particular, bug 1380812 aka "split incognito" mode that you're referring to is not sufficient). For that, we are also tracking potential improvements to the extension API for making requests "on behalf of tabs/frames" in bug 1670278 (see its related bugs).

I find that this change will be annoying, but regardless of that view...

Will this make the content.XHR and content.fetch APIs irrelevant? Within content scripts the window variants used the extension context and did not set Referrer/Origin while the content variants used the page context. Is this expected to make all requests use the page context and associated Referrer/Origin?

(In reply to sdaniele3 from comment #21)

I find that this change will be annoying, but regardless of that view...

Will this make the content.XHR and content.fetch APIs irrelevant? Within content scripts the window variants used the extension context and did not set Referrer/Origin while the content variants used the page context. Is this expected to make all requests use the page context and associated Referrer/Origin?

We haven't decided that yet. In the implementation, there is a difference between requests initiated from the content script context and the page context. Once we get to the implementation, it may be more obvious whether it makes sense to drop the content variants.

Note that if your concern is the ability to clear the referrer: note that the fetch API accepts the referrerPolicy option to specify whether/how the Referer header is set.

Whiteboard: webext? → mv3:m1

Shane and I have been discussing about this bug today, looking into what details to add in a comment here to make it more clearly actionable
and we agreed on the following:

  • In this bug we will aim for:

    • introducing the restrictions that are expected to be applied to content scripts in mv3 (moving content.XHR/fetch to the XHR/fetch that used to allow cross-origin requests, and remove content)
    • along with a pref to re-enabled the current behavior as available in mv2 (mainly to let the developers to test issues triggered by the other restrictions and differences in mv3 content scripts, e.g. the extension CSP that is enforced on the content scripts in mv3, without having to make all the necessary changes at once)

  • Bug 1605197 and Bug 1670278 to be moved from blockers to blocked issues (which also match the priorities set of these bugs, this one is a P2 and the other two are both P3s)

  • Bug 1605197 and Bug 1670278 would still be blocking mv3 and they should be part of an mv3 milestone (but Bug 1670278 whiteboard is already set to mv3:m1 as this one), because those bugs needs to be fixed to fill the gaps that mv3 content script would have (described at length in the previous comments in this issues)

Blocks: 1605197, 1670278
No longer depends on: 1605197, 1670278
Whiteboard: mv3:m1 → mv3:m2
Whiteboard: mv3:m2 → mv3:m2 [mv3-m2]
Points: --- → 3
Assignee: nobody → rob
Summary: Deprecate Cross-Origin requests from content scripts ahead of Manifest v3 → Deprecate Cross-Origin requests from content scripts in Manifest v3
See Also: → 1766813

This patch ensures that fetch/XHR from MV3 content scripts have the
same capabilities as the web page, by relying on the fetch/XHR methods
inherit from the window prototype that use the page's principal, instead
of overwriting them with methods that use the expanded principal (MV2).

For completeness, although the WebSocket constructor does not require
origin permissions, I have also removed the similar special hack for
WebSocket, so that its behavior is also consistent with the page.

It is worth noting that currently, the page's CSP affects these methods,
instead of the content script's CSP (bug 1766813).

Besides the fix, this patch has test changes:

  • test_ext_contentscript_csp.js was modified, to remove MV3 tests that
    rely on content.fetch and content.WebSocket, since the "content"
    global has been removed with the unification of these methods in
    content scripts. Two test expectations were changed to account for
    the CSP enforcement regression (bug 1766813).

  • test_ext_contentscript_json_api.js was added because there is no
    specific coverage for the use of JSON APIs in content scripts, despite
    special handling in the code touched by this patch (from bug 1284020).
    I have also added a comment to the implementation to explain why the
    special handling was/is needed (this behavior remains unchanged).

  • test_ext_secfetch.js was modified to test the fetch behavior of MV3.
    A previously-commented out test (fetch with CORS) was enabled since
    that behavior was resolved in MV3, and the (still failing) MV2 test
    results are annotated with the current failure (due to bug 1605197).

  • test_ext_webSocket.js was modified to test the WebSocket behavior of
    MV3. Since this patch doesn't change the behavior for moz-extension:
    documents, the iframe tests are not affected by the fix. New
    content scripts-specific tests were introduced, along with test
    expectations for MV2 (unchanged) and MV3 (changed by this patch).

  • test_ext_xhr_cors.js was introduced to test various scenarios with
    using XHR from MV3 content scripts. MV2 test coverage was not strictly
    required because it was mostly covered by test_ext_permission_xhr.js,
    but included for comparison. There is new test coverage for requests
    with CORS (which was/is still failing in MV2 - bug 1605197).

Pushed by rob@robwu.nl: https://hg.mozilla.org/integration/autoland/rev/36906ca5d8d7 Use page's fetch/XHR/WebSocket in MV3 content scripts r=rpl

https://hg.mozilla.org/mozilla-central/rev/36906ca5d8d7

Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox101: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch

Is this covered in the recent MV3 changes?

Flags: needinfo?(rob)

We added a mention for this to the host_permissions page
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/host_permissions
but I'll leave it to Rob if he think that is enough.

The content script-specific note should be rephrased to make it clear that the permission's effect on content script is only in MV2, not MV3. I.e. the first and last bullet point at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/permissions#host_permissions

Side note, it would also be nice if that page is linked from the new host_permissions docs.

The "XHR and fetch" section should also be updated at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Content_scripts#content_script_environment
In MV3, there is no content variable any more, and no content.fetch etc.

This should all be documented at https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/101

Flags: needinfo?(rob)

Previously I was able to load json data packed with the extension from a content script via the fetch API (without having to expose it as a web_accessible_resources). This isn't possible with manifest v3 anymore:

Security Error: Content at https://developer.mozilla.org/ may not load or link to moz-extension://a23d0bd8-686d-484d-99e3-4bd6cab49616/file.json.

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1578405#c9 this should still be possible. However on the other hand fetch/xhr requests get the same "rights" as the website in MV3.

Can someone please clarify this? Thanks in advance.

Flags: needinfo?(tomica)

(In reply to robbendebiene from comment #31)

Previously I was able to load json data packed with the extension from a content script via the fetch API (without having to expose it as a web_accessible_resources). This isn't possible with manifest v3 anymore:

This is intentional. Fetch/XHR in a content script should behave like fetch/XHR of the normal web page. Extension resources can only be fetched if exposed via web_accessible_resources (or via the background, if you don't want to make it web-accessible).

Flags: needinfo?(tomica)
See Also: → 1871516
See Also: → 1896267
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: