1037424 - implement a whitelist of web sites allowed to use screensharing in Firefox 33

Status: VERIFIED FIXED

(Core :: WebRTC, defect, P1)

Description

[Florian Quèze [:florian]]

We want a whitelist to control in Firefox 33 the feature implemented in bug 983504, until we can figure out a correct privacy UI explaining to the user what the risks are.

Not sure if this whitelist should be handled by the UI or back-end code, but we agreed to use a pref visible from about:config, so that webdevelopers can experiment with screensharing, and other services can potentially whitelist themselves by asking the user to install an add-on that would tweak the pref.

Comment 1

[:shell escalante]

With this whitelist - Please make sure there is a pref for screen sharing.  It could re-use or replace the test pref (bug 1037589) depending what's logical.

Comment 2

[Florian Quèze [:florian]]

(Martin Thomson [:mt] from bug 1035577 comment #2)
> (In reply to Florian Quèze [:florian] [:flo] from comment #1)
> > Bug 1037424 - implement a whitelist of web sites allowed to use
> > screensharing in Firefox 33
> >  This bug could either be handled in the UI or in the back-end, and so could
> > either be addressed by the Firefox or WebRTC team.
> 
> I think that this is best handled in the DOM.  That way we can be more
> responsive to requests for screensharing.  Once we open this up to more than
> just the whitelist and only use the whitelist for firefox/tabs and desktop
> sharing, then that code will need to live in the DOM anyway.

Comment 3

[Maire Reavy [:mreavy]]

Martin -- Can you explain how we would handle this on the DOM side?  (I'm referring to Comment 2.)

Comment 4

[Martin Thomson [:mt:]]

I think that we can (and probably should) use nsIPermissionManager to control this, with values primed from a pref.  That is, nsIPermissionManager has three screen sharing settings: forbid (don't bug me), default (ask, but don't share desktop or firefox), privileged (allow sharing of anything).  If there is no value already set in nsIPermissionManager, we would look at a pref to determine whether the site is listed; if listed, it gets "privileged", otherwise, "default".

Having thought more on the topic, there are two ways to approach this, both perfectly valid:

1. the UI can do the above and pass down a boolean "privileged" parameter with a request to MediaManager.

2. the DOM side can do the above

Both are perfectly OK.

(In reply to Florian Quèze [:florian] [:flo] from comment #0)
> Not sure if this whitelist should be handled by the UI or back-end code, but
> we agreed to use a pref visible from about:config, so that webdevelopers can
> experiment with screensharing, and other services can potentially whitelist
> themselves by asking the user to install an add-on that would tweak the pref.

I think that tweaking a value in nsIPermissionManager is the right way to address this rather than a pref.  The pref is just a necessary way to bootstrap permission manager (since the values there aren't present in existing profiles).

Comment 5

[Florian Quèze [:florian]]

(In reply to Martin Thomson [:mt] from comment #4)
> I think that we can (and probably should) use nsIPermissionManager to
> control this, with values primed from a pref.

I disagree. Using the permission manager for this seems more complicated than strictly needed, and using the permission manager for screen sharing in a way that differs from the way it's used for camera and microphone sharing would be confusing and error prone.

We will use the permission manager for things related to screen sharing; we need it to store the "don't bother me" information (bug 1037438).

The implementation I would suggest is:
- media manager read the value of the pref. The pref contains a comma separated list of whitelisted domain names.
- if the domain of the page requesting screensharing is not found in the whitelist, behave as if screensharing wasn't implemented (ie. return the same error as Firefox 32 did).

Comment 6

[Martin Thomson [:mt:]]

I guess that we have a disagreement then, because that solution might be OK for 33, where we only have a whitelist, but it doesn't address the case where we enable application sharing more widely.

Can you help me understand what is complicated here?

Comment 7

[Florian Quèze [:florian]]

(In reply to Martin Thomson [:mt] from comment #6)
> I guess that we have a disagreement then, because that solution might be OK
> for 33

We are targeting 33 here.

> where we only have a whitelist, but it doesn't address the case
> where we enable application sharing more widely.

I assume we would keep the whitelist pref longer than 33 to limit which websites have access to the sharing we feel aren't safe (ie. sharing the whole screen or sharing the window of the browser).

> Can you help me understand what is complicated here?

nsIPermissionManager doesn't have a UI. We need to tweak the various pieces of UI exposing it for each additional permission we want to expose to users. I don't understand which UI you want to expose here (if none, that makes it pretty difficult to use for web developers testing their app locally).

I intend to expose some UI for the "don't bother me" part in bug 1037438, but to not use the "allow" value of the permission. Using it with a different meaning than for the other existing permissions seems confusing to me.

Comment 8

[Martin Thomson [:mt:]]

(In reply to Florian Quèze [:florian] [:flo] from comment #7)
> I assume we would keep the whitelist pref longer than 33 to limit which
> websites have access to the sharing we feel aren't safe (ie. sharing the
> whole screen or sharing the window of the browser).
> 
> > Can you help me understand what is complicated here?
> 
> nsIPermissionManager doesn't have a UI. We need to tweak the various pieces
> of UI exposing it for each additional permission we want to expose to users.
> I don't understand which UI you want to expose here (if none, that makes it
> pretty difficult to use for web developers testing their app locally).

Now I'm more confused:

http://dxr.mozilla.org/mozilla-central/source/browser/components/preferences/aboutPermissions.xul

More concretely, what I'm proposing is this:

1. Add the usual buttons on the permissions doorhanger, excluding the always allow: "Allow, Deny, Always Deny"

2. Allow and Deny are transitory.

3. Always Deny updates nsIPermissionManager to "Block"

4. The "Always Allow" option in nsIPermissionManager can't be set using the doorhanger, but it can be set in aboutPermissions.xul

That gives sites the option for users to enable the permission for sites they trust without going to about:config and it means that we avoid them having to edit an unwieldy pref.

We already have a pref to override the doorhanger, I don't expect that anything new will be needed there.  Aside from that, we don't want a site to be able to bypass the doorhanger.

Comment 9

[Florian Quèze [:florian]]

(In reply to Martin Thomson [:mt] from comment #8)

> 3. Always Deny updates nsIPermissionManager to "Block"

This requires some UI to undo it, and so needs localizable strings. bug 1037438 will cover this.

> 4. The "Always Allow" option in nsIPermissionManager can't be set using the
> doorhanger, but it can be set in aboutPermissions.xul

You are proposing making the "Always Allow" do something different for screen sharing than it does for all the other permissions. Selecting "Always Allow" instead of "Prompt" (the default) and then getting a prompt is confusing.

Comment 10

[Martin Thomson [:mt:]]

(In reply to Florian Quèze [:florian] [:flo] from comment #9)
> This requires some UI to undo it, and so needs localizable strings. bug
> 1037438 will cover this.

Of course.
 
> > 4. The "Always Allow" option in nsIPermissionManager can't be set using the
> > doorhanger, but it can be set in aboutPermissions.xul
> 
> You are proposing making the "Always Allow" do something different for
> screen sharing than it does for all the other permissions. Selecting "Always
> Allow" instead of "Prompt" (the default) and then getting a prompt is
> confusing.

It wouldn't *say* "Always Allow":

http://dxr.mozilla.org/mozilla-central/source/browser/locales/en-US/chrome/browser/preferences/aboutPermissions.dtd

Comment 11

[Maire Reavy [:mreavy]]

Hi Marco -- I talked to Florian about this bug earlier today, and we agreed that someone on the WebRTC platform team should handle this one.  So I asked Paul Kerr to take it for this sprint (starting tomorrow).

Comment 12

[Marco Mucci [:MarcoM]]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #11)
> Hi Marco -- I talked to Florian about this bug earlier today, and we agreed
> that someone on the WebRTC platform team should handle this one.  So I asked
> Paul Kerr to take it for this sprint (starting tomorrow).

Sounds good, thanks for the update Maire.

Comment 13

[Paul Kerr [:pkerr]]

Attachment: WIP: Implement prefs-based screenshare domain whitelist in MediaManager

Work in progress. Need to do a few more tests and a sanity check.

Comment 14

[Martin Thomson [:mt:]]

Comment on attachment 8464251 [details] [diff] [review]
WIP: Implement prefs-based screenshare domain whitelist in MediaManager

This needs to base the decision on the document principal.

Comparison of domain names needs input normalization to ensure that we correctly compare names.  I've some code for that in PeerIdentity.cpp already.

If we really do need to do suffix-matching, please require a '*' prefix for matches.

Comment 15

[Paul Kerr [:pkerr]]

> 
> If we really do need to do suffix-matching, please require a '*' prefix for
> matches.

I have seen these types of lists grow to unwieldy length when multiple serving hosts are added under the domain. I agree with you suggestion of the '*' prefix; I was considering that.

Comment 16

[Martin Thomson [:mt:]]

(In reply to Paul Kerr [:pkerr] from comment #15)
> I have seen these types of lists grow to unwieldy length when multiple
> serving hosts are added under the domain. I agree with you suggestion of the
> '*' prefix; I was considering that.

Note: we should use the same rules as for certificate validation: * = any leaf node.  That is, *.example.com is good for foo.example.com and bar.example.com but not x.y.example.com.  

But I want to emphasize this, only do this if we really need it.  Chances are, the whitelist is going to be unwieldy if we still have it by Ffx40, so that aspect doesn't bother me.

Comment 17

[Paul Kerr [:pkerr]]

Attachment: WIP: Implement prefs-based screenshare domain whitelist in MediaManager

Work in progress. Added UTF8 to ASCII normalization of domain names per Martin's catch.

Comment 18

[Paul Kerr [:pkerr]]

Attachment: Part 1: Implement prefs-based screenshare domain whitelist in MediaManager

This patch does not attempt to do extended domain matching or handle wildcards in the whitelist. That will be left to a possible part 2.

Comment 19

[Paul Kerr [:pkerr]]

Are there any screensharing tests that need to have the whitelist handling added?

Comment 20

[Martin Thomson [:mt:]]

Comment on attachment 8464720 [details] [diff] [review]
Part 1: Implement prefs-based screenshare domain whitelist in MediaManager

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

::: dom/media/MediaManager.cpp
@@ +125,5 @@
> +
> +  PRUint32 begin = 0;
> +  PRUint32 end = 0;
> +  nsCString domainName;
> +  /* 

trailing space

Comment 21

[Paul Kerr [:pkerr]]

Attachment: Part 1: Implement prefs-based screenshare domain whitelist in MediaManager

Fixed bit rot.

Comment 22

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb9d2f1e6619

Comment 23

[Ethan Hugg [:ehugg]]

Comment on attachment 8464993 [details] [diff] [review]
Part 1: Implement prefs-based screenshare domain whitelist in MediaManager

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

::: modules/libpref/src/init/all.js
@@ +365,5 @@
>  // do not enable screensharing before implementing app/window sharing: Bug 1036653
>  // do not enable screensharing before source constraints are finalized: Bug 1033885
>  // do not enable screensharing before UX is ready: Bug 1035577
>  pref("media.getusermedia.screensharing.enabled", false);
> +pref("media.getusermedia.screensharing.allowed_domains", "");

The code in MediaManager.cpp has this as "allow_domains"

Comment 24

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

> >  pref("media.getusermedia.screensharing.enabled", false);
> > +pref("media.getusermedia.screensharing.allowed_domains", "");
> 
> The code in MediaManager.cpp has this as "allow_domains"

Thanks; will fix

Comment 25

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

Attachment: Fix typo in checkin for allowed_domains

Comment 26

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1622eb8d6fc

Comment 27

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bb9d2f1e6619
https://hg.mozilla.org/mozilla-central/rev/c1622eb8d6fc

Comment 28

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: fix_bug1037424.patch

Comment 29

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: fix_bug1037424.patch

Jesup asked on irc for this to use aPrivileged instead

Comment 30

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

Comment on attachment 8466393 [details] [diff] [review]
fix_bug1037424.patch

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

r+ if we have a global browser-sharing pref that isn't bypassed by priv'ed requests

Comment 31

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: fix_bug1037424.patch

Comment 32

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/16d2665da011

Comment 33

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

Comment on attachment 8464993 [details] [diff] [review]
Part 1: Implement prefs-based screenshare domain whitelist in MediaManager

Approval Request Comment
[Feature/regressing bug #]: N/A

[User impact if declined]: We need a whitelist to allow screensharing without security/privacy problems.

[Describe test coverage new/current, TBPL]: all the screensharing tests use this

[Risks and why]: quite low risk - only risk would be allowing it where it shouldn't.

[String/UUID change made/needed]: none

This would include the simple followups here

Comment 34

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/79626072eee7
https://hg.mozilla.org/releases/mozilla-aurora/rev/4f0cef5f09b4

Comment 35

[Nils Ohlmeier [:drno]]

(In reply to Paul Kerr [:pkerr] from comment #19)
> Are there any screensharing tests that need to have the whitelist handling
> added?

We have right now dom/media/tests/mochitest/test_getUserMedia_basicScreenshare.html 
                                                             _basicWindowshare.html
                                            test_peerConnection_basicScreenshare.html
                                                               _basicWindowshare.html
All these tests use mochi.test as domain. But as the tests are still passing I don't think we need to change anything here right now.

Comment 36

[Nils Ohlmeier [:drno]]

I went to http://mozilla.github.io/webrtc-landing/gum_test.html
By default I got a permission problem. After adding 'mozilla.github.io' to the user pref 'media.getusermedia.screensharing.allowed_domains' screen sharing was possible again.

Comment 37

[Nils Ohlmeier [:drno]]

This does not work with todays Aurora 33.0a2 (2014-08-11).
media.getusermedia.screensharing.enabled is true
media.getusermedia.screensharing.allowed_domains is an empty string

If I got to http://mozilla.github.io/webrtc-landing/gum_test.html with Aurora and click on 'Screen' or 'Window' the screen sharing starts instead of a permission error coming up.

Comment 38

[Nils Ohlmeier [:drno]]

If I compare the checkins to m-c: https://hg.mozilla.org/mozilla-central/rev/16d2665da011
With the uplift to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/4f0cef5f09b4

It appears that a couple of lines around this:
  if (!Preferences::GetBool("media.getusermedia.browser.enabled", false)) {
were not uplifted. Not sure if that is the root cause for Aurora to still give me a screen share without white listed domains.

Comment 39

[Nils Ohlmeier [:drno]]

(In reply to Nils Ohlmeier [:drno] from comment #37)
> This does not work with todays Aurora 33.0a2 (2014-08-11).

Turns out that for my test I had media.navigator.permission.disabled set to true. The result seems to be (universally on Aurora and Nightly) that every web page gets the permission to share any screen or window. Effectively the media.navigator.permission.disabled disables the media.getusermedia.screensharing.allowed_domains setting all together.
Is the interaction between media.getusermedia.screensharing.allowed_domains and media.navigator.permission.disabled anywhere documented?
My expectation would be that media.navigator.permission.disabled only affects the door hanger. But that as long as the domain is not contained in allowed_domains it would result it would not be allowed to screen share no matter how media.navigator.permission.disabled is set.
Shell: Who can help deciding how the interaction between these two settings is suppose to work?

Comment 40

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

> Turns out that for my test I had media.navigator.permission.disabled set to
> true. The result seems to be (universally on Aurora and Nightly) that every
> web page gets the permission to share any screen or window. Effectively the
> media.navigator.permission.disabled disables the
> media.getusermedia.screensharing.allowed_domains setting all together.
> Is the interaction between media.getusermedia.screensharing.allowed_domains
> and media.navigator.permission.disabled anywhere documented?

No, though the effect of permission.disabled has been to disable all permission checks, so this is just a continuation of the current behavior.

> My expectation would be that media.navigator.permission.disabled only
> affects the door hanger. But that as long as the domain is not contained in
> allowed_domains it would result it would not be allowed to screen share no
> matter how media.navigator.permission.disabled is set.
> Shell: Who can help deciding how the interaction between these two settings
> is suppose to work?

As these are used for about:config prefs, and permission.disabled in particular is primarily for enabling testing, we can decide.

Comment 41

[:shell escalante]

Randell's Comment 40 says that we can decide the behavior.  I would lean towards even though they disabled the door hanger on permissions that the whitelist is still respected (see comment 40).  Adding Adam and Mika for their input - in case that goes against the media.navigator.permission.disabled=true intent.

Comment 42

[Adam Roach [:abr]]

(In reply to sescalante from comment #41)
> Randell's Comment 40 says that we can decide the behavior.  I would lean
> towards even though they disabled the door hanger on permissions that the
> whitelist is still respected (see comment 40).  Adding Adam and Mika for
> their input - in case that goes against the
> media.navigator.permission.disabled=true intent.

Where Jesup says "primarily for enabling testing," I would state things more strongly. Anyone running with this pref set to "true" is begging for every page on the internet to grab their microphone and/or camera and spy on them. It's just a radically unsafe setting.

From that perspective, discussing whether the whitelist is respected is an extremely low-order bit. If you're running around on the internet at large with that pref set to true, then you've already given away pretty much every expectation of privacy.

What I *would* support is adding an obnoxiously large, persistent visual indicator whenever this pref is set so as to warn people that they're doing something extremely inadvisable.

Comment 43

[kolAflash]

First:
Though media.getusermedia.screensharing.allowed_domains was initially meant as temporary solution!?
> | Initial message by Florian Quèze [:florian] [:flo] 2014-07-11 06:34:38 PDT
> We want a whitelist to control in Firefox 33 the feature implemented in
> bug 983504, until we can figure out a correct privacy UI explaining to the
> user what the risks are.
Please think about ending this temporary solution now and switching to something more decent (after more than 10 Firefox releases)! I personally think the current solution for whitelisted domains is already pretty perfect for all domains. So just get rid of the whitelist!
"Would you like to share your screen with ..."
- Share Selected Items
- Don't Share
- Not Now

If you want to make it even more look like a warning add some (red colored) text like:
"Are you sure what you're doing? This will allow others to see everything that's happening on your screen or in the selected window."

And if you like to be even more careful, put a countdown to "Share Selected Items" for all domains not in media.getusermedia.screensharing.allowed_domains. So you can't click it before waiting at least 10 seconds.

BUT please get rid of excluding all not whitelisted domains completely from screen sharing!
This is privileging big, central providers and stopping the people from running their own WebRTC services on their own domain. E.g. running Jitsi Meet on your own server https://github.com/jitsi/jitsi-meet/ (don't confuse with the XMPP client Jitsi)



Second:
What for there is media.navigator.permission.disabled ?
In the current Firefox 46 it's enabling video- and audio-sharing without further question for all domains, while doing nothing to screen sharing.
I think it's indeed a very dangerous setting. Only reason I could think of is for developement reasons.
So what about replacing media.navigator.permission.disabled with a whitelist of domains, initially set to "" (empty string). So a developer can add the domains he's developing for.

Comment 44

[Eric Rescorla (:ekr)]

(In reply to kolAflash from comment #43)
> First:
> Though media.getusermedia.screensharing.allowed_domains was initially meant
> as temporary solution!?
> > | Initial message by Florian Quèze [:florian] [:flo] 2014-07-11 06:34:38 PDT
> > We want a whitelist to control in Firefox 33 the feature implemented in
> > bug 983504, until we can figure out a correct privacy UI explaining to the
> > user what the risks are.
> Please think about ending this temporary solution now and switching to
> something more decent (after more than 10 Firefox releases)! I personally
> think the current solution for whitelisted domains is already pretty perfect
> for all domains. So just get rid of the whitelist!
> "Would you like to share your screen with ..."
> - Share Selected Items
> - Don't Share
> - Not Now
> 
> If you want to make it even more look like a warning add some (red colored)
> text like:
> "Are you sure what you're doing? This will allow others to see everything
> that's happening on your screen or in the selected window."

It's this kind of language that is precisely the problem, because it gives people entirely the wrong intuition about the threat.

Comment 45

[Adam Roach [:abr]]

The hardest part of this problem has always been one of concisely conveying the *kind* of danger involved. Although the nature of the threat hasn't been explored in this bug, it's pretty well-worn in other conversations on the topic.  We need to make it clear to the user that they are granting the website access that will allow it to read their email or access their bank account. Basically, anything that CORS protects you from, sharing the browser window circumvents. "Dangerous" or "Very Dangerous" simply doesn't convey the gravity of the situation.

The fact that this is so incredibly non-obvious and not easy to explain, even to people who understand the web security model, is why this is taking so long to unwind. We need some way of informing users of the staggering gravity of what they're agreeing to, and no proposals to date have come anywhere close.

Comment 46

[kolAflash]

Understanding that. But please don't go the way to take the users freedom for their safety.

If you go that way, you might end up completely locking a computers root/administrator account, like iOS and Android do, to protect a user from himself.

On the other hand, you might be pushing users to install other software, maybe including nasty adware, for screen sharing.


An explicit warning and the choice to deny is the right thing and what I would absolutely expect. But don't trade freedom for safety without choice.

Comment 47

[kolAflash]

P.S.
Maybe put a permanent warning on the screen (always in foreground), instead of that little microphone+camera+screen icon.

http://picpaste.de/sharing.png

https://i.redditmedia.com/cw-Bm7tyhDJ3_cyF_A5qQLR5EgzezBUl-WmpUspatec.jpg?w=1024&s=abc44b4ad46c7c9907b32c8d8001d72b


A better permanent warning could be something telling:

<icon firefox/><icon micrphone/><icon webcam/><icon screensharing/>
<red>Attention you're sharing your screen. <button>Stop that!</button></red>
Sharing will automatically stop in 2:42 Minutes. <button>extend by 3 Minutes</button>

Mockup:
http://picpaste.de/warning-proposal-bnQzQRQ1.png


By the way, maybe you should do some similar, better warning for webcam/microphone sharing too. But please don't take the users freedom to share those data (webcam, microphone, screen) with a freely chosen domain at all.

Comment 48

[Eric Rescorla (:ekr)]

(In reply to kolAflash from comment #47)
> P.S.
> Maybe put a permanent warning on the screen (always in foreground), instead
> of that little microphone+camera+screen icon.
> 
> http://picpaste.de/sharing.png
> 
> https://i.redditmedia.com/cw-Bm7tyhDJ3_cyF_A5qQLR5EgzezBUl-WmpUspatec.
> jpg?w=1024&s=abc44b4ad46c7c9907b32c8d8001d72b
> 
> 
> A better permanent warning could be something telling:
> 
> <icon firefox/><icon micrphone/><icon webcam/><icon screensharing/>
> <red>Attention you're sharing your screen. <button>Stop that!</button></red>
> Sharing will automatically stop in 2:42 Minutes. <button>extend by 3
> Minutes</button>
> 
> Mockup:
> http://picpaste.de/warning-proposal-bnQzQRQ1.png

I think you've misunderstood the issue, which is not about user visibility of sharing being on , but about the interaction with CSRF.

> By the way, maybe you should do some similar, better warning for
> webcam/microphone sharing too. But please don't take the users freedom to
> share those data (webcam, microphone, screen) with a freely chosen domain at
> all.

Comment 49

[kolAflash]

(In reply to Eric Rescorla (:ekr) from comment #48)
> [...]
> I think you've misunderstood the issue, which is not about user visibility
> of sharing being on , but about the interaction with CSRF.

I guess I understand the issue. But please tell me if I'm wrong!

As far as I know, an attacker could capture everything that's on the users screen. Including what's in the Firefox address bar and might contain GET variables carrying user credentials.

Or is there any other internal, technical relation?
After all, the captured screen should be just another video stream like the one from the webcam. Isn't it!?

Or is there any difference, to when I'm just capturing my screen using an USB webcam (holding it in from of my screen) and share the webcams video stream via WebRTC?

Comment 50

[Eric Rescorla (:ekr)]

(In reply to kolAflash from comment #49)
> (In reply to Eric Rescorla (:ekr) from comment #48)
> > [...]
> > I think you've misunderstood the issue, which is not about user visibility
> > of sharing being on , but about the interaction with CSRF.
> 
> I guess I understand the issue. But please tell me if I'm wrong!
> 
> As far as I know, an attacker could capture everything that's on the users
> screen. Including what's in the Firefox address bar and might contain GET
> variables carrying user credentials.
> 
> Or is there any other internal, technical relation?
> After all, the captured screen should be just another video stream like the
> one from the webcam. Isn't it!?
> 
> Or is there any difference, to when I'm just capturing my screen using an
> USB webcam (holding it in from of my screen) and share the webcams video
> stream via WebRTC?

The point is that the Web site can *cause* window open or navigation events, so it can (for instance) open your gmail window and then look at it with the screen sharing mechanism. It can do it briefly, and it can use vision tracking to see when you are looking away. See https://lists.w3.org/Archives/Public/public-webrtc/2013Mar/0024.html for a long writeup of this.

Comment 51

[kolAflash]

OK, nice attack vector!
This also might be problem for whitelisted domains, if undermined by an attacker...


So what about asking the user this (also for whitelisted domains):

Would you like to allow "..." to (partially) control and capture your screen?

Comment 52

[kolAflash]

P.S.
This also might be a problem for whitelisted domains, if undermined by an attacker... ... or completely under control by an intelligence agency or something similar.


So I'd suggest to improve the warning for all domains and maybe also include a countdown for the button, but removing whitelist.

Additionally the warning could contain a hyperlink-formatted (blue, underlined) text "understand the dangers" which gives a four or five sentences tooltip-explanation of the attack vector on click/mouseover.

Comment 53

[Adam Roach [:abr]]

I'd like you to look at how much back-and-forth it took you with an expert on this topic to understand the depth of the danger involved.

Then, take a deep breath. Close your eyes. Concentrate on the ramifications of that fact. Focus. Take several minutes.

After you do that, come back and tell us, with a straight face, that you know how to present this to users in a way that they'll get it.

Comment 54

[john]

First, thanks for working on Firefox -- it's great to have a choice of browsers!

I'm wondering if there's been any progress on the "correct privacy UI" mentioned at the beginning of this thread?  

It feels like you are trying to protect users from themselves. When someone clicks a spear phishing email link and it affects the outcome of an election, for example, does everybody blame their email client? 

The internet is a dangerous place and you are right to worry about security for this new capability. But ultimately, it seems like all a browser can do is ask the user if they trust the hosting web site with the capability in question. Microphone and video capture seem very similar to the screen sharing use case. An open mic could capture someone saying their social security number on the phone.

Comment 55

[john]

<straightface>

Proposed language: "shoulderlooker.com wants to view the contents of your entire screen."

[the usual options] 

</straightface>

Comment 56

[Eric Rescorla (:ekr)]

1. This has been fixed in Firefox 52.
2. Microphone and video aren't the same for the reasons indicated upthread

Comment 57

[john]

(In reply to Eric Rescorla (:ekr) from comment #56)
> 1. This has been fixed in Firefox 52.
> 2. Microphone and video aren't the same for the reasons indicated upthread

awesome thank you!