Geolocation should require a manifest entry for all apps, like all other webapi permissions. If that is not present, the request should be denied. Yes this is a different behavior from web content but we made an explicit choice early on to go down this route and chose explicit enumeration for all permissions, even if that's different from web content.

I agree it would be cleaner that way... And exceptions are the good intentions that pave the road to hell.

Except that fixing this bug is actually what would introduce the exception. Right now the rules we have are: * Apps can do anything that normal webpages can do * If they want to do more, they need to enumerate that in the manifest What this bug proposes is that we'd by default forbid apps from doing something that normal webpages can do. We can certainly do that, but we need more details: * In which contexts of an app that hasn't asked for geolocation do we forbid it from being used? If the firefox app doesn't ask for geolocation, should the webpages opened by it be able to? What about if another app doesn't ask for geolocation, should identity-provider website like facebook connect opened through window.open be able to use geolocation? * Which other features should we lock down? desktop notifiactions? IndexedDB? Orientation events? cookies? (the last one is obviously no, but the other ones seem less obvious).

Lucas, as per Jonas' comment 2, it sounds like there may be a substantial amount of work to review all of the APIs that are exposed to apps and change the default behaviour. I'm inclined to go with the current behaviour in v1. However, I do remember you discussing how we will be locked in to this behaviour going forward if we don't make this change in v1. Is this correct? Can you provide more justification for this work at this point in the v1 schedule?

Just for the record, this bug makes me very sad. :) The fundamental concept is that it should be able to determine the security properties of an app by inspecting the manifest. This is important for reviewers to be able to focus their efforts on what matters. With geolocation permissions for example, we are now saying "this app can request geolocation if the appropriate permission is enumerated in the manifest, and it can also request it if its NOT enumerated in the manifest." So what--if anything--does the geolocation permission mean for non-certified apps? With either approach the app could theoretically cooperate with 3rd parties (web content or other apps via web activities) and obtain this data anyway, but I think that's better handled through clear app review guidelines and marketplace ToS. To address Lawrence's question, the amount of effort to determine the scope of changes is not hard to quantify; we would just need to enumerate the which manifest permissions map to existing permissions available to web content, which is a finite and well defined set (to answer Jonas's question, we don't need to worry about APIs that don't have equivalent manifest permissions). The effort would mostly included updating some Gaia apps to add the appropriate permissions in the manifest. From a quick scan, the permissions in question would be: geolocation, fmradio & desktop-notification "storage" doesn't apply I think because that stands for unlimited storage, not storage in general (and so is not available to web content), and is implicit for all apps. Likewise "systemXHR" is not the same thing as regular XHR.

I don't think "we parse the property from the manifest" is a good reason to require something to be enumerated in the manifest. The reason we have "geolocation" in the manifest at all is so that the camera app can do geotagging without a permission prompt being displayed. If it wasn't for that we could remove "geolocation" support from manifests completely. That doesn't seem like a good reason to change what all other apps that want to use geolocation needs to put in their manifest. I could buy that there are good reasons to require geolocation in the manifest, but I don't think that is a good argument. I'd rather see us come up with a policy based security and/or privacy which determines what we require apps to put in the manifest. Also, I don't think that it makes sense to allow <iframe>s to access geolocation if the main application can't and rely on reviews to catch apps working around our intents. This is for three reasons: * A big reason for enumerating permissions in the manifest is so that we can see that when an app intends to use a specific capability, we want to make sure that the application doesn't use that capability for evil (intentionally or not). What seems to be proposed here would require the opposite. That when geolocation is *not* enumerated in the manifest we should review the code to make sure that it doesn't get geolocation anyway. * It would be impossible for us to review for applications working around geolocation policies using iframes since we can't review the contents of the iframe as it's just pointing to an arbitrary website. For example a camera application might have very good reasons to include an <iframe> and take information from that iframe and add to the created image file. For example an iframe might analyze the contents of the image and do face recognition and append metadata about the people in the picture. We have no way to ensure that the iframe wouldn't also use the geolocation API and append geotags to the image file. * We are not just talking about applications that are reviewed here. There are both packaged applications that are installed directly from websites as well as hosted applications which have the ability to change their manifest at will at any time. So to sum up, I'm not opposed to requiring geolocation having to be enumerated in application manifests, but I think that A) We need a better policy for what is required to be enumerated in the manifest than "the manifest parser can consume it". I.e. I think we should have a policy based on privacy or security. B) If a permission isn't enumerated in the manifest, then <iframe>s, including <iframe mozbrowser>s, and windows opened by the app should also not have the permission to use geolocation. I.e. the browser app should be required to enumerate geolocation in order to allow webpages inside it to use the geolocation API.

Disclaimer: much of this is for posterity so forgive my preaching. I think our list of permissions is not arbitrary, permissions are in the manifest because we believe they have meaningful security and privacy implications and should be centrally inspected and controlled. How we control them is important: it makes for a very confusing model if some permissions in the manifest are enforced, while others are ignored (and under rather arbitrary rules based upon legacy behavior). The goal of the app permission model is to have an internally consistent model for controlling what APIs apps can obtain direct access to. It does not--and cannot--prevent apps from trying to obtain access to the data that would come from that API via other means (such as cooperating with other apps and content, directly via activities or indirectly via servers). I realize we have a set of APIs that predate this permission model, and I believe we had some very explicit discussions that these APIs should fall under the app permission model when called from an app, and not under the legacy browser model. I'm totally fine with your proposal that if a permission is denied for an app then it should also be denied for any 3rd party content that it contains. It does mean that app will require a superset of all possible permissions for all content they may ever display, but that may actually be desirable since that provides a mechanism for the app to suppress 3rd party permission prompts that are unnecessary for its intended purpose.

Marking bb+ per triage conversations in this bug. Also filing a separate bug to track enforcement of app permission restrictions on embedded non-app content. As mentioned above, I believe this really would only immediately impact 3 existing permissions per https://docs.google.com/a/mozilla.com/spreadsheet/ccc?key=0Akyz_Bqjgf5pdENVekxYRjBTX0dCXzItMnRyUU1RQ0E#gid=0: - desktop-notification - fmradio - geolocation ... but I would love to have someone sanity check that.

I have mixed feelings on going forward with this, given how close we are to a target deadline. Our app developers currently do not have an understanding that they need to specify the "geolocation" in the "permissions" property if they have a hosted app (very common on apps using geolocation on marketplace). I do understand Lucas's motivations of why we want this (the manifest being a trustworthy piece of data to understand what permissions this app needs), but I'm wondering if it's too late in v1 to be able to evangelize this change. Then again, if we go out the door without this fixed in v1, it's going to be far harder to evangelize in the future.

Reworking title. This bug *only* reproduces with geolocation. I am not getting a reproduction of this with desktop-notification or fmradio. Feel free to add more checks in to make sure those two work at a second-level though.

What's actually happening with this bug - is a change going to be made (in v1) to require geolocation in the manifest? Its not clear if there is a consensus.

(In reply to Andrew Williamson [:eviljeff] from comment #11) > What's actually happening with this bug - is a change going to be made (in > v1) to require geolocation in the manifest? Its not clear if there is a > consensus. Yes.

Note: Desktop notifications currently broken per bug 824421; fix /may/ reintroduce the issue for desktop notifications.

Is anyone working on this? How do we proceed?

I believe this is all that's needed to remove geolocation exception treatment so the prompt is only shown if the permission is actually on the manifest.

(In reply to Kan-Ru Chen [:kanru] from comment #14) > Is anyone working on this? How do we proceed? I am currently working on it. Antonio that's not the right fix. We have to create a new pricipal here for comparison.

(In reply to Gregor Wagner [:gwagner] from comment #16) > (In reply to Kan-Ru Chen [:kanru] from comment #14) > > Is anyone working on this? How do we proceed? > > I am currently working on it. > Antonio that's not the right fix. We have to create a new pricipal here for > comparison. Hmm... so you can get a distinct prompt for all the different hosted content inside the same app (so each origin on the browser app gets its own permission stored as if it were an app)? That'll be handy but not exactly on the bug description, my bad :). Reverting it to you since you're on it... if you're busy otherwise feel free to reassign it to me.

Comment on attachment 700351 [details] [diff] [review] patch Review of attachment 700351 [details] [diff] [review]: ----------------------------------------------------------------- Needs some updating.

This patch has been reviewed offline :)

https://hg.mozilla.org/integration/mozilla-inbound/rev/f17992358a63 https://hg.mozilla.org/releases/mozilla-b2g18/rev/b626a918b9e4

Any chance of getting this applied to Firefox20 (Aurora), for Android at least? It makes things a whole lot easier if permissions work in a similar way for all mobile platforms.

Andrew, that would be a separate bug for Android support.

Sanity test looks okay - hosted apps aren't getting geolocation by default anymore.

The Android team is working on this in bug 832960, but we have some questions: * Can anyone explain why this only affects geolocation and desktop-notification? * Can anyone explain why the fix is in ContentPremissionPrompt.js (where the prompt is requested) and not in PermissionsInstaller.installPermissions (where the manifest is scanned and permissions are set for the app)?

(In reply to Mark Finkle (:mfinkle) from comment #26) > The Android team is working on this in bug 832960, but we have some > questions: > * Can anyone explain why this only affects geolocation and > desktop-notification? Geolocation and desktop notification WebAPIs happen to be WebAPIs supported by the web content in the browser with prompting for use. So what this patch is enforcing is that even if WebAPIs we support in the browser web content are accessed, we still require that the permission be specified in the app manifest for that particular WebAPI. This particular use case will also be present with any particular WebAPI in the future that happens to be prompting for access (e.g. getUserMedia will have this same problem).

(In reply to Mark Finkle (:mfinkle) from comment #26) > The Android team is working on this in bug 832960, but we have some > questions: > * Can anyone explain why this only affects geolocation and > desktop-notification? These are the two features which bring up security dialogs. (Arguably so does appcache and IDB, but we're working on getting rid of those). > * Can anyone explain why the fix is in ContentPremissionPrompt.js (where the > prompt is requested) and not in PermissionsInstaller.installPermissions > (where the manifest is scanned and permissions are set for the app)? It's in both, no? I.e. the PermissionsInstaller is driven by the table in PermissionsTable.jsm, which contains these two permissions. IIRC (which I might not), the prompt code needed to be changed such that if origin X within app Y asks to use geolocation, we need to check that the "main origin" of Y doesn't contain UNKNOWN. This since the PermissionsInstaller installed the permission on the "main origin" of Y.