Tool operating as 3rd party iframe unable to use LockManager
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox116 | --- | fixed |
People
(Reporter: pfranza, Assigned: saschanaz)
References
(Regressed 1 open bug)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:106.0) Gecko/20100101 Firefox/106.0
Steps to reproduce:
Tool is a 3rd party iframe with total cookie protection turned on, the cookies/localstorage are being partitioned correctly, however when I goto use a LockManager.request it throws an error that
Uncaught (in promise) DOMException: LockManager.request: request() is not allowed in this context
Actual results:
Uncaught (in promise) DOMException: LockManager.request: request() is not allowed in this context
Expected results:
I would have expected the call to either just be partition the locks like everything else is partitioned, or to just allow the locks and make the lock local to the page only, or in the worst case just remove LockManager.request from the page so I it clear that it isn't supported and I can degrade.
Comment 1•2 years ago
|
||
The Bugbug bot thinks this bug should belong to the 'Core::Privacy: Anti-Tracking' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
Updated•2 years ago
|
Comment 2•2 years ago
|
||
saschanaz, could you take a look at this bug? Thanks.
Assignee | ||
Comment 3•2 years ago
|
||
It's the GetStorageAccess API that returns ePartition{}OrDeny
and currently the implementation always denies such situation.
I'm not 100% sure partitioned context can just be allowed here. Does ContentPrincipalInfoHashKey correctly handle such partitioning?
I'll write some tests and see what happens there.
Assignee | ||
Updated•2 years ago
|
Updated•2 years ago
|
Comment 4•2 years ago
|
||
(In reply to Kagami :saschanaz from comment #3)
It's the GetStorageAccess API that returns
ePartition{}OrDeny
and currently the implementation always denies such situation.I'm not 100% sure partitioned context can just be allowed here. Does ContentPrincipalInfoHashKey correctly handle such partitioning?
This is a case where if we had the StorageKey abstraction (Bug 1776271 for the formal abstraction, but the OPFS patch stack is adding a GetStorageKey method that will return a PrincipalInfo for now at least) and the appropriate policy machinery integrated into the concept of a StorageKey, we could map ePartitionOrDeny to a null principal to provide for partitioning/isolation. Of course, a null principal is not a content principal, so ContentPrincipalInfoHashKey would not work for that.
Note however that there is a bit of messiness where the algorithm to obtain a storage key explicitly says "If key’s origin is an opaque origin, then return failure.", but in this case we would be using the null principal internally to correspond to a partitioned origin. GetStorageKey for an explicit opaque origin with a null principal origin would fail.
Assignee | ||
Comment 5•2 years ago
•
|
||
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #4)
Note however that there is a bit of messiness where the algorithm to obtain a storage key explicitly says "If key’s origin is an opaque origin, then return failure.", but in this case we would be using the null principal internally to correspond to a partitioned origin. GetStorageKey for an explicit opaque origin with a null principal origin would fail.
Indeed, and https://github.com/w3c/web-locks/pull/75 wants to fix that. I'm not sure how exactly to obtain a storage bottle map
algorithm maps to our implementation, though.
In any case, we might want to fix the spec first before fixing the implementation.
Edit: Oh wait, I somehow thought you pointed to https://w3c.github.io/web-locks/#obtain-a-lock-manager which has the same step. Meh.
Assignee | ||
Comment 6•2 years ago
|
||
but the OPFS patch stack is adding a GetStorageKey method that will return a PrincipalInfo for now at least
Seems that's bug 1790207.
Assignee | ||
Comment 7•2 years ago
|
||
Of course, a null principal is not a content principal, so ContentPrincipalInfoHashKey would not work for that.
I used ContentPrincipalInfoHashKey only because every relevant principal was content principal, that should be able to change.
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D163336
Assignee | ||
Comment 9•2 years ago
|
||
Note for me: Migrate to nsIPrincipal from PrincipalInfo? https://groups.google.com/a/mozilla.org/g/dev-platform/c/A721Mg-SFFw
Assignee | ||
Comment 10•2 years ago
|
||
Note for me 2: No real need to use mochitest when https://wpt.fyi/results/web-locks/partitioned-web-locks.tentative.https.html?label=experimental&label=master&aligned exists?
Comment 12•2 years ago
|
||
The following field has been copied from a duplicate bug:
Field | Value | Source |
---|---|---|
Status | NEW | bug 1764547 |
For more information, please visit auto_nag documentation.
Assignee | ||
Comment 13•2 years ago
•
|
||
Just experimented a bit and it seems nsIGlobalObject::GetStorageAccess returns eAllow for first-party iframe nested in a third-party iframe. Is it expected? Maybe Paul knows:
(Context: An existing WPT test assumes such case should be partitioned too: https://searchfox.org/mozilla-central/rev/75da1dd5d4b9b991f919a41594194eab93cdef62/testing/web-platform/tests/web-locks/partitioned-web-locks.tentative.https.html#101-103)
Comment 14•2 years ago
•
|
||
AFAIC currently we only partition by top level site without any ancestor bits so it would make sense for storage to be unpartitioned in this case. Tim, what do you think? Should we additionally take ancestors into account?
Also see https://github.com/web-platform-tests/wpt/pull/33460
Comment 15•2 years ago
|
||
From the privacy perspective, we only consider if the iframe domain is considered same-party as the top-level domain to determine storage access; we don't consider the ancestor chain. So, the first-party nested iframe is still considered first-party in this case.
There were some discussions for HTTP cache before regarding whether we should consider the ancestors for partitioning caches. No conclusion was made at the time because taking ancestors into account could regress performance. Still, it does provide certain benefits, like preventing cache attacks in a nested first-party iframe.
Regarding whether we should consider ancestors when partitioning Weblock, I am leaning more toward considering the ancestor chain if it doesn't regress performance. Also, the WPT test implies that we should do this. However, I am unsure whether we should apply the same thing to Storage Access because it affects other storage and is likely to have a negative impact. We should have a consensus about the behavior of storage access across browser vendors before we make any changes.
Updated•2 years ago
|
Assignee | ||
Comment 16•2 years ago
|
||
Depends on D163337
Comment 17•1 year ago
|
||
Hello Kagami,
a gentle ping here for updates to ensure this S2 is still on track, thank you. :)
Comment 19•1 year ago
|
||
Comment 21•1 year ago
|
||
Backed out for causing wpt failures on partitioned-web-locks.tentative.https.html
Failure logs:
- https://treeherder.mozilla.org/logviewer?job_id=419506196&repo=autoland
- TV https://treeherder.mozilla.org/logviewer?job_id=419486726&repo=autoland
Backout link: https://hg.mozilla.org/integration/autoland/rev/f9cb00c2cdc956ea9c3083d9ba66cd9848cd26db
Assignee | ||
Comment 23•1 year ago
|
||
The TV failure looks like an existing issue since the failure happens inside PartitionedStorageHelper.
The Android WPT failure is weird, is dFPI not enabled on Android? It seems the relevant flags does not filter Android out, not sure what's the issue here.
Comment 24•1 year ago
|
||
As discussed on Slack, it looks like dFPI isn't enabled by default on Android yet. We should consider changing the default in StaticPrefList.yaml
https://searchfox.org/mozilla-central/rev/c0adc2160976e2c118e2e5709d08aac071fddce9/modules/libpref/init/StaticPrefList.yaml#11335,11347 . Before we do that we should check with the Fenix folks though. For now you can enable dFPI via pref in the test config and see if it fixes the test failure.
Comment 25•1 year ago
|
||
Comment 26•1 year ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f602589f22e5
https://hg.mozilla.org/mozilla-central/rev/4f4e44d1026b
Description
•