Don't auto allow access to media devices unless a the security origin of the requester is the same …

Don't auto allow access to media devices unless a the security origin of the requester is the same as its ancestors.

This adds a field in MediaStreamRequest to verify that the security origin of the requester is the same as its ancestors.
This field have to be set on the UI-thread and is therefore set just before sending the request to the UI.

BUG=448378
TEST= please see bug report

Committed: https://crrev.com/ac8f9263b91c2b8b5c5ae7aa71af34c46f50d22d
Cr-Commit-Position: refs/heads/master@{#313904}

[perkj_chrome, 2015-01-15 16:13:49]

perkj@chromium.org changed reviewers:
+ tommi@chromium.org

[perkj_chrome, 2015-01-15 16:13:50]

for discussion.

[perkj_chrome, 2015-01-23 10:29:56]

On 2015/01/15 16:13:50, perkj wrote:
> for discussion.
ping

[tommi (sloooow) - chröme, 2015-01-23 10:44:04]

the whole approach lg. can you ping me again after making the changes you
mentioned offline?

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
content/browser/renderer_host/media/media_stream_manager.cc:299: bool
HasUIRequest() const { return ui_request_.get() != NULL; }
nit: nullptr

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
content/browser/renderer_host/media/media_stream_manager.cc:300:
scoped_ptr<MediaStreamRequest> GetUIRequest() { return ui_request_.Pass(); }
Since this transfers ownership I'd like to give it a more descriptive name.
'Get' methods can usually be called twice in a row and return the same value. 
What about DetachUIRequest()?  'Relinquish' could also be used.

[perkj_chrome, 2015-01-26 13:43:39]

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
content/browser/renderer_host/media/media_stream_manager.cc:299: bool
HasUIRequest() const { return ui_request_.get() != NULL; }
On 2015/01/23 10:44:04, tommi wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
content/browser/renderer_host/media/media_stream_manager.cc:300:
scoped_ptr<MediaStreamRequest> GetUIRequest() { return ui_request_.Pass(); }
On 2015/01/23 10:44:04, tommi wrote:
> Since this transfers ownership I'd like to give it a more descriptive name.
> 'Get' methods can usually be called twice in a row and return the same value. 
> What about DetachUIRequest()?  'Relinquish' could also be used.

Done.

[perkj_chrome, 2015-01-26 15:13:09]

On 2015/01/26 13:43:39, perkj wrote:
>
https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
> File content/browser/renderer_host/media/media_stream_manager.cc (right):
> 
>
https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
> content/browser/renderer_host/media/media_stream_manager.cc:299: bool
> HasUIRequest() const { return ui_request_.get() != NULL; }
> On 2015/01/23 10:44:04, tommi wrote:
> > nit: nullptr
> 
> Done.
> 
>
https://codereview.chromium.org/795703003/diff/60001/content/browser/renderer...
> content/browser/renderer_host/media/media_stream_manager.cc:300:
> scoped_ptr<MediaStreamRequest> GetUIRequest() { return ui_request_.Pass(); }
> On 2015/01/23 10:44:04, tommi wrote:
> > Since this transfers ownership I'd like to give it a more descriptive name.
> > 'Get' methods can usually be called twice in a row and return the same
value. 
> > What about DetachUIRequest()?  'Relinquish' could also be used.
> 
> Done.

ok - I think this is ready for another round.

[tommi (sloooow) - chröme, 2015-01-27 21:07:14]

lgtm

https://codereview.chromium.org/795703003/diff/100001/chrome/browser/media/me...
File chrome/browser/media/media_stream_devices_controller.cc (right):

https://codereview.chromium.org/795703003/diff/100001/chrome/browser/media/me...
chrome/browser/media/media_stream_devices_controller.cc:503: if
(!request_.all_ancestors_have_same_origin)
nit: Would it make sense to have a UMA stat for this?

https://codereview.chromium.org/795703003/diff/100001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/795703003/diff/100001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:378: 
fix whitespace

[perkj_chrome, 2015-01-28 16:19:56]

perkj@chromium.org changed reviewers:
+ alexmos@chromium.org

[perkj_chrome, 2015-01-28 16:19:57]

Hi Alex, can you take a look at the change in FrameTreeNode I made to check the
parent security origin and its use?

[alexmos, 2015-01-28 18:13:50]

LGTM.

Just a note that url::Origin::IsSameAs does not currently do the right thing for
file:// origins and unique origins (such as for data urls).  They are all
serialized as "null" and would be considered equal to one another.  Not sure if
this matters for your scenario.

[tommi (sloooow) - chröme, 2015-01-28 18:19:09]

On 2015/01/28 18:13:50, alexmos wrote:
> LGTM.
> 
> Just a note that url::Origin::IsSameAs does not currently do the right thing
for
> file:// origins and unique origins (such as for data urls).  They are all
> serialized as "null" and would be considered equal to one another.  Not sure
if
> this matters for your scenario.

Thanks for the heads up. It should be fine for this case since device access is
by default not allowed for file://.

[tommi (sloooow) - chröme, 2015-01-28 18:19:25]

The CQ bit was checked by tommi@chromium.org

[commit-bot: I haz the power, 2015-01-28 18:19:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/795703003/100001

[commit-bot: I haz the power, 2015-01-28 18:27:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-28 18:27:02]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[perkj_chrome, 2015-01-28 20:28:33]

perkj@chromium.org changed reviewers:
+ jochen@chromium.org

[perkj_chrome, 2015-01-28 20:28:34]

Jochen, would you mind the content/public changes ?

[jochen (gone - plz use gerrit), 2015-01-29 15:22:15]

lgtm

[perkj_chrome, 2015-01-30 11:12:05]

The CQ bit was checked by perkj@chromium.org

[commit-bot: I haz the power, 2015-01-30 11:12:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/795703003/120001

[commit-bot: I haz the power, 2015-01-30 12:43:19]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-30 12:44:06]

Patchset 7 (id:??) landed as
https://crrev.com/ac8f9263b91c2b8b5c5ae7aa71af34c46f50d22d
Cr-Commit-Position: refs/heads/master@{#313904}