Closed Bug 1575625 Opened 5 years ago Closed 3 years ago

moz-extension:// schema is not considered secure for Cache Web API

Categories

(WebExtensions :: Storage, defect, P3)

69 Branch
defect

Tracking

(firefox90 fixed)

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: zelus, Assigned: anatal)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/76.0.3809.100 Safari/537.36

Steps to reproduce:

  1. Clone the Mozilla webextensions-examples repo: https://github.com/mdn/webextensions-examples

  2. Add the following code to webextensions-examples/beastify/popup/choose_beast.js

caches.open("foo").then(function(cache) {
console.debug("Caches Open.");
return cache.keys();
}).then(keys => {
console.debug("Caches Then.");
console.log(keys);
return keys;
});

  1. Load the Beastify extension as a temporary add-on in about:debugging.

  2. Inspect the Beastify extension using the Inspect button in about:debugging.

  3. Click the Beastify icon in the top-right corner.

Actual results:

In the console, this error appears: "SecurityError: The operation is insecure."

Expected results:

The following output should have appeared:
Caches Open.
Caches Then.

Array(0)

Background/context/use cases:

For a Firefox Add-On I would like to make use of the Cache Web API, but the Cache Web API doesn't seem to be usable in any version of Firefox on any page with a schema other than https://. All Firefox Add-Ons use the moz-extension:// schema at some level, and it's not considered secure, so the Cache Web API can't be used on Add-On pages.

The Add-On I'm writing intends to use the Cache Web API to cache web requests that contain large JSON responses. I want to save the user the trouble of re-downloading content requested from an API that has been explicitly allowed by the content_security_policy in the Add-On manifest. Additionally, I want to provide a way for the user to "walk" through their cached web requests and view them, download them, or re-run them individually - like an auditing tool.

In the published version of the Add-On localStorage is being used, however, it's not very performant (it blocks disk I/O; files larger than 2MB are slow to load) and the Cache Web API has modern features like Promises. The Cache Web API seems superior to localStorage.

There doesn't seem to be a way to make moz-extension:// pages secure to satisfy the schema condition for using the Cache Web API. In Google Chrome however, the equivalent schema (chrome-extension://) is considered secure and does allow for the Cache Web API to be used.

Hi,
I'm setting the component in order to involve the development team in reviewing this issue. Feel free to change it, if I'm wrong.
Thanks!

Component: Untriaged → Storage
Product: Firefox → WebExtensions
Version: 69 Branch → Firefox 69
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: Firefox 69 → 69 Branch

I think it likely makes sense to just return true here for moz-webextension instead of also checking the backgroundServiceWorker.enabled pref. This came up in discussion with Andre after I recommended use of Cache API storage from a web extension.

I expect I was behind the decision to be very conservative about this (or my other very conservative stances elsewhere propagated to this), but this actually seems quite reasonable to me to grant access to... the only reason we're not using [SecureContext] here was for testing purposes which made this a weird policy check. Note that this is still separate from storing moz-webextension schemes for Requests in Cache API storage (which should also eventually happen, but should be fine and can happen after this).

Does this make sense, :rpl, or am I forgetting something big? Thanks!

Flags: needinfo?(lgreco)
Assignee: nobody → anatal

Hey Andrew,
you are definitely right, we discussed explicitly about this in D60244, the conversation we had about this can still be found nearby this comment https://phabricator.services.mozilla.com/D60244#inline-428541, where I did mention exactly this bug.

Quoting the parts related to this here:

My first comment about this:

I'm wondering if it would be reasonable to guard it with its own pref, separately from the pref that toggle the availability of the extension background ServiceWorker, I remember that there were some requests to make the Cache API available to the extension code (separately from the requests to allow the extension to register their own service workers), e.g. Bug 1575625 (comment 1 does mention the use case for the extension developer that reported that enhancement request).
Is there any issue that we may trigger if we do enable the Cache API before we are going to allow the extension to register a service worker?

your reply:

It's reasonable for it to be its own preference, but then we might want these Cache API checks to basically be the logical OR-ing of the service worker pref and the explicit Cache API pref to avoid users creating fundamentally broken preference permutations.
The bar for turning Cache API on and leaving it on is much lower, we just want to make sure we're intentional about letting it ride beyond nightly.

my last reply:

...
totally agree, "permutations" of these preferences isn't definitely something that we want to deal with :-)
...
oky, sounds great, I may try to do it in this patch, but it isn't really a priority and so it may as well something to defer to a follow up (which could actually be Bug 1575625 itself)
how that would sounds to you?


And so it looks that yes, we were meant to follow up about this issue at some point.

Thanks to André for picking this up!!!

It is also worth to mention (because we will likely hit these tests while changing the related behaviors), as part of Bug 1609920 we landed also two test cases that are covering the expected behavior of using the Cache API from an extension page:

Let me know if there is any other detail that may help André with this, otherwise I'll be looking forward to the patch that will fix this issue and I'll be very happy to review and signing it off from our perspective \o/

Flags: needinfo?(lgreco)

Make Cache API available to webextensions (moz-extension:// schemas)

Thanks @rpl and @asuth. These were useful pointers and the change worked.

I pushed a preliminary wip patch to unblock my team in Europe to start working on it on Monday while I sync with @asuth on the morning PST on the details to make it fully landable.

Pushed by bugmail@asutherland.org:
https://hg.mozilla.org/integration/autoland/rev/f97c935ebd9a
Make Cache API available to webextensions (moz-extension:// schemas) r=asuth,rpl
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

For this documentation should we update:

Flags: needinfo?(lgreco)

(In reply to Richard Bloor from comment #10)

For this documentation should we update:

The first option (mention it in the Cache WebAPI doc page) sounds like a really good option to me, and so my that would have my +1 on double-checking with who is maintaining that MDN page if it would be a reasonable choice also from their perspective (feel free to mention my handle on github comments if it is being discussed in a github issues or pull request).

Mentioning it also in the Secure contexts may also be reasonable but in practice it may not be always already the case ([1]), and so for now I would be ok if we would choose to only apply the change to the Cache WebAPI doc page.


[1]: in most cases we would consider that a bug to fix, but we would still have a case by case discussion with the team that maintains the webAPI to double-check if we should allow extension to use a particular API. In practice it is not unlikely that there may be some more "Secure Contexts"-only WebAPIs that needs fixes (like for this one) to be available AND work correctly in extension globals. That isSecureContext property that is explicitly mentioned in that doc page seems to be already returning true when accessed from an extension pages, but in practice the condition that isSecureContext property is checking internally may not be always the only one that needs to be satisfied in practice to make a certain WebAPI to be visible and also work as expected in a moz-extension global.

Flags: needinfo?(lgreco)

After discussion on Bug-1575625-Make-Cache-API-available-to-webextensions #8397 the consensus was that no further documentation was needed as this was a bug fix of functionality that should have been working.

Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.