Opened 22 months ago

Closed 22 months ago

Last modified 4 weeks ago

#18995 closed task (fixed)

Investigate CacheStorage feature for tracking usage in Tor Browser

Reported by: gk Owned by: arthuredelstein
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Major Keywords: ff45-esr, TorBrowserTeam201605R
Cc: mcs, brade, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

In ESR45 we have a new CacheStorage feature that might be usable for tracking users. We should bind it to our cache isolation code if so. Even though being part of the ServiceWorker spec it is not bound to it. Quoting from comment:8:ticket:18545

The API page includes "It provides a master directory of all the named caches that a ServiceWorker, other type of worker or window scope can access (you don't have to use it with service workers, even though that is the spec that defines it) and maintains a mapping of string names to corresponding Cache objects." Also, some of the top-level objects are present in regular DOM windows. See: ​https://lists.torproject.org/pipermail/tbb-dev/2016-May/000372.html

Child Tickets

Change History (11)

comment:1 Changed 22 months ago by gk

Seems the worker case is no issue for us:

https://mxr.mozilla.org/mozilla-esr45/source/dom/cache/CacheStorage.cpp#200

Given https://bugzilla.mozilla.org/show_bug.cgi?id=1145744 and us disabling 3rd party cookies I wonder whether there is still stuff left we need to tackle.

comment:2 Changed 22 months ago by arthuredelstein

Cc: arthuredelstein added

comment:3 in reply to:  1 Changed 22 months ago by arthuredelstein

Owner: changed from tbb-team to arthuredelstein
Status: newassigned

Replying to gk:

Seems the worker case is no issue for us:

https://mxr.mozilla.org/mozilla-esr45/source/dom/cache/CacheStorage.cpp#200

CacheStorage is also "blocked" in the non-worker case. Here's the relevant line:
https://mxr.mozilla.org/mozilla-esr45/source/dom/base/nsGlobalWindow.cpp#10244

I manually confirmed that caches.open("test") reports a SecurityError in our TBB/ESR45 alpha. I will work on writing a mochitest that ensures that the CacheStorage is inactive in both workers and main-thread content.

comment:4 Changed 22 months ago by arthuredelstein

Here's a regression test that is supposed to check whether CacheStorage is active in private browsing mode:

https://github.com/arthuredelstein/tor-browser/commit/18995
Hash cdb08d1a81ee5ffb7e9308942ea67035be864f2b

Last edited 22 months ago by arthuredelstein (previous) (diff)

comment:5 Changed 22 months ago by arthuredelstein

Keywords: TorBrowserTeam201605R added; TorBrowserTeam201605 removed
Status: assignedneeds_review

comment:6 Changed 22 months ago by gk

Looks good to me. Could we get rid of the commented SimpleTest.js inclusion in the .html files? I think the "#" is not necessary before the bug number there either. "Bug 18995" is perfectly fine. :)

comment:7 in reply to:  6 Changed 22 months ago by arthuredelstein

Replying to gk:

Looks good to me. Could we get rid of the commented SimpleTest.js inclusion in the .html files? I think the "#" is not necessary before the bug number there either. "Bug 18995" is perfectly fine. :)

Thanks for the review. Here's the new version:
https://github.com/arthuredelstein/tor-browser/commit/18995+1
Hash cea5f1efcd9590885127a0763432143aca01524c

comment:8 Changed 22 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Thanks! Applied to tor-browser-45.1.0esr-6.0-1 (commit cea5f1efcd9590885127a0763432143aca01524c)..

comment:9 Changed 4 weeks ago by arthuredelstein

Keywords: tbb-dont-uplift added

comment:10 Changed 4 weeks ago by arthuredelstein

Keywords: tbb-no-uplift added; tbb-dont-uplift removed

comment:11 Changed 4 weeks ago by arthuredelstein

Keywords: tbb-no-uplift removed
Note: See TracTickets for help on using tickets.