Closed Bug 1534339 Opened 10 months ago Closed Last month

OriginAttributes.firstPartyDomain does not support IPv6 addresses

Categories

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

56 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: robwu, Assigned: xeonchen)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor][domsecurity-active])

Attachments

(1 file)

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,
Blocks: 1534350

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

Flags: needinfo?(ettseng)

(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.

Assignee: nobody → xeonchen
Flags: needinfo?(ettseng)
Priority: -- → P2
Whiteboard: [tor]
Status: NEW → ASSIGNED
Whiteboard: [tor] → [tor][domsecurity-active]

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.

Pushed by xeonchen@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/14d6dd504f86
make OriginAttributes deserializable; r=baku
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72
Regressions: 1595179

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

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla72 → ---
Regressions: 1599083
No longer regressions: 1599083
Depends on: 1599083
Pushed by xeonchen@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6ee737ea06ad
make OriginAttributes deserializable; r=baku
Status: REOPENED → RESOLVED
Closed: 3 months agoLast month
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
You need to log in before you can comment on or make changes to this bug.