1049087 - pre-populate the whitelist for screensharing in Fx33

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Maire Reavy [:mreavy]]

For Fx33, we should pre-populate the whitelist with certain domains so that the feature is more useful "out of the box" for users.

The user retains the ability to update the whitelist at any time by going into about:config.

Comment 1

[Ethan Hugg [:ehugg]]

I'd like to point out that the implementation does not support wildcards.  Each domain in the list must be an exact match.  Code is here:
http://mxr.mozilla.org/mozilla-central/source/dom/media/MediaManager.cpp#137

Comment 2

[Ethan Hugg [:ehugg]]

I don't have a list of domains yet but I'm pretty sure we'll need wildcards for subdomains so we can add "*.webex.com" for example.

This is mentioned on bug 1037424 here as a "possible part 2":
https://bugzilla.mozilla.org/show_bug.cgi?id=1037424#c18

Comment 3

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

Attachment: set temporary allowed screensharing domain of mozilla.github.io - replace for release

Comment 4

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

Comment on attachment 8481304 [details] [diff] [review]
set temporary allowed screensharing domain of mozilla.github.io - replace for release

Approval Request Comment
[Feature/regressing bug #]: screensharing

[User impact if declined]: harder for people to test in beta

[Describe test coverage new/current, TBPL]: screensharing tests in tbpl use this

[Risks and why]: Provide a temporary value for the whitelist to allow testers to use http://mozilla.github.io/webrtc-landing/gum_test.html to verify screensharing.  Should be replaced by mid-beta with whatever we decide will be the initial release whitelist.

[String/UUID change made/needed]: none

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8481304 [details] [diff] [review]
set temporary allowed screensharing domain of mozilla.github.io - replace for release

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

::: modules/libpref/init/all.js
@@ +371,5 @@
>  #endif
>  
>  pref("media.getusermedia.screensharing.enabled", true);
> + // temporary value, not intended for release - bug 1049087
> +pref("media.getusermedia.screensharing.allowed_domains", "mozilla.github.io");

If it's not meant for release you should protect it with !defined(RELEASE_BUILD).

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c86bdb3ff21

Comment 7

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/2b60f30db362 for something in that push causing b2g build bustage like https://tbpl.mozilla.org/php/getParsedLog.php?id=47078117&tree=Mozilla-Inbound

Comment 8

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/618e47b766b5

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/618e47b766b5

Comment 10

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/4422793dc9f5

Comment 11

[Ethan Hugg [:ehugg]]

Attachment: pre-populate the whitelist for screensharing in Fx33

Comment 12

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8484458 [details] [diff] [review]
pre-populate the whitelist for screensharing in Fx33

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

::: modules/libpref/init/all.js
@@ +380,5 @@
>  #endif
>  
>  pref("media.getusermedia.screensharing.enabled", true);
>  #ifdef RELEASE_BUILD
> +pref("media.getusermedia.screensharing.allowed_domains", "webex.com,*.webex.com,collaborate.com,*.collaborate.com");

I see it as problematic that none of these domains use HTTPS+HSTS, because that means this protection is really trivial to circumvent in many cases.

Comment 13

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

Comment on attachment 8484458 [details] [diff] [review]
pre-populate the whitelist for screensharing in Fx33

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

Note: we've restricted screen/windowsharing to https: in a different bug

Comment 14

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

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

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/efb085f286fd

Comment 16

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

Comment on attachment 8484458 [details] [diff] [review]
pre-populate the whitelist for screensharing in Fx33

Approval Request Comment
[Feature/regressing bug #]: screensharing

[User impact if declined]: inability to access approved sites

[Describe test coverage new/current, TBPL]: nightly, externally tested by Cisco

[Risks and why]: virtually none

[String/UUID change made/needed]: none

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e6bf3fa80963

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/90713d332601