1264595 - test isolation by mediaSource URI by first party domain (Tor 15703)

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

We need to have some tests for verifying isolation by mediaSource URI by first party domain.

TOR bug: 15703.

Comment 1

[Arthur Edelstein [:arthur]]

Here's a link that tracks the Tor Browser regression test patch for media source isolation:
https://torpat.ch/15703

Comment 2

[Dave Huseby [:huseby]]

Attachment: WIP patch to add tests from Tor Browser.

Comment 4

[Jonathan Hao (inactive) [:jhao]]

Attachment: Test mediaSource URL isolation.

WIP patch.  I don't know why I always get video element error.

Comment 5

[Jonathan Hao (inactive) [:jhao]]

I can't make the original Tor's test work in no-isolation mode either.

It turns out the media source object is auto revoked when the event loop reaches a "stable state" according to bug 1116382.  The code is here: http://searchfox.org/mozilla-central/rev/064025c802c22cd5ad122746733cbd34ea47393c/dom/url/URL.cpp#279

That's why by the time I pass that url to the other tab, it's already revoked.

I don't think we can test this anymore.  Arthur, do you agree?

Comment 6

[Jonathan Hao (inactive) [:jhao]]

According to https://developer.mozilla.org/zh-TW/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIAppShell#runInStableState()

The runnable will be executed once current event finishes.  I don't know if we can force the first tab to be stuck in the same event.

Comment 7

[Arthur Edelstein [:arthur]]

(In reply to Jonathan Hao [:jhao] from comment #5)
> I can't make the original Tor's test work in no-isolation mode either.
> 
> It turns out the media source object is auto revoked when the event loop
> reaches a "stable state" according to bug 1116382.  The code is here:
> http://searchfox.org/mozilla-central/rev/
> 064025c802c22cd5ad122746733cbd34ea47393c/dom/url/URL.cpp#279
> 
> That's why by the time I pass that url to the other tab, it's already
> revoked.
> 
> I don't think we can test this anymore.  Arthur, do you agree?

Yes, I think you are right. This behavior was introduced in bug 1116382 and was designed to follow the W3C recommendation to autorevoke mediasource URIs. Recently, however, the media-source draft spec has been changed to recommend manual revocation instead, to mirror the File API behavior:
Old: https://www.w3.org/TR/2014/CR-media-source-20140717/#widl-URL-createObjectURL-DOMString-MediaSource-mediaSource
New: https://www.w3.org/TR/2016/CR-media-source-20160705/#widl-URL-createObjectURL-DOMString-MediaSource-mediaSource
Here is the committee's discussion on this change:
https://github.com/w3c/media-source/issues/10

As a manual test of the autorevocation behavior, I forked the example here:
https://developer.mozilla.org/en-US/docs/Web/API/MediaSource#Examples
Here is my fork:
https://arthuredelstein.github.io/netfix/demo/bufferAll.html
And here is what I changed:
https://github.com/arthuredelstein/netfix/commit/5d015a4ccd7e1b1dfab38c8e233a569b2a8931a7

The video fails on Firefox because the mediaSource URI has been revoked by the time the function passed to setTimeout runs. It succeeds on Google Chrome.

I think we should leave this bug open given that it seems somewhat likely that autorevocation will be removed, to reflect the new recommendations in the W3C spec. Karl, do you have an opinion on this?

Comment 8

[Karl Tomlinson (:karlt)]

(In reply to Arthur Edelstein [:arthuredelstein] from comment #7)
Yes, and thanks for the links.

I don't think autorevocation should be removed and filed
https://github.com/w3c/media-source/issues/156
to revert that change.

I expect async messages will be processed after the auto-revocation happens (in a single content process world), unless we have some bugs with nested event loops.
I don't know, but I wonder whether there is a way for one page to trigger synchronous execution of JS in an iframe.

Comment 9

[Jonathan Hao (inactive) [:jhao]]

Thanks to Arthur and Karl.  I'll put this bug into backlog.

Comment 10

[Jonathan Hao (inactive) [:jhao]]

This bug keeps showing in bug tracking pages, but it's kind of annoying that we can't do anything right now.  I propose that we wrote a test to check that mediaSource URL is indeed revoked in the next event, and close this bug.  If the auto-revokation is removed in the future, that test will break and we'll know that it's time to do the isolation test.

Comment 11

[Jonathan Hao (inactive) [:jhao]]

Attachment: Test whether auto-revocation is removed.

This test is to track the auto-revocation behavior, so that if auto-revocation is removed, we should be noticed.

Karl, may I ask you to review this patch?

Comment 12

[Jonathan Hao (inactive) [:jhao]]

Attachment: Test mediaSource URL isolation, adapted from Tor Browser patch 15703.

Hi Arthur and Baku, do you think we should land this test even though we have to skip it?

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 8799658 [details] [diff] [review]
Test mediaSource URL isolation, adapted from Tor Browser patch 15703.

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

no, we should have a valid test.

::: browser/components/originattributes/test/browser/browser_mediaSourceURLIsolation.js
@@ +52,5 @@
> +  return result;
> +}
> +
> +IsolationTestTools.runTests(TEST_PAGE, doTest);
> +

extra line

Comment 14

[Arthur Edelstein [:arthur]]

Hi, Jonathan. One possibility that occurs to me would be to introduce some code that experiments at runtime to see if autorevocation is in effect. If the experiment determines that autorevocation is not happening, then the code would proceed with isolation tests. That way, in the future, if Firefox is patched to use manual revocation again, then the isolation tests will be already present.

Comment 15

[Jonathan Hao (inactive) [:jhao]]

That's an interesting idea.  So you mean we write a test that checks the auto-revocation behavior first.  If it's still there, then we do nothing.  Otherwise we run our isolation test.

Baku, do you think this is valid?

Comment 16

[Karl Tomlinson (:karlt)]

Comment on attachment 8799645 [details] [diff] [review]
Test whether auto-revocation is removed.

This is also tested by https://hg.mozilla.org/mozilla-central/rev/d71b2d0ce994 but having a Mozilla-specific tests with a Mozilla-specific message seems reasonable.

There was also talk of removing stalled events with MSE, but that is only in the failure path, which should not be reached, so not a problem.

Comment 17

[Jonathan Hao (inactive) [:jhao]]

Let's land the auto-revocation test only.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/336d0983b749
Test whether auto-revocation is removed. r=karlt

Comment 19

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/336d0983b749