1534339 - OriginAttributes.firstPartyDomain does not support IPv6 addresses

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P2)

Description

[Rob Wu [:robwu]]

Bug 1473247 intended to add support for IP addresses to the firstPartyDomain member of OriginAttributes. This works.
Bug 1470156 attempted to fix an assertion failure by replacing characters such as ":" with "+" in OriginAttributes::CreateSuffix. Consequently, when the OriginAttributes are reconstructed via OriginAttributes::PopulateFromSuffix, the resulting object differs from the original (e.g. for IPv6 addresses).
This breaks stuff, e.g. cookie access, as shown below.

I think that bug 1470156 should have been resolved by removing the assertion (since the whole suffix is already URL-encoded a few lines later), and the browser.cookies extension API should perform some basic validation to avoid non-sensical values in firstPartyDomain.

STR:

1. Visit about:config and set privacy.firstparty.isolate to true .

2. Start a server at localhost, available over IPv6. Example, with Node.js:

   node -e 'require("http").createServer((_,res)=>res.end()).listen(8000)'
   

3. Visit http://[::1]:8000/

4. Open the devtools console, and set a cookie:

   document.cookie = "foo=bar; max-age=3600"
   

5. Refresh the page.

6. Look at the Network tab of the Devtools console, click on the request and confirm that the Cookie header is set and contains foo=bar.

7. Run document.cookie from the JS console and confirm that the result is the string "foo=bar".

8. Open the same URL (from step 3) in a new content process: open it in a new tab, or (if that doesn't work) restart Firefox.

9. Repeat step 6 (see cookie header network tab) and step 7 (read document.cookie).

Expected:

• Step 9 must have the same result as step 6 and 7, i.e. the "foo=bar" cookie must be seen.

Actual:

• At step 9, the Cookie request header has the expected value, but document.cookie is empty.
  --> Any cookies set from the content process are visible to document.cookie, but cookies set in a different process (e.g. main process) are not.

When I patch Firefox and remove the normalization at https://searchfox.org/mozilla-central/rev/2f1020dc4176d38dd5f3d0496f3c46849862ee0b/caps/OriginAttributes.cpp#148-149, then I get the expected result.

Explicitly performing the same kind of normalization at the cookie access also yields the expected result. OA is serialized when sent down from the parent to the content process (so the cookies are keyed by "[++1]" instead of "[::1]"), so explicitly serializing and deserializing OA in the child allows the cookie to be found via the cookie key with "[++1]" as firstPartyDomain:

diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp
index a77d0e4f8a35..b4436f26ada9 100644
--- a/netwerk/cookie/CookieServiceChild.cpp
+++ b/netwerk/cookie/CookieServiceChild.cpp
@@ -291,9 +291,13 @@ void CookieServiceChild::GetCookieStringFromCookieHashTable(
 
   nsCOMPtr<nsILoadInfo> loadInfo;
   mozilla::OriginAttributes attrs;
+  nsAutoCString suffix;
   if (aChannel) {
     loadInfo = aChannel->LoadInfo();
     attrs = loadInfo->GetOriginAttributes();
+    attrs.CreateSuffix(suffix);
+    attrs.mFirstPartyDomain.Truncate();
+    NS_ENSURE_TRUE_VOID(attrs.PopulateFromSuffix(suffix));
   }
 
   nsCookieService::GetBaseDomain(TLDService, aHostURI, baseDomain,

Comment 1

[Daniel Veditz [:dveditz]]

Ethan: I believe firstpartyisolation is your team? Can you please triage this?

Comment 2

[Ethan Tseng [:ethan]]

(In reply to Daniel Veditz [:dveditz] from comment #1)

Ethan: I believe firstpartyisolation is your team? Can you please triage this?

Thanks for notifying me. We'll triage it in the Tor Uplift meeting today. Keep the needinfo for now.

Comment 3

[Gary Chen [:xeonchen]]

Attachment: Bug 1534339 - make OriginAttributes deserializable;

CreateSuffix is irreversible by PopulateFromSuffix because it uses a multi-to-one mapping.
Since only ':' will happen in a IPv6 format, we can make it a 1-to-1 mapping so that the firstPartyDomain and geckoViewUserContextId are consistent after CreateSuffix and PopulateFromSuffix.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14d6dd504f86
make OriginAttributes deserializable; r=baku

Comment 5

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/14d6dd504f86

Comment 6

[Alexandru Michis [:malexandru]]

Backed out as requested by :xeonchen for causing Bug 1595179.

Backout link: https://hg.mozilla.org/integration/autoland/rev/9f6892c03bd8dfcc604461ea90e71430c220fb74

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ee737ea06ad
make OriginAttributes deserializable; r=baku

Comment 8

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6ee737ea06ad