For example, the mini-manifest might have name: "Gmail", which would cause us to prompt the user asking if she wants to install "Gmail." But the downloaded package might actually be for an app *not* named GMail; if so, then the user did not really consent to install that app. Consequently, we need to verify that all of the fields of the mini-manifest (or are least the ones we use) are present in the signed manifest file, and that each value of each property in the mini-manifest exactly matches the corresponding value in the signed manifest file.

Going to generalize this - this applies also to a non-signed packaged app. When this issue occurs, we probably should throw a DOMError back.

Stealing since Brian is busy with other bugs. In this patch we check that 2 manifests are similar by making sure that: - they have the same set of locales - for each locale and for the default root, that the application name and developer information are the same.

Comment on attachment 693070 [details] [diff] [review] patch Review of attachment 693070 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/AppsUtils.jsm @@ +284,5 @@ > + for (let locale in aManifest2.locales) { > + locales2.push(locale); > + } > + } > + if (locales1.sort().join() !== locales2.sort().join()) { If I am not wrong, you can get rid of 'locales1' and 'locales2' and have this condition as: |if (aManifest1.slice().sort().join() !== aManifest2.slice().sort().join) { ... }| I didn't test it that way though.

As the code is right now, it is far from obvious that we're checking all the fields that we actually need to match. I suggest that after we parse the mini manifest, we strip out every field that isn't checked by this function, and pass the stripped version on to the later layers. The function that does the stripping should be next to the function that does these checks. This would ensure that we never forget to check consistency of any new fields we decide to rely on from the mini manifest.

I though about that during the revision, but after reading again "or at least the ones we use" in the bug description I thought that the attached patch would be enough for v1 since we are only using the fields that are being checked in the patch [1]. In any case, you suggestion sounds reasonable to me. [1] http://mxr.mozilla.org/gaia/source/apps/system/js/app_install_manager.js#93

There are valid fields that we display from the mini-manifest that are not in the app manifest, like the package size. So I'm not 100% convinced by this approach.

(In reply to Fabrice Desré [:fabrice] from comment #6) > There are valid fields that we display from the mini-manifest that are not > in the app manifest, like the package size. So I'm not 100% convinced by > this approach. For size, we should just verify that the size of the JAR matches the size in the mini-manifest. The ultimate goal is to be able to say "it's safe to present the information in the mini-manifest to the user even though it isn't signed because of X and Y." In this case, X is "because we verified that it matches the information in the app manifest signed within the JAR" and Y is "because we verified that the size in the mini-manifest matches the actual size of the JAR."

I don't think we need to verify that the size information is correct at all. By the time we have downloaded the package, the size information is no longer important and is never going to be exposed to the user. The size information is only used for displaying UI to the user before the download is started.

(In reply to Jonas Sicking (:sicking) from comment #8) > I don't think we need to verify that the size information is correct at all. > > By the time we have downloaded the package, the size information is no > longer important and is never going to be exposed to the user. The size > information is only used for displaying UI to the user before the download > is started. We also use the size information to check that the device has enough free space to store the app.

But doesn't that also happen before the download?

Yes

Cool, so sounds like there's no need to do any verification of the size property in the mini-manifest.

https://hg.mozilla.org/releases/mozilla-aurora/rev/80bb6b8d8359 https://hg.mozilla.org/releases/mozilla-b2g18/rev/8559edffe139

A couple of sanity checks on mistakes on my behalf building packaged apps has definitely revealed that the checks here are indeed working. Marking as verified.