1384330 - Don't expose window.navigator.mozAddonManager data when privacy.resistFingerprinting=true

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Arthur Edelstein [:arthur]]

In Tor Browser, to protect against fingerprinting, we block window.navigator.mozAddonManager data from being exposed to AMO. We would like to propose uplifting this protection behind the "privacy.resistFingerprinting" pref.

The original patch is at https://torpat.ch/21684 and the Tor ticket is https://bugs.torproject.org/21684

Comment 1

[Tim Huang[:timhuang]]

Per discussion with Arthur and Tom. Firefox won't block navigator.mozAddonManager for AMO when 'privacy.resistFingerprinting' is true. However, we will still provide a pref for allowing Tor can disable this entirely.

Comment 2

[Tim Huang[:timhuang]]

Attachment: Bug 1384330 - Part 1: Blocking the mozAddonManager when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

This patch adds a hidden pref 'privacy.resistFingerprinting.block_mozAddonManager',
which is false by default. When this is true, the navigator.mozAddonManager will
be blocked even the website is AMO.

The purpose of this pref is for Tor browser can disable navigator.mozAddonManager
through this.

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

Comment 3

[Tim Huang[:timhuang]]

Attachment: Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

The patch adds a test case which will first test with the pref off to see that
navigator.mozAddonManager can be accessed successfully. And then turn the pref
on to see that whether navigator.mozAddonManager is blocked.

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

Comment 4

[Andy McKay]

Is the intent to still allow add-ons to be install from AMO? Because I'm concerned that at some point this will prevent AMO from working at all. If you want to prevent install, then there is another pref for that.

Comment 5

[Arthur Edelstein [:arthur]]

Comment on attachment 8908025 [details]
Bug 1384330 - Part 1: Blocking the mozAddonManager when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179706/#review185074

Thanks!

Comment 6

[Arthur Edelstein [:arthur]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179708/#review185076

The code for the "pref off" and the code for the "pref on" case are very similar. I would suggest combining them into a single loop instead.

::: browser/components/resistfingerprinting/test/browser/browser_block_mozAddonManager.js:9
(Diff revision 1)
> + */
> +
> +const TEST_PATH = "https://example.com/browser/browser/" +
> +                  "components/resistfingerprinting/test/browser/"
> +
> +add_task(async function test_with_pref_is_off() {

nit: change name to "test_with_pref_off" or "test_when_pref_is_off"

Comment 7

[Stuart Colville [:muffinresearch]]

(In reply to Andy McKay [:andym] from comment #4)
> Is the intent to still allow add-ons to be install from AMO? Because I'm
> concerned that at some point this will prevent AMO from working at all. If
> you want to prevent install, then there is another pref for that.

AMO should still fallback to an install button (link pointing to the download) if mozAddonManager is not present.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 8908025 [details]
Bug 1384330 - Part 1: Blocking the mozAddonManager when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179706/#review185162

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179708/#review185164

I'll wait for you to make Arthur's changes here.

Comment 10

[Tim Huang[:timhuang]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

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

Comment 11

[Arthur Edelstein [:arthur]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179708/#review185286

Thanks, Tim!

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

https://reviewboard.mozilla.org/r/179708/#review185590

::: browser/components/resistfingerprinting/test/browser/browser_block_mozAddonManager.js:24
(Diff revision 2)
> +    let tab = await BrowserTestUtils.openNewForegroundTab(
> +      gBrowser, TEST_PATH + "file_dummy.html");
> +
> +    await ContentTask.spawn(tab.linkedBrowser, pref, function(aPref) {
> +      if (aPref) {
> +        ok(content.navigator.mozAddonManager === undefined,

Use |is| here so we can see what it is in the case that it fails.

Comment 13

[Tim Huang[:timhuang]]

Comment on attachment 8908025 [details]
Bug 1384330 - Part 1: Blocking the mozAddonManager when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

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

Comment 14

[Tim Huang[:timhuang]]

Comment on attachment 8908026 [details]
Bug 1384330 - Part 2: Add a test case for testing that navigator.mozAddonManager is correctly blocked when pref 'privacy.resistFingerprinting.block_mozAddonManager' is true.

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

Comment 15

[Tim Huang[:timhuang]]

Try looks fine.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a6cf61dfd96

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/bb14bbb30c41
https://hg.mozilla.org/mozilla-central/rev/2189a968511e

Comment 17

[marius.santa]

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.

Comment 18

[Tim Huang[:timhuang]]

STR

1. Turn the pref 'privacy.resistFingerprinting.block_mozAddonManager' on.
2. Open a tab with the URL 'https://addons.mozilla.org/'.
3. Open the Web Console in Web developer (Tools->Web Developer->Web Console)
4. Check that whether navigator.mozAddonManager exists by using Web Console.

Expected behavior:

navigator.mozAddonManager is undefined in 'https://addons.mozilla.org/'

Thanks,

Comment 19

[marius.santa]

Attachment: nav addonmanager.PNG

I can reproduce this issue on Firefox 56.0a1 (20170725030209) under Wind 10 64-bit. 

This issue is verified as fixed on Firefox 57.0 (20171112125346), Firefox  	58.0b6 (20171123161455) and Firefox 59.0a1 (20171127100433) under Wind 10 64-bit, Mac OS X 10.13 and ubuntu 14.04 LTS