1436226 - Hardcode VP8/VP9 algorithm choice when resisting fingerprinting

Status: NEW

(Core :: Audio/Video: Playback, enhancement, P2)

Description

[Arthur Edelstein [:arthur]]

In Tor Browser, we set pref("media.benchmark.vp9.threshold", 0) to prevent fingerprinting of users via their browser's vp8/vp9 algorithm choice. Here's the original patch:

https://torpat.ch/22548

and the original ticket:

https://trac.torproject.org/22548

We'd like to propose introducing a patch here that results in the same behavior when privacy.resistFingerprinting=true. Namely, Android and Mac use vp8, and Windows and Linux use vp9.

Comment 1

[Ralph Giles (:rillian)]

We can't take an unconditional patch like this. Bryce, can you take a look and see if there's anything tor-specific we could hook to disable the test?

Comment 2

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

Here's a fix using the privacy.resistFingerprinting pref. Would this be acceptable?

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

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

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

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

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

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

Comment 6

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

I think I'm mostly happy with where that test has ended up. I'll land it as part of a separate bug pending below discussion. It highlighted the following which I want to make sure is okay before we land the patch.

The check which returns if `webm/video`, for some given codec, IsTypeSupported does a few things[0]. My reading of it is that (ignoring AV1 here):

- `media.mediasource.webm.enabled` being true means we'll always say we have support. This means the pref trumps resist fingerprinting in this case. It looks like for the test harness this is always set on[1], and will be set on a platform specific basis in the browser[2].
- If the codec is vp8 we report support.
- If IsWebMForced returns true we report support. The above patch means that this will always return true for Linux and Windows.

My concern is that `media.mediasource.webm.enabled` can shift under us, and trumps the resit fingerprinting.

I'm not familiar with the Tor Browser changes, is there anything there the forces that ensures the value of `media.mediasource.webm.enabled`?

[0] https://searchfox.org/mozilla-central/source/dom/media/mediasource/MediaSource.cpp#130
[1] https://searchfox.org/mozilla-central/source/testing/profiles/prefs_general.js#195
[2] https://searchfox.org/mozilla-central/source/modules/libpref/init/all.js#592

Comment 7

[Arthur Edelstein [:arthur]]

Thanks for noticing this issue. I agree it's a potential problem and I don't think Tor Browser does anything to prevent it. So maybe we should add a privacy.resistFingerprinting check to this line:
https://searchfox.org/mozilla-central/source/dom/media/mediasource/MediaSource.cpp#130

Comment 8

[Ralph Giles (:rillian)]

Comment on attachment 8950452 [details] [diff] [review]
0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

Passing review to Bryce.

Comment 9

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

Updating patch to make sure `media.mediasource.webm.enabled` doesn't interfere when privacy.resistFingerprinting = true.

Comment 10

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8952909 [details] [diff] [review]
0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

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

::: dom/media/mediasource/MediaSource.cpp
@@ +128,5 @@
>      }
>      return NS_OK;
>    }
>    if (mimeType == MEDIAMIMETYPE("video/webm")) {
> +    if (!((Preferences::GetBool("media.mediasource.webm.enabled", false) &&

I'm concerned with this if becoming hard(er) to read. I think in terms of readability, as well as signalling intention we could have a separate check here before the existing one. Something like (pseudocode):

// Comment as to why we're doing this, link to this bug
// We'll need to extend this check for further codecs in webm
resistFingerprinting = checkResistFingerprinting()
if (resistFingerprinting && vp9) {
#ifdef osx||android
  return false;
#else
  return true;
#endif
}

This would remove the need to make changes in multiple places, and delimits our fingerprint resisting code from other checks.

Comment 11

[Tim Huang[:timhuang]]

Attachment: Bug 1436226 - Hardcode the VP8/VP9 algorithm when fingerprinting resistance is on. r?bryce!

The patch makes the algorithm selection of VP8/VP9 be hardcoded
regardless the CPU speed if 'privacy.resistfingerprinting' is on. We
would hardcode it into VP9 for Windows and Linux and into VP8 for MAC
and Android.

Comment 12

[Jean-Yves Avenard [:jya]]

I've had a similar proposal for bug https://bugzilla.mozilla.org/show_bug.cgi?id=1461454

However this has been lost in the ether.

Comment 13

[Jean-Yves Avenard [:jya]]

Comment on attachment 8952909 [details] [diff] [review]
0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

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

::: dom/media/mediasource/MediaSource.cpp
@@ +129,5 @@
>      return NS_OK;
>    }
>    if (mimeType == MEDIAMIMETYPE("video/webm")) {
> +    if (!((Preferences::GetBool("media.mediasource.webm.enabled", false) &&
> +           !nsContentUtils::ShouldResistFingerprinting()) ||

agree with bryce above, put this into is WebMForced

Comment 14

[Thorin [:thorin]]

FYI: Bug 1614958 (75+) Android: now enables/disables VP9 based on hardware

Comment 15

[Tim Huang[:timhuang]]

Unassign myself because I am no longer actively working on this.