Opened 13 months ago

Closed 5 months ago

#22084 closed task (fixed)

Neuter NetworkInformation API on Tor Browser Mobile

Reported by: gk Owned by: igt0
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile tbb-fingerprinting, TorBrowserTeam201712R
Cc: brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The Networkinformation API is enabled by default in the mobile version of Firefox. This is governed by a preference, dom.netinfo.enabled. This API (https://developer.mozilla.org/en-US/docs/Web/API/NetworkInformation) provides a variety of information, e.g. the connection type a device is using to communicate with the network or the maximum downlink speed. We should avoid fingerprinting risks associated with this API. For a discussion among Mozilla engineers see: https://groups.google.com/forum/#!topic/mozilla.dev.platform/lCZmhCDGHPY

Child Tickets

Attachments (5)

0001-Spoofing-network-information-API-and-blocking-ontype.patch (4.1 KB) - added by igt0 5 months ago.
Version 1 - Backport commits from firefox
0001-PATCH-Bug-1372072-Part-1-Spoofing-network-informatio.patch (1.9 KB) - added by igt0 5 months ago.
Version 2 - Backport code from Firefox
0002-PATCH-Bug-1372072-Part-2-Add-a-test-case-for-check-w.patch (2.8 KB) - added by igt0 5 months ago.
Version 2 - Backport test from firefox
0001-PATCH-Bug-1372072-Part-1-Spoofing-network-informatio.2.patch (1.9 KB) - added by igt0 5 months ago.
Version 3 - Remove superfluous lines
0002-PATCH-Bug-1372072-Part-2-Add-a-test-case-for-check-w.2.patch (2.8 KB) - added by igt0 5 months ago.
Version 3 - Remove superfluous lines

Download all attachments as: .zip

Change History (20)

comment:1 Changed 13 months ago by tom

Keywords: tbb-fingerprinting added

See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1360242

That link to google groups didn't work for me, but searching for "netinfo" in the group found the singular result.

comment:2 Changed 8 months ago by gk

Summary: Neuter NetworkInformatin API on Tor Browser MobileNeuter NetworkInformation API on Tor Browser Mobile

comment:3 Changed 8 months ago by igt0

Owner: changed from tbb-team to igt0
Status: newaccepted

I am working on it.

comment:5 in reply to:  4 ; Changed 8 months ago by gk

Replying to igt0:

There is already an upstream fix for this bug:

https://hg.mozilla.org/mozilla-central/rev/69970dbe2b5a
https://hg.mozilla.org/mozilla-central/rev/12f8c79dabb4

Thanks. I guess we want to backport those for our esr52-based branch. Do you think you want to do that?

comment:6 in reply to:  5 Changed 8 months ago by igt0

Replying to gk:

Replying to igt0:

There is already an upstream fix for this bug:

https://hg.mozilla.org/mozilla-central/rev/69970dbe2b5a
https://hg.mozilla.org/mozilla-central/rev/12f8c79dabb4

Thanks. I guess we want to backport those for our esr52-based branch. Do you think you want to do that?

Yep, I am on it.

comment:7 Changed 6 months ago by mcs

Cc: brade mcs added

Changed 5 months ago by igt0

Version 1 - Backport commits from firefox

comment:8 Changed 5 months ago by igt0

Status: acceptedneeds_review

comment:9 Changed 5 months ago by gk

Keywords: TorBrowserTeam201712R added

comment:10 Changed 5 months ago by gk

Keywords: TorBrowserTeam201712 added; TorBrowserTeam201712R removed
Status: needs_reviewneeds_revision

Thanks. I think the code backported looks good. However, I think we should have a better patch format. First, we should keep the Mozilla bug number. That makes it easier to find the patch(es) in our tree. Second, we should keep the patch(set) structure: one commit in our tree should match one commit taken from Mozilla. This allows us to pinpoint possible issues easier. Third, we should fix up the commit message if needed (in your case you still have included "This tests not only windows, but also workers." even though you rightly omitted the workers related part).

A workflow that works fine for me is having mozilla-central as a git remote and cherry picking the patches from that one into tor-browser one-by-one, fixing them up if needed and adapting the commit message afterwards with git commit --amend if needed as well.

Changed 5 months ago by igt0

Version 2 - Backport code from Firefox

Changed 5 months ago by igt0

Version 2 - Backport test from firefox

comment:11 in reply to:  10 Changed 5 months ago by igt0

Thanks,

I attached the back ported patches following the proposed workflow:

Part 1: https://trac.torproject.org/projects/tor/attachment/ticket/22084/0001-PATCH-Bug-1372072-Part-1-Spoofing-network-informatio.patch

Part 2: https://trac.torproject.org/projects/tor/attachment/ticket/22084/0002-PATCH-Bug-1372072-Part-2-Add-a-test-case-for-check-w.patch

Replying to gk:

Thanks. I think the code backported looks good. However, I think we should have a better patch format. First, we should keep the Mozilla bug number. That makes it easier to find the patch(es) in our tree. Second, we should keep the patch(set) structure: one commit in our tree should match one commit taken from Mozilla. This allows us to pinpoint possible issues easier. Third, we should fix up the commit message if needed (in your case you still have included "This tests not only windows, but also workers." even though you rightly omitted the workers related part).

A workflow that works fine for me is having mozilla-central as a git remote and cherry picking the patches from that one into tor-browser one-by-one, fixing them up if needed and adapting the commit message afterwards with git commit --amend if needed as well.

comment:12 Changed 5 months ago by cypherpunks

Status: needs_revisionneeds_review

comment:13 Changed 5 months ago by gk

Status: needs_reviewneeds_revision

We are close! Could you just remove two superfluous newlines? One at:

 #include "nsINetworkProperties.h"
+#include "nsContentUtils.h"
+

and the other one, which is git complaining about, at the end of the test file:

+  await testWindow();
+});
+

Changed 5 months ago by igt0

Version 3 - Remove superfluous lines

Changed 5 months ago by igt0

Version 3 - Remove superfluous lines

comment:14 in reply to:  13 Changed 5 months ago by igt0

Status: needs_revisionneeds_review

Thanks for the feedback! I updated the patch.

https://trac.torproject.org/projects/tor/attachment/ticket/22084/0001-PATCH-Bug-1372072-Part-1-Spoofing-network-informatio.2.patch

https://trac.torproject.org/projects/tor/attachment/ticket/22084/0002-PATCH-Bug-1372072-Part-2-Add-a-test-case-for-check-w.2.patch

Replying to gk:

We are close! Could you just remove two superfluous newlines? One at:

 #include "nsINetworkProperties.h"
+#include "nsContentUtils.h"
+

and the other one, which is git complaining about, at the end of the test file:

+  await testWindow();
+});
+

comment:15 Changed 5 months ago by gk

Keywords: TorBrowserTeam201712R added; TorBrowserTeam201712 removed
Resolution: fixed
Status: needs_reviewclosed

Thanks for all three fixups. The patches got applied to tor-browser-52.5.2esr-7.5-2 as commit a1beadc5b70e1b6e4727506656723684cf3225bf and a72faadea544a71ae5ca95ec816f2684c205b56a).

Note: See TracTickets for help on using tickets.