Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only r?tanvi draft
authorTom Ritter <tom@mozilla.com>
Thu, 29 Mar 2018 13:30:08 -0500
changeset 781816 0a6845ce3b32517a3324b556bee427b7422ceef3
parent 781815 ced00e3a921574e911fab6d1274ba0e57a1dc172
child 781817 e9cbcaa2f44c6c2c5171497c116dc4fbcb3126e6
push id106421
push userbmo:tom@mozilla.com
push dateFri, 13 Apr 2018 18:10:23 +0000
reviewerstanvi
bugs1397624
milestone61.0a1
Bug 1397624 Add a new privacy.firstparty.isolate.pbmode.enabled pref to enable FPI in PBM only r?tanvi MozReview-Commit-ID: CR0dhrdeKyr
caps/OriginAttributes.cpp
caps/OriginAttributes.h
docshell/base/nsDocShell.cpp
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/WrapperFactory.cpp
modules/libpref/init/all.js
toolkit/components/extensions/parent/ext-cookies.js
--- a/caps/OriginAttributes.cpp
+++ b/caps/OriginAttributes.cpp
@@ -11,35 +11,38 @@
 #include "nsIEffectiveTLDService.h"
 #include "nsIURI.h"
 #include "nsIURIWithPrincipal.h"
 
 namespace mozilla {
 
 using dom::URLParams;
 
-bool OriginAttributes::sFirstPartyIsolation = false;
+bool OriginAttributes::sFirstPartyIsolationEverywhere = false;
+bool OriginAttributes::sFirstPartyIsolationPBModeOnly = false;
 
 void
 OriginAttributes::InitPrefs()
 {
   MOZ_ASSERT(NS_IsMainThread());
   static bool sInited = false;
   if (!sInited) {
     sInited = true;
-    Preferences::AddBoolVarCache(&sFirstPartyIsolation,
+    Preferences::AddBoolVarCache(&sFirstPartyIsolationEverywhere,
                                  "privacy.firstparty.isolate");
+    Preferences::AddBoolVarCache(&sFirstPartyIsolationPBModeOnly,
+                                 "privacy.firstparty.isolate.pbmode.enabled");
   }
 }
 
 void
 OriginAttributes::SetFirstPartyDomain(const bool aIsTopLevelDocument,
                                       nsIURI* aURI)
 {
-  bool isFirstPartyEnabled = IsFirstPartyEnabled();
+  bool isFirstPartyEnabled = IsFirstPartyEnabled(*this);
 
   // If the pref is off or this is not a top level load, bail out.
   if (!isFirstPartyEnabled || !aIsTopLevelDocument) {
     return;
   }
 
   nsCOMPtr<nsIEffectiveTLDService> tldService =
     do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID);
@@ -74,17 +77,17 @@ OriginAttributes::SetFirstPartyDomain(co
     }
   }
 }
 
 void
 OriginAttributes::SetFirstPartyDomain(const bool aIsTopLevelDocument,
                                       const nsACString& aDomain)
 {
-  bool isFirstPartyEnabled = IsFirstPartyEnabled();
+  bool isFirstPartyEnabled = IsFirstPartyEnabled(*this);
 
   // If the pref is off or this is not a top level load, bail out.
   if (!isFirstPartyEnabled || !aIsTopLevelDocument) {
     return;
   }
 
   mFirstPartyDomain = NS_ConvertUTF8toUTF16(aDomain);
 }
--- a/caps/OriginAttributes.h
+++ b/caps/OriginAttributes.h
@@ -75,30 +75,32 @@ public:
   // |uri!key1=value1&key2=value2| and returns the uri without the suffix.
   MOZ_MUST_USE bool PopulateFromOrigin(const nsACString& aOrigin,
                                        nsACString& aOriginNoSuffix);
 
   // Helper function to match mIsPrivateBrowsing to existing private browsing
   // flags. Once all other flags are removed, this can be removed too.
   void SyncAttributesWithPrivateBrowsing(bool aInPrivateBrowsing);
 
-  // check if "privacy.firstparty.isolate" is enabled.
-  static inline bool IsFirstPartyEnabled()
+  // Check if "privacy.firstparty.isolate" or "privacy.firstparty.isolate.pbmode.enabled" is set.
+  static inline bool IsFirstPartyEnabled(OriginAttributes attrs)
   {
-    return sFirstPartyIsolation;
+    return sFirstPartyIsolationEverywhere ||
+      (attrs.mPrivateBrowsingId > 0 && sFirstPartyIsolationPBModeOnly);
   }
 
   // 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 sFirstPartyIsolationEverywhere;
+  static bool sFirstPartyIsolationPBModeOnly;
 };
 
 class OriginAttributesPattern : public dom::OriginAttributesPatternDictionary
 {
 public:
   // To convert a JSON string to an OriginAttributesPattern, do the following:
   //
   // OriginAttributesPattern pattern;
old mode 100644
new mode 100755
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3171,17 +3171,18 @@ nsDocShell::CanAccessItem(nsIDocShellTre
     static_cast<nsDocShell*>(targetDS.get())->GetOriginAttributes();
   OriginAttributes accessingOA =
     static_cast<nsDocShell*>(accessingDS.get())->GetOriginAttributes();
 
   // 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()) {
+  MOZ_ASSERT(targetOA.mPrivateBrowsingId == accessingOA.mPrivateBrowsingId);
+  if (OriginAttributes::IsFirstPartyEnabled(targetOA)) {
     if (aAccessingItem->ItemType() == nsIDocShellTreeItem::typeContent &&
         (accessingDS == accessingRootDS || accessingDS->GetIsMozBrowser())) {
 
       nsCOMPtr<nsIDocument> accessingDoc = aAccessingItem->GetDocument();
 
       if (accessingDoc) {
         nsCOMPtr<nsIPrincipal> accessingPrincipal = accessingDoc->NodePrincipal();
 
@@ -10606,17 +10607,17 @@ nsDocShell::DoURILoad(nsIURI* aURI,
 
   // Inherit origin attributes from aPrincipalToInherit if inheritAttrs is true.
   // Otherwise we just use the origin attributes from docshell.
   if (inheritAttrs) {
     MOZ_ASSERT(aPrincipalToInherit, "We should have aPrincipalToInherit here.");
     attrs = aPrincipalToInherit->OriginAttributesRef();
     // If firstPartyIsolation is not enabled, then PrincipalToInherit should
     // have the same origin attributes with docshell.
-    MOZ_ASSERT_IF(!OriginAttributes::IsFirstPartyEnabled(), attrs == GetOriginAttributes());
+    MOZ_ASSERT_IF(!OriginAttributes::IsFirstPartyEnabled(attrs), attrs == GetOriginAttributes());
   } else {
     attrs = GetOriginAttributes();
     attrs.SetFirstPartyDomain(isTopLevelDoc, aURI);
   }
 
   rv = loadInfo->SetOriginAttributes(attrs);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -52,33 +52,37 @@ AccessCheck::subsumes(JSCompartment* a, 
 }
 
 bool
 AccessCheck::subsumes(JSObject* a, JSObject* b)
 {
     return subsumes(js::GetObjectCompartment(a), js::GetObjectCompartment(b));
 }
 
-// Same as above, but considering document.domain.
+// Same as above, but considering document.domain for First Party Isolation purposes.
 bool
 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
+    //   don't have the same FPI setting. But when that happens I believe this should
+    //   return false]]
     return BasePrincipal::Cast(aprin)->FastSubsumesConsideringDomain(bprin);
 }
 
 bool
 AccessCheck::subsumesConsideringDomainIgnoringFPD(JSCompartment* a,
                                                   JSCompartment* b)
 {
-    MOZ_ASSERT(!OriginAttributes::IsFirstPartyEnabled());
     nsIPrincipal* aprin = GetCompartmentPrincipal(a);
     nsIPrincipal* bprin = GetCompartmentPrincipal(b);
+    // [[Tanvi: Necessary?]]
+    MOZ_ASSERT(!OriginAttributes::IsFirstPartyEnabled(aprin->OriginAttributesRef()) &&
+        !OriginAttributes::IsFirstPartyEnabled(bprin->OriginAttributesRef()));
     return BasePrincipal::Cast(aprin)->FastSubsumesConsideringDomainIgnoringFPD(bprin);
 }
 
 // Does the compartment of the wrapper subsumes the compartment of the wrappee?
 bool
 AccessCheck::wrapperSubsumes(JSObject* wrapper)
 {
     MOZ_ASSERT(js::IsWrapper(wrapper));
--- a/js/xpconnect/wrappers/WrapperFactory.cpp
+++ b/js/xpconnect/wrappers/WrapperFactory.cpp
@@ -134,18 +134,27 @@ ShouldWaiveXray(JSContext* cx, JSObject*
     if (!(flags & Wrapper::CROSS_COMPARTMENT))
         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);
+
+    nsIPrincipal* oldprin = GetCompartmentPrincipal(oldCompartment);
+    nsIPrincipal* newprin = GetCompartmentPrincipal(newCompartment);
+    bool firstPartyIsolationMatches = OriginAttributes::IsFirstPartyEnabled(oldprin->OriginAttributesRef()) ==
+      OriginAttributes::IsFirstPartyEnabled(newprin->OriginAttributesRef());
+    if (!firstPartyIsolationMatches) {
+        return false;
+    }
+
     bool sameOrigin = false;
-    if (OriginAttributes::IsFirstPartyEnabled()) {
+    if (OriginAttributes::IsFirstPartyEnabled(oldprin->OriginAttributesRef())) {
         sameOrigin =
             AccessCheck::subsumesConsideringDomain(oldCompartment, newCompartment) &&
             AccessCheck::subsumesConsideringDomain(newCompartment, oldCompartment);
     } else {
         sameOrigin =
             AccessCheck::subsumesConsideringDomainIgnoringFPD(oldCompartment, newCompartment) &&
             AccessCheck::subsumesConsideringDomainIgnoringFPD(newCompartment, oldCompartment);
     }
@@ -364,18 +373,23 @@ DEBUG_CheckUnwrapSafety(HandleObject obj
         // If the caller is chrome (or effectively so), unwrap should always be allowed.
         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 {
+        nsIPrincipal* originprin = GetCompartmentPrincipal(origin);
+        nsIPrincipal* targetprin = GetCompartmentPrincipal(target);
+        bool deepCompare = OriginAttributes::IsFirstPartyEnabled(originprin->OriginAttributesRef()) ||
+          OriginAttributes::IsFirstPartyEnabled(targetprin->OriginAttributesRef());
+
         // Otherwise, it should depend on whether the target subsumes the origin.
-        MOZ_ASSERT(handler->hasSecurityPolicy() == !(OriginAttributes::IsFirstPartyEnabled() ?
+        MOZ_ASSERT(handler->hasSecurityPolicy() == !(deepCompare ?
                                                        AccessCheck::subsumesConsideringDomain(target, origin) :
                                                        AccessCheck::subsumesConsideringDomainIgnoringFPD(target, origin)));
     }
 }
 #else
 #define DEBUG_CheckUnwrapSafety(obj, handler, origin, target) {}
 #endif
 
@@ -439,20 +453,29 @@ 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::IsFirstPartyEnabled() ?
+
+    nsIPrincipal* originprin = GetCompartmentPrincipal(origin);
+    nsIPrincipal* targetprin = GetCompartmentPrincipal(target);
+
+    // This ensures we do a deep comparison even if only one of the principals as FPI enabled.
+    // That way a domain string will be compared to an empty one, and it will fail.
+    bool firstPartyIsolation = OriginAttributes::IsFirstPartyEnabled(originprin->OriginAttributesRef()) ||
+      OriginAttributes::IsFirstPartyEnabled(targetprin->OriginAttributesRef());
+
+    bool originSubsumesTarget = firstPartyIsolation ?
                                   AccessCheck::subsumesConsideringDomain(origin, target) :
                                   AccessCheck::subsumesConsideringDomainIgnoringFPD(origin, target);
-    bool targetSubsumesOrigin = OriginAttributes::IsFirstPartyEnabled() ?
+    bool targetSubsumesOrigin = firstPartyIsolation ?
                                   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
@@ -1337,17 +1337,19 @@ pref("privacy.donottrackheader.enabled",
 pref("privacy.permissionPrompts.showCloseButton", false);
 // 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);
+pref("privacy.firstparty.isolate", false);
+// Enable First Party Isolation in Private Browsing Mode
+pref("privacy.firstparty.isolate.pbmode.enabled",  false);
 // 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);
--- a/toolkit/components/extensions/parent/ext-cookies.js
+++ b/toolkit/components/extensions/parent/ext-cookies.js
@@ -273,16 +273,19 @@ const query = function* (detailsIn, prop
 
 const normalizeFirstPartyDomain = (details) => {
   if (details.firstPartyDomain != null) {
     return;
   }
   if (Services.prefs.getBoolPref("privacy.firstparty.isolate")) {
     throw new ExtensionError("First-Party Isolation is enabled, but the required 'firstPartyDomain' attribute was not set.");
   }
+  if (isPrivateCookieStoreId(details.storeId) && Services.prefs.getBoolPref("privacy.firstparty.isolate.pbmode.enabled")) {
+    throw new ExtensionError("First-Party Isolation is enabled (in PBM), but the required 'firstPartyDomain' attribute was not set despite using the private store.");
+  }
 
   // When FPI is disabled, the "firstPartyDomain" attribute is optional
   // and defaults to the empty string.
   details.firstPartyDomain = "";
 };
 
 this.cookies = class extends ExtensionAPI {
   getAPI(context) {