When privacy.resistFingerprinting = true, set new windows to rounded dimensions [tor 19459]

REOPENED
Unassigned

Status

()

Core
XUL
P3
normal
REOPENED
a year ago
2 months ago

People

(Reporter: arthuredelstein, Unassigned)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [fingerprinting][tor][fp:m1][fp-triaged])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(5 attachments)

In Tor Browser (where privacy.resistFingerprinting is enabled), when a new window is opened, the dimensions of the content are always set to 1000x1000, or, if enough screen space is not available, to the nearest 200x100. This reduces the entropy in screen-size and window-size that can otherwise be used for fingerprinting or linking across sites.

We'd like to propose uplifting this patch: torpat.ch/19459

We also have code for auto-quantizing the window dimensions after it is resized by the user but we'll keep that to a separate ticket because it is more complex and probably more controversial as well...

Updated

a year ago
Priority: -- → P2
Summary: When privacy.resistFingerprinting = true, set new windows to rounded dimensions → When privacy.resistFingerprinting = true, set new windows to rounded dimensions [tor 19459]
Whiteboard: [tor 19459][fingerprinting] → [fingerprinting]

Comment 1

a year ago
Can an extra preference to disable auto-sizing be implemented eg privacy.resistFingerprinting.windowRounding so that Firefox users (as compared to TBB users) can disable this functionality, but TBB can easily switch it on?

I use resistFingerprinting but resize my window (allowing for non inner window elements) to match common screen resolutions. From the scratch pad I can enter window.resizeTo(1614,946) to achieve 1366x768 or window.resizeTo(1528,978) for 1280x800 etc. I don't go fullscreen, I don't resize, and I don't change (show/hide) my sidebar/toolbars/etc. Its static.

https://trac.torproject.org/projects/tor/ticket/14098

I am but one user, but surely, when hiding in the Firefox set of users, we would want common screen resolutions, not round hundreds. I would rather, as a non-Tor browser and being in the Firefox subset, be returning more common or realistic values. I also think a lot of power users would not like their browser being resized on them. And additionally, the current Tor dimensions of tall and skinny (on wide desktops) does not suit all. Regardless of which strategy (using a small set of rounded hundreds or an even smaller set of common screen resolutions) is better (and I do think that at the end of the day, some enforced set resolutions is the only way to gain significant numbers to hide in), surely at this stage, a toggle would be desirable - especially if you are to not drive away users of privacy.resistFingerprinting. A single extra pref at this stage would allow some flexibility.

Comment 2

a year ago
(In reply to Simon Mainey from comment #1)
> https://trac.torproject.org/projects/tor/ticket/14098

Sorry, wrong ticket link. I can't find the discussion I previously read about screen resolutions vs rounded hundreds. I understand the rounded hundreds argument for TOR users, but I also believe an even smaller set of common screen resolutions is more practical/usable, and is even better to hide in, for Firefox users.
(In reply to Simon Mainey from comment #1)
> Can an extra preference to disable auto-sizing be implemented eg
> privacy.resistFingerprinting.windowRounding so that Firefox users (as
> compared to TBB users) can disable this functionality, but TBB can easily
> switch it on?
> 
> I use resistFingerprinting but resize my window (allowing for non inner
> window elements) to match common screen resolutions. From the scratch pad I
> can enter window.resizeTo(1614,946) to achieve 1366x768 or
> window.resizeTo(1528,978) for 1280x800 etc. I don't go fullscreen, I don't
> resize, and I don't change (show/hide) my sidebar/toolbars/etc. Its static.

Hi Simon -- in the proposed patch (https://torpat.ch/19459) there are two prefs, privacy.window.maxInnerWidth and privacy.window.maxInnerHeight, that let you set the startup size of the window. Does that address your concerns? It also helps you avoid having to use resizeTo().

> https://trac.torproject.org/projects/tor/ticket/14098
> 
> I am but one user, but surely, when hiding in the Firefox set of users, we
> would want common screen resolutions, not round hundreds. I would rather, as
> a non-Tor browser and being in the Firefox subset, be returning more common
> or realistic values. I also think a lot of power users would not like their
> browser being resized on them. And additionally, the current Tor dimensions
> of tall and skinny (on wide desktops) does not suit all. Regardless of which
> strategy (using a small set of rounded hundreds or an even smaller set of
> common screen resolutions) is better (and I do think that at the end of the
> day, some enforced set resolutions is the only way to gain significant
> numbers to hide in), surely at this stage, a toggle would be desirable -
> especially if you are to not drive away users of
> privacy.resistFingerprinting. A single extra pref at this stage would allow
> some flexibility.

I'm not sure there is a set of common screen resolutions that makes sense. We are enforcing the content window dimensions, not the screen dimensions, so taskbars, menubars, the window chrome, etc., affect the size available for content. I think the dimensions you are setting your window to are likely to increase your fingerprintability rather than decrease it.

I am open to ideas to improving this feature, but because window dimensions provide a lot of fingerprinting entropy, I think it's important that privacy.resistFingerprinting includes this feature.

Comment 4

a year ago
(In reply to Arthur Edelstein [:arthuredelstein] from comment #3)
> Hi Simon -- in the proposed patch (https://torpat.ch/19459) there are two
> prefs, privacy.window.maxInnerWidth and privacy.window.maxInnerHeight, that
> let you set the startup size of the window. Does that address your concerns?
> It also helps you avoid having to use resizeTo().

That's just it. I don't use resizeTo(). Firefox remembers it's last settings. I have set it once, catering for all non-inner window items, and I haven't had to change it since (since the resist.Fingerprinting pref landed). What I have fits nicely on my monitor, allows me to have a sidebar, extra toolbars, etc - tweaks for  theme and padding, whatever I want. And everybody will have different needs. Also, all my reported dimensions are the same. See http://ip-check.info/?lang=en for a test to measure inner, browser and usable-screen measurements, and panotoclick for screen = all are the same Screen (1280 x 800 pixels 24 bit color depth). It is easy enough to retrieve dimensions, do the math and resize to get the right measurement.

If privacy.window.maxInnerWidth and privacy.window.maxInnerHeight actually means I can set them as eg 1200 and 800, and the inner window is set exactly as that (not some MAX figure as the pref name suggests), then that would do the trick, I guess, as it would have the same effect.

> I'm not sure there is a set of common screen resolutions that makes sense.
> We are enforcing the content window dimensions, not the screen dimensions,
> so taskbars, menubars, the window chrome, etc., affect the size available
> for content. I think the dimensions you are setting your window to are
> likely to increase your fingerprintability rather than decrease it.
> 
> I am open to ideas to improving this feature, but because window dimensions
> provide a lot of fingerprinting entropy, I think it's important that
> privacy.resistFingerprinting includes this feature.

I can't find the tor ticket thread I read a few months ago, but the two sides of the discussion went ..
- round hundred combinations eg 8 heights (or more), 8 widths (or more depending on your code which currently favors portrait rather than landscape etc) = 64 combinations total (not saying that's how your code would handle it, and I would assume the set is much smaller)
- "common" screen resolutions: just pick 5 of them (or 10 even, or less: I think someone argued for just 2 at some point) = 5 total eg 1920x1080, 1366x768, 1280x800, 1440x900, 1600x900 etc that cover 95% of end user's needs. Eg my monitor's usable screen is approx 1680x800 (i.e minus the double height task bar), so after code calculates the non inner window pieces, it can easy work out that I can use 2 of those settings: 1280x800 and 1366x768. It would then know from Telemetry (see point below) that the second option is most common.
- privacy.window.maxInnerWidth and privacy.window.maxInnerHeight can be used for the 5% who need it to override.
- surely 10 is smaller set than 64
- As a Firefox subset (not a TOR subset), surely all those not using the privacy.resistFingerprinting for screen will leak a "common" resolution (see next point on telemetry data). Obviously some code out there may only use this one measure, but most will likely use multiple measurements, so the key is to make all Firefox users with privacy.resistFingerprinting as a set, use as few dimensions as possible.
- https://metrics.mozilla.com/firefox-hardware-report/  Firefox telemetry common display resolutions (Sept 2016). 33% 1366x768 , 20% 1920x1080, etc

I think we'll go round in circles with this. I definitely want the end result to be ENFORCEMENT of browser inner window dimensions for Firefox users with privacy.resistFingerprinting, as the aim is to reduce entropy in this subset. What TBB does I don't mind/care. But for Firefox, I would rather there were more options, albeit a LIMITED set, so some users can have widescreen/landscape and some users can have portrait, and so it caters for users with different setups (such as my big fat side bar). I am sure a limited set of 10 resolutions or less could be worked out - in round hundreds (it would pass the test of time, as who knows what the future will bring in manufacturing).

In fact, why not just let users set one of eg six pre-defined resolutions with no exceptions eg ("privacy.window.maxDimensions", 1) where
1= 1500x800 (more width and allows for a sidebar)
2= 1000x800 (currently what TBB throws at me on startup, portrait)
3+4= next sizes down

NO idea if every real world scenario would fit in that set, but surely the smallest size would cover all, or fallback to the old code? Just throwing some ideas at you from an end user perspective who would HATE "my" Firefox to lose any usability (side bars etc) due to a lack of discussion or options, or be forced to too narrow a width for viewing. There has to be some flexibility or people will be turned off. Thanks :)
(In reply to Simon Mainey from comment #4)

> If privacy.window.maxInnerWidth and privacy.window.maxInnerHeight actually
> means I can set them as eg 1200 and 800, and the inner window is set exactly
> as that (not some MAX figure as the pref name suggests), then that would do
> the trick, I guess, as it would have the same effect.

Yes. The "max" part of the name refers to the behavior that, if the screen is too small to accomodate the target sizes, then it chooses smaller sizes with the same quantization.

> > I'm not sure there is a set of common screen resolutions that makes sense.
> > We are enforcing the content window dimensions, not the screen dimensions,
> > so taskbars, menubars, the window chrome, etc., affect the size available
> > for content. I think the dimensions you are setting your window to are
> > likely to increase your fingerprintability rather than decrease it.
> > 
> > I am open to ideas to improving this feature, but because window dimensions
> > provide a lot of fingerprinting entropy, I think it's important that
> > privacy.resistFingerprinting includes this feature.
>
> NO idea if every real world scenario would fit in that set, but surely the
> smallest size would cover all, or fallback to the old code? Just throwing
> some ideas at you from an end user perspective who would HATE "my" Firefox
> to lose any usability (side bars etc) due to a lack of discussion or
> options, or be forced to too narrow a width for viewing. There has to be
> some flexibility or people will be turned off. Thanks :)

Your feedback is welcome, and I agree that it's worth investigating further what set of sizes we should permit. Also, after this patch lands, we can always revise the set of allowed sizes again as we get user feedback.

Comment 6

a year ago
Note: privacy.resistFingerprinting is enabled.

Whoer (https://whoer.net/#extended)
results:
- height 800 
- width 1280 
- availHeight 800 
- availWidth 1280 
- window size 1268x800 (1280x800) 

JoDonym (http://ip-check.info/?lang=en)
"inner", "browser" and "usable-screen" : all 1280x800

Panoptoclick (https://panopticlick.eff.org/)
"Screen Size": 1280x800

Browserspy (http://browserspy.dk/screen.php)
"Screen" : 1280x800

Whoer.net is taking the vertical scroll bar of 12 pixels into account. Is this an entropy issue? Such as blocked content from one user resulting in no scroll bar, and others getting it. Do scroll bar widths vary with themes?

===
Arthur, with a little more thought, I agree that round hundreds are best (we can't predict the future). So please ignore the using "common" screen resolutions. A select few round-hundred combinations can cover all screens (they don't HAVE to use the most height or width available, we can have an upper and lower bound) - it would certainly be lower than the dozen or two screen resolutions floating around (not that you would use them all). Just MORE-but-ENFORCED (or enforced at a later date) options for end users would be ideal.
Whiteboard: [fingerprinting] → [fingerprinting][tor]

Updated

a year ago
Assignee: nobody → tihuang

Updated

a year ago
Priority: P2 → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Updated

a year ago
Attachment #8843814 - Flags: review?(arthuredelstein)
Attachment #8843815 - Flags: review?(arthuredelstein)

Comment 9

a year ago
mozreview-review
Comment on attachment 8843814 [details]
Bug 1330882 - Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled (adopt from Tor #19459).

https://reviewboard.mozilla.org/r/117404/#review119178

I'd like to see those changes.

Not really about this bug but, how does this size rounding work with fullscreen? Do we not support fullscreen when the fingerprint pref is set or what?

::: xpfe/appshell/nsXULWindow.cpp:1072
(Diff revision 1)
> +  int32_t availForContentWidthCSS =
> +    std::min(maxInnerWidth, NSToIntRound((0.95 * availWidth - chromeWidth) /
> +                                         devicePerCSSPixels));
> +  int32_t availForContentHeightCSS =
> +    std::min(maxInnerHeight, NSToIntRound((0.95 * availHeight - chromeHeight) /
> +                                          devicePerCSSPixels));

0.95 is rather magical. Why is that needed? Especially with width, where content width is usually pretty much the same as chrome width.

::: xpfe/appshell/nsXULWindow.cpp:1079
(Diff revision 1)
> +    NSToIntRound(devicePerCSSPixels *
> +                 (availForContentWidthCSS - (availForContentWidthCSS % 200)));
> +  int32_t targetContentHeight =
> +    NSToIntRound(devicePerCSSPixels *
> +                 (availForContentHeightCSS - (availForContentHeightCSS % 100)));
> +

This needs some good comment, why 200 and 100 are being used there.

::: xpfe/appshell/nsXULWindow.cpp:1128
(Diff revision 1)
>      }
>  
>      if (gotSize) {
>        SetSpecifiedSize(specWidth, specHeight);
>      }
>  

This code would be easier to understand if there was 'else' after if (ShouldResistFingerprinting).
Now one needs to know that ResizeToRoundedDimensions sets mIgnoreXULSize and mIgnoreXULSizeMode.

Also, could we call ResizeToRoundedDimensions to
ForceRoundedDimensions or something to hint that it may do also other thing than just resize.
(setting mIgnoreXULSize = true; and mIgnoreXULSizeMode = true; is quite major side effect)
Attachment #8843814 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] (high review load) from comment #9)
> Comment on attachment 8843814 [details]
> Bug 1330882 - Part 1: Making new windows to be rounded size when
> fingerprinting resistance is enabled.
> 
> https://reviewboard.mozilla.org/r/117404/#review119178
> 
> I'd like to see those changes.
> 
> Not really about this bug but, how does this size rounding work with
> fullscreen? Do we not support fullscreen when the fingerprint pref is set or
> what?
> 

I think this bug is targeting to enforce newly opened windows to be a rounded size after fingerprinting resistance is enabled. So, this patch here does not handle with fullscreen or resizing. The work of fullscreen or resizing will be a separate bug and how to do it is under discussion. One possible solution is that window.innerWidth and innerHeight reports a nearest rounded value after resize or maximize windows. Another one is that automatically quantize the window size after resizing or maximizing, fullscreen might be disabled in this case.

Arthur, what is the way that Tor browser deal with fullscreen? 

> ::: xpfe/appshell/nsXULWindow.cpp:1072
> (Diff revision 1)
> > + int32_t availForContentWidthCSS =
> > + std::min(maxInnerWidth, NSToIntRound((0.95 * availWidth - chromeWidth) /
> > + devicePerCSSPixels));
> > + int32_t availForContentHeightCSS =
> > + std::min(maxInnerHeight, NSToIntRound((0.95 * availHeight - chromeHeight) /
> > + devicePerCSSPixels));
> 
> 0.95 is rather magical. Why is that needed? Especially with width, where
> content width is usually pretty much the same as chrome width.
> 

Arthur, could you answer this for me? And I agree that width doesn't have to use this magical number.
Flags: needinfo?(arthuredelstein)

Comment 11

a year ago
mozreview-review
Comment on attachment 8843815 [details]
Bug 1330882 - Part 2: Disallow the session restore to modify window size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/117406/#review119220

Almost there, but I'd like to take a look at the final patch.

::: browser/components/sessionstore/SessionStore.jsm:210
(Diff revision 1)
> + * A global value to tell that fingerprinting resistance is enabled or not.
> + * If it's enabled, the session restore won't restore the window's size and
> + * size mode.
> + * This value is controlled by preference privacy.resistFingerprinting.
> + */
> +var gIsResistFingerPrinting = false;

nit: can you rename this to `gResistFingerPrintingEnabled` ?

::: browser/components/sessionstore/SessionStore.jsm:698
(Diff revision 1)
>      this._prefBranch.addObserver("sessionstore.max_windows_undo", this, true);
> +
> +    try {
> +      gIsResistFingerPrinting = Services.prefs.getBoolPref("privacy.resistFingerprinting");
> +    } catch (ex) {
> +      if (ex.result != Cr.NS_ERROR_UNEXPECTED) {

When will this happen? Are you not initializing this pref in firefox.js or perhaps even all.js?

::: browser/components/sessionstore/SessionStore.jsm:704
(Diff revision 1)
> +        Cu.reportError(ex);
> +      }
> +    }
> +    Services.prefs.addObserver("privacy.resistFingerprinting", () => {
> +      gIsResistFingerPrinting = Services.prefs.getBoolPref("privacy.resistFingerprinting");
> +    }, false);

I *think* `false` is the default. Can you make this work like all the other pref observers in this class:
`Services.prefs.addObserver("privacy.resistFingerprinting", this);` and do the variable setting in `onPrefChange` below.

::: browser/components/sessionstore/SessionStore.jsm:3867
(Diff revision 1)
>        if (aSizeMode != "maximized" || win_("sizemode") != "maximized") {
>          aWindow.resizeTo(aWidth, aHeight);
>        }
>      }
> -    if (aSizeMode && win_("sizemode") != aSizeMode)
> +    if (aSizeMode && win_("sizemode") != aSizeMode &&
> +        !gIsResistFingerPrinting)

nit: 80 chars is not a hard limit; if it improves legibility of the code we pragmatically allow an upper limit of 120 chars. IOW: you can leave this (and probably the one above too) on the same line!
Attachment #8843815 - Flags: review?(mdeboer) → review-
(In reply to Mike de Boer [:mikedeboer] from comment #11)
> Comment on attachment 8843815 [details]
> Bug 1330882 - Part 2: Disallow the session restore to modify window size
> when fingerprinting resistance is enabled.
> 
> ::: browser/components/sessionstore/SessionStore.jsm:698
> (Diff revision 1)
> > this._prefBranch.addObserver("sessionstore.max_windows_undo", this, true);
> > +
> > + try {
> > + gIsResistFingerPrinting = Services.prefs.getBoolPref("privacy.resistFingerprinting");
> > + } catch (ex) {
> > + if (ex.result != Cr.NS_ERROR_UNEXPECTED) {
> 
> When will this happen? Are you not initializing this pref in firefox.js or
> perhaps even all.js?
> 

Yes, this pref 'privacy.resistFingerprinting' has not been added into firefox.js or all.js. But, we are planning to add this pref in a separate bug. And that bug will remove these try-catch code sections.
(In reply to Tim Huang[:timhuang] from comment #12)
> Yes, this pref 'privacy.resistFingerprinting' has not been added into
> firefox.js or all.js. But, we are planning to add this pref in a separate
> bug. And that bug will remove these try-catch code sections.

Well, adding this pref to either of these files is a low bar and trivial to do. I'd prefer it if you'd just add them here, but if you'd like to track that work somewhere else, please CC me on the bug :)
(In reply to Tim Huang[:timhuang] from comment #10)

> Arthur, what is the way that Tor browser deal with fullscreen? 

In fullscreen, we have a separate patch in torbutton (an extension that is bundled with Tor Browser) that introduces empty margins at top and bottom or left and right. It also applies a zoom so that the screen is filled as much as possible. We also have a patch that quantizes the content bounds while the user resizes the window (e.g., by dragging the corner). But our patches need work, and I think it is good to keep them separate.

> > 0.95 is rather magical. Why is that needed? Especially with width, where
> > content width is usually pretty much the same as chrome width.
> 
> Arthur, could you answer this for me? And I agree that width doesn't have to
> use this magical number.

Yes, that's true. In fact in Tor Browser we are working to get rid of 0.95 from both width and height. But that  requires a bug fix for 581863 that I am working on here: https://trac.torproject.org/projects/tor/ticket/20905

But I agree, for now you can get rid of the 0.95 factor from the width.
Flags: needinfo?(arthuredelstein)
(Reporter)

Comment 15

a year ago
mozreview-review
Comment on attachment 8843815 [details]
Bug 1330882 - Part 2: Disallow the session restore to modify window size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/117406/#review119656

::: browser/components/sessionstore/SessionStore.jsm:210
(Diff revision 1)
> + * A global value to tell that fingerprinting resistance is enabled or not.
> + * If it's enabled, the session restore won't restore the window's size and
> + * size mode.
> + * This value is controlled by preference privacy.resistFingerprinting.
> + */
> +var gIsResistFingerPrinting = false;

I would suggest calling this gResistFingerprintingEnabled (lowercase p). We have been treating fingerprinting as a single word in all other references.

::: browser/components/sessionstore/SessionStore.jsm:698
(Diff revision 1)
>      this._prefBranch.addObserver("sessionstore.max_windows_undo", this, true);
> +
> +    try {
> +      gIsResistFingerPrinting = Services.prefs.getBoolPref("privacy.resistFingerprinting");
> +    } catch (ex) {
> +      if (ex.result != Cr.NS_ERROR_UNEXPECTED) {

I agree with Mike that we should give this pref a default value.
Attachment #8843815 - Flags: review?(arthuredelstein) → review+
(Reporter)

Comment 16

a year ago
mozreview-review
Comment on attachment 8843814 [details]
Bug 1330882 - Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled (adopt from Tor #19459).

https://reviewboard.mozilla.org/r/117404/#review119658

::: commit-message-32dcd:1
(Diff revision 1)
> +Bug 1330882 - Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled. r?smaug

Would be nice to refer here to tor #19459 :)

::: browser/base/content/browser.js:1173
(Diff revision 1)
>      // their tasks BEFORE the browser window is shown. SessionStore uses it to
>      // restore tabs into windows AFTER important parts like gMultiProcessBrowser
>      // have been initialized.
>      Services.obs.notifyObservers(window, "browser-window-before-show", "");
>  
> +    let isResistFingerPrinting = false;

Call this `isResistFingerprintingEnabled` (lowercase p).
Attachment #8843814 - Flags: review?(arthuredelstein) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 19

a year ago
mozreview-review-reply
Comment on attachment 8843815 [details]
Bug 1330882 - Part 2: Disallow the session restore to modify window size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/117406/#review119220

> I *think* `false` is the default. Can you make this work like all the other pref observers in this class:
> `Services.prefs.addObserver("privacy.resistFingerprinting", this);` and do the variable setting in `onPrefChange` below.

We still have to give a `false` here since it will throw an exception if we don't give it.
Comment hidden (mozreview-request)

Comment 21

a year ago
mozreview-review
Comment on attachment 8843815 [details]
Bug 1330882 - Part 2: Disallow the session restore to modify window size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/117406/#review120458

Well, since Gijs rs'd the patch that adds the pref, can you land that one first and this patch without the try...catch? With that, r=me. Thanks!
Attachment #8843815 - Flags: review?(mdeboer) → review+

Comment 22

a year ago
mozreview-review
Comment on attachment 8845319 [details]
Bug 1330882 - Part 3: Add a test case for opening new windows as rounded size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118498/#review120718

::: browser/components/resistfingerprinting/test/browser/.eslintrc.js:5
(Diff revision 1)
> +"use strict";
> +
> +module.exports = {
> +  "extends": [
> +    "../../../../../testing/mochitest/browser.eslintrc.js"

I have no idea what this is.
I assume it is right

::: browser/components/resistfingerprinting/test/browser/browser_roundedWindow.js:11
(Diff revision 1)
> +const { classes: Cc, Constructor: CC, interfaces: Ci, utils: Cu } = Components;
> +
> +const TEST_DOMAIN = "http://example.net/";
> +const TEST_PATH = TEST_DOMAIN + "browser/browser/components/resistFingerprinting/test/browser/";
> +
> +let desireWidth  = 1000;

should it be desired, not desire


Why initialize the values here, when they are overridden later.
Attachment #8845319 - Flags: review?(bugs) → review+

Comment 23

a year ago
mozreview-review
Comment on attachment 8845319 [details]
Bug 1330882 - Part 3: Add a test case for opening new windows as rounded size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118498/#review120728

::: browser/components/resistfingerprinting/test/browser/browser_roundedWindow.js:53
(Diff revision 1)
> +      is(content.innerWidth, obj.desireWidth,
> +        "The window.innerWidth has a correct rounded value");
> +      is(content.innerHeight, obj.desireHeight,
> +        "The window.innerHeight has a correct rounded value");
> +    }
> +  );

I think there should be tests for window.open() usage too, when it is opening windows with different sizes.
How does this all work when one does something like
var w = window.open("", "", "width=10000");
w.onresize = function() { console.log(w.innerWidth); }

Please add some tests at least for window.open handling.

Comment 25

a year ago
mozreview-review
Comment on attachment 8843814 [details]
Bug 1330882 - Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled (adopt from Tor #19459).

https://reviewboard.mozilla.org/r/117404/#review120722

::: xpfe/appshell/nsXULWindow.h:94
(Diff revision 2)
>     NS_IMETHOD EnsureContentTreeOwner();
>     NS_IMETHOD EnsurePrimaryContentTreeOwner();
>     NS_IMETHOD EnsurePrompter();
>     NS_IMETHOD EnsureAuthPrompter();
> +   NS_IMETHOD ForceRoundedDimensions();
> +   NS_IMETHOD GetAvailScreenSize(int32_t* availWidth, int32_t* availHeight);

arguments should be in form aName

::: xpfe/appshell/nsXULWindow.cpp:1001
(Diff revision 2)
>        wwatch->GetNewAuthPrompter(ourWindow, getter_AddRefs(mAuthPrompter));
>    }
>    return mAuthPrompter ? NS_OK : NS_ERROR_FAILURE;
>  }
> +
> +NS_IMETHODIMP nsXULWindow::GetAvailScreenSize(int32_t* availWidth, int32_t* availHeight)

ditto

::: xpfe/appshell/nsXULWindow.cpp:1071
(Diff revision 2)
> +                                           1000);
> +  int32_t availForContentWidthCSS =
> +    std::min(maxInnerWidth, NSToIntRound((availWidth - chromeWidth) /
> +                                         devicePerCSSPixels));
> +  int32_t availForContentHeightCSS =
> +    std::min(maxInnerHeight, NSToIntRound((0.95 * availHeight - chromeHeight) /

Why this magical 0.95? Please add a comment
Attachment #8843814 - Flags: review?(bugs) → review+
It looks like that the window.open() can still change its size after fingerprinting resistance is enabled. I will update the test case and write a new patch to address this issue.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
What about one opening a new window and resizing afterwards using window.innerWidth = <new value> or such?

Comment 32

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review120990

I'd rather see this feature break less the web than more, so rounding the size to some reasonable would be good.
And perhaps not even round, but just add upper-limits for the size - similar limits what the other patch gives for newly opened windows.

::: toolkit/components/windowwatcher/nsWindowWatcher.cpp:2481
(Diff revision 1)
>      // at the widget level, or whatever), let's re-fetch the scale factor for
>      // wherever it really ended up
>      treeOwnerAsWin->GetUnscaledDevicePixelsPerCSSPixel(&scale);
>    }
> -  if (aSizeSpec.SizeSpecified()) {
> +  if (aSizeSpec.SizeSpecified() &&
> +      (aIsCallerChrome || !nsContentUtils::ShouldResistFingerprinting())) {

Isn't this too strict? Why prevent all the sizes?
I mean, why not just round the size?

Also, does this affect whether the new window is opened in a new tab or new window?

Couple of lines above we already do checks for too small windows. Couldn't that round the size?
Attachment #8845798 - Flags: review?(bugs) → review-
(In reply to Olli Pettay [:smaug] from comment #31)
> What about one opening a new window and resizing afterwards using
> window.innerWidth = <new value> or such?

I don't think we have to limit this behavior here since the window size which is set by a user is what matters browser fingerprinting. IOW, if a website opens a new window then setting its size through window.innerWidth or innerHeight does not reveal anything about a user because this value is given by the website but not by the user.

And we still remain the capability that users can customize their window size by dragging the window corner or scripts like window.innerWidth even if the fingerprinting resistance is on, which is consistent with Tor browser. Maybe we have to disable this for preventing increasing entropy, but it needs more discussions and not in the scope of this bug.
(In reply to Tim Huang[:timhuang] from comment #33)
> (In reply to Olli Pettay [:smaug] from comment #31)
> > What about one opening a new window and resizing afterwards using
> > window.innerWidth = <new value> or such?
> 
> I don't think we have to limit this behavior here since the window size
> which is set by a user is what matters browser fingerprinting. IOW, if a
> website opens a new window then setting its size through window.innerWidth
> or innerHeight does not reveal anything about a user because this value is
> given by the website but not by the user.
> 

What you mean? The actual window size changes when window.innerWidth is set to a new value.
How is that not revealing the exactly the same information as opening a new window?
First set innerWidth to very high value and after that read what its current value is and you now know
what the size of the screen is.


> And we still remain the capability that users can customize their window
> size by dragging the window corner
That is fine, but I'm not talking about it :)

> or scripts like window.innerWidth even if
> the fingerprinting resistance is on, which is consistent with Tor browser.
> Maybe we have to disable this for preventing increasing entropy, but it
> needs more discussions and not in the scope of this bug.
Well, I don't then quite understand what Tor is trying to do.
Maybe Arthur could clarify that?
Flags: needinfo?(arthuredelstein)
(In reply to Olli Pettay [:smaug] from comment #34)
> (In reply to Tim Huang[:timhuang] from comment #33)
> > (In reply to Olli Pettay [:smaug] from comment #31)
> > > What about one opening a new window and resizing afterwards using
> > > window.innerWidth = <new value> or such?
> > 
> > I don't think we have to limit this behavior here since the window size
> > which is set by a user is what matters browser fingerprinting. IOW, if a
> > website opens a new window then setting its size through window.innerWidth
> > or innerHeight does not reveal anything about a user because this value is
> > given by the website but not by the user.
> > 
> 
> What you mean? The actual window size changes when window.innerWidth is set
> to a new value.
> How is that not revealing the exactly the same information as opening a new
> window?
> First set innerWidth to very high value and after that read what its current
> value is and you now know
> what the size of the screen is.
> 

I think your concern is right here. But, IIUC, the window.innerWidth/Height is a replaceable attributes [1] if it is a content window, so a normal web cannot use this way to acquire available screen size. Only chrome window can change size through innerWidth/innerHeight. So, are you saying that we should make chrome window cannot change size when fingerprinting resistance is on, or, at least, window size should be rounded after setting.  

[1] http://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#14616
Web pages definitely can change the size of a window.
Something like
data:text/html,<a href='' onclick='window.w = window.open("about:blank", "", "width=400"); window.w.onload = function() { window.w.onresize=function() { console.log(window.w.innerWidth); }; window.w.innerWidth = 5000 }' >test</a>

Comment 37

a year ago
Some environments (typically tiling window managers, but also the "Window Rules" feature in KDE's KWin in "force" mode) overrule attempts by programs to enforce dimensions (eg. ignoring Vim's request to snap its dimensions to multiples of the character cell size), so you'll want to take that into consideration.

In the worst case (appearance-wise), I'd do what Vim does and reconcile the difference by padding out the viewport with empty "window background" on the bottom and right edges.

(In my case, I turn off window decorations to ensure that the top row of pixels on the screen gets events routed to the tab bar, so it'd be nice if I could have Firefox retain that ability while mitigating fingerprinting by simply snapping the viewport size to what would be more typical for users with 1280x1024 monitors, then distributing the leftover space somewhere else in the window.)

Comment 38

a year ago
see the discussion at https://trac.torproject.org/projects/tor/ticket/9881

BY setting
* browser.link.open_newwindow.restriction -> 0
* dom.disable_window_move_resize -> true
* full-screen-api.enabled -> false
This mitigates "malicious" windows sizes

Note: you can still right click a link and open in a new window (been a while, I would have to double check).
(In reply to Olli Pettay [:smaug] from comment #34)
> (In reply to Tim Huang[:timhuang] from comment #33)
> > or scripts like window.innerWidth even if
> > the fingerprinting resistance is on, which is consistent with Tor browser.
> > Maybe we have to disable this for preventing increasing entropy, but it
> > needs more discussions and not in the scope of this bug.
> Well, I don't then quite understand what Tor is trying to do.
> Maybe Arthur could clarify that?

We have a separate patch in torbutton (part of Tor Browser; see https://trac.torproject.org/14429) that enforces "quantized" innerWidth and innerHeight for content. So when the user resizes a window, the window "snaps" to a multiple of 100 x 100 pixels.

However this patch is quite complex and needs some polishing before it can be uplifted to Firefox. In the meantime we think the patch Tim is uplifting in this ticket provides a good partial benefit. The size of the screen is not revealed unless the user maximizes the window. And tabs can't be easily correlated by size unless the user manually resizes the window to something unique.

(In reply to Olli Pettay [:smaug] from comment #36)
> Web pages definitely can change the size of a window.
> Something like
> data:text/html,<a href='' onclick='window.w = window.open("about:blank", "",
> "width=400"); window.w.onload = function() { window.w.onresize=function() {
> console.log(window.w.innerWidth); }; window.w.innerWidth = 5000 }' >test</a>

That's an interesting demonstration, but I think it doesn't reveal the size of the screen, because a large window like that simply expands beyond the screen's boundaries.
Flags: needinfo?(arthuredelstein)
(Reporter)

Comment 40

a year ago
mozreview-review-reply
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review120990

I do think there is a benefit to rounding in that it prevents multiple tabs on the new window from being easily correlated by contentWindow.innerWidth/Height. We could round the window size to the nearest 200 x 100 larger than the specified size. An upper limit also probably makes sense.
(Reporter)

Comment 41

a year ago
mozreview-review
Comment on attachment 8845319 [details]
Bug 1330882 - Part 3: Add a test case for opening new windows as rounded size when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118498/#review121226

Looks good.
Attachment #8845319 - Flags: review?(arthuredelstein) → review+
(Reporter)

Comment 42

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review121228
Attachment #8845798 - Flags: review?(arthuredelstein)

Comment 43

a year ago
(In reply to Arthur Edelstein [:arthuredelstein] from comment #39)
> That's an interesting demonstration, but I think it doesn't reveal the size
> of the screen, because a large window like that simply expands beyond the
> screen's boundaries.

While I doubt it's a significant enough slice of the user base to matter, I should mention that there do exist situations where that's not entirely true.

It's been years, but I know that one of the window managers I experimented with in the past would clamp and move windows to ensure that the resize handle couldn't pass out of the bounds of the screen or get covered up by an always-on-top panel.

In that context, all you'd need to do is specify some ridiculously large size and then read off what you actually got.
(In reply to Arthur Edelstein [:arthuredelstein] from comment #39)

> (In reply to Olli Pettay [:smaug] from comment #36)
> > Web pages definitely can change the size of a window.
> > Something like
> > data:text/html,<a href='' onclick='window.w = window.open("about:blank", "",
> > "width=400"); window.w.onload = function() { window.w.onresize=function() {
> > console.log(window.w.innerWidth); }; window.w.innerWidth = 5000 }' >test</a>
> 
> That's an interesting demonstration, but I think it doesn't reveal the size
> of the screen, because a large window like that simply expands beyond the
> screen's boundaries.

On my machines that limits the width of the window to 1920, which is the actual size
(In reply to Olli Pettay [:smaug] from comment #44)
> On my machines that limits the width of the window to 1920, which is the
> actual size

Same for my system. So I think that we have to have some restriction for setting window.innerWidth/innerHeight. If someone sets them with a value which is greater than the maximum rounded size, the innerWidth/Height should be set to the maximum rounded size. If the value is less than the maximum rounded size, we should round the size to the nearest upper 200x100.
(In reply to Tim Huang[:timhuang] from comment #45)
> (In reply to Olli Pettay [:smaug] from comment #44)
> > On my machines that limits the width of the window to 1920, which is the
> > actual size
> 
> Same for my system. So I think that we have to have some restriction for
> setting window.innerWidth/innerHeight. If someone sets them with a value
> which is greater than the maximum rounded size, the innerWidth/Height should
> be set to the maximum rounded size. If the value is less than the maximum
> rounded size, we should round the size to the nearest upper 200x100.

Makes sense to me. Thanks to both of you for finding this issue.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Reporter)

Comment 52

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review123192

Can more of the code in this patch be refactored to share code with the other patches? I'm not 100% sure, but it seems to me the code for rounding both new windows and SetInnerWidth should be almost entirely sharable. (Sorry if I'm wrong about this.) It would be nice to combine in terms of consistency and future maintenance.

::: dom/base/nsContentUtils.cpp:2167
(Diff revision 2)
>    return !isChrome && sPrivacyResistFingerprinting;
>  }
>  
> +/* static */
> +void
> +nsContentUtils::CalcMaximumInnerWindowSizeForResistFingerprinting(int32_t  aChromeWidth,

I find this name confusing. Maybe call this `CalcAvailableInnerWindowSizeForResistingFingerprinting`?

::: dom/base/nsContentUtils.cpp:2189
(Diff revision 2)
> +  // height. It is not necessary for the width since the content width
> +  // is usually pretty much the same as the chrome width.
> +  availContentWidth = std::min(sPrivacyMaxInnerWidth,
> +                                (aScreenWidth - aChromeWidth));
> +  availContentHeight = std::min(sPrivacyMaxInnerHeight,
> +                                NSToIntRound(0.95 * aScreenHeight - aChromeHeight));

It occurs to me now that we could change the correction from `0.95 *`  to `-40 +` because the correction shouldn't really depend on available screen size. Instead it depends on the height of the titlebar on system windows. In my experience, on GTK this height is 26 pixels, so 40px should be pretty safe for most systems.

Also the correction is not needed at all on Mac or Windows, because they correctly include decorations in the outerHeight. So maybe use `%ifdef MOZ_WIDGET_GTK` for the correction?
Attachment #8845798 - Flags: review?(arthuredelstein)
(Reporter)

Comment 53

a year ago
mozreview-review
Comment on attachment 8847962 [details]
Bug 1330882 - Part 5: Add more test cases for rounded windows test.

https://reviewboard.mozilla.org/r/120900/#review123200

Looks great.

::: commit-message-3c023:6
(Diff revision 1)
> +Bug 1330882 - Part 5: Add more test cases for rounded windows test. r?smaug,arthuredelstein
> +
> +This patch adds two more test cases, browser_roundedWindow_open.js and
> +browser_roundedWindow_windowSetting.js. The browser_roundedWindow_open.js tests
> +the window.open() with window features, it will test window.open() with numbers
> +of window features to see that whether the opend window is correctly rounded.

Typo: opened

::: browser/components/resistfingerprinting/test/browser/head.js:8
(Diff revision 1)
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +"use strict";
> +
> +
> +// This function calculates the maximum available window size and returns them

Nit: "window dimensions" or "window sizes"
Attachment #8847962 - Flags: review?(arthuredelstein) → review+

Comment 54

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review123504

Better to review this after the feedback from Arthur has been addressed.
Attachment #8845798 - Flags: review?(bugs)

Comment 55

a year ago
mozreview-review
Comment on attachment 8847962 [details]
Bug 1330882 - Part 5: Add more test cases for rounded windows test.

https://reviewboard.mozilla.org/r/120900/#review123506
Attachment #8847962 - Flags: review?(bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 61

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review123870

::: dom/base/nsGlobalWindow.cpp:14643
(Diff revision 3)
>    if (!ValueToPrimitive<int32_t, eDefault>(aCx, aValue, &value)) {
>      aError.Throw(NS_ERROR_UNEXPECTED);
>      return;
>    }
>  
> +  if (nsContentUtils::ShouldResistFingerprinting(GetDocShell())) {

I wonder if all this should go to the actual methods dealing with size changes, that way we wouldn't need to do rather ugly string comparisons.
But I guess this might be a tad simpler after all.

::: dom/base/nsGlobalWindow.cpp:14692
(Diff revision 3)
> +          int32_t* targetContentWidth = nullptr;
> +          int32_t* targetContentHeight = nullptr;
> +          int32_t unused = 0, screenWidth = 0, screenHeight = 0;
> +          int32_t chromeWidth = 0, chromeHeight = 0;
> +          int32_t inputWidth = 0, inputHeight = 0;
> +

Nit, each variable definition goes to its own line.
Attachment #8845798 - Flags: review?(bugs) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

Thanks, Tim!
Attachment #8845798 - Flags: review?(arthuredelstein) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Hi, Arthur. I don't know why your r+ in comment 69 doesn't show up on the review board. And the new version of the patch 'part 4' is only for rebasing. So you can either ignore the review request or give it r+ directly.
This was autolanded successfully, but pulsebot was down at the time and the bug never got updated.
Keywords: checkin-needed

Comment 78

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ad5841073c68
Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled (adopt from Tor #19459). r=arthuredelstein,smaug
https://hg.mozilla.org/integration/autoland/rev/792b96eb03a0
Part 2: Disallow the session restore to modify window size when fingerprinting resistance is enabled. r=arthuredelstein,mikedeboer
https://hg.mozilla.org/integration/autoland/rev/08657a598ae9
Part 3: Add a test case for opening new windows as rounded size when fingerprinting resistance is enabled. r=arthuredelstein,smaug
https://hg.mozilla.org/integration/autoland/rev/0fa4f1c4f1ee
Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled. r=smaug
https://hg.mozilla.org/integration/autoland/rev/937027c942ae
Part 5: Add more test cases for rounded windows test. r=arthuredelstein,smaug
(Reporter)

Comment 80

a year ago
mozreview-review
Comment on attachment 8845798 [details]
Bug 1330882 - Part 4: Making the window.open() can only open rounded windows and the inner window will be automatically rounded after setting size through innerWidth/Height and outerWidth/Height when fingerprinting resistance is enabled.

https://reviewboard.mozilla.org/r/118948/#review127778
Attachment #8845798 - Flags: review?(arthuredelstein) → review+

Updated

a year ago
Depends on: 1352141

Updated

a year ago
Depends on: 1352305
Depends on: 1353894

Updated

a year ago
Depends on: 1355717

Updated

a year ago
Whiteboard: [fingerprinting][tor] → [fingerprinting][tor][fp:m1]

Updated

a year ago
Depends on: 1364398

Comment 81

11 months ago
mozreview-review
Comment on attachment 8843814 [details]
Bug 1330882 - Part 1: Making new windows to be rounded size when fingerprinting resistance is enabled (adopt from Tor #19459).

https://reviewboard.mozilla.org/r/117404/#review148656

::: browser/base/content/browser.js:1193
(Diff revision 7)
> +      document.documentElement.setAttribute("sizemode", "normal");
> +    } else if (!document.documentElement.hasAttribute("width")) {
>        const TARGET_WIDTH = 1280;
>        const TARGET_HEIGHT = 1040;
>        let width = Math.min(screen.availWidth * .9, TARGET_WIDTH);
>        let height = Math.min(screen.availHeight * .9, TARGET_HEIGHT);

This code is about the very first browser window opened in a fresh profile. How does disabling this code help resist fingerprinting? Without it, what dimensions is that window going to have?

Updated

11 months ago
Flags: needinfo?(tihuang)

Comment 82

11 months ago
Without this code, users have a small screen resolution, less than 1280x1040, will open a maximized window when opening in a fresh profile. A maximized window can reveal your screen resolution which is a fingerprintable factor. So, we try to not open a maxmized window when fingerprinting resistance is enabled.

Actually, 'privacy.resistFingerprinting' is false by default for Firefox, so this is not our use case. However, TorBrowser relies on this to prevent opening a maxmized window for certain users because they enable it by default.
Flags: needinfo?(tihuang)

Comment 83

9 months ago
This approach seems to only work for composing/floating window managers, but has no effect on tiling window managers, since there's no clear concept of window size there.

I think the extra space should be somehow filled in somehow on non-floating WMs.

Comment 84

9 months ago
Whatever approach is used to fill in the extra space in situations where the WM overrules Firefox, I'd appreciate if it weren't too difficult to tweak Firefox's toolbar layout to convert as much of that padding as possible into usable toolbar space.

(And I define "usable" as being "multiple rows", not "make the icons bigger so you can't fit as many")

Updated

7 months ago
Depends on: 1401440
(In reply to Arthur Edelstein (Tor Browser dev) [:arthuredelstein] from comment #0)
> In Tor Browser (where privacy.resistFingerprinting is enabled), when a new
> window is opened, the dimensions of the content are always set to 1000x1000,
> or, if enough screen space is not available, to the nearest 200x100. 

I did some testing with Nightly on Mac OS, and the results are not as expected.
58.0a1 (2017-10-20) (64-bit)

Here are some examples:
1. Screen resolution: 1680x1050
Expected new windows size: 1000x1000
Actual new window dimensions: 1000x800

2. Screen resolution: 1344x840
Expected: 1000x800
Actual: 600x500

3. Screen resolution: 1024x640
Expected: 900x600
Actual: 600x500

4. Screen resolution: 1280x1024
Expected: 1000x1000
Actual: 1000x800

5. Screen resolution: 1050x1680
Expected: 1000x1000
Actual: 1000x1100

Tim Huang, could you please comment on this? Maybe I misunderstood what "the nearest 200x100" mean.
Flags: needinfo?(tihuang)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 86

6 months ago
(In reply to Tom Grabowski [:TomGrab] from comment #85)
> (In reply to Arthur Edelstein (Tor Browser dev) [:arthuredelstein] from
> comment #0)
> > In Tor Browser (where privacy.resistFingerprinting is enabled), when a new
> > window is opened, the dimensions of the content are always set to 1000x1000,
> > or, if enough screen space is not available, to the nearest 200x100. 
> 
> I did some testing with Nightly on Mac OS, and the results are not as
> expected.
> 58.0a1 (2017-10-20) (64-bit)
> 
> Here are some examples:
> 1. Screen resolution: 1680x1050
> Expected new windows size: 1000x1000
> Actual new window dimensions: 1000x800
> 
> 2. Screen resolution: 1344x840
> Expected: 1000x800
> Actual: 600x500
> 
> 3. Screen resolution: 1024x640
> Expected: 900x600
> Actual: 600x500
> 
> 4. Screen resolution: 1280x1024
> Expected: 1000x1000
> Actual: 1000x800
> 

These four situations are expected since we need to subtract the hight of chrome elements when we calculate the window size. And system utilities, like the dock of MACOS, could make the available window size less. Given that, the actual available window size for content would be smaller than the real screen resolution. 


> 5. Screen resolution: 1050x1680
> Expected: 1000x1000
> Actual: 1000x1100
> 

This seems wrong to me, I will take a look, thanks.
Flags: needinfo?(tihuang)

Comment 87

6 months ago
Hi Tom,

I have tested my MAC with a monitor with 1080x1920 resolution, and the result shows that it gives me a 1000x1000 content window when fingerprinting resistance is enabled. That's to say the protection works correctly on my platform. So, I am wondering that the result you gave for the fifth test is the outer window size or the inner window size. The protection supposes to work for the inner window, the content's window. You can use the https://browserleaks.com/javascript to see the screen resolution as well as the available window size for the content window.
Flags: needinfo?(tgrabowski)
(In reply to Tim Huang[:timhuang] from comment #87)
> Hi Tom,
> 
> I have tested my MAC with a monitor with 1080x1920 resolution, and the
> result shows that it gives me a 1000x1000 content window when fingerprinting
> resistance is enabled. That's to say the protection works correctly on my
> platform. So, I am wondering that the result you gave for the fifth test is
> the outer window size or the inner window size. The protection supposes to
> work for the inner window, the content's window. You can use the
> https://browserleaks.com/javascript to see the screen resolution as well as
> the available window size for the content window.

My outer window was 1000x1100. The 100 pixel is the UI at the top, I guess. 
I'm going to run a few more tests, as I just got my hands on another machine with some external screens. I will post here if I see any issues.
Flags: needinfo?(tgrabowski)

Comment 89

6 months ago
I can't get this working, not even close. I'm on ArchLinux, using Xorg+i3, firefox 56.0.1. Step to repro:

* Create a new profile (--no-remote --profilemanager).
* Set privacy.resistFingerprinting to true.
* Restart Firefox (just in case!).
* Visit https://browserleaks.com/javascript:
  * Result is 1920×1088 24-bit TrueColor (working area: 1912×1088)

I can consistently reproduce this on different machines. These values are definitely not rounded to anything not-unique.
(In reply to Hugo Osvaldo Barrera from comment #89)
> I can't get this working, not even close. I'm on ArchLinux, using Xorg+i3,
> firefox 56.0.1. Step to repro:
> 
> * Create a new profile (--no-remote --profilemanager).
> * Set privacy.resistFingerprinting to true.
> * Restart Firefox (just in case!).
> * Visit https://browserleaks.com/javascript:
>   * Result is 1920×1088 24-bit TrueColor (working area: 1912×1088)
> 
> I can consistently reproduce this on different machines. These values are
> definitely not rounded to anything not-unique.

Hugo,

Perhaps the problem is that you are using Firefox 56.0.1. I'm not sure whether this feature is fully implemented there.
Could you try downloading DevEdition Firefox and running it there?  You can use this link: https://www.mozilla.org/en-US/firefox/developer/  (Dev Edition will create its own independent profile, so no need to worry about creating profiles manually - you can use this browser independently from your 56.0.1)

Also, once you start your browser and set privacy.resistFingerprinting to true, open a new window by clicking on "New Window" (in DevEdition it should be in the drop-down menu on the right). See if that newly opened window has the expected size.

Thanks,
Tom
Flags: needinfo?(hugo)

Comment 91

6 months ago
Similar results on firefox-dev:

* Ran firefox-dev (58.0b1 (64-bit)).
* Set privacy.resistFingerprinting to true.
* Restart Firefox (just in case!).
* Visit https://browserleaks.com/javascript:
  * Result is 1920×1086 24-bit TrueColor (working area: 1912×1086)

I guess the 2px difference is probably due to the chrome changes in the dev release, but it's still isn't working as expected.
Flags: needinfo?(hugo)

Comment 92

6 months ago
I tested this issue on Ubuntu 16.04 on The latest nightly using these steps:

- Go to about:config
- Turn privacy.resistFingerprinting parameter to true
- Open new browser window by clicking on "New Window"

I registered these results on the following resolutions:

1. 3840 x 2160 (16:9)
   New window size = 1000 x 1000 24-bit TrueColor (Working Area 990 x 1000)
2. 2028 x 1280 (16:10)
   New window size = 1000 x 1000 24-bit TrueColor (Working Area 990 x 1000)
3. 1920 x 1080 (16:9)
   New window size = 1000 x 900 24-bit TrueColor (Working Area 990 x 900)
4. 1600 x 1200 (4:3)
   New window size = 1000 x 1000 24-bit TrueColor (Working Area 990 x 1000)
5. 1280 x 1024 (5:4)
   New window size = 1000 x 800 24-bit TrueColor (Working Area 990 x 800)
6. 1280 x 720 (16:9)
   New window size = 1000 x 500 24-bit TrueColor (Working Area 990 x 500)


The test cases I have didn't match my monitors resolutions, so I tested using my monitor resolutions.
Could you please tell my if these results are the desirable once?

Thanks.
Flags: needinfo?(tihuang)

Comment 93

6 months ago
Hi Hani,

I would say that they are desirable results in terms of the window size. However, it kinds a little bit unexpected for the result of the working area, they are supposed to be 1000 in these cases. May I ask you do you have some customizations alongside the content window? If you do, this could be the reason why there is a 10px decrease in the working area.
Flags: needinfo?(tihuang)

Comment 94

6 months ago
I have the default setting on Ubuntu 16.04.
Could you please tell me more what kind of customizations I could have?

Comment 95

6 months ago
@Tim - AFAIK, the "working area" at https://browserleaks.com/javascript is simply the inner window minus the scrollbar(s) - if you click "more" you can see the values are generated from div.clientWidth and div.clientHeight

On Windows 7 the (vertical) scrollbar removes 17 pixels. Hugo above has 8 pixels difference. Hani on Ubuntu 16.04 (GNOME? KDE?) it removes 10 pixels (on Ubuntu 14.04 with GNOME it removed 13 pixels - see https://trac.torproject.org/projects/tor/ticket/22137 )

See Bug 1397996, and if you go to https://www.hackerfactor.com/blog/index.php?/archives/761-Exploiting-the-TOR-Browser.html and scroll down to the "Test results for your browser" you can see what I mean. If you look at the JS for that it uses innerHeight and clientHeight and math.

Comment 96

6 months ago
Thanks for pointing this out, Simon.

So, I think the 10px is for the scroll bar, but not the customizations.

Comment 97

6 months ago
I'd assume that the rounding is done *after* taking things like scrollbar size into consideration. Otherwise this'll still leak fingerprintable data (especially when the system theme is a custom one with an uncommonly sized scrollbar).

Comment 98

6 months ago
^^ Not every page generates a scrollbar. There are also two possible srollbars (vertical, horizontal). The sizing is also done before the browser is displayed or anything loaded. Bug 1397996 may one day address uniformity of scrollbars across platforms

Comment 99

6 months ago
Maybe always show a scrollbar (at least when this setting is active), and make just the display area fixed?

What I mean is, if we're making the display area 1000x1000px, keep the scrollbars _outside_ that area, and also always show them (kinda like `overflow: scroll` would? This fixes both part of this issue and 1397996.

Comment 100

6 months ago
(In reply to Hugo Osvaldo Barrera from comment #91)
> Similar results on firefox-dev:
> 
> * Ran firefox-dev (58.0b1 (64-bit)).
> * Set privacy.resistFingerprinting to true.
> * Restart Firefox (just in case!).
> * Visit https://browserleaks.com/javascript:
>   * Result is 1920×1086 24-bit TrueColor (working area: 1912×1086)

note: previous result by Hugo on 56 was working area (1912x1088)

What is your screen resolution? To me this sounds like you are grabbing the results from https://browserleaks.com/javascript AFTER you have maximized firefox or put it into full screen.
I'm not using a composing window manager, so there's not concept of maximized/minimized full windows; just windows.

My screen resolution is 2880x1800, and Xorg is configured at 160dpi.

Updated

5 months ago
Depends on: 1418537

Comment 102

5 months ago
@Arthur et al: Just throwing some ideas out there - floating scrollbars, floating findbar - not just for privacy.resistFingerprinting, but in all modes. Here is an (old) floating scrollbars (I have no idea if it still works) - https://github.com/ardiman/userChrome.js/tree/master/floatingscrollbar. Perhaps we could create two new tickets for these so at least it's tracked, and add it to the Tor Uplift Project?

Comment 103

5 months ago
(In reply to Simon Mainey from comment #102)
> @Arthur et al: Just throwing some ideas out there - floating scrollbars,
> floating findbar - not just for privacy.resistFingerprinting, but in all
> modes. Here is an (old) floating scrollbars (I have no idea if it still
> works) -
> https://github.com/ardiman/userChrome.js/tree/master/floatingscrollbar.

Interesting. Sounds like a good idea to me to explore that approach.

> Perhaps we could create two new tickets for these so at least it's tracked,
> and add it to the Tor Uplift Project?

Fine with me. Some of this is tracked in our https://trac.torproject.org/projects/tor/ticket/16456 although this one is a more generic *bar-is-changing-the-reported-screen-size-ticket. It seems there is already a ticket (1418537) for the bookmark issue, good.

Comment 104

5 months ago
> Interesting. Sounds like a good idea to me to explore that approach.

OK, I explored it - see https://bugzilla.mozilla.org/show_bug.cgi?id=1397996#c2 .. now what? :)

Updated

4 months ago
Whiteboard: [fingerprinting][tor][fp:m1] → [fingerprinting][tor][fp:m1][fp-triaged]
Duplicate of this bug: 870346

Comment 106

3 months ago
Bumping this to P3 and tagging for eventual investigate into corner cases that aren't working.
Assignee: artines1 → nobody
Priority: P1 → P3
You need to log in before you can comment on or make changes to this bug.