Make ShouldAllowAccessFor calls aware of ancestor chains same-siteness
Categories
(Core :: Privacy: Anti-Tracking, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox126 | --- | fixed |
People
(Reporter: bvandersloot, Assigned: bvandersloot)
References
(Blocks 1 open bug)
Details
Attachments
(5 files)
This makes triple-keying of Storages possible, because they rely on ShouldAllowAccessFor to determine when to use a partitioned principal.
|
Assignee | |
Comment 1•4 months ago
|
||
THIS DOESN'T WORK
the loadInfo flags aren't quite equivalent. In particular the doc channel has an unexpected disagreement.
I tried also switching over to using AntiTrackingUtils::IsThirdParty* calls instead of the new loadInfo flag here, but that broke something.
Putting this up as a minimal version that gets the spirit of what I am trying to do.
Depends on D203155
|
|
Assignee | |
Comment 2•4 months ago
|
||
Depends on D203180
|
||
Updated•4 months ago
|
|
|
||
Updated•4 months ago
|
|
|
Assignee | |
Comment 3•4 months ago
|
||
Depends on D203289
|
|
Assignee | |
Comment 4•3 months ago
|
||
I believe that about:blank is not the only url that should be ignored for third-partiness checks.
While working to make the Storage Partitioning implementation respect the entire ancestor chain (partitioning A(B(A)) contexts),
I realized that looking at the ancestor chain was not the only difference between AntiTrackingUtils::IsThirdParty* calls and
ThirdPartyUtil::IsThirdParty* calls. A big one was whether or not the other inheriting URIs were respected (e.g. about:srcdoc, blob://, ...).
I could have just left similar checks in the ATU:: versions, but I think this is the correct fix. I recall a thread a while back where you
told me most places we check for about:blank, we should also check for these other things too and I think this is one!
Additional context that may not be useful: this patch set eliminates the relaxed "privacy" definition of third-party and leaves the AntiTrackingUtils::IsThirdParty* functions as convenience functions that don't do any third-party checking themselves.
Depends on D203929
|
|
Assignee | |
Comment 5•3 months ago
|
||
Storage Access API should auto-grant in ABA frames, but should not initially "haveStorageAccess".
This does that!
Depends on D205231
Pushed by bvandersloot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/dcd6bbea816a part 1 - Make AntiTracking::IsThirdParty* aware of ABA case - r=anti-tracking-reviewers,cookie-reviewers,timhuang https://hg.mozilla.org/integration/autoland/rev/15e06d10788e part 2 - Make Workers use ancestor chain for third-partiness check - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/ac4c41cb3b67 part 3 - Fix tests relying on ABA being unpartitioned - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/4fa3fbf3a3ae part 4 - Generalize inheriting principals in ThirdPartyUtil - r=nika https://hg.mozilla.org/integration/autoland/rev/455ce831c73c part 5 - Handle ABA in Storage Access API - r=timhuang,anti-tracking-reviewers
|
||
Comment 7•3 months ago
|
||
Backed out for causing build bustages @ toolkit/components/resistfingerprinting/nsRFPService.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/dd4ea6fd487cdd13b3e9274caf128a106928dc92
|
|
Assignee | |
Comment 8•3 months ago
|
||
Looks like there was an upstream change on March 27 that caused a conflict. Rebasing, building, and trying again.
Pushed by bvandersloot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bfa9862e3480 part 1 - Make AntiTracking::IsThirdParty* aware of ABA case - r=anti-tracking-reviewers,cookie-reviewers,timhuang https://hg.mozilla.org/integration/autoland/rev/1ca7a0f27bc0 part 2 - Make Workers use ancestor chain for third-partiness check - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/1c49f0c3b677 part 3 - Fix tests relying on ABA being unpartitioned - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/49739f9ec590 part 4 - Generalize inheriting principals in ThirdPartyUtil - r=nika https://hg.mozilla.org/integration/autoland/rev/8c7a9f405031 part 5 - Handle ABA in Storage Access API - r=timhuang,anti-tracking-reviewers
|
||
Comment 10•3 months ago
•
|
||
Backed out for causing multiple failure.
- Push with failures - xpcshell failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | caps/tests/unit/test_oa_partitionKey_pattern.js | xpcshell return code: 0
- Push with failures - devtools failures
- Failure Log
- Failure line: [1075] Assertion failure: !aParentContext->IsInProcess(), at /builds/worker/checkouts/gecko/toolkit/components/antitracking/StorageAccessAPIHelper.cpp:478
- Push with failures - mochitests failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | toolkit/mozapps/extensions/test/browser/browser_installtrigger_install.js | Test timed out -
|
|
Assignee | |
Comment 11•3 months ago
|
||
Looks like a few different issues I didn't catch on my try runs.
First is definitely my bad- I didn't re-do a try run of XPC tests after changing some code for review because I didn't think I changed it enough.
Second is a 1proc issue that was pretty subtle, but actually just means we needed to have a more lax assertion.
Third was a fun consequence of a change we made in part 4 on a deprecating API.
Fixing all of these, and resubmitting!
Thank you!
|
||
Comment 12•3 months ago
|
||
Pushed by bvandersloot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9ae9252761ac part 1 - Make AntiTracking::IsThirdParty* aware of ABA case - r=anti-tracking-reviewers,cookie-reviewers,timhuang https://hg.mozilla.org/integration/autoland/rev/5dcfe3aa8497 part 2 - Make Workers use ancestor chain for third-partiness check - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/d65ac05bd9f8 part 3 - Fix tests relying on ABA being unpartitioned - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/ca1c6f8819f7 part 4 - Generalize inheriting principals in ThirdPartyUtil - r=nika,extension-reviewers,zombie https://hg.mozilla.org/integration/autoland/rev/f00e9fde550f part 5 - Handle ABA in Storage Access API - r=timhuang,anti-tracking-reviewers
|
||
Comment 13•3 months ago
•
|
||
Backed out for causing multiple failures.
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | TestStoragePrincipalHelper.TestGetPrincipalCookieBehavior5 | Value of: testPrincipal->OriginAttributesRef().mPartitionKey.EqualsLiteral( "(https,example.org)")
+Linux wpt: https://treeherder.mozilla.org/logviewer?job_id=453205814&repo=autoland
|
|
Assignee | |
Comment 14•3 months ago
|
||
Fixing both the gtest and the WPT. The WPTs are both unexpected passes, so that is good. The linked failure was a simple miss in my try run coverage.
This is a tough one to scope out what might be affected because it is a deep change to how we do storage partitioning (and a light change to srcdoc iframes) and those tests are scattered and not listed anywhere.
|
|
||
Comment 15•3 months ago
|
||
Pushed by bvandersloot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fe34837410c4 part 1 - Make AntiTracking::IsThirdParty* aware of ABA case - r=anti-tracking-reviewers,cookie-reviewers,timhuang https://hg.mozilla.org/integration/autoland/rev/4f8d611d76cf part 2 - Make Workers use ancestor chain for third-partiness check - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/0b5a5480697d part 3 - Fix tests relying on ABA being unpartitioned - r=anti-tracking-reviewers,timhuang,asuth https://hg.mozilla.org/integration/autoland/rev/a3ef4c30dc52 part 4 - Generalize inheriting principals in ThirdPartyUtil - r=nika,extension-reviewers,zombie https://hg.mozilla.org/integration/autoland/rev/e786976ead27 part 5 - Handle ABA in Storage Access API - r=timhuang,anti-tracking-reviewers
|
||
Comment 16•3 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/fe34837410c4
https://hg.mozilla.org/mozilla-central/rev/4f8d611d76cf
https://hg.mozilla.org/mozilla-central/rev/0b5a5480697d
https://hg.mozilla.org/mozilla-central/rev/a3ef4c30dc52
https://hg.mozilla.org/mozilla-central/rev/e786976ead27
Description
•