Allow to relax the signature requirements

RESOLVED FIXED in Firefox -esr60

Status

()

defect
P2
normal
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

unspecified
mozilla63
Points:
---

Firefox Tracking Flags

(firefox-esr60 fixed, firefox62 fixed, firefox63 fixed)

Details

Attachments

(2 attachments)

Assignee

Description

Last year
Downstream, in Linux distros, Firefox is installed as a system application, in a directory that is not writable by users, and that is handled by the distro package manager. The package manager can be used by users to install addons such as language packs.

Signing the xpis in those packages is a hard problem, and from a downstream perspective, doesn't seem required. If malware is able to install an unsigned addon that can be picked up by firefox, the system has bigger problems than an unsigned addon.

There is some precedent in treating the package manager-handled directories in a special way, in the form of the extensions.autoDisableScopes preference, which allows to override the auto-disabling of "side loaded" addons in specific addon scopes.

I propose that we add a similar preference allowing to specify what scopes addons need to be signed in.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

Last year
Assignee: nobody → mh+mozilla
Assignee

Comment 3

Last year
Something I'm not entirely sure about is whether to hide the warning about unsigned addons or not for those.
Comment hidden (mozreview-request)
Attachment #8981080 - Flags: review?(rhelmer) → review?(aswan)
(In reply to Mike Hommey [:glandium] from comment #0)
> Downstream, in Linux distros, Firefox is installed as a system application,
> in a directory that is not writable by users, and that is handled by the
> distro package manager. The package manager can be used by users to install
> addons such as language packs.

Does this have applications other than language packs?  We're now signing all the language packs produced in automation so ideally Linux distros could just use those.

If that's not sufficient, product will need to approve any weakening of the signing policy.  A more complete explanation of when/how this would be used would be helpful.
Flags: needinfo?(mh+mozilla)
Assignee

Comment 6

Last year
Speaking for Debian, downloading and shipping signed langpacks is not a solution. Packages need to be built from source. We build the langpacks from source tarballs generated from the l10n-central repository. The best that could be done is to extract the signatures from the signed langpacks and regenerate the langpacks. Which means the xpis have to have the exact same content, which is probably not too much of a problem, since the source should be identical... except manifest.json also needs to have the exact same buildid... which doesn't sound great. Also, this would be very fragile and likely to generate invalid xpis.
Flags: needinfo?(mh+mozilla)
Assignee

Comment 7

Last year
It'd also be desirable for system-wide webextensions packages.
If this was just about langpacks, those have a separate pref for whether signing is required and I would have suggested letting distributions create builds that honor that preference.  But given comment 7 it sounds like the request is broader than just langpacks.

So, lets start with an opinion from security here, Dan?

(In reply to Mike Hommey [:glandium] from comment #7)
> It'd also be desirable for system-wide webextensions packages.

How are these handled today?  Or is this something new the distributions want to start doing?  Mostly for my own curiosity, what sort of system-wide webextensions would be installed?
Flags: needinfo?(dveditz)
Assignee

Comment 9

Last year
Debian stable is still on 52esr at the moment, so Debian stable still has mostly old xul extensions. In Debian testing/unstable, things are likely moving towards webextensions. See for example:

https://qa.debian.org/developer.php?login=pkg-mozext-maintainers%40lists.alioth.debian.org

Some of those are probably webextensions already (eg. I suspect tree-style-tab is). There may be others not maintained by the "Debian Mozilla Extension Maintainers" team.

> If this was just about langpacks, those have a separate pref for whether signing is required

AIUI, that pref would drop signing requirements for *all* langpacks. The change I'm proposing still makes user-installed langpacks (from amo, or dropped in directly in ~/.mozilla/...) require a signature.
(In reply to Andrew Swan [:aswan] from comment #8)
> So, lets start with an opinion from security here, Dan?

The biggest danger is users downloading stuff from random websites -- those need to be signed. For a distro-built Firefox packages installed via the distro's package manager (which should be using signatures itself) and then installed into a privileged (not user-writable) directory isn't less safe than installing Firefox in the first place. It's also equivalent to how Mozilla treats system-addons: the ones bundled with Firefox aren't signed (but updates put in the user's profile must be).

This sounds like what glandium wants in comment 0. I'm not sure that's what this patch does, though. We'd want something like a non-user-overridable (i.e. default-only) pref to define magic directories -- this one looks like the user could override. Permission check the directory/files when we load them? Just the install directory and sub-directories?

We'd also want it to be a build-time config switch (like the signing requirement, maybe protected by the same switch?) so that people can't enable this in the Firefox builds Mozilla ships.
Flags: needinfo?(dveditz)
Assignee

Comment 11

Last year
Well, users could override because there's no change in the corresponding prefs files, but it could be added as a locked pref in greprefs.js.
Comment hidden (mozreview-request)
Assignee

Updated

Last year
Attachment #8981080 - Flags: review?(rhelmer) → review?(aswan)
Assignee

Comment 13

Last year
(In reply to Mike Hommey [:glandium] from comment #11)
> Well, users could override because there's no change in the corresponding
> prefs files, but it could be added as a locked pref in greprefs.js.

The updated patch does that.
Priority: -- → P2
(In reply to Mike Hommey [:glandium] from comment #13)
> (In reply to Mike Hommey [:glandium] from comment #11)
> > Well, users could override because there's no change in the corresponding
> > prefs files, but it could be added as a locked pref in greprefs.js.
> 
> The updated patch does that.

I'm not seeing this in the patch, just a regular pref that any user could use to disable signing.
Using preferences at all for this seems like overkill, how about just a build-time flag for whether signing is required in system scopes, defaulting to true, but can be overridden by Linux distros?
Flags: needinfo?(mh+mozilla)
Assignee

Comment 15

Last year
You're missing the part where it's locked. Users can't change it.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #15)
> You're missing the part where it's locked. Users can't change it.

Ah yes, the original comment mentioned greprefs.js.
Anyway, why would we use a locked pref instead of a build-time flag for this.
Assignee

Comment 17

Last year
--disable-signing-in-system-scopes?
Canonical needs this fix so that they can roll out 61 to their users.

Do we have any thoughts on what to do next? Or can they take the fix as is?
Comment hidden (mozreview-request)
Sorry i didn't notice this had been updated, but I'm about to be offline for a week, redirecting...
Attachment #8981080 - Flags: review?(aswan) → review?(kmaglione+bmo)
Assignee

Updated

11 months ago
Attachment #8981080 - Flags: review?(kmaglione+bmo) → review?(aswan)
Assignee

Comment 21

11 months ago
Hasn't moved in a week, so moving back to :aswan, who should be back, now.

Comment 22

11 months ago
mozreview-review
Comment on attachment 8981080 [details]
Bug 1464766 - Allow to relax the addon signature requirements.

https://reviewboard.mozilla.org/r/247192/#review267340

sorry for the delay...
Attachment #8981080 - Flags: review?(aswan) → review+

Comment 23

11 months ago
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/e5cfaecd8a95
Allow to relax the addon signature requirements. r=aswan

Comment 24

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e5cfaecd8a95
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Assignee

Comment 25

11 months ago
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: See comment 0
User impact if declined: See comment 0
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): For mozilla.org builds, this is effectively a no-op.
String or UUID changes made by this patch: N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8996476 - Flags: approval-mozilla-esr60?
Assignee

Comment 26

11 months ago
Comment on attachment 8981080 [details]
Bug 1464766 - Allow to relax the addon signature requirements.

Approval Request Comment
[Feature/Bug causing the regression]: See comment 0
[User impact if declined]: See comment 0
[Is this code covered by automated tests?]: N/A
[Has the fix been verified in Nightly?]: It's NPOTB, but I'm using this patch downstream (Debian)
[Needs manual test from QE? If yes, steps to reproduce]: N/A
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's effectively a no-op for mozilla.org builds. Downstreams can opt-in to disable signature requirement in very specific locations.
[String changes made/needed]: N/A
Attachment #8981080 - Flags: approval-mozilla-beta?
Comment on attachment 8981080 [details]
Bug 1464766 - Allow to relax the addon signature requirements.

Sounds useful for distros and shouldn't be risky for our builds, let's uplift for beta 15.
Attachment #8981080 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment on attachment 8996476 [details] [diff] [review]
Backport for ESR60

Makes life easier for downstream Linux distros and doesn't affect Mozilla-built Firefox binaries. Approved for ESR 60.2.
Attachment #8996476 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+

Comment 31

11 months ago
uplift
In a move I can only attribute to post-PTO brain, I landed the wrong patch on ESR60 (and hilarity ensued). Backed out:
https://hg.mozilla.org/releases/mozilla-esr60/rev/a98818a640cc

And re-landed with the right patch this time:
https://hg.mozilla.org/releases/mozilla-esr60/rev/5ae88d739d8c
You need to log in before you can comment on or make changes to this bug.