Closed Bug 1669716 Opened 4 years ago Closed 3 years ago

cookies extension API unaware of dFPI; firstPartyDomain does not work with dynamic First-Party Isolation

Categories

(WebExtensions :: General, defect, P1)

defect

Tracking

(firefox-esr78 unaffected, firefox-esr91 wontfix, firefox92 wontfix, firefox93 wontfix, firefox94 fixed)

RESOLVED FIXED
94 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- wontfix
firefox92 --- wontfix
firefox93 --- wontfix
firefox94 --- fixed

People

(Reporter: robwu, Assigned: robwu)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: dev-doc-complete, regression)

Attachments

(1 file)

Since bug 1640135, dFPI started to use a new key (partitionKey) instead of the existing firstPartyDomain key to isolate cookies. This is problematic for extensions, because the lack of dFPI awareness prevents them from modifying/removing cookies, as seen in the comments from https://bugzilla.mozilla.org/show_bug.cgi?id=1388873#c2 onwards.

bug 1640135 claims:

Adding another attribute for dFPI seems reasonable and is quite cheap, we should probably consider doing this.

... a bit late now, but I want to emphasize that the addition of a new origin attribute is not cheap. It requires consumers (such as the extension API) to be adapted.

Tim, in order to resolve this bug I'd like to explore the feasibility to map the external-facing firstPartyDomain (extension) API to the internal firstPartyDomain + partitionKey attribute. This would only be possible if FPI and dFPI are mutually exclusive.

In https://bugzilla.mozilla.org/show_bug.cgi?id=1640135#c1, Anne hinted that they may indeed be mutually exclusive. Can you confirm whether this is the case?

Flags: needinfo?(tihuang)

Yes, the dFPI and FPI are mutually exclusive. So, the dFPI will be disable once the FPI is enabled, see Bug 1631676.

Flags: needinfo?(tihuang)

Rob to provide details how to fix this and find a bug to block.

Flags: needinfo?(rob)

This bug is already a blocker of bug 1549587.

Since dFPI and FPI are mutually exclusive, we can solve this by changing the logic, to have the following logic when reading or writing cookies when the firstPartyDomain is specified in the cookies extension API:

  1. Is FPI enabled? Use originAttributes.firstPartyDomain
  2. Is dFPI enabled? Use originAttributes.partitionKey
  3. Otherwise, use originAttributes.firstPartyDomain (for backwards-compatibility, to allow users to migrate cookies from FPI to non-FPI).

This logic does mean that it is not possible for an extension to modify / clean up cookies from dFPI when dFPI is disabled.
To counter that, we could introduce a pref to allow users to toggle the behavior of the extension API when FPI and dFPI are both disabled.

My preferred solution is to revert bug 1640135, especially since FPI and dFPI are supposed to be mutually exclusive.

Severity: -- → S2
Flags: needinfo?(rob)
Priority: -- → P2

(In reply to Tim Huang[:timhuang] from comment #2)

Yes, the dFPI and FPI are mutually exclusive. So, the dFPI will be disable once the FPI is enabled, see Bug 1631676.

(In reply to Rob Wu [:robwu] from comment #4)

My preferred solution is to revert bug 1640135, especially since FPI and dFPI are supposed to be mutually exclusive.

Tim - is there any reason to use partitionKey and firstPartyDomain at the same time, considering that they are mutually exclusive? There is a non-zero cost to support these two separate properties, and if they are mutually exclusive there does not seem to be an obvious reason for keeping them separate / independent.

Flags: needinfo?(tihuang)

No, I don't think there is a reason to use the partitionKey and firstPartyDomain at the same time. But, I do think that we should use different originAttributes for these two mutually exclusive features. The key firstPartyDomain was built only for FPI and should be only used for FPI. We chose to use a new Key 'partitionKey' is because we could explore possibilities for dFPI and we don't want to mess with FPI. For example, we have implemented partitioning by sites and static network partitioning by using partitionKey. These are somehow incompatible with FPI. So, even these two features are mutually exclusive, I don't think we should use the same originAttributes. And it's supposed to be hard to revert bug 1640135 right now since we've built something on top of it.

Flags: needinfo?(tihuang)

With dDPI rolling out to more users (#1686296) and Mozilla marketing it on their blog (a major privacy advance in Firefox), should the priority by bumped?

Sure, "Total Cookie Protection" is a good thing, but deleted cookies must be better than partitioned cookies :)

I can't get to this bug this week, but I want this to be fixed "soon".

Assignee: nobody → rob
Status: NEW → ASSIGNED
Priority: P2 → P1
See Also: → 1691907

Notes from looking into what it takes to support the functionality.

I tried hard to abstract the internal details of FPI and dFPI, so that extension developers don't have to somehow mimic the internals of Firefox to create/remove/update cookies. (the alternative is to introduce a new property and let extensions deal with the complexity of detecting (d)FPI and hoping that they choose a good value)

  1. First, I compared FPI and dFPI, and listed relevant prefs and the internal representation.
  2. Then I evaluated whether it's possible to reliably choose the right destination for an extension-provided "firstPartyDomain" value (based on FPI/dFPI prefs).
  3. Then I described an updated format of the how the extension-supplied "firstPartyDomain" value can be parsed, with little risk of breakage.
  4. Finally, I considered to make the firstPartyDomain property a required property when dFPI is enabled.

I'll leave this comment open for feedback and will attach patches with the implementation within a few weeks.

1. FPI and dFPI

  • FPI and dFPI are mutually exclusive (bug 1631676). If a user somehow enables both (via about:config), then FPI takes precedence.
    • FPI is enabled by default in the Tor Browser, and off by default everywhere else.
    • dFPI is off by default, but enabled by default in private browsing mode and in Nightly.
    • dFPI can be toggled through about:prefences (ETP) and about:config, FPI only through about:config.
  • FPI:
    • privacy.firstparty.isolate = true, then FPI is enabled (and only cookies with a non-empty firstPartyDomain are used).
    • NOTE: Even if FPI is disabled, then firstPartyDomain can still be a non-empty string, e.g. for SafeBrowsing cookies.
  • dFPI: based on multiple preferences (StoragePrincipalHelper implementation uses StoragePartitioningEnabled):
    • If FPI is enabled, then dFPI is disabled.
    • Let cookieBehavior be the bool value of network.cookie.cookieBehavior or network.cookie.cookieBehavior.pbmode (for private browsing browsing cookies, i.e. cookieStoreId = "firefox-private" in Firefox's extension API).
    • If cookieBehavior == 5 (BEHAVIOR_REJECT_TRACKER_AND_PARTITION_FOREIGN), then dFPI is enabled.
    • If cookieBehavior == 4 (BEHAVIOR_REJECT_TRACKER) AND privacy.storagePrincipal.enabledForTrackers is false (this is not the case by default; I filed bug 1710241 to consider removal of the pref), then dFPI is enabled.
    • Otherwise dFPI is disabled.
    • "User" (developer) documentation for the cookieBehavior preference is at https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Privacy/State_Partitioning#disable_dynamic_state_partitioning

The internal representation of the firstPartyDomain/partitionKey OriginAttributes:

  • firstPartyDomain is a domain, whereas partitionKey is a tuple (scheme,domain,port) (or (scheme,domain) if the default port is used). (implementation)
  • The site-based serialization instead of just the domain is from bug 1637516, which introduced two preferences;
    • privacy.dynamic_firstparty.use_site (default true) enables the site-serialization for partitionKey
    • privacy.firstparty.isolate.use_site from is false by default and the true version is not supported (no tests, no documentation). Supporting this would require a data migration of some sort. Bug 1630869 tracks this.

2. Supporting FPI and dFPI in one property based on prefs

The existing "firstPartyDomain" property of the extension API can only be re-used if there is a reliable way to map the internal OriginAttributes to that property. This is easy, because dFPI and FPI are mutually exclusive. The converse, mapping firstPartyDomain to originAttributes.firstPartyDomain and originAttributes.partitionKey is a bit more involved, but somewhat doable. Given a firstPartyDomain property (e.g. passed to browser.cookies.set or browser.cookies.remove):

  • Is the key a known firstPartyDomain value? Use originAttributes.firstPartyDomain
  • Is FPI enabled? Use originAttributes.firstPartyDomain
  • Is dFPI enabled? Use originAttributes.partitionKey
  • Otherwise neither FPI nor dFPI are enabled.
    • Since it is more likely for dFPI than FPI to be flipped, assume that the value is a originAttributes.partitionKey by default.
      • This change affects backwards-compatibility. Previously we used originAttributes.firstPartyDomain instead.
        • note: users who like to migrate FPI cookies to non-FPI can temporarily enable FPI via the pref. To move these over to FPI, they can flip the FPI preference again.

FPI and dFPI currently have a different internal representation, but may eventually converge towards a single format. For this reason, I decided against parsing the extension-provided value to guess the intended key (FPI's firstPartyDomain vs dFPI's partitionKey).

3. Parsing firstPartyDomain value from extension

Currently, the firstPartyDomain value is used as is as the value for OriginAttributes.firstPartyDomain. The value is sanitized by the OriginAttributes internals. The use of raw values previously led to issues such as bug 1470156 and bug 1534339. It will also cause issues when the internal representation of FPI changes (bug 1630869). Another issue is that it's not obvious to extensions that they have to compute the eTLD+1 of a tab's URL to use as the firstPartyDomain.

To solve these issues, the raw format is restricted, and the following new logic is used instead:

  • if the input is a known firstPartyDomain value, use it as is.
  • if input looks like a URL (scheme://host[:port]), parse it and generate the (scheme,domain,port) tuple.
    • note: the output from browser.cookies.get/browser.cookies.getAll is also formatted to look like a URL.
    • note: internally, the "domain" is not the host component of a tab's URL. It should actually be the eTLD+1. To prevent extensions from knowing the parsing rules, the eTLD+1 is extracted from the given URL.
  • If FPI is enabled, use the input as is for firstPartyDomain (for backwards-compatibility, no other checks are performed)
    • Out of scope for this bug, but in a follow-up bug the validation can become stricter (possibly along with bug 1630869).
  • When FPI is disabled, other values (e.g. non-URL) are rejected. This is to prevent extensions that aren't designed with dFPI in mind to inadvertently copy cookies to a different partition.

4. firstPartyDomain value requirement

Currently, firstPartyDomain is optional but required when FPI is enabled. This is to prevent extensions that are unaware of FPI from accidentally modifying cookies in an incompatible way.

For dFPI, I can either go with required (to make sure that extensions use the correct partition), or make it optional (since partitionKey is an empty string for first-party cookies). The advantage of marking it as required is that extensions will surely not break the value. The disadvantage is that extensions that are only interested in first-party cookies will need to be updated to add firstPartyDomain: "". The latter is not that bad of an idea, and since firstPartyDomain has been supported for a long while in Firefox, extensions will still be backwards-compatible.

See Also: → 1630869, 1710241
Flags: needinfo?(tom)

Does reusing the firstPartyDomain key name for a (related but still) different purpose partitionKey risk confusion? Would we benefit if we decided to deprecate it and introduce a more generic name?

This change affects backwards-compatibility. Previously we used originAttributes.firstPartyDomain instead.

This would also potentially break backward compatibility again if we ever swtich to FPI (by default, or in some specific mode)?

The latter is not that bad of an idea, and since firstPartyDomain has been supported for a long while in Firefox, extensions will still be backwards-compatible.

I don't understand what you're saying here. In any case, cross-browser compatibility and MV3 are also considerations for making the key required.

(In reply to Tomislav Jovanovic :zombie from comment #11)

Does reusing the firstPartyDomain key name for a (related but still) different purpose partitionKey risk confusion? Would we benefit if we decided to deprecate it and introduce a more generic name?

Although the internals are different, they are conceptually similar, as in they are used for partitioning cookies. I aimed for re-using the two to allow extensions that were designed for FPI to still work with dFPI. After all, it doesn't matter how the internal representation is, as long as an extension reads the "firstPartyDomain" property and passed it back when creating/updating a cookie.

This change affects backwards-compatibility. Previously we used originAttributes.firstPartyDomain instead.

This would also potentially break backward compatibility again if we ever swtich to FPI (by default, or in some specific mode)?

FPI is the default of Tor, and since FPI is stricter than dFPI, it's probably not likely for it to switch to dFPI. FPI has been around for a while, so with the efforts spent on dFPI I don't expect the switch from dFPI to FPI. Even if that did happen, then cookies have to be migrated by the browser to avoid loss of sessions. And after that, to an extension it shouldn't matter whether dFPI or FPI is used.

The only situation where the difference is significant is when an extension wants to be aware of FPI and dFPI, for example when a user has used both in one profile. I am consciously in favor of supporting only FPI xor dFPI for API simplicity. After all, when (d)FPI is used, the cookies from the other setting are never used, and an extension doesn't really have to worry about it.

The latter is not that bad of an idea, and since firstPartyDomain has been supported for a long while in Firefox, extensions will still be backwards-compatible.

I don't understand what you're saying here.

Firefox recognizes the firstPartyDomain property in the API already. In the default configuration, it's just an empty string. An extension that (unconditionally) passes the firstPartyDomain property to cookies.set/cookies.remove is compatible with Firefox versions without a patch for this bug.

In any case, cross-browser compatibility and MV3 are also considerations for making the key required.

I'd aim for cross-browser compatibility where possible. If the key is not required when dFPI is enabled, then extensions without awareness of (d)FPI will inadvertently mix third-party cookies with first-party cookies. As either approach does not only have advantages, I'm not fully decided on whether this should be optional or required (slightly favoring required).

I don't see the relevance of MV3 here though.

See Also: → 1710769
See Also: 1710769

Thanks for this excellent write-up and analysis. I'm a little fuzzy on Section 3, specifically

  • "if input looks like a URL (scheme://host[:port]), parse it and generate the (scheme,domain,port) tuple. "
  • But then if FPI is enabled use the value directly
  • And if dFPI is enabled, use the tuple?
  • And if dFPI is not enabled, use the tuple also?
  • And if it looks like a url but fpi is enabled, use it anyway even though that may not(?) work with fpi
  • And if it can't be parsed but FPI isn;t enabled what do we do?

But none of that really worries me.


I don't forsee FPI become the default over dFPI. But someday I do hope that all of the FPI code goes away, and the FPI behavior (that of being strict with no compromises) is implemented by the dFPI code as some sort of extra, hidden dFPI mode. I don't know when - if ever - that will happen.

Flags: needinfo?(tom)

(In reply to Tom Ritter [:tjr] (ni? for response to sec-[advisories/bounties/ratings/cves]) from comment #14)

Thanks for this excellent write-up and analysis. I'm a little fuzzy on Section 3, specifically

  • "if input looks like a URL (scheme://host[:port]), parse it and generate the (scheme,domain,port) tuple. "

The input describes the input from the extension, the tuple/domain is generated as needed depending on the privacy.dynamic_firstparty.use_site / privacy.firstparty.isolate.use_site prefs. I'm accepting URLs for FPI too, so that extensions designed for compatibility with dFPI will also work with FPI.

  • But then if FPI is enabled use the value directly

In case it wasn't clear: the list should be read from top to down, whenever a condition is matched the described action is taken.
So in this case, the input doesn't look like a URL, and the value is likely a domain and used as is.
Even if it isn't a domain, it's still used as-is (and the behavior is consistent with today's behavior of the cookies extension API).

  • And if dFPI is enabled, use the tuple?

Yes. If dFPI is enabled, the input should look like a URL, which is internally converted to a tuple.

  • And if dFPI is not enabled, use the tuple also?

Yes if the input is an URL, a tuple is generated. This design allows extensions to migrate between non-dFPI and FPI.

  • And if it looks like a url but fpi is enabled, use it anyway even though that may not(?) work with fpi

If FPI is enabled, the input URL is reduced to a host name (or a tuple depending on privacy.firstparty.isolate.use_site) and used with the originAttributes.firstPartyDomain key (instead of partitionKey).

  • And if it can't be parsed but FPI isn;t enabled what do we do?

"When FPI is disabled, other values (e.g. non-URL) are rejected. This is to prevent extensions that aren't designed with dFPI in mind to inadvertently copy cookies to a different partition."

Blocks: dfpi-hq

Here is a status update on what I've been up to so far:

  • I have written unit tests and experimented with the implementation to explore the feasibility of re-using the firstPartyDomain property to internally use firstPartyDomain and partitionKey, as described before.
  • A difference compared to comment 10 is that the format is deterministic, independent of prefs, because "dFPI enabled" is not clear-cut; There is not one global dFPI flag, but one for normal and one for private browsing mode. Hence I used the following baseline logic:
    • If FPI is enabled, a string value is required (potentially ""). Otherwise the value is optional, and defaults to partitionKey: "", firstPartyDomain: "", which covers non-dFPI cookies (and first-party cookies when dFPI is enabled).
    • If input value contains :/, then it's partitionKey.
    • Otherwise treat the value as a firstPartyDomain attribute (FPI).
    • But there are exceptions, see below
  • Recognizing and formatting a partitionKey is more complicated than "take the scheme, eTLD+1 and port". Sometimes the result is incompatible with the logic described in my previous bullet point). OriginAttributes::SetPartitionKey delegates to PopulateTopLevelInfoFromURI, which has special cases for certain schemes and IP addresses.

I think that it's doable to get the original goal of re-using the firstPartyDomain option in the extension API to work with FPI and dFPI.
However, I've since seen that Google is also working on a feature comparable to dFPI (see Intent to Prototype: CHIPS), so it may make sense to introduce a new property after all, despite the existence of partitionKey in the extension API.

At the API level, I am considering to introduce a new method to allow extensions to get the expected cookie jar (e.g. firstPartyDomain and/or the extension API representation for partitionKey) for a given tabId / frameId. I am considering this, because it is not really obvious which keys an extension should choose when they read or write cookies for a specific document.

Depends on: 1724768
Depends on: 1724771
Depends on: 1724774
Blocks: 1732473
Keywords: dev-doc-needed
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/20b66bbd2a66
Add partitionKey support to cookies extension API r=timhuang,zombie
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 94 Branch

MDN documentation changes made in partitionKey in the cookies API #9382

Compatibility data changes made in Details of support for partitionKey in cookies #12789

(In reply to Pulsebot from comment #18)

Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/20b66bbd2a66
Add partitionKey support to cookies extension API r=timhuang,zombie

== Change summary for alert #31817 (as of Tue, 12 Oct 2021 06:09:29 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
10% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 166.05 -> 150.00
8% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 160.91 -> 148.15

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31817

(In reply to Andra Esanu from comment #22)

(In reply to Pulsebot from comment #18)

Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/20b66bbd2a66
Add partitionKey support to cookies extension API r=timhuang,zombie

== Change summary for alert #31817 (as of Tue, 12 Oct 2021 06:09:29 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
10% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 166.05 -> 150.00
8% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 160.91 -> 148.15

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31817

This attributed perf improvement is rather unlikely, unless the test uses the cookies extension API.

Dev document needed changes completed in partitionKey in the cookies API #9382 (MDN documentation) and Details of support for partitionKey in cookies #12789 (Compatibility data).

Keywords: dev-doc-needed

(In reply to Rob Wu [:robwu] from comment #23)

(In reply to Andra Esanu from comment #22)

(In reply to Pulsebot from comment #18)

Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/20b66bbd2a66
Add partitionKey support to cookies extension API r=timhuang,zombie

== Change summary for alert #31817 (as of Tue, 12 Oct 2021 06:09:29 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
10% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 166.05 -> 150.00
8% tsvgr_opacity linux1804-64-shippable-qr e10s fission stylo webrender 160.91 -> 148.15

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31817

This attributed perf improvement is rather unlikely, unless the test uses the cookies extension API.

Andra could you revisit this alert and see if there is a more likely culprit?

Flags: needinfo?(aesanu)

I'm investigating this. I will let you know when I have my updates.

Flags: needinfo?(aesanu)

We decided to mark this alert as invalid. Thank you for the help.

Has Regression Range: --- → yes
See Also: → 1873418
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: