1141386 - [EME] gmp-clearkey's base64decode doesn't handle padding properly

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

My last change to gmp-clearkey's base64 key/Id decode didn't handle padding properly. It assumed that the input key was always 22 bytes long, i.e. that JS always stripped the pading '=' characters from base64 encoded key/Ids. This is not always the case.

Comment 1

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

Attachment: Patch

Strip trailing padding, and only then validate the length of the key/Id for base64 decode.

We should get this fix landed ASAP. I'm going to write a gtest so we don't regress...

Comment 2

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

Attachment: Patch 2: Add gtest for gmp-clearkey's base64 decode

Add a gtest, so we don't regress this...

Comment 3

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/6fdc1033f217
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2de53dd52d4

Comment 4

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

Of course, WAE strikes again:

Backout.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c40f6146cb1

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b4b21a37cfa5

Comment 6

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/026d6fc9f516
https://hg.mozilla.org/integration/mozilla-inbound/rev/f98b2e123dce

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6fdc1033f217
https://hg.mozilla.org/mozilla-central/rev/c2de53dd52d4
https://hg.mozilla.org/mozilla-central/rev/8c40f6146cb1
https://hg.mozilla.org/mozilla-central/rev/026d6fc9f516
https://hg.mozilla.org/mozilla-central/rev/f98b2e123dce

Comment 8

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

Comment on attachment 8574982 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME won't work on some EME sites
[Describe test coverage new/current, TreeHerder]: There's a test in the next patch. Performed local testing too.
[Risks and why]: Low; isolated to EME, and I've tested the target site.
[String/UUID change made/needed]: None.

Comment 9

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

Comment on attachment 8575028 [details] [diff] [review]
Patch 2: Add gtest for gmp-clearkey's base64 decode

Approval Request Comment
[Feature/regressing bug #]: EME
[User impact if declined]: EME won't work on some EME sites
[Describe test coverage new/current, TreeHerder]: This is the test for the fix to EME in the preceeding patch.
[Risks and why]: Low; isolated to EME, and I've tested the target site.
[String/UUID change made/needed]: None.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5a30ae259c1e
https://hg.mozilla.org/releases/mozilla-aurora/rev/1de2809cfcd6