1891690 - EXSLT leaks timezones also when RFP is enabled

Status: RESOLVED FIXED

(Core :: XSLT, defect)

Description

[Pier Angelo Vendrame]

Attachment: Example on how to get the XSLT datetime with JS

SLT has a function to write the current date and time (date:date-time()) in the local timezone.
However, when RFP is enabled, it should report it in UTC.
I'm attaching a PoC on how to get its value from JS.

Downstream we adopted an easy patch for Tor Browser, but we had to use the coarse check on ShouldResistFingerprinting because the XSLT function is evaluated with a txEarlyEvalContext, which doesn't allow to access the document.
I'm attaching it here as a patch file, since I'd expect it not to be accepted, since it uses the coarse check.

Comment 1

[Pier Angelo Vendrame]

Attachment: Tor Browser's patch

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

S4 because of not an issue for Firefox in the default settings.

Comment 3

[Thorin [:thorin]]

Attachment: xslt.png

this leak undermines a lot of current RFP timezone protection. Whilst timezone name is still protected (350+ timezone names exist), at any given time there are 40-50 (I think, varies depending on the date) current timezone offsets in effect, which is still high entropy

I appreciate this is not front facing or an official FF setting, but we rely on FF users to help develop, test and refine RFP (and an uplift will reduce patches downstream). Thanks :)

Comment 4

[Pier Angelo Vendrame]

If the patch with the coarse RFP check is fine I'd be happy to open a Phab request.
Otherwise, with some mentoring I think I could try to expose the RFP check to EXSLT parsing.

Comment 5

[Tom Ritter [:tjr] (OOTO until April)]

Does aContext->getContextNode()->Document()->ShouldResistFingerprinting(...) work?

Feel free to leave this until you get a 128 rebase sorted out if that's easier workflow-wise for you.

Comment 6

[Pier Angelo Vendrame]

I think I had already tried that way, but Document() is private. Only ==, != and the destructor are public in txXPathNode.

Comment 7

[Fatih Kilic [:fkilic]]

Attachment: Bug 1891690: Return GMT when RFPTarget::JSDateTimeUTC is enabled. r?timhuang

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c20348dfb89d
Return GMT when RFPTarget::JSDateTimeUTC is enabled. r=timhuang

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/c20348dfb89d

Comment 10

[Pier Angelo Vendrame]

Would it be possible to uplift this to ESR 128?

Comment 11

[Fatih Kilic [:fkilic]]

Sure, we haven't caught any issues in XLST tests and the test I wrote, but I'm little hesitant to directly uplifting it. Is it okay if we uplift it after a while to see if we get any crashes in the wild?

Comment 12

[Pier Angelo Vendrame]

Sure, works for me, thank you very much!
I asked because downstream we have another problem around the same code, so I was thinking of replacing my downstream patch with this one anyway, and then fix the other problem on top of it.
Basically, the precision of XLST is still very high when using RFP.
So, in addition to checking JSDateTimeUTC, we would like to add a second check on ReduceTimerPrecision and act accordingly (or we will wait for the patch to be added to Firefox, if you manage to get to this problem before 🙂, I don't think I'll be able to get to it for a few days).

Comment 13

[Fatih Kilic [:fkilic]]

(In reply to Pier Angelo Vendrame from comment #12)

Sure, works for me, thank you very much!
I asked because downstream we have another problem around the same code, so I was thinking of replacing my downstream patch with this one anyway, and then fix the other problem on top of it.
Basically, the precision of XLST is still very high when using RFP.
So, in addition to checking JSDateTimeUTC, we would like to add a second check on ReduceTimerPrecision and act accordingly (or we will wait for the patch to be added to Firefox, if you manage to get to this problem before 🙂, I don't think I'll be able to get to it for a few days).

Done!

Comment 14

[Thorin [:thorin]]

verified fixed in nightly131 - https://arkenfox.github.io/TZP/tzp.html#region

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Does this need an ESR128 approval request?

Comment 16

[Fatih Kilic [:fkilic]]

I mean I was going to ask for it but there's this bug 1916109. I wanted to wait for its fix before asking but I don't know honestly. It only happened once, and it can only happen during testing, so it is up to depending on the risks associated. I'll update if someone reviews it.

Comment 17

[Fatih Kilic [:fkilic]]

I pushed the fix for the intermittent issue. So I guess to uplift Bug 1912129, we first have apply patches in the order of Bug 1891690 then Bug 1916109, then Bug 1912129. If this is something I can do, I can do it.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

If you want to do so, that works for me.

Comment 19

[Fatih Kilic [:fkilic]]

and how would I do it :/ I can think of two options,

• Modifying D220905 to stack all three patches into a single patch
• Creating uplift requests for this bug and Bug 1916109
• or maybe some other option?

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Just nominate each bug individually, yeah.

Comment 21

[Fatih Kilic [:fkilic]]

Attachment: Bug 1891690: Return GMT when RFPTarget::JSDateTimeUTC is enabled. r?timhuang

Original Revision: https://phabricator.services.mozilla.com/D216411

Comment 22

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: This patch improves the fingerprinting prevention/privacy of the users. We want to uplift this patch so that the privacy targeted browsers (e.g. Tor, Mullvad) can receive the updates. No big impact if declined.
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: Enable privacy.resistFingerprinting, visit https://arkenfox.github.io/TZP/tzp.html#region, verify that timezone is 0
• Risk associated with taking this patch: There has been a data race issue but it is addressed in https://phabricator.services.mozilla.com/D221686, so it should land after this patch.
• Explanation of risk level: Data race failures on CI if landed without https://phabricator.services.mozilla.com/D221686
• String changes made/needed: No
• Is Android affected?: yes

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/6529a7bf108f

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Backed out for browser_exslt_timezone_load.js failures.
https://treeherder.mozilla.org/logviewer?job_id=473695861&repo=mozilla-esr128&lineNumber=17834

https://hg.mozilla.org/releases/mozilla-esr128/rev/53e6211dad4ced2a2b88c0d3453d8f8ca52d2c55

Comment 25

[Fatih Kilic [:fkilic]]

Very interesting. I have no idea. My personal ESR build succeeded (though I ran it with privacy.resistFingerprinting and not privacy.fingerprintingProtection + overrides) also we aren't getting any fails on other repos. I wonder if there's something wrong with the overrides in ESR. I'll investigate. I'm sorry for the disturbance.

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/ab97e704a3f9