Maire Reavy [:mreavy] Reporter
	Description • 10 years ago

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.

	Ethan Hugg [:ehugg] Assignee
	Comment 1 • 10 years ago

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

	Ethan Hugg [:ehugg] Assignee
	Comment 2 • 10 years ago

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

	Randell Jesup [:jesup] (needinfo me)
	Comment 4 • 10 years ago

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

	Gian-Carlo Pascutto [:gcp]
	Comment 5 • 10 years ago

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).

	Randell Jesup [:jesup] (needinfo me)
	Comment 6 • 10 years ago

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

	Phil Ringnalda (:philor)
	Comment 7 • 10 years ago

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

	Randell Jesup [:jesup] (needinfo me)
	Comment 8 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 10 years ago

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

	Randell Jesup [:jesup] (needinfo me)
	Comment 10 • 10 years ago

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

	Gian-Carlo Pascutto [:gcp]
	Comment 12 • 10 years ago

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.

	Randell Jesup [:jesup] (needinfo me)
	Comment 13 • 10 years ago

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

	Randell Jesup [:jesup] (needinfo me)
	Comment 14 • 10 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 15 • 10 years ago

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

	Randell Jesup [:jesup] (needinfo me)
	Comment 16 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 10 years ago

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