Closed Bug 1488583 Opened 6 years ago Closed 5 years ago

Remove unsupported, misleading "dom.indexedDB.enabled" preference

Categories

(Core :: Storage: IndexedDB, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox72 --- fixed

People

(Reporter: asuth, Assigned: asuth)

References

Details

(Whiteboard: DWS_NEXT )

Attachments

(1 file)

We have an undocumented preference "dom.indexedDB.enabled" that per bug 1079355 comment 2 existed so that we could easily disable IndexedDB in case of a massive security problem.  In bug 1079355 the preference was changed so that it only impacted Content in recognition of the use of IndexedDB under-the-hood by Firefox.

In practice, this preference has been used in the wild in an attempt to disable storage APIs.  (Which makes sense because the preference seems self-describing!)  Confusingly for users, the actual preference that controls all storage is the cookies one.

The good news is that the preferences UI has been improved in the past several years and as documented at https://support.mozilla.org/en-US/kb/enable-and-disable-cookies-website-preferences the "Cookies and Site Data" UI can entirely disable APIs like IndexedDB for sites by setting "Block cookies and site data".  Individual sites can then be granted permissions as well, as documented at https://support.mozilla.org/en-UlS/kb/enable-and-disable-cookies-website-preferences

To restate that for anyone end up at this bug wanting to disable IndexedDB: *Firefox treats all types of storage as equivalent*.  The existing "cookies and site data" permissions are sufficient to block use of IndexedDB APIs.  They also allow modes of operation where storage is closed at shutdown and allow manual clearing of storage. Please see the following support article:
https://support.mozilla.org/en-US/kb/enable-and-disable-cookies-website-preferences

In any event, because it's an unsupported pref that's insufficiently tested, holes have developed, resulting in the scenario described at https://bugzilla.mozilla.org/show_bug.cgi?id=1450160#c5 where we can end up crashing.  We've reached consensus that the pref and its checks should be removed since it's a misleading preference and we already have privacy mechanisms in place that comprehensively block storage (and have thorough tests!).


Note: We've had some other bugs related to this in the past and I'm duping them to this bug in the interest of having a single bug with a coherent explanation at the top of why we're doing away with the preference.  I may also set some dependencies, using this bug as a sort of meta bug.

Other note: Bug 1450160 is see also rather than blocked by this bug because more investigation is probably needed since the reporter in that case was using the preference to reproduce an asymmetry that led to a crash rather than explicitly using the preference for testing purposes.
>the preference seems self-describing
It mostly is. It says it disables IndexedDB, not something else. If users don't know what it is they should read MDN or something. If they assume the preference does something else based of nothing that's clearly their own fault. The actual unexpected behavior is that disabled API throws (bug 1192643).
>the "Cookies and Site Data" UI can entirely disable APIs like IndexedDB for sites
This is not useful for some cases. It's actually almost completely useless IMO. Blocking cookies and localStorage breaks many sites. So users need them while they browse such sites. Once they close them they usually don't need their storage(Sometimes they want for few seconds to restore accidentally closed tabs) except for a limited amount of sites such as where they are logged in. So all storage of not whitelisted sites should be cleared once last tab of this site closed. This is not possible with builtin features, so it has to be done with extensions. However extensions APIs are limited to cookies and localStorage (bug 1340511) This makes reasonable disabling indexedDB since this breaks much less sites than disabling cookies/localStorage (and it would break even less if disabled API will behave as it was never added so regular feature detection will work).
Firefox privacy features such as private windows and cleaning data on quit are inconvenient for this purpose because you'll have to manually open and close windows all the time. Otherwise sites will be able to track users by connecting closed earlier tabs to the newly opened ones.
It sounds like you probably want something built on top of Firefox's container mechanism.  While that does require using extensions, I think it gets you where you want to be in a (more) supported way.  Specifically, I suggest checking out https://addons.mozilla.org/en-US/firefox/addon/temporary-containers/ which I understand allows you to run tabs in their own containers (which guarantees separate storage) and dispose of the containers when you're done with them (which purges the storage from disk).
The problem with "temporary" containers is that they are actually permanent containers that are automatically created and deleted. This means that for every container there is an additional instance of some storage-manager with buffers which consumes additional memory (quick test shows about 50mb in main process for every 10 one-tab containers, that's significant for people with many tabs) and does unnecessary disk writes (more than with disabled iDB & deletion of cookies&localStorage from one container storage). It also suffers from webext API limitations, e.g. it can't prevent a tab opening in default container from link click and therefore has to close it and open a new one, lowering performance, causing problems with addons monitoring tab opening & positioning and ugly animation. So this more supported way seems to be less convenient, although generally more reliable. Maybe this can be improved by adding native temporary containers (something based on private browsing?), that can be made permanent manually. Thought APIs are limited on purpose so to fix problems related to reopening tabs builtin configuration of where to open tabs based on user actions needs to be added what probably not happening any time soon.
To clarify, you're right that the containers are permanent[1] unless otherwise enhanced, but there's no additional storage/memory usage beyond the inherent result of partitioning the containers so they can't see each other.  If you load "example.com" into 2 containers, the network fetches will happen twice, the HTTP cache will store things twice, and any content-visible storage will be used twice, there's no getting around that.  That may already be what you're saying, I just want to be clear that the containers don't spin up additional management apparatus.  The partitioning happens by way of suffixes on the origin.

For the disk writes issue, the Tracking Protection effort (https://wiki.mozilla.org/Security/Tracking_protection) is where engineering focus is going into limiting access to storage for trackers, including providing fake LocalStorage implementations that appear to be LocalStorage but don't persist to disk.  That's probably also where any hooks would be added to disable specific storage subsystems.

For containers opening pages in tabs that then need to be closed, I think the extensions probably want to be using the webRequest's onBeforeRequest hook[2] to intercept and cancel those requests with a `details.type` ResourceType[3] of `main_frame` (and possibly `sub_frame`) that are cross-origin and then reissue the requests as a tab open.  This would roughly correspond to intercepting fetch "navigation" requests.  (There might need to be some special-casing around link shorteners/trackers like twitter's `t.co`.)  This is possibly something that could also be added as an explicit extension point for containers, but the above should work now.


1: Caveat: If the global cookie preference is set to "session only", storage APIs will either not touch disk (cookies, legacy LocalStorage) or will purge the data at shutdown (QuotaManager controlled storage like NextGen LocalStorage, IndexedDB, and DOM Cache API).
2: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/onBeforeRequest
3: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/ResourceType
>the HTTP cache will store things twice
>there's no getting around that
Any storage could have some deduplication mechanism, especially file content of HTTP cache, it's probably very easy to detect if files for same url(alternatively match everything by hash+length so same files stored once even for different urls) from different containers are the same and only store one copy until a file changes. Since many sites use same libraries this also affect "regular" container usage.
>That's probably also where any hooks would be added to disable specific storage subsystems
The question is when, relative to this bug.
>webRequest's onBeforeRequest hook[2] to intercept and cancel those requests with a `details.type` ResourceType[3] of `main_frame` (and possibly `sub_frame`) that are cross-origin and then reissue the requests as a tab open
Maybe i'm missing something but for what i know containers are per tab and not per request, onBeforeRequest happens after the tab is opened and tab container can not be changed (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/update#Parameters doesn't list cookieStoreId as available for updateProperties). So extensions have no way to prevent a tab opening triggered by mouse click nor to change that tab container.
Anyway this is much more complex solution which is not ready yet. Because at this point most sites (all for many users) work fine if window.indexedDB is undefined I think there should be such preference. If it's name is considered misleading it could be changed.
(In reply to bugzilla from comment #6)
> Any storage could have some deduplication mechanism, especially file content
> of HTTP cache, it's probably very easy to detect if files for same
> url(alternatively match everything by hash+length so same files stored once
> even for different urls) from different containers are the same and only
> store one copy until a file changes. Since many sites use same libraries
> this also affect "regular" container usage.

De-duplication of this kind opens the door to tracking via timing side channel so we intentionally avoid this.

> >That's probably also where any hooks would be added to disable specific storage subsystems
> The question is when, relative to this bug.

The tracking protection team is making fast progress!  But this bug would not be blocked on that, no.

> >webRequest's onBeforeRequest hook[2] to intercept and cancel those requests with a `details.type` ResourceType[3] of `main_frame` (and possibly `sub_frame`) that are cross-origin and then reissue the requests as a tab open
> Maybe i'm missing something but for what i know containers are per tab and
> not per request, onBeforeRequest happens after the tab is opened and tab
> container can not be changed
> (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/
> tabs/update#Parameters doesn't list cookieStoreId as available for
> updateProperties). So extensions have no way to prevent a tab opening
> triggered by mouse click nor to change that tab container.

At a low implementation level, containers are both per-tab and per-request, as the container is baked into the underlying origin/principal and that's stored on both the document/window and all of its request.  The container definitely cannot be mutated.

You're right that I hand-waved the new tab thing a little too much.  In the event of explicit user intent to "open link in new tab" or <a target>, the new tab will be created, yes.  I was over-simplifying and assuming a same-tab single click.  The onBeforeRequest can still intercept the navigation request in the case the tab has been opened and avoid any duplicate work by preventing the actual load, but in that case the extension would need to try and re-navigate the newly opened tab.  It's possible that this tab itself can't actually be re-navigated because of how containers are exposed to webextensions and some additional hacking would be necessary.

> Anyway this is much more complex solution which is not ready yet. Because at
> this point most sites (all for many users) work fine if window.indexedDB is
> undefined I think there should be such preference. If it's name is
> considered misleading it could be changed.

It's not a configuration we have intentionally supported nor is it a configuration we intend to support in the future.  I understand that it's a configuration you find useful, and I regret that after attempting to understand your use-cases behind your choice to toggle the setting that I don't have any truly matching solutions to offer you.

Tracking protection (https://wiki.mozilla.org/Security/Tracking_protection) is really the only product feature we have at this time that intentionally 'breaks' web page functionality.  Breakage of sites by tracking protection is actively investigated and mitigated/fixed.

Containers are the other related partial solution, and as you point out, the webextensions hooks around containers aren't as fleshed out as they could be.  I have seen some advocacy for allowing webextensions more ability to interact with the tab opening process in the past, so I haven't filed an additional bug there.  I'm not sure there's a ton of hope for that functionality anytime soon, as it is a hard UX problem for latency reasons and perhaps a bigger standards/compatibility issue since that potentially entails breaking either the window.opener relationship or the security guarantees of the compartments implementation.  The good news here, at least, is that window.opener severing is being adopted more broadly and might make such enhancements in the future more practical.
>De-duplication of this kind opens the door to tracking via timing side channel so we intentionally avoid this.
Timing what? First load in container happens normally (all metadata is separated), after asynchronously checked transparently to the page, only changing pointer to file. If container cache(metadata) is cleared file is loaded and asynchronously deduplicated again.
>Tracking protection (https://wiki.mozilla.org/Security/Tracking_protection) is really the only product feature we have at this time that intentionally 'breaks' web page functionality
>Tracking Protection is a new platform-level technology that blocks HTTP loads at the network level
Either this definition is outdated or this is just reinvented wheel (tons of content-blocking addons exist already) almost completely useless for mitigating JS tracking.
>nor is it a configuration we intend to support in the future
How is it decided what should have a switch and what shouldn't? Because many other *.enabled prefs exist much longer than tracking protection, e.g. geo.enabled and javascript.enabled.
(In reply to bugzilla from comment #8)
> >De-duplication of this kind opens the door to tracking via timing side channel so we intentionally avoid this.
> Timing what? First load in container happens normally (all metadata is
> separated), after asynchronously checked transparently to the page, only
> changing pointer to file. If container cache(metadata) is cleared file is
> loaded and asynchronously deduplicated again.

Timing time-to-load/access the content post de-duplication, possibly after trying to trigger the load of the site in a different container.  If the data loads faster than it would have off of disk, the user may have visited it in another container and/or the contents of a given file may be known to match.  Even if the reads are cold, it's also possible that the time to load de-duplicated data may still be different due to additional data steps, etc.

Just practically speaking, researchers are excellent at finding novel timing attacks and since data de-duplication increases implementation complexity, it's not the highest engineering priority.  Especially when the point of containers is to provide site isolation for privacy purposes, etc.


> >Tracking protection (https://wiki.mozilla.org/Security/Tracking_protection) is really the only product feature we have at this time that intentionally 'breaks' web page functionality
> >Tracking Protection is a new platform-level technology that blocks HTTP loads at the network level
> Either this definition is outdated or this is just reinvented wheel (tons of
> content-blocking addons exist already) almost completely useless for
> mitigating JS tracking.

It's my limited understanding that the tracking protection differentiates itself from existing anti-tracking addons by going beyond blocking access to specific JS trackers and also having the ability to deny a loaded page context access to storage, to provide it with fake storage, or to provide it with specially partitioned storage.  These are not currently things that a webextension can do without invasive content script injection or re-writing of requests.  This effort seemed most aligned with your goals of denying partial access to storage, but more comprehensively avoid escapes like alternate storage backends/etc.

If you're not interested, that's fine, I just want to make sure that as we remove this preference that I provide some guidance to anyone who finds this bug later about how to best transition their use-cases if they're sticking with Firefox.

> >nor is it a configuration we intend to support in the future
> How is it decided what should have a switch and what shouldn't? Because many
> other *.enabled prefs exist much longer than tracking protection, e.g.
> geo.enabled and javascript.enabled.

Again, to be clear, I'm not suggesting tracking protection is the reason for our planning to remove this preference.  It's just one of the better possible migration paths for your use-case (as I understand it).

The preferences are largely an artifact of product/release management.  When we introduce a new implementation of something, we're very aware that it may have bugs, including potentially very scary security bugs.  In the event there's a serious bug, it's easiest to have an off switch so that we can easily disable the feature without having to wait for engineers to solve the problem.  Preferences are Firefox's mechanism for dynamic configuration, so we use them.

In many cases, it's possible to leave these preferences around forever.  Many web API's are standalone features that exist in isolation.  In some cases, it's not possible to flip the preference without breaking Firefox.  In other cases what the preference seems to do is not actually what people think it does or why it's being advocated to flip the preference out in the wilds of the internet.

When the preference starts breaking things, a product/engineering decision has to be made about whether the complexity of the switch is worth it, whether it's useful as it exists, and whether an alternate approach already exists or could be useful.

IndexedDB is an older API and a trailblazer in terms of complexity in terms of process and threading models, evolving through multiple evolutions of our bindings layers as well (XPConnect -> WebIDL).  Along those lines, the IndexedDB kill switch was implemented at a reasonably low level before we could easily turn on and off Web API's exposed to content.  (Using your example of "geo.enabled", if you look at https://searchfox.org/mozilla-central/rev/ecf61f8f3904549f5d65a8a511dbd7ea4fd1a51d/dom/webidl/Navigator.webidl#132 you can see that we actually turn off the entire exposure of the interface via that reference to the pref there.)  As referenced in comment 0, this implementation decision leads to crashes in certain scenarios where the preference has been enabled, plus has caused breakage in extensions.

It's also the case that the IndexedDB kill-switch was created before the web had begun to converge on how to express the various situations of "this API is not implemented or we're pretending we don't implement it" and "you're not allowed to use this API".  The geolocation kill-switch just pretends the API isn't implemented.

The user intent of people flipping the preference has been reasonably clear: they're using it to stop websites from storing persistent data, and in fact they don't want it breaking webextensions, etc.  As covered in comment 0, the existing cookies permissions model provide a consistent means of ensuring *all* storage for an origin is either persistent, per-session, or forbidden; with global and per-origin settings, including special settings for third-party content (AKA iframes).  A user disabling indexedDB might be surprised to find that a site would still be able to register a ServiceWorker to perform offline storage and store massive amounts of tracking data in the Cache API (or 200k of cookies or 5 megabytes of localstorage, etc.).

So their privacy goal isn't being fulfilled, but the number of permutations for how Firefox is operating has increased, and the support burden has also increased.  I've been involved in a number of bug report investigations where it turned out the problems were due to this preference being flipped.  While we absolutely want the user to be in control of their Firefox, there are practical limitations on how many operational permutations we can support.  Disabling IndexedDB while still letting a site store tracking data on a user increases the support/implementation complexity without delivering real user benefit.

The main limitation here that you hit upon is that there is a difference between disabling cookies (and possibly localstorage) versus IndexedDB and Cache API/ServiceWorkers.  That's why I've suggested the tracking protection effort as an alternative, because it is dealing explicitly with that scenario.

And it's possible we could do more permission-wise, such as providing a separate permission for the more sophisticated storage APIs like IndexedDB and the Cache API which have higher storage limits.  But we also like to try and be aware of the game-theoretic implications of all of this.  If we block IndexedDB and a bad actor would like to store more data, they can use sub-domains or other origins in iframes and shard their data storage out amongst those iframes, communicating via postMessage().  This provides them with the same storage they had before (plus making it harder for a user to manually wipe the data by spreading it amongst a lot of domains), but with potentially much worse performance characteristics for the browser.  Because the cookies API and LocalStorage are synchronous APIs, Firefox has had to implement a number of optimizations that increase memory utilization, etc.  As a very widely adopted example of resilience to missing storage APIs, check out https://github.com/localForage/localForage which prefers IndexedDB in Firefox but will happily fall back to localStorage if IndexedDB is disabled/not present.

So at least, in the large, disabling the larger storage mechanisms on their own isn't a long term solution in the face of bad actors who will adapt.  Which, again, is where tracking protection comes in because it tries to apply more clever logic and consistent rules to pages and iframes as well as being informed by the tracking protection/annotation databases obtained from third parties.

Just practically speaking, researchers are excellent at finding novel timing attacks
Well, there are always trade offs with convenience. People do such trade offs by using Firefox over TorBrowser, or TorBrowser on OS with persistent storage over LiveCD etc.
Also even if files are not deduplicated on disk it may be deduplicated in RAM, or at least have additional RAM cache size limiting - it seems that current limits are old implementation not expecting many containers.
the tracking protection differentiates itself from existing anti-tracking addons by going beyond blocking access to specific JS trackers and also having the ability to deny a loaded page context access to storage, to provide it with fake storage, or to provide it with specially partitioned storage
If you're not interested, that's fine, I just want to make sure that as we remove this preference that I provide some guidance to anyone who finds this bug later about how to best transition their use-cases if they're sticking with Firefox.
I am, but https://wiki.mozilla.org/Security/Tracking_protection doesn't mention any storage blocking/faking at all.
why it's being advocated to flip the preference out in the wilds of the internet
So their privacy goal isn't being fulfilled
It's wrong to fight users misunderstanding preferences by removing them. It's probably better to have official documentation for such preferences and extend about:config warranty void warning with something like "unofficial documentation for these preferences may be misleading, the official one is available at ...". Otherwise you'll have to remove many other prefs as somebody will discover them and write a misleading article.
As referenced in comment 0, this implementation decision leads to crashes in certain scenarios where the preference has been enabled, plus has caused breakage in extensions
I agree it's bad as it is. What i'm trying to say is that the existing cookies permissions model is not perfect and there should be a kill-switch that just pretends the API isn't implemented because users have more control over the cookies and localStorage compared to IndexedDB, so it's fine for some usecases if sites will be able to use localStorage, not everyone assumes that indexedDB.enabled disables everything.
A user disabling indexedDB might be surprised to find that a site would still be able to register a ServiceWorker to perform offline storage and store massive amounts of tracking data
Projects encouraging changing about:config preferences usually mention there are multiple ways to store data. And that a ServiceWorkers have their own switch. Again, if you are afraid people are not informed properly this can be solved by small official wiki page(which maybe already exists) referenced from about:config warning.
Disabling IndexedDB while still letting a site store tracking data on a user increases the support/implementation complexity
Is it that difficult to just not expose the API?
without delivering real user benefit
That's what i'm trying to argue from the beginning: there is benefit. It allows to disable storage which is yet not controlled as good as cookies/localStorage.
with potentially much worse performance
Users are warned about potential performance problem and they will probably discover if problems are site-related.
So at least, in the large, disabling the larger storage mechanisms
What matters is not their size but how much control there is other what is stored at what time where time is more precise than built-in per-session.
Which, again, is where tracking protection comes in
Either i'm missing something or you've posted wrong link. Please explain where there is a TP reference of storage faking, related prefs etc.

Blocks: 1581384
Assignee: nobody → bugmail
Status: NEW → ASSIGNED

There were 3 checks for this preference. Prior to bug 1079355 which ignored the
pref for system principals, the checks were all straightforward pref checks.

The conditionals are somewhat confusing as written. The main idea is the code
inside the blocks only executes if:

  • The preference is disabled.
  • The RV was success (we don't want to steal error handling, just prevent
    success.)
  • We're not dealing with the system principal.

As such, I'm removing the full conditional blocks.

Pushed by bugmail@asutherland.org:
https://hg.mozilla.org/integration/autoland/rev/c2ab1dc00f21
Remove unsupported, misleading "dom.indexedDB.enabled" preference r=ttung
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

We're not really documenting removal of preferences any more, as it doesn't seem to be very useful to web devs.

Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: