diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index e4eed6fa1d12..3c9c1ae3aad3 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -13786,8 +13786,13 @@ nsIDocument::RequestStorageAccess(mozilla::ErrorResult& aRv) // Note: If this has returned true, the top-level document is guaranteed // to not be on the Content Blocking allow list. DebugOnly isOnAllowList = false; + // If we have a parent document, it has to be non-private since we verified + // earlier that our own document is non-private and a private document can + // never have a non-private document as its child. + MOZ_ASSERT_IF(parent, !nsContentUtils::IsInPrivateBrowsing(parent)); MOZ_ASSERT_IF(NS_SUCCEEDED(AntiTrackingCommon::IsOnContentBlockingAllowList( parent->GetDocumentURI(), + false, AntiTrackingCommon::eStorageChecks, isOnAllowList)), !isOnAllowList); diff --git a/netwerk/base/nsChannelClassifier.cpp b/netwerk/base/nsChannelClassifier.cpp index 8fbc7ff74e56..976ee8647eb0 100644 --- a/netwerk/base/nsChannelClassifier.cpp +++ b/netwerk/base/nsChannelClassifier.cpp @@ -524,6 +524,7 @@ nsChannelClassifier::ShouldEnableTrackingProtectionInternal( } rv = AntiTrackingCommon::IsOnContentBlockingAllowList(topWinURI, + NS_UsePrivateBrowsing(aChannel), aAnnotationsOnly ? AntiTrackingCommon::eTrackingAnnotations : AntiTrackingCommon::eTrackingProtection, diff --git a/toolkit/components/antitracking/AntiTrackingCommon.cpp b/toolkit/components/antitracking/AntiTrackingCommon.cpp index c9fa270b46a1..df96c86383b3 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.cpp +++ b/toolkit/components/antitracking/AntiTrackingCommon.cpp @@ -11,6 +11,7 @@ #include "mozilla/AbstractThread.h" #include "mozilla/IntegerPrintfMacros.h" #include "mozilla/Logging.h" +#include "mozilla/Pair.h" #include "mozilla/StaticPrefs.h" #include "mozIThirdPartyUtil.h" #include "nsContentUtils.h" @@ -152,11 +153,11 @@ CookiesBehavior(nsIPrincipal* aPrincipal) } bool -CheckContentBlockingAllowList(nsIURI* aTopWinURI) +CheckContentBlockingAllowList(nsIURI* aTopWinURI, bool aIsPrivateBrowsing) { bool isAllowed = false; nsresult rv = - AntiTrackingCommon::IsOnContentBlockingAllowList(aTopWinURI, + AntiTrackingCommon::IsOnContentBlockingAllowList(aTopWinURI, aIsPrivateBrowsing, AntiTrackingCommon::eStorageChecks, isAllowed); if (NS_SUCCEEDED(rv) && isAllowed) { @@ -177,7 +178,9 @@ CheckContentBlockingAllowList(nsPIDOMWindowInner* aWindow) nsPIDOMWindowOuter* top = aWindow->GetScriptableTop(); if (top) { nsIURI* topWinURI = top->GetDocumentURI(); - return CheckContentBlockingAllowList(topWinURI); + nsIDocument* doc = top->GetExtantDoc(); + bool isPrivateBrowsing = doc ? nsContentUtils::IsInPrivateBrowsing(doc) : false; + return CheckContentBlockingAllowList(topWinURI, isPrivateBrowsing); } LOG(("Could not check the content blocking allow list because the top " @@ -193,7 +196,8 @@ CheckContentBlockingAllowList(nsIHttpChannel* aChannel) nsCOMPtr topWinURI; nsresult rv = chan->GetTopWindowURI(getter_AddRefs(topWinURI)); if (NS_SUCCEEDED(rv)) { - return CheckContentBlockingAllowList(topWinURI); + return CheckContentBlockingAllowList(topWinURI, + NS_UsePrivateBrowsing(aChannel)); } } @@ -1085,6 +1089,7 @@ AntiTrackingCommon::MaybeIsFirstPartyStorageAccessGrantedFor(nsPIDOMWindowInner* nsresult AntiTrackingCommon::IsOnContentBlockingAllowList(nsIURI* aTopWinURI, + bool aIsPrivateBrowsing, AntiTrackingCommon::ContentBlockingAllowListPurpose aPurpose, bool& aIsAllowListed) { @@ -1133,19 +1138,23 @@ AntiTrackingCommon::IsOnContentBlockingAllowList(nsIURI* aTopWinURI, NS_ENSURE_SUCCESS(rv, rv); // Check both the normal mode and private browsing mode user override permissions. - const char* types[] = { - "trackingprotection", - "trackingprotection-pb" + Pair types[] = { + {"trackingprotection", false}, + {"trackingprotection-pb", true} }; for (size_t i = 0; i < ArrayLength(types); ++i) { + if (aIsPrivateBrowsing != types[i].second()) { + continue; + } + uint32_t permissions = nsIPermissionManager::UNKNOWN_ACTION; - rv = permMgr->TestPermission(topWinURI, types[i], &permissions); + rv = permMgr->TestPermission(topWinURI, types[i].first(), &permissions); NS_ENSURE_SUCCESS(rv, rv); if (permissions == nsIPermissionManager::ALLOW_ACTION) { aIsAllowListed = true; - LOG_SPEC(("Found user override type %s for %s", types[i], _spec), + LOG_SPEC(("Found user override type %s for %s", types[i].first(), _spec), topWinURI); // Stop checking the next permisson type if we decided to override. break; diff --git a/toolkit/components/antitracking/AntiTrackingCommon.h b/toolkit/components/antitracking/AntiTrackingCommon.h index 5d1bcb57da60..1b56e49770ea 100644 --- a/toolkit/components/antitracking/AntiTrackingCommon.h +++ b/toolkit/components/antitracking/AntiTrackingCommon.h @@ -138,6 +138,7 @@ public: // Check whether a top window URI is on the content blocking allow list. static nsresult IsOnContentBlockingAllowList(nsIURI* aTopWinURI, + bool aIsPrivateBrowsing, ContentBlockingAllowListPurpose aPurpose, bool& aIsAllowListed); diff --git a/toolkit/components/antitracking/test/browser/browser.ini b/toolkit/components/antitracking/test/browser/browser.ini index 2941c3a80748..0a4f30bd4830 100644 --- a/toolkit/components/antitracking/test/browser/browser.ini +++ b/toolkit/components/antitracking/test/browser/browser.ini @@ -15,6 +15,7 @@ support-files = popup.html storageAccessAPIHelpers.js +[browser_allowListSeparationInPrivateAndNormalWindows.js] [browser_backgroundImageAssertion.js] [browser_blockingCookies.js] support-files = server.sjs diff --git a/toolkit/components/antitracking/test/browser/browser_allowListSeparationInPrivateAndNormalWindows.js b/toolkit/components/antitracking/test/browser/browser_allowListSeparationInPrivateAndNormalWindows.js new file mode 100644 index 000000000000..8b274a2bcebf --- /dev/null +++ b/toolkit/components/antitracking/test/browser/browser_allowListSeparationInPrivateAndNormalWindows.js @@ -0,0 +1,47 @@ +ChromeUtils.import("resource://gre/modules/Services.jsm"); + +// This test works by setting up an exception for the private window allow list +// manually, and it then expects to see some blocking notifications (note the +// document.cookie setter in the blocking callback.) +// If the exception lists aren't handled separately, we'd get confused and put +// the pages loaded under this test in the allow list, which would result in +// the test not passing because no blocking notifications would be observed. + +// Testing the reverse case would also be interesting, but unfortunately there +// isn't a super easy way to do that with our antitracking test framework since +// private windows wouldn't send any blocking notifications as they don't have +// storage access in the first place. + +add_task(async _ => { + let uri = Services.io.newURI("https://example.net"); + Services.perms.add(uri, "trackingprotection-pb", + Services.perms.ALLOW_ACTION); + + registerCleanupFunction(_ => { + Services.perms.removeAll(); + }); +}); + +AntiTracking.runTest("Test that we don't honour a private allow list exception in a normal window", + // Blocking callback + async _ => { + document.cookie = "name=value"; + }, + + // Non blocking callback + async _ => { + // Nothing to do here. + }, + + // Cleanup callback + async _ => { + await new Promise(resolve => { + Services.clearData.deleteData(Ci.nsIClearDataService.CLEAR_ALL, value => resolve()); + }); + }, + null, // no extra prefs + false, // run the window.open() test + false, // run the user interaction test + true, // expect blocking notifications + false); // run in a normal window +