Closed Bug 1369319 Opened 4 years ago Closed 4 years ago

Disable device sensors when 'privacy.resistFingerprinting' is true

Categories

(Core :: DOM: Device Interfaces, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: timhuang, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fingerprinting][tor][fp:m2])

Attachments

(2 files)

For fingerprinting resistance, we want to disable device sensors when 'privacy.resistFingerprinting' is true.

We want to provide the similar behavior as 'device.sensors.enabled' = false.
Priority: -- → P1
Whiteboard: [fingerprinting][tor][fp:m1] → [fingerprinting][tor][fp:m2]
Because the device sensors API, like device motion, orientation and etc., could reveal users' hardware, which means this is a fingerprinting vector. So, we want to block device sensors events when 'privacy.resistfingerprinting' is true. Provide the same behavior as setting 'device.sensors.enabled' to false would be desirable for this. 

What do you think about this, bz?
Flags: needinfo?(bzbarsky)
That seems fine to me.
Flags: needinfo?(bzbarsky)
Comment on attachment 8879562 [details]
Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/150876/#review155690

::: dom/system/nsDeviceSensors.cpp:573
(Diff revision 1)
>    mLastAcceleration.reset();
>    mLastDOMMotionEventTime = TimeStamp::Now();
>  }
> +
> +bool
> +nsDeviceSensors::IsSensorEventBlocked(nsIDOMWindow* aWindow) {

Curly on next line, please.
Attachment #8879562 - Flags: review?(bzbarsky) → review+
Comment on attachment 8879562 [details]
Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/150876/#review155696

::: commit-message-d39cd:1
(Diff revision 1)
> +Bug 1369319 - Part 1: Blocking device sensor events when 'privacy.resistFingerprinting' is true. r?bz,arthuredelstein

s/Blocking/Block/, by the way.

::: commit-message-d39cd:4
(Diff revision 1)
> +Bug 1369319 - Part 1: Blocking device sensor events when 'privacy.resistFingerprinting' is true. r?bz,arthuredelstein
> +
> +This patch adds a nsDeviceSensors::IsSensorEventBlocked() function to check
> +whether device sensor events should be blocked and replace original

s/replace/replaces the/
Comment on attachment 8879563 [details]
Bug 1369319 - Part2: Add a test case to verify that device sensor events have been blocked when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/150878/#review155698

::: commit-message-49514:1
(Diff revision 1)
> +Bug 1369319 - Part2: Add a test case to verify that device sensor evevnts have been blocked when 'privacy.resistFingerprinting' is true. r?bz,arthuredelstein

"events"

::: browser/components/resistfingerprinting/test/mochitest/test_device_sensor_event.html:28
(Diff revision 1)
> +    }, window);
> +  }
> +
> +  function doTest() {
> +    window.addEventListener("devicemotion", () => {
> +      ok(false, "The device motion event should not be fired.")

Does this test fail without the patch?  What causes a devicemotion event to fire in automation?
Attachment #8879563 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] (if a patch has no decent message, automatic r-) from comment #7)
> 
> Does this test fail without the patch?  What causes a devicemotion event to
> fire in automation?

Yes, it does. The 'devicemotion' event is going to be fired immediately after this event been added when pref 'device.sensors.test.events' is turned on.
Comment on attachment 8879562 [details]
Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/150876/#review156464

::: dom/system/nsDeviceSensors.h:77
(Diff revision 2)
>  
>    inline bool IsSensorEnabled(uint32_t aType) {
>      return mWindowListeners[aType]->Length() > 0;
>    }
>  
> +  bool IsSensorEventBlocked(nsIDOMWindow* aWindow);

I think this would be more clear if it were called "AreSensorEventsDisabled()"
Attachment #8879562 - Flags: review?(arthuredelstein) → review+
Attachment #8879563 - Flags: review?(arthuredelstein) → review+
Pushed by jhao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a3110d353a4
Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true. r=arthuredelstein,bz
https://hg.mozilla.org/integration/autoland/rev/457beb838cdc
Part2: Add a test case to verify that device sensor events have been blocked when 'privacy.resistFingerprinting' is true. r=bz
https://hg.mozilla.org/mozilla-central/rev/2a3110d353a4
https://hg.mozilla.org/mozilla-central/rev/457beb838cdc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Comment on attachment 8879563 [details]
Bug 1369319 - Part2: Add a test case to verify that device sensor events have been blocked when 'privacy.resistFingerprinting' is true.

https://reviewboard.mozilla.org/r/150878/#review158458
Attachment #8879563 - Flags: review?(arthuredelstein)
Good job!  Thanks, Tim, and reviewers.
https://hg.mozilla.org/mozilla-central/rev/457beb838cdc#l2.15
> +  <script type="application/javascript;version=1.7">

Please do not add new versioned JavaScript. See bug 1342144 for details. Fortunately, this patch does not actually use versioned script features (legacy generators). So it is sufficient to remove the version parameter.
Flags: needinfo?(tihuang)
(In reply to Masatoshi Kimura [:emk] from comment #18)
> https://hg.mozilla.org/mozilla-central/rev/457beb838cdc#l2.15
> > +  <script type="application/javascript;version=1.7">
> 
> Please do not add new versioned JavaScript. See bug 1342144 for details.
> Fortunately, this patch does not actually use versioned script features
> (legacy generators). So it is sufficient to remove the version parameter.

Thanks, I will open a bug to remove this.
Flags: needinfo?(tihuang)
Depends on: 1390391
You need to log in before you can comment on or make changes to this bug.