Last Comment Bug 839723 - Validator should not allow developers to submit certified permissions
: Validator should not allow developers to submit certified permissions
Status: RESOLVED FIXED
:
Product: Marketplace
Classification: Server Software
Component: Validation (show other bugs)
: 1.0
: x86 Mac OS X
: -- normal (vote)
: 2013-02-14
Assigned To: Matt Basta [:basta]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-08 17:25 PST by Lisa Brewster [:adora]
Modified: 2013-02-13 15:08 PST (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments

Description Lisa Brewster [:adora] 2013-02-08 17:25:32 PST
Developers should be prevented from submitting apps that request certified permissions.

Example:  https://marketplace.firefox.com/reviewers/apps/review/jaxogram

"permissions": {
"systemXHR": { "description": "Access Jaxo's app server and OAuth" },
"browser":{ "description": "OAuth browser window" },
"contacts": { "description": "Import Orkut friends as contacts", "access": "readcreate" },
"device-storage:pictures": { "description": "Share photos", "access": "readcreate" },
"geolocation": { "description": "Geotag photos" },
"storage": { "description": "Use local storage for access tokens" },
"camera": { "description": "Share photos" }
}

This app's type isn't certified, but the developer needs to use web activities for camera access instead of requesting access directly.
Comment 1 Matt Basta [:basta] 2013-02-09 12:54:32 PST
The problem here is that there are multiple lists of permissions and it seems that the permissions change when nobody's looking. What is the canonical, stable list of certified permissions?

Or better yet, we could whitelist the (much shorter) list of permissions that developers *can* use in their apps.
Comment 2 Jason Smith [:jsmith] 2013-02-09 14:28:07 PST
Paul could probably point you in the right direction here. I can't access his perms matrix doc for some reason right now, so I don't have the list memorized.
Comment 3 Paul Theriault [:pauljt] 2013-02-10 12:40:01 PST
The permissions matrix is documented here:
https://docs.google.com/spreadsheet/ccc?key=0Akyz_Bqjgf5pdENVekxYRjBTX0dCXzItMnRyUU1RQ0E#gid=0

Every effort is being taken to keep this accurate and in sync with the implementation of the matrix which is here:
https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/PermissionsTable.jsm

Note that at the time of this post, a last minute permission has been added to gecko by bug 838308, so the "keyboard" permission isn't in the code yet, but it will be shortly.

Also note that security doesn't depend on the marketplace doing this check (but I can see why you would want to do it for usability reasons). 

An app can request any certified permission when it is installed, it just wont get it. (or more specifically, it will get the permission setting as spelled out in PermissionTable.jsm, so if this is a certified-only permission, it will get an entry set to Ci.nsIPermissionManager.DENY_ACTION)

If you do want to check a whitelist, then that should be easy to calculate from filtering the spreadsheet. Just be aware that this whitelist will need to be regularly updated as these APIs evolve.
Comment 4 Andrew Williamson [:eviljeff] 2013-02-11 02:35:28 PST
(In reply to Matt Basta [:basta] from comment #1)
> The problem here is that there are multiple lists of permissions and it
> seems that the permissions change when nobody's looking. What is the
> canonical, stable list of certified permissions?
> 
> Or better yet, we could whitelist the (much shorter) list of permissions
> that developers *can* use in their apps.

the whitelist is the ideal solution - I've seen some manifests where it the developer has misread the spec/MDN/whatever.  We'd need a separate whitelist for privileged and unprivileged apps.
Comment 6 Jason Smith [:jsmith] 2013-02-13 14:57:22 PST
Even quickly looking at the patch there, this doesn't look right. You forgot about contacts.

https://docs.google.com/spreadsheet/ccc?key=0Akyz_Bqjgf5pdENVekxYRjBTX0dCXzItMnRyUU1RQ0E#gid=0

Note You need to log in before you can comment on or make changes to this bug.