Closed Bug 1384330 Opened 3 years ago Closed 3 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla57
Tracking Status
firefox57 --- verified
firefox58 --- verified
firefox59 --- verified

People

(Reporter: arthur, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor 21684][fingerprinting][fp:m3])

Attachments

(3 files)

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
Summary: Don't expose window.navigator.mozAddonManager when privacy.resistFingerprinting=true → Don't expose window.navigator.mozAddonManager data when privacy.resistFingerprinting=true
Priority: -- → P2
Whiteboard: [tor 21684] → [tor 21684][fingerprinting][fp:m3]
Assignee: nobody → tihuang
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.
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.
Flags: needinfo?(scolville)
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!
Attachment #8908025 - Flags: review?(arthuredelstein) → review+
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"
Attachment #8908026 - Flags: review?(arthuredelstein)
(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.
Flags: needinfo?(scolville)
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
Attachment #8908025 - Flags: review?(dtownsend) → review+
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.
Attachment #8908026 - Flags: review?(dtownsend)
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!
Attachment #8908026 - Flags: review?(arthuredelstein) → review+
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.
Attachment #8908026 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/mozilla-central/rev/bb14bbb30c41
https://hg.mozilla.org/mozilla-central/rev/2189a968511e
Status: NEW → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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.
Flags: needinfo?(tihuang)
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,
Flags: needinfo?(tihuang)
Attached image 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
Status: RESOLVED → VERIFIED
See Also: → 1651405
You need to log in before you can comment on or make changes to this bug.