1944310 - Network cache clears partitions even when clearing by principal providing concrete partition to clear

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect, P2)

Description

[Manuel Bucher [:manuel]]

Tests in Bug 1940760 for Bug 1838506.

Notes from thread in #anti-tracking:mozilla.org thread.

Basically the test setup:

        (1)                  (2)              (last) = (1)
         | Step 1             | Step 3              | Step 4
         |                    |                     |
+--------v---------+   +------v----------+   +------v-----------+
| second-origin    |   | first-origin:   |   | second-origin    |
|                  |   | clear-cache     |   |                  |
|                  |   |                 |   |                  |
| +-iframe-------+ |   |                 |   | +-iframe-------+ |
| | first-origin | |   |                 |   | | first-origin | |
| |              | |   |                 |   | |              | |
| +-----------+--+ |   |                 |   | +-+------------+ |
+-------------+----+   +-----------------+   +---+--------------+
              |                                  |
              | Step 2            +--------------+ Step 5
              |                   |
              v                   v
 these two should return the same data. They don't and therefore fail the test.

This test and opposite test case fails. With opposite test being: clear in partitioned iframe doesn't clears unpartitioned data.

Alternative description on what is going on:

You have first origin partitioned and first origin unpartitioned. first origin unpartitioned sends a clear site data call for cache. first origin partitioned ends up being cleared too.

This behavior isn't covered by current partition test cases: toolkit/components/cleardata/tests/unit/test_network_cache:79-137

I was originally investigating Bug 1941111, but that bug is likely different, due to clearing different domains (or entire cache). That behavior isn't observed in the tests in Bug 1940760). This is critical to fix before landing clear-site-data: cache header to avoid cross-site leakage described in issue 11

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Removing the WPT expectations in should make it possible to repro the bug
testing/web-platform/meta/clear-site-data/clear-cache-partitioning.sub.https.html.ini

Comment 2

[Manuel Bucher [:manuel]]

This bug seems to be caused by the change introduced in Bug 1819147. Switching back to Equals fixes the problem. There seem to be some middle ground to be found if both use cases are to be supported.

Comment 3

[Manuel Bucher [:manuel]]

I wouldn't mark this bug as a regression, due to the current feature not being landed while change in Bug 1819147 was introduced. However, ed: Do you have insights into what best to do here?

For me it seems like we need to be able to differentiate in clearOriginAttributes whether the partitionKey is unspecified therefore should be ignored and the partitionKey being specified to be empty so should only clear unpartitioned data.

Comment 4

[Manuel Bucher [:manuel]]

Pernosco session for future reference: https://pernos.co/debug/PPCiSM7ZXzlQL3zBs48V4g/index.html

Comment 5

[Manuel Bucher [:manuel]]

Requesting info a bit too early. Just figured out that Services.cache2.clearOriginAttributes shouldn't care about partitionKey

https://searchfox.org/mozilla-central/rev/5e7382bf8bbb88d8260c72990cfea9b626d9b307/netwerk/cache2/nsICacheStorageService.idl#65-71

/**
 * Evict any cache entry having the same originAttributes.
 *
 * @param aOriginAttributes
 *   The origin attributes in string format to compare the entries with.
 */
void clearOriginAttributes(in AString aOriginAttributes);

But we also have the clearByPrincipal

https://searchfox.org/mozilla-central/rev/5e7382bf8bbb88d8260c72990cfea9b626d9b307/netwerk/cache2/nsICacheStorageService.idl#48-54

/**
 * Evict any cache entry having the same origin of aPrincipal.
 *
 * @param aPrincipal
 *   The principal to compare the entries with.
 */
void clearOrigin(in nsIPrincipal aPrincipal);

So we do have different entry points, but still need to differentiate between these two cases in the function. I thought both functions used the same entry point, but this makes implementation somewhat straight forward.

Comment 6

[edgul]

I don't have a full understanding of how the partitionKey works for cache entries, but I find the fact that clearOriginAttributes respects only a portion of the OA makes the API misleading, as written. There may be good reason for this partitionKey exclusion, but we should consider a clearer function name and/or an additional API for distinct inclusion/exclusion of OA components.

Manuel,

1. Does clearOrigin work for you? Under the hood both these APIs eventually get to a shared EvictByContextInternal, at least in some cases.
2. Can you point me to your WIP patch on this?

Anyone else,
if you know how the partitionKey is populated for cacheEntries, could you fill us in? Based on Bug 1819147 Comment 0, the functionality is different than how cookies treat generate partitionKeys.

Comment 7

[Manuel Bucher [:manuel]]

2. Can you point me to your WIP patch on this?

My fix is basically reverting the patch of Bug 1819147.

Comment 8

[Manuel Bucher [:manuel]]

1. Does clearOrigin work for you? Under the hood both these APIs eventually get to a shared EvictByContextInternal, at least in some cases.

No, clearOrigin doesn't work for me.

I'm already using clearOrigin (it is the only one that accepts the principal as argument). The corresponding C++ implementation is CacheStorageService::ClearOrigin. It uses mozilla::net::CacheFileIOManager::EvictByContextInternal under the hood, indeed.

Unfortunately the test that accomponies Bug 1819147 doesn't fail when switching back from EqualsIgnoringFPD to Equals. I'm investigating how to check whether a cache item exists. Trying stuff like (but that doesn't work yet):

/netwerk/test/unit/test_cache2_clear_with_usercontext_oa.js#28-54

  chan.loadInfo.originAttributes = { userContextId: 0, partitionKey: `(https,example.com)` };
  // [...]
  let exists = cache_storage.exists(make_uri(url), "O^partitionKey=%28https%2Cexample.com%29,");

However when using the disk cache I get NS_ERROR_UNAVAILABLE. Switching over to "memory" cache, exists is always false, no matter what I try to pass for const nsACString& aIdExtension (not sure whether that really is what would be passed for retrieving partitionKey).

Edit: I seem to need to get the partitioned cache object like this instead:

let cache_storage = getCacheStorage("disk", Services.loadContextInfo.custom(false, { userContextId: 0, partitionKey: "(https,example.com)" }));

Comment 9

[Manuel Bucher [:manuel]]

Attachment: Bug 1944310 - Make clearOrigin(nsIPrincipal) and clearOriginAttributes respect the partitionKey r=#necko,emz!

Revert logic changes of Bug 1819147.

This makes the behavior of clearOriginAttributes again in sync with the
documentation:

Evict any cache entry having the same originAttributes.1

partitionKey is part of the originAttributes2, therefore both
functions shouldn't clear unrelated partitions.

To clear the cache (and other things) across partitionKeys,
a wildcard with deleteByOriginAttributes()3 can be used.

To clear the cache for the "unpartitioned" for i.e. "http://example.com"
in userContextId: 0, it would then be necessary to pass the partition:

Services.cache2.clearOriginAttributes(JSON.stringify({ userContextId:0, paritionKey: "(http,example.com)" }))

Outside of this patch it would be good to figured out why the need for
clearing across origins arose in Bug 1819147 to address that use case
separately.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/435514e2b477
Make clearOrigin(nsIPrincipal) and clearOriginAttributes respect the partitionKey r=emz,necko-reviewers,edgul,valentin

Comment 11

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/435514e2b477