1369319 - Disable device sensors when 'privacy.resistFingerprinting' is true

Status: RESOLVED FIXED

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

Description

[Tim Huang[:timhuang]]

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.

Comment 1

[Tim Huang[:timhuang]]

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?

Comment 2

[Boris Zbarsky [:bzbarsky]]

That seems fine to me.

Comment 3

[Tim Huang[:timhuang]]

Attachment: Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

This patch adds a nsDeviceSensors::IsSensorEventBlocked() function to check
whether device sensor events should be blocked and replace original
'device.sensors.enabled' check with this function. This function will not only
check 'privacy.resistFingerprinting' but also check 'device.sensors.enabled'.

Review commit: https://reviewboard.mozilla.org/r/150876/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150876/

Comment 4

[Tim Huang[:timhuang]]

Attachment: Bug 1369319 - Part2: Add a test case to verify that device sensor events have been blocked when 'privacy.resistFingerprinting' is true.

Review commit: https://reviewboard.mozilla.org/r/150878/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/150878/

Comment 5

[Boris Zbarsky [: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.

Comment 6

[Boris Zbarsky [: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/#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 7

[Boris Zbarsky [:bzbarsky]]

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?

Comment 8

[Tim Huang[:timhuang]]

(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 9

[Tim Huang[:timhuang]]

Comment on attachment 8879562 [details]
Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150876/diff/1-2/

Comment 10

[Tim Huang[:timhuang]]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150878/diff/1-2/

Comment 11

[Arthur Edelstein [:arthur]]

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()"

Comment 12

[Tim Huang[:timhuang]]

Comment on attachment 8879562 [details]
Bug 1369319 - Part 1: Block device sensor events when 'privacy.resistFingerprinting' is true.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150876/diff/2-3/

Comment 13

[Tim Huang[:timhuang]]

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.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/150878/diff/2-3/

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2a3110d353a4
https://hg.mozilla.org/mozilla-central/rev/457beb838cdc

Comment 16

[Arthur Edelstein [:arthur]]

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

Comment 17

[Ethan Tseng [:ethan]]

Good job!  Thanks, Tim, and reviewers.

Comment 18

[Masatoshi Kimura [:emk]]

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.

Comment 19

[Tim Huang[:timhuang]]

(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.