1587182 - Add support for not accepting cookies from hosts that don't have a cookie stored yet to dynamic FPI

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, enhancement)

Description

[(no longer active)]

Right now when you set network.cookie.cookieBehavior to 5, we apply the ETP rules and additionally will partition cookies and storage/communications for sites in the third-party context if the ETP rules do not apply to them.

It would be nice to add a new pref to allow experimenting with a cookie policy similar to BEHAVIOR_LIMIT_FOREIGN where we don't accept cookies from hosts unless that host already has cookies saved previously. This will make our dynamic FPI implementation behave much closer to WebKit's cookie policy.

Here is the rough set of steps for this:

1. Let's add a new boolean pref in this bug, e.g. privacy.dynamic_firstparty.limitForeign, false by default. This can be done by adding an entry to StaticPrefList.yaml.
2. We need to introduce this pref to nsICookieSettings. That interface describes the object that expresses the cookie policy that a given document has been loaded with, and is immutable throughout the lifetime of a document (even if the user modifies the browser's cookie policy while the document is alive). We can add a new attribute to that interface, e.g. called limitForeignContexts. The mozilla::net::CookieSettings object which implements the interface would receive the current pref value from here and will return true from limitForeignContexts if cookie behavior is 3 or is 5 and our new pref is true.
3. Replace this condition with checking whether limitForeignContexts returns true.

This should be enough to implement the new behaviour. Now if you rebuild and set network.cookie.cookieBehavior to 5 and privacy.dynamic_firstparty.limitForeign to true, you should see a behaviour similar to WebKit.

We'd also need to have some tests for this. In https://searchfox.org/mozilla-central/source/toolkit/components/antitracking/test/browser we currently have a number of partitioning tests: browser_partitioned* and browser_subResourcesPartitioned.js. The last one tests third-party cookies in a top-level page, and that one should definitely be modified to add support for the new pref in. This link highlights the parts of the tests which I think would be modified if the test is run with the new pref turned on. I think you should modify the test to make runTests() accept a second argument to state whether the new pref should be true or false and cover both cases in the test.

browser_partitionedCookies.js is a test that checks the partitioning of cookies in the third-party context. It uses this helper. I believe the subtests using runPartitioningTestInNormalAndPrivateMode() are probably the interesting one to focus on here. These tests create three top-level pages from three origins, attempt to store three pieces of "data" in the "data backend" being tested in the third-party context (e.g. inside a 3rd-party cookie in this test) and then read them out to verify the three distinct values are properly stored and can be retrieved.

Right now as you can see the test just tries to store ["A", "B", "C"] and expects to read back ["A", "B", "C"]. With the new pref turned on, it should read back ["", "", ""] (I think). So we should extend the partitionedstorage_head.js framework to allow passing in extraPrefs as well as override the expectation data we expect to read back. This could all be done through adding more arguments to runPartitioningTestInNormalAndPrivateMode().

More generally though it would be nice to verify that the rest of the browser_partitioned* tests which all use the same mini-test frameworks continue to pass with the new pref being set to either true or false. So perhaps a better way to do this instead of special casing everything in browser_partitionedCookies.js is to modify the partitionedstorage_head.js to run each test twice, once with and once without the new pref set, and add just one argument to runPartitioningTestInNormalAndPrivateMode() to say whether we're running the cookie backend tests (e.g. testCategory, could be "cookies", "cache", "serviceworker", "indexeddb", etc.) and inside partitionedstorage_head.js use the testCategory argument to specialize our expectation value when testCategory == "cookies". Doing things that way would give us a more comprehensive test coverage which is nicer.

Comment 1

[Johann Hofmann [:johannh]]

I read the description but I'm not 100% clear on if you want the "cookie stored yet" count to include localStorage and indexedDB. Only counting cookies here seems a bit... backwards. Do you agree? :)

Comment 2

[(no longer active)]

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

I read the description but I'm not 100% clear on if you want the "cookie stored yet" count to include localStorage and indexedDB.

No, I really meant just cookies.

Only counting cookies here seems a bit... backwards. Do you agree? :)

It depends on what you mean. From another perspective, the entire heuristic makes no sense. :-)

The whole goal of this heuristic would be to be compatible with WebKit, so implementing something incompatible with them is an explicit non-goal. The reason why I'm picking only cookies is WebKit compatibility, that's all.

FWIW, ideally you'd want something that also considers past user interaction, since this heuristic would be vulnerable to the same privacy holes that caused WebKit to implement ITP. Again, the reason to ignore past user interaction is WebKit compatibility as well...

Comment 3

[Johann Hofmann [:johannh]]

I see, thanks!

Comment 4

[Gary Chen [:xeonchen]]

Attachment: Bug 1587182 - add `limitForeignContexts` attribute to support dynamic FPI; r=ehsan

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9e030967f2d
add `limitForeignContexts` attribute to support dynamic FPI; r=Ehsan

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/c9e030967f2d