1197876 - [meta] Require signing for locale packs

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Locale packs are capable of injecting script into chrome documents (see bug 1193926 comment 9) and so we must require them to be signed.

Comment 1

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked for FF41 onward.

Comment 2

[leichixian]

What about Nightly language packs? Locale teams usually do with aurora(sometimes not in time) and for nightly I've seen many people translating by themselves(like me). Have you ever thought about them? Or maybe you should force l10n teams do with nightly but not aurora.

Comment 3

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Dave, do you think we will fix this in FF41 time frame? If not, I would like to un-track for 41 and leave tracking for 42, 43.

Comment 4

[Dave Townsend [:mossop]]

Apparently we're not doing this for older language packs so we wouldn't be able to uplift this anyway

Comment 5

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Dave, Thanks for the confirmation.

Comment 6

[Dave Townsend [:mossop]]

What Firefox version should we start requiring this?

Comment 7

[Robert Kaiser]

Note that this means that all the locale packs we generate during the normal Firefox build process will stop working the way they did before and need to be looped through AMO to work. Also, a lot of Linux distros provide L10n for their release builds only through locale packs and they build themselves so those packs are probably unsigned and will stop working. In other words, we need to make quite some noise around this if we do it.

Comment 8

[Kris Maglione [:kmag]]

It's hard to say. We haven't come up with a plan for signing locale packs for betas yet, so requiring signing on non-release builds seems like a bad idea. And we don't even build locale packs for nightly or Aurora, so it's going to be hard to do smoke tests.

Given comment 7, I think we should probably try to reach out to people who might be affected before we make a decision. And we definitely need to figure out what we're going to do about language packs for betas.

Comment 9

[Dave Townsend [:mossop]]

In which case until there is a plan for when this needs to happen I'm going to re-assign this. I'm happy to do the patch when we know when we want it, it will be pretty simple.

Comment 10

[Liz Henry (:lizzard) (relman/hg->git project)]

Kris, any news on what we are planning here? Thanks.

Comment 11

[Kris Maglione [:kmag]]

It's up to kev, at this point. I can answer any specific technical questions, but the decisions aren't mine to make.

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Kev, is this still an issue for 43? 
I'm assuming it isn't planned to affect 43 here.

Comment 13

[Andy McKay]

We should do this, but I think we've missed the boat for 43. I need to find out how locale packs are generated at Mozilla at least and make sure when we land code for this we've got a plan in place.

Comment 14

[Andy McKay]

Confirmed that language packs on AMO for 42+ are all signed as per bug 1198425.

Comment 15

[Liz Henry (:lizzard) (relman/hg->git project)]

So as I understand it, this is ok on release, and we don't have concrete plans yet for pre-release channels.   
Once we do have plans, please re-nominate this for tracking or let release management know it's moving forward.

Comment 16

[Axel Hecht [:Pike]]

Linux distros feel like the most challenging user right now.

For localizers, being able to test on aurora is important, and we're keeping the pref there to allow developers to test their add-ons, right?

Comment 17

[Andy McKay]

(In reply to Axel Hecht [:Pike] from comment #16)
> Linux distros feel like the most challenging user right now.
> 
> For localizers, being able to test on aurora is important, and we're keeping
> the pref there to allow developers to test their add-ons, right?

Yes, on Aurora we will allow users to turn off the pref.

Comment 18

[Andy McKay]

Based on the conversation in bug 1388143, could we get signed language packs in the build for 59 so that we can flip the flag to require this?

Comment 19

[Jordan Lund (:jlund)]

callek is in charge of all things l10n for ReleaseEngineering.

callek, can you have a look at this, scoping out requirements from us, the automation solution, and coordination with teams involved?

Some open questions needed to be answered:

- how and where this is still needed
- how it differs from esr52 (the langpack format)
- how we can bake this effort into esr59 tcmigration timeline.
- can we sign the langpacks after we build, through AMO? Skipping any work needed on our signing servers.
- How do we enforce they are signed? Do we need build sys support?
- For esr 59.0.0 what's the minimum needed? Can we add enforced signing support but manually sign initially?

Comment 20

[Axel Hecht [:Pike]]

(In reply to Jordan Lund (:jlund) from comment #19)
> callek is in charge of all things l10n for ReleaseEngineering.
> 
> callek, can you have a look at this, scoping out requirements from us, the
> automation solution, and coordination with teams involved?
> 
> Some open questions needed to be answered:

Taking some of those questions.

> - how and where this is still needed
> - how it differs from esr52 (the langpack format)

They're just regular web extension zip files, with xpi extension. And we want them to be signed.

> - how we can bake this effort into esr59 tcmigration timeline.
> - can we sign the langpacks after we build, through AMO? Skipping any work
> needed on our signing servers.

I personally don't like the idea of having things on ftp that people can't use.
Also, at least for automated generation of snaps, we'll want the signing process to be part of the task graph, I think.
And yes, the language packs that are part of the snap need to be signed, too.

> - How do we enforce they are signed? Do we need build sys support?
> - For esr 59.0.0 what's the minimum needed? Can we add enforced signing
> support but manually sign initially?

Comment 21

[Jordan Lund (:jlund)]

Thanks Axel for that.

I've created bug 1426181 to track releng/automation work.

Comment 22

[:shell escalante]

Adding conversation to Bug....

We want to use the existing pref for the real implementation to control language pack signing requirement, the same as other add-on signing (xpinstall.signature.required).  

But if we only use that existing pref (xpinstall.signature.required) for testing in Beta - as soon as the patch lands it requires language packs be signed.  

That might not be bad in Beta - but if for some reason release engineering doesn't get the back end together in time (for end to end testing)... my concern (which may be misplaced) is there is no fall back option to ignore the patch (which would continue to allow unsigned language packs).

We can't flip the xpinstall.signature to false to ignore the patch - because that would allow ALL unsigned add-ons.  

I didn't know if there was a possibility of a pref to ignore the patch if we don't make it for 60?

Comment 23

[Andrew Swan [:aswan]]

I thought what we talked about today is creating a (possibly temporary?) new preference so that QA can starting testing now (with language packs signed in the AMO staging or dev environment since signed language packs for 60 don't exist yet).
When QA signs off we can either make that pref work just like xpinstall.signatures.required (ie only honored on non-release branches), or remove that pref and fold that behavior into xpinstall.signatures.required.

Comment 24

[:shell escalante]

Exactly aswan said is perfect.  Sorry I was confusing.

Comment 25

[krupa raj [:krupa--use this to needinfo]]

This is currently being tested.

Comment 26

[Andrew Swan [:aswan]]

unassigning myself from the tracking bug

Comment 27

[Julien Cristau [:jcristau]]

Setting 60 to wontfix as I believe 1454141 is hoping to land for 61.

Comment 28

[Justin Wood (:Callek)]

:aswan this is done, right?

Comment 29

[Andrew Swan [:aswan]]

Depends on the scope I guess.  We're not requiring signed language packs on dev edition yet.  Kev, is that something worth keeping this open for?

Comment 30

[Jorge Villalobos [:jorgev] (he/him)]

Is dev edition still a pending issue here?

Comment 31

[Andrew Swan [:aswan]]

I don't remember what the issue was for dev edition. We allow unsigned addons of other types on dev edition with a flip of xpinstall.signatures.required. We do the same for langpacks but with a different pref (extensions.langpacks.signatures.required).
I'm going to close this meta bug, if there was some other dev edition issue that wasn't well captured here it should probably just be a new bug.