1277920 - Sign system add-ons in the application directory

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

This was reported over IRC. A user (or malware) can bypass add-on signature verification by putting an unsigned add-on in {firefox application directory}/browser/features/ (where system add-ons live, I think). When Firefox next runs, all of those add-ons are loaded up (seemingly without checking for signatures?).

Comment 1

[Benjamin Smedberg]

Rob, I thought system addons were signature-checked like everything else?

Comment 2

[Robert Helmer [:rhelmer]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #1)
> Rob, I thought system addons were signature-checked like everything else?

Updates are, but XPI files that ship with Firefox itself are not. Nothing in the Firefox application directory is signed AFAIK.

It was determined at the time the system add-on implementation landed (I'll find the bug) that system add-ons has the same risk profile as adding files into omni.ja, since that is not signed as well.

We'd *really like* to check for signatures, but signing content that gets shipped in Firefox is not something that releng is able to do yet, as far as I am aware.

Comment 3

[Robert Helmer [:rhelmer]]

bug 1212059 comment 2 is the bug I am thinking of.

Comment 4

[Daniel Veditz [:dveditz]]

I draw your attention to the part of that comment that says

  "Sadly this is going to get abused sooner or later. Can get away with it in the short term but
  eventually either the add-ons or the manifest will have to be signed."

This is the bug to fix that. Dropping a .xpi into a directory is a far lower bar than correctly modifying omni.ja that gets regularly updated to a fresh copy.

Comment 5

[Mike Hommey [:glandium]]

Please note that Linux distros won't be able to sign either, but it's not really a problem because the Firefox directory is not writable in the first place. If it is, users have more serious problems than Firefox system addons.

(Just something to consider when the check is implemented; it should be opt-in or opt-out at configure time, I guess)

For the short term, wouldn't hardcoding the list of system addons in the addons manager somehow help a little? It would make it as difficult to alter the existing xpis as with omni.ja, although malware could get away with entirely replacing system addons, then...

Comment 6

[Robert Helmer [:rhelmer]]

(In reply to Mike Hommey [:glandium] from comment #5)
> Please note that Linux distros won't be able to sign either, but it's not
> really a problem because the Firefox directory is not writable in the first
> place. If it is, users have more serious problems than Firefox system addons.
> 
> (Just something to consider when the check is implemented; it should be
> opt-in or opt-out at configure time, I guess)
> 
> For the short term, wouldn't hardcoding the list of system addons in the
> addons manager somehow help a little? It would make it as difficult to alter
> the existing xpis as with omni.ja, although malware could get away with
> entirely replacing system addons, then...

Hardcoding the addon IDs would preclude us from shipping updates that hadn't shipped with Firefox in the first place (which is relatively rare so far, but has been used for hotfix-style activities). This means malware would need to override one of our existing add-ons rather than being able to drop in a new one, which raises the bar a little bit but not insurmountable.

Note that we *do* have a way to control the set of system add-ons installed, via Balrog. That bit is done by prefs though and presumably malware that can touch the Firefox application directory can even more easily touch the profile.

I think the right fix here is to ship signed add-ons. This gives us the flexibility we need, and forces malware authors to go the route of patching the Firefox binary to disable signing, which is as far down the road as we can kick this can.

Comment 7

[Robert Helmer [:rhelmer]]

Taking this on the toolkit side, and I will follow up on with releng to make this a reality.

Comment 8

[Robert Helmer [:rhelmer]]

The code change here is trivial - for future reference, this line should be removed:
https://dxr.mozilla.org/mozilla-central/rev/51377a64158941f89ed73f388ae437cfa494c030/toolkit/mozapps/extensions/internal/XPIProvider.jsm#714

Instead, there should be an early check that the proper key was used for KEY_APP_SYSTEM_ADDONS and KEY_APP_SYSTEM_DEFAULTS:
https://dxr.mozilla.org/mozilla-central/rev/51377a64158941f89ed73f388ae437cfa494c030/toolkit/mozapps/extensions/internal/XPIProvider.jsm#704-705

AddonManager.SIGNEDSTATE_SYSTEM is the state that means that the "Mozilla Components" OU was used to sign this XPI.

We need bug 1281571 to be resolved and for it to work on any branch that we land the above code on.

Comment 9

[Robert Helmer [:rhelmer]]

I don't think this is a great approach, since we don't want to leave these as standalone XPIs in the app dir.

We might as well do bug 1348981 now (which moves validation and location of add-ons into the omni jar) followed by bug 1357205 (which moves the add-ons themselves into the omni jar).

Once either of the above bugs are finished, the problem will be whether or not to sign omni jar and (related) what to do if the app starts up with an unsigned omni jar.

Comment 10

[Robert Helmer [:rhelmer]]

FWIW I think what I said in comment 6 is not valid, since we can treat the app and profile install locations differently (and the profile install location contains signed XPIs only)