1281949 - screen.orientation should be spoofed when privacy.resistFingerprinting is enabled (Tor 18958)

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Arthur Edelstein [:arthur]]

From https://trac.torproject.org/18958:

Firefox 43 introduced a new orientation API that includes the unprefixed screen.orientation property and possibly support for onchange events. See:

    ​https://developer.mozilla.org/en-US/docs/Web/API/Screen/orientation
    ​https://w3c.github.io/screen-orientation/

The implementation of the new API did not carry forward the concept of respecting the privacy.resistFingerprinting pref, which was implemented in bug 418986.

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1281949-When-resisting-fingerprinting-spoof-scre.patch

Here's the patch, rebased from the Tor Browser patch, that spoofs 'screen.orientation', blocks 'orientationchange' and 'mozorientationchange' events, and blocks the use of 'screen.mozLockOrientation' and 'screen.mozUnlockOrientation' when the pref 'privacy.resistFingerprinting' is true.

Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c28500ad90e&selectedJob=22827546

Comment 2

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8764817 [details] [diff] [review]
0001-Bug-1281949-When-resisting-fingerprinting-spoof-scre.patch

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

Nits only.

::: dom/base/ScreenOrientation.cpp
@@ +589,5 @@
> +ScreenOrientation::ShouldResistFingerprinting() const
> +{
> +  bool resist = false;
> +  nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner();
> +  if (owner) {

I'm a fan of the

if (nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner()) {
  ...owner...
}

style.

::: dom/base/nsGlobalWindow.cpp
@@ +13243,5 @@
>  nsGlobalWindow::EnableOrientationChangeListener()
>  {
>    MOZ_ASSERT(IsInnerWindow());
> +  if (!nsContentUtils::ShouldResistFingerprinting(mDocShell)
> +      && !mOrientationChangeObserver) {

Nit: In DOM, we tend to do:

if (aReallyLongCondition &&
    !bSuperDuperLongCondition) {
  ...
}

Comment 3

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1281949-When-resisting-fingerprinting-spoof-scre.patch

(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #2)

> Nits only.

Thank you for the quick review! Here's the new version with nits fixed.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26a493fd7137
When resisting fingerprinting, spoof screen.orientation

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/26a493fd7137

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Does
 nsGlobalWindow::EnableOrientationChangeListener()
 {
   MOZ_ASSERT(IsInnerWindow());
-
-  if (!mOrientationChangeObserver) {
+  if (!nsContentUtils::ShouldResistFingerprinting(mDocShell) &&
+      !mOrientationChangeObserver) {
actually work?
Is mDocShell set there? Isn't that set only on outer windows.

Comment 7

[Arthur Edelstein [:arthur]]

(In reply to Olli Pettay [:smaug] from comment #6)
> Does
>  nsGlobalWindow::EnableOrientationChangeListener()
>  {
>    MOZ_ASSERT(IsInnerWindow());
> -
> -  if (!mOrientationChangeObserver) {
> +  if (!nsContentUtils::ShouldResistFingerprinting(mDocShell) &&
> +      !mOrientationChangeObserver) {
> actually work?
> Is mDocShell set there? Isn't that set only on outer windows.

Thanks for pointing this out. I opened bug 1433815 to work on fixing this.