1835907 - Only propagate storage access during self-initiated, same-origin navigation

Status: RESOLVED FIXED

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

Description

[Benjamin VanderSloot [:bvandersloot]]

See the changes in https://github.com/privacycg/storage-access/pull/141.

Particularly the flag to the source snapshot params used during create navigation params by fetching, and the conditional copy of sourceDocument's relevant settings object's flag over to the new environment that will be created

Comment 1

[Benjamin VanderSloot [:bvandersloot]]

Note: this means that we should not always initially set hasStorageAccess to true if the permission is set.

Comment 2

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 1 - Add has storage access bit and triggering window id to the LoadInfo - r=smaug!

Comment 3

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 2 - Add has storage access bit and triggering window id to the SessionHistoryEntries - r=smaug!

Depends on D184821

Comment 4

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 3 - Only propogate storage access bit on same-origin, self-initiated subframe navigations - r=#anti-tracking!

Depends on D184822

Comment 5

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 4 - Test the storage access permission before prompting the user - r=#anti-tracking!

Because the Document's Channel's LoadInfo is no longer being set to reflect the storage-access permission, we need to test if the permission is set before we ask the user about it.

Currently I opted for an IPC (and one we already have) in Fission. We could cut this out with an extra bit on the document, but I don't think this is performance-critical since it only happens on prompt-able requestStorageAccess calls.

Depends on D184823

Comment 6

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 5 - Refactor the window's mStorageAccessPermissionGranted variable and its Getters to a more accurate name: mUsingStorageAccess - r=#anti-tracking!

This probably should have been done earlier, but became obvious with uses of Document::HasStorageAccessPermissionGrated in this stack.

Depends on D184824

Comment 7

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 6 - Update anti-tracking tests to work when storage access is not automatically given to iframes where the permission is already granted - r=#anti-tracking!

Depends on D184825

Comment 8

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1835907, part 7 - Update DOM Cache API tests to remove most TCP-specific code, disable in xorigin - r=asuth!

Now that iframes don't automatically get "storage access" when the permission is allowed, we need to call requestStorageAccess in the iframes that use that access. This test makes that change.

In xorigin tests, the test gets the storage access in the test's iframe then creates a new iframe to run the tests in. I just added calls to the storage access API in that second iframe.

Depends on D184826

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/74f90708260a
part 1 - Add has storage access bit and triggering window id to the LoadInfo - r=smaug,necko-reviewers,kershaw,pbz
https://hg.mozilla.org/integration/autoland/rev/bb9f48eec5bf
part 3 - Only propogate storage access bit on same-origin, self-initiated subframe  navigations - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/4790e44c234c
part 4 - Test the storage access permission before prompting the user - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/86e3f98ceb31
part 5 - Refactor the window's mStorageAccessPermissionGranted variable and its Getters to a more accurate name: mUsingStorageAccess - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/989479621780
part 6 - Update anti-tracking tests to work when storage access is not automatically given to iframes where the permission is already granted - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/91ef29afec50
part 7 - Update DOM Cache API tests to remove most TCP-specific code, disable in xorigin - r=asuth

Comment 10

[Cosmin Sabou [:CosminS]]

Backed out for causing multiple failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=91ef29afec50af8628de6ae793ff088fb9d907e9&selectedTaskRun=FcLDZfPORdOd9kJvJlpEFA.0

Failure logs:

• devtools/client/netmonitor/test/browser_net_http3_request_details.js - https://treeherder.mozilla.org/logviewer?job_id=425947260&repo=autoland
• devtools/client/netmonitor/test/browser_net_headers_sorted.js - https://treeherder.mozilla.org/logviewer?job_id=425942845&repo=autoland
• dom/security/test/sec-fetch/test_iframe_history_manipulation.html - https://treeherder.mozilla.org/logviewer?job_id=425943810&repo=autoland
• toolkit/components/antitracking/bouncetrackingprotection/test/browser/browser_bouncetracking_simple.js - https://treeherder.mozilla.org/logviewer?job_id=425945971&repo=autoland
• toolkit/mozapps/extensions/test/xpinstall/browser_required_useractivation.js - https://treeherder.mozilla.org/logviewer?job_id=425949356&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/7454614ef39046960ee75b81928128e9a77aa0e2

Comment 11

[Benjamin VanderSloot [:bvandersloot]]

Found the issue: I was injecting a user interaction into the loadinfo by accident. This is resolved, so I'm going to push again once static analysis and a fresh try run finishes.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e6a83f1b7940
part 1 - Add has storage access bit and triggering window id to the LoadInfo - r=smaug,necko-reviewers,kershaw,pbz
https://hg.mozilla.org/integration/autoland/rev/51e533c3d812
part 3 - Only propogate storage access bit on same-origin, self-initiated subframe  navigations - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/f543a9922f2e
part 4 - Test the storage access permission before prompting the user - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/bd2d6d649e2e
part 5 - Refactor the window's mStorageAccessPermissionGranted variable and its Getters to a more accurate name: mUsingStorageAccess - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/6f1909eb87de
part 6 - Update anti-tracking tests to work when storage access is not automatically given to iframes where the permission is already granted - r=anti-tracking-reviewers,pbz
https://hg.mozilla.org/integration/autoland/rev/6d987539c7b6
part 7 - Update DOM Cache API tests to remove most TCP-specific code, disable in xorigin - r=asuth

Comment 13

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e6a83f1b7940
https://hg.mozilla.org/mozilla-central/rev/51e533c3d812
https://hg.mozilla.org/mozilla-central/rev/f543a9922f2e
https://hg.mozilla.org/mozilla-central/rev/bd2d6d649e2e
https://hg.mozilla.org/mozilla-central/rev/6f1909eb87de
https://hg.mozilla.org/mozilla-central/rev/6d987539c7b6