791741 - [tracking] Sign packaged apps for users

Status: RESOLVED FIXED

(Marketplace Graveyard :: Security, defect, P1)

Description

[Andy McKay]

In bug 791732 we allow packaged apps to be signed for users. This bug covers how we actually sign the apps. Note this is all about verification, not encryption. How the client will be checking the apps is covered in bug 772365.

Comment 1

[Jason Smith [:jsmith]]

Also probably needs to be done before v1 ships.

Comment 2

[Andy McKay]

I'm going to give some of the code here a clean and a try before using signtool.

https://adblockplus.org/blog/signing-firefox-extensions-with-python-and-m2crypto
https://github.com/nmaier/xpisign.py/

Comment 3

[Wil Clouser [:clouserw]]

Andy is working on the signing this week

Comment 4

[Andy McKay]

xpisign code is in, blocked on bug 793876

https://github.com/mozilla/zamboni/commit/540bfe

Waiting for an end-to-end to test to be possible before resolving.

Comment 5

[Andy McKay]

Waiting on bug 793876.

Comment 6

[Andy McKay]

Sent example signed app on -dev to bsmith to check it's done correctly.

Comment 7

[Wil Clouser [:clouserw]]

Any updates here?

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Please point me to the the script (OpenSSL response file) that was/is used to generate the certificates, so I can modify them to correct some of the metadata. I want to change the key usage, hash algorithm, and key size, in particular. I am writing the documentation for what the certificates should contain and I want to test the steps in the documentation before I post it. Thanks.

Comment 9

[Jeremy Orem [:oremj]]

The dev cert/key generation was a one-off. If you document what all is needed, then we can use that information when creating the production cert/key.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Here are the steps to sign an app that will be accepted by the device:

1. Get the updated xpisign.py from https://github.com/briansmith/xpisign.py/tree/b2g
2. Generate test certificates according to the script in bug 793876.

In python:
from xpisign import xpisign
xpisign("unsigned.jar", "examplla-marketplace-app-signing-1-combined.pem", "signed.jar", optimize_signatures=True, omit_sf_entry_sections=True, , omit_created_by=True)

The three boolean parameters in the end are all unnecessary. They make the JAR file smaller by removing redundant information. The key thing to look for is that the zigbert.sf file **MUST** have a SHA1-Digest-Manifest entry in its main section. This is something that the unmodified xipsign.py script does NOT do, and it is why you must use my modified version, or another tool that does output SHA1-Digest-Manifest.

Comment 11

[Jason Smith [:jsmith]]

Not on-device - removing nom.

Inevitably the platform pieces are actually what blocks (and should in some ways could be tested interdependently of marketplace), but not the marketplace piece.

Comment 12

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

The example signing script that shows all the settings needed for the certs is now in bug 772365 attachment 682525 [details] [diff] [review].

That script also does some rudimentary sanity checks on the app before signing them. There are probably more sanity checks that need to be done before signing.

Note that the Java jarsigner tool, and NSS's signtool, basically assume that the party that generated the unsigned app is the same party that is signing the app, so they don't do (m)any checks to identify malicious apps.

One thing that is particularly important: we should always generate the signed version of the JAR by creating a new JAR and copying entries from the unsigned JAR to the new JAR, and then signing the new JAR. We should never sign the origial unsigned JAR directly. It is possible to create corrupted metadata in a JAR (e.g. lie about the size of an entry or possibly other things.) Creating a brand-new JAR with 100% clean metadata avoids this problem.

Comment 13

[Wil Clouser [:clouserw]]

basecamp-blocking+ this.  We discusssed this in our meeting. Lucas, Caitlin, and I all think we need this this to ship.  Please talk to us before changing the flag. Thanks.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Wil Clouser [:clouserw] from comment #13)
> basecamp-blocking+ this.  We discusssed this in our meeting. Lucas, Caitlin,
> and I all think we need this this to ship.  Please talk to us before
> changing the flag. Thanks.

So exactly what is the work effort associated with this bug? I can't tell if this is an implementation bug or a meta bug. If it's a meta, it doesn't block - we block on the individual work items, as meta bugs are not measurable.

Even so - if you are going to flag this for basecamp blocking, then we need to give clarity on two items - the priority as a basecamp blocker (which in this case is a P1) and a convergence milestone (which I have no idea how we would be able to set in a marketplace component).

Comment 15

[Jason Smith [:jsmith]]

The inherent problem here is a need to have an implementation completed for signing of packaged apps for users. I think for clarity, let's stick this under the b2g general bucket and send this into platform triage. That will mitigate the tracking concerns possibly.

Comment 16

[Dietrich Ayala (:dietrich)]

Hi Brian. What actual work needs to be done here, and when do you expect it to be complete?

Comment 17

[Andrew Overholt [:overholt]]

If this is the Marketplace-side of verification, why isn't it in the Marketplace component?

Comment 18

[Jason Smith [:jsmith]]

(In reply to Andrew Overholt [:overholt] from comment #17)
> If this is the Marketplace-side of verification, why isn't it in the
> Marketplace component?

I think that's right. Let's move it back.

Comment 19

[Damon Sicore (:damons)]

Can we get a priority and target milestone for this>

Comment 20

[Jason Smith [:jsmith]]

Talking with Rob, this appears to be a meta bug. The actual work is happening in the dependencies. I'll flag the dependencies instead.

Comment 21

[Wil Clouser [:clouserw]]

bug 819053 is directly blocking bug 818641 which is a more useful association than routing through this tracking bug.  Closing this as fixed.