+++ This bug was initially created as a clone of Bug #1269180 +++ But 1269180 was originally opened to report errors in the browser console originating from the MatchPattern constructor. The original bug has multiple causes, but one of them is that the extension being tested tries to use the webRequest api to monitor loading of extension-local resources (i.e., moz-extension://... urls). This is currently unimplemented, this bug is to track work to design and implement that functionality.

I'm curious why you'd want to monitor loading of extension local resources using webRequest.

Is this something you are getting in bug 1273138 mixedpuppy and we should dupe?

Well, matchpattern fails on moz-extension, and fetch (within an extension background script) is also not working with moz-extension. Neither are covered by bug 1273138.

failing test.

A fix with working tests. However...looking for feedback on the matchpattern change.

Comment on attachment 8807760 [details] [diff] [review] webReqMozExtension Review of attachment 8807760 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/extensions/test/mochitest/test_ext_webrequest_mozextension.html @@ +60,5 @@ > + fetch(browser.extension.getURL("finished.html")).then(() => { > + browser.test.notifyPass("fetch recieved"); > + browser.test.sendMessage("done"); > + }, () => { > + browser.test.fail("fetch recieved"); Please use `notifyFail` instead, and `awaitFinish` rather than `awaitMessage("done")` below. ::: toolkit/modules/addons/MatchPattern.jsm @@ +17,5 @@ > this.EXPORTED_SYMBOLS = ["MatchPattern", "MatchGlobs", "MatchURLFilters"]; > > /* globals MatchPattern, MatchGlobs */ > > +const PERMITTED_SCHEMES = ["http", "https", "file", "ftp", "data", "moz-extension"]; If we do this, I think we want to restrict extensions to matching URLs for their own extension. At least for now.

(In reply to Kris Maglione [:kmag] from comment #7) > If we do this, I think we want to restrict extensions to matching URLs for > their own extension. At least for now. That would prevent addons monitoring other addons (original use case in comment 0). It would also require a different test.

It would, but there's still a lot of disagreement about whether extensions should be able to monitor other extensions, and this is currently blocking other bugs, so I'd rather handle the non-controversial use case first.

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review92148 Sorry, but security has already weighed in on this. Until we have a security model for cross-extension permissions, we need to restrict this to the extension's own URLs.

Sorry, I didn't actually mean to push that to review. I think I got overly eager updating patches.

This patch automatically adds the extension url as a whitelist and webrequest match pattern and protects against the addition of extension urls otherwise.

(In reply to Kris Maglione [:kmag] from comment #11) > Comment on attachment 8809482 [details] > Bug 1271354 support moz-extension in webrequests, > Sorry, but security has already weighed in on this. Until we have a security > model for cross-extension permissions, we need to restrict this to the > extension's own URLs. This has been on my mind. Is there anything recorded around the reasons for this? Who on the security team decided this? It's hard to imagine requests from a webextension is going to be more sensitive than all the pages I normally visit (given all_urls permission). OTOH allowing something like ublock the ability to monitor communications by other addons could be useful. I'm also uncertain how (otherwise given some kind of ability to do it) an alternate tab manager addon would even work without being able to query all tabs, including about: tabs.

Oh, what I left out of the above question/comment is that we could require "moz-extension" to be explicit in the permissions for an addon to be able to use that with other APIs (tabs.query, webrequest, etc).

(In reply to Shane Caraveo (:mixedpuppy) from comment #14) > This has been on my mind. Is there anything recorded around the reasons for > this? Who on the security team decided this? Yes, but the bugs in question are not public. (In reply to Shane Caraveo (:mixedpuppy) from comment #15) > Oh, what I left out of the above question/comment is that we could require > "moz-extension" to be explicit in the permissions for an addon to be able to > use that with other APIs (tabs.query, webrequest, etc). We could, except that the hosts for moz-extension: URLs aren't predictable, so they'd have to request blanket permission for the entire scheme.

Comment on attachment 8811033 [details] [diff] [review] webReqMozExtension Review of attachment 8811033 [details] [diff] [review]: ----------------------------------------------------------------- The general idea seems sound, but can we instead pass the whitelisted prefixes to the MatchPattern constructor, and have it automatically apply it for the <all_urls> permission and whitelist it when doing moz-extension: checks?

(In reply to Kris Maglione [:kmag] from comment #16) > (In reply to Shane Caraveo (:mixedpuppy) from comment #14) > > This has been on my mind. Is there anything recorded around the reasons for > > this? Who on the security team decided this? > > Yes, but the bugs in question are not public. Can I get cc'd on them? > (In reply to Shane Caraveo (:mixedpuppy) from comment #15) > > Oh, what I left out of the above question/comment is that we could require > > "moz-extension" to be explicit in the permissions for an addon to be able to > > use that with other APIs (tabs.query, webrequest, etc). > > We could, except that the hosts for moz-extension: URLs aren't predictable, > so > they'd have to request blanket permission for the entire scheme. Yes, that is the idea.

Turning on host permissions checking in bug 1311815 means that test_webRequest_background_events [1] needs to match against the moz-extension URIs, or get disabled. Therefore, if bug 1311815 lands before this, this bug should include fixing and enabling the test. 1) searchfox.org/mozilla-central/search?q=test_webRequest_background_events

(In reply to Tomislav Jovanovic :zombie from comment #20) > Turning on host permissions checking in bug 1311815 means that > test_webRequest_background_events [1] needs to match against the > moz-extension URIs, or get disabled. > > Therefore, if bug 1311815 lands before this, this bug should include fixing > and enabling the test. > > 1) searchfox.org/mozilla-central/search?q=test_webRequest_background_events An addon should not require permission to itself in order to make a request. Further that test has all_urls permission.

Hello All, I raised bug: 1344754, which was redirected as DUP to this bug. I have ported my chrome extension to Firefox web extension, would like to release it to customers. It is blocked by this bug. How? --> My extension need to identify proxy server to perform authentication. I can identify the proxy server only during re-direct. The API browser.webRequest.onBeforeRedirect.addListener didn't listen to any redirect, ultimately failed my extension at the nacent stage. I already see 10 months of discussion in this bug, which should have been done before supporting API. Question: When is the probable fix date for this bug. Will there by support for <all_urls> ? If not, I've no other choice except to drop my extension plan, since extension has to communicate with different server address and they keep on changing as per release.

(In reply to sankar88.aj from comment #23) > Hello All, > > I raised bug: 1344754, which was redirected as DUP to this bug. I de-duped, but still closed your bug, see my comments there.

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review135486 ::: toolkit/modules/addons/MatchPattern.jsm:22 (Diff revision 2) > this.EXPORTED_SYMBOLS = ["MatchPattern", "MatchGlobs", "MatchURLFilters"]; > > /* globals MatchPattern, MatchGlobs */ > > const PERMITTED_SCHEMES = ["http", "https", "file", "ftp", "data"]; > -const PERMITTED_SCHEMES_REGEXP = PERMITTED_SCHEMES.join("|"); > +const PERMITTED_SCHEMES_REGEXP = PERMITTED_SCHEMES.concat("moz-extension").join("|"); [...PERMITTED_SCHEMES, "moz-extension"].join("|") ::: toolkit/modules/addons/MatchPattern.jsm:77 (Diff revision 2) > this.schemes = []; > return; > } > > + // We disallow the host to be * for moz-extension URLs. > + if (match[2] == "*" && this.schemes[0] == "moz-extension") { I'd rather we not allow to modify other extension URLs, even if they specify them explicitly.

(In reply to Kris Maglione [:kmag] from comment #26) > ::: toolkit/modules/addons/MatchPattern.jsm:77 > (Diff revision 2) > > this.schemes = []; > > return; > > } > > > > + // We disallow the host to be * for moz-extension URLs. > > + if (match[2] == "*" && this.schemes[0] == "moz-extension") { > > I'd rather we not allow to modify other extension URLs, even if they specify > them explicitly. I'm actually not certain this is even necessary since we double check manifest permissions via overlapsPermissions, as long as we disallow moz-ext in manifest permissions. If it is required this gets complicated to check due to how and where MatchPattern is being used.

(In reply to Shane Caraveo (:mixedpuppy) from comment #27) > (In reply to Kris Maglione [:kmag] from comment #26) > > I'd rather we not allow to modify other extension URLs, even if they specify > > them explicitly. > > I'm actually not certain this is even necessary since we double check > manifest permissions via overlapsPermissions, as long as we disallow moz-ext > in manifest permissions. OK, let's do that, then.

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review135486 > I'd rather we not allow to modify other extension URLs, even if they specify them explicitly. Tests show that an addon cannot use permissions to other addons.

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review151534 ::: toolkit/components/extensions/Extension.jsm:577 (Diff revision 3) > > this.permissions.add(perm); > } > + > + // An extension always gets permission to its own url. > + let matcher = new MatchPattern(this.getURL("*"), {ignorePath: true}); Nit: No `"*"` please. ::: toolkit/components/extensions/Extension.jsm:578 (Diff revision 3) > + whitelist.push(matcher); > + this.permissions.add(matcher.pattern); How does this not cause permissions tests to fail? ::: toolkit/components/extensions/Extension.jsm:579 (Diff revision 3) > } > + > + // An extension always gets permission to its own url. > + let matcher = new MatchPattern(this.getURL("*"), {ignorePath: true}); > + whitelist.push(matcher); > + this.permissions.add(matcher.pattern); Shouldn't be necessary. ::: toolkit/components/extensions/MatchPattern.cpp:300 (Diff revision 3) > nsCOMPtr<nsIAtom> scheme = NS_AtomizeMainThread(StringHead(aPattern, index)); > if (scheme == nsGkAtoms::_asterisk) { > mSchemes = AtomSet::Get<WILDCARD_SCHEMES>(); > } else if (permittedSchemes->Contains(scheme)) { > mSchemes = new AtomSet({scheme}); > + } else if (scheme == nsGkAtoms::moz_extension) { Nit: Please just add this as an `||` clause to the above condition. ::: toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_mozextension.html:17 (Diff revision 3) > +<body> > + > +<script type="text/javascript"> > +"use strict"; > + > +let peakAchu; Hm. ::: toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_mozextension.html:67 (Diff revision 3) > + > + let waitForConsole = new Promise(resolve => { > + SimpleTest.monitorConsole(resolve, messages); > + }); > + > + extension.startup(); `await` please. ::: toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_mozextension.html:69 (Diff revision 3) > + SimpleTest.monitorConsole(resolve, messages); > + }); > + > + extension.startup(); > + await extension.awaitFinish("loaded"); > + extension.unload(); `await` ::: toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_mozextension.html:148 (Diff revision 3) > + return; > + } > + browser.test.log(`tab created ${tabId} ${JSON.stringify(tabInfo)} ${tab.url}`); > + let tabs = await browser.tabs.query({url: browser.extension.getURL("*")}); > + browser.test.assertEq(1, tabs.length, "got one tab"); > + browser.test.assertEq(tabs.length && tabs[0].id, tab.id, "got the right tab"); Nit: *correct ::: toolkit/modules/addons/MatchPattern.jsm:125 (Diff revision 3) > -this.MatchPattern = function(pat) { > +this.MatchPattern = function(pat, extensionUrl) { > this.pat = pat; > if (!pat) { > this.matchers = []; > } else if (pat instanceof String || typeof(pat) == "string") { > this.matchers = [new SingleMatchPattern(pat)]; > } else { > this.matchers = pat.map(p => new SingleMatchPattern(p)); > } > + if (extensionUrl) { > + this.matchers.push(new SingleMatchPattern(extensionUrl)); > + } This doesn't seem to be used anymore.

Status on this: there are a bunch of permission related test failures, which I started to fix in the tests. However, one of the failures has to do with permissions on reload of an extension and I'm not certain that should be worked around in tests. I haven't finished investigating.

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review160052

Comment on attachment 8809482 [details] Bug 1271354 support moz-extension in webrequests, https://reviewboard.mozilla.org/r/92062/#review160056 Sorry, didn't mean to reset the review flag there.

Pushed by mixedpuppy@gmail.com: https://hg.mozilla.org/integration/autoland/rev/60823f6c03f2 support moz-extension in webrequests, r=kmag

Even I stumbled over this for a while. We need to document how an addon can use webrequest to monitor requests on its own urls. Specifically, even if the addon has <all_urls> permission, it must specify the moz-ext url in the filters when adding a listener. Here's how: let uriMatch = browser.extension.getURL("*"); browser.webRequest.onBeforeRequest.addListener(details => { ... }, {urls: ["<all_urls>", uriMatch]}, ["blocking"]);

I can't make this work (Firefox Nightly, 57.0a1). manifest.json: -------------- { "manifest_version": 2, "name": "example1", "version": "1.0", "browser_action": { "default_icon": "icons/on.svg", "default_title": "on" }, "background": { "scripts": ["background.js"] }, "permissions": ["webRequest", "<all_urls>"] } -------------- background.js: -------------- function logURL(requestDetails) { console.log("Loading: " + requestDetails.url); } browser.webRequest.onBeforeRequest.addListener( logURL, {urls: ["<all_urls>", browser.extension.getURL("*")]} ); browser.browserAction.onClicked.addListener(() => { browser.tabs.create({ url: "my-page.html" }); }) -------------- I see logging when I visit any http/https page. When I click the icon the packaged page "my-page.html" is opened, but I don't see any logging. What am I doing wrong?

Look at toolkit/components/extensions/test/mochitest/test_chrome_ext_webrequest_mozextension.html specifically the test_webRequest_mozextension_fetch and test_webRequest_mozextension_tab_query tests. I'll see if I can get around to testing what you tried.

It works with fetch(), which is what the test code uses. But apparently not with browser.tabs.create(): browser.webRequest.onBeforeRequest.addListener( logURL, {urls: ["<all_urls>", browser.extension.getURL("*")]} ); browser.tabs.create({ url: "https://example.com" // gets intercepted }); browser.tabs.create({ url: browser.extension.getURL("my-page.html") // does not get intercepted }); fetch(browser.extension.getURL("https://example.com")); // gets intercepted fetch(browser.extension.getURL("my-page.html")); // gets intercepted

There are caveats on snooping on web-ext loads. web-extension requests are not http requests and so do not use httpchannel (even when accessed via fetch). We kind of fake the request via the content policy manager (which is the only way we see non-http requests). So the first is you only get onBeforeRequest and onCompleted or onErrorOccurred for those requests. The second is that if it is initiated from a system principal, the extension will not see the request at all. This is why you do not see the request when opening the page from tabs.create. You do see the request for example.com when opened via tabs.create because that is a real http request and we handle that from different code paths.

So I've tried to document various aspects of this: * extensions automatically get their own URL as a host permission: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json/permissions#Host_permissions * tabs.query can include "moz-extension://..." in `url`: I've added a couple of examples for this: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/tabs/query#Examples * you can listen for requests to your own URLs, with some caveats: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest#Intercepting_extension_requests Please let me know if this covers it, or if I've got something wrong.

That looks good.

https://bugzilla.mozilla.org/show_bug.cgi?id=1271354#c11 Regarding the above, is there any open bug about allowing WebExtensions to block/monitor requests from other WebExtensions?