1369548 - DRM video taints the particular video element permanently

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P3)

Description

[Ying]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce:

Purchase a $0.00 movie from YouTube if you don't already have a test DRM movie.
(https://www.youtube.com/watch?v=MeFoUwes8nE for instance)
A windows machine is probably needed for DRM support. 

1) start playing the movie
2) click YouTube logo to go to home page, select any normal non-DRM video
3) when video starts playing, do the following:

videoElem = $('video');
canvasElem = document.createElement('canvas');
document.body.appendChild(canvasElem);
ctx2d = canvasElem.getContext('2d');
ctx2d.drawImage(videoElem, 0, 0);


Actual results:

[Exception... "Component is not available"  nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)"  location: "JS frame :: debugger eval code :: <TOP_LEVEL> :: line 6"  data: no]


Expected results:

Try the same on Edge or Chrome, observe that the drawImage call is successful, and that the video frame shows up at the bottom of the page.

Playing a non-DRM video on a video element that has previously been used for DRM content should clear the DRM tainting.

Comment 1

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

I followed up out of band about this, documenting here to clear the need-info.

Firefox will throw on calls to HTMLMediaElement.captureStream if the HTMLMediaElement has a mediaKeys attached. YouTube attaches a MediaKeys to play the DRM video, and then attempts to unset the MediaKeys by calling HTMLMediaElement.setMediaKeys(null) when switching resources, but unfortunately still has a resource loaded in the HTMLMediaElement at the time setMediaKeys(null) is called, and our decode pipeline does not support the MediaKeys changing while a resource is loaded.

So the long term fix is for us to change our media pipeline to handle having the CDM being transplanted/removed at runtime. Or even just special case setMediaKeys(null) in some non-terrible fashion.

Comment 2

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

Probably Kilik could take this. May need help from jw or jya as fixing this will involve changes to how the MDSM and MFR respond when the CDMProxy is removed/re-added.

Comment 3

[Kilik Kuo [:kikuo] (inactive)]

Just came back from vacation, ni? me to track.

Comment 4

[Kilik Kuo [:kikuo] (inactive)]

Attachment: Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

Review commit: https://reviewboard.mozilla.org/r/162962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/162962/

Comment 5

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8891933 [details]
Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

https://reviewboard.mozilla.org/r/162962/#review168252

I could not rent/purchase DRM content from Google play store or YT because I have no "valid" credit card for these services :( (being told invalid card / unqualified user was frustrating...) Anyway, I played around and wrote a test script to simulate this operation [1].

I found that if |HTMLMediaElement::AbortExistingLoads| which is implicitly invoked by calling i.e.
a) videoelement.src = null
b) videoelement.load()
is called before |HTMLMediaElement::SetMediaKey(null)|, the context2d could drawImage correctly with the following clear content.
That is because mDecoder is shutted down and is set to null in HTMLMediaElement, so the attached mMediaKey could be unbound from HTMLMediaElement.

[1] https://github.com/kilikkuo/H2TPServer/blob/master/eme_playback/test_eme_unset_mediakeys_and_capture.html


Then I had a chance to rent a video via certain "valid" credit card and checked YT's behavior,
it would go like this after clicking the logo while playing an encrypted content,
1) videoelement.setMediaKeys(null);
2) videoelement.load();

So, for this issue, the harmless way (tested　ａｎｄ　ｗｏｒｋｅｄ)　is to break the association between MediaElement and MediaKey when MediaKey is set to null no matter what.
And maybe a follow-up could be to siganl MediaDecoder that the cdm proxy is invalid now, and all calls to it should be alarmed.

Chris, any ideas ?

Comment 6

[Kilik Kuo [:kikuo] (inactive)]

(In reply to Kilik Kuo [:kikuo] from comment #5)
> 
> [1]
> https://github.com/kilikkuo/H2TPServer/blob/master/eme_playback/
> test_eme_unset_mediakeys_and_capture.html
> 

Also, I assume there's no similar test right now for this case.
I need to make the above a new mochitest then.

Comment 7

[Blake Wu [:bwu][:blakewu]]

(In reply to Kilik Kuo [:kikuo] from comment #5)
> Comment on attachment 8891933 [details]
> Bug 1369548 - Detach MediaKey from HTMLMediaElement when setting MediaKey to
> null.
> 
> https://reviewboard.mozilla.org/r/162962/#review168252
> 
> I could not rent/purchase DRM content from Google play store or YT because I
> have no "valid" credit card for these services :( (being told invalid card /
> unqualified user was frustrating...) Anyway, I played around and wrote a
> test script to simulate this operation [1].
If you need to check YT DRM content, just use my credit card to buy a movie instead of renting one. I should not have rented it last time..
I will expense it.

Comment 8

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

Comment on attachment 8891933 [details]
Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

https://reviewboard.mozilla.org/r/162962/#review170100

::: dom/html/HTMLMediaElement.cpp:6909
(Diff revision 1)
>      // with a new DOMException whose name is NotSupportedError.
>  
>      // 5.2.2 If the association cannot currently be removed, let this object's
>      // attaching media keys value be false and reject promise with a new
>      // DOMException whose name is InvalidStateError.
> -    if (mDecoder) {
> +    if (mDecoder && aMediaKeys != nullptr) {

This is going in the right direction, but the CDMProxy from the old MediaKeys will still be set connected to the MediaDecoder. So the HTMLMediaElement will still be able to play the restricted content, and it really should not be able to if the MediaKeys object has been unset.

We need to also do something in the MediaDecoder to tell it to forget its CDMProxy. We'll probably need to have it shutdown its MediaDataDecoders, and revert to a "waiting for CDM" state.

Ideally re-attaching the same MediaKeys would result in the loaded media being able to be played again.

And also attaching a new MediaKeys instancec should result in the loaded media being able to be played again after new license requests were issued. The player would need to have the initdata available, that is, it would need to not rely on receiving an "encrypted" event when the second MediaKeys is attached.

Anything less than this, and we're liable to find new edge cases some time in future.

Comment 9

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8891933 [details]
Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

https://reviewboard.mozilla.org/r/162962/#review170100

> This is going in the right direction, but the CDMProxy from the old MediaKeys will still be set connected to the MediaDecoder. So the HTMLMediaElement will still be able to play the restricted content, and it really should not be able to if the MediaKeys object has been unset.
> 
> We need to also do something in the MediaDecoder to tell it to forget its CDMProxy. We'll probably need to have it shutdown its MediaDataDecoders, and revert to a "waiting for CDM" state.
> 
> Ideally re-attaching the same MediaKeys would result in the loaded media being able to be played again.
> 
> And also attaching a new MediaKeys instancec should result in the loaded media being able to be played again after new license requests were issued. The player would need to have the initdata available, that is, it would need to not rely on receiving an "encrypted" event when the second MediaKeys is attached.
> 
> Anything less than this, and we're liable to find new edge cases some time in future.

Most of the changes were done in Bug 1395922. Thanks for the guidance !

Comment 10

[Kilik Kuo [:kikuo] (inactive)]

Comment on attachment 8891933 [details]
Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/162962/diff/1-2/

Comment 11

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

Comment on attachment 8891933 [details]
Bug 1369548 - HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down.

https://reviewboard.mozilla.org/r/162962/#review205192

Comment 12

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

Thanks for taking this on!

Comment 13

[Kilik Kuo [:kikuo] (inactive)]

It seems that after YT has changed the style of its site, the script described in Comment 0 cannot actually display the drawn image on the bottom of the page even on Chrome. But this fix is still needed.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9941e68b5a53
HTMLMediaElement::SetMediaKeys should continue working even the decoder is shutting down. r=cpearce

Comment 15

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/9941e68b5a53