When "privacy.firstparty.isolate" is true, double-key permissions to origin + firstPartyDomain
Categories
(Core :: DOM: Security, defect, P2)
Tracking
()
People
(Reporter: arthur, Assigned: xeonchen)
References
(Blocks 1 open bug, Regressed 1 open bug)
Details
(Whiteboard: [tor 21569][domsecurity-backlog2])
Attachments
(23 files, 9 obsolete files)
8.50 KB,
patch
|
Details | Diff | Splinter Review | |
18.68 KB,
patch
|
Details | Diff | Splinter Review | |
4.05 KB,
patch
|
Details | Diff | Splinter Review | |
6.91 KB,
patch
|
Details | Diff | Splinter Review | |
4.55 KB,
patch
|
Details | Diff | Splinter Review | |
1.53 KB,
patch
|
Details | Diff | Splinter Review | |
11.58 KB,
patch
|
Details | Diff | Splinter Review | |
9.51 KB,
patch
|
Details | Diff | Splinter Review | |
18.49 KB,
patch
|
Details | Diff | Splinter Review | |
3.33 KB,
patch
|
Details | Diff | Splinter Review | |
11.31 KB,
patch
|
Details | Diff | Splinter Review | |
4.47 KB,
patch
|
Details | Diff | Splinter Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Permissions settings can be a tracking vector. When we are isolating by first party, we should double-key permissions to the origin and the first-party (and possibly other origin attribute fields)?
Updated•7 years ago
|
Updated•7 years ago
|
Updated•7 years ago
|
Reporter | ||
Updated•7 years ago
|
Reporter | ||
Comment 1•7 years ago
|
||
Here is Tor Browser's current patch: https://torpat.ch/21569
Reporter | ||
Updated•6 years ago
|
Reporter | ||
Comment 2•6 years ago
|
||
Reporter | ||
Comment 3•6 years ago
|
||
Reporter | ||
Comment 4•6 years ago
|
||
Reporter | ||
Comment 5•6 years ago
|
||
I've added a couple of WIP patches. Johann, I wonder if you might have time to have a look and let me know if this approach would be acceptable? (I'm still working on fixing some unit tests.)
Comment 6•6 years ago
|
||
Comment on attachment 8981570 [details] [diff] [review] 0001-Bug-1330467-Change-SitePermissions-to-use-principal.patch Review of attachment 8981570 [details] [diff] [review]: ----------------------------------------------------------------- Hi Arthur, this looks like a great approach to me! Switching nsIPermissionManager to use only principals is in fact already tracked in bug 1402957 and since the permission manager stores (stripped) origins anyway I don't think we're in danger of losing anything by not using the URI interfaces. There's one thing I wonder. We could have SitePermissions.jsm functions accept EITHER an nsIPrincipal or an nsIURI (which would be converted to a principal then), right? That would provide us with a few advantages: - Your patch would touch way less code, you could just leave all the e.g. test usage with nsIURI in place - We don't have to fear accidentally missing to fix up any consumer of SitePermissions.jsm - It's a little more ergonomic for front-end developers I guess creating the principal on the fly is a little expensive, so we might want to document that the nsIURI part is mostly intended to be used in tests. I think that in most real world code it should be possible to get the content principal without taking it from the URI. I'm happy to do a more thorough review of this whenever you think it's ready :) Thanks! ::: browser/components/preferences/sitePermissions.js @@ +314,4 @@ > > for (let p of this._permissionsToChange.values()) { > let uri = Services.io.newURI(p.origin); > + let principal = Services.scriptSecurityManager.createCodebasePrincipal(uri, {}); Hm, is this stripping OA on purpose? Otherwise this could be simplified to Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(p.origin). ::: browser/modules/webrtcUI.jsm @@ +364,4 @@ > uri = Services.io.newURI(aRequest.documentURI); > } > > + let principal = Services.scriptSecurityManager.createCodebasePrincipalFromOrigin(uri); I don't think that works with an nsIURI, you likely want to pass in aRequest.origin. But it's even easier than that, you can just use browser.contentPrincipal here.
Comment 8•6 years ago
|
||
(In reply to Marion Daly [:mdaly] from comment #7) > Status on this? 302 to Arthur who is working on this bug. Arthur, are there any updates here?
Reporter | ||
Comment 9•6 years ago
|
||
Hi all -- no updates at the moment, but I hope to work on this in the next couple of weeks.
Updated•6 years ago
|
Reporter | ||
Comment 10•6 years ago
|
||
Reporter | ||
Comment 11•6 years ago
|
||
Reporter | ||
Comment 12•6 years ago
|
||
Reporter | ||
Comment 13•6 years ago
|
||
Reporter | ||
Comment 14•6 years ago
|
||
Reporter | ||
Comment 15•6 years ago
|
||
Reporter | ||
Comment 16•6 years ago
|
||
Reporter | ||
Comment 17•6 years ago
|
||
Reporter | ||
Comment 18•6 years ago
|
||
Reporter | ||
Comment 19•6 years ago
|
||
Reporter | ||
Comment 20•6 years ago
|
||
Reporter | ||
Comment 21•6 years ago
|
||
Reporter | ||
Comment 22•6 years ago
|
||
Reporter | ||
Comment 23•6 years ago
|
||
Reporter | ||
Comment 24•6 years ago
|
||
Reporter | ||
Comment 25•6 years ago
|
||
Reporter | ||
Comment 26•6 years ago
|
||
Reporter | ||
Comment 27•6 years ago
|
||
Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69f9fb7cba27&selectedJob=212120566
Comment hidden (obsolete) |
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 29•5 years ago
|
||
Assignee | ||
Comment 30•5 years ago
|
||
Assignee | ||
Comment 31•5 years ago
|
||
Assignee | ||
Comment 32•5 years ago
|
||
Assignee | ||
Comment 33•5 years ago
|
||
Assignee | ||
Comment 34•5 years ago
|
||
Assignee | ||
Comment 35•5 years ago
|
||
Assignee | ||
Comment 36•5 years ago
|
||
Assignee | ||
Comment 37•5 years ago
|
||
Assignee | ||
Comment 38•5 years ago
|
||
Assignee | ||
Comment 39•5 years ago
|
||
Assignee | ||
Comment 40•5 years ago
|
||
Assignee | ||
Comment 41•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 42•5 years ago
|
||
Comment 43•5 years ago
|
||
Pushed by xeonchen@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1d48bdbb4035 part 1. Don't strip first party domain from permissions key; r=johannh,Ehsan https://hg.mozilla.org/integration/autoland/rev/0ae215d91758 part 2. Add SitePermissions APIs functions to accept principal; r=johannh https://hg.mozilla.org/integration/autoland/rev/f939c61e051f part 3. Use principal for permissions in pageinfo/permissions.js; r=johannh https://hg.mozilla.org/integration/autoland/rev/6ac44130d2bb part 5. Access permissions by principal in PermissionUI.jsm; r=johannh https://hg.mozilla.org/integration/autoland/rev/35d96a4ff659 part 6. Use principal for permissions in sitePermissions.js; r=johannh https://hg.mozilla.org/integration/autoland/rev/b43fa07d5756 part 7. Confirm FPI in permission manager tests; r=Ehsan https://hg.mozilla.org/integration/autoland/rev/4686eebd8962 part 8. Make canvas permission respect FPI; r=johannh,jrmuizel https://hg.mozilla.org/integration/autoland/rev/2648f5bb1804 part 9. Use principal with mobile permissions; r=johannh,snorp https://hg.mozilla.org/integration/autoland/rev/2cd09bae2bdf part 10. Use principal for permissions in browser site-identity; r=johannh https://hg.mozilla.org/integration/autoland/rev/2f2308fe5747 part 11. Use principal for permissions in webrtc UI; r=johannh https://hg.mozilla.org/integration/autoland/rev/0229d5353d50 part 12. FPI isolation of translation permissions; r=johannh
Comment 44•5 years ago
|
||
whooo fingers crossed
Comment 45•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1d48bdbb4035
https://hg.mozilla.org/mozilla-central/rev/0ae215d91758
https://hg.mozilla.org/mozilla-central/rev/f939c61e051f
https://hg.mozilla.org/mozilla-central/rev/6ac44130d2bb
https://hg.mozilla.org/mozilla-central/rev/35d96a4ff659
https://hg.mozilla.org/mozilla-central/rev/b43fa07d5756
https://hg.mozilla.org/mozilla-central/rev/4686eebd8962
https://hg.mozilla.org/mozilla-central/rev/2648f5bb1804
https://hg.mozilla.org/mozilla-central/rev/2cd09bae2bdf
https://hg.mozilla.org/mozilla-central/rev/2f2308fe5747
https://hg.mozilla.org/mozilla-central/rev/0229d5353d50
Comment 46•5 years ago
|
||
Good job. Well done!
Comment 47•5 years ago
|
||
Backed out 11 changesets (Bug 1330467) as requested by xeonchen on IRC.
Backout: https://hg.mozilla.org/integration/autoland/rev/3866561a7bae97a6c9ec3c18174747ec36bba090
Assignee | ||
Comment 48•5 years ago
|
||
(In reply to Mihai Alexandru Michis [:malexandru] from comment #47)
Backed out 11 changesets (Bug 1330467) as requested by xeonchen on IRC.
Backout: https://hg.mozilla.org/integration/autoland/rev/3866561a7bae97a6c9ec3c18174747ec36bba090
Thanks!
Comment 49•5 years ago
|
||
Pushed by xeonchen@gmail.com: https://hg.mozilla.org/integration/autoland/rev/38e1751e35df part 1. Don't strip first party domain from permissions key; r=johannh,Ehsan https://hg.mozilla.org/integration/autoland/rev/0ba364f5941f part 2. Add SitePermissions APIs functions to accept principal; r=johannh https://hg.mozilla.org/integration/autoland/rev/51c96b44ff30 part 3. Use principal for permissions in pageinfo/permissions.js; r=johannh https://hg.mozilla.org/integration/autoland/rev/ebf3f2f197f6 part 5. Access permissions by principal in PermissionUI.jsm; r=johannh https://hg.mozilla.org/integration/autoland/rev/1bb35c7f8236 part 6. Use principal for permissions in sitePermissions.js; r=johannh https://hg.mozilla.org/integration/autoland/rev/974de551ec76 part 7. Confirm FPI in permission manager tests; r=Ehsan https://hg.mozilla.org/integration/autoland/rev/66b30aa15bcd part 8. Make canvas permission respect FPI; r=johannh,jrmuizel https://hg.mozilla.org/integration/autoland/rev/51414d51854c part 9. Use principal with mobile permissions; r=johannh,snorp https://hg.mozilla.org/integration/autoland/rev/0f0888b33389 part 10. Use principal for permissions in browser site-identity; r=johannh https://hg.mozilla.org/integration/autoland/rev/299bbef55ad6 part 11. Use principal for permissions in webrtc UI; r=johannh https://hg.mozilla.org/integration/autoland/rev/446a73750a48 part 12. FPI isolation of translation permissions; r=johannh
Comment 50•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/38e1751e35df
https://hg.mozilla.org/mozilla-central/rev/0ba364f5941f
https://hg.mozilla.org/mozilla-central/rev/51c96b44ff30
https://hg.mozilla.org/mozilla-central/rev/ebf3f2f197f6
https://hg.mozilla.org/mozilla-central/rev/1bb35c7f8236
https://hg.mozilla.org/mozilla-central/rev/974de551ec76
https://hg.mozilla.org/mozilla-central/rev/66b30aa15bcd
https://hg.mozilla.org/mozilla-central/rev/51414d51854c
https://hg.mozilla.org/mozilla-central/rev/0f0888b33389
https://hg.mozilla.org/mozilla-central/rev/299bbef55ad6
https://hg.mozilla.org/mozilla-central/rev/446a73750a48
Comment 51•5 years ago
|
||
[Tracking Requested - why for this release]: After this has spun in nightly for a bit; we would ideally uplift it to beta since this is for Tor and we want it to be in the ESR if at all possible.
Comment 53•5 years ago
|
||
Regression bug 1554805
Updated•5 years ago
|
Description
•