Closed Bug 1330890 Opened 8 years ago Closed 7 years ago

Use UTC timezone when privacy.resistFingerprinting = true [tor 16622]

Categories

(Core :: General, defect, P1)

Product:
Core
Component:
General
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: arthur, Assigned: tjr)

References

(Blocks 2 open bugs)

Details

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

Attachments

(2 files, 1 obsolete file)

Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622).
(deleted), text/x-review-board-request
ehsan.akhgari
: review+
arthur
: review+
Details
Bug 1330890 - Part 2: Add a test case for using UTC timezone when 'privacy.resistfingerprinting' is true.
(deleted), text/x-review-board-request
ehsan.akhgari
: review+
arthur
: review+
Details
In Tor Browser, to resist fingerprinting by content scripts, we set the timezone to UTC. Here is the Tor Browser patch: torpat.ch/16622 We would like to propose uplifting this patch.
Whiteboard: [tor][fingerprinting] → [tor 16622][fingerprinting]
Can this be added to the Tor Uplift Tracking list please ( https://wiki.mozilla.org/Security/Tor_Uplift/Tracking )
Priority: -- → P2
Summary: Use UTC timezone when privacy.resistFingerprinting = true → Use UTC timezone when privacy.resistFingerprinting = true [tor 16622]
Whiteboard: [tor 16622][fingerprinting] → [fingerprinting]
Whiteboard: [fingerprinting] → [fingerprinting][tor 16622]
(In the uplifted patch, I would suggest using the "privacy.resistFingerprinting" pref, instead of "privacy.use_utc_timezone".)
Assignee: nobody → tihuang
Priority: P2 → P1
Attachment #8855734 - Flags: review?(arthuredelstein)
Attachment #8855735 - Flags: review?(arthuredelstein)
Attachment #8855736 - Flags: review?(arthuredelstein)
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review131686
Attachment #8855734 - Flags: review?(arthuredelstein) → review+
Comment on attachment 8855736 [details] Bug 1330890 - Part 2: Add a test case for using UTC timezone when 'privacy.resistfingerprinting' is true. https://reviewboard.mozilla.org/r/127648/#review131688 Looks good to me. Does the new async/await syntax work in browser tests like this one?
Attachment #8855736 - Flags: review?(arthuredelstein) → review+
Comment on attachment 8855735 [details] Bug 1330890 - Part 2: Add nsRFPService.cpp, which is a service of fingerprinting resistance. https://reviewboard.mozilla.org/r/127646/#review131700 ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:66 (Diff revision 1) > + bool isRPFEnabled = Preferences::GetBool(RESIST_FINGERPRINTING_PREF); > + > + if (isRPFEnabled) { > + PR_SetEnv("TZ=UTC"); > + } else { > + unsetenv("TZ"); My understanding from the unsetenv documentation is that this function will delete the environment variable altogether. So I think this code will fail to respect a value for TZ already set by the user before the process was started. Also it's probably good to use only the `PR_*` functions to ensure consistent env var handling. Maybe a safer approach would be to store an `mInitialTZValue` in the ::Init function, using PR_GetEnv, and then here (for isRPFEnabled == false) use `PR_SetEnv(mInitialTZValue)`.
Attachment #8855735 - Flags: review?(arthuredelstein) → review-
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review135904 (My apologies for the long delay here.) ::: toolkit/xre/nsAppRunner.cpp:5162 (Diff revision 1) > } > } > > void > +UseUTCTimeZoneIfNeeded() { > + if (mozilla::Preferences::GetBool("privacy.resistFingerprinting", false)) { Nit: please use nsContentUtils::ShouldResistFingerprinting() instead. ::: toolkit/xre/nsAppRunner.cpp:5163 (Diff revision 1) > } > > void > +UseUTCTimeZoneIfNeeded() { > + if (mozilla::Preferences::GetBool("privacy.resistFingerprinting", false)) { > + SaveToEnv("TZ=UTC"); You need to also call _tzset() on Windows, otherwise this would have no effect there. https://msdn.microsoft.com/en-us/library/90s5c885.aspx It would be interesting to see why this wasn't uncovered by tests... Perhaps we're calling _tzset() later on somehow and you're getting lucky? There are two calls to this function from js/src/vm/DateTime.cpp. But you shouldn't count on those being called first.
Attachment #8855734 - Flags: review?(ehsan) → review-
Comment on attachment 8855735 [details] Bug 1330890 - Part 2: Add nsRFPService.cpp, which is a service of fingerprinting resistance. https://reviewboard.mozilla.org/r/127646/#review135910 This is gneerally fine, but I have a better suggestion on how (and where) to do this. nsContentUtils already reads and caches this pref. It seems like a mistake to have another cache and observer for this pref. Can you please just make the bool var cache used there into an observer that just runs a little bit of code during initialization and when the pref changes? It would be nice if you make the observer object have a UpdatePrefs() method or some such that you call both during initialization and during Observe() so that you can basically fold parts 1 and 2 here into the same patch, as the work in initializing and responding to the changes to the pref should be exactly the same. BTW looks like it is nsJSUtils::ResetTimeZone() which calls _tzset() for you -- you can just rely on that but please add a comment explaining that you're relying on this happening.
Attachment #8855735 - Flags: review?(ehsan) → review-
Comment on attachment 8855736 [details] Bug 1330890 - Part 2: Add a test case for using UTC timezone when 'privacy.resistfingerprinting' is true. https://reviewboard.mozilla.org/r/127648/#review135914
Attachment #8855736 - Flags: review?(ehsan) → review+
Whiteboard: [fingerprinting][tor 16622] → [fingerprinting][tor 16622][fp:m1]
Attachment #8855735 - Attachment is obsolete: true
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review139206 Thanks, looks good overall but there are some bugs in the patch... ::: toolkit/components/resistfingerprinting/nsRFPService.h:22 (Diff revision 2) > +public: > + NS_DECL_ISUPPORTS > + NS_DECL_NSIOBSERVER > + > + static nsRFPService* GetOrCreate(); > + static bool IsResistFingerprintingEnabled() { Nit: open brace goes on its own line. ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:29 (Diff revision 2) > +#define RESIST_FINGERPRINTING_PREF "privacy.resistFingerprinting" > + > +NS_IMPL_ISUPPORTS(nsRFPService, nsIObserver) > + > +static StaticRefPtr<nsRFPService> gRFPService; > +static bool sInitialized = false; Please be consistent in how you name your static variables. Either stick to the s prefix or the g prefix. :-) (The s prefix is probably better.) ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:38 (Diff revision 2) > +nsRFPService* > +nsRFPService::GetOrCreate() > +{ > + if (!sInitialized) { > + gRFPService = new nsRFPService(); > + gRFPService->Init(); Missing error handling. ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:51 (Diff revision 2) > +nsresult > +nsRFPService::Init() > +{ > + nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID); > + > + prefs->AddObserver(RESIST_FINGERPRINTING_PREF, this, false); You need to remove this observer somewhere, otherwise you're leaking, right? We typically handle xpcom-shutdown and RemoveObserver() there... ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:60 (Diff revision 2) > + if (!tzValue) { > + mInitialTZValue = nsCString(tzValue); > + } > + > + UpdatePrefs(NS_LITERAL_CSTRING(RESIST_FINGERPRINTING_PREF)); > + return NS_OK; Please return the error codes from things like do_GetService and AddObserver.
Attachment #8855734 - Flags: review?(ehsan) → review-
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review139320 Thanks a lot, looks great! ::: dom/base/nsContentUtils.cpp:655 (Diff revision 3) > Preferences::AddBoolVarCache(&sRequestIdleCallbackEnabled, > "dom.requestIdleCallback.enabled", false); > > Element::InitCCCallbacks(); > > + nsRFPService::GetOrCreate(); Nit: Since this returns a value, can you please use Unused << to ignore it explicitly?
Attachment #8855734 - Flags: review?(ehsan) → review+
Hi, Arthur. I believe that you haven't looked the patch of part 1 yet since you had granted the review of Part 1 before. However, this is a new version of Part 1. Would you mind to review this again?
Flags: needinfo?(arthuredelstein)
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review139708 ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:75 (Diff revision 4) > + rv = prefs->AddObserver(RESIST_FINGERPRINTING_PREF, this, false); > + NS_ENSURE_SUCCESS(rv, rv); > + > + // We backup the original TZ value here. > + const char* tzValue = PR_GetEnv("TZ"); > + if (!tzValue) { Should this be `if (tzValue)`? ::: toolkit/components/resistfingerprinting/nsRFPService.cpp:84 (Diff revision 4) > + UpdatePrefs(NS_LITERAL_CSTRING(RESIST_FINGERPRINTING_PREF)); > + return rv; > +} > + > +void > +nsRFPService::UpdatePrefs(const nsACString& aPrefName) Should this be called UpdatePref instead of UpdatePrefs? And it's not clear to me (at least at this stage) why you need to pass the pref name at all. Maybe documenting this function at the declaration will help to clarify.
Flags: needinfo?(arthuredelstein)
Comment on attachment 8855734 [details] Bug 1330890 - Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). https://reviewboard.mozilla.org/r/127644/#review139708 > Should this be `if (tzValue)`? Oops, you are right. I will fix this. > Should this be called UpdatePref instead of UpdatePrefs? And it's not clear to me (at least at this stage) why you need to pass the pref name at all. Maybe documenting this function at the declaration will help to clarify. This is a good point. I will take your suggestion to change the name into UpdatePref(). At this point, we only observe one pref, so UpdatePref() is a better name to fit its meaning. The reason why I pass the pref name is to check that it is the correct pref that we want to observe. But, it could be better to put this check at Observe().
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0fedf8c86ceb Part 1: Spoofing the time zone as UTC when fingerprinting resistance is enabled (adopt from Tor #16622). r=arthuredelstein,Ehsan https://hg.mozilla.org/integration/autoland/rev/80144f502fb6 Part 2: Add a test case for using UTC timezone when 'privacy.resistfingerprinting' is true. r=arthuredelstein,Ehsan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0fedf8c86ceb https://hg.mozilla.org/mozilla-central/rev/80144f502fb6
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1377744
Depends on: 1382840
Depends on: 1385597
new Date().toLocaleString() still leaks the original timezone. Tested in latest nightly (2017-09-05) https://browserleaks.com/javascript
Flags: needinfo?(tihuang)
(In reply to Anna from comment #29) > new Date().toLocaleString() still leaks the original timezone. Tested in > latest nightly (2017-09-05) > > https://browserleaks.com/javascript Thanks for reporting this! Tim, could you check the issue?
Sure no problem. Thanks for working on this! :)
I have tried my Nightly (57.0a1 (2017-09-05)). It seems that timezone is correctly spoofed after I flipped 'privacy.resistFingerprinting' to true. Anna, would you mind to provide your STR?
Flags: needinfo?(tihuang) → needinfo?(earthlng)
Tested on Windows, sorry I forgot to mention that. Using a new untouched profile, toLocaleString() leaks the real timezone. When I change my system's timezone, toLocaleString() changes accordingly. Date() and toLocaleFormat() spoof correctly to UTC however.
Flags: needinfo?(earthlng)
Maybe this caching stuff is the problem: https://dxr.mozilla.org/mozilla-central/source/js/src/builtin/Date.js#28 "for Date's toLocaleString operation"
STR: use Windows and set your timezone to Sydney (UTC+10) for example (= far enough from UTC+0 to easily spot the difference). IDK if this is a Windows-only problem or if Linux and/or MacOS are also affected.
Flags: needinfo?(tihuang)
Some results > Windows FF55.0.3 > System Time Fri Sep 08 2017 20:10:41 GMT+0000 (UTC) > toLocaleString 9/9/2017, 8:10:41 AM > toLocaleFormat Friday, September 08, 2017 20:10:41 ^^ gives me away as UTC+12 > Windows FF56.0b10 > Fri Sep 08 2017 20:13:20 GMT+0000 (UTC) > toLocaleString 9/9/2017, 8:13:20 AM > toLocaleFormat Friday, 8 September 2017 8:13:20 p.m. ^^ ditto more here: https://github.com/ghacksuserjs/ghacks-user.js/issues/230 - does not seem to be an issue with Linux or Mac > Linux FF55.0.3 > System Time Thu Sep 07 2017 15:51:39 GMT+0000 (UTC) > toLocaleString 9/7/2017, 3:51:39 PM > toLocaleFormat Thu Sep 7 15:51:39 2017 > Mac 56.0b9 > System Time | Fri Sep 08 2017 10:43:51 GMT+0000 (UTC) > toLocaleString | 9/8/2017, 10:43:51 AM > toLocaleFormat | Fri Sep 8 10:43:51 2017
Note: toLocaleFormat: - Friday, September 08, 2017 20:10:41 (FF55) is my main FF with (a lot of) pref changes - Friday, 8 September 2017 8:13:20 p.m (FF56) was a default new profile Is there is ticket in for standardizing locale date/time formats with resistFingerpinting?
(In reply to Simon Mainey from comment #37) > Note: toLocaleFormat: > - Friday, September 08, 2017 20:10:41 (FF55) is my main FF with (a lot of) > pref changes > - Friday, 8 September 2017 8:13:20 p.m (FF56) was a default new profile > > Is there is ticket in for standardizing locale date/time formats with > resistFingerpinting? AFAIK, there is no bug opened for this.
Flags: needinfo?(tihuang)
toLocaleString: from the above I cannot tell the format this is US format M/D or European M/D (which my machine is). It should be consistent with en-US and use M/D. The format should be identical eg MM/DD/YYYY HH:MM:SS toLocaleFormat: can leak both - locale/language (eg spelling Freitag for Friday => German etc for days and months) - format which creates bits for entropy (12hr vs 24hr, September vs Sep, order etc - see examples above and at the github link - https://github.com/ghacksuserjs/ghacks-user.js/issues/230 ) What other calls/JS can be used to reveal these (I am not a coder!)? Is currency a part of Firefox, or about to be (IDK!)? There is a hidden pref "javascript.use_us_english_locale" => true ( see Bug 867501 ) which should fix the language differences (I do not have a non-English machine to test), but does not fix the formatting differences between users There is also "intl.locale.matchOS" which should be default false in order to use the app locale. There are other language settings (intl.accept_languages, general.useragent.locale etc) which I think are already covered Please advise if you would like me to create a ticket for this and what to entitle it- thanks
(In reply to Simon Mainey from comment #39) Wouldn't it make more sense to resist fingerprinting by using international standard values? (eg. UTC timezone, ISO 8601 date format, etc.) It doesn't seem right to de facto standardize on strange, archaic things like middle-endian date formats.
Nightly 57 (2017-09-05) console message: > Date.prototype.toLocaleFormat is deprecated; consider using Intl.DateTimeFormat instead Intl.DateTimeFormat leaks the time + timezone as well, at least on Windows.
(In reply to Stephan Sokolow from comment #40) > (In reply to Simon Mainey from comment #39) > > Wouldn't it make more sense to resist fingerprinting by using international > standard values? > > (eg. UTC timezone, ISO 8601 date format, etc.) > > It doesn't seem right to de facto standardize on strange, archaic things > like middle-endian date formats. Good point. As long as everyone is enforced to the same, I don't think it really matters but I would guess that the majority of FF users use international standard. I was just looking at the many "en-US" language settings for consistency
Upstream: Bug 1409973
Blocks: 1409973
This bug was not fixed properly. I'm reopening it until bug 1409973 is fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This issue can be seen using this website: https://browserleaks.com/javascript Look at the reported date in the "Date/Time" section.
Per offline discussion with Tom Ritter we don't necessarily block 58 for this, though this will be an important one to do (with bug 1409973) after finishing other Milestone 3's.
This bug should be closed once bug 1409973 is fixed.
Whiteboard: [fingerprinting][tor 16622][fp:m1] → [fingerprinting][tor 16622][fp:m1][fp-triaged]
No longer blocks: 1409973
Depends on: 1409973
Assignee: artines1 → tom
(In reply to Ethan Tseng [:ethan] from comment #47) > This bug should be closed once bug 1409973 is fixed. Bug 1409973 landed
Whoo!
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED

Hi,

this bug is still present in firefox 67 - enabling the privacy resisting feature sets timezone unconditionally to UTC.

Regards,
Daniel

(In reply to Daniel Baumann from comment #50)

Hi,

this bug is still present in firefox 67 - enabling the privacy resisting feature sets timezone unconditionally to UTC.

That's the intended behavior presently, unless you mean Bug 1377744 which would improve the situation...

This is about the only feature in resistFingerprinting that actively results to non-obviously falsified data shown on websites, in contrast to all other sub-festures that merely degrade functionality or only falsify informational data like browser version.

I understand it to be necessary for torbrowser, but I'd like to use this mode also normally, and this might actively cause harm, as the timezone is not usually shown on the website, so there's no way for a user to distinguish which is a local time from the server and which is UTC time.
In non-tor/Proxy use, timezone encodes the timezone location, which can be also inferred by the IP, so no real loss of privacy.

Therefore it would be nice to have some override possibility like for Accept-Language, like in:
"If you customize the preferred language list (Accept-Language), you will be warned bug 1039069"

Blocks: 1377744
No longer depends on: 1377744
Blocks: 1709867

Is it possible to get this functionality WITHOUT privacy.resistfingerprinting?

That is, I'd like to try leaving privacy.resistfingerprinting off, but still want Firefox to use UTC as the timezone and present this to websites.

(In reply to Hugo Osvaldo Barrera from comment #53)

Is it possible to get this functionality WITHOUT privacy.resistfingerprinting?

That is, I'd like to try leaving privacy.resistfingerprinting off, but still want Firefox to use UTC as the timezone and present this to websites.

This will depend on OS I think; but if you launch Firefox with the environment variable TZ=UTC - that might do it. Caveat emptor - ths is totally unsupported and might break at any time.

No longer blocks: 1377744
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: