Closed Bug 1388143 Opened 8 years ago Closed 8 years ago

Language packs can be used to bypass extension restrictions

Categories

(Toolkit :: Add-ons Manager, defect, P1)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
firefox-esr52 - wontfix
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: kmag, Unassigned)

References

Details

(Keywords: csectype-priv-escalation, sec-high)

Attachments

(1 file, 1 obsolete file)

Proof of concept
1.29 KB, application/x-xpinstall
Details
Attached file Proof of concept —
Language packs don't enforce signing restrictions, or really any other restrictions, so a langpack XPI can be used to run arbitrary chrome code in lieu of a legacy extension. See attached proof of concept.
Is there anything we can do about this in the 57 timeframe? There is work going on to move over to langpacks based on webext (bug 1365031) but I don't think this (and all our related infra) will be ready in time.
We can probably just turn on signing enforcement for them, at this point. It looks like all of the langpacks hosted on AMO are signed now.
For instance, is there a reason langpacks need to overlay anything other than DTDs?
Signing sounds like less work (for something that's going away soon), sgtm.
If we can agree on how we're going to handle the chrome entries (:mossop and :kmag disagree on this, and :kmag submitted a patch for :mossop to review in bug 1384689 - my understanding is that if :mossop r+, we go with :kmag suggestion, if not, we continue the conversation), then I believe we can have all pieces landed in 57. The generation patch is just awaiting the outcome of the :mossop vs. :kmag discussion (bug 1365440) and consumption patch (bug 1365709) is also mostly ready with :aswan committed to get it landed as soon as the decision is made. That means that we could quite soon have the new model operational in Gecko. I'm not sure if it's enough to move all the machinery, but it may be enough to move it before November or at least ASAP.
That's good to hear. I'm not too concerned about 57, since signing and legacy add-on restrictions can be disabled there, anyway. It might be a good idea to turn on langpack signing enforcement in release/beta/ESR, though, in case someone else notices this is possible in the mean time.
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5) > If we can agree on how we're going to handle the chrome entries (:mossop and > :kmag disagree on this, and :kmag submitted a patch for :mossop to review in > bug 1384689 - my understanding is that if :mossop r+, we go with :kmag > suggestion, if not, we continue the conversation), then I believe we can > have all pieces landed in 57. Note that with the patch in bug 1384689 I think new-style language packs will still have a similar vulnerability
(In reply to Dave Townsend [:mossop] from comment #7) > Note that with the patch in bug 1384689 I think new-style language packs > will still have a similar vulnerability From the approach we talked about on IRC, the language packs wouldn't be able to add arbitrary entries. We'd add static override entries whenever a langpack is installed, and then generate the locale declarations based on resource mappings in the manifest.json. So the only way they'd be able to do something similar is if we're executing code from locale resources.
Correct - I'm also trying to get override entries out of langpacks - bug 1377911 - so we could eventually limit it down to just locale entries.
This is just one of the many ways in which language packs can execute random code. See bug 868939 for an example. As for signing, I'm surprised to hear that our langpacks are signed. CCing Andreas to add information here. We do have langpacks that we build ourselves for beta, which aren't on AMO at all, and I'm not sure how good community-contributed langpacks get checks for ingestions like script tags or chrome registry mongling.
Right after every (stable) release, we semi-automatically pull langpacks from ftp and sign them automatically. Further langpacks can be sumitted to AMO by anyone like any other add-on and are reviewed according to our policies. For example, they shouldn't contain executable code. Those are signed after human approval.
(In reply to Andreas Wagner [:TheOne] [use NI] from comment #11) > For example, they shouldn't contain executable code. Maybe they shouldn't, but they can (see bug 868939 for example, or this bug for that matter). They should be signed and signing enforced.
Keywords: csectype-priv-escalation, sec-high
(In reply to Daniel Veditz [:dveditz] from comment #12) > (In reply to Andreas Wagner [:TheOne] [use NI] from comment #11) > > For example, they shouldn't contain executable code. > > Maybe they shouldn't, but they can (see bug 868939 for example, or this bug > for that matter). They should be signed and signing enforced. I can't access that bug. Either way my point in saying "they shouldn't" was that reviewers should catch that during review. I agree that we should enforce signing, no argument there :)
See Also: → 1197876
Priority: -- → P2
I wonder how far we're off from switching langpacks formats? If it's close, we could mitigate this by looking a bit more sternly at langpacks when pulling them (i.e., using grep) for a few times and then be done with it?
My current plan is to make the switch on October 1st in the 58 cycle.
(In reply to Frederik Braun [:freddyb] from comment #14) > If it's close, we could mitigate this by looking a bit more sternly at > langpacks when pulling them (i.e., using grep) for a few times and then be > done with it? Agreed. I mainly filed this as an example of what we need to avoid with the new langpack format, and to inform the migration timeline. Removing support for old-style langpacks should be the fix.
This is still a problem for ESR 52 until mid-2018 -- at least another 8 months. Can we turn this on now?
Flags: needinfo?(kmaglione+bmo)
Flags: needinfo?(gandalf)
We're requiring sec-high/critical bugs be owned now, and for this change kmag seems the likely choice. If you're not able to take it on at this time please assign it to your manager.
Assignee: nobody → kmaglione+bmo
> Can we turn this on now? I'm not sure what are you asking about. We enabled the new langpacks in 58 and we switched our build system to build new langpacks in 58. The AMO seems to be ready so we expect to move away from old langpacks as 58 comes to beta and stable.
Flags: needinfo?(gandalf)
Assignee: kmaglione+bmo → nobody
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #19) > I'm not sure what are you asking about. We enabled the new langpacks in 58 > and we switched our build system to build new langpacks in 58. The AMO seems > to be ready so we expect to move away from old langpacks as 58 comes to beta > and stable. That doesn't at all help ESR 52 users be safe from this. Can we require signed language packs for them?
Flags: needinfo?(gandalf)
Hi Chris, please assign this bug to an appropriate owner. thanks!
Assignee: nobody → ckarlof
We still need to turn off support for legacy langpacks when the time comes. Andrew is probably the best person for this, at this point. Dan, are you suggesting we should try to do something about this in ESR52? That would require a completely different approach than we're aiming for in 58/59.
Assignee: ckarlof → aswan
Flags: needinfo?(kmaglione+bmo)
Priority: P2 → P1
Bah, mid-air.
Flags: needinfo?(dveditz)
Signing of language packs is bug 1197876 and questions about Linux distro's (comment 16) haven't been resolved. However it also hasn't been pursued because it hasn't been on peoples radar. My guess is there's quite a bit of work here in getting a patch for ESR to support this and then changes to AMO and any other distribution mechanisms.
(In reply to Andy McKay [:andym] from comment #24) > Signing of language packs is bug 1197876 and questions about Linux distro's > (comment 16) haven't been resolved. However it also hasn't been pursued > because it hasn't been on peoples radar. My guess is there's quite a bit of > work here in getting a patch for ESR to support this and then changes to AMO > and any other distribution mechanisms. I'd be fine with only requiring langpack signing on Windows/OS-X for ESR. Linux really isn't a huge target for this kind of malware. As for the difficulty, it should be a 1-line change, adding langpacks to the SIGNED_TYPES array, and maybe updating some tests.
Are the langpacks that are compatible with ESR 52 actually signed right now?
(In reply to Axel Hecht [:Pike] from comment #26) > Are the langpacks that are compatible with ESR 52 actually signed right now? The ones shipped with 52 release should be signed on AMO. Updates shipped with ESR point releases should not be, but the 52 release versions *should* be compatible.
> That doesn't at all help ESR 52 users be safe from this. Can we require signed language packs for them? Oh, now I understand the question. Unfortunately, I don't know the consequences of signing old langpacks in 52. I only wrote the code for the new langpacks. If we're comfortable claiming that 52 will not receive string changes, maybe we can release 52 langpacks as 52.*, sign them and be done?
Flags: needinfo?(gandalf)
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #22) > Dan, are you suggesting we should try to do something about this in ESR52? > That would require a completely different approach than we're aiming for in 58/59. Yes, I was suggesting adding legacy langpacks to the list of types that must be signed. I'm not sure why they weren't already included -- we've always known langpacks were dangerous. (In reply to Andy McKay [:andym] from comment #24) > questions about Linux distro's [bug 1197876 comment 16] haven't been resolved. That comment wasn't very enlightening about the problem. Any other links? Presumably this is distros customizing their versions and therefore needed modified language packs? If so couldn't they send their variant language packs through our auto-signer? Or worst case we add a pref that allows unsigned language packs (even in release builds).
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #29) > (In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're > blocked) from comment #22) > > Dan, are you suggesting we should try to do something about this in ESR52? > > That would require a completely different approach than we're aiming for in 58/59. > > Yes, I was suggesting adding legacy langpacks to the list of types that must > be signed. I'm not sure why they weren't already included -- we've always > known langpacks were dangerous. We didn't add the signing requirement at least in part so that we could have other systems generate language packs. For example, pontoon is, and pootle was, creating language packs for languages which we don't support on hg.m.o yet. Also, the signing requirements are only useful if they're actually paired with something that denies a signature to malicious langpacks, and there are no descent rules for that. Or manpower knowing what to look for. > (In reply to Andy McKay [:andym] from comment #24) > > questions about Linux distro's [bug 1197876 comment 16] haven't been resolved. > > That comment wasn't very enlightening about the problem. Any other links? > Presumably this is distros customizing their versions and therefore needed > modified language packs? If so couldn't they send their variant language > packs through our auto-signer? Or worst case we add a pref that allows > unsigned language packs (even in release builds). Linux distributions ship a multi-locale build by putting all language packs as part of the generated bundle. One example is the snap distribution bundle that we generate for beta builds, http://searchfox.org/mozilla-central/source/taskcluster/docker/firefox-snap/runme.sh. No idea how to integrate such a task with add-on signing.
(In reply to Axel Hecht [:Pike] from comment #30) > We didn't add the signing requirement at least in part so that we could have > other systems generate language packs. For example, pontoon is, and pootle > was, creating language packs for languages which we don't support on hg.m.o > yet. > > Also, the signing requirements are only useful if they're actually paired > with something that denies a signature to malicious langpacks, and there are > no descent rules for that. Or manpower knowing what to look for. Did you see comment 11? I don't know how effective it is but there is a system in place. Andreas, do you have (or can you easily find out) how many manually reviewed language packs actually exist? > > (In reply to Andy McKay [:andym] from comment #24) > > > questions about Linux distro's [bug 1197876 comment 16] haven't been resolved. > > > > That comment wasn't very enlightening about the problem. Any other links? > > Presumably this is distros customizing their versions and therefore needed > > modified language packs? If so couldn't they send their variant language > > packs through our auto-signer? Or worst case we add a pref that allows > > unsigned language packs (even in release builds). > > Linux distributions ship a multi-locale build by putting all language packs > as part of the generated bundle. One example is the snap distribution bundle > that we generate for beta builds, > http://searchfox.org/mozilla-central/source/taskcluster/docker/firefox-snap/ > runme.sh. > > No idea how to integrate such a task with add-on signing. I don't know much about how Linux distributions of Firefox work, isn't it the case that we give them permission to do their own builds with the official branding? And from this comment, it sounds like they also just build and bundle language packs themselves? Its not great but what if we added a build-time flag to disable language pack signing that Linux distributions could use but we could still require signing in the builds we distribute.
Note that those distros almost certainly currently use the built-in langpack extension IDs, which they won't have access to sign. They'd need to change the build process to generate distro-specific IDs if they wanted to use automated signing.
The snap builds are actually created by moco-owned automation, like https://ftp.mozilla.org/pub/firefox/candidates/57.0b9-candidates/build1/snap/, and they're part of the release automation task graph, https://tools.taskcluster.net/groups/KI7EEE3BQbGJ9au4M2rHFQ/tasks/AXrcvvE5RLmB_pWrWzFJ_w/details. I'm not sure if we have bizdev commitments to generate those builds. Commitments, that, if we made them, probably had the security and the timeliness of the resulting build in mind, just to make sure we're not dismissing the value of us doing them ad-hoc.
We have two different prongs here, one is disabling old-style language packs in m-c, I can file a separate public bug for that to avoid drawing unnecessary attention to it since we can't uplift that change. The other prong is about ESR on Linux. Two concrete suggestions so far have been: 1. require signing on windows and mac but not linux 2. provide a build-time flag to let linux distributions build without enforced signing Re comment 33, I'm not sure what snap builds have to do with this. First, I don't see snap builds in a small sampling of ESR directories. But even if we have ESR snap builds, I assume those could use our existing signed langpacks? Dan, is option #1 above sufficient?
Flags: needinfo?(dveditz)
I prefer #2, but if we can get #1 done then "worse is better" works for me. (In reply to Axel Hecht [:Pike] from comment #30) > Also, the signing requirements are only useful if they're actually paired > with something that denies a signature to malicious langpacks, and there are > no descent rules for that. Or manpower knowing what to look for. Yet we're already shipping them. Signing can't make that worse.
Flags: needinfo?(dveditz)
Bug 1413322 has landed, disabling old language packs on Nightly, it will ride the trains for 58. I haven't linked to it here to avoid drawing attention to it. Also, note that we have bug 1244131 which is basically a public version of this bug. Again, I think we should just leave it alone for now to avoid attracting attention to this until we've got fixes deployed. The only real loose end is signing for ESR, I'll work on that this week.
I'm wondering, do we actually need to sign the language packs that we ship as part of multi-locale binaries? My suspicion is that none of the xpis in browser/features are signed? That is, which install locations are exempt of signing requirements, under which conditions?
(In reply to Axel Hecht [:Pike] from comment #37) > I'm wondering, do we actually need to sign the language packs that we ship > as part of multi-locale binaries? My suspicion is that none of the xpis in > browser/features are signed? > > That is, which install locations are exempt of signing requirements, under > which conditions? xpis in browser/features (system add-ons) are not signed but there are a bunch of other limitations on what we will run there (among other things, their IDs have to match a built-in list). There is another directory in the profile where we keep updates to system-addons, those have to be signed differently from regular addons. Everything else, including the regular profile location and system-wide install locations follow the regular signing rules (ie, required unless using Nightly or dev edition with the xpinstall.signatures.required preference set to false)
MozReview-Commit-ID: LbP2rPEBlr7
Comment on attachment 8925720 [details] [diff] [review] Add built-time option for requiring signing of language packs [Security approval request comment] How easily could an exploit be constructed based on the patch? Not very easily Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Not directly. They suggest an issue with language packs, an attacker wouldn't have to be especially skilled to go find bug 1244131 which is public and details the vulnerability. Which older supported branches are affected by this flaw? If not all supported branches, which bug introduced the flaw? Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This patch is ESR-only. Other comments on this bug describe the situation on other branches. How likely is this patch to cause regressions; how much testing does it need? It is unlikely to cause regressions but it should receive some manual testing.
Attachment #8925720 - Flags: sec-approval?
Attachment #8925720 - Flags: review?(kmaglione+bmo)
Comment on attachment 8925720 [details] [diff] [review] Add built-time option for requiring signing of language packs Review of attachment 8925720 [details] [diff] [review]: ----------------------------------------------------------------- r=me for the AOM parts. The build config parts need review from a build peer. ::: toolkit/moz.configure @@ +516,5 @@ > > +option(name='--disable-langpack-signing', > + help='Do not require language packs to be signed') > + > +langpack_signing = depends_if('--disable-langpack-signing')(lambda _: True) This seems backwards. It should wind up setting MOZ_REQUIRE_LANGPACK_SIGNING when --disable-langpack-signing is passed, rather than when it isn't. @@ +519,5 @@ > + > +langpack_signing = depends_if('--disable-langpack-signing')(lambda _: True) > + > +set_config('MOZ_REQUIRE_LANGPACK_SIGNING', langpack_signing) > +set_define('MOZ_REQUIRE_LANGPACK_SIGNING', langpack_signing) This needs review from a build peer.
Attachment #8925720 - Flags: review?(kmaglione+bmo) → review+
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #41) > ::: toolkit/moz.configure > @@ +516,5 @@ > > > > +option(name='--disable-langpack-signing', > > + help='Do not require language packs to be signed') > > + > > +langpack_signing = depends_if('--disable-langpack-signing')(lambda _: True) > > This seems backwards. It should wind up setting MOZ_REQUIRE_LANGPACK_SIGNING > when --disable-langpack-signing is passed, rather than when it isn't. It does look backward, but I ripped it off from: https://dxr.mozilla.org/mozilla-esr52/rev/20a294c099c81a5c1f742a62170d1bf046639fd9/toolkit/moz.configure#519-525 and after running "mach configure" with and without --disable-langpack-signing in mozconfig, the contents of objdir/config/autoconf.mk have what I expect.
Attachment #8925720 - Flags: review?(mh+mozilla)
Comment on attachment 8925720 [details] [diff] [review] Add built-time option for requiring signing of language packs Review of attachment 8925720 [details] [diff] [review]: ----------------------------------------------------------------- It seems to me this is the wrong knob, especially for linux distros. Langpacks are only one part of the problem. Distros also ship addons in system, read-only locations, and they can't nor should be required to sign them.
Attachment #8925720 - Flags: review?(mh+mozilla) → review-
Also, globally disabling signing for langpacks means that users of those builds could end up installing unsigned langpacks that don't come from their distro. So all in all, it seems to me the signature requirement should be gated on where the addons lives, and I /think/ we already do that.
Allowing unsigned langpacks based on their location would give side-load installers an easy way to inject chrome-privileged code into the browser. Is there a particular reason Linux distros shouldn't be required to sign their langpacks? We're moving toward requiring all system add-ons to either be signed or bundled in omni.ja. That should apply to langpacks as well as other add-on types.
On linux, side-loading on system locations is a non problem. As in, system locations are normally not writable by normal users, and if something manages to inject chrome-privileged code into the browser by writing to system locations, then the user has a much bigger problem than their browser being infected... As for requiring signing for linux distros, how do you suggest that integrates with the CI they use that builds their packages? Short answer: it's simply not possible.
(In reply to Mike Hommey [:glandium] from comment #44) > and I /think/ we already do that. cf. bug 1255590. The code added in that bug was moved in bug 1294483 and is now in toolkit/mozapps/extensions/internal/XPIProvider.jsm. I'd argue KEY_APP_SYSTEM_ADDONS should be treated the same as KEY_APP_SYSTEM_SHARE/KEY_APP_SYSTEM_LOCAL on linux.
(In reply to Mike Hommey [:glandium] from comment #46) > On linux, side-loading on system locations is a non problem. As in, system > locations are normally not writable by normal users, and if something > manages to inject chrome-privileged code into the browser by writing to > system locations, then the user has a much bigger problem than their browser > being infected... I agree that this is largely a non-problem on Linux, but more because side-load installers don't heavily target Linux users than because those locations are non-writable by default. They're non-writable by default on Windows and OS-X, too, but side-loads there are still a major problem. > As for requiring signing for linux distros, how do you suggest that > integrates with the CI they use that builds their packages? Short answer: > it's simply not possible. See the discussion above. We have a signing API that can be integrated CI builds. For distributions that don't want to use it, and for non-official builds, they can disable the signing requirement (though possibly not if they want to use official branding).
Speaking for Debian, the CI doesn't have access to the internet. Plus, that kills reproducibility.
Also see comment 44 about globally disabling the signing requirement. It's a bad idea.
(In reply to Mike Hommey [:glandium] from comment #49) > Speaking for Debian, the CI doesn't have access to the internet. Plus, that > kills reproducibility. Well, there are three options here: 1) Require signing for langpacks on Linux, without a configuration option. 2) Don't require signing for langpacks on Linux, without a configuration option. 3) Make signing of langpacks configurable. Requiring singing based on the add-on location isn't really an option. If we did it, we would wind up doing it only on Linux, since add-on side-loading is a major problem on other platforms. And adding complexity to the add-on manager code solely for the sake of a few Linux distros isn't really an option at this point, when we're trying to remove as much of the accumulated complexity of the past decade from the add-on manager as possible. So if you don't like option #3, would you prefer option #1 or #2?
Did you miss the fact that we're *already* disabling signing for system locations "solely for the sake of a few Linux distros"?
(In reply to Mike Hommey [:glandium] from comment #52) > Did you miss the fact that we're *already* disabling signing for system > locations "solely for the sake of a few Linux distros"? If you want to depend on that situation continuing, then it sounds like you're choosing option #1? However, please note that I'm not promising that that situation will continue.
Please don't lose sight of the fact that this is a stopgap specifically for Linux ESR users. We have a new langpack format that does not have the problem described in this bug, it landed in 58 so it will be in ESR59 and this won't be an issue then. If Linux distributions that build their own langpacks put them in a global location that is exempt from signing requirements, I'm happy to replace this patch with a much simpler one that just treats langpacks like extensions with regard to the signature requirement.
(In reply to Andrew Swan [:aswan] from comment #54) > Please don't lose sight of the fact that this is a stopgap specifically for > Linux ESR users. We have a new langpack format that does not have the > problem described in this bug, it landed in 58 so it will be in ESR59 and > this won't be an issue then. Again, please don't depend on this remaining the case. New-style langpacks are safer than old-style langpacks, but not completely safe. They'll continue being signed when we distribute them, and signatures will likely become mandatory in the near future.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #55) > Again, please don't depend on this remaining the case. New-style langpacks > are safer than old-style langpacks, but not completely safe. They'll > continue being signed when we distribute them, and signatures will likely > become mandatory in the near future. Yep, that's bug 1197876. I think we can consider it separately from this one.
sec-approval+ for checkin on 11/28 or later, two weeks into the next cycle.
status-firefox57: --- → ?
status-firefox58: --- → ?
status-firefox-esr52: --- → affected
tracking-firefox-esr52: --- → ?
Whiteboard: [checkin on 11/28]
Comment on attachment 8925720 [details] [diff] [review] Add built-time option for requiring signing of language packs You'll need to specifically nominate the patch for ESR52 approval though...
Attachment #8925720 - Flags: sec-approval? → sec-approval+
Now I'm really confused, back in comment 17 the request was to turn this on for ESR and the attached patch is ESR-only. If we land this on 11/28 then it won't go out until ESR 52.6 (and ESR 59 ships 6 weeks after that). In addition, there's the whole Linux situation, in light of all yesterday's comments I think the simpler patch to just require signing for all langpacks and let Linux distributions use the global directory makes more sense than the existing patch. Its unclear to me if we can make that change in central right now as well. If we can, then I suggest we just quietly do that over on bug 1197876 and uplift that to esr52. Then we can just keep this bug hidden and it can cover all the remaining exploitable issues with new-style langpacks. :gandalf, is AMO signing new langpacks now and if so, is there any other reason not to turn on required signing for langpacks in central?
> :gandalf, is AMO signing new langpacks now and if so, is there any other reason not to turn on required signing for langpacks in central? I don't know what AMO does. Pike?
Flags: needinfo?(l10n)
If I go to https://addons.mozilla.org/en-US/firefox/language-tools/ and download some of the language packs, they are signed.
What we need to do for m-c is that the language packs we generate in automation are signed to start. That is, the taskgraph of mozilla-central nightlies doesn't publish anything unsigned to ftp.m.o. I don't see a way to integrate the signing process on the one hand, and the discovery and chain-of-trust on the other hand, for the multi-locale builds that we generate.
Flags: needinfo?(l10n)
(In reply to Axel Hecht [:Pike] from comment #64) > What we need to do for m-c is that the language packs we generate in > automation are signed to start. That is, the taskgraph of mozilla-central > nightlies doesn't publish anything unsigned to ftp.m.o. That seems like a good goal but is it something we have to do before turning on signing? > I don't see a way to integrate the signing process on the one hand, and the > discovery and chain-of-trust on the other hand, for the multi-locale builds > that we generate. I don't understand what this means, can you spell it out for somebody not familiar with all the ins and outs of builds and locales?
Flags: needinfo?(l10n)
(In reply to Andrew Swan [:aswan] from comment #65) > (In reply to Axel Hecht [:Pike] from comment #64) > > What we need to do for m-c is that the language packs we generate in > > automation are signed to start. That is, the taskgraph of mozilla-central > > nightlies doesn't publish anything unsigned to ftp.m.o. > > That seems like a good goal but is it something we have to do before turning > on signing? Yes, putting things on ftp that are broken is bad. > > I don't see a way to integrate the signing process on the one hand, and the > > discovery and chain-of-trust on the other hand, for the multi-locale builds > > that we generate. > > I don't understand what this means, can you spell it out for somebody not > familiar with all the ins and outs of builds and locales? I can't, but Callek should be. Callek, I think this is easiest to reason about in the light of the snap build we're doing in release automation.
Flags: needinfo?(l10n) → needinfo?(bugspam.Callek)
Flags: needinfo?(bugspam.Callek)
Flags: needinfo?(bugspam.Callek)
Ok, let me shed some light on what is current state of things here, things are slightly different between Nightly and ESR. [forgive me if this is not as coherent as it could be, juggling mental state on some other things too] ==On ESR52==: L10n repack jobs, take the already signed en-US stuff, produce localized copies that are signed (.exe, .zip, .dmg, etc) but unsigned .xpi langpacks. This same job uploads all those artifacts (including .xpi langpacks) to ftp/archive.mozilla.org. We sign from iside the job via in-tree makefile-fu with calls out to relengs signing server infrastructure. That infrastructure has signing keys for all that we can sign, with code to actually do the signing embedded. .xpi's are not one of those things. The signing pieces of the job are roughly synchroneous, basically meaning that we upload-->wait-->get new signed file back. Too much of a delay can cause the whole set of nightly repacks to fail. ==On Nightly== L10n repack jobs, take the already signed en-US stuff, produce localized copies that are unsigned, said copies are only uploaded directly to the specific repack task. Then there is a seperate signing task that signs "internals" [win, osx] which consist of the .exe's inside the .zip and the Binaries inside the actual OSX disk image. Outputting compressed archives of those things. (we don't sign .xpi's) - This signing task uses the same backend infrastructure as the ESR52 signing servers, but with a slightly different frontend proxy format (different security check style). We then take those artifacts and "repackage" them into a installer.exe (for windows) or a disk image [.dmg] (for osx). These artifacts are placed on the taskcluster task itself. then, for windows (not osx) we take those repackaged installers and sign them using that very same infrastructure as the first step here on nightly. After all that we have a seperate task that gathers those artifacts from all those prior steps and uploads the final versions of them to archive.mozilla.org. This includes the unsigned language packs. ==Snap Multilocale==: We take the en-US linux binary, and then pull in the langpacks from archive.m.o directly. and bundle it together. ========= On relengs side there is little flexibility in how we can do this on ESR52, we can try and devote some resources to modifying the in tree build system, and our own signing infrastructure to sign the .xpi using <some key>. But I don't believe we have releng resources work on this inside of Q4. [I've CC'd catlee to this bug, as he would help determine human resource allotment] As far as Nightly/ESR59/etc it would be a different approach but would need some similar work in the signing systems, and would need some "taskgraph" work to support us signing the langpack xpi's and plumbing them appropriately to upload these signed copies to archive.m.o. An ideal solution to me would be that we (mozilla) generate the langpacks unsigned, upload them to AMO directly for signing using some API, and then fetch them for upload to archive.m.o, but that is a process I don't envision being able to achieve for ESR52 and may not be as easy as a "releng signs the xpi" solution on Nightly. ======== If that is unclear, feel free to rope me into a meeting and we can discuss. :Aki is our signing server expert, :catlee would be the human for "can releng adjust other priorities to devote resources"
Flags: needinfo?(bugspam.Callek)
Welp, we've got security asking for signing to be enforced (comment 17) and l10n asking to not enforce signing until signing can be integrated into the release process (which doesn't sound like a short-term project). I understand the argument in the abstract for why we shouldn't have unsigned langpacks on ftp, but it doesn't seem so terrible to redirect people who want to download them to get them from AMO instead. Anyway, I'm not sure how to get out of this impasse. Dan?
Flags: needinfo?(dveditz)
(In reply to Andrew Swan [:aswan] from comment #61) > Now I'm really confused, back in comment 17 the request was to turn this on > for ESR and the attached patch is ESR-only. If we land this on 11/28 then > it won't go out until ESR 52.6 (and ESR 59 ships 6 weeks after that). Yes. We're shipping ESR52.5 in the next five days and have the release candidate made. We can't ship this in ESR52.5 without a rebuild and wouldn't take it unless it is a potential chemspill. It missed the ESR52.5 window.
(In reply to Justin Wood (:Callek) from comment #67) > That infrastructure has signing keys for all that we can sign, with code > to actually do the signing embedded. .xpi's are not one of those things. When do the included system addons get signed? Could we do whatever that is with the langpacks? (In reply to Andrew Swan [:aswan] from comment #68) > I understand the argument in the abstract for why we shouldn't have unsigned > langpacks on ftp, but it doesn't seem so terrible to redirect people who > want to download them to get them from AMO instead. Same here. > Anyway, I'm not sure how to get out of this impasse. Dan? I'm not either. If we hadn't diverted all the Windows folks onto ESR I could more easily ignore this, since most of the people affected would then be linux folks and the linux distros don't seem to want this fixed.
Flags: needinfo?(dveditz)
(In reply to Daniel Veditz [:dveditz] from comment #70) > When do the included system addons get signed? Could we do whatever that is > with the langpacks? They don't. Updates get signed but ones that ship with the browser are unsigned (other things like bug 1348981 try to make it harder to trivially run your own unsigned extensions in this way)
Attachment #8925720 - Attachment is obsolete: true
I have little context but comment 68-70 make it sound like we can ignore this for now for Nightlies? Current ESR 52.x is what needs to be addressed? If so, as callek pointed out, it would be hard to do this for current esr52 cycle in Q4 given we only have a few working weeks left outside of Austin and holidays. This would be more manageable to do this for the next ESR cycle, 59.x. As we go from 52->59, we will switching over to 100% Taskcluster and, in doing so, we'll be touching and redefining a lot of our release automation. Could this wait until then? Taskcluster ESR effort will take place in Q1, 2018. Could we plan and coordinate this effort in Austin with affected teams?
By the time ESR 59 comes around, we'll be off the legacy langpack format. The original request from security (in comment 20) was to do something for ESR52. But this bug is stuck on the issues with signing in automation described above. Unassigning myself since I can't do anything useful on that front.
Assignee: aswan → nobody
Okay, so this is purely for soon to be retired esr52. Since we are at a standstill, please allow me to lean one way to see if we can resolve and move on. tl;dr - I'm hesitant to fix anything legacy for a long standing known vuln. Given that coupled with the fact that it is not clear whether to action, I propose WONTFIX. Context to that position: Could we fix this before esr 59 ships? Yes, and it's a question of what should drop if we do fix. When could this be fixed? Given we are wrapping up Q4 targets, Austin is in 2 weeks, and the 2 week holidays following that, nothing would be done before Jan. Let's say end of Jan, early Feb. What does that mean? we will have 52.6.0 out the door in early Jan, leaving 3 more releases before esr59 starts where this will no longer be an issue. Important to note that once esr59 starts, we will no longer be offering 52.x as initial installs. The how and who? If we do action this bug through our build system within release automation, we would need to coordinate with build team and extend our signing server backend. If we do this with addons team, we would need to post sign through addon signing api and upload after the release automation runs. Conclusion: I think it all boils down to benefit and cost here. Do we want to take resources from the above mentioned teams given we only have 3 more releases left on esr52? In addition, during those three releases in Q1, releng will be rewriting the whole release automation stack to support Taskcluster for esr59. I'm on the side it's likely not worth fixing given this bug has been open for 4 months and the vulnerability longer than that. But if security wants to weigh in with the risk of not doing anything for esr52, feel free to push back and we should revisit. I'm not an expert and have little context. I'm also open to creating a meeting with involved players to conclude offline if we can't decide in this bug.
To be clear, all that work to sign langpacks is work that we need to do anyway. It is precisely because of all the inter-team dependencies you mention that we've had bug 1197876 for more than 2 years and not apparently made any progress toward doing it. With the move to the webextension packaging for langpacks, the most obvious vulnerability is gone (on all branches except esr52, hence the push for turning on signing there) but there are other vulnerabilities that signing would give us some amount of protection against.
To be clear, while, yes, this is a long-standing vulnerability, it's only recently become interesting. It became slightly interesting when we introduced mandatory signing for extensions. It became *very* interesting in 57, where we removed support for legacy extensions, which this vulnerability almost entirely circumvents.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #76) > It became *very* interesting in > 57, where we removed support for legacy extensions, which this vulnerability > almost entirely circumvents. But unless I'm misunderstanding you, the particular vulnerability you are referring to is gone again in 58 (via bug 1413322)
(In reply to Andrew Swan [:aswan] (PTO through 11/27) from comment #77) > But unless I'm misunderstanding you, the particular vulnerability you are > referring to is gone again in 58 (via bug 1413322) Yes, this particular vulnerability is gone in 58, and it's only interesting in the current ESR for the sake of bypassing signature checks.
Is there an ESR52 patch ready for nomination here?
I don't see any changes happening for ESR 52 here, hopefully we could get something for ESR 59.
status-firefox-esr52: affected → wontfix
(In reply to Andy McKay [:andym] from comment #80) > I don't see any changes happening for ESR 52 here, hopefully we could get > something for ESR 59. AIUI this is fixed in 58 and higher, so there's nothing to do for ESR59.
Requiring Firefox language packs to be signed as per bug bug 1197876 would still go towards the security teams goals of signing content used in Firefox. There will still need to be changes in release management for this, however that could continue in bug 1197876.
(In reply to Kris Maglione [:kmag] (long backlog; ping on IRC if you're blocked) from comment #76) > To be clear, while, yes, this is a long-standing vulnerability, it's only > recently become interesting. It became slightly interesting when we > introduced mandatory signing for extensions. It became *very* interesting in > 57, where we removed support for legacy extensions, which this vulnerability > almost entirely circumvents. The 57.0 release enforcement doesn't matter or change anything for esr52.x though right? We should be avoiding any engineering automation effort to fix esr52. Instead we either explore either a manual fix or call this bug WONTFIX and move on to esr59 and its own requirements (comment 82, bug 1197876). By manual fix, I'm wondering if we can do something like the following: (In reply to Zibi Braniecki [:gandalf][:zibi] from comment #28) > > If we're comfortable claiming that 52 will not receive string changes, maybe > we can release 52 langpacks as 52.*, sign them and be done? I'll schedule a meeting for later this week to discuss ideas like this and whether or not we should close the bug. If this gets closed in the mean time, I'll cancel said meeting. Let me know if you would like to be there.
(In reply to Jordan Lund (:jlund) from comment #83) > We should be avoiding any engineering automation effort to fix esr52. > Instead we either explore either a manual fix or call this bug WONTFIX and > move on to esr59 and its own requirements (comment 82, bug 1197876). By > manual fix, I'm wondering if we can do something like the following: Multiple things are getting conflated here. Even if/when we manage to get signing in place, there are still inherent vulnerabilities in the langpack format, that's what this bug is really about it. Please don't close this bug just based on what happens w.r.t. signing in automation.
We'll not do anything for 52 ESR. For 59 and 59 ESR, we'll focus on getting signing enforced with the help of jlund's team. To track the signing of add-ons please follow bug 1197876. The original POC focuses on a legacy language pack. However, comment 54 refers to security problem in the new format. As per aswan's comment in bug 84, I suggest we keep this bug open to see what we can do about that issue.
Whiteboard: [checkin on 11/28]
I don't think we can fix the data injection problem by talking about the data. The problem is what the code does with the data it got. We can define safe APIs for restricted use-cases, like plain HTML documents with fluent-dom aka l20n. For react or other js stacks in the middle between our localized data and the UI, there's no real control. Short of saying "you can't use that js library", that is. I don't think there's a point in keeping this bug open.
Per comment 86.
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox57: ? → wontfix
status-firefox58: ? → fixed
tracking-firefox-esr52: ? → -
Resolution: --- → WONTFIX
(In reply to Axel Hecht [:Pike] from comment #86) > I don't think we can fix the data injection problem by talking about the > data. The problem is what the code does with the data it got. > > We can define safe APIs for restricted use-cases, like plain HTML documents > with fluent-dom aka l20n. For react or other js stacks in the middle between > our localized data and the UI, there's no real control. As of bug 1432699, this is no longer really an issue.
You meant bug 1432966? Certainly helps, as long as folks rolling their own l10n don't hack around innerHTML sanitization. I'm old, I've seen weird stuff.
Group: toolkit-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: