Opened 3 years ago

Closed 7 weeks ago

Last modified 4 days ago

#21549 closed task (fixed)

Investigate wasm for linkability/fingerprintability/disk avoidance issues

Reported by: gk Owned by: tbb-team
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff68-esr, tbb-9.0, TorBrowserTeam201910R, tbb-backport, BugSmashFund
Cc: dmr, mcs, brade, legind Actual Points: 1.5
Parent ID: Points: 1
Reviewer: Sponsor:

Description (last modified by dmr)

In order to avoid the asm.js disaster we should investigate whether wasm complies with our design requirements. It got enabled in Firefox 52 but re-disabled in ESR 52.

Child Tickets

Change History (38)

comment:1 Changed 3 years ago by cypherpunks

Keywords: ff52-esr removed

status-firefox-esr52: disabled
https://bugzilla.mozilla.org/show_bug.cgi?id=1342060#c23
But, please, don't postpone such tasks to the next esr.

comment:2 Changed 3 years ago by gk

Keywords: ff59-esr added
Sponsor: Sponsor4

If we get earlier to it fine with me. But that needs to get addressed latest with the switch to esr59. Addjusting the key words and sponsor field.

comment:3 Changed 23 months ago by gk

Keywords: ff60-esr added; ff59-esr removed

Firefox 60 is the new ESR.

comment:4 Changed 17 months ago by gk

Keywords: TorBrowserTeam201807 added
Priority: MediumHigh

comment:5 Changed 17 months ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201807 removed

Move our tickets to August.

comment:6 Changed 16 months ago by gk

Priority: HighVery High

comment:7 Changed 16 months ago by gk

I think we can disable WASM for 8.0 until we are sure we are good here. If we happen to do the analysis and proper patches earlier, great.

comment:8 Changed 16 months ago by dmr

Cc: dmr added

comment:9 in reply to:  1 Changed 16 months ago by dmr

Description: modified (diff)

Replying to cypherpunks:

status-firefox-esr52: disabled
https://bugzilla.mozilla.org/show_bug.cgi?id=1342060#c23
But, please, don't postpone such tasks to the next esr.

Indeed disabled in esr52.
pref javascript.options.wasm

More details (beyond the comment):
https://bugzilla.mozilla.org/show_bug.cgi?id=1342440
https://hg.mozilla.org/releases/mozilla-esr52/rev/e07850f191a0

Changing the description so that others don't need to read comments to know about this.

comment:10 Changed 16 months ago by gk

Cc: mcs brade added
Keywords: TorBrowserTeam201808R added; TorBrowserTeam201808 removed
Status: newneeds_review

Let's disable it for now until we are confident we can enable it and let it govern by the security slider. See: bug_21549 (https://gitweb.torproject.org/user/gk/tor-browser.git/commit/?h=bug_21549&id=19ad2e4d07a06e529920f082eb7d5fdd87e380d4) in my public tor-browser repository.

comment:11 Changed 16 months ago by mcs

r=mcs, r=brade
Looks good to us.

comment:12 Changed 16 months ago by gk

Keywords: TorBrowserTeam201808 added; TorBrowserTeam201808R removed
Status: needs_reviewnew

Thanks. Cherry-picked to tor-browser-60.1.0esr-8.0-1 (commit 3c6862f6cc8a7dc59b9eba41638100638ecb33b0). Setting back to new for the investigation part.

comment:13 Changed 16 months ago by reportUrl

Is this enough after tiering was enabled?
https://bugzilla.mozilla.org/show_bug.cgi?id=1277562

comment:14 Changed 15 months ago by gk

Keywords: TorBrowserTeam201809 added; TorBrowserTeam201808 removed

Moving our tickets to September 2018

comment:15 Changed 12 months ago by legind

Cc: legind added

comment:16 Changed 12 months ago by legind

Relevant to HTTPS Everywhere, which would like to rewrite portions of the code in WASM for better performance: https://github.com/EFForg/https-everywhere/issues/17143.

comment:17 in reply to:  16 Changed 12 months ago by gk

Replying to legind:

Relevant to HTTPS Everywhere, which would like to rewrite portions of the code in WASM for better performance: https://github.com/EFForg/https-everywhere/issues/17143.

I think we could bypass the investigation for HTTPS Everywhere (and extensions in general) at least by assuming it belongs to privileged code (which it does) and that the linkability etc. concerns only applies to content (which I think is not unreasonable). The idea then could be to implement a switch to being able to disable WASM and similar things (ASM.js comes to mind) for content only while leaving chrome like code untouched.

Or maybe the chrome/content separation already exists and WebExtensions are just not properly added to the chrome bucket?

comment:18 Changed 12 months ago by legind

@gk I'm concerned that extensions are a single-click install in most cases, and privileging them in general will open identifiable characteristics to possibly irresponsible third parties. Can we whitelist WASM by extension ID?

comment:19 in reply to:  18 Changed 12 months ago by gk

Replying to legind:

@gk I'm concerned that extensions are a single-click install in most cases, and privileging them in general will open identifiable characteristics to possibly irresponsible third parties. Can we whitelist WASM by extension ID?

Maybe. However, the current policy is that users are responsible themselves for possible fallout if they are installing other extensions into their Tor Browser. This is not recommended for all sorts of reasons.

Webextensions got already "privileged" by allowing JavaScript to run in general if it is disabled in the browser (you might remember https://bugzilla.mozilla.org/show_bug.cgi?id=1329731). I think it is more straightforward to follow the reasoning dveditz outlined in https://bugzilla.mozilla.org/show_bug.cgi?id=1329731#c7 arguing that disabling WASM for extensions (while allowing it for the remaining parts of the privileged browser) is not the right solution.

FWIW: that Mozilla bug might be a good start for investigating how to enable WASM for extensions only but not content (by checking the principal accordingly).

comment:20 Changed 12 months ago by gk

Oh, and one additional point: we might want to have some WebExtensions exceptions on the higher levels of the security slider for things like JIT (see: #23719) which is kind of related.

comment:21 Changed 6 months ago by legind

Update: in https://github.com/EFForg/https-everywhere/pull/18093, HTTPS Everywhere includes WASM optimizations. So Tor Browser would see an OOTB performance increase and memory overhead decrease by enabling WASM in extension-land.

comment:22 Changed 6 months ago by gk

Keywords: ff68-esr TorBrowserTeam201907 added; ff60-esr TorBrowserTeam201809 removed

We have #23719 for the slider related things and this bug for the general enabling. Talking to Luke I think we are in good shape (surprise!) and nothing stood out. One thing that remains to test is whether New Identity is actually blowing away WASM related cache in case someone used Tor Browser outside of Private Browsing Mode (there are no WASM cache entries written to disk in that mode).

comment:23 Changed 4 months ago by pili

Sponsor: Sponsor44-can

Adding Sponsor 44 to ESR68 tickets

comment:24 Changed 4 months ago by gk

Keywords: TorBrowserTeam201908 added; TorBrowserTeam201907 removed

Moving more tickets to August

comment:25 Changed 3 months ago by gk

Keywords: TorBrowserTeam201909 added; TorBrowserTeam201908 removed

Moving tickets to September

comment:26 Changed 3 months ago by pili

Points: 1

comment:27 Changed 2 months ago by gk

Keywords: tbb-9.0 added

comment:28 Changed 2 months ago by pili

Keywords: TorBrowserTeam201910 added

comment:29 Changed 2 months ago by pili

Keywords: TorBrowserTeam201909 removed

comment:30 Changed 2 months ago by sysrqb

Okay, I started from the beginning, and then worked backwards :)

This was tedious because most of the initial patches were simply refactoring asmjs compiler code such that the code could be shared with wasm. Reading these patches was helpful except the code has significantly changed since four years ago (and wasm has mostly replaced asmjs in the codebase now). As an example, the compiler cached asmjs code until earlier this year. I'm taking a different approach now, but I'll post this anyway. The next comments will be based on the current implementation.

https://bugzilla.mozilla.org/show_bug.cgi?id=1188259

  • Meta ticket of implementing wasm

https://bugzilla.mozilla.org/show_bug.cgi?id=1224389

  • Odin: refactor in preparation for WebAssembly decoder Commits:
    • f7c5ebaa664b0dc07836fe06d3609e239ba7e71a - okay
      • Mostly refactoring
    • 4ef42db6e3cad192ccefaacc823aab012a57c6e5 - okay
      • Mostly code simplification
    • 28f2c8f11d211e30e1f0fb77902c9ba4d2a7d1c5 -

https://bugzilla.mozilla.org/show_bug.cgi?id=1232205

  • wasm:Baseline JIT Commits:
    • 9e9fafcd38f7e3ec3db1d4508bf4230a2d4b3678 - okay
    • 8938a215ec16a20f9fa4f4156d9bd611e8fa8562 - okay
    • 5f37b8277391842f5e56c211c3ca4acd86237d72 - okay
    • 8ec23c07dd09da1e46bf17f05ecad83f8aa12e4e - okay
    • 9af7246a474bb824f39562b68e5f7fae0921a8d9 - okay
      • All refactoring and JIT impl, no caching
    • 136c1d78af1bf603c4bcd2fc2488d3c3f9661569 - okay
      • Adds mechanism for running baseline wasm tests
    • 65b0869d9e684d486c1ac4cf90c60bf3dfc22c82 - okay
      • Adds test directive
    • 2a5c7f48c92c2163b9f69ebc9f287c7ae8db7c7c -
      • Baseline JIT impl.
      • Pushes code onto masm stack? Is this masm stack isolated?
      • ffiCall? WasmTableCallPtrReg?
    • bd9aa07076b543051bcfbd8937c32a9ad2bec883 - okay
      • Add pref and jsshell runtime arg

comment:31 Changed 2 months ago by sysrqb

The WebAssembly API consists of:

These are exported and implemented:
WebAssembly.compile()
WebAssembly.compileStreaming()
WebAssembly.instantiate()
WebAssembly.instantiateStreaming()
WebAssembly.validate()
WebAssembly.Global
WebAssembly.Memory
WebAssembly.Module
WebAssembly.Instance
WebAssembly.Table

  • WebAssembly.compile is for ahead-of-time compiling a WASM module. It takes in a buffer of WASM and returns a Promise of a WebAssembly.Module.
  • WebAssembly.compileStreaming is similar to compile() except it takes in a Response object or Promise for a wasm module (compared with a buffer containing the module).
  • WebAssembly.instantiate has two prototypes.
    • The first takes in a buffer of WASM and returns a Promise of a tuple: (WebAssembly.Module, WebAssembly.Instance). The Module is the compiled WASM, and the Instance is an instantiation of that Module.
    • The second takes in an existing WebAssembly.Module (already compiled WASM) and returns an Instance.
  • WebAssembly.instantiateStreaming takes in a a Response object or Promise (like compileStreaming) and returns a Promise of a tuple: (WebAssembly.Module, WebAssembly.Instance) like instantiate().
  • WebAssembly.validate() takes in a buffer of WASM binary code and returns a boolean true if the buffer contained valid wasm, and false otherwise.
  • WebAssembly.Global is an object containing a WASM "global" variable. Multiple Global instances may be passed into WebAssembly.instantiate(), but these globals are only accessible by WASM if they are explicitly "imported" by the WASM Module.
  • WebAssembly.Instance is an instance of a Module initialized with Global and/or Memory instances.
  • WebAssembly.Memory().buffer is an ArrayBuffer representing the "heap" memory available in the WASM module. This is readable and writable by both JavaScript and WASM. From WASM, pointers are represented by an index of the Memory ArrayBuffer.
  • WebAssembly.Table is an "array-like" structure storing function-references. In the future, Tables may store more than only function-references. Tables allow calling arbitrary functions from WASM (after the WASM module assigns a function-reference at an index in the table). It seems like this provides a level of abstraction compared with exporting a function from WASM into JavaScript, because function-references can be added/deleted/modified in the Table at run-time.

comment:32 Changed 2 months ago by sysrqb

I don't see any significant fingerprinting or linkability issues in the wasm implementation. I don't see any caching across principals. There is a minor fingerprinting risk due to timing leaks (how quickly the wasm byte code is returned as a Module), and how quickly a wasm function is executed. There is the additional risk of malicious byte code exploiting a compiler bug. However, these seem no worse than the existing javascript attack surface - wasm simply adds more options.

comment:33 Changed 7 weeks ago by sysrqb

Keywords: TorBrowserTeam201910R added; TorBrowserTeam201910 removed
Status: newneeds_review

I have two branches - one for tor-browser (deleting the override) and the other for torbutton (adding as a pref controlled by the security slider).

Both branches are named bug21549_00.

https://github.com/sysrqb/torbutton/

comment:34 in reply to:  33 Changed 7 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

Replying to sysrqb:

I have two branches - one for tor-browser (deleting the override) and the other for torbutton (adding as a pref controlled by the security slider).

Both branches are named bug21549_00.

https://github.com/sysrqb/torbutton/

Looks good. Merged to master (Torbutton) as commit 324e8fd7f73c7f97f8174713c6a112ec75669e56 and tor-browser-68.2.0-9.5-1 (tor-browser) as commit 80134f6a3d2026ff95943e806cb424bf9eff2c2f.

Nice review work!

comment:35 Changed 7 weeks ago by gk

Keywords: tbb-backport added

comment:36 Changed 5 weeks ago by sysrqb

Actual Points: 1.5

comment:37 Changed 4 days ago by pili

Keywords: BugSmashFund added

BugSmashFund can be used for the ESR work done so far

comment:38 Changed 4 days ago by pili

Sponsor: Sponsor44-can

Sponsor 44 only covered PM and Team Lead work

Note: See TracTickets for help on using tickets.