983158 - Security Review: persistent getUserMedia permissions

Status: RESOLVED FIXED

(mozilla.org :: Security Assurance: Review Request, task)

Description

[Florian Quèze [:florian]]

Attachment: Screenshot

This feature has been implemented in bug 804611 and is currently in nightlies.

We know we will need to add some form of security restrictions to it before it reaches mozilla-beta (eg. not grant persistent camera access to http websites), but it seems better to have input from a security & privacy review before implementing restrictions.


The visible new UI for this feature is limited to the "Always Share" and "Never Share" secondary items of the getUserMedia permission prompt. Screenshot attached.

Revoking permissions is possible from:
- The Permissions tab of the Tool->Page Info dialog.
- about:permissions (not discoverable for now)
- Clicking the identity/padlock button in the URL bar: this panel lists only permissions that have non default values. But it's possible to set the permissions back to "Always ask" or "Deny".
- The "Stop Sharing" button I added in bug 885796: if the user stops sharing with a web page but had granted persistent permissions, we forget the permissions and will prompt again next time.

Comment 1

[Florian Quèze [:florian]]

What's the ETA here? We've got only 20 days left before Firefox 30 (which has persistent gUM permissions) moves to beta.

Comment 2

[Daniel Veditz [:dveditz]]

* On non-SSL sites the "Always Share" option should be missing or disabled

* If permissions are stored for a site (I assume you're using PermissionManager, so by domain rather than origin) then saved permissions should be ignored and ONLY used if the scheme is "https".

[I hate that the PermissionManager only stores domains and not origins, but that's a more general issue]

Adam filed bug 993495 on the above two points.

* I'm unhappy with the lack of discoverability of permissions in general. Australis makes the tool menu undiscoverable for most of our users. They might notice the Page Info item on the context menu, though it's not really obvious permissions would be in there. Clicking on the globe/lock icon to bring up Larry offers a "More Information" button. That opens Page Info to the security tab so that's an alternate route to Permissions for that page. I guess that will have to do, but this is another area that Chrome beats us on.

* PeerConnections have some severe privacy implications for some users. There really ought to be a permission prompt and stored permissions for that as well and it'd be nice for users to combine it all into one request rather than have two doorhangers. I don't think the current API allows for that though so I guess that's out of scope for a "gUM" review.

Comment 3

[Daniel Veditz [:dveditz]]

Back to gUM UI specifically:

the address-bar icon for the tab you're on is good

if you click it to bring the door-hanger back it's cumbersome to stop sharing. you /can/ stop it so this is just a personal use-of-ease objection, but if someone blunders into your room and you need to kill the feed quickly a UI that takes 3 clicks (one of them on a very small triangle target) isn't helping.

* if you do stop sharing from the doorhanger, but you've persisted permissions for that site they can just start it up again, right? Maybe some link from the door hanger to the permissions tab the way Larry links to the security tab of Page Info would be helpful. This is a suggestion, not a requirement.

The persistent camera indicator in the navigation bar is good, but it's hellishly unnoticeable way over on the right in neutral understated outline form. Maybe it's less bad on a narrower monitor? It's also not customizable -- I wanted to place it over on the far left where my eyes go all the time, not on the right with the miscellaneous "buttons I hardly use or care about". It functions fine as a button to use if you know you want to do something with it, but it blends in so well that it fails as warning light which is really what it ought to be. Like the red 'rec' light in your video camera (or your parent's video camera, I suppose).

Comment 4

[Florian Quèze [:florian]]

Attachment: Permissions shown by Larry

(In reply to Daniel Veditz [:dveditz] from comment #2)
> * On non-SSL sites the "Always Share" option should be missing or disabled
> 
> * If permissions are stored for a site (I assume you're using
> PermissionManager, so by domain rather than origin) then saved permissions
> should be ignored and ONLY used if the scheme is "https".

Is it enough to just check the url scheme, or is there some other check we need to perform? Do we need the whole page to be loaded over https, or just the frame that's doing the gUM request?

> * I'm unhappy with the lack of discoverability of permissions in general.
> [...] Clicking on the globe/lock icon to
> bring up Larry offers a "More Information" button. That opens Page Info to
> the security tab so that's an alternate route to Permissions for that page.
> I guess that will have to do, but this is another area that Chrome beats us
> on.

Opening Larry actually shows the permissions directly. See attached screenshot.

> * PeerConnections have some severe privacy implications for some users.
> There really ought to be a permission prompt and stored permissions for that
> as well and it'd be nice for users to combine it all into one request rather
> than have two doorhangers. I don't think the current API allows for that
> though so I guess that's out of scope for a "gUM" review.

This is indeed out of the scope here.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

temporarily marking s-s until bug 993495 is resolved

Comment 6

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> Do we need the whole page to be loaded over https, or just
> the frame that's doing the gUM request?

I've verified (with the test I attached to bug 1000213) that the URL used (and the hostname shown in the UI) is the frame URL.

Comment 7

[Al Billings [:abillings - ex-MoCo]]

(In reply to Randell Jesup [:jesup] from comment #5)
> temporarily marking s-s until bug 993495 is resolved

Does this still need to be hidden?

Comment 8

[Daniel Veditz [:dveditz]]

(In reply to Florian Quèze [:florian] [:flo] from comment #4)
> > * If permissions are stored for a site (I assume you're using
> > PermissionManager, so by domain rather than origin) then saved permissions
> > should be ignored and ONLY used if the scheme is "https".
> 
> Is it enough to just check the url scheme, or is there some other check we
> need to perform? Do we need the whole page to be loaded over https, or just
> the frame that's doing the gUM request?

You can check the scheme of the origin requesting the permission.

How do you handle permissions for sub-frames that are a different origin? Even if you correctly state the frame's domain in the door-hanger users are likely to miss that and assume they're making decisions about the top-level URL.

> Opening Larry actually shows the permissions directly. See attached
> screenshot.

How does that work when the origin with camera permissions is a non-same-origin subframe rather than the top-level origin Larry is talking about?

Comment 9

[Florian Quèze [:florian]]

(In reply to Daniel Veditz [:dveditz] from comment #8)

> How do you handle permissions for sub-frames that are a different origin?
> Even if you correctly state the frame's domain in the door-hanger users are
> likely to miss that and assume they're making decisions about the top-level
> URL.

I verified a while ago (comment 6) that the hostname shown in the door-hanger is the hostname of the frame requesting the device and not the top-level URL.

And yes, users are likely to not notice that the url in the door-hanger is different from the URL in the url bar.

> > Opening Larry actually shows the permissions directly. See attached
> > screenshot.
> 
> How does that work when the origin with camera permissions is a
> non-same-origin subframe rather than the top-level origin Larry is talking
> about?

It probably "doesn't work", in that I think Larry only shows the saved permissions for the top-level origin.