Opened 15 months ago

Last modified 8 months ago

#19417 needs_revision defect

asm.js files should not be cached to disk in Tor Browser and no linkability risk

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: tbb-disk-leak, tbb-linkability, GeorgKoppen201609, TorBrowserTeam201609, ff52-esr
Cc: mcs, brade, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by gk)

#19400 revealed that asm.js files are cached to disk which violates at least our no-disk-leaks requirement. The upstream bug is https://bugzilla.mozilla.org/show_bug.cgi?id=1047105.

Child Tickets

Change History (29)

comment:1 Changed 15 months ago by gk

Keywords: tbb-disk-leak added; tbb-disk-leaks removed

comment:2 Changed 15 months ago by bugzilla

It's even worse than just tbb-disk-leak because it's not cleared by New Identity like #18589 does. (tbb-newnym?)

comment:3 Changed 15 months ago by gk

Keywords: tbb-newnym added

Yes, good point.

comment:4 Changed 15 months ago by bugzilla

Also another one bug might be filed to BMO, because FF's Clear All History doesn't clear this cache (even when Cache option is selected).

comment:5 Changed 15 months ago by gk

Keywords: GeorgKoppen201606 TorBrowserTeam201606R added
Status: newneeds_review

bug_19417 (https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_19417) has a fix up for review that takes New Identity into account and blows the whole storage away during start-up (in case the user specified the no-disk option which is the default one).

comment:6 Changed 15 months ago by mcs

r=brade, r=mcs
Looks good. Should we also clear the asmjscache during shutdown?

comment:7 Changed 15 months ago by gk

Keywords: TorBrowserTeam201606 added; TorBrowserTeam201606R removed
Status: needs_reviewassigned

Thanks. Hm, I have no strong opinion here. I guess we could. On the other hand investing time to solve this ticket properly is my preferred way forward :)

comment:8 Changed 15 months ago by gk

Let's go with the patch in bug_19417 for now: commit ecc71020f6e3e6db5a2e8dc87af354cf2c86733b on maint-1.9.5 and e264e6027a78488f9813e3796d29f52df1a189ba on master.

comment:9 Changed 15 months ago by gk

Keywords: tbb-newnym removed

comment:10 Changed 15 months ago by cypherpunks

I propose a radical solution for these and all disk: create a ramdisk and put torbrowser folder into it.

comment:11 Changed 15 months ago by gk

Cc: arthuredelstein added
Description: modified (diff)
Keywords: tbb-linkability added
Summary: asm.js files should not be cached to disk in Tor Browserasm.js files should not be cached to disk in Tor Browser and no linkability risk

After thinking about it more it seems to me there is the additional risk that this mechanism could be used to embed supercookies. Like, deliver JS to a user that contains an identifier -> get that into the asmjscache -> once this is loaded anywhere ping the identifier back.

Looking at https://blog.mozilla.org/luke/2014/01/14/asm-js-aot-compilation-and-startup-performance/ does not rule that scenario out:

The cache entry is keyed on: the origin of the script, the source characters of the asm.js module, the type of CPU and its features, the Firefox build-id (which changes on every major or minor release).

Note this would be especially problematic for Tor Browser users as we are currently not changing the build-id.

Not sure what "the origin of the script" means but I doubt "URL bar domain". It could mean as well that the asmjs cache is not caring about starting SOP either.

Reading between the lines on that blog post it appears to me that there is indeed a way to disable this whole caching mechanism with: javascript.options.parallel_parsing set to false. It's worth investigating this closer I think.

comment:12 in reply to:  11 ; Changed 15 months ago by mcs

Replying to gk:

After thinking about it more it seems to me there is the additional risk that this mechanism could be used to embed supercookies. Like, deliver JS to a user that contains an identifier -> get that into the asmjscache -> once this is loaded anywhere ping the identifier back.

Looking at https://blog.mozilla.org/luke/2014/01/14/asm-js-aot-compilation-and-startup-performance/ does not rule that scenario out:

The cache entry is keyed on: the origin of the script, the source characters of the asm.js module, the type of CPU and its features, the Firefox build-id (which changes on every major or minor release).

Note this would be especially problematic for Tor Browser users as we are currently not changing the build-id.

Not sure what "the origin of the script" means but I doubt "URL bar domain". It could mean as well that the asmjs cache is not caring about starting SOP either.

They do use principals in the asmjscache code, so maybe there is some protection (not enough for us though).

Are you now thinking that we should unconditionally disable the asmjscache?

I spent a little time trying to figure out how to cleanly disable the cache when in private browsing mode (I was hoping progress would be reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1047105 but that has not happened so far).

For web workers, it might work to avoid calling JS::SetAsmJSCacheOps() inside dom/workers/RuntimeService.cpp when in private browsing mode.

For content windows, I think we could make changes inside dom/base/nsJSEnvironment.cpp to ensure that AsmJSCacheOpenEntryForRead() and AsmJSCacheOpenEntryForWrite() do nothing when operating inside a private browsing mode window.

comment:13 in reply to:  12 Changed 15 months ago by gk

Replying to mcs:

Replying to gk:

After thinking about it more it seems to me there is the additional risk that this mechanism could be used to embed supercookies. Like, deliver JS to a user that contains an identifier -> get that into the asmjscache -> once this is loaded anywhere ping the identifier back.

Looking at https://blog.mozilla.org/luke/2014/01/14/asm-js-aot-compilation-and-startup-performance/ does not rule that scenario out:

The cache entry is keyed on: the origin of the script, the source characters of the asm.js module, the type of CPU and its features, the Firefox build-id (which changes on every major or minor release).

Note this would be especially problematic for Tor Browser users as we are currently not changing the build-id.

Not sure what "the origin of the script" means but I doubt "URL bar domain". It could mean as well that the asmjs cache is not caring about starting SOP either.

They do use principals in the asmjscache code, so maybe there is some protection (not enough for us though).

Are you now thinking that we should unconditionally disable the asmjscache?

Well, doing so with javascript.options.parallel_parsing set to false does not work at least. :) What to do depends on how serious the issue is. If there are no linkability issues then making it just obey PBM is fine with me. If it can get used to track users across sites then we might want to disable it first and then enable it successively as soon as this is fixed.

I actually tried to find proper sites that could be loaded in an iframe AND would exhibit asmjscaching but I failed so far. Seeing what is happening on the user's disk in this case might actually be revealing. Who knows what "origin if the script" boils down to given that nobody was caring about user privacy much in this case anyway.

I spent a little time trying to figure out how to cleanly disable the cache when in private browsing mode (I was hoping progress would be reported in https://bugzilla.mozilla.org/show_bug.cgi?id=1047105 but that has not happened so far).

For web workers, it might work to avoid calling JS::SetAsmJSCacheOps() inside dom/workers/RuntimeService.cpp when in private browsing mode.

For content windows, I think we could make changes inside dom/base/nsJSEnvironment.cpp to ensure that AsmJSCacheOpenEntryForRead() and AsmJSCacheOpenEntryForWrite() do nothing when operating inside a private browsing mode window.

Seems to be worth a try to me.

comment:14 Changed 15 months ago by gk

Okay. Running the asm.js benchmark in https://people.torproject.org/~gk/misc/asmjscache_iframe.html generates a cache entry that is keyed to https+++kripken.github.io which happens as well if I run the test directly on the site.

comment:15 Changed 15 months ago by cypherpunks

I assume simply disabling asm.js (by setting javascript.options.asmjs to false) is not an option here?

comment:16 Changed 15 months ago by gk

Keywords: TorBrowserTeam201607 added; TorBrowserTeam201606 removed

comment:17 in reply to:  15 Changed 15 months ago by gk

Replying to cypherpunks:

I assume simply disabling asm.js (by setting javascript.options.asmjs to false) is not an option here?

It is one and I think we are going to do that as a stopgap solution.

comment:18 Changed 15 months ago by cypherpunks

Keywords: tbb-crash added

Note that this bug appears to be the root cause of #19657, triggering a crash/ASan.

comment:19 Changed 15 months ago by gk

Keywords: tbb-crash removed

Let's leave tbb-crash for #19400. This ticket is concerned with the cache and linkability issue.

comment:20 Changed 15 months ago by gk

Keywords: TorBrowserTeam201607R added; TorBrowserTeam201607 removed
Status: assignedneeds_review

Okay, bug_19417_v2 (https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_19417_v2) in my Torbutton repo and bug_19417 in my tor-browser repo (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_19417) are up for review.

comment:21 Changed 14 months ago by mcs

r=brade, r=mcs
Looks good.

comment:22 Changed 14 months ago by gk

Keywords: TorBrowserTeam201607 added; TorBrowserTeam201607R removed
Status: needs_reviewassigned

Applied to torbutton (master and maint-1.9.5) as commit 621916d0f79336b95d7390c2e30e2c81c0d2a504 and 48252096e210b78ab56a5623b296301929faea9f and to tor-borwser (tor-browser-45.2.0esr-6.5-1 and tor-browser-45.2.0esr-6.0-1) as commit 699ae74066ddc7000a3ea5f4ed68b170d1886065 and 9f49b80ab4a5c2326ca47f0736d0b865fa2272f9.

comment:23 in reply to:  19 Changed 14 months ago by bugzilla

Replying to gk:

Let's leave tbb-crash for #19400. This ticket is concerned with the cache and linkability issue.

Are you going to file another ticket for tbb-newnym issue?

(Hmm, from your changelog: Disable asm.js (but add code to clear on New Identity if enabled) - newnym fix advertised (disk only?))

Last edited 14 months ago by bugzilla (previous) (diff)

comment:24 Changed 13 months ago by gk

Keywords: GeorgKoppen201609 TorBrowserTeam201609 added; GeorgKoppen201606 TorBrowserTeam201607 removed

We should backport https://hg.mozilla.org/mozilla-central/rev/07979189c602 and let asmjs be governed by the slider again. I think we can keep the NEWNYM handling, though.

comment:25 Changed 13 months ago by gk

Keywords: TorBrowserTeam201609R added; TorBrowserTeam201609 removed
Status: assignedneeds_review

bug_19417_v2 in my tor-browser repo has the fixes we need to apply to the browser part (the backport of Mozilla's patch and not disabling asmjs anymore):

https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_19417_v2&id=ecdfc89579b6a403beda082c536a1d0b960363f5
https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_19417_v2&id=7af178793a20483896cd657fa06bb6ddc587b3a8

bug_19417_v4 in my torbutton repo puts the pref under the rule of the security slider again:

https://gitweb.torproject.org/user/gk/torbutton.git/commit/?h=bug_19417_v4&id=4953431fd7d14096a3f4217416a4578c96d40ec5

Please review (especially the patch backport).

comment:26 Changed 13 months ago by arthuredelstein

Looks good to me.

comment:27 Changed 13 months ago by mcs

The patches look OK, but do we have the GetPrivateBrowsingId() call in ESR 45?

comment:28 Changed 13 months ago by gk

Keywords: TorBrowserTeam201609 added; TorBrowserTeam201609R removed
Status: needs_reviewneeds_revision

Dang :/ Testing patches beforehand actually helps I guess.

comment:29 Changed 8 months ago by gk

Keywords: ff52-esr added

ESR 52 should have the patches for the caching. We should revisit this ticket when switching, especially as problems without asmjs show up (see #21298).

Note: See TracTickets for help on using tickets.