1178518 - Support for verifying signed packages

Status: RESOLVED FIXED

(Core :: Networking, defect, P2)

Description

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Bug for tracking the implementation of package signature verification in the new FxOS security model. See https://wiki.mozilla.org/FirefoxOS/New_security_model#Verifying_signatures_-_bug_1153422 for more details.

Comment 1

[Jonathan Hao (inactive) [:jhao]]

I'll write down what I think need to be done in this bug, based on my understanding.
However, there are some points I'm not so sure about. Maybe Paul or Dave could tell us more.

1. Make Gecko recognize URLs like this:

https://website.com/RSSReader2000/package.pak!//index.html

so that it will download package.pak instead of index.html from the Internet.

1-1. If we can't get package.pak successfully, we should try to get index.html directly, right?
1-2. The separator used in http://w3ctag.github.io/packaging-on-the-web is "!/" (one slash) instead of "!//" (two slashes) in the wiki. Which one are we going to adopt?

2. Verify the signature.
2-1. But the signature format isn't decided yet, right? So is this bug blocked by some other bugs?

3. Retrieve index.html from the package.pak. The package format is defined here http://w3ctag.github.io/packaging-on-the-web/#streamable-package-format
Give index.html to the rest of Gecko.

4. Populate the caches of other resources contained in package.pak as in http://w3ctag.github.io/packaging-on-the-web/#populating-caches
4.1 Is this part optional for now?

Please correct me if I missed or misunderstood anything.

I've been tracing code in DocShell and Necko and trying to deal with the specialized URL.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

(In reply to Jonathan Hao [:jhao] from comment #1)
> I'll write down what I think need to be done in this bug, based on my
> understanding.
> However, there are some points I'm not so sure about. Maybe Paul or Dave
> could tell us more.
> 
> 1. Make Gecko recognize URLs like this:
> 
> https://website.com/RSSReader2000/package.pak!//index.html
> 
> so that it will download package.pak instead of index.html from the Internet.

Most of this I implemented in bug 1036275.

The channel is intercepted at:
https://hg.mozilla.org/mozilla-central/annotate/e7e69cc8c07b/netwerk/protocol/http/nsHttpChannel.cpp#l4809

It then calls to the packagedAppService, which fetches the package, and puts the content in the cache entries.
The parsing of the package is done in nsMultiMixedConv (because the package is essentially multipart/*)

> 
> 1-1. If we can't get package.pak successfully, we should try to get
> index.html directly, right?
> 1-2. The separator used in http://w3ctag.github.io/packaging-on-the-web is
> "!/" (one slash) instead of "!//" (two slashes) in the wiki. Which one are
> we going to adopt?

The one in the wiki !//
We gathered some telemetry, and it seems that this was very infrequent on the web, especially due to the double-slash.
Also, this URL format is much easier to understand and account for, than the Link/described-by mechanism in the w3ctag spec.
> 
> 2. Verify the signature.
> 2-1. But the signature format isn't decided yet, right? So is this bug
> blocked by some other bugs?
> 

This is something I also think needs to be decided as soon as possible, as it affects a large part of our architecture.
Optimally, we should include the signature in each part's header, in order to be able to check it as we're downloading the package.
If we also need to verify the integrity of the entire package, that would slightly more annoying, as we'd have to download the entire thing, before serving content to the listeners.

> 3. Retrieve index.html from the package.pak. The package format is defined
> here http://w3ctag.github.io/packaging-on-the-web/#streamable-package-format
> Give index.html to the rest of Gecko.
> 
> 4. Populate the caches of other resources contained in package.pak as in
> http://w3ctag.github.io/packaging-on-the-web/#populating-caches
> 4.1 Is this part optional for now?

That is actually what we do at the moment.
Feel free ping me on #necko if you have any questions

Comment 3

[Paul Theriault [:pauljt] (no longer reading bugmail)]

> > 
> > 2. Verify the signature.
> > 2-1. But the signature format isn't decided yet, right? So is this bug
> > blocked by some other bugs?
> > 
> 
> This is something I also think needs to be decided as soon as possible, as
> it affects a large part of our architecture.
> Optimally, we should include the signature in each part's header, in order
> to be able to check it as we're downloading the package.
> If we also need to verify the integrity of the entire package, that would
> slightly more annoying, as we'd have to download the entire thing, before
> serving content to the listeners.

So this bug I think should be about this part - i.e. the signature verification. 

In terms of proposal, I have two that I can think of at the moment, but I'm trying to get some guidance from the security engineering team here. I'll try to get the right people together to start this discussion.

Comment 4

[Henry Chang [:hchang]]

(In reply to Valentin Gosu [:valentin] from comment #2)
> (In reply to Jonathan Hao [:jhao] from comment #1)
> > I'll write down what I think need to be done in this bug, based on my
> > understanding.
> > However, there are some points I'm not so sure about. Maybe Paul or Dave
> > could tell us more.
> > 
> > 1. Make Gecko recognize URLs like this:
> > 
> > https://website.com/RSSReader2000/package.pak!//index.html
> > 
> > so that it will download package.pak instead of index.html from the Internet.
> 
> Most of this I implemented in bug 1036275.
> 
> The channel is intercepted at:
> https://hg.mozilla.org/mozilla-central/annotate/e7e69cc8c07b/netwerk/
> protocol/http/nsHttpChannel.cpp#l4809
> 
> It then calls to the packagedAppService, which fetches the package, and puts
> the content in the cache entries.
> The parsing of the package is done in nsMultiMixedConv (because the package
> is essentially multipart/*)
> 
> > 
> > 1-1. If we can't get package.pak successfully, we should try to get
> > index.html directly, right?
> > 1-2. The separator used in http://w3ctag.github.io/packaging-on-the-web is
> > "!/" (one slash) instead of "!//" (two slashes) in the wiki. Which one are
> > we going to adopt?
> 
> The one in the wiki !//
> We gathered some telemetry, and it seems that this was very infrequent on
> the web, especially due to the double-slash.
> Also, this URL format is much easier to understand and account for, than the
> Link/described-by mechanism in the w3ctag spec.
> > 
> > 2. Verify the signature.
> > 2-1. But the signature format isn't decided yet, right? So is this bug
> > blocked by some other bugs?
> > 
> 
> This is something I also think needs to be decided as soon as possible, as
> it affects a large part of our architecture.
> Optimally, we should include the signature in each part's header, in order
> to be able to check it as we're downloading the package.
> If we also need to verify the integrity of the entire package, that would
> slightly more annoying, as we'd have to download the entire thing, before
> serving content to the listeners.
> 

I have a question here. Does it mean, if the policy is to sign every single file 
instead of the entire package, we can serve content once the requested resource 
is seen in the stream (before download complete) and verified signed properly?

Thanks!

Comment 5

[Jonathan Hao (inactive) [:jhao]]

Attachment: Signed Package Verifier (interface)

Hi Henry,

Is this interface suitable for your usage?

Comment 6

[Henry Chang [:hchang]]

Comment on attachment 8641551 [details] [diff] [review]
Signed Package Verifier (interface)

Review of attachment 8641551 [details] [diff] [review]:
-----------------------------------------------------------------

It looks perfect to me! Thanks!

Comment 7

[Honza Bambas (:mayhemer)]

Comment on attachment 8641551 [details] [diff] [review]
Signed Package Verifier (interface)

Review of attachment 8641551 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/nsISignedPackageVerifier.idl
@@ +15,5 @@
> +                      in nsIVerificationCallback aCallback);
> +
> +  void checkIntegrity(in AString aFileName,
> +                      in AString aPayload,
> +                      in nsIVerificationCallback aCallback);

this needs comment, don't you think?  what exactly are the arguments?  who is the intended caller?

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Also, wouldn't it be better to have an incremental way of checking the integrity, so we don't need to keep a copy of the payload?

Comment 9

[Henry Chang [:hchang]]

(In reply to Honza Bambas (not reviewing) (:mayhemer) from comment #7)
> Comment on attachment 8641551 [details] [diff] [review]
> Signed Package Verifier (interface)
> 
> Review of attachment 8641551 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/protocol/http/nsISignedPackageVerifier.idl
> @@ +15,5 @@
> > +                      in nsIVerificationCallback aCallback);
> > +
> > +  void checkIntegrity(in AString aFileName,
> > +                      in AString aPayload,
> > +                      in nsIVerificationCallback aCallback);
> 
> this needs comment, don't you think?  what exactly are the arguments?  who
> is the intended caller?

Let me detail my proposal to these interfaces:

There are two phases for signed package verification:

The first phase is to use the signature in the package header to verify
the first embedded manifest file, no matter it's a separate manifest or
the app manifest. This verification would be done in |verifyManifest|

Once the manifest is verified, we need to check the integrity of each
sub-resource based on the info provided in the manifest [1] we verified 
earlier on. In order to avoid JSON data processing, I propose to do the 
check in the XPCOM that this bug is supposed to create to not involve 
the manifest parsing. (Since the expected integrity hash is in the manifest.)
The integrity check would be done in |checkIntegrity| and the intended caller
is PackagedAppService.

Since we haven't decided the actual format of the signature and the integrity
hash function, we just leave it for nsISignedPackageVerifier to decide.

One issue to |checkIntegrity| is passing every single resource to a function
might be not efficient. One solution might be to calculate the hash in place
and pass it to the function to check.

[1] https://wiki.mozilla.org/User:Ptheriault/Packagedprivilegedcontent#Example

Comment 10

[Jonathan Hao (inactive) [:jhao]]

Attachment: Signed Package Verifier (interface)

Added comments. They're just as Henry described.

Comment 11

[Jonathan Hao (inactive) [:jhao]]

Attachment: Signed Package Verifier (WIP)

Added implementation of verifyManifest, but it needs testing.

Comment 12

[Jonathan Hao (inactive) [:jhao]]

Attachment: Signed Package Verifier (WIP)

Added implementation of checkIntegrity()

Comment 13

[Valentin Gosu [:valentin] (he/him)]

Hi Jonathan,
Do you think we could make this IDL similar to nsICryptoHash ?
The way that works is you call init(), then update(content), then when the content is done, you call finish() which generates the hash for you.

I was thinking we could do something similar, as it would really benefit from the way we download web packaged apps.
So we'd have
init(signature) - the signature we get from the package metadata
updateManifest(content) - appends content to the

Comment 14

[Valentin Gosu [:valentin] (he/him)]

Accidentally hit Save.

> updateManifest(content) - appends content to the
internal buffer.

finish() - checks the signature for the manifest is correct, and otherwise throws
         - this should be done in a sync way. From what I can tell we can do that easily: https://dxr.mozilla.org/mozilla-central/source/modules/libjar/nsJAR.cpp#608

checkIntegrity(aFileName, aHashValue) should also be sync

We could have both sync and async versions if needed, but the sync version should improve the complexity and performance of PackagedAppService, once we integrate it with the SignatureVerifier.
What do you think?

Comment 15

[Henry Chang [:hchang]]

I can do with both. The async version is complicated but I think I have figured it out clearly and had a complete implementation. Anyways, I'd like Johnathan to decide it.

Comment 16

[Jonathan Hao (inactive) [:jhao]]

Hi Valentin,

I think checkIntegrity could be sync. As for signature verification, I chose to make it async because I wanted to use verifySignedManifestAsync in nsIX509CertDB.

I will look into nsJAR.cpp. If we can imitate the usage, we can go with sync version.

Comment 17

[Jonathan Hao (inactive) [:jhao]]

Attachment: Signed Package Verifier (WIP)

I can verify the signature produced by the signing tool of trusted hosted apps now, using TrustedHostedAppTestRoot as trusted root.

Hi Paul,

Which root are we planning to trust in production code eventually?
Will it be AppMarketplaceProdPublicRoot?

The available roots now are listed in https://dxr.mozilla.org/mozilla-central/source/security/manager/ssl/nsIX509CertDB.idl?offset=600#320

Comment 18

[Jonathan Hao (inactive) [:jhao]]

Attachment: SIgned Package Verifier (WIP)

I created PrivilegedPackageRoot as the trusted root of our verification process. Its corresponding signingDB resides locally in my machine, so only I can sign it now.

There's still some minor changes to do.

Also, this patch needs to be rebased because I found that fabrice just removed all the trusted hosted app stuff in bug 1196988.

Comment 19

[Jonathan Hao (inactive) [:jhao]]

Attachment: SIgned Package Verifier

Hi David and Richard,

Could you review my patch?

Thanks.

Comment 20

[Valentin Gosu [:valentin] (he/him)]

It seems you forgot to include test_signed_package_verifier.js in this last version.

Comment 21

[Valentin Gosu [:valentin] (he/him)]

(In reply to Valentin Gosu [:valentin] from comment #20)
> It seems you forgot to include test_signed_package_verifier.js in this last
> version.
And SignedPackageVerifier.manifest

Comment 22

[Jonathan Hao (inactive) [:jhao]]

Attachment: SIgned Package Verifier

Oops, adding missing files.

Comment 23

[Jonathan Hao (inactive) [:jhao]]

Attachment: MozReview Request: Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

Comment 24

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8652799 [details]
MozReview Request: Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

It's my first time using MozReview. If I did anything wrong, please tell me.

Comment 25

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8652799 [details]
MozReview Request: Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

Comment 26

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8652799 [details]
MozReview Request: Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes

https://reviewboard.mozilla.org/r/17293/#review15389

I think we should restructure this a bit and push most of the verification logic into nsIX509CertDB. See for example how support was added for signed directories. That way, the verification code is all in once place (which means it's easier to manage and we get the benefit of code re-use).

::: netwerk/protocol/http/nsISignedPackageVerifier.idl:18
(Diff revision 1)
> + *   https://wiki.mozilla.org/User:Ptheriault/Packagedprivilegedcontent

This documentation should probably be in a more permanent location than a wiki user page.

::: netwerk/protocol/http/nsISignedPackageVerifier.idl:22
(Diff revision 1)
> +interface nsISignedPackageVerifier : nsISupports

I'm not sure I understand the need for this new interface. Can't we just extend nsIX509CertDB to provide what's needed?

::: netwerk/protocol/http/nsISignedPackageVerifier.idl:35
(Diff revision 1)
> +   *   - aHashValue should be computed by the caller of this method

I don't think the caller should compute the hash. That's the responsibility of the verification code (i.e. the implementation of this interface).

::: security/apps/AppSignatureVerification.cpp:1029
(Diff revision 1)
> +                              aSignatureStream, aSignerCert);

nit: indentation

::: security/apps/moz.build:39
(Diff revision 1)
> +    ('privileged-package-root.inc', 'privilegedPackageRoot', 'privileged-package-root.der'),

privileged-package-root.der needs to be checked into the tree as well, right?

::: security/manager/ssl/nsIX509CertDB.idl:362
(Diff revision 1)
> +  nsIX509Cert verifySignedManifestSync(in AppTrustedRoot trustedRoot,

I don't think we want a synchronous version of this. Signature verification can take a while, particularly on mobile or old hardware, and we shouldn't block the main thread while that's happening.

Comment 27

[Jonathan Hao (inactive) [:jhao]]

I think the reusable logic is already there in nsIX509CertDB, which is verifySignedManifestAsync. What we need additionally for new security model is to parse the manifest (which is why we created this interface, so that we can parse JSON in JavaScript) and store the resource hashes. It probably would be weired to store the resource hashes in nsIX509CertDB.

After discussion with Henry, we would like to put most of the logic in PackagedAppVerifier.cpp which Henry's currently implementing as described in https://bugzil.la/1178525#c36
I'll rewrite my code in C++ there after Henry lands bug 1178525. However, I'm not sure what's the best way to parse JSON in C++.

David, do you think this is OK?

Comment 28

[Jonathan Hao (inactive) [:jhao]]

I talked to Kan-Ru. It seems that parsing JSON with C++ will be a lot of trouble.

Perhaps we should at least have a JavaScript component that parse the manifest for us.

Comment 29

[Jonathan Hao (inactive) [:jhao]]

Also, if we push the logic into nsIX509CertDB, we'll have to store the resource hashes in nsIX509CertDB.
I think this would be weird because nsIX509CertDB is a service.
What if multiple packages are opened? Do we have to store hases of multiple packages at the same time?

Comment 30

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Ok - I think I understand. The goal is to use nsIX509CertDB just to verify the signature on the manifest and have the other verification logic in necko, which is doing the resource loading, right? I think that's a reasonable approach.

Comment 31

[Jonathan Hao (inactive) [:jhao]]

Attachment: Add an AppTrustedRoot for signed packaged app

Comment 32

[Jonathan Hao (inactive) [:jhao]]

Attachment: Packaged App Utils

This component contains some utility functions implemented in JavaScript to do the verification and integrity check for PackagedAppVerifier.

TODO: change it to asynchronous

Comment 33

[Jonathan Hao (inactive) [:jhao]]

Attachment: Verify signed packages

This patch integrates PackagedAppUtils with PackagedAppVerifier.

Hi Henry, could you check if this matches your design?

Comment 34

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8658679 [details] [diff] [review]
Add an AppTrustedRoot for signed packaged app

Hi David,

This patch contains changes to add an AppTrustedRoot for signed packages. Could you review this patch?

As for the two other patches, though one of them contains changes of nsIX509CertDB, that change will be removed when we modified the verification process to asynchronous. Since they will contain only changes in Necko, I'll ask someone from the Necko team to review them when they're done.

Comment 35

[Henry Chang [:hchang]]

Comment on attachment 8658692 [details] [diff] [review]
Verify signed packages

Review of attachment 8658692 [details] [diff] [review]:
-----------------------------------------------------------------

It looks nice to me. Once we decide 

1) whether to rewrite PackagedAppVerifier in js (which means PackagedAppUtils no longer needed)
2) Sync or async verification (pretty much async according to previous comments)

then we can go further!

Thanks!

Comment 36

[Henry Chang [:hchang]]

Comment on attachment 8658684 [details] [diff] [review]
Packaged App Utils

Review of attachment 8658684 [details] [diff] [review]:
-----------------------------------------------------------------

We probably need another API to parse the "package-identifier" from the manifest or combine in the "verifyManifest" function. If we go for JS version PackagedAppVerifier, then it's not necessary.

Comment 37

[Dana Keeler (she/her) (use needinfo) (:keeler for reviews)]

Comment on attachment 8658679 [details] [diff] [review]
Add an AppTrustedRoot for signed packaged app

Review of attachment 8658679 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, but the certificate's key's public exponent should be something other than 3 (like 65537).

Comment 38

[Jonathan Hao (inactive) [:jhao]]

(In reply to Henry Chang [:henry] from comment #35)
> Comment on attachment 8658692 [details] [diff] [review]
> Verify signed packages
> 
> Review of attachment 8658692 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It looks nice to me. Once we decide 
> 
> 1) whether to rewrite PackagedAppVerifier in js (which means
> PackagedAppUtils no longer needed)

It's up to you, but I don't think you need to rewrite it unless our reviewer from Necko thinks it's necessary.

> 2) Sync or async verification (pretty much async according to previous

Yeah, David said we don't need synchronous version. We can do this first.

> then we can go further!
> 
> Thanks!

Hi Valentin,
Do you think we need to rewrite PackagedAppVerifier in js to avoid separating logic in two components? I implemented PackagedAppUtils in js because we need to parse the manifest in JSON format.

Comment 39

[Valentin Gosu [:valentin] (he/him)]

No, I don't think a rewrite in JS is necessary at this point. We could do it later, if it turns out to be a problem, but I think it will be just fine. Thanks!

Comment 40

[Henry Chang [:hchang]]

(In reply to Valentin Gosu [:valentin] from comment #39)
> No, I don't think a rewrite in JS is necessary at this point. We could do it
> later, if it turns out to be a problem, but I think it will be just fine.
> Thanks!

Great! Let's go for it! I'll provide a patch to change the verification to async way ASAP. Thanks!

Comment 41

[Jonathan Hao (inactive) [:jhao]]

Attachment: Add an AppTrustedRoot for signed packaged app

Change the certificate's key's public exponent to 65537.

Comment 42

[Jonathan Hao (inactive) [:jhao]]

Thanks David!

I've changed the public exponent to 65537.
When we finished the asynchronous version, I'll ask Valentin to do the review, but you're also welcome to check if the usage of nsIX509CertDB methods are correct.

Comment 43

[Jonathan Hao (inactive) [:jhao]]

Attachment: PackagedAppUtils

Hi Valentin,

This patch implements the verification in JavaScript, to be used by PackagedAppVerifier.

Could you review it? Thanks!

Comment 44

[Jonathan Hao (inactive) [:jhao]]

Attachment: Use PackagedAppUtils in PackagedAppVerifier

Hi Valentin,

This patch modifies PackagedAppVerifier and PackagedAppService to make them use PackagedAppUtils to do the verification.

Could you review it? Thanks.

Comment 45

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a2e099e2a9a

Comment 46

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8664036 [details] [diff] [review]
PackagedAppUtils

Review of attachment 8664036 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 47

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8664038 [details] [diff] [review]
Use PackagedAppUtils in PackagedAppVerifier

Review of attachment 8664038 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits fixed.

::: netwerk/protocol/http/PackagedAppService.cpp
@@ +225,5 @@
>  static bool
>  GetOriginalResponseHeader(nsIRequest* aRequest, nsACString& aHeader)
>  {
> +  nsCOMPtr<nsIMultiPartChannel> multiPartChannel(do_QueryInterface(aRequest));
> +  multiPartChannel->GetOriginalResponseHeader(aHeader);

This method probably would not be called if aRequest isn't a nsIMultiPartChannel, but it would be safer to return false if it's null or can't be QI'd to a multipart channel.

::: netwerk/protocol/http/PackagedAppVerifier.cpp
@@ +320,5 @@
>      FireVerifiedEvent(false, true);
>      return;
>    }
>  
> +  uint32_t index = uriAsAscii.Find("!//");

the URI could contain !// in the query part for example, but that doesn't make it a valid packaged app URI.
Check if it's a valid URI like this:
https://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#5006-5010
If the filePath contains !// then you can cut the same way you do below.

Comment 48

[Valentin Gosu [:valentin] (he/him)]

NI just to let Honza know that I reviewed these patches even though I am not a necko peer.

Comment 49

[Jonathan Hao (inactive) [:jhao]]

Attachment: Packaged App Utils

Add r=valentin to commit message.

Comment 50

[Jonathan Hao (inactive) [:jhao]]

Attachment: Use PackagedAppUtils in PackagedAppVerifier

Nits are fixed. Thanks to Valentin!

Comment 51

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8664624 [details] [diff] [review]
Packaged App Utils

r+ given previously by valentin

Comment 52

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8664627 [details] [diff] [review]
Use PackagedAppUtils in PackagedAppVerifier

r+ given previously by valentin

Comment 53

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=02c3d5fbe2d4

Comment 54

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=78cdfbefa98e

Comment 55

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fa59b21618e

Comment 56

[Jonathan Hao (inactive) [:jhao]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7275ecb1b69

Comment 57

[Jonathan Hao (inactive) [:jhao]]

Attachment: Packaged App Utils

Add package-manifest.in to android and b2gdroid to make xpcshell tests passed.

Comment 58

[Jonathan Hao (inactive) [:jhao]]

Attachment: Verify signed packages

Change an nsresult to NS_METHOD to avoid compile error on Windows.

Comment 59

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8665222 [details] [diff] [review]
Packaged App Utils

Previous r+ by Valentin.

Comment 60

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8665223 [details] [diff] [review]
Verify signed packages

Previously r+ by Valentin.

Comment 61

[Jonathan Hao (inactive) [:jhao]]

Try server results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a98d28c9d36

* Android 4.0 API11+ opt tc[Tier-2] also failed on other people's build, so I assume that's not caused by me.
* Aries Engineering Build success after retrigger.
* Windows 7 and 8 have been pending all night, so I'll give up on those tests.

Ready to check in!

Comment 62

[Carsten Book [:Tomcat]]

Hi, when trying to push the first patch i got :

Push rejected because the following IDL interfaces were
remote: modified without changing the UUID:
remote:   - nsIX509CertDB in changeset 22771346ac18
remote: 
remote: To update the UUID for all of the above interfaces and their
remote: descendants, run:
remote:   ./mach update-uuids nsIX509CertDB

could check this ? thanks !

Comment 63

[Jonathan Hao (inactive) [:jhao]]

Attachment: Add an AppTrustedRoot for signed packaged app

Update the uuid of nsIX509CertDB

Comment 64

[Jonathan Hao (inactive) [:jhao]]

Comment on attachment 8665283 [details] [diff] [review]
Add an AppTrustedRoot for signed packaged app

Previously r+ by David.

Comment 65

[Jonathan Hao (inactive) [:jhao]]

Hi Carsten,

Sorry. I've now updated the uuid of nsIX509CertDB.

Please try again. Thanks!

Comment 66

[Honza Bambas (:mayhemer)]

(In reply to Valentin Gosu [:valentin] (Vacation until Oct 5th) from comment #48)
> NI just to let Honza know that I reviewed these patches even though I am not
> a necko peer.

We'll see how it goes ;)  If those are changes to your code mostly, you are ok to review them.  Thanks!

Comment 67

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ed0301097c86
https://hg.mozilla.org/integration/mozilla-inbound/rev/98c837a33821
https://hg.mozilla.org/integration/mozilla-inbound/rev/e014457c7890

Comment 68

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ed0301097c86
https://hg.mozilla.org/mozilla-central/rev/98c837a33821
https://hg.mozilla.org/mozilla-central/rev/e014457c7890

Comment 69

[Carsten Book [:Tomcat]]

landed yesterday on inbound!