1101304 - [EME] MediaKeys::CheckPrincipal needs to take into account CORS

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

The MedaaKeys' CheckPrincipal() function needs to take into account CORS, otherwise it can't play an EME media file that is allowed to be played on its origin.

Comment 1

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1101304.patch

From my reading of the spec, CheckPrincipals isn't necessary. We just need to null the |initData| field of the encrypted event if the media and page are CORS cross-origin.

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1101304-test.patch

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8554779 [details] [diff] [review]
1101304-test.patch

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

::: dom/media/test/test_eme_access_control.html
@@ +28,5 @@
> +    manager.finished(token);
> +  });
> +
> +  v.src = uri;
> +  v.play();

Do you need to play() the video? I'd have thought not.

@@ +40,5 @@
> +  manager.started(token);
> +
> +  var v = document.createElement("video");
> +  v.crossOrigin = true;
> +

Add an encrypted event listener and assert it doesn't fire.

@@ +53,5 @@
> +
> +function startTest(test, token)
> +{
> +  TestNoCORS(test, token);
> +  TestWithCORS(test, token);

You should also have a CORS same-origin case, which should successfully load and in which the initData is visible.

This case should call allowed.sjs?short-cenc.mp4 or somesuch, to ensure that the access control headers are set on the resource load.

Also, we need to ensure that MSE behaves correctly cross origin too. Can you please test all these cases in MSE too?

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8554778 [details] [diff] [review]
1101304.patch

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

I will refrain from reviewing this until the test has cross origin XHR+MSE cases.

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1101304-test.patch v2

Addressed review comments.

test_eme_playback now loads eme.js from |test1.m.t| origin, and has a test case for loading MSE data from |test2.m.t| origin.

Comment 7

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8555668 [details] [diff] [review]
1101304-test.patch v2

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

Thanks!

Comment 8

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/470dc6922fa2
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dcf0be77eab

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/470dc6922fa2
https://hg.mozilla.org/mozilla-central/rev/9dcf0be77eab

Comment 10

[Chris Pearce [:cpearce (Not reading bugmail)]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7503ad43a7fd
https://hg.mozilla.org/releases/mozilla-beta/rev/7bc573c193ea

Comment 11

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Beta patch

Patch for beta branch as part of EME platform uplift.

Comment 12

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8572324 [details] [diff] [review]
Beta patch

Requesting retroactive approval for Beta landing as part of EME platform uplift.

Comment 13

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Beta patch

Patch for beta branch as part of EME platform uplift.

Comment 14

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8572327 [details] [diff] [review]
Beta patch

Requesting retroactive approval for Beta landing as part of EME platform uplift.

Comment 15

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8572324 [details] [diff] [review]
Beta patch

Previously approved as part of the EME platform landing on Beta.