Closed
Bug 768868
Opened 12 years ago
Closed 12 years ago
App manifest should support application type
Categories
(Core Graveyard :: DOM: Apps, defect, P1)
Core Graveyard
DOM: Apps
Tracking
(blocking-basecamp:+)
People
(Reporter: ladamski, Assigned: fabrice)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
The application type should be included in the manifest. The three types of applications are "Web Installed" (formerly called "Untrusted"), "Trusted" and "Certified".
This is necessary to determine how the manifest and application should be verified, and which security model to apply at runtime.
Reporter | ||
Updated•12 years ago
|
Comment 1•12 years ago
|
||
Lucas - Is the specification of untrusted vs. trusted vs. certified apps a requirement for basecamp? kilimanjaro? I know I saw something about security and permissions model, but wanted to confirm. If it is, we should nominate this as such.
Reporter | ||
Comment 2•12 years ago
|
||
Sorry, yes it it, I meant to nominate it. We need certified apps for B2G at minimum, and we should also support trusted for a number of important use cases.
blocking-basecamp: --- → ?
Updated•12 years ago
|
blocking-kilimanjaro: --- → ?
Comment 3•12 years ago
|
||
Should this be a type of the application, or should the type simply be inferred by some other claim? For instance, a certified app I assume needs to refer to some formal certification. So if it includes, say, "trust_certificate": "/app-trust.cert" (making up a manifest syntax) then it can be inferred it is a certified application.
Reporter | ||
Comment 4•12 years ago
|
||
We could possibly infer that between web installed and trusted or certified, but not between trusted and certified. Having an explicit field would help reviewers or anyone else to quickly understand the expected behavior and potential risk of the feature.
blocking-basecamp: ? → +
Reporter | ||
Updated•12 years ago
|
Assignee: ladamski → anant
Comment 5•12 years ago
|
||
Updated spec: https://github.com/mozilla/webapps-spec/commit/40946eea5a1b43356966fb86ab92f3a47112c00d
We should leave this bug open to track enforcement in various WebRTs, and the marketplace validator (unless there are already open bugs for that).
Updated•12 years ago
|
blocking-kilimanjaro: ? → ---
Updated•12 years ago
|
Component: General → DOM: Mozilla Extensions
OS: Mac OS X → All
Product: Web Apps → Core
Hardware: x86 → All
Comment 6•12 years ago
|
||
Moved to Core --> DOM:Mozilla Extensions for the mozapps API portion of the implementation. Followup questions:
- Who could implement this?
- What front-end implementation changes do we need (Desktop, Android, Firefox OS)?
Updated•12 years ago
|
Updated•12 years ago
|
Assignee: nobody → anant
Updated•12 years ago
|
Assignee: anant → fabrice
Assignee | ||
Comment 8•12 years ago
|
||
Actually I think we don't need anything in the manifest, at least for v1.
- Certified apps will only be the pre-installed ones, and we can identify them easily.
- Packaged apps with a valid signature will be privileged
- Unsigned packaged apps and hosted app will just be "installed"
I opsted a patch in Bug 781620 to support what we can since we don't have yet signature support for packaged apps.
Comment 9•12 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #8)
> Actually I think we don't need anything in the manifest, at least for v1.
> - Certified apps will only be the pre-installed ones, and we can identify
> them easily.
> - Packaged apps with a valid signature will be privileged
> - Unsigned packaged apps and hosted app will just be "installed"
>
> I opsted a patch in Bug 781620 to support what we can since we don't have
> yet signature support for packaged apps.
Good point. Renoming.
blocking-basecamp: + → ?
Updated•12 years ago
|
blocking-basecamp: ? → +
Comment 10•12 years ago
|
||
Andrew - Why did we re-plus this? Maybe I missed the part in triage about why we made this call.
Comment 11•12 years ago
|
||
I thought this was the thing Jonas said we need done or we won't be able to make phone calls.
Comment 12•12 years ago
|
||
(In reply to Andrew Overholt [:overholt] from comment #11)
> I thought this was the thing Jonas said we need done or we won't be able to
> make phone calls.
Oh. I thought that was a different bug. Jonas - Can you clarify?
Reporter | ||
Comment 13•12 years ago
|
||
From a security standpoint its usually better to be explicit than infer. It helps app reviewers or anyone else looking at the manifest to understand the expected app behavior, also I think it would help developers with testing. Since a developer can't sign a test app with a valid store key, how would the device know to treat that app as a privileged app? I think a better workflow is, a) developer sets phone to developer mode (insert magic here), b) developer marks their app as a privileged app in manifest, c) developer tests app installation and runtime effectively.
So I'd rather have an explicit field that we test against our inferences, rather than just relying on the inference.
Assignee | ||
Comment 14•12 years ago
|
||
Lucas, your flow looks ok once we have this magic "dev mode". In normal mode, what should be the behavior when a manifest requests a privilege level higher than the one we infer? Do we:
- lower the privilege level ?
- reject the app installation ?
Reporter | ||
Comment 15•12 years ago
|
||
I think we should reject the installation. Its better to fail explicitly than degrade implicitly when security contracts fail.
Assignee | ||
Updated•12 years ago
|
Depends on: sign-packaged-apps
Assignee | ||
Comment 16•12 years ago
|
||
For hosted apps we enforce them to only have "web" level privileges. For packaged apps they also currently can only have "web" privileges, but this patch has the pieces needed once we'll have signature support.
In all cases we default to "web" if no type is given in the manifest.
Attachment #653600 -
Flags: review?(21)
Updated•12 years ago
|
Blocks: Apps-Dev-Doc-Needed
Keywords: dev-doc-needed
Assignee | ||
Comment 17•12 years ago
|
||
I added a pref to let developers override the privilege level granted to their app.
Attachment #653600 -
Attachment is obsolete: true
Attachment #653600 -
Flags: review?(21)
Attachment #653912 -
Flags: review?(21)
Assignee | ||
Comment 18•12 years ago
|
||
v3, in which I fix a typo and add dev-mode support to hosted apps.
Attachment #653912 -
Attachment is obsolete: true
Attachment #653912 -
Flags: review?(21)
Attachment #654041 -
Flags: review?(21)
Comment 19•12 years ago
|
||
Comment on attachment 654041 [details] [diff] [review]
patch v3
Review of attachment 654041 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/apps/src/Webapps.jsm
@@ +434,5 @@
> }).bind(this));
> },
>
> installPackage: function(aData) {
> + dump("XxXxX installPackage " + JSON.stringify(aData) + "\n");
leftover?
Attachment #654041 -
Flags: review?(21) → review+
Assignee | ||
Comment 20•12 years ago
|
||
Updated•12 years ago
|
QA Contact: jsmith
Comment 21•12 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/118cc431d56f - leaking in mochitest-2, failing in test_principal_extendedorigin_appid_appstatus.html.
Assignee | ||
Comment 22•12 years ago
|
||
Updated•12 years ago
|
Whiteboard: [qa+]
Comment 23•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Comment 24•12 years ago
|
||
Test Cases to be used:
- Install a hosted app without type specified
- Install a hosted app with web specified
- Install a hosted app with privileged specified
- Install a hosted app with an invalid type specified
- Install a packaged app with web specified
- Install a packaged app with privileged specified
- Install a packaged app with certified specified
Comment 25•12 years ago
|
||
The hosted app test cases pass with flying colors. The packaged app test cases are blocked are blocked on bug 788872.
Keywords: verifyme
Whiteboard: [qa verification blocked]
Comment 26•12 years ago
|
||
Install packaged app with web specified - pass
Install packaged app with privileged specified - fail
Install packaged app with certified specified - fail
Followup bug will be filed for the two failures.
Assignee | ||
Comment 27•12 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #26)
> Install packaged app with web specified - pass
> Install packaged app with privileged specified - fail
> Install packaged app with certified specified - fail
>
> Followup bug will be filed for the two failures.
I'm not sure what you are testing, but just putting "privileged" or "certified" in the manifest is not enough to grant you this status (unless you set to true the dom.mozApps.dev_mode preference).
Comment 28•12 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #27)
> (In reply to Jason Smith [:jsmith] from comment #26)
> > Install packaged app with web specified - pass
> > Install packaged app with privileged specified - fail
> > Install packaged app with certified specified - fail
> >
> > Followup bug will be filed for the two failures.
>
> I'm not sure what you are testing, but just putting "privileged" or
> "certified" in the manifest is not enough to grant you this status (unless
> you set to true the dom.mozApps.dev_mode preference).
Sorry for the lack of description (should have provided more details).
I should have said:
I was able to install a packaged app with the privileged and certified app type, but that shouldn't have been allowed. Privileged apps right now require that the packaged app be signed as well. Certified apps shouldn't even be installable from the web via the installPackage function.
Comment 29•12 years ago
|
||
Oh, https://bugzilla.mozilla.org/show_bug.cgi?id=791318#c2 probably clarified what I saw in my testing then.
Assignee | ||
Comment 30•12 years ago
|
||
> I should have said:
>
> I was able to install a packaged app with the privileged and certified app
> type, but that shouldn't have been allowed. Privileged apps right now
> require that the packaged app be signed as well. Certified apps shouldn't
> even be installable from the web via the installPackage function.
I disagree. There's no reason to prevent that as far as we don't grant elevated privileges to the app. What you say is that we should blame users for authors mistakes.
Comment 31•12 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #30)
> > I should have said:
> >
> > I was able to install a packaged app with the privileged and certified app
> > type, but that shouldn't have been allowed. Privileged apps right now
> > require that the packaged app be signed as well. Certified apps shouldn't
> > even be installable from the web via the installPackage function.
>
> I disagree. There's no reason to prevent that as far as we don't grant
> elevated privileges to the app. What you say is that we should blame users
> for authors mistakes.
Hmm...I can see your perspective as well in that you could install the packaged app and only use functionality within the "web" privileges context. Although, the opposite perspective could see that as risking ending up in scenarios where you can install partially-functional apps. I could go either way on this one, although as long as we don't give escalated privileges to an app that does not have the right to have those privileges, than that's the important piece.
Jonas - Can you give a third opinion here?
Updated•12 years ago
|
Whiteboard: [qa verification failed] → [blocked-on-input Jonas Sicking]
Hmm.. I don't feel super strongly here. I could see allowing installing these apps, but putting up a warning to the user that the app might not be working correctly because some aspects have been disabled for security reasons. But I don't think it's a priority to build such a warning in v1.
That leaves us with the option of installing something that might not work very well, or preventing the installation of something that might work ok.
I was initially going to say that we might as well allow the installation for now and in v2 add the warning above.
However, I realized that running a privileged app in non-privileged mode has a dramatic effect on the app in addition to the lacked permission. Running the app in a non-privileged mode means that we won't be applying the CSP policy to it. This can make the app easier to hack, or flat out make it easy to hack since the author didn't bother adding certain checks since it wasn't needed with the CSP policy.
This is very fixable. We could ensure that an app marked as privileged but installed from an untrusted source doesn't get any privileged permissions, but does get a CSP policy.
I would say that we should either disable the ability to install privileged apps as unprivileged apps, *or* ensure that they still get the appropriate CSP policy applied.
Comment 33•12 years ago
|
||
Marking as verified since I think what's here works with one followup based on Jonas's response.
I dropped a comment on bug 768029 to see how much work it would take to extend our CSP policy support to an untrusted app in the scenario in comment 32. Based on that, we'll know what followup to take probably.
Status: RESOLVED → VERIFIED
Whiteboard: [blocked-on-input Jonas Sicking]
Reporter | ||
Comment 34•12 years ago
|
||
Per comments in thread I don't think we should allow apps to downgrade and still install. When an app is supposed to be privileged yet fails verification, the install process should fail.
Comment 35•12 years ago
|
||
Added documentation for type property to this MDN page:
https://developer.mozilla.org/en-US/docs/Apps/Manifest#Fields
Keywords: dev-doc-needed → dev-doc-complete
Updated•7 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•