Closed Bug 1178518 Opened 4 years ago Closed 4 years ago

Support for verifying signed packages

Categories

(Core :: Networking, defect, P2)

defect

Tracking

()

RESOLVED FIXED
FxOS-S8 (02Oct)
blocking-b2g 2.5+
Tracking Status
firefox44 --- fixed

People

(Reporter: pauljt, Assigned: jhao)

References

Details

Attachments

(3 files, 17 obsolete files)

19.43 KB, patch
jhao
: review+
Details | Diff | Splinter Review
13.73 KB, patch
jhao
: review+
Details | Diff | Splinter Review
7.25 KB, patch
jhao
: review+
Details | Diff | Splinter Review
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.
Assignee: nobody → jhao
Status: NEW → ASSIGNED
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.
(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
> > 
> > 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.
(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!
Priority: -- → P1
Hi Henry,

Is this interface suitable for your usage?
Attachment #8641551 - Flags: feedback?(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!
Attachment #8641551 - Flags: feedback?(hchang) → feedback+
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?
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?
(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
Added comments. They're just as Henry described.
Attachment #8641551 - Attachment is obsolete: true
blocking-b2g: --- → 2.5+
Target Milestone: --- → FxOS-S5 (21Aug)
Attached patch Signed Package Verifier (WIP) (obsolete) — Splinter Review
Added implementation of verifyManifest, but it needs testing.
Attachment #8642304 - Attachment is obsolete: true
Attached patch Signed Package Verifier (WIP) (obsolete) — Splinter Review
Added implementation of checkIntegrity()
Attachment #8646134 - Attachment is obsolete: true
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
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?
Flags: needinfo?(jhao)
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.
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.
Flags: needinfo?(jhao)
Attached patch Signed Package Verifier (WIP) (obsolete) — Splinter Review
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
Attachment #8647371 - Attachment is obsolete: true
Attached patch SIgned Package Verifier (WIP) (obsolete) — Splinter Review
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.
Attachment #8650847 - Attachment is obsolete: true
Attached patch SIgned Package Verifier (obsolete) — Splinter Review
Hi David and Richard,

Could you review my patch?

Thanks.
Attachment #8651729 - Attachment is obsolete: true
Attachment #8652241 - Flags: review?(rlb)
Attachment #8652241 - Flags: review?(dkeeler)
It seems you forgot to include test_signed_package_verifier.js in this last version.
(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
Attached patch SIgned Package Verifier (obsolete) — Splinter Review
Oops, adding missing files.
Attachment #8652241 - Attachment is obsolete: true
Attachment #8652241 - Flags: review?(rlb)
Attachment #8652241 - Flags: review?(dkeeler)
Attachment #8652675 - Flags: review?(rlb)
Attachment #8652675 - Flags: review?(dkeeler)
Bug 1178518 - Support for verifying signed packages; r?dkeeler, rbarnes
Attachment #8652799 - Flags: review?(rlb)
Attachment #8652675 - Attachment is obsolete: true
Attachment #8652675 - Flags: review?(rlb)
Attachment #8652675 - Flags: review?(dkeeler)
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.
Attachment #8652799 - Flags: review?(dkeeler)
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 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.
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?
Flags: needinfo?(dkeeler)
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.
Depends on: 1178525
No longer depends on: 1170823
Depends on: 1170823
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?
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.
Flags: needinfo?(dkeeler)
Depends on: 1198669
Priority: P1 → P2
Attached patch Packaged App Utils (obsolete) — Splinter Review
This component contains some utility functions implemented in JavaScript to do the verification and integrity check for PackagedAppVerifier.

TODO: change it to asynchronous
Attached patch Verify signed packages (obsolete) — Splinter Review
This patch integrates PackagedAppUtils with PackagedAppVerifier.

Hi Henry, could you check if this matches your design?
Attachment #8658692 - Flags: feedback?(hchang)
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.
Attachment #8658679 - Flags: review?(dkeeler)
Attachment #8652799 - Attachment is obsolete: true
Attachment #8652799 - Flags: review?(rlb)
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!
Attachment #8658692 - Flags: feedback?(hchang) → feedback+
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 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).
Attachment #8658679 - Flags: review?(dkeeler) → review+
(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.
Flags: needinfo?(valentin.gosu)
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!
Flags: needinfo?(valentin.gosu)
(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!
Change the certificate's key's public exponent to 65537.
Attachment #8658679 - Attachment is obsolete: true
Attachment #8659125 - Flags: review+
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.
Depends on: 1205228
Attached patch PackagedAppUtils (obsolete) — Splinter Review
Hi Valentin,

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

Could you review it? Thanks!
Attachment #8658684 - Attachment is obsolete: true
Attachment #8664036 - Flags: review?(valentin.gosu)
Hi Valentin,

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

Could you review it? Thanks.
Attachment #8658692 - Attachment is obsolete: true
Attachment #8664038 - Flags: review?(valentin.gosu)
Comment on attachment 8664036 [details] [diff] [review]
PackagedAppUtils

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

Looks good to me.
Attachment #8664036 - Flags: review?(valentin.gosu) → review+
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.
Attachment #8664038 - Flags: review?(valentin.gosu) → review+
NI just to let Honza know that I reviewed these patches even though I am not a necko peer.
Flags: needinfo?(honzab.moz)
Attached patch Packaged App Utils (obsolete) — Splinter Review
Add r=valentin to commit message.
Attachment #8664036 - Attachment is obsolete: true
Nits are fixed. Thanks to Valentin!
Attachment #8664038 - Attachment is obsolete: true
Comment on attachment 8664624 [details] [diff] [review]
Packaged App Utils

r+ given previously by valentin
Attachment #8664624 - Flags: review+
Comment on attachment 8664627 [details] [diff] [review]
Use PackagedAppUtils in PackagedAppVerifier

r+ given previously by valentin
Attachment #8664627 - Flags: review+
Add package-manifest.in to android and b2gdroid to make xpcshell tests passed.
Attachment #8664624 - Attachment is obsolete: true
Change an nsresult to NS_METHOD to avoid compile error on Windows.
Attachment #8664627 - Attachment is obsolete: true
Comment on attachment 8665222 [details] [diff] [review]
Packaged App Utils

Previous r+ by Valentin.
Attachment #8665222 - Flags: review+
Comment on attachment 8665223 [details] [diff] [review]
Verify signed packages

Previously r+ by Valentin.
Attachment #8665223 - Flags: review+
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!
Keywords: checkin-needed
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 !
Flags: needinfo?(jhao)
Keywords: checkin-needed
Update the uuid of nsIX509CertDB
Attachment #8659125 - Attachment is obsolete: true
Comment on attachment 8665283 [details] [diff] [review]
Add an AppTrustedRoot for signed packaged app

Previously r+ by David.
Flags: needinfo?(jhao)
Attachment #8665283 - Flags: review+
Hi Carsten,

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

Please try again. Thanks!
Flags: needinfo?(cbook)
Keywords: checkin-needed
(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!
Flags: needinfo?(honzab.moz)
landed yesterday on inbound!
Flags: needinfo?(cbook)
You need to log in before you can comment on or make changes to this bug.