Closed Bug 1515173 Opened 5 years ago Closed 5 years ago

support signing omni.jar, system addons, and language packs

Categories

(Cloud Services :: Operations: Autograph, enhancement)

Product:
Cloud Services
Component:
Operations: Autograph
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: u581815, Unassigned)

References

(Blocks 1 open bug)

Details

refs bug 1514081

Per irc chat with :Callek, we're targetting Q1 2019 to sign the syslog addon, omni.jar and language packs with autograph


We should be able to reuse the apk or xpi v1 signers (since they handle JAR-like formats).



> omni.ja is packed specially (to make disk I/O less of a pain)

so we might need additionally QA there to make sure we're not breaking it.
Summary: support signing omni.jar, sys log addon, and language packs → support signing omni.jar, system addons, and language packs
(In reply to Greg Guthe [:g-k] [:gguthe] from comment #0)
> refs bug 1514081
> 
> Per irc chat with :Callek, we're targetting Q1 2019 to sign the syslog
> addon, omni.jar and language packs with autograph

Slight correction, signing of the system addons (which is already happening via autograph aiui, just not using releng automation which this will need to do)

It's also not completely clear if this will be a Q1/H1 OKR level goal or not yet, I will know that early in January.

(In reply to Greg Guthe [:g-k] [:gguthe] from comment #0)
> > omni.ja is packed specially (to make disk I/O less of a pain)
> 
> so we might need additionally QA there to make sure we're not breaking it.

I think releng may be able to achieve this "specially" piece outside of autograph as well, if we need to do any magic to ensure this maintains its expectations.
(In reply to Justin Wood (:Callek) from comment #1)
> I think releng may be able to achieve this "specially" piece outside of
> autograph as well, if we need to do any magic to ensure this maintains its
> expectations.

catlee and I chatted about this yesterday - the main gotcha we know about is that we want to avoid screwing up the performance optimizations that are used when building the omni jar (the files are ordered in a certain way, and there's a custom header). 

All we really need for the signature is to add a new directory and a few files (META-INF/*) in the root and it can be at the end of the file, so maybe this is a non-issue as long as we are mindful in how we actually modify the file. If it's possible to just append rather than rebuilding the archive that might be enough.

We should use automation to make sure there isn't a startup perf impact (I'm not sure if we usually do this post-signing?) and QA just to make sure we're not breaking anything else etc.

The XPIs in the application directory currently don't have any optimizations like this so we should be able to just use the normal XPI signing for these, for the record we're going to stop having these standalone XPIs and move bundled add-ons into the omni jar at some point (bug 1357205) but I don't think that has any effect on this bug.
Does this require entirely new code in Firefox? And if so, could we use something else than jarsigning?
I'm thinking that a COSE signature file that covers the entire omni.jar and is verified prior to loading it would be simpler to implement and maintain.
(In reply to Julien Vehent [:ulfr] from comment #3)
> Does this require entirely new code in Firefox? And if so, could we use
> something else than jarsigning?
> I'm thinking that a COSE signature file that covers the entire omni.jar and
> is verified prior to loading it would be simpler to implement and maintain.


This is in service of bug 1514081, and rhelmer would be a good person to answer
Flags: needinfo?(rhelmer)
(In reply to Julien Vehent [:ulfr] from comment #3)
> Does this require entirely new code in Firefox? And if so, could we use
> something else than jarsigning?
> I'm thinking that a COSE signature file that covers the entire omni.jar and

I was planning to re-use the signature verification code, https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/toolkit/mozapps/extensions/internal/XPIInstall.jsm#339 which I believe is implemented in https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/security/apps/AppSignatureVerification.cpp#1359-1360

It looks like the latter supports COSE if I'm reading this right, depending on the policy setting:

https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/security/apps/AppSignatureVerification.cpp#1256-1275

Looks like the policy is controlled by this pref: https://searchfox.org/mozilla-central/rev/2e5e28f518524f8af5b158ddb605022b6a2d68cf/security/manager/ssl/security-prefs.js#85

> is verified prior to loading it would be simpler to implement and maintain.

So would this be a separate file, and not require modifying omni jar? We're not thinking about enforcement, just detecting corruption.
Flags: needinfo?(rhelmer)
COSE is part of XPI signing, but it's not yet rolled out. We're introducing it as a new signing method in future releases.

We could sign omni.jar exactly the same way we sign XPIs. It would certainly make things simpler, but the signing process will break the file alignment (we had that issue with android APKs, which are also JARs). Could we rerun the omni.jar packing tool post-signing?
(In reply to Julien Vehent [:ulfr] from comment #6)
> COSE is part of XPI signing, but it's not yet rolled out. We're introducing
> it as a new signing method in future releases.
> 
> We could sign omni.jar exactly the same way we sign XPIs. It would certainly
> make things simpler, but the signing process will break the file alignment
> (we had that issue with android APKs, which are also JARs). Could we rerun
> the omni.jar packing tool post-signing?

My current thinking is, yes we can. Catlee and myself both think this is doable outside of the signing server (autograph) side.
Blocks: 1515712

rhelmer, mdeboer,

Would either of you have context as to the priority of the omni.jar signing piece? Or know who could get me that information?

I'm trying to determine where it should fit on Release Engineering's roadmap. e.g.

  • Why it's important?
  • Is there a hard or soft deadline?
  • What happens if we don't do it?

Thanks!

Flags: needinfo?(rhelmer)
Flags: needinfo?(mdeboer)

Jordan, I answered your question in bug 1514081.

Flags: needinfo?(mdeboer)
Flags: needinfo?(rhelmer)

Trying to turn COSE signing in AMO, but QA found a bug with Fx treating langpacks w/ COSE signatures as corrupt: https://github.com/mozilla/addons-server/issues/10588

  • Does Firefox treat langpacks like regular XPIs? If not, then it's probably a bug in autograph.

    • Does it follow the security.signed_app_signatures.policy pref to accept and validate COSE signatures when present or use a different pref?

    • Do langpacks need a certain CN or OU in their PKCS7 cert?

(In reply to Greg Guthe [:g-k] [:gguthe] from comment #10)

Trying to turn COSE signing in AMO, but QA found a bug with Fx treating langpacks w/ COSE signatures as corrupt: https://github.com/mozilla/addons-server/issues/10588

  • Does Firefox treat langpacks like regular XPIs? If not, then it's probably a bug in autograph.

    • Does it follow the security.signed_app_signatures.policy pref to accept and validate COSE signatures when present or use a different pref?

    • Do langpacks need a certain CN or OU in their PKCS7 cert?

Flags: needinfo?(aswan)

(In reply to Robert Helmer [:rhelmer] from comment #11)

(In reply to Greg Guthe [:g-k] [:gguthe] from comment #10)

Trying to turn COSE signing in AMO, but QA found a bug with Fx treating langpacks w/ COSE signatures as corrupt: https://github.com/mozilla/addons-server/issues/10588

  • Does Firefox treat langpacks like regular XPIs? If not, then it's probably a bug in autograph.

Yes, signatures on langpacks are validated in the same way as signatures on extensions, themes, etc.

  • Does it follow the security.signed_app_signatures.policy pref to accept and validate COSE signatures when present or use a different pref?

I'm not sure, it calls this function:
https://searchfox.org/mozilla-central/rev/9eb30227b21e0aa40d51d9f9b08bb0b113c5fadb/security/manager/ssl/nsIX509CertDB.idl#263-266
I don't know much about what happens inside that function, I would suggest checking with keeler.

  • Do langpacks need a certain CN or OU in their PKCS7 cert?

Yes, they need to have the addon id as the CN. It gets a trickier for addons with ids that are longer than the CN field, there is a little more information here:
https://wiki.mozilla.org/Add-ons/Extension_Signing

Flags: needinfo?(aswan)

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

Thanks :aswan!

  • Does it follow the security.signed_app_signatures.policy pref to accept and validate COSE signatures when present or use a different pref?

I'm not sure, it calls this function:
https://searchfox.org/mozilla-central/rev/9eb30227b21e0aa40d51d9f9b08bb0b113c5fadb/security/manager/ssl/nsIX509CertDB.idl#263-266
I don't know much about what happens inside that function, I would suggest checking with keeler.

:keeler any idea why adding a COSE ES256 signature would break signature verification but only for langpacks? :cgrebs has a test one in https://github.com/mozilla/addons-server/issues/10588#issuecomment-461300120

Flags: needinfo?(dkeeler)

Stepping through verifying that file, what I'm seeing is it's complaining about lines in META-INF/manifest.mf being too long. Apparently there's a line limit of 72 bytes.(although it looks like you can essentially wrap them to the next line by prepending it with a space?) (this is all in https://searchfox.org/mozilla-central/source/security/apps/AppSignatureVerification.cpp#279 ). It looks like it's been that way for a while, though, so I don't see why switching to COSE would have an effect, unless the new signing implementation isn't careful about line lengths.

Flags: needinfo?(dkeeler)

(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #14)

Stepping through verifying that file, what I'm seeing is it's complaining about lines in META-INF/manifest.mf being too long. Apparently there's a line limit of 72 bytes.(although it looks like you can essentially wrap them to the next line by prepending it with a space?) (this is all in https://searchfox.org/mozilla-central/source/security/apps/AppSignatureVerification.cpp#279 ). It looks like it's been that way for a while, though, so I don't see why switching to COSE would have an effect, unless the new signing implementation isn't careful about line lengths.

Aha! Thanks :keeler!

Autograph is in fact not careful about line lengths. We fixed this for APK JARs https://github.com/mozilla-services/autograph/pull/145/files#diff-38e4bd38907085f68f0624f5485466eeR23 but weren't sure if Fx checked that part of the spec too.

Yep that was the issue. Dana++

Just to update everyone on the autograph side of things:

  • the AMO team is testing 100% of traffic with backwards compatible COSE signatures in -stage this week. Assuming that goes well we'll start testing AMO -prod in the next few weeks per https://groups.google.com/a/mozilla.com/forum/#!topic/autograph-users/h94cxEbdOso (let me know if you want an invite to that mailing list)

  • we deployed an XPI signer to the autograph -stage HSM environment and have the config changes in place for -prod (bug 1518642). Assuming we sign system addons for each release and they don't need to be backwards compatible with old Fx versions, we can use SHA2 signatures across the board (bug 1501307) and sign with taskcluster, signingscript, and autograph. I think the next step would be adding system addon support to signingscript.

:Callek,

  • Are system addons and omni.jar currently signed as system addons using the addon-shipper lambda?
  • Do they need to be backwards compatible with old Fx versions (in which case we should use AMO) or can we switch to taskcluster and the SHA2 signatures?
  • Are we planning on signing lang packs with AMO since they're web extensions? (I'm assuming so, but want to confirm)
Flags: needinfo?(bugspam.Callek)

(In reply to Greg Guthe [:g-k] [:gguthe] from comment #16)

  • Are system addons and omni.jar currently signed as system addons using the addon-shipper lambda?
  • Do they need to be backwards compatible with old Fx versions (in which case we should use AMO) or can we switch to taskcluster and the SHA2 signatures?

:Rehan do you know the answers to Greg's questions about system addons here? If you don't do you know a name to redirect this to whom would know

  • Are we planning on signing lang packs with AMO since they're web extensions? (I'm assuming so, but want to confirm)

Greg, so "today" yes language packs will be signed in AMO via sending them through AMO api's. in the near future (within 2019) we hope to get off AMO's APIs for this piece of release process and instead sign them directly with autograph via scriptworker, and these will need to maintain the same CN as they do when submitted through AMO. (and I suspect but don't know for sure, the same cert type that Firefox currently accepts). So it does have some requirements we should think about here.

This near future is part of what this bug was filed about, but is presumably less urgent than the omni.ja signing pieces.

Flags: needinfo?(bugspam.Callek) → needinfo?(rdalal)

Jason Thomas (:jason) usually handles the signing of system addons and can probably answer some (if not all) of these questions.

Flags: needinfo?(rdalal) → needinfo?(jthomas)

I am going to redirect this to :wezhou since I don't work with add-on signing anymore.

Flags: needinfo?(jthomas) → needinfo?(wezhou)

Are system addons and omni.jar currently signed as system addons using the addon-shipper lambda?

System addons are signed manually using the steps documented in [1]

I don't know if omni.jar is an system addon or not. Usually, the signing requester tells us an addon is to be signed as an system addon, and we will sign it as such.

I'm not sure what "addon-shipper" lambda is. But, there is a lambda known as "addon-signxpi" lambda, which is used to sign what's called "internal addon/extension". Information about signing this type of addon is in [2] and [3]. If omni.jar were considered as an internal addon, someone would have followed those docs and signed it (cloudops rarely get involved signing internal addons).

Do they need to be backwards compatible with old Fx versions (in which case we should use AMO) or can we switch to taskcluster and the SHA2 signatures?

Sorry but I don't how to answer this question. I assume someone who owns the omni.jar file should know.

Flags: needinfo?(wezhou)

(In reply to :wezhou from comment #20)

Are system addons and omni.jar currently signed as system addons using the addon-shipper lambda?

System addons are signed manually using the steps documented in [1]

I don't know if omni.jar is an system addon or not. Usually, the signing requester tells us an addon is to be signed as an system addon, and we will sign it as such.

omni JAR(s) and any XPI files (such as bundled system add-ons) shipped with Firefox have never been signed AFAIK.

The tl;dr is that "default" system add-ons are packaged as XPI files and bundled with Firefox and persist in the (read-only) application directory, and "updates" are signed by Mozilla and are delivered to the users (read/write) profile directory. :wezhou has been doing signatures for "updates" but not "defaults".

The distinction between default and updates for system add-ons is documented further at https://firefox-source-docs.mozilla.org/toolkit/mozapps/extensions/addon-manager/SystemAddons.html#default-built-in-system-add-ons

I'm not sure what "addon-shipper" lambda is. But, there is a lambda known as "addon-signxpi" lambda, which is used to sign what's called "internal addon/extension". Information about signing this type of addon is in [2] and [3]. If omni.jar were considered as an internal addon, someone would have followed those docs and signed it (cloudops rarely get involved signing internal addons).

Do they need to be backwards compatible with old Fx versions (in which case we should use AMO) or can we switch to taskcluster and the SHA2 signatures?

Sorry but I don't how to answer this question. I assume someone who owns the omni.jar file should know.

I don't think we need to worry about backwards compatibility for files that are shipped with Firefox since they'll only be verified by new code (bug 1515712). We'll need to take more care around updates but I think we can handle that separately without trouble.

See Also: → 1533818

Bug 1515712 has landed and we're starting to see data from Nightly users, so I think we're ready to go whenever signed omni ja and XPIs start shipping as well. Thanks!

No longer blocks: 1515712
Depends on: 1515712

I estimated end of April for this work, but I think signing omni.ja may slip until May 7'th give or take. I was blocked a bit on a longer-tail of py3 and some bikeshedding in build-team about the code we use to read the omni.ja files (to support the preloading/etc)

I appreciate your patience, let me know if there is any issues from product with the timeline shift.

This work is done, with the exception of sysaddon signing (which was dropped per discussion with :aswan and :rhelmer over e-mail)

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.