Closed Bug 1000253 Opened 6 years ago Closed 5 years ago

Background tabs with persistent device permissions can access devices without the user noticing

Categories

(Firefox :: General, defect)

30 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 33
Tracking Status
firefox30 + verified
firefox31 + verified
firefox32 + verified
firefox33 --- verified

People

(Reporter: florian, Assigned: florian)

References

Details

(Keywords: privacy, sec-moderate, Whiteboard: [adv-main30-] p=5 s=33.1 [qa!])

Attachments

(3 files)

If a user has 2 tabs:
- selected tab, with ongoing usage of webrtc (eg. video call)
- second, background tab, with a webapp to which the user has already granted persistent device permissions.

Then the webapp in the second tab can access the microphone or camera at any moment without user intervention, and without any visible change to the UI (because the icon near the URL bar is not shown for background tabs, and the toolbar icon showing the device is in use was already visible because of the webrtc usage in the selected tab).

I'm afraid this means any webapp that has received persistent device permissions can eavesdrop calls (the only difficult part here seems to be to know that a call is ongoing).

Suggested solution: when a webapp has persistent device permissions, don't grant device access immediately if the tab isn't selected; grant device access only once the user selects the tab.
I don't think this change could break any existing application, because without persistent permission the permission prompt isn't shown until the tab is selected.
Going to rate this "moderate" rather than "low" because people are uptight about this kind of surreptitious recording. A mitigating factor is that you did trust the background tab enough to grant it persistent permissions so hopefully it won't do anything too terrible.

We really shouldn't let non-foreground tabs initiate recording (in fact I'd like to limit those actions to being in a user-initiated events, something like the pop-up blocker, but I bet that breaks reasonable use cases).

The nav-bar icon does have a drop-down showing the tabs with active recording but maybe it needs a number when there's more than 1 so people can notice/remember. Some animation swooping into the camera icon might also help?
Group: core-security
(In reply to Daniel Veditz [:dveditz] from comment #1)
> Going to rate this "moderate" rather than "low" because people are uptight
> about this kind of surreptitious recording. A mitigating factor is that you
> did trust the background tab enough to grant it persistent permissions so
> hopefully it won't do anything too terrible.
> 
> We really shouldn't let non-foreground tabs initiate recording (in fact I'd
> like to limit those actions to being in a user-initiated events, something
> like the pop-up blocker, but I bet that breaks reasonable use cases).

Unfortunately, it breaks nearly every WebRTC demo out there.


> The nav-bar icon does have a drop-down showing the tabs with active
> recording but maybe it needs a number when there's more than 1 so people can
> notice/remember. Some animation swooping into the camera icon might also
> help?
Delaying camera/mic until the tab goes active may be a reasonable compromise.  We definitely need them to be able to be active in the background, though I'd still like a more noticeable indicator of this.

Persistent permission is a tricky problem.  There are definitely some UI supports that could enhance privacy around them (and I expect to see extensions adding more (and less) default privacy/notification), and some things we could do by default.

Stuff like (internal or extension):
* "XYZZY is asking for access and you haven't reconfirmed in 2 weeks; should they continue to be granted?"
* Time-limited semi-transparent overlay that they've been given access automatically in general, or only when in background.
* Popup-and-ask when background tab with persistent permission asks for access (but not when in foreground)
* Extension blocking background access-by-default

Most/all of this falls into the UX side of things
Component: WebRTC: Audio/Video → General
Product: Core → Firefox
I think we should address this somehow before we ship bug 804611. That might mean delaying the shipping of bug 804611 - would that be problematic for the webrtc roadmap somehow?
Flags: firefox-backlog+
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #4)
> I think we should address this somehow before we ship bug 804611. That might
> mean delaying the shipping of bug 804611 - would that be problematic for the
> webrtc roadmap somehow?

If this is a blocker for shipping bug 804611 let's get a backout from 30 ready for next week's Betas.
Flags: needinfo?(rjesup)
Flags: needinfo?(gavin.sharp)
WebRTC doesn't need this in Fx 30 for our roadmap.  However, devs are asking for it.  So I think we should back this out from 30 and seriously re-target 31.  

I'll start a discussion on dev-media to start hashing out a solution for this bug.  I'll copy everyone who is cc'd on this bug on my post and ask Shell to put this bug on WebRTC's radar for Fx 31 release.   If someone prefers me to use a different mailing list than dev-media for this, please let me know.
Flags: needinfo?(rjesup)
Florian, can you prepare a backout for beta approval?
Flags: needinfo?(gavin.sharp) → needinfo?(florian)
Instead of a full backout, I removed only the "Always" option that's causing the security issue here.

I think keeping the "Never" option has value, and keeping the tests running definitely has value.
Attachment #8428747 - Flags: review?(rjesup)
Attachment #8428747 - Flags: review?(dolske)
Flags: needinfo?(florian)
Attached patch Test fixSplinter Review
While working on attachment 8428747 [details] [diff] [review] I found a mistake in the test; that test fix should also go to mozilla-central.
Attachment #8428754 - Flags: review?(dolske)
Attachment #8428747 - Flags: review?(rjesup) → review+
Attachment #8428747 - Flags: review?(dolske) → review?(MattN+bmo)
Attachment #8428754 - Flags: review?(dolske) → review?(MattN+bmo)
Comment on attachment 8428747 [details] [diff] [review]
Remove the 'Always' option for mozilla-beta

Review of attachment 8428747 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine to me for beta.
Attachment #8428747 - Flags: review?(MattN+bmo) → review+
Attachment #8428754 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8428747 [details] [diff] [review]
Remove the 'Always' option for mozilla-beta

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 804611
User impact if declined: moderate security risk
Testing completed (on m-c, etc.): The patch contains tests.
Risk to taking this patch (and alternatives if risky): reasonably low.
String or IDL/UUID changes made by this patch: none.
Attachment #8428747 - Flags: approval-mozilla-beta?
Attachment #8428747 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
For Firefox 30 the patch here doesn't 'fix' the issue, but disables the new feature we are concerned about.
This bug is status-firefox30: fixed, bug 804611 is status-firefox30: disabled.
Only allowing the active tab to initiate user media requests does break some practical uses - eg calls which may be interrupted, timelapse app, etc. A VOIP app where users auto accept calls would also get broken. But user media is used for more than just WebRTC.

The toolbar device icon could show a numeric badge if multiple tabs are accessing user media.

Please do not leave this issue as 'no permanent or no background usermedia initations'. Currently our users have to edit about:config and automatically grant permissions to EVERYTHING which is horrible privacy wise but only way they can use our app on Firefox.
(In reply to admin from comment #18)
> Only allowing the active tab to initiate user media requests does break some
> practical uses - eg calls which may be interrupted, timelapse app, etc. A
> VOIP app where users auto accept calls would also get broken. But user media
> is used for more than just WebRTC.

You should bring these usecases up in the W3 Media Capture Task Force's meetings and on the media-capture mailing list.

There is a tricky line here between enabling user-desired behavior and privacy risks, especially those the user may be unaware of.

> The toolbar device icon could show a numeric badge if multiple tabs are
> accessing user media.

Perhaps (it does show an indicator in FF now, and you can see/jump-to any tab that has one open).  The concern is that this is insufficiently obvious to a user.

Options
-------
Apps with persistent permission that require getting access to mic/camera while in the background are:
1) allowed.  Downside: user privacy risk and user not expecting this (per discussion above)
2) denied with a different error - they can force themselves to come to the front if needed.
3) get forced to pop a requester even though they're "trusted" to get access to the camera/mic
4) get access while in the foreground and then hold the mediastream in order to enable camera/mic when needed in the background.  Downside: camera light always on, making it a poor indicator of actual privacy.  Users will likely dislike this.
5) add another permission level past persistent that allows access anytime.  Could be inline ("Really Always"), on first background use ("Remember I allowed this" or "I allowed this, click here to disable background access"), or via prefs/about:permission UI.  Downside: UI complexity and user confusion, discoverability (prefs/about:permission).  Note that apps that need this could prompt the user to do it (esp. if combined with #2)
6) allow specific app/apps to be white-listed for background access via an extension. Downside: requires an extension.
7) allow background access, but combine it with additional user notification (not permission).
a) modal "I allowed it box" - ugh, forget this
b) non-modal doorhanger that collapses when you interact with something else.  downside: still mildly intrusive.  Might be gameable in some cases (get user to click madly then ask for access).
c) extra-visible "camera-in-use" indicator state (extra highlight and pulse, etc).  For how long? How obvious can this be made?  User understanding/confusion?


> Please do not leave this issue as 'no permanent or no background usermedia
> initations'. Currently our users have to edit about:config and automatically
> grant permissions to EVERYTHING which is horrible privacy wise but only way
> they can use our app on Firefox.

We absolutely need persistent permissions, which is why we're working on this, but the background issue is thorny.
In order to solve

> I'm afraid this means any webapp that has received persistent device permissions
> can eavesdrop calls (the only difficult part here seems to be to know that a call is ongoing).

Action only needs to be taken if there is already another usermedia access ongoing, otherwise there will be a visual indication of the usermedia access (which is not very apparent right now, but I think that should be changed to make it more apparent instead of blocking background usermedia access).

Please correct me if I am wrong, but a foreground page that access persistent usermedia right now has about the same level of apparentness as a background page accessing usermedia? There is only no apparentness when the user is already in a call in another tab. 

I think any notification that requires user interaction to dismiss revokes a major part of the reason for persistent allow. Notifications that require interaction to dismiss turns 'persistent allow' into 'ask for permission after granting access'. 

Would this be an option as well?

7d) Icon on any tabs  that are accessing usermedia (eg animated red recording circle that persists for 1 second after access).
Also, a malicious party that is able to get the user to allow and 'Remember this decision' in Firefox would likewise be able to get the user to click 'Allow' in Adobe Flash and eavesdrop in the background without any notification. Same for Chromium's usermedia (background access, but has notification).
That things are broken elsewhere, is not a convincing argument. Besides, Flash is on its way out. We should look forward.

(In reply to Randell Jesup [:jesup] from comment #19)
> 3) get forced to pop a requester even though they're "trusted" to get access
> to the camera/mic
> 5) add another permission level past persistent that allows access anytime. 
> Could be inline ("Really Always"), on first background use ("Remember I
> allowed this" or "I allowed this, click here to disable background access"),
> or via prefs/about:permission UI.  Downside: UI complexity and user
> confusion, discoverability (prefs/about:permission).  Note that apps that
> need this could prompt the user to do it (esp. if combined with #2)

My 2 cents: I would combine 3 and 5 into:

8) When a background tab requests gUM access - even one that previously has gained "persistent permission" - pop up a more severe "eavesdrop permission" request that lets users grant "eavesdrop permission" just once or permanently for a site. We can then discuss ""persistent eavesdrop permission" with this dialog explaining that this allows foo.org to start eavesdropping later at random intervals.

I think this setting is too difficult to explain to a user ahead of it happening.
Lastly, a general comment to our existing permissions: A non-modal heads-up whenever a persistent permission grants a site something would be welcome I think.

Analogy: My rooted android phone puts up a message for two seconds whenever an app - that I previously OK'ed - is granted superuser permission. It's a timely reminder about what's going on on my system that doesn't seem to cost much, and gives me a great deal of peace, because it lets me detect and react to bad and/or unexpected behavior, and ensures me that this isn't going on all of the time, or at times I'm not expecting it to.
Whiteboard: [leave-open] → [leave-open][adv-main30-]
Florian, this will have to land in 31 soon. Otherwise, we will have to do as 30. Thanks.
Flags: needinfo?(florian)
Whiteboard: [leave-open][adv-main30-] → [leave-open][adv-main30-] p=0 [qa?]
(In reply to Sylvestre Ledru [:sylvestre] from comment #24)
> Florian, this will have to land in 31 soon. Otherwise, we will have to do as
> 30. Thanks.

I think I can make it happen within 2 weeks.
Flags: needinfo?(florian)
Whiteboard: [leave-open][adv-main30-] p=0 [qa?] → [leave-open][adv-main30-] p=5 [qa+]
Florian -- Would you like me to start a discussion on dev-media and dev-firefox like I offered to in Comment 6 (above)?  (Or you could do it?)   My apologies that I didn't get to this before today.  The push for Fx32/v2.0 wound up being more time consuming than I thought.
Flags: needinfo?(florian)
(In reply to Maire Reavy [:mreavy] (Please needinfo me) from comment #26)
> Florian -- Would you like me to start a discussion on dev-media and
> dev-firefox like I offered to in Comment 6 (above)?

I don't think that's strictly required, but you can if you feel the need for it.

The thing I'm planning to do here is close to the solution I suggested in comment 0:
- keep denying access to the device immediately from the mediamanager code if the user saved a "DENY" persistent permission.
- no longer check for "allow" persistent permissions in the mediamanager code; instead do it from the code that shows the permission prompt, in the popupshowing event handler (this happens only once the tab is selected and the window focused).

I think this change would be enough to fix the security issue here, and would be a net win for webapp developers compared to what we currently have (no persistent "allow" permissions).

I understand that this doesn't address all possible use cases, but I think the remaining use cases can be dealt with in follow-ups (and they may need further discussion in dev-media before implementing).

Does this seem good to you Maire?
Flags: needinfo?(florian)
Yeah, this solution works for me.  This isn't a perfect solution for all use cases, but none of the possible solutions are.  I agree with you that it would address the security/privacy issues and unblock this bug -- which is the primary goal. 

I would like to take the discussion of remaining use cases to the mailing lists (dev-media, dev-firefox, and the WebRTC working group mailing list) and file follow up bugs so they don't fall off our radar.

Thanks!
Status: NEW → ASSIGNED
Whiteboard: [leave-open][adv-main30-] p=5 [qa+] → [leave-open][adv-main30-] p=5 s=33.1 [qa+]
Attached patch PatchSplinter Review
This implements the solution I proposed in comment 27.
Attachment #8439313 - Flags: review?(rjesup)
Attachment #8439313 - Flags: review?(MattN+bmo)
Attachment #8439313 - Flags: review?(rjesup) → review+
Attachment #8439313 - Flags: review?(MattN+bmo) → review+
Blocks: 1015486
Comment on attachment 8439313 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 804611
User impact if declined: moderate security risk
Testing completed (on m-c, etc.): Currently on m-c. There are tests ensuring that the 'Always allow' feature works.
Risk to taking this patch (and alternatives if risky): reasonably low.
String or IDL/UUID changes made by this patch: none.
Attachment #8439313 - Flags: approval-mozilla-beta?
Attachment #8439313 - Flags: approval-mozilla-aurora?
Attachment #8439313 - Flags: approval-mozilla-beta?
Attachment #8439313 - Flags: approval-mozilla-beta+
Attachment #8439313 - Flags: approval-mozilla-aurora?
Attachment #8439313 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/00594a5ed74d
https://hg.mozilla.org/releases/mozilla-beta/rev/686333592e1f (includes attachment 8428754 [details] [diff] [review])
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [leave-open][adv-main30-] p=5 s=33.1 [qa+] → [adv-main30-] p=5 s=33.1 [qa+]
Target Milestone: --- → Firefox 33
Hi Florin, can a contact be assigned for QA verification.
Flags: needinfo?(florin.mezei)
Flags: needinfo?(florin.mezei)
QA Contact: catalin.varga
Note for QA:
You don't really need an app using the device 'in the background'. An app that uses the device onload that _you_ load in the background is enough to test.
For example, load: data:text/html,<a href="https://apprtc.webrtc.org/">apprtc</a>
and then right click the link and select "Open Link in New Tab" in the context menu.
Verified as fixed using the following environments:

FF Beta 31.02
Build Id: 20140616143923

FF Aurora 32
Build Id:20140618004002

FF Nightly 33
Build Id:20140618030215

OS: Win 7 x64, Mac Os X 10.8.5, Ubuntu 13.04 x64

The user is granting access to local media only after the tab is selected if Always Share was previously selected and automatically deny access if Never Share was previously selected.

FF 30
Build Id:20140605174243
OS: Win 7 x64, Mac Os X 10.8.5, Ubuntu 13.04 x64

For FF 30 the Always Share option is removed.
Status: RESOLVED → VERIFIED
Whiteboard: [adv-main30-] p=5 s=33.1 [qa+] → [adv-main30-] p=5 s=33.1 [qa!]
You need to log in before you can comment on or make changes to this bug.