Closed Bug 1409973 Opened 7 years ago Closed 6 years ago

Make Date.toLocaleDateString and Intl.DateTimeFormat anti-fingerprintable

Categories

(Core :: JavaScript Engine, enhancement, P1)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
relnote-firefox --- -
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: tjr, Assigned: tjr, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: feature, Whiteboard: [fingerprinting][tor][fp-triaged])

Attachments

(2 files, 3 obsolete files)

Bug 1409973 - Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance
59 bytes, text/x-review-board-request
zbraniecki
: review+
Details
Bug 1409973 - Update JS runtime default locale when pref javascript.use_us_english_local changes.
59 bytes, text/x-review-board-request
Gijs
: review+
zbraniecki
: feedback+
Details 
https://jsfiddle.net/x1awkbg2/1/ shows different values based on the user's OS, Locale, and timezone.

> log(new Date().toLocaleFormat()); <- leaks OS and maybe locale
> log(new Date().toLocaleString()); <- leaks timezone and locale
> log(new Intl.DateTimeFormat().format(new Date())); <- leaks timezone and maybe locale


The timezone was intended to be covered by Bug 1333933 (which also has more information about this bug) and the locale in Bug 1369330.

Per Georg: The Intl.DateTimeFormat one seems to be https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-52.4.0esr-7.5-1&id=9973f6ce594863aad8bec9efec44947a0a5db548 which could get upstreamed.

Tor's bug for LocaleFormat and LocaleString is https://trac.torproject.org/projects/tor/ticket/15473 (which includes some in-depth investigation.)
FYI: relevant Tor tickets
- https://trac.torproject.org/projects/tor/ticket/21784 - incl objects TypedArray and Array
- https://trac.torproject.org/projects/tor/ticket/22125 - Unit test for js locale
- https://trac.torproject.org/projects/tor/ticket/21608 - incl DateTimeFormat.formatToParts
- https://trac.torproject.org/projects/tor/ticket/22130 - date format
Basically, whole Intl is a fingerprinting vector as it, in one way or the other, exposes a CLDR database bits that will differ between versions of the browser and locales.

Going through every single API separately doesn't make much sense. DateTimeFormat - yes, NumberFormat - yes, Collator - yes, RelativeTimeFormat - yes, PluralRules - yes, and so on.

I would say what you really want is limit ECMA402 Intl API to only expose one language
Date.toLocaleFormat is going to be removed in bug 818634, there also seem to be some other changes happening there.
Component: DOM: Security → JavaScript Engine
Depends on: 1369330, 1333933
Depends on: 818634
Summary: Make Date.toLocaleFormat, Date.toLocaleDateString and Intl.DateTimeFormat anti-fingerprintable → Make Date.toLocaleDateString and Intl.DateTimeFormat anti-fingerprintable
(In reply to Tom Ritter [:tjr] from comment #0)
> Per Georg: The Intl.DateTimeFormat one seems to be
> https://gitweb.torproject.org/tor-browser.git/commit/?h=tor-browser-52.4.
> 0esr-7.5-1&id=9973f6ce594863aad8bec9efec44947a0a5db548 which could get
> upstreamed.

bug 1330149 has a patch to change ICU's default time zone when the TZ environment variable is set on Windows systems (ICU already uses TZ on Unix-based systems [1]).

[1] https://searchfox.org/mozilla-central/rev/1c4901d060e3953e41088c038b62a0c026c1e1fb/intl/icu/source/common/putil.cpp#1110
I'd be happy to take patches for this. Getting ICU to support it seems like most of the effort.

Some prefs already get piped into the JS engine; see the ReloadPrefsCallback() in js/xpconnect/src/XPCJSContext.cpp; we can use that kind of mechanism to pass the "privacy.resistFingerprinting" setting through to JS and then to ICU.
Mentor: jorendorff
Priority: -- → P3
This should now be fixed with bug 1330149 merged.

STR:
1. Go to about:config
2. Open console (ctrl+shift+I)
3. Evaluate `new Date().toLocaleString("en", {timeZoneName:"long"})`
4. Enable "privacy.resistFingerprinting" and re-evaluate step 3
5. Disable "privacy.resistFingerprinting" and re-evaluate step 3

Windows 10
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 7:49:15 AM Pacific Daylight Time"
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 2:49:19 PM GMT"
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 7:49:24 AM Pacific Daylight Time"


Ubuntu 16.04
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 7:57:11 AM Pacific Daylight Time"
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 2:57:17 PM GMT"
> new Date().toLocaleString("en", {timeZoneName:"long"})
"11/4/2017, 6:57:20 AM GMT-08:00"


The time zone is set to "GMT-08:00" on Linux, because ICU doesn't grok "TZ=:/etc/localtime" [1], but that should probably tracked in a different issue...

[1] https://searchfox.org/mozilla-central/rev/af86a58b157fbed26b0e86fcd81f1b421e80e60a/toolkit/components/resistfingerprinting/nsRFPService.cpp#304
Assignee: nobody → cfu
Priority: P3 → P1
Whiteboard: [fingerprinting][tor] → [fingerprinting][tor][fp-triaged]
Attached image Screen Shot 2018-01-05 at 4.12.17 PM.png (obsolete) — 
It seems that we still have to handle locale, since Bug 1369330 depends on the pref javascript.use_us_english_locale but it doesn't affect Date.toLocaleString().

The attachment illustrates a test result with Japanese version of Nightly 59.0a1 on macOS. It can be found that the default locale is still "ja" even when javascript.use_us_english_locale is true. So I guess a possible solution is to spoof the first argument of Date.toLocaleString() by "en" when privacy.resistFingerprinting is true and it is undefined. Perhaps around here https://searchfox.org/mozilla-central/rev/652fbd6270de0d3ec424d2b88f8375ff546f949f/js/src/builtin/Date.js#94
The browser needs to be restarted after changing the "javascript.use_us_english_locale" option.
After some further experiments, debugger console in about:config and about:preferences seems to use a different JS runtime whose default locale is not affected by javascript.use_us_english_locale, so I got that result. I think it comes from https://searchfox.org/mozilla-central/rev/f42618c99dcb522fb674221acfbc68c2d92e7936/dom/workers/RuntimeService.cpp#985

In other cases, the default locale of JS runtime is initialized from https://searchfox.org/mozilla-central/rev/f42618c99dcb522fb674221acfbc68c2d92e7936/js/xpconnect/src/XPCLocale.cpp#141,149

So the solution just looks good, I will write some test cases.
Blocks: 1330890
No longer depends on: 1330890
Attachment #8940088 - Attachment is obsolete: true
Attachment #8941312 - Flags: review?(tom)
As anba mentioned in comment 10, the browser needs to be restarted, or the intl:app-locales-changed event have to be fired, to make javascript.use_us_english_locale effective. Now I fire the event in the test, but do you think we should do this where we set javascript.use_use_english_locale?
https://searchfox.org/mozilla-central/rev/03877052c151a8f062eea177f684a2743cd7b1d5/toolkit/components/resistfingerprinting/LanguagePrompt.jsm#106,114
I would prefer if you used a different event. This event is specific to app locales changing and controlled by LocaleService. You're not changing app-locales, you're mocking them for a certain subset of uses.

I'd prefer the whole app not to try to retranslate itself because you triggered this setting.

And tbh, you should be able to just set an observer on this pref, rather than having it on the event.


p.s. the longer I look into this the more confident I am that naming it "use english" is short sighted that doesn't scale to other cultures and we should look into making it not english specific (for example, en-US in China, Spain or Saudi Arabia will rather make you stand out)
Comment on attachment 8941312 [details]
Bug 1409973 - Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance.

https://reviewboard.mozilla.org/r/211610/#review217538

::: browser/components/resistfingerprinting/test/mochitest/test_bug1409973_date_time_format.html:40
(Diff revision 1)
> +  });
> +  SpecialPowers.Services.obs.notifyObservers(null, topic);
> +  await observed;
> +
> +  // Check now timezone is UTC and default locale is en-US.
> +  SimpleTest.is(date.toLocaleString(undefined, {timeZoneName: "long"}), ref, "Date.toLocaleString");

Can you add a test for toLocaleFormat also?
(In reply to Tom Ritter [:tjr] from comment #15)
> Comment on attachment 8941312 [details]
> Bug 1409973 - Add tests for Date.toLocaleString and
> Intl.DateTimeFormat.format fingerprinting resistance.
> 
> https://reviewboard.mozilla.org/r/211610/#review217538
> 
> :::
> browser/components/resistfingerprinting/test/mochitest/
> test_bug1409973_date_time_format.html:40
> (Diff revision 1)
> > +  });
> > +  SpecialPowers.Services.obs.notifyObservers(null, topic);
> > +  await observed;
> > +
> > +  // Check now timezone is UTC and default locale is en-US.
> > +  SimpleTest.is(date.toLocaleString(undefined, {timeZoneName: "long"}), ref, "Date.toLocaleString");
> 
> Can you add a test for toLocaleFormat also?

Date.toLocaleFormat has been removed in bug 818634.
Attachment #8941312 - Flags: review?(tom)
Comment on attachment 8941312 [details]
Bug 1409973 - Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance.

https://reviewboard.mozilla.org/r/211610/#review218298

LGTM, but you should address :zibi's comments.

Also, I don't think my r+ is enough to land and you'd need a peer to review.
Attachment #8941312 - Flags: review+
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #14)
> I would prefer if you used a different event. This event is specific to app
> locales changing and controlled by LocaleService. You're not changing
> app-locales, you're mocking them for a certain subset of uses.
> 
> I'd prefer the whole app not to try to retranslate itself because you
> triggered this setting.
> 
> And tbh, you should be able to just set an observer on this pref, rather
> than having it on the event.

Thank you very much for the comment. It seems better to update JS runtime default locale when the pref javascript.use_us_english_locale changes, so I added a pref listener as you suggested.

Because the ownership of XPCLocaleObserver instance belongs to observables, currently ObserverService
https://searchfox.org/mozilla-central/rev/137f1b2f434346a0c3756ebfcbdbee4069e15dc8/js/xpconnect/src/XPCLocale.cpp#83
I think adding another strong reference to Preferences doesn't cause memory leak.

Could you please help review the patches?

> p.s. the longer I look into this the more confident I am that naming it "use
> english" is short sighted that doesn't scale to other cultures and we should
> look into making it not english specific (for example, en-US in China, Spain
> or Saudi Arabia will rather make you stand out)

The goal is to make all users with fingerprinting resistance enabled look the same. So even en-US users are minority in some regions like China, their local websites can't still identify each individual.
Attachment #8941312 - Flags: review?(gandalf)
Attachment #8942608 - Flags: review?(gandalf)
(In reply to Chung-Sheng Fu [:cfu] from comment #20)
> The goal is to make all users with fingerprinting resistance enabled look
> the same. So even en-US users are minority in some regions like China, their
> local websites can't still identify each individual.

Agreed: There has to be an expectation that some values (such as IP) are already masked (your real ISP provided IP+time are pretty much a dead giveaway and are outside the scope of the app to be honest). This is what VPNs etc are for.

As long as the subset of RFP users all have the same value it removes the usefulness of that value in the entropy set, as Chung-Sheng Fu says. Values we cannot control, such as an end user's IP, are a moot point (not that we couldn't try to find other solutions to help mitigate this, eg in this case building in the tor protocol into Firefox as has been mentioned elsewhere, or adding a VPN service like in Opera)
Comment on attachment 8942608 [details]
Bug 1409973 - Update JS runtime default locale when pref javascript.use_us_english_local changes.

https://reviewboard.mozilla.org/r/212876/#review219524

lgtm!
Attachment #8942608 - Flags: review?(gandalf) → review+
Comment on attachment 8941312 [details]
Bug 1409973 - Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance.

https://reviewboard.mozilla.org/r/211610/#review219526

::: browser/components/resistfingerprinting/test/mochitest/test_bug1409973_date_time_format.html:28
(Diff revision 2)
> +  SimpleTest.is(date.toLocaleString(undefined, {timeZoneName: "long"}), ref, "Date.toLocaleString");
> +  let formatter = new Intl.DateTimeFormat(undefined, {
> +    month: "numeric", day: "numeric", year: "numeric", hour: "numeric", minute: "numeric", second: "numeric",
> +    timeZoneName: "long"
> +  });
> +  SimpleTest.is(formatter.format(date), ref, "Intl.DateTimeFormat.format");

Please, switch this test to use `resolvedOptions()` to  verify the right settings are being used.
Attachment #8941312 - Flags: review?(gandalf) → review+
Assignee: cfu → tom
Flags: needinfo?(tom)
Attachment #8941312 - Attachment is obsolete: true
Attachment #8942608 - Attachment is obsolete: true
I'm going to try and pick this up from CS. I believe I addressed your comment about resolvedOptions() but I am not 100% certain.  (And in any event, the r+'s will not transfer).  Could you please review my versions?
Flags: needinfo?(tom) → needinfo?(gandalf)
So, the patch tests that the timezone is UTC and language is en-US, but does nothing to check if it's the result of the anti fingerprinting setting or a default for the system.

Shouldn't you first temporarily modify the language/timezone, then test that without the setting it is modified, and with the setting it's not?
Flags: needinfo?(gandalf) → needinfo?(tom)
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #27)
> So, the patch tests that the timezone is UTC and language is en-US, but does
> nothing to check if it's the result of the anti fingerprinting setting or a
> default for the system.
> 
> Shouldn't you first temporarily modify the language/timezone, then test that
> without the setting it is modified, and with the setting it's not?

That makes sense. I worked on this a bit today and ran into the following.

I set up the timezone checker like so (and am in Central timezone, so it doesn't match my system):

>  // First be sure we have a non-UTC timezone and a non en-US locale.
>  var setTimeZone = SpecialPowers.Cu.getJSTestingFunctions().setTimeZone;
>  setTimeZone("EST5EDT");
>  // Now sanity check the defaults  
>  let date = new Date(2003, 4, 6, 2, 30, 15);
>  SimpleTest.is(date.toString(), "Tue May 06 2003 02:30:15 GMT-0400 (EDT)", "Sanity check of control timezone failed.");
>  let defaultOptions = new Intl.DateTimeFormat(undefined, {
>    month: "numeric", day: "numeric", year: "numeric", hour: "numeric", minute: "numeric", second: "numeric",
>    timeZoneName: "long"
>  }).resolvedOptions();
>  SimpleTest.is(defaultOptions.locale, "en-US", "Intl.DateTimeFormat.format.locale");
>  SimpleTest.is(defaultOptions.timeZone, "EST5EDT", "Intl.DateTimeFormat.format.timeZone");
>  SimpleTest.is(defaultOptions.timeZoneName, "long", "Intl.DateTimeFormat.format.timeZoneName");

Then I create a reference toLocaleString:

>  // Then create output it as UTC en-US date string; which should be constant
>  const referenceLocaleString = date.toLocaleString("en-US", {timeZone: "UTC", timeZoneName: "long"});
>  SimpleTest.is(referenceLocaleString, "5/6/2003, 6:30:15 AM GMT", "Sanity check of reference timezone failed.");

Everything so far worked fine.

Then I went to set a non-default locale like so:

> const localeService = SpecialPowers.Cc["@mozilla.org/intl/localeservice;1"]
>  .getService(SpecialPowers.Ci.mozILocaleService);
> localeService.setRequestedLocales(['ko-KR']);

I think that after I set the locale, calling date.toLocaleString() (with no args) should give me something like '2012. 12. 20. 오전 3:00:00' but it continues to give me '5/6/2003, 2:30:15 AM'.  I'm not sure how to set the locale such that I've changed the default (and can test the pref.)
Flags: needinfo?(tom) → needinfo?(gandalf)
> I think that after I set the locale, calling date.toLocaleString() (with no args) should give me something like '2012. 12. 20. 오전 3:00:00' but it continues to give me '5/6/2003, 2:30:15 AM'.  I'm not sure how to set the locale such that I've changed the default (and can test the pref.)

The issue is that you're changing the request locale, not app locale. After you do that, we renegotiate app locale but since available locales don't have your new requested one, we just stick to the previous one.

I'm going to fix it in bug 1358653 over the next week or so. Is it ok for this bug to wait for that?
Flags: needinfo?(tom)
Yup, I'll wait!
Depends on: 1358653
Flags: needinfo?(tom)
Flags: needinfo?(gandalf)
Attachment #8945016 - Flags: review?(gandalf)
Attachment #8945017 - Flags: review?(gandalf)
Comment on attachment 8945016 [details]
Bug 1409973 - Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance

https://reviewboard.mozilla.org/r/215210/#review228888

::: browser/components/resistfingerprinting/test/chrome/test_bug1409973_date_time_format.html:10
(Diff revision 2)
> +<script>
> +  ChromeUtils.import("resource://gre/modules/Services.jsm");
> +
> +  const originalLocales = Services.locale.getAvailableLocales();
> +  Services.locale.setAvailableLocales(['ko-KR']);
> +  Services.locale.setRequestedLocales(["ko-KR"]);

I'm not 100% sure, but I think that talos tests run in sequence reusing the same process.

If you switch everything to a mock `ko-KR`, without cleaning it up at the end, you'll keep the mock locale in the process which may affect other tests (unfortunately lots of them depend on the locale being `en-US`).

Brute force way to do it is:

```
let originalAvailable = Services.locale.getAvailableLocales();
let originalRequested = Services.locale.getRequestedLocales();

....

// cleanup
Services.locale.setRequestedLocales(originalRequested);
Services.locale.setAvailableLocales(originalAvailable);
```

::: browser/components/resistfingerprinting/test/chrome/test_bug1409973_date_time_format.html:32
(Diff revision 2)
> +  SimpleTest.is(defaultOptions.timeZone, "EST5EDT", "defaultOptions Intl.DateTimeFormat.format.timeZone");
> +  SimpleTest.is(defaultOptions.timeZoneName, "long", "defaultOptions Intl.DateTimeFormat.format.timeZoneName");
> +
> +  // Then create output it as UTC en-US date string; which should be constant
> +  const referenceLocaleString = date.toLocaleString("en-US", {timeZone: "UTC", timeZoneName: "long"});
> +  SimpleTest.is(referenceLocaleString, "5/6/2003, 6:30:15 AM GMT", "Sanity check of reference timezone failed.");

I would recommend doing it via ` new Intl.DateTimeFormat("en-US", {...})` - `date.toLocaleString` uses the same logic, but doesn't give you an object and in result you can't test `resolvedOptions` which is way better than testing the exact string output out of Intl API.
Attachment #8945016 - Flags: review?(gandalf) → review+
Comment on attachment 8945017 [details]
Bug 1409973 - Update JS runtime default locale when pref javascript.use_us_english_local changes.

I feel comfortable offering f+ on this patch, but you should get review from someone who better understands C++ memory management model when it comes to strong vs weak observers.
Attachment #8945017 - Flags: review?(gandalf) → feedback+
Comment on attachment 8945017 [details]
Bug 1409973 - Update JS runtime default locale when pref javascript.use_us_english_local changes.

https://reviewboard.mozilla.org/r/215212/#review229110

r=me on using a strong observer here to match the other strong observer that's already there, and given that the code that instantiates these objects explicitly says that the observer service keeps them alive (line 86 post-patch)
Attachment #8945017 - Flags: review+
Attachment #8945017 - Flags: review?(gandalf) → feedback+
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/790b8ab1032b
Add tests for Date.toLocaleString and Intl.DateTimeFormat.format fingerprinting resistance r=gandalf
https://hg.mozilla.org/integration/autoland/rev/a8c0ea1bb08f
Update JS runtime default locale when pref javascript.use_us_english_local changes. r=Gijs
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/790b8ab1032b
https://hg.mozilla.org/mozilla-central/rev/a8c0ea1bb08f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox60: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Too late for 58 & 59

Would be nice to add that to the release notes.

Potential wording:
Date.toLocaleDateString and Intl.DateTimeFormat are now anti-fingerprintable
status-firefox58: affected → wontfix
status-firefox59: --- → wontfix
relnote-firefox: --- → ?
I'm not sure this is worth a release note, this is behind a (hidden) pref and seems like a bit of a detail not a top-level feature?
Flags: needinfo?(sledru)
(In reply to Julien Cristau [:jcristau] from comment #41)
> I'm not sure this is worth a release note, this is behind a (hidden) pref
> and seems like a bit of a detail not a top-level feature?

I agree...
In the past, we mentioned some anti finger printing features, they have been piked up by the press and as it is part of our overall narrative, this is why I suggested that.
Maybe we should ask to marketing or product what they think.
Flags: needinfo?(sledru)
The general risk with these kind of prefs is that users will flip it on without actually knowing what it does (because some blog told them to), and they end up seeing strange behaviours on the websites they browse without knowing why.

privacy.resistFingerprinting is a perfect example of this. Ever since it was introduced, Reddit sees a rise in "why is my Firefox detected as Firefox 52" posts.

If these kinds of prefs are in the release notes, then there should be a solid UX story behind them.
(In reply to André Bargull [:anba] from comment #7)
> The time zone is set to "GMT-08:00" on Linux, because ICU doesn't grok
> "TZ=:/etc/localtime" [1], but that should probably tracked in a different
> issue...

Tracked upstream in <https://ssl.icu-project.org/trac/ticket/13694>.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: