Open Bug 1420286 Opened 4 years ago Updated 10 months ago

WebExtension executeScript fails with symlink when loaded temporarily on linux

Categories

(WebExtensions :: Untriaged, defect)

57 Branch
defect
Not set
normal

Tracking

(Not tracked)

REOPENED

People

(Reporter: sharmalay, Unassigned, NeedInfo)

References

Details

Attachments

(4 files, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171115095126

Steps to reproduce:

1) Download and extract the attached minimal web extension.
2) Navigate to about:debugging in Firefox.
3) Click "Load Temporary Add-on", navigate to extracted web extension folder and select manifest.json under the sym/ directory.
4) Go to any page in a new tab, click on the browser action for the temporary extension.


Actual results:

Errors are logged to the extension's background page:

"uncaught exception: Unable to load script: moz-extension://2894ec3a-b5df-43f8-8620-3b9ffa10c97d/content.js  (unknown)"

"Unchecked lastError value: Error: An unexpected error occurred  ExtensionCommon.jsm:407
	withLastError resource://gre/modules/ExtensionCommon.jsm:407:9
	wrapPromise/< resource://gre/modules/ExtensionCommon.jsm:460:11"


Expected results:

The content script should have been executed on the page, logging to the console.

Tested on Ubuntu 16.04.3 with Firefox 57 from repo, Firefox 58 beta and Firefox nightly (was unable to debug background page in nightly, but content script did not log to console as expected).

$ apt policy firefox
firefox:
  Installed: 57.0+build4-0ubuntu0.16.04.5
  Candidate: 57.0+build4-0ubuntu0.16.04.5

Firefox 58.0b5 (20171120142222)

Firefox 59.0a1 (20171123100420)

Chrome browser is able to temporarily load this test extension.
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit
Sorry, but this is an intentional security measure.
Status: UNCONFIRMED → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
Product: Toolkit → WebExtensions

Same issue here.
From my point of view, allow symbolic links isn't very harmful during develop, and is very convenient for sharing assets, and I can't see a clear disadvantage since Google Chrome support.

Please reconsider WONTFIX - there are numerous maintainers wanting this to be re-enabled on Debian. Ironically it is making Debian security worse, because we are forced to bundle copies of javascript libraries with every webextension, making it difficult for the security team to update any that have security holes.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=922944

The same symlinks in the same webextensions work fine on Chromium, I might add.

I should note that the Debian issue concerns system sideloaded extensions, but I'd imagine it's caused by the same underlying issue as for the temporarily-loaded extensions.

Investigation so far, into how to patch this out in Debian:

Setting MOZ_LOG=ScriptLoader:5,CSP:5, one can see that at some point control flow hits ScriptLoader::StartLoad in dom/script/ScriptLoader.cpp, and then CSPService::ConsultCSP in dom/security/nsCSPService.cpp. Unfortunately there is no explicit security policy logic in any of these files, probably the policy is interpreted from another file.

ExtensionProtocolHandler::AllowExternalResource in netwerk/protocol/res/ExtensionProtocolHandler.cpp seems related; it even mentions "system extensions use symlinks to point to resources in the repo dir which we have to allow loading"; I will try to binary-patch it out of my local firefox and see if it works.

Aha, MOZ_LOG=ExtProtocol:5 gives the following very specific error message:

[Parent 1726570: Main Thread]: D/ExtProtocol Rejecting external unpacked extension resource [/usr/share/javascript/punycode/punycode.js] from extension directory [/usr/share/webext/umatrix]

The error message comes from LogExternalResourceError in netwerk/protocol/res/ExtensionProtocolHandler.cpp and is only called from that same file, in ExtensionProtocolHandler::NewStream. A patch should be very straightforward.

Do I understand correctly that the debian package manager installs webextensions in an unpacked form, so that the only way they can be used is with temporary installation? It seems like a very inconvenient way to use an extension, the extension framework is generally built around the assumption that normal extension use is all from packed xpis.

I can't begin to guess what you mean by "inconvenient" - users never have to touch the unpacked version, they type in apt-get install webext-$whatever, that's about as convenient as it can get.

There is actually already code in Firefox that deals with these unpacked extensions, and even code to specially treat "system" extensions distributed with firefox itself. My suggested patch would simply be to allow the FOSS distributed to easily add some more paths to this whitelist. Currently only NS_GRE_DIR is whitelisted; for Debian's needs we would also need /usr/share/mozilla/extensions [*] which in fact appears to be defined by firefox itself in toolkit/xre/nsXREDirProvider.cpp - so actually it's plausible for this fix to be made in Firefox itself, rather than Debian.

Attached patch 922944.patch (obsolete) — Splinter Review

I am currently building this patch to test it. It is Debian-specific, but hopefully you can see it would be straightforward to put the bulk of this in Firefox. The patch does 3 things:

  • Don't short-circuit the AppDirContains check to false if IsDevelopmentBuild is true, since we are distributing to users. I see no security reason for this check, as long as we deal with the subsequent logic correctly.
  • Check /usr/share/mozilla/extensions instead of NS_GRE_DIR, but perhaps as Firefox upstream you would want this to be multiple paths
  • Remove the MOZ_TRY(extensionDir->Normalize()); step to normalize the extension path. In Debian, extensions in /usr/share/mozilla/extensions are actually symlinks elsewhere, e.g. to /usr/share/webext - this is so Chromium can also use them, symlinked into a neutral location.

BTW, Chromium follows these symlinks just fine, we don't need to patch it.

(In reply to infinity0 from comment #9)

I can't begin to guess what you mean by "inconvenient" - users never have to touch the unpacked version, they type in apt-get install webext-$whatever, that's about as convenient as it can get.

You mentioned using temporary installation in comment 5, which sounds like you have to take an explicit action to install the extension at the beginning of every browser session?

Support for unpacked extensions exists primarily for the extension development process, you're asking to weaken an existing security precaution to support a different scenario. General assertions like "I see no security reason for this" or "chromium already does this" aren't sufficient, somebody needs to analyze the implications carefully. That's only worthwhile if there is a compelling use case here. I'm not saying there isn't one, but if there is, it hasn't been explained clearly.

(In reply to Andrew Swan [:aswan] from comment #11)

(In reply to infinity0 from comment #9)

I can't begin to guess what you mean by "inconvenient" - users never have to touch the unpacked version, they type in apt-get install webext-$whatever, that's about as convenient as it can get.

You mentioned using temporary installation in comment 5, which sounds like you have to take an explicit action to install the extension at the beginning of every browser session?

No, Debian installs unpacked extensions to /usr/share/mozilla/extensions, which is what we have been doing with Firefox since before 2010, when I started doing this with Debian. This has "just worked" and Firefox loads them automatically - the user doesn't have to do anything.

My comment 5 was just stating that this scenario and the scenario described in comment 1 are the same bug, even though it is exhibited in different scenarios, to explain why I'm not filing a new bug.

[..] I'm not saying there isn't one, but if there is, it hasn't been explained clearly.

Does the above describe it clearly?

(In reply to infinity0 from comment #12)

My comment 5 was just stating that this scenario and the scenario described in comment 1 are the same bug, even though it is exhibited in different scenarios, to explain why I'm not filing a new bug.

Ah, I see, thanks for the clarification. Official Firefox builds disabled unpacked sideloaded extensions quite a while ago (bug 1385057). And sideloaded extensions of any sort were discontinued more recently (bug 1602839 and its dependencies). I guess Debian changes build flags to override these? In any case, the supported method to install an extension globally on a particular computer is to use policies (https://support.mozilla.org/ta-LK/kb/customizing-firefox-using-policiesjson) to install a packed xpi. It's not my decision, but I suspect it will be difficult to persuade anybody to spend time examining the security implications of relaxing the restriction on symlinks to enable an unsupported scenario when there is a supported alternative.

Thanks for the detail, though I note bug 1602839 says:

We need to continue supporting some sideloading for ESR and 3rd party distributions (linux, non-Firefox branded distributions).

and

Support a build-time flag (replacing preference) so that ESR and non-Firefox distributions can continue to support sideloading as necessary for their use cases.

Assuming what they mean by "support" is the same as what you mean by "support", then what I'm talking about (that has existed for over a decade) is a "supported" use-case albeit not for "official builds from Mozilla".

Furthermore the code I am touching explicitly tests that the extension location is a directory ("// The host should map to a directory for unpacked extensions"), meaning it cannot possibly be code that affects the official Mozilla builds, since they disable this scenario totally.

What I am suggesting is very basic in terms of the security logic needed to justify it. I'm not sure how to tag people on bugzilla but it looks like Shane Caraveo (:mixedpuppy), having commented on bug 1602839, would either be or know the person who could approve my suggestion?

A little bit of background on the code in ExtensionProtocolHandler::NewStream() that does not allow loading an unpacked extension file that is a symlink target outside of the extension directory: we have this restriction to prevent a situation where a user is given an unpacked extension that symlinks outside of the extension directory to a sensitive file and, as a result, the extension code running in a sandboxed process can read the contents of the file. Installing an extension should not grant the extension access to unexpected files on the filesystem (although to be clear, we are only dealing with unpacked/unzipped extensions here, not the typical extensions downloaded from addons.mozilla.org or extensions that bundled as an XPI.)

The code was added as part of the implementation to allow extensions to work when content processes (and now the extension process) are sandboxed and have no filesystem access to these files.

The term "system extension" refers to some special extensions shipped with Firefox that have more permissions than standard WebExtensions.

@infinity0, thanks for diving in and working on a patch. ExtensionProtocolHandler::NewStream() is only used for unpacked extensions. With Debian, do we need to support the symlink target being arbitrary locations on the filesystem or are the symlink targets always within a particular directory?

We should be able to come up with a solution that works here.

Flags: needinfo?(infinity0)

Hey Haik, thanks for the background! That roughly matches what I was guessing, and I made my patches along the lines of those security considerations.

With Debian, do we need to support the symlink target being arbitrary locations on the filesystem or are the symlink targets always within a particular directory?

We use symlinks quite a lot in Debian, which makes the current situation a bit awkward. However I have some working patches, so please read on:

In the general case, things on Debian can look like this:

/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1/file1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1/file2 -> ../../../../javascript/package1/file1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension2 -> ../../../webext/extension2
/usr/share/webext/extension2
/usr/share/webext/extension2/file3
/usr/share/webext/extension2/file4 -> ../../javascript/package2/file2
/usr/share/javascript/package1/file1
/usr/share/javascript/package2/file2

The symlink targets can in theory be anywhere, though typically they are under /usr. The extension directories (i.e. the symlink sources) are always underneath either /usr/share/mozilla/extensions/{uuid-of-firefox}/ or /usr/share/webext, for extensions shared between firefox and chromium.

ExtensionProtocolHandler::AllowExternalResource actually already contains logic to handle this type of situation, but only when IsDevelopmentBuild() is switched on, and it only works for a single whitelisted path - anything underneath NS_GRE_DIR is assumed to be trusted, and allowed to symlink anywhere else. This is the behaviour that exists in firefox today.

My patches extend this behaviour a bit, to provide a solution to this current issue for Debian. I will follow up this post with 2 patches, here is the explanation for them:

In my patch #1, I ignore IsDevelopmentBuild() and then change NS_GRE_DIR to /usr/share/mozilla/extensions on non-Mac systems. This matches similar logic from nsXREDirProvider::GetSystemExtensionsDirectory elsewhere in the source tree - although I'm not sure if the latter is actually used, XRE according to online docs has been totally replaced. In general a hardcoded path is unclean, and an enhancement to this patch might define a more "official" systems-extension-directory instead of hardcoding. Anyway, this fairly straightforward patch is sufficient to solve this issue for extension1 in my example above.

Then we run into another problem - firefox resolves symlinks when recording the extension base directory. So extension2 is considered to reside in /usr/share/webext, not To make our solution work for extension2, we have to also apply patch #2, to make firefox keep the symlink source. However, we run into another issue: Firefox apparently purposefully breaks symlink resolution in file:// URLs for security reasons I didn't fully dig into. To work around this, we therefore add a Normalize() step in SubstitutingProtocolHandler::ResolveURI which is called when loading the extension's files. There is one weirdness which you can read in the patch, but it worked fine for me when I compiled and tested them locally.

Feedback, and especially about this patch #2, would be greatly appreciated.

Flags: needinfo?(infinity0)
Attached patch 1420286-1.patchSplinter Review
Attachment #9143454 - Attachment is obsolete: true
Attached patch 1420286-2.patch (obsolete) — Splinter Review
Attached patch 1420286-2.patchSplinter Review

Whoops my bad, the previous version had a leftover from an old experiment. I'll also take this chance to fixup a part of my previous comment:

So extension2 is considered to reside in /usr/share/webext, not

extension2 is considered to reside in /usr/share/webext, not /usr/share/mozilla/extensions. This patch #2 instead makes the latter effective.

Attachment #9143742 - Attachment is obsolete: true

The changes you're pursuing in the patches is removing code that has existed for well over 2 years now. What I'd like to understand is, what changed? Is debian doing something different, or did something debian was already doing stop working?

If something that was working has stopped working, when? Use mozregression to pinpoint what changed that caused the issue. Then we can better consider if there is a path to support the existing debian system.

If something new is being done in debian you may well have to consider an alternative path.

Flags: needinfo?(infinity0)

What I'd like to understand is, what changed? Is debian doing something different, or did something debian was already doing stop working?

Debian is not doing anything different, I believe these symlink issues stem from the switch from XPI extensions to WebExtensions which happened around 2 years ago as you're observing.

I am not sure if (A) this code we're talking about here was also responsible for loading XPI extensions, but generally (B) XPI extensions (as I remember) were more self-contained in terms of their javascript than WebExtensions, so I do not remember a case where we needed to deduplicate embedded JS libs. This is not the case with WebExtensions, which tend to embed third-party JS libs quite freely, and we try to reduce the security maintenance burden this causes, by deduplicating with symlinks to /usr/share/javascript.

We have been symlink stuff from /usr/share/mozilla/extensions/{app-uuid}/ into /usr/share/xul-ext/ since forever, also to deduplicate between the various Mozilla apps. For example, lots of old XPI extensions were useful on both Firefox and Thunderbird. This never previously caused an issue, I would guess either because of (A) if that is true, or else (B) if (A) is not true.

Flags: needinfo?(infinity0)

I should further clarify that this bug was originally noticed in Debian just over 1 year ago, XPI was dropped 2 years ago, and we took our time migrating over from the old XPI extensions since they still worked in the ESR, and volunteers are lazy. :)

Indeed we've lived with it for a while - extensions are not a critical part of Debian - but as we package more WebExtensions we'll be running into this problem with more frequency. Given the ample free time during these lockdowns I therefore took some of it to figure out a patch.

You're confusing the file format (extension contents packed into an xpi file) with the way the extension is implemented (webextensions vs legacy extensions).
We do very limited testing of unpacked extensions, even if you land a patch that fixes the immediate problem, there's a real possibility that they'll break again in the future. You'll be much better off in the long run if your packaging system produces packed xpi files.

Edit: let's address the issue raised by Shane and Andrew first. Namely, Debian doesn't need to use unpacked WebExtensions. If debian switched to using XPI's for the extensions, symlinks would not be used because each extension is a single XPI file. A WebExtension can be in .XPI form or unpacked (unzipped) as files on the filesystems. The latter is intended for use by developers working on an extension.

(In reply to infinity0 from comment #16)

Hey Haik, thanks for the background! That roughly matches what I was guessing, and I made my patches along the lines of those security considerations.

With Debian, do we need to support the symlink target being arbitrary locations on the filesystem or are the symlink targets always within a particular directory?

We use symlinks quite a lot in Debian, which makes the current situation a bit awkward. However I have some working patches, so please read on:

In the general case, things on Debian can look like this:

/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1/file1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension1/file2 -> ../../../../javascript/package1/file1
/usr/share/mozilla/extensions/{uuid-of-firefox}/extension2 -> ../../../webext/extension2
/usr/share/webext/extension2
/usr/share/webext/extension2/file3
/usr/share/webext/extension2/file4 -> ../../javascript/package2/file2
/usr/share/javascript/package1/file1
/usr/share/javascript/package2/file2

The symlink targets can in theory be anywhere, though typically they are under /usr. The extension directories (i.e. the symlink sources) are always underneath either /usr/share/mozilla/extensions/{uuid-of-firefox}/ or /usr/share/webext, for extensions shared between firefox and chromium.

OK, got it. The example makes it clear.

ExtensionProtocolHandler::AllowExternalResource actually already contains logic to handle this type of situation, but only when IsDevelopmentBuild() is switched on, and it only works for a single whitelisted path - anything underneath NS_GRE_DIR is assumed to be trusted, and allowed to symlink anywhere else. This is the behaviour that exists in firefox today.

IsDevelopmentBuild() is used to identify cases where the developer is running Firefox out of a source tree. e.g., they have a mozilla-central repo built and are running ./mach run. On Mac and Linux, our builds use symlinks where the target of the symlink is the build object dir. This is to make builds more efficient by reducing copying and storage space (the object dir could be on another filesystem.)

My patches extend this behaviour a bit, to provide a solution to this current issue for Debian. I will follow up this post with 2 patches, here is the explanation for them:

In my patch #1, I ignore IsDevelopmentBuild() and then change NS_GRE_DIR to /usr/share/mozilla/extensions on non-Mac systems. This matches similar logic from nsXREDirProvider::GetSystemExtensionsDirectory elsewhere in the source tree - although I'm not sure if the latter is actually used, XRE according to online docs has been totally replaced. In general a hardcoded path is unclean, and an enhancement to this patch might define a more "official" systems-extension-directory instead of hardcoding. Anyway, this fairly straightforward patch is sufficient to solve this issue for extension1 in my example above.

Better not to change NS_GRE_DIR/GreD for this because we use it as the directory containing the executable and it is used in many places. I think that would break the development build case

From a security perspective, it would make sense to allow following symlinks out of unpacked extensions if the extension is in the system extension dir (from the directory service provider using key XRESysSExtPD) where we expect it to require administrator privileges to write to. Regardless of IsDevelopmentBuild(), we could additionally allow following symlinks out of XRESysSExtPD. I think that would solve the extension1 issue, but not extension2 in your example.

Then we run into another problem - firefox resolves symlinks when recording the extension base directory. So extension2 is considered to reside in /usr/share/webext, not To make our solution work for extension2, we have to also apply patch #2, to make firefox keep the symlink source. However, we run into another issue: Firefox apparently purposefully breaks symlink resolution in file:// URLs for security reasons I didn't fully dig into. To work around this, we therefore add a Normalize() step in SubstitutingProtocolHandler::ResolveURI which is called when loading the extension's files. There is one weirdness which you can read in the patch, but it worked fine for me when I compiled and tested them locally.

Feedback, and especially about this patch #2, would be greatly appreciated.

To support the example2 case, adding a new directory service key to map to a directory that contains system extensions shared with other browsers sounds reasonable to me--now that our WebExtension API is compatible with other browsers. That would be /usr/share/webext for Debian.

Can you say if that would address the problem for Debian? By that I mean allowing symlinks out of XRESysSExtPD and additionally a new directory service entry that would be for shared extensions.

Edit: let's address the issue raised by Shane and Andrew first. Namely, Debian doesn't need to use unpacked WebExtensions. If debian switched to using XPI's for the extensions, symlinks would not be used because each extension is a single XPI file.

The reason Debian uses symlinks is to deduplicate embedded javascript libraries. Putting this in a single XPI file would prevent us from doing this, and force us to do a lot of other stuff which drains volunteer time - namely, adding infrastructure and scripts to track and traverse metadata describing which XPIs contain which javascript libraries, so that we can adhere to their copyright distribution requirements, and so that when security bugs are found, we fix all of the copies of those libraries, which would also involve re-building all of those XPI files again.

Instead of doing all of that, we use symlinks, that is why. This is analogous to why Debian also prefers shared libraries, static libraries are harder to deal with on a distribution-wide basis. The economics are different for well-funded individual projects that only need to watch out for embedded libraries in their own applications - like big browsers - but we have to do this across all packages. You probably are aware of the difficulties Google have in getting security fixes into random Android apps, since they generally embed their libraries. We really really really want to avoid this situation. It also gives FOSS a bad name - people in non-tech mainstream media like to say "Android has security problems because it's open", but that's really not the case with a shared library model, as we can demonstrate with Debian.

So we would much appreciate it if unpacked extensions with symlink traversal are supported going forward.

Can you say if that would address the problem for Debian? By that I mean allowing symlinks out of XRESysSExtPD and additionally a new directory service entry that would be for shared extensions.

This sounds like it would fit our use-case yes. Shall I update my above patch for this? Presumably the new directory service entry should be configurable by the build - could you please give some hints on that too.

I'd say there is a good chance I'll wontfix this again, but since the conversation is happening here, I'm reopening for now.

Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: WONTFIX → ---
Flags: needinfo?(dveditz)
Flags: needinfo?(dveditz)
Flags: needinfo?(dveditz)

(In reply to infinity0 from comment #25)

Can you say if that would address the problem for Debian? By that I mean allowing symlinks out of XRESysSExtPD and additionally a new directory service entry that would be for shared extensions.

This sounds like it would fit our use-case yes. Shall I update my above patch for this? Presumably the new directory service entry should be configurable by the build - could you please give some hints on that too.

This solution sounds acceptable to me. Ideally Debian would not use unpacked extensions and should work towards that. In the meantime, changing the ExtensionProtocolHandler::AllowExternalResource() to allow following symlinks outside of an unpacked WebExtension if the extension is installed in the system extension dir (/usr/share/mozilla/extensions) OR a new system extension dir (Debian would use /usr/share/webext) proposed here for common extensions seems acceptable to me.

That is assuming the WebExtension team supports this to. I would defer to Shane on this.

Since the system extension dir is expected to be read-only for most users and populated by the system administrator, I'm not concerned about the security impact. This would only apply to Mac and Linux. And we could go further and not include Mac or even limit this to a pref to be toggled on by the distro.

There are some efficiency gains when using an XPI instead of an unpacked WebExtension. For the unpacked case, the parent process has to open each resource file as it is used and send the file descriptor to the content or extension process using it. When the WebExtension is in XPI form, the parent process only opens the XPI file and sends the file descriptor to the child process where it is opened and then decompressed in memory. So unpacked extensions with several files have more overhead and file descriptor usage.

I have little knowledge of apt packaging, but could Debian use postinstall scripts to copy files instead of symlinking?

(In reply to Haik Aftandilian [:haik] from comment #27)

This solution sounds acceptable to me. Ideally Debian would not use unpacked extensions and should work towards that. In the meantime, changing the ExtensionProtocolHandler::AllowExternalResource() to allow following symlinks outside of an unpacked WebExtension if the extension is installed in the system extension dir (/usr/share/mozilla/extensions) OR a new system extension dir (Debian would use /usr/share/webext) proposed here for common extensions seems acceptable to me.

So one issue with this is that with the removal of support for sideloading, there's no need for the directory service to keep track of system-wide extension directories any more. The corresponding code hasn't been removed from the directory service yet but if we add code to the extension protocol handler that depends on it, it's just going to make it harder to clean that up later.

Currently I'm really not very supportive of this, but I am willing to at least consider whether there is a path forward that will work.

Unpacked unsigned extensions relying on symlinks essentially bypasses security considerations made in the extension system, even if it is limited to "admin accessible only" directories. I've asked @dveditz to look over this bug to get a better picture of security considerations we may want if we were to support this. Personally I think distributing the packed/signed extensions from AMO is a better path, we probably have more experience and resources put into addressing problems in addons.

Additionally, reiterating comment 28, our direction has been moving towards removing system level installs and relying on enterprise policy files. Those system directories are still available on distros using a build flag, but supporting that functionality is a chunk of work and maintenance.

If we were to accept this functionality I would want to see (as a start):

  • use of an existing system level directory (there are already 3 to choose from, a new directory means more work in AddonManager)
  • a build flag that is not part of the official mozilla builds, along with a pref to enable it during automated tests
  • extensive enough tests to ensure the functionality continues to work properly

Also, unpacked/unsigned extensions will not work with blocklist v3, so those extensions would eventually not be blockable.

(In reply to Shane Caraveo (:mixedpuppy) from comment #29)

Additionally, reiterating comment 28, our direction has been moving towards removing system level installs and relying on enterprise policy files. Those system directories are still available on distros using a build flag, but supporting that functionality is a chunk of work and maintenance.

Sorry, I wasn't aware of how we're moving away from system install directories. It sounds like what I proposed doesn't fit well with the direction we're going in for these type of WebExtensions.

(In reply to Haik Aftandilian [:haik] from comment #31)

(In reply to Shane Caraveo (:mixedpuppy) from comment #29)

Additionally, reiterating comment 28, our direction has been moving towards removing system level installs and relying on enterprise policy files. Those system directories are still available on distros using a build flag, but supporting that functionality is a chunk of work and maintenance.

Sorry, I wasn't aware of how we're moving away from system install directories. It sounds like what I proposed doesn't fit well with the direction we're going in for these type of WebExtensions.

They still exist, and we actually still read prior loaded extensions from them. But we no longer sideload new extensions from them unless you build with a build flag to enable that. We did that explicitly to not break the next esr and provide a way for linux distros to still use them. We probably wont change this more in the near future, but eventually we'd like to remove all sideloading.

Thanks for the discussion. I have a few comments regarding the longer-term topics, and then my next post will be a (non-working) patch and some questions about it.

I have little knowledge of apt packaging, but could Debian use postinstall scripts to copy files instead of symlinking?

We'd want the upgrade of the library to trigger some behaviour of a package depending on it (here, copying the library), but in general libraries aren't aware of who depends on it, and trying to detect this on every package upgrade would be costly for the user. There is a related mechanism called triggers, but again this requires the libraries to explicitly cause the triggers to be run - meant for manpages, fontconfigs, caches, etc.

So, it would likely be easier & simpler to just do the copying.

Personally I think distributing the packed/signed extensions from AMO is a better path

Understood, I will explore this in Debian - it will involve writing more scripts to re-pack our extensions, and track metadata on what libraries were embedded. It may take a year or so to get around to implementing.

Regarding the signing however, Debian and other FOSS distros generally want to retain the ability to arbitrarily patch packages, so it is essential for Firefox to retain the ability to load unsigned extensions, of course as defined and controlled by the system policy.

our direction has been moving towards removing system level installs and relying on enterprise policy files. [..] We probably wont change this more in the near future, but eventually we'd like to remove all sideloading.

I can see that this might also be a pain for us in the far future. Would it be possible to add support for a policy entry that says "install all extensions in this directory"? Otherwise we'd need every packaged extension to contain postinstall triggers (as mentioned above) to automatically update the policy file, which would be fiddly and error-prone. If this sounds sensible I can file a separate issue.

Attached patch 1420286.patchSplinter Review

Here is an initial attempt at another patch along the lines of comment 24. It builds, but doesn't work. I basically extended nsXREDirProvider.cpp to define an additional directory for shared WebExtensions, and mirrored the existing code. Of course the repetition is not nice, for now I just wanted to see if the approach worked. However it doesn't work.

Adding some LOG calls I can see that the new NS_GetSpecialDirectory calls fail. I don't know what to replace them with. Searching through the code, I do see calls like NS_GetSpecialDirectory(XRE_[...] in other files - there are not many, but they are there - so I don't understand why this instance is failing. I tried to find alternative directory services, but could only see one NS_DIRECTORY_SERVICE_CONTRACTID which is what NS_GetSpecialDirectory uses.

BTW I am working on top of firefox 75.0 code as this is the latest version in Debian - please let me know if I should instead work from mozilla-central hg.

a build flag that is not part of the official mozilla builds, along with a pref to enable it during automated tests

There is an existing macro ENABLE_SYSTEM_EXTENSION_DIRS which seems to be on by default, or at least I don't see it explicitly set in the Debian build scripts. It's only used in a very few places, so could we just repurpose it?

extensive enough tests to ensure the functionality continues to work properly

Where would these tests go, and are there other similar tests I can use as starting examples?

Duplicate of this bug: 1654463

Please at least enable them in the plugin development, it's useful, thanks!

You need to log in before you can comment on or make changes to this bug.