Open Bug 1397624 Opened 4 years ago Updated 23 days ago

Provide an option for first-party isolation in Private Browsing Mode

Categories

(Firefox :: Private Browsing, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: arthur, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [tor])

Attachments

(4 files)

I would like to propose adding a pref to enable first-party isolation in Private Browsing mode. First-party isolation (currently behind the privacy.firstparty.isolate pref) prevents third party from tracking users across websites (aka supercookies).

According to a recent survey by DuckDuckGo (https://duckduckgo.com/download/Private_Browsing.pdf):
"""
  • 41.0 ±2.5% believe that Private Browsing “Prevents websites from tracking me.”
  • 39.1 ±2.6% believe that Private Browsing “Prevents ads from tracking me.”
"""

Including first-party isolation in PBM would help to meet these expectations without having to block content directly.

This functionality could be behind a pref, e.g. "privacy.firstparty.isolate.pbm". Perhaps one day the pref could be enabled by default.

The existing implementation of first-party isolation (using OriginAttributes) means that I think the necessary code changes are relatively small.
Oops, the second sentence should be "First-party isolation ... prevents third parties from tracking users across websites by storing stateful information in the browser (aka supercookies)."
I want to clarify my explanation. The pref I am proposing would be a boolean, "privacy.firstparty.isolate.pbm", with the following effect:

* When "privacy.firstparty.isolate.pbm" is false, Firefox behaves normally.
* When "privacy.firstparty.isolate" is true, first-party isolation is enforced in Private Browsing Windows only.

The pref would be false by default.
(In reply to Arthur Edelstein (Tor Browser dev) [:arthuredelstein] from comment #2)
> I want to clarify my explanation. The pref I am proposing would be a
> boolean, "privacy.firstparty.isolate.pbm", with the following effect:
> 
> * When "privacy.firstparty.isolate.pbm" is false, Firefox behaves normally.
> * When "privacy.firstparty.isolate" is true, first-party isolation is
> enforced in Private Browsing Windows only.
> 
> The pref would be false by default.

What do you mean Firefox behaves "normally"? Why would "privacy.firstparty.isolate" is true be PB mode only - how do I then use FPI in normal browsing mode?

Isn't it easier to just have:
privacy.firstparty.isolate = applies to normal browsing mode
privacy.firstparty.isolate.pbm = applies to PB mode

Thus allowing 4 combinations: all on, all off, normal on and PB off, normal off and PB on
(In reply to Simon Mainey from comment #3)

I'm sorry, I made a typo and I wish I could edit comment 2. I meant to write:

* When "privacy.firstparty.isolate.pbm" is false, Firefox behaves normally.
* When "privacy.firstparty.isolate.pbm" is true, first-party isolation is enforced in Private Browsing windows only.

"private.firstparty.isolate" would continue to apply as a global mode.
OK, that makes sense now.

"private.firstparty.isolate" is not global (it excludes PB Mode which does not currently use OriginAttributes, but I know what you mean) hence this ticket.

But from your comment 4 I see that *.pbm applies only to PB Mode windows, and as per current FPI pref, that only applies to normal windows. So exactly what we wanted, right - as per comment 3?
See Also: → FirstPartyIsolation
Whiteboard: [tor]
Can we rename the ticket to "Provide an option to restrict first-party isolation to Private Browsing Mode" - thanks
(In reply to Arthur Edelstein (Tor Browser dev) [:arthuredelstein] from Bug 1404017)
> Another alternative would be to convert privacy.resistFingerprinting to an
> integer. Then the integer could have three values:
> 
> 0: Fingerprinting Resistance off
> 1: Fingerprinting Resistance on in all windows
> 2: Fingerprinting Resistance on in Private-Browsing windows only

+1 for this but with privacy.firstparty.isolate
Marking P3 for PBM backlog.

Also, AFAIK the only private-browsing-mode config option we have so far is privacy.trackingprotection.pbmode.enabled

We should stick to a consistent naming scheme for private-browsing-mode configs, so I propose:

privacy.firstparty.isolate.pbmode.enabled
Priority: -- → P3
(In reply to Luke Crouch [:groovecoder] from comment #8)
> Marking P3 for PBM backlog.
> 
> Also, AFAIK the only private-browsing-mode config option we have so far is
> privacy.trackingprotection.pbmode.enabled
> 
> We should stick to a consistent naming scheme for private-browsing-mode
> configs, so I propose:
> 
> privacy.firstparty.isolate.pbmode.enabled

Is there a way to turn tracking protection mode outside of pbmode? FPI and AntiFingerpriting both currently exist as boolean prefs for the whole browser. Making them ints expose 3 states, while making a new pref makes it four:

Disabled Everywhere
Disabled in Normal Browsing, Enabled in PBM
Enable in Normal Browsing, Disabled in PBM
Enabled Everywhere

I don't have a strong preference, just noting the state expansion implicit in making it a new pref.
For TP, privacy.trackingprotection.enabled=true overrides privacy.trackingprotection.pbmode.enabled.

So we have 3 states with the 2 prefs:

Disabled everywhere: enabled = false; pbmode.enabled = false
Disabled in Normal Browsing, Enabled in PBM: enabled = false; pbmode.enabled = true (this is the default state)
Enabled everywhere: enabled = true; pbmode.enabled = true|false (the global overrides the pbmode)
See Also: → 1312655
Comment on attachment 8963461 [details]
Bug 1397624 Refactor browser_firstPartyIsolation.js so it will be able to test bth FPI prefs

https://reviewboard.mozilla.org/r/232392/#review238026


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:135
(Diff revision 3)
>    await BrowserTestUtils.browserLoaded(tab.linkedBrowser, true, function(url) {
>      return url == BASE_URL + "dummy.html";
>    });
>  
> -  await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
> +  await ContentTask.spawn(tab.linkedBrowser, {shouldBeIsolated}, async function(attrs) {
> +    let assertFunc = function(b) { return attrs.shouldBeIsolated ? b : !b; };

Error: 'assertfunc' is assigned a value but never used. [eslint: no-unused-vars]

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:308
(Diff revision 3)
> +  await openWindow_test(gBrowser, true);
> +  await window_open_redirect_test(gBrowser, true);
> +  await window_open_iframe_test(gBrowser, true);
> +  await form_test(gBrowser, true);
> +  await window_open_form_test(gBrowser, true);
>  });

Error: Newline required at end of file but not found. [eslint: eol-last]
Comment on attachment 8963721 [details]
Bug 1397624 Add tests for privacy.firstparty.isolate.pbmode.enabled

https://reviewboard.mozilla.org/r/232618/#review238028


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:135
(Diff revision 1)
>    await BrowserTestUtils.browserLoaded(tab.linkedBrowser, true, function(url) {
>      return url == BASE_URL + "dummy.html";
>    });
>  
>    await ContentTask.spawn(tab.linkedBrowser, {shouldBeIsolated}, async function(attrs) {
>      let assertFunc = function(b) { return attrs.shouldBeIsolated ? b : !b; };

Error: 'assertfunc' is assigned a value but never used. [eslint: no-unused-vars]
Comment on attachment 8963460 [details]
Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only

https://reviewboard.mozilla.org/r/232390/#review242102

Overall, this looks good, but I have a few questions about what happens to old data / old principals when FPI is first turned on.  Also, I'm not a peer reviewer for most of this, so we will need secondary review.

::: docshell/base/nsDocShell.cpp:3218
(Diff revision 4)
>    // When the first party isolation is on, the top-level docShell may not have
>    // the firstPartyDomain in its originAttributes, but its document will have
>    // it. So we get the firstPartyDomain from the nodePrincipal of the document
>    // before we compare the originAttributes.
> -  if (OriginAttributes::IsFirstPartyEnabled()) {
> +  // [[Tanvi: I don't know if this assert is necessary, but I'm being cautious.]]
> +  MOZ_ASSERT(targetOA.mPrivateBrowsingId == accessingOA.mPrivateBrowsingId);

Not necessary, but couldn't hurt.  Could catch issues if pbmode origin attribute isn't getting set appropriately.

::: js/xpconnect/wrappers/AccessCheck.cpp:66
(Diff revision 4)
>  AccessCheck::subsumesConsideringDomain(JSCompartment* a, JSCompartment* b)
>  {
> -    MOZ_ASSERT(OriginAttributes::IsFirstPartyEnabled());
>      nsIPrincipal* aprin = GetCompartmentPrincipal(a);
>      nsIPrincipal* bprin = GetCompartmentPrincipal(b);
> +    // [[Tanvi: I know from testing that sometimes we will compare two principles that

What do you mean?  See my next comment.

::: js/xpconnect/wrappers/AccessCheck.cpp:78
(Diff revision 4)
>                                                    JSCompartment* b)
>  {
> -    MOZ_ASSERT(!OriginAttributes::IsFirstPartyEnabled());
>      nsIPrincipal* aprin = GetCompartmentPrincipal(a);
>      nsIPrincipal* bprin = GetCompartmentPrincipal(b);
> +    // [[Tanvi: Necessary?]]

From my understanding, when you enable First Party Isolation, you start fresh, both becasue none of your old cookies make any sense anymore and because your old cookies get wiped.  Is that correct?  Looking at the rest of this diff, it seems like the old principals/data may still be in use.  Once FPI is enabled, don't all principals have to have a FPI Origin Attribute set?

If so, if you set FPI in regular mode, and compare 2 principals, both will have FPI enabled.

And if you set FPI only in private mode, you are presumably only comparing principals in private mode, so again FPI will be enabled in both.

However, this check doesn't hurt, so I would keep it.

::: js/xpconnect/wrappers/WrapperFactory.cpp:143
(Diff revision 4)
>      // compartment boundary. In that case, we only want to preserve waivers
>      // in transactions between same-origin compartments.
>      JSCompartment* oldCompartment = js::GetObjectCompartment(originalObj);
>      JSCompartment* newCompartment = js::GetContextCompartment(cx);
> +
> +    nsIPrincipal* oldprin = GetCompartmentPrincipal(oldCompartment);

Although the changes in this file look fine from the surface, I'm not an appropriate reviewer for this file.

::: js/xpconnect/wrappers/WrapperFactory.cpp:465
(Diff revision 4)
>      bool targetIsChrome = AccessCheck::isChrome(target);
> -    bool originSubsumesTarget = OriginAttributes::IsFirstPartyEnabled() ?
> +
> +    nsIPrincipal* originprin = GetCompartmentPrincipal(origin);
> +    nsIPrincipal* targetprin = GetCompartmentPrincipal(target);
> +
> +    // This ensures we do a deep comparison even if only one of the principles as FPI enabled.

s/principles as/principals has/
Comment on attachment 8963720 [details]
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled

https://reviewboard.mozilla.org/r/232616/#review242104

Are you sure you want to remove this pref?  Did it help with single sign on or alleviate any breakage?

Few comments/questions in the review, but again we will need secondary review.

::: browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js:39
(Diff revision 2)
>      // Insert a cookie into this iframe.
>      childFrame.contentDocument.cookie = obj.cookieStr;
>  
>      // Open the tab here and focus on it.
>      let openedPath = obj.page;
> -    if (!obj.isPrefEnabled) {
> +    if (!obj.shouldRestrictAccess) {

Is there any case where shouldRestrictAccess would be false, since you are removing privacy.firstparty.isolate.restrict_opener_access and setting it to always be true.

::: browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js:40
(Diff revision 2)
>      childFrame.contentDocument.cookie = obj.cookieStr;
>  
>      // Open the tab here and focus on it.
>      let openedPath = obj.page;
> -    if (!obj.isPrefEnabled) {
> +    if (!obj.shouldRestrictAccess) {
>        // If the pref is not enabled, we pass the cookie value through the query string

update comment to remove reference to "pref"

::: browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html:10
(Diff revision 2)
> -    // If the query string is given, we are expecting the window.opener can be accessed
> -    // across different first party domains, so we will match the cookie value.
> +    // The access of window.opener should be treated as cross-origin.
> +    // Therefore, it should fail.
> -    // Otherwise, the access of window.opener should be treated as cross-origin.
> -    // Therefore, it should fail at this setting.
>      let openerRestriction = true;
> -    let cookieValue;
> +    document.title = "fail";

cookieValue still needs to be defined.

I'm not clear what this is testing now.
Comment on attachment 8963461 [details]
Bug 1397624 Refactor browser_firstPartyIsolation.js so it will be able to test bth FPI prefs

https://reviewboard.mozilla.org/r/232392/#review242110

r+ with the fix to error messages.

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:19
(Diff revision 4)
> -  await ContentTask.spawn(tab.linkedBrowser, { firstPartyDomain: BASE_DOMAIN }, async function(attrs) {
> +  await ContentTask.spawn(tab.linkedBrowser, { shouldBeIsolated, firstPartyDomain: BASE_DOMAIN }, async function(attrs) {
> +    let assertFunc = function(b) { return attrs.shouldBeIsolated ? b : !b; };
>      info("document principal: " + content.document.nodePrincipal.origin);
> -    Assert.equal(docShell.getOriginAttributes().firstPartyDomain, "",
> -                 "top-level docShell shouldn't have firstPartyDomain attribute.");
> -    Assert.equal(content.document.nodePrincipal.originAttributes.firstPartyDomain,
> +    Assert.ok(docShell.getOriginAttributes().firstPartyDomain == "",
> +                 "top-level docShell shouldn't have firstPartyDomain attribute under any circumstance.");
> +    Assert.ok(assertFunc(content.document.nodePrincipal.originAttributes.firstPartyDomain ==

The error messages need to be updated, since with the updated code, in some cases, you may not want isolation.

Assume shouldBeIsolated is false.  We call assertFunc from line 15.  Assume the input b = false (firstPartyDomain attributes aren't set properly).

Then we will be returning !b, which is true.  In this case, the document should not have firstPartyDomain.  Hence, the error message needs to be updated.

To help fix this in a way where the error messages make sense, you could put actual variable values into the error messages.

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:135
(Diff revision 4)
>    await BrowserTestUtils.browserLoaded(tab.linkedBrowser, true, function(url) {
>      return url == BASE_URL + "dummy.html";
>    });
>  
> -  await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
> +  await ContentTask.spawn(tab.linkedBrowser, {shouldBeIsolated}, async function(attrs) {
> +    let assertFunc = function(b) { return attrs.shouldBeIsolated ? b : !b; };

As reviewbot says, this assertFunc isn't used. In this case, whether we shouldBeIsolated or not, postmessage should work, right?  So maybe this test doesn't need changing?

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:224
(Diff revision 4)
> +    Assert.ok(assertFunc(docShell.getOriginAttributes().firstPartyDomain == attrs.firstPartyDomain),
>                   "window.open() should have firstPartyDomain attribute");
>  
>      // The document is http://example.com/browser/browser/components/originattributes/test/browser/test_firstParty.html
>      // so the firstPartyDomain will be overriden to 'example.com'.
> -    Assert.equal(content.document.nodePrincipal.originAttributes.firstPartyDomain,
> +    Assert.ok(assertFunc(content.document.nodePrincipal.originAttributes.firstPartyDomain ==

This and the following 2 error messages could be more descriptive to mention example.com by name.
Attachment #8963461 - Flags: review?(tanvi) → review+
Comment on attachment 8963721 [details]
Bug 1397624 Add tests for privacy.firstparty.isolate.pbmode.enabled

https://reviewboard.mozilla.org/r/232618/#review242114

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:35
(Diff revision 3)
>    });
>  
>    browser.removeTab(tab);
>  }
>  
>  /**

Add comment about private mode.

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:134
(Diff revision 3)
>    // So we wait until dummy.html is loaded
>    await BrowserTestUtils.browserLoaded(tab.linkedBrowser, true, function(url) {
>      return url == BASE_URL + "dummy.html";
>    });
>  
> -  await ContentTask.spawn(tab.linkedBrowser, {shouldBeIsolated}, async function(attrs) {
> +  await ContentTask.spawn(tab.linkedBrowser, {}, async function() {

looks like you cleaned up the issues in the previous patch with postmessage here.

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:327
(Diff revision 3)
> +  await window_open_redirect_test(gBrowser, false);
> +  await window_open_iframe_test(gBrowser, false);
> +  await form_test(gBrowser, false);
> +  await window_open_form_test(gBrowser, false);
> +
> +  Services.cookies.removeAll();

Do you need to call this before line 317 as well, when you switch from global isolation to no global isolation?  Why or why not?
Attachment #8963721 - Flags: review?(tanvi) → review+
Comment on attachment 8963720 [details]
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled

https://reviewboard.mozilla.org/r/232616/#review242270

::: browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js:39
(Diff revision 2)
>      // Insert a cookie into this iframe.
>      childFrame.contentDocument.cookie = obj.cookieStr;
>  
>      // Open the tab here and focus on it.
>      let openedPath = obj.page;
> -    if (!obj.isPrefEnabled) {
> +    if (!obj.shouldRestrictAccess) {

Yes. When FPI is disabled, we do not restrict operner access.

::: browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js:40
(Diff revision 2)
>      childFrame.contentDocument.cookie = obj.cookieStr;
>  
>      // Open the tab here and focus on it.
>      let openedPath = obj.page;
> -    if (!obj.isPrefEnabled) {
> +    if (!obj.shouldRestrictAccess) {
>        // If the pref is not enabled, we pass the cookie value through the query string

Now, 'pref' refers to privacy.firstparty.isolate so I think this is sill accurate.

::: browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html:10
(Diff revision 2)
> -    // If the query string is given, we are expecting the window.opener can be accessed
> -    // across different first party domains, so we will match the cookie value.
> +    // The access of window.opener should be treated as cross-origin.
> +    // Therefore, it should fail.
> -    // Otherwise, the access of window.opener should be treated as cross-origin.
> -    // Therefore, it should fail at this setting.
>      let openerRestriction = true;
> -    let cookieValue;
> +    document.title = "fail";

Hm, I'm not sure why this file is so wrong.  Maybe I didn't roll a commit? In any event, I left some stuff out that I fixed.
Comment on attachment 8963721 [details]
Bug 1397624 Add tests for privacy.firstparty.isolate.pbmode.enabled

https://reviewboard.mozilla.org/r/232618/#review242114

> Do you need to call this before line 317 as well, when you switch from global isolation to no global isolation?  Why or why not?

I do; it's called in resetup.
Comment on attachment 8963460 [details]
Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only

https://reviewboard.mozilla.org/r/232390/#review242102

> From my understanding, when you enable First Party Isolation, you start fresh, both becasue none of your old cookies make any sense anymore and because your old cookies get wiped.  Is that correct?  Looking at the rest of this diff, it seems like the old principals/data may still be in use.  Once FPI is enabled, don't all principals have to have a FPI Origin Attribute set?
> 
> If so, if you set FPI in regular mode, and compare 2 principals, both will have FPI enabled.
> 
> And if you set FPI only in private mode, you are presumably only comparing principals in private mode, so again FPI will be enabled in both.
> 
> However, this check doesn't hurt, so I would keep it.

No; It's my understanding tha FPI doesn't wipe any cookies. When you first enable FPI, you basically switch how you query the cookie jar, so there is nothing to be found when you start querying. When you turn FPI off, you go back to querying the old way, and you'll get all your original cookies. When you enable FPI again, you start querying int he FPI way, and you start getting all the cookies you had accumulated during the first time you had FPI enabled.

> you are presumably only comparing principals in private mode, so again FPI will be enabled in both.

I believe a comparison occurs between a principal from non-private mode and a principal from private mode. And therefore FPI may only be enabled in one of those principals.
I addressed all of the comments except those stemming from the discussion in Comment 35, which I think we need to sort out.

For rebasing purposes, I addressed the comments on the third patch (like fixing the error messages) in the fourth patch.
Comment on attachment 8963461 [details]
Bug 1397624 Refactor browser_firstPartyIsolation.js so it will be able to test bth FPI prefs

https://reviewboard.mozilla.org/r/232392/#review242296


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:135
(Diff revision 5)
>    await BrowserTestUtils.browserLoaded(tab.linkedBrowser, true, function(url) {
>      return url == BASE_URL + "dummy.html";
>    });
>  
> -  await ContentTask.spawn(tab.linkedBrowser, {}, async function() {
> +  await ContentTask.spawn(tab.linkedBrowser, {shouldBeIsolated}, async function(attrs) {
> +    let assertFunc = function(b) { return attrs.shouldBeIsolated ? b : !b; };

Error: 'assertfunc' is assigned a value but never used. [eslint: no-unused-vars]

::: browser/components/originattributes/test/browser/browser_firstPartyIsolation.js:308
(Diff revision 5)
> +  await openWindow_test(gBrowser, true);
> +  await window_open_redirect_test(gBrowser, true);
> +  await window_open_iframe_test(gBrowser, true);
> +  await form_test(gBrowser, true);
> +  await window_open_form_test(gBrowser, true);
>  });

Error: Newline required at end of file but not found. [eslint: eol-last]
Comment on attachment 8963460 [details]
Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only

https://reviewboard.mozilla.org/r/232390/#review242102

> No; It's my understanding tha FPI doesn't wipe any cookies. When you first enable FPI, you basically switch how you query the cookie jar, so there is nothing to be found when you start querying. When you turn FPI off, you go back to querying the old way, and you'll get all your original cookies. When you enable FPI again, you start querying int he FPI way, and you start getting all the cookies you had accumulated during the first time you had FPI enabled.
> 
> > you are presumably only comparing principals in private mode, so again FPI will be enabled in both.
> 
> I believe a comparison occurs between a principal from non-private mode and a principal from private mode. And therefore FPI may only be enabled in one of those principals.

The only time I can think of where we could compare two principals for equality from non-private mode and private mode and want the result to be true even when all origin attributes (ex: pbmode and fpi) doesn't match is for permissions and password manager.  I'm not sure if/how we share permissions across regular mode and private mode.  And I'm not sure how we fill or save passwords in private mode.

Anyway, I would say to err on the safer side (include the asserts, return false).  Do a few manual tests on passwords and permissions in regular/private mode when FPI is set to true in private mode.  And then catch any edge cases we haven't thought of in nightly when returning false causes an issue or an assert fails.

> s/principles as/principals has/

"principals has" instead of "principals as"
Comment on attachment 8963460 [details]
Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only

Please get a second peer reviewer for this change, particularly js/xpconnect/wrappers/WrapperFactory.cpp.
Flags: needinfo?(tanvi)
Attachment #8963460 - Flags: review?(tanvi) → review+
Comment on attachment 8963720 [details]
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled

https://reviewboard.mozilla.org/r/232616/#review242270

> Now, 'pref' refers to privacy.firstparty.isolate so I think this is sill accurate.

Okay, then please be more specific.  "If the pref privacy.firstparty.isolate is not enabled,..."
Comment on attachment 8963720 [details]
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled

https://reviewboard.mozilla.org/r/232616/#review244360

::: browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js:2
(Diff revision 3)
>  /**
>   * Bug 1339336 - A test case for testing pref 'privacy.firstparty.isolate.restrict_opener_access'

Update the comment

::: browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html:13
(Diff revision 3)
> -    // Therefore, it should fail at this setting.
>      let openerRestriction = true;
>      let cookieValue;
>      if (window.location.search.length > 0) {
>        cookieValue = window.location.search.substr(1);
>        openerRestriction = false;

Why do we set the openerRestriction to false here? Do we need to bring back the previous comment about when the query string is given?

This test needs a little more explanation.
Comment on attachment 8963720 [details]
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled

Hi Tom, please respond to the comments and then reflag.  Thanks!
Attachment #8963720 - Flags: review?(tanvi)
I am a bit curious now: A few days before with Firefox 62.0.3 and permanent private browsing mode I tested the functionality of first party isolation via the same embedded YouTube video on different websites by changing its volume and checking if the setting is remembered across websites with privacy.firstparty.isolate set to false and true.

If privacy.firstparty.isolate was set to false the volume was remembered across different websites while I could never reproduce this with privacy.firstparty.isolate set to true (the volume was always on its default setting).


Then I did read somewhere that first party isolation does not work with private browsing and found this bug report. But my observations conflict with this. Did Firefox get some other changes meanwhile that improved this behavior? Or does first party isolation just work partially with the current Firefox implementation or is it eventually even just more aggressive than in non - private browsing?
(In reply to sworddragon2 from comment #47)
> Then I did read somewhere that first party isolation does not work with
> private browsing and found this bug report. But my observations conflict
> with this. Did Firefox get some other changes meanwhile that improved this
> behavior? Or does first party isolation just work partially with the current
> Firefox implementation or is it eventually even just more aggressive than in
> non - private browsing?

FPI should behave the same way between Private Browsing and not. Private Browsing has no effect on FPI (right now). (It will have no existing state when you open it; and it wipe your cookies when you close a private window of course.) You see a strange behavior with this; feel free to file a new bug and cc me.
Assignee: tom → nobody
Priority: P2 → P3

Firefox provides now State Partitioning ( https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Privacy/State_Partitioning ) which includes:

  • Network Partitioning to isolate storages that are not intended to store specific data (e.g. Caches; and thus might be used for tracking) which should be enabled at default via privacy.partition.network_state = true if I'm not wrong.

  • Dynamic State Partitioning to isolate all storages that are intended to store data (e.g. Cookies and localStorage) which can be enabled by setting network.cookie.cookieBehavior = 5 (also exposed via the options GUI). There are heuristics that allow access out if this partitioned sandbox to presumably avoid the breakages that the old first party isolation ( privacy.firstparty.isolate = true ) caused (e.g. paying on the PayPal popup called by the Steam storesite not working) which can in theory also be abused for tracking. But there are options to disable these heuristics (privacy.restrict3rdpartystorage.heuristic.opened_window_after_interaction, privacy.restrict3rdpartystorage.heuristic.recently_visited, privacy.restrict3rdpartystorage.heuristic.redirect, privacy.restrict3rdpartystorage.heuristic.window_open; presumably all set to true to enable the heuristics (the default) and to false to disable them - but the linked article isn't clear about that).

I have now disabled the old first party isolation by reverting my settings for it (setting privacy.firstparty.isolate and privacy.firstparty.isolate.block_post_message both to false) and enabled both State Partitions (by setting network.cookie.cookieBehavior = 5 via the GUI; the Network Partitioning via privacy.partition.network_state was already set to true as expected). I also did set all 4 heuristic settings mentioned above to false to allow no tracking exceptions (and assuming e.g. the above mentioned PayPal window and similar things break again until I temporary enabled the related heuristic settings again).

Since in my private browsing setup Network Partitioning was enabled at default and Dynamic State Partitioning was provided via the GUI I assume this will work reliable and official in (permanent) private browsing mode. Doesn't that mean this ticket is in theory outdated and can be closed or is there still something missing (e.g. what happens actually now if Cookies get fully disabled in the options GUI? I guess it simply disables read/write access to cookies and all other items that fall under Dynamic Storage Partitioning too like localStorage. But what happens if an user disables all cookies that way but enables them only for specific sites? Will they be double-paired or is third-party read/write access denied? Also I guess with the old FPI implementation an user could disable all third-party cookies while having FPI active while with the new FPI implementation third-party cookies should always work - but that should not be an issue as they should never be able to track (besides the heuristic-exceptions if enabled). And disabling cookies for specific sites (e.g. Google Services and other sites to disable their cookie/GDPR dialogues) while Dynamic Storage Partitioning is enabled works straightforward as well)?

I guess if userContext gets also ready in the future for (permanent) private browsing mode this would now be very good as for example I would not have to always restart the browser again to avoid mixing up stuff (e.g. Gmail and the Google Search) which requires me to always setup again some site-specific settings once I visit them (e.g. disabling Cookies on Google Services for the reason mentioned above). But I guess I should test it myself and create a ticket for that if needed (if it not already exists).

Flags: needinfo?(tom)

Doesn't that mean this ticket is in theory outdated and can be closed or is there still something missing

It probably can be closed in favor of Bug 1649876 which would switch our FPI implementation over to dFPI; and bring along with it the ability to enable it selectively in PBM. However until we've been able to do that, and also until Tor has been able to audit dFPI to the point where they're comfortable switching over to it (e.g. there isn't some heuristic enabled by default that can't be disabled) I'd like to keep this open.

Flags: needinfo?(tom)
See Also: → 1649876

Note that we will be shipping dFPI in Private Browsing in 89, if all goes to plan (bug 1698810). I don't really see a point in keeping this bug since I'm confident we will never implement FPI in PBM.

You need to log in before you can comment on or make changes to this bug.