Cannot update an preinstalled packaged app (signed or otherwise) to a signed packaged app

RESOLVED FIXED in Firefox 25

Status

defect
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: jsmith, Assigned: amac)

Tracking

Trunk
mozilla25
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(blocking-b2g:leo+, firefox23 wontfix, firefox24 wontfix, firefox25 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Details

(Whiteboard: [LeoVB+])

Attachments

(2 attachments, 1 obsolete attachment)

1.01 allows unsigned packaged apps to update to signed packaged apps, especially in the preinstalled packaged app use case. However, 1.1 now enforces bug 814136, which enforces this requirement on preinstalled packaged apps. This is problematic however if we end up in the case that a user decides to do a FOTA update from 1.01 --> 1.1 without updating their preinstalled packaged apps that were not signed correctly, as doing app updates on any of these apps on 1.1 will fail. This scenario will be a problem for HERE Maps and Firefox Marketplace. There's also some questioning if we should even enforce not allowing updating of unsigned packaged apps to signed packaged apps as well.
Strawman: insist that all preinstalled apps be signed if we intend for them to ever be updated, and race to sign all such preinstalled apps before the release of the first version of the first device.
In the general case, I don't understand why we forbid going unsigned -> signed. I mean, it's probably more secure than doing unsigned -> unsigned anyway, that we allow.
I was going to reopen bug 886814 but then I saw the argument had already taken place. Sleeping is a nasty habit :P.

The problem here isn't just updating a unsigned app to a signed app... Even if we did sign all the preinstalled apps before shipping (which on some cases I don't think it will be possible) a patch will still be needed here. I'm afraid that the current appId setting code is interleaved with the signature verification code, and that does work only for *downloaded* apps, not for preinstalled apps (since preinstalled apps have never needed to be signed).

In any case, I believe there are two possible solutions here. 

1. We can add a ids.json to all preinstalled apps (signing them is optional and counterproductive to testing, IMO) with the correct id and version. But that would mean getting a correct id from the store before shipping, which can or cannot be possible.

2. We can set the id for privileged preinstalled apps to APP_ID_PENDING. And when updating an app, if the previous id is APP_ID_PENDING allow the update, and set the correct id. We can also check, during the normal install flow, if a downloaded app id is APP_ID_PENDING and reject the install in that case (that should not happen since someone would have had to sign it, but better safe than sorry)

I would prefer implementing 2, myself, since I think it gives an equivalent level of security (since you can install APP_ID_PENDING apps only via direct access to the phone) and will impact much less on the current building procedures. I can implement the other one if that's the one preferred though. 

So which one would you prefer?
Assignee: nobody → amac
Summary: Cannot update an unsigned preinstalled packaged app to a signed packaged app → Cannot update an preinstalled packaged app (signed or otherwise) to a signed packaged app
(In reply to Julien Wajsberg [:julienw] from comment #2)
> In the general case, I don't understand why we forbid going unsigned ->
> signed. I mean, it's probably more secure than doing unsigned -> unsigned
> anyway, that we allow.

For the general case, going from unsigned to signed is installing a new app, not updating one. Security wise, I don't think we should allow this because it means changing from a mostly toothless (on a permission sense) app to a more dangerous app via what seems a harmless update.

But currently, this cannot happen anyway, because you cannot serve unsigned apps from a signing-enabled store. If your app was installed from 

https://whateverstore.com/wonderfulapps/myapp.manifest

then either whateverstore.com is a signing-enabled store, or it isn't. If it is, then all the zips it serves have to be signed. So you cannot install an unsigned app from a location and then update it to a signed app from the same location.
Requesting leo?. As per C3, this needs a Gecko patch to work (just signing the external apps won't work), and we should fix this before shipping, one way or another, cause otherwise preinstalled privileged apps won't be updatable.
blocking-b2g: --- → leo?
(In reply to Antonio Manuel Amaya Calvo from comment #4)

> For the general case, going from unsigned to signed is installing a new app,
> not updating one. Security wise, I don't think we should allow this because
> it means changing from a mostly toothless (on a permission sense) app to a
> more dangerous app via what seems a harmless update.

oki I get it and this makes sense, thanks !
On one hand, having signed app means more security (because it's signed) but on the other hand it means also more permissions, I missed that part.
Triage - agree to set leo+
blocking-b2g: leo? → leo+
This implements solution 2, cause it was easier, less change, less risk, and because it has less impact on the build system.
Attachment #767834 - Flags: review?(fabrice)
Hmm I requested review from Fabrice, but he's away till this saturday. Requesting it from Fernando since he's also done extensive work on Webapps.jsm
Attachment #767834 - Flags: review?(fabrice) → review?(ferjmoreno)
Attachment #767834 - Flags: review?(ferjmoreno) → review+
Comment on attachment 767834 [details] [diff] [review]
Implement solution 2 from comment 3

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

::: dom/apps/src/Webapps.jsm
@@ +352,5 @@
>      }
>  
>      app.origin = "app://" + aId;
>  
> +    // Do this for all preinstalled apps... we can't know at this 

Nit: trailing WS, here and the next line.

@@ +2265,5 @@
>      function checkForStoreIdMatch(aStoreId, aStoreVersion) {
>        // Things to check:
>        // 1. if it's a update:
> +      //   a. We should already have this storeId, or the original storeId must start
> +      //      with STORE_ID_PENDING_PREFIX 

Also here.
Keeping r+ from the previous version, r=ferjm
Attachment #767834 - Attachment is obsolete: true
Attachment #770400 - Flags: review+
This is the patch adapted to the b2g18 tree.
Attachment #770405 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/1d8be4df9c73
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Keywords: verifyme
QA Contact: jsmith
Verified on 7/7 b2g18 build by confirming a successful app update for the marketplace preinstalled packaged app.
Depends on: 895708
Whiteboard: [LeoVB+]
No longer depends on: 895708
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.