Open Bug 1436226 Opened 2 years ago Updated 1 month ago

Hardcode VP8/VP9 algorithm choice when resisting fingerprinting

Categories

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

enhancement

Tracking

()

People

(Reporter: arthur, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor 22548] [fingerprinting][fp-triaged])

Attachments

(2 files, 1 obsolete file)

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.
Whiteboard: [tor 22548]1428351 → [tor 22548]
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?
Assignee: nobody → bvandyk
Priority: -- → P3
Here's a fix using the privacy.resistFingerprinting pref. Would this be acceptable?
Attachment #8950452 - Flags: review?(giles)
Assignee: bvandyk → arthuredelstein
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
Flags: needinfo?(arthuredelstein)
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
Flags: needinfo?(arthuredelstein)
Comment on attachment 8950452 [details] [diff] [review]
0001-Bug-1436226-Hardcode-vp8-vp9-choice-when-privacy.res.patch

Passing review to Bryce.
Attachment #8950452 - Flags: review?(giles) → review?(bvandyk)
Updating patch to make sure `media.mediasource.webm.enabled` doesn't interfere when privacy.resistFingerprinting = true.
Attachment #8950452 - Attachment is obsolete: true
Attachment #8950452 - Flags: review?(bvandyk)
Attachment #8952909 - Flags: review?(bvandyk)
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.
Attachment #8952909 - Flags: review?(bvandyk)
Whiteboard: [tor 22548] → [tor 22548] [fingerprinting]
Assignee: arthuredelstein → tihuang
Priority: P3 → P2
Whiteboard: [tor 22548] [fingerprinting] → [tor 22548] [fingerprinting][fp-triaged]

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.

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.

See Also: → 1461454
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

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

You need to log in before you can comment on or make changes to this bug.