1713139 - QuotaCleaner deleteByBaseDomain support for sessionStorage and legacy localStorage

Status: RESOLVED FIXED

(Toolkit :: Data Sanitization, enhancement)

Description

[Paul Zühlcke [:pbz]]

Bug 1705036 updates the QuotaCleaner to support clearing by base domain for next gen storage. However, since next gen storage isn't enabled on release, we also need to update the legacy localStorage and sessionStorage implementations to support clearing by base domain.
Most notably the implementation is lacking the ability to clear storage partitioned under the domain to be cleared.
For example: Clearing storage keyed under https://example.com^partitionKey=(https,example.org) where example.org is the domain to be cleared.

The QuotaCleaner currently uses these observer notifications to clear by host: https://searchfox.org/mozilla-central/rev/2b372b94ce057097a6ef8eb725f209faa9d1dc4d/toolkit/components/cleardata/ClearDataService.jsm#507-510

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Can't this just be posed in terms of the 2 primitives of clearing by the host (which would first be promoted to the base domain) and then using deleteByOriginAttributes which is handled by legacy LS via observer notification?

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

At a meta level, I think there's potentially an interesting abstraction layering question between what the high level operations the clear data service exposes and what the lower level data clearing primitives are. The docs for the cleaners.

This ends up related to what the underlying storage schemes are and whether they are structured in such a way that specialized primitives are desirable to allow the storage subsystems to more efficiently clean things. Or if there are complexities related to UX-flows where there's a 2 phase approach where phase 1 is gather up all the origins with data and phase 2 is to explicitly clear those origins with data, and whether the the set of origins with data has changed by the time of phase 2, and so a primitive needs to be able to express "clear everything" versus just "clear each of these origins which I'm really hoping constitutes everything".

In the case of QuotaManager, all storage is currently entirely done on a per-origin basis and this will be the case for all currently planned changes, so it's mainly a question of being able to express things atomically with a predicate that can work on any thread. The OriginAttributes filter mechanism is currently the best option for this, but maybe it could be enlarged further?

Comment 3

[Paul Zühlcke [:pbz]]

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #1)

Can't this just be posed in terms of the 2 primitives of clearing by the host (which would first be promoted to the base domain) and then using deleteByOriginAttributes which is handled by legacy LS via observer notification?

Good point! Ideally yes. However we are only interested in a portion of the partitionKey and OriginAttributesPattern does not support matching parts of an OriginAttributes field. Specifically, we want to match (<scheme>, baseDomain, <port>) for any scheme and port.
I had looked into extending OriginAttributesPattern to support that, however adding specialized methods in the storage implementations where needed seemed like a simpler approach that may also have better performance. We can still look into this though if fits better for the legacy storage implementation. I hope I'm getting this right, eventually the purge observer notifications will go away and we will only clear via nsIQuotaManagerService?

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #2)

At a meta level, I think there's potentially an interesting abstraction layering question between what the high level operations the clear data service exposes and what the lower level data clearing primitives are. The docs for the cleaners.

This ends up related to what the underlying storage schemes are and whether they are structured in such a way that specialized primitives are desirable to allow the storage subsystems to more efficiently clean things. Or if there are complexities related to UX-flows where there's a 2 phase approach where phase 1 is gather up all the origins with data and phase 2 is to explicitly clear those origins with data, and whether the the set of origins with data has changed by the time of phase 2, and so a primitive needs to be able to express "clear everything" versus just "clear each of these origins which I'm really hoping constitutes everything".

In the case of QuotaManager, all storage is currently entirely done on a per-origin basis and this will be the case for all currently planned changes, so it's mainly a question of being able to express things atomically with a predicate that can work on any thread. The OriginAttributes filter mechanism is currently the best option for this, but maybe it could be enlarged further?

If performance allows for it I prefer to collect and filter entries to be cleared in the ClearDataService as a central place for data clearing logic. This isn't possible for all storages, since they may not (want to / cannot) expose data via XPCOM.

Comment 4

[Paul Zühlcke [:pbz]]

I'll look further into extending the OriginAttributesPattern, maybe we can add a child-pattern and a dictionary just for the partitionKey here: https://searchfox.org/mozilla-central/rev/2b372b94ce057097a6ef8eb725f209faa9d1dc4d/dom/chrome-webidl/ChromeUtils.webidl#758
Though we have to be mindful of performance, since we'd shift from a simple string compare to parsing and comparing partitionKey components.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Paul Zühlcke [:pbz] from comment #3)

If performance allows for it I prefer to collect and filter entries to be cleared in the ClearDataService as a central place for data clearing logic. This isn't possible for all storages, since they may not (want to / cannot) expose data via XPCOM.

As a general aside: there are a bunch of places data is potentially stored right now that are defined to live on the ServiceWorkerRegistration like notifications and push registrations that are not actually implemented that way in Gecko, but which in the future should be moved to more directly tied to the registration storage[1]. That's all TBD, but just as a big picture perspective, I think we do want to try and centralize that storage somewhat.

To what you're saying here, which I may be mis-interpreting, I understand this to be:

1. Get all the origins from QuotaManager that it knows about.
2. In ClearDataService, iterate over all the origins and decide what to clear.
3. Tell QuotaManager to clear exactly the results of filtering that data.

My concern is that there's inherently time between step 1 and step 3 there where a new origin "new.nefariousorigin^partitionKey=(https,example.org)" for example could write data to disk after the list of all origins is retrieved from QM for the purposes of clearing example.org.

If QuotaManager is explicitly told "clear the pattern matching *.example.org / ^partitionKey=(https,example.org), QuotaManager can both make sure it clears everything with that data in that instant while also potentially blocking any new writes that match those patterns until that block is explicitly relaxed. (This is a general meta problem of trying to clear data while globals are alive or could have initiated asynchronous things that could still be making their ways to storage back-ends.)

Alternately, QuotaManager could be told to block all of its operations during that time, but that's more than a little scary in terms of creating browser deadlocks (ex: remote settings is built on IndexedDB and would be impacted) and OOMs as content processes can continue to generate large streams of data.

(In reply to Paul Zühlcke [:pbz] from comment #4)

I'll look further into extending the OriginAttributesPattern, maybe we can add a child-pattern and a dictionary just for the partitionKey here: https://searchfox.org/mozilla-central/rev/2b372b94ce057097a6ef8eb725f209faa9d1dc4d/dom/chrome-webidl/ChromeUtils.webidl#758
Though we have to be mindful of performance, since we'd shift from a simple string compare to parsing and comparing partitionKey components.

If the partitionKey isn't atomic, (edited here on re-reading) I agree it should be exploded into its constituent values into a child-pattern and dictionary on OriginAttributes (in addition to the compound string). Either way, QuotaManager is currently going to potentially do a tremendous amount of disk I/O to provide a list of all origins so I wouldn't be too concerned about the in-memory pattern matching overhead.

That said, for QuotaManager V4, it could potentially be nice if there's an easy way to map OriginAttributesPattern in such a way that an efficient SQL query could be used that could leverage indices, etc. (For example, there are cases where origins/domains are stored in reversed order so that prefix checks can be performed instead of suffix checks, etc.)

1: Especially since it's likely ServiceWorker registrations will soon no longer have an immutable scope, which is the current key used in most cases.

Comment 6

[Johann Hofmann [:johannh]]

My concern is that there's inherently time between step 1 and step 3 there where a new origin "new.nefariousorigin^partitionKey=(https,example.org)" for example could write data to disk after the list of all origins is retrieved from QM for the purposes of clearing example.org.

We've had this discussion in the past, right? IIRC we ended up with "we need a UI-oriented solution for guiding the user to closing any open browsing contexts that may store information after or during data cleanup" and until we have that then I would consider this (otherwise very valid) concern out of scope for data clearing work :)

Comment 7

[Paul Zühlcke [:pbz]]

(In reply to Andrew Sutherland [:asuth] (he/him) from comment #5)

To what you're saying here, which I may be mis-interpreting, I understand this to be:

1. Get all the origins from QuotaManager that it knows about.
2. In ClearDataService, iterate over all the origins and decide what to clear.
3. Tell QuotaManager to clear exactly the results of filtering that data.

Yes, this is how we currently clear data via the quota service.

My concern is that there's inherently time between step 1 and step 3 there where a new origin "new.nefariousorigin^partitionKey=(https,example.org)" for example could write data to disk after the list of all origins is retrieved from QM for the purposes of clearing example.org.

If QuotaManager is explicitly told "clear the pattern matching *.example.org / ^partitionKey=(https,example.org), QuotaManager can both make sure it clears everything with that data in that instant while also potentially blocking any new writes that match those patterns until that block is explicitly relaxed. (This is a general meta problem of trying to clear data while globals are alive or could have initiated asynchronous things that could still be making their ways to storage back-ends.)

This is under the assumption that the site being cleared (and with it the partitioned bucket) is still active, right? We could fix this in the frontend by closing associated tabs before clearing site data. This may not work for all consumers though. What about chrome code?
Having leftover data leads to two problems. Firstly the web application might break due to inconsistent state. Secondly there is a privacy issue where the user expects that data has been fully cleared, but we end up with data left over.

(In reply to Paul Zühlcke [:pbz] from comment #4)

I'll look further into extending the OriginAttributesPattern, maybe we can add a child-pattern and a dictionary just for the partitionKey here: https://searchfox.org/mozilla-central/rev/2b372b94ce057097a6ef8eb725f209faa9d1dc4d/dom/chrome-webidl/ChromeUtils.webidl#758
Though we have to be mindful of performance, since we'd shift from a simple string compare to parsing and comparing partitionKey components.

If the partitionKey isn't atomic, (edited here on re-reading) I agree it should be exploded into its constituent values into a child-pattern and dictionary on OriginAttributes (in addition to the compound string).

Not sure if we can easily change the partitionKey format at this point without some major refactoring. Maybe only changing the pattern would work...

Either way, QuotaManager is currently going to potentially do a tremendous amount of disk I/O to provide a list of all origins so I wouldn't be too concerned about the in-memory pattern matching overhead.

I'm not worried about the overhead for this specific use case. We should be mindful of other usages of the pattern that currently match exact partitionKey. We could also add a separate field partitionKeyPattern so we only have to parse the string if the consumer sets it.

That said, for QuotaManager V4, it could potentially be nice if there's an easy way to map OriginAttributesPattern in such a way that an efficient SQL query could be used that could leverage indices, etc. (For example, there are cases where origins/domains are stored in reversed order so that prefix checks can be performed instead of suffix checks, etc.)

+1
How do we currently map pattern to query? I assume this is storage specific.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Paul Zühlcke [:pbz] from comment #7)

This is under the assumption that the site being cleared (and with it the partitioned bucket) is still active, right? We could fix this in the frontend by closing associated tabs before clearing site data. This may not work for all consumers though. What about chrome code?

For the case of the new origin, yeah, the assumption is that there are open tabs/windows which could open a new (third-party) iframe.

In theory ServiceWorkers could also open new third party contexts via Clients.openWindow, but we limit this to happening in response to user interaction, so it shouldn't be a problem. ServiceWorkers are a notable complication in terms of active globals because ServiceWorkers can stay around for up to a minute after their tabs are closed.

I'm not sure what you're asking in terms of chrome code. The user is likely to break the browser if they randomly wipe out data stored under the system principal and I don't think the about:preferences UI exposes this data. (Browser toolbox's storage tab does, however.)

How do we currently map pattern to query? I assume this is storage specific.

Currently this is all just in-memory stuff. QuotaManager's current on-disk format for everything is just full origin strings (with some escaping/normalization for directory names to avoid illegal characters) and all matching is done in-memory against QuotaManager's fully-in-memory list of all known origins. In the future, QMv4 may start to store some of this data more canonically on disk in a database (with extremely high likelihood of all the pages existing in the page cache so we never actually hit disk).

QuotaManager currently has its own OriginScope matching helper for this which has a variant Pattern which stores an OriginAttributesPattern. This is used in data clearing as well as by the DirectoryLock abstraction for checking overlaps which is relevant in the response I'm going to write to :johannh's comment 6.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

Dealing with open windows / workers

(In reply to Johann Hofmann [:johannh] from comment #6)

We've had this discussion in the past, right? IIRC we ended up with "we need a UI-oriented solution for guiding the user to closing any open browsing contexts that may store information after or during data cleanup" and until we have that then I would consider this (otherwise very valid) concern out of scope for data clearing work :)

We've definitely discussed this in the past. I no longer think we need to wait for a UI solution, though, thanks to the changes made by Fission which means we can know about every window global in the parent process, their hierarchical relationship, and potentially communicate with them. (Previously, the Clients API let us know about all globals, but not their relationships.) Also, now we have CookieJarSettings.

I think it's now viable that the ClearDataService can work to mark browsing contexts as having their storage access banned/revoked and have that be propagated to any iframes even through navigations, at least until such time as the top-level context is reloaded. I believe this to be consistent with user intent; if the user clears data for a site, then they probably don't want the site able to write the exact same data back immediately. But if they explicitly reload the site or otherwise explicitly navigate to it, they probably do. For example, in cases where the site is broken and the user is told to clear their cookies and reload, this approach should work.

A potential sketch for this might be, as viewed by Quota Manager:

• Have all globals have a concept of a privacyEpoch. This is captured at user-initiated navigation time for top-level browsing contexts, and propagated to their iframes or successor navigations if the navigations aren't user initiated.
• Whenever ClearDataService performs a clear operation, it increments privacyEpoch and conveys this new epoch along with the underlying clearing operations ("clear baseDomain example.org, clear DFPI/partition key matching example.org at all").
• QuotaManager runs the clear operations. This:
  • Breaks all existing DirectoryLocks (the handles the QM clients use for origin directories) matching the rules and clears any data.
  • Remembers this new pattern and the new privacyEpoch. Any new requests that come in that have a privacyEpoch earlier than this new epoch will be checked against the clearing operation patterns and prevented from accessing storage if they match. This part could alternately be handled via the Clients API which QM really needs to start using as its source of Principals instead of just trusting whatever the actor tells it.
• ClearDataService could optionally attempt to update all the impacted CookieJarSettings in all the processes to let them know that their storage access has been revoked. Once this happens, subsystems like QuotaManager / the ClientManagerService could drop their rules which are redundant. (They only need to know the rules during the transition time.)

In a case like the user clearing ALL data, the OriginAttributesPattern could potentially be set against just the privacyEpoch, with this implying the epoch might want to go on the Principal. Noting that right now QuotaCleaner.deleteAll uses getUsage to get all the origins and then does manual filtering to http/https/file to avoid wiping out webextensions, but really should just be a "clear all http/https/file schemes" pattern or similar. Noting that QuotaManager could absolutely report back what origins it clears as it clears them, and/or it could have a "pretend" option that first reports everything that would be cleared, and then ClearDataService could confirm it wants them all cleared. QM could also check if any new origins show up and report those somehow, but it's not clear that a user would necessarily have any desire to have to be re-prompted to deal with extra origins that came into existence while the user was in the settings UI.

Comment 10

[Paul Zühlcke [:pbz]]

I've filed Bug 1713645 for splitting up the partitionKey. That's not a short-term project though. It would require a lot of refactoring and testing. I started working on just adding a OA pattern field today though, that seems to work well. With my changes to OriginAttributesPatternDictionary I can clear the legacy partitioned storage like this:

Services.obs.notifyObservers(
    null,
    "extension:purge-localStorage",
    aBaseDomain
);
Services.obs.notifyObservers(
    null,
    "clear-origin-attributes-data",
    JSON.stringify({ partitionKeyPattern: { baseDomain: aBaseDomain } })
);

Edit: The first call clears all storage for base domain and and subdomains, across all origin attributes. That means it also clears aBaseDomain partitioned as a third-party. The second call clears all entries where the partitionKey contains the base domain. That means all third-party storage partitioned under aBaseDomain.

Still looking into session storage.

Comment 11

[Paul Zühlcke [:pbz]]

Attachment: Bug 1713139 - Clear partitioned storage for session storage and legacy localStorage implementation.

Depends on D116606

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/90baa3bc0a53
Clear partitioned storage for session storage and legacy localStorage implementation. r=dom-storage-reviewers,johannh,asuth

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/90baa3bc0a53