Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled r?tanvi draft
authorTom Ritter <tom@mozilla.com>
Thu, 29 Mar 2018 11:13:12 -0500
changeset 781815 ced00e3a921574e911fab6d1274ba0e57a1dc172
parent 781810 2243b83d2c5ca7a13b592e0fed4639657a2882f1
child 781816 0a6845ce3b32517a3324b556bee427b7422ceef3
push id106421
push userbmo:tom@mozilla.com
push dateFri, 13 Apr 2018 18:10:23 +0000
reviewerstanvi
bugs1397624
milestone61.0a1
Bug 1397624 Remove privacy.firstparty.isolate.restrict_opener_access and act as if it were always enabled r?tanvi MozReview-Commit-ID: 8qYAIli90zm
browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js
browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html
caps/OriginAttributes.cpp
caps/OriginAttributes.h
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
modules/libpref/init/all.js
--- a/browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js
+++ b/browser/components/originattributes/test/browser/browser_windowOpenerRestriction.js
@@ -6,42 +6,42 @@ const CC = Components.Constructor;
 
 const FIRST_PARTY_OPENER = "example.com";
 const FIRST_PARTY_TARGET = "example.org";
 const OPENER_PAGE = "http://" + FIRST_PARTY_OPENER + "/browser/browser/components/" +
                     "originattributes/test/browser/file_windowOpenerRestriction.html";
 const TARGET_PAGE = "http://" + FIRST_PARTY_TARGET + "/browser/browser/components/" +
                     "originattributes/test/browser/file_windowOpenerRestrictionTarget.html";
 
-async function testPref(aIsPrefEnabled) {
+async function testPref(aShouldRestrictAccess) {
   // Use a random key so we don't access it in later tests.
   let cookieStr = "key" + Math.random().toString() + "=" + Math.random().toString();
 
   // Open the tab for the opener page.
   let tab = BrowserTestUtils.addTab(gBrowser, OPENER_PAGE);
 
   // Select this tab and make sure its browser is loaded and focused.
   gBrowser.selectedTab = tab;
   tab.ownerGlobal.focus();
 
   let browser = gBrowser.getBrowserForTab(tab);
   await BrowserTestUtils.browserLoaded(browser);
 
   await ContentTask.spawn(browser, {cookieStr,
                                     page: TARGET_PAGE,
-                                    isPrefEnabled: aIsPrefEnabled}, async function(obj) {
+                                    shouldRestrictAccess: aShouldRestrictAccess}, async function(obj) {
     // Acquire the iframe element.
     let childFrame = content.document.getElementById("child");
 
     // 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) {
       // If the pref is not enabled, we pass the cookie value through the query string
       // to tell the target page that it should check the cookie value.
       openedPath += "?" + obj.cookieStr;
     }
 
     // Issue the opener page to open the target page and focus on it.
     this.openedWindow = content.open(openedPath);
     this.openedWindow.focus();
@@ -60,39 +60,24 @@ async function testPref(aIsPrefEnabled) 
   });
   BrowserTestUtils.removeTab(tab);
 
   // Reset cookies
   Services.cookies.removeAll();
 }
 
 add_task(async function runTests() {
-  let tests = [true, false];
-
   // First, we test the scenario that the first party isolation is enabled.
   await SpecialPowers.pushPrefEnv({"set":
     [["privacy.firstparty.isolate", true]]
   });
-
-  for (let enabled of tests) {
-    await SpecialPowers.pushPrefEnv({"set":
-      [["privacy.firstparty.isolate.restrict_opener_access", enabled]]
-    });
-
-    await testPref(enabled);
-  }
+  await testPref(true);
 
   // Second, we test the scenario that the first party isolation is disabled.
   await SpecialPowers.pushPrefEnv({"set":
     [["privacy.firstparty.isolate", false]]
   });
 
-  for (let enabled of tests) {
-    await SpecialPowers.pushPrefEnv({"set":
-      [["privacy.firstparty.isolate.restrict_opener_access", enabled]]
-    });
-
-    // When first party isolation is disabled, this pref will not affect the behavior of
-    // window.opener. And the correct behavior here is to allow access since the iframe in
-    // the opener page has the same origin with the target page.
-    await testPref(false);
-  }
+  // When first party isolation is disabled, this pref will not affect the behavior of
+  // window.opener. And the correct behavior here is to allow access since the iframe in
+  // the opener page has the same origin with the target page.
+  await testPref(false);
 });
--- a/browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html
+++ b/browser/components/originattributes/test/browser/file_windowOpenerRestrictionTarget.html
@@ -1,29 +1,29 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <meta http-equiv="content-type" content="text/html; charset=utf-8">
   <title>title not set</title>
   <script>
-    // 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.
-    // Otherwise, the access of window.opener should be treated as cross-origin.
-    // Therefore, it should fail at this setting.
+    // The access of window.opener should be treated as cross-origin.
+    // Therefore, it should fail.
     let openerRestriction = true;
     let cookieValue;
     if (window.location.search.length > 0) {
       cookieValue = window.location.search.substr(1);
       openerRestriction = false;
     }
+    document.title = "fail";
 
     try {
       let openerFrame = window.opener.frames.child;
       let result = openerFrame.document.cookie === cookieValue;
-      if (result && !openerRestriction) {
+
+      if (!openerRestriction && result) {
         document.title = "pass";
       }
     } catch (e) {
       if (openerRestriction) {
         document.title = "pass";
       }
     }
   </script>
--- a/caps/OriginAttributes.cpp
+++ b/caps/OriginAttributes.cpp
@@ -12,29 +12,26 @@
 #include "nsIURI.h"
 #include "nsIURIWithPrincipal.h"
 
 namespace mozilla {
 
 using dom::URLParams;
 
 bool OriginAttributes::sFirstPartyIsolation = false;
-bool OriginAttributes::sRestrictedOpenerAccess = false;
 
 void
 OriginAttributes::InitPrefs()
 {
   MOZ_ASSERT(NS_IsMainThread());
   static bool sInited = false;
   if (!sInited) {
     sInited = true;
     Preferences::AddBoolVarCache(&sFirstPartyIsolation,
                                  "privacy.firstparty.isolate");
-    Preferences::AddBoolVarCache(&sRestrictedOpenerAccess,
-                                 "privacy.firstparty.isolate.restrict_opener_access");
   }
 }
 
 void
 OriginAttributes::SetFirstPartyDomain(const bool aIsTopLevelDocument,
                                       nsIURI* aURI)
 {
   bool isFirstPartyEnabled = IsFirstPartyEnabled();
--- a/caps/OriginAttributes.h
+++ b/caps/OriginAttributes.h
@@ -81,35 +81,24 @@ public:
   void SyncAttributesWithPrivateBrowsing(bool aInPrivateBrowsing);
 
   // check if "privacy.firstparty.isolate" is enabled.
   static inline bool IsFirstPartyEnabled()
   {
     return sFirstPartyIsolation;
   }
 
-  // check if the access of window.opener across different FPDs is restricted.
-  // We only restrict the access of window.opener when first party isolation
-  // is enabled and "privacy.firstparty.isolate.restrict_opener_access" is on.
-  static inline bool IsRestrictOpenerAccessForFPI()
-  {
-    // We always want to restrict window.opener if first party isolation is
-    // disabled.
-    return !sFirstPartyIsolation || sRestrictedOpenerAccess;
-  }
-
   // returns true if the originAttributes suffix has mPrivateBrowsingId value
   // different than 0.
   static bool IsPrivateBrowsing(const nsACString& aOrigin);
 
   static void InitPrefs();
 
 private:
   static bool sFirstPartyIsolation;
-  static bool sRestrictedOpenerAccess;
 };
 
 class OriginAttributesPattern : public dom::OriginAttributesPatternDictionary
 {
 public:
   // To convert a JSON string to an OriginAttributesPattern, do the following:
   //
   // OriginAttributesPattern pattern;
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -56,27 +56,27 @@ AccessCheck::subsumes(JSObject* a, JSObj
 {
     return subsumes(js::GetObjectCompartment(a), js::GetObjectCompartment(b));
 }
 
 // Same as above, but considering document.domain.
 bool
 AccessCheck::subsumesConsideringDomain(JSCompartment* a, JSCompartment* b)
 {
-    MOZ_ASSERT(OriginAttributes::IsRestrictOpenerAccessForFPI());
+    MOZ_ASSERT(OriginAttributes::IsFirstPartyEnabled());
     nsIPrincipal* aprin = GetCompartmentPrincipal(a);
     nsIPrincipal* bprin = GetCompartmentPrincipal(b);
     return BasePrincipal::Cast(aprin)->FastSubsumesConsideringDomain(bprin);
 }
 
 bool
 AccessCheck::subsumesConsideringDomainIgnoringFPD(JSCompartment* a,
                                                   JSCompartment* b)
 {
-    MOZ_ASSERT(!OriginAttributes::IsRestrictOpenerAccessForFPI());
+    MOZ_ASSERT(!OriginAttributes::IsFirstPartyEnabled());
     nsIPrincipal* aprin = GetCompartmentPrincipal(a);
     nsIPrincipal* bprin = GetCompartmentPrincipal(b);
     return BasePrincipal::Cast(aprin)->FastSubsumesConsideringDomainIgnoringFPD(bprin);
 }
 
 // Does the compartment of the wrapper subsumes the compartment of the wrappee?
 bool
 AccessCheck::wrapperSubsumes(JSObject* wrapper)
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -135,17 +135,17 @@ ShouldWaiveXray(JSContext* cx, JSObject*
         return true;
 
     // Otherwise, this is a case of explicitly passing a wrapper across a
     // 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);
     bool sameOrigin = false;
-    if (OriginAttributes::IsRestrictOpenerAccessForFPI()) {
+    if (OriginAttributes::IsFirstPartyEnabled()) {
         sameOrigin =
             AccessCheck::subsumesConsideringDomain(oldCompartment, newCompartment) &&
             AccessCheck::subsumesConsideringDomain(newCompartment, oldCompartment);
     } else {
         sameOrigin =
             AccessCheck::subsumesConsideringDomainIgnoringFPD(oldCompartment, newCompartment) &&
             AccessCheck::subsumesConsideringDomainIgnoringFPD(newCompartment, oldCompartment);
     }
@@ -365,17 +365,17 @@ DEBUG_CheckUnwrapSafety(HandleObject obj
         MOZ_ASSERT(!handler->hasSecurityPolicy());
     } else if (CompartmentPrivate::Get(origin)->forcePermissiveCOWs) {
         // Similarly, if this is a privileged scope that has opted to make itself
         // accessible to the world (allowed only during automation), unwrap should
         // be allowed.
         MOZ_ASSERT(!handler->hasSecurityPolicy());
     } else {
         // Otherwise, it should depend on whether the target subsumes the origin.
-        MOZ_ASSERT(handler->hasSecurityPolicy() == !(OriginAttributes::IsRestrictOpenerAccessForFPI() ?
+        MOZ_ASSERT(handler->hasSecurityPolicy() == !(OriginAttributes::IsFirstPartyEnabled() ?
                                                        AccessCheck::subsumesConsideringDomain(target, origin) :
                                                        AccessCheck::subsumesConsideringDomainIgnoringFPD(target, origin)));
     }
 }
 #else
 #define DEBUG_CheckUnwrapSafety(obj, handler, origin, target) {}
 #endif
 
@@ -439,20 +439,20 @@ WrapperFactory::Rewrap(JSContext* cx, Ha
     MOZ_ASSERT(!js::IsWindow(obj));
     MOZ_ASSERT(dom::IsJSAPIActive());
 
     // Compute the information we need to select the right wrapper.
     JSCompartment* origin = js::GetObjectCompartment(obj);
     JSCompartment* target = js::GetContextCompartment(cx);
     bool originIsChrome = AccessCheck::isChrome(origin);
     bool targetIsChrome = AccessCheck::isChrome(target);
-    bool originSubsumesTarget = OriginAttributes::IsRestrictOpenerAccessForFPI() ?
+    bool originSubsumesTarget = OriginAttributes::IsFirstPartyEnabled() ?
                                   AccessCheck::subsumesConsideringDomain(origin, target) :
                                   AccessCheck::subsumesConsideringDomainIgnoringFPD(origin, target);
-    bool targetSubsumesOrigin = OriginAttributes::IsRestrictOpenerAccessForFPI() ?
+    bool targetSubsumesOrigin = OriginAttributes::IsFirstPartyEnabled() ?
                                   AccessCheck::subsumesConsideringDomain(target, origin) :
                                   AccessCheck::subsumesConsideringDomainIgnoringFPD(target, origin);
     bool sameOrigin = targetSubsumesOrigin && originSubsumesTarget;
 
     const Wrapper* wrapper;
 
     CompartmentPrivate* originCompartmentPrivate =
       CompartmentPrivate::Get(origin);
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1338,20 +1338,16 @@ pref("privacy.permissionPrompts.showClos
 // Enforce tracking protection in all modes
 pref("privacy.trackingprotection.enabled",  false);
 // Enforce tracking protection in Private Browsing mode
 pref("privacy.trackingprotection.pbmode.enabled",  true);
 // Annotate channels based on the tracking protection list in all modes
 pref("privacy.trackingprotection.annotate_channels",  true);
 // First Party Isolation (double keying), disabled by default
 pref("privacy.firstparty.isolate",                        false);
-// If false, two windows in the same domain with different first party domains
-// (top level URLs) can access resources through window.opener.
-// This pref is effective only when "privacy.firstparty.isolate" is true.
-pref("privacy.firstparty.isolate.restrict_opener_access", true);
 // Anti-fingerprinting, disabled by default
 pref("privacy.resistFingerprinting", false);
 // We automatically decline canvas permission requests if they are not initiated
 // from user input. Just in case that breaks something, we allow the user to revert
 // this behaior with this obscure pref. We do not intend to support this long term.
 // If you do set it, to work around some broken website, please file a bug with
 // information so we can understand why it is needed.
 pref("privacy.resistFingerprinting.autoDeclineNoUserInputCanvasPrompts", true);