Re: Intent to migrate the permissions database to use origins instead of host names
I tried to get some folks internally to look at this, and so far nobody has said that they oppose landing the changes as is very strongly so unless there is no strong objections, I am going to ask Michael to submit his work for landing tomorrow. Cheers, Ehsan On Wed, Jul 1, 2015 at 2:54 AM, Michael Layzell mich...@thelayzells.com wrote: The patches I am working on already use Bobby Holley's OriginAttributes, in fact we use the origin attribute on the nsIPrincipal, and only expose an nsIPrincipal from the API. Internally, we use the origin attribute for serialization, but to external consumers of the API, all that is available are APIs for adding and removing permissions VIA nsIPrincipal, and some convenience methods which exist for legacy reasons which allow you to add/remove permissions via nsIURIs, creating NoAppCodebasePrincipals for them. You are right, that if we decide that double keying is something we want, that it would be possible to add it to the permission manager by simply modifying nsIPrincipal's OriginAttributes. In terms of a mechanism for DENY being treated differently, it would be possible (albeit very hacky) to always insert DENY entries for URIs with a host property at http://HOST, and then, during the lookup, also check http://HOST, and if it is DENY, expose a DENY for the other origin. I don't really like this solution, as it creates lots of edge cases and other complications which I'm not sure if we want to expose to API consumers, as well as making the semantics of permissions diverge from nsIPrincipals, (the coherence of the new semantics with nsIPrincipal is something I quite like). On Tue, Jun 30, 2015 at 8:53 PM, Jonas Sicking jo...@sicking.cc wrote: On Tue, Jun 30, 2015 at 3:55 PM, Martin Thomson m...@mozilla.com wrote: I wonder, has the subject of double-keying been raised in this context? It comes up frequently in this context. And when I say double-keying, I mean forming a key from the tuple of the requesting principal and the top level browsing context principal (though origin may suffice). If there are disruptive changes afoot, then segregating based on what is shown to the user might be sensible. Bobby Holley has added infrastructure on nsIPrincipal called OriginAttributes which is intended to be an extension hook to allow things like double keying. As long as we use the 'origin' attribute on nsIPrincipal, and make sure that all callers pass in an nsIPrincipal rather than an nsIURI, then we should be able to relatively easy add double keying in the future. / Jonas -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to migrate the permissions database to use origins instead of host names
The patches I am working on already use Bobby Holley's OriginAttributes, in fact we use the origin attribute on the nsIPrincipal, and only expose an nsIPrincipal from the API. Internally, we use the origin attribute for serialization, but to external consumers of the API, all that is available are APIs for adding and removing permissions VIA nsIPrincipal, and some convenience methods which exist for legacy reasons which allow you to add/remove permissions via nsIURIs, creating NoAppCodebasePrincipals for them. You are right, that if we decide that double keying is something we want, that it would be possible to add it to the permission manager by simply modifying nsIPrincipal's OriginAttributes. In terms of a mechanism for DENY being treated differently, it would be possible (albeit very hacky) to always insert DENY entries for URIs with a host property at http://HOST, and then, during the lookup, also check http://HOST, and if it is DENY, expose a DENY for the other origin. I don't really like this solution, as it creates lots of edge cases and other complications which I'm not sure if we want to expose to API consumers, as well as making the semantics of permissions diverge from nsIPrincipals, (the coherence of the new semantics with nsIPrincipal is something I quite like). On Tue, Jun 30, 2015 at 8:53 PM, Jonas Sicking jo...@sicking.cc wrote: On Tue, Jun 30, 2015 at 3:55 PM, Martin Thomson m...@mozilla.com wrote: I wonder, has the subject of double-keying been raised in this context? It comes up frequently in this context. And when I say double-keying, I mean forming a key from the tuple of the requesting principal and the top level browsing context principal (though origin may suffice). If there are disruptive changes afoot, then segregating based on what is shown to the user might be sensible. Bobby Holley has added infrastructure on nsIPrincipal called OriginAttributes which is intended to be an extension hook to allow things like double keying. As long as we use the 'origin' attribute on nsIPrincipal, and make sure that all callers pass in an nsIPrincipal rather than an nsIURI, then we should be able to relatively easy add double keying in the future. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to migrate the permissions database to use origins instead of host names
On Tue, Jun 30, 2015 at 3:55 PM, Martin Thomson m...@mozilla.com wrote: I wonder, has the subject of double-keying been raised in this context? It comes up frequently in this context. And when I say double-keying, I mean forming a key from the tuple of the requesting principal and the top level browsing context principal (though origin may suffice). If there are disruptive changes afoot, then segregating based on what is shown to the user might be sensible. Bobby Holley has added infrastructure on nsIPrincipal called OriginAttributes which is intended to be an extension hook to allow things like double keying. As long as we use the 'origin' attribute on nsIPrincipal, and make sure that all callers pass in an nsIPrincipal rather than an nsIURI, then we should be able to relatively easy add double keying in the future. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to migrate the permissions database to use origins instead of host names
On Tue, Jun 30, 2015 at 4:16 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2015-06-30 6:04 PM, Jonas Sicking wrote: There are actually one downside with this change. It means that if a user denies access to https://website.com to use cookies, then http://website.com will still have full ability to use cookies since it's a different origin. That is a good point. Are you more worried about importing existing DENY entries, or recording new ones? But I guess. But I agree I don't see a way to address this without changing the nsIPermissionManager API. / Jonas For a DENY policy it often makes more sense to deny a whole domain name, since more often than not the http and https website are run by the same people. Whereas for an ACCEPT policy, it very often makes more sense to only allow a given origin. Unfortunately I can't think of a clean way to support this off the top of my head. Michael's current patches move us to store an origin field, potentially with some trailing origin attributes. I can't think of any way to make DENY work across schemes and port numbers other than inventing custom catch-all entries for such entries :( Another complication is how should we treat cross-app origins? Do we want to treat a DENY entry for one app override all others? If not, do we want to treat appId/isInBrowserElement specially here? What about future other origin attributes? Can you think of a clean way to address this? The problem stems from that the permission manager doesn't store policies, but rather stores unformatted data that is then up to the *reader* to interpret as a policy. Indeed. I.e. right now you just call pm.add(https://website.com, camera, ALLOW); It is then up to the reader to interpret this as either camera is available only for https://website.com; or camera is available for https://website.com and any subdomain. This is decided by either calling testPermission or testExactPermission. Most developers do not know about this difference and so I have uncountably many times had to tell people to use testExactPermission to get desired behavior. Obviously this is extra bad because testPermission is the function people jump most immediately to due to its simpler name. What I think the permission manager really needs to do is to have an API like: pm.addOriginRule(https://website.com, camera, PROMPT); pm.addDomainRule(domain.com, cookies, DENY, INCLUDE_SUBDOMAINS | OVERRIDE_EXISTING); pm.testPermission(myprincipal, geolocation); I'd really prefer if we addressed this without attempting to make big changes to the permission manager API, as that is a much bigger project than what we have been planning to do here and will require the consumers to be rewritten. On Tue, Jun 30, 2015 at 11:58 AM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: Historically our permissions database has used host names as part of the permission entry, which means that if we stored a permission for http://example.com, it would also apply to https://example.com (and http://example.com:8080 for that matter). This is undesirable specially once you note that this potentially makes us store permissions for MITMed connections and later on use them even for secure origins. Michael has been working on fixing this over in bug 1165263. We expect to land this change within the Firefox 42 timeframe. Here is the details of the changes we are planning to make: 1. Instead of storing a host name, appId and isInBrowserElement flag for each entry, we will store the full origin (including the origin attributes.) 2. Fix up the places where the permission manager APIs expose the notion of host/appId/isInBrowserElement to the consumers. 3. Drop the file hack from the permissions manager (bug 817007.) We're going to use the following algorithm for migrating the existing permission entries: * If the entry takes the form of a valid URI (such as entries for file:// URIs), then just insert it directly * For each host name foo, check to see whether the Places database includes a visit to that domain or any subdomain. * For each found entry in the Places database, we inject a permission for the visited origin, with the appId and isInBrowserElement origin attributes from the original permission. * If there are no existing visited entries, inject one permission for http://foo, and one for https://foo. The interesting part of how the migration step works is that we have no good way to know what origins need the permission after the upgrade by just looking at the host name, so we try to find something in the Places database that would help answer this question, and at the lack of that, we assume that the two common origins that can be constructed from the host name (namely the http and https origins with the default ports) can have the permission, which is our best guess. This means that in some edge cases such as the user
Re: Intent to migrate the permissions database to use origins instead of host names
On Tue, Jun 30, 2015 at 5:50 PM, Jonas Sicking jo...@sicking.cc wrote: On Tue, Jun 30, 2015 at 4:16 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: On 2015-06-30 6:04 PM, Jonas Sicking wrote: There are actually one downside with this change. It means that if a user denies access to https://website.com to use cookies, then http://website.com will still have full ability to use cookies since it's a different origin. That is a good point. Are you more worried about importing existing DENY entries, or recording new ones? But I guess. But I agree I don't see a way to address this without changing the nsIPermissionManager API. That should say Both I guess. / Jonas ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Intent to migrate the permissions database to use origins instead of host names
Historically our permissions database has used host names as part of the permission entry, which means that if we stored a permission for http://example.com, it would also apply to https://example.com (and http://example.com:8080 for that matter). This is undesirable specially once you note that this potentially makes us store permissions for MITMed connections and later on use them even for secure origins. Michael has been working on fixing this over in bug 1165263. We expect to land this change within the Firefox 42 timeframe. Here is the details of the changes we are planning to make: 1. Instead of storing a host name, appId and isInBrowserElement flag for each entry, we will store the full origin (including the origin attributes.) 2. Fix up the places where the permission manager APIs expose the notion of host/appId/isInBrowserElement to the consumers. 3. Drop the file hack from the permissions manager (bug 817007.) We're going to use the following algorithm for migrating the existing permission entries: * If the entry takes the form of a valid URI (such as entries for file:// URIs), then just insert it directly * For each host name foo, check to see whether the Places database includes a visit to that domain or any subdomain. * For each found entry in the Places database, we inject a permission for the visited origin, with the appId and isInBrowserElement origin attributes from the original permission. * If there are no existing visited entries, inject one permission for http://foo, and one for https://foo. The interesting part of how the migration step works is that we have no good way to know what origins need the permission after the upgrade by just looking at the host name, so we try to find something in the Places database that would help answer this question, and at the lack of that, we assume that the two common origins that can be constructed from the host name (namely the http and https origins with the default ports) can have the permission, which is our best guess. This means that in some edge cases such as the user having a foo.com permission that belongs to an origin such as http://foo.com:8080 but not having visited foo.com recently (by default in the last 180 days) we will lose the stored permission. We hope that these edge cases will happen very rarely in practice. Please let me know if you have any questions or concerns. Cheers, -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to migrate the permissions database to use origins instead of host names
On Tue, Jun 30, 2015 at 4:18 PM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: I personally am not sure if that is a sound idea for all permission types. It's probably the right thing for geolocation, but not for cookies. As I understand it, the key for permission manager is a simple string. Rather than make new APIs and restructure in a way that is perhaps incompatible with existing users, could you make a set of different key builders to serve all these ends: pm.addRule(pm.makeOriginRule(uri), camera, blah) pm.addRule(pm.makeDomainRule(example.com, INCLUDE_SUBDOMAINS), cookies, DENY) The plan to migrate to an origin basis is OK without this, of course. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Intent to migrate the permissions database to use origins instead of host names
On 2015-06-30 6:55 PM, Martin Thomson wrote: I wonder, has the subject of double-keying been raised in this context? It comes up frequently in this context. And when I say double-keying, I mean forming a key from the tuple of the requesting principal and the top level browsing context principal (though origin may suffice). Yes, this was discussed the last time we discussed permissions on this list, IIRC, and this idea was brought up. I personally am not sure if that is a sound idea for all permission types. It's probably the right thing for geolocation, but not for cookies. This is also another part of the bigger problem here that the permission manager doesn't store policies. If there are disruptive changes afoot, then segregating based on what is shown to the user might be sensible. On Tue, Jun 30, 2015 at 3:04 PM, Jonas Sicking jo...@sicking.cc wrote: There are actually one downside with this change. It means that if a user denies access to https://website.com to use cookies, then http://website.com will still have full ability to use cookies since it's a different origin. For a DENY policy it often makes more sense to deny a whole domain name, since more often than not the http and https website are run by the same people. Whereas for an ACCEPT policy, it very often makes more sense to only allow a given origin. The problem stems from that the permission manager doesn't store policies, but rather stores unformatted data that is then up to the *reader* to interpret as a policy. I.e. right now you just call pm.add(https://website.com, camera, ALLOW); It is then up to the reader to interpret this as either camera is available only for https://website.com; or camera is available for https://website.com and any subdomain. This is decided by either calling testPermission or testExactPermission. Most developers do not know about this difference and so I have uncountably many times had to tell people to use testExactPermission to get desired behavior. Obviously this is extra bad because testPermission is the function people jump most immediately to due to its simpler name. What I think the permission manager really needs to do is to have an API like: pm.addOriginRule(https://website.com, camera, PROMPT); pm.addDomainRule(domain.com, cookies, DENY, INCLUDE_SUBDOMAINS | OVERRIDE_EXISTING); pm.testPermission(myprincipal, geolocation); / Jonas On Tue, Jun 30, 2015 at 11:58 AM, Ehsan Akhgari ehsan.akhg...@gmail.com wrote: Historically our permissions database has used host names as part of the permission entry, which means that if we stored a permission for http://example.com, it would also apply to https://example.com (and http://example.com:8080 for that matter). This is undesirable specially once you note that this potentially makes us store permissions for MITMed connections and later on use them even for secure origins. Michael has been working on fixing this over in bug 1165263. We expect to land this change within the Firefox 42 timeframe. Here is the details of the changes we are planning to make: 1. Instead of storing a host name, appId and isInBrowserElement flag for each entry, we will store the full origin (including the origin attributes.) 2. Fix up the places where the permission manager APIs expose the notion of host/appId/isInBrowserElement to the consumers. 3. Drop the file hack from the permissions manager (bug 817007.) We're going to use the following algorithm for migrating the existing permission entries: * If the entry takes the form of a valid URI (such as entries for file:// URIs), then just insert it directly * For each host name foo, check to see whether the Places database includes a visit to that domain or any subdomain. * For each found entry in the Places database, we inject a permission for the visited origin, with the appId and isInBrowserElement origin attributes from the original permission. * If there are no existing visited entries, inject one permission for http://foo, and one for https://foo. The interesting part of how the migration step works is that we have no good way to know what origins need the permission after the upgrade by just looking at the host name, so we try to find something in the Places database that would help answer this question, and at the lack of that, we assume that the two common origins that can be constructed from the host name (namely the http and https origins with the default ports) can have the permission, which is our best guess. This means that in some edge cases such as the user having a foo.com permission that belongs to an origin such as http://foo.com:8080 but not having visited foo.com recently (by default in the last 180 days) we will lose the stored permission. We hope that these edge cases will happen very rarely in practice. Please let me know if you have any questions or concerns. Cheers, -- Ehsan ___ dev-platform mailing list
