Bug 1632474 - Consider src in unsafe allow all checks for feature policy. r=baku

Differential Revision: https://phabricator.services.mozilla.com/D73147

browser/base/content/test/permissions/browser_permission_delegate_geo.js

@@ -185,8 +185,8 @@ add_task(async function testUsePersistentPermissionsFirstParty() {
   });
 });
 
-// Test that we should prompt if we are in unsafe permission delegation. The
-// prompt popup should include both first and third party origin.
+// Test that we do not prompt for maybe unsafe permission delegation if the
+// origin of the page is the original src origin.
 add_task(async function testPromptInMaybeUnsafePermissionDelegation() {
   await BrowserTestUtils.withNewTab(CROSS_SUBFRAME_PAGE, async function(
     browser
@@ -194,8 +194,7 @@ add_task(async function testPromptInMaybeUnsafePermissionDelegation() {
     // Persistent allow top level origin
     PermissionTestUtils.add(uri, "geo", Perms.ALLOW_ACTION);
 
-    await checkGeolocation(browser, "frameAllowsAll", PromptResult.PROMPT);
-    await checkNotificationBothOrigins(uri.host, "example.org");
+    await checkGeolocation(browser, "frameAllowsAll", PromptResult.ALLOW);
 
     SitePermissions.removeFromPrincipal(null, "geo", browser);
     PermissionTestUtils.remove(uri, "geo");
@@ -205,28 +204,25 @@ add_task(async function testPromptInMaybeUnsafePermissionDelegation() {
 // Test that we should prompt if we are in unsafe permission delegation and
 // change location to origin which is not explicitly trusted. The prompt popup
 // should include both first and third party origin.
-add_task(async function testPromptChangeLocatioUnsafePermissionDelegation() {
+add_task(async function testPromptChangeLocationUnsafePermissionDelegation() {
   await BrowserTestUtils.withNewTab(CROSS_SUBFRAME_PAGE, async function(
     browser
   ) {
     // Persistent allow top level origin
     PermissionTestUtils.add(uri, "geo", Perms.ALLOW_ACTION);
 
-    // Request change location.
-    await ContentTask.spawn(browser, { host: uri.host }, async function(args) {
-      let frame = content.document.getElementById("frameAllowsAll");
-      await new Promise(resolve => {
-        function listener() {
-          frame.removeEventListener("load", listener, true);
-          resolve();
-        }
-        frame.addEventListener("load", listener, true);
-
-        frame.contentWindow.location =
-          "https://test1.example.com/browser/browser/base/content/test/permissions/permissions.html";
-      });
+    let iframe = await SpecialPowers.spawn(browser, [], () => {
+      return content.document.getElementById("frameAllowsAll").browsingContext;
     });
 
+    let otherURI =
+      "https://test1.example.com/browser/browser/base/content/test/permissions/permissions.html";
+    let loaded = BrowserTestUtils.browserLoaded(browser, true, otherURI);
+    await SpecialPowers.spawn(iframe, [otherURI], async function(_otherURI) {
+      content.location = _otherURI;
+    });
+    await loaded;
+
     await checkGeolocation(browser, "frameAllowsAll", PromptResult.PROMPT);
     await checkNotificationBothOrigins(uri.host, "test1.example.com");
 
@@ -242,10 +238,24 @@ add_task(async function testExplicitlyAllowedInChain() {
     // Persistent allow top level origin
     PermissionTestUtils.add(uri, "geo", Perms.ALLOW_ACTION);
 
-    const iframeAncestor = await SpecialPowers.spawn(browser, [], () => {
+    let iframeAncestor = await SpecialPowers.spawn(browser, [], () => {
       return content.document.getElementById("frameAncestor").browsingContext;
     });
 
+    let iframe = await SpecialPowers.spawn(iframeAncestor, [], () => {
+      return content.document.getElementById("frameAllowsAll").browsingContext;
+    });
+
+    // Change location to check that we actually look at the ancestor chain
+    // instead of just considering the "same origin as src" rule.
+    let otherURI =
+      "https://test2.example.com/browser/browser/base/content/test/permissions/permissions.html";
+    let loaded = BrowserTestUtils.browserLoaded(browser, true, otherURI);
+    await SpecialPowers.spawn(iframe, [otherURI], async function(_otherURI) {
+      content.location = _otherURI;
+    });
+    await loaded;
+
     await checkGeolocation(
       iframeAncestor,
       "frameAllowsAll",

browser/base/content/test/permissions/temporary_permissions_frame.html

@@ -7,6 +7,6 @@
 <body>
   <iframe id="frameAncestor"
     src="https://test1.example.com/browser/browser/base/content/test/permissions/temporary_permissions_subframe.html"
-    allow="geolocation 'src' https://example.org"></iframe>
+    allow="geolocation https://test1.example.com https://test2.example.com"></iframe>
 </body>
 </html>

browser/base/content/test/webrtc/browser_devices_get_user_media_in_xorigin_frame.js

@@ -357,7 +357,7 @@ var gTests = [
             "expected " + Object.keys(expected).join(" and ") + " to be shared"
           );
 
-          await closeStream(false, "frame1");
+          await closeStream(false, aIframeId);
         } else if (aExpect == PromptResult.DENY) {
           const observerPromise = expectObserverCalled(
             "recording-window-ended"
@@ -409,6 +409,46 @@ var gTests = [
         PromptResult.ALLOW
       );
 
+      // Wildcard attributes still get delegation when their src is unchanged.
+      await checkPersistentPermission(
+        Perms.PROMPT_ACTION,
+        "camera",
+        "frame4",
+        PromptResult.PROMPT
+      );
+      await checkPersistentPermission(
+        Perms.DENY_ACTION,
+        "camera",
+        "frame4",
+        PromptResult.DENY
+      );
+      await checkPersistentPermission(
+        Perms.ALLOW_ACTION,
+        "camera",
+        "frame4",
+        PromptResult.ALLOW
+      );
+
+      // Wildcard attributes still get delegation when their src is unchanged.
+      await checkPersistentPermission(
+        Perms.PROMPT_ACTION,
+        "microphone",
+        "frame4",
+        PromptResult.PROMPT
+      );
+      await checkPersistentPermission(
+        Perms.DENY_ACTION,
+        "microphone",
+        "frame4",
+        PromptResult.DENY
+      );
+      await checkPersistentPermission(
+        Perms.ALLOW_ACTION,
+        "microphone",
+        "frame4",
+        PromptResult.ALLOW
+      );
+
       await checkPersistentPermission(
         Perms.PROMPT_ACTION,
         "screen",
@@ -428,6 +468,12 @@ var gTests = [
         "frame1",
         PromptResult.PROMPT
       );
+      await checkPersistentPermission(
+        Perms.ALLOW_ACTION,
+        "screen",
+        "frame4",
+        PromptResult.PROMPT
+      );
 
       // Denied by default if allow is not defined
       await checkPersistentPermission(
@@ -541,20 +587,16 @@ var gTests = [
       await checkTempPermission("screen");
     },
   },
-
-  {
-    desc:
-      "Prompt and display both first party and third party origin in maybe unsafe permission delegation",
-    run: async function checkPromptNoDelegate() {
-      await promptNoDelegate("test1.example.com");
-      await promptNoDelegate("test1.example.com", true, false);
-      await promptNoDelegate("test1.example.com", false, true);
-    },
-  },
   {
     desc:
       "Don't reprompt while actively sharing in maybe unsafe permission delegation",
     run: async function checkNoRepromptNoDelegate() {
+      // Change location to ensure that we're treated as potentially unsafe.
+      await promiseChangeLocationFrame(
+        "frame4",
+        "https://test2.example.com/browser/browser/base/content/test/webrtc/get_user_media.html"
+      );
+
       // Check that we get a prompt.
       let observerPromise = expectObserverCalled("getUserMedia:request");
       let promise = promisePopupNotificationShown("webRTC-shareDevices");
@@ -566,7 +608,7 @@ var gTests = [
       is(
         PopupNotifications.getNotification("webRTC-shareDevices").options
           .secondName,
-        "test1.example.com",
+        "test2.example.com",
         "Use third party's origin as secondName"
       );
 
@@ -610,13 +652,6 @@ var gTests = [
       await closeStream(false, "frame4");
     },
   },
-  {
-    desc:
-      "Prompt and display both first party and third party origin when sharing screen in unsafe permission delegation",
-    run: async function checkPromptNoDelegateScreenSharing() {
-      await promptNoDelegateScreenSharing("test1.example.com");
-    },
-  },
   {
     desc:
       "Change location, prompt and display both first party and third party origin in maybe unsafe permission delegation",
@@ -646,6 +681,12 @@ var gTests = [
       "Prompt and display both first party and third party origin and temporary deny in frame does not change permission scope",
     skipObserverVerification: true,
     run: async function checkPromptBothOriginsTempDenyFrame() {
+      // Change location to ensure that we're treated as potentially unsafe.
+      await promiseChangeLocationFrame(
+        "frame4",
+        "https://test2.example.com/browser/browser/base/content/test/webrtc/get_user_media.html"
+      );
+
       // Persistent allowed first party origin
       let browser = gBrowser.selectedBrowser;
       let uri = gBrowser.selectedBrowser.documentURI;

dom/base/Document.cpp

@@ -3485,6 +3485,7 @@ nsresult Document::InitFeaturePolicy(nsIChannel* aChannel) {
   if (parentPolicy) {
     // Let's inherit the policy from the parent HTMLIFrameElement if it exists.
     mFeaturePolicy->InheritPolicy(parentPolicy);
+    mFeaturePolicy->SetSrcOrigin(parentPolicy->GetSrcOrigin());
   }
 
   // We don't want to parse the http Feature-Policy header if this pref is off.

dom/security/featurepolicy/FeaturePolicy.cpp

@@ -119,6 +119,17 @@ void FeaturePolicy::AppendToDeclaredAllowInAncestorChain(
   mDeclaredFeaturesInAncestorChain.AppendElement(aFeature);
 }
 
+bool FeaturePolicy::IsSameOriginAsSrc(nsIPrincipal* aPrincipal) const {
+  MOZ_ASSERT(aPrincipal);
+
+  if (!mSrcOrigin) {
+    return false;
+  }
+
+  return BasePrincipal::Cast(mSrcOrigin)
+      ->Subsumes(aPrincipal, BasePrincipal::ConsiderDocumentDomain);
+}
+
 void FeaturePolicy::SetDeclaredPolicy(Document* aDocument,
                                       const nsAString& aPolicyString,
                                       nsIPrincipal* aSelfOrigin,

dom/security/featurepolicy/FeaturePolicy.h

@@ -80,6 +80,8 @@ class FeaturePolicy final : public nsISupports, public nsWrapperCache {
     mDefaultOrigin = aPrincipal;
   }
 
+  void SetSrcOrigin(nsIPrincipal* aPrincipal) { mSrcOrigin = aPrincipal; }
+
   nsIPrincipal* DefaultOrigin() const { return mDefaultOrigin; }
 
   // Inherits the policy from the 'parent' context if it exists.
@@ -115,6 +117,8 @@ class FeaturePolicy final : public nsISupports, public nsWrapperCache {
   bool AllowsFeatureExplicitlyInAncestorChain(const nsAString& aFeatureName,
                                               nsIPrincipal* aOrigin) const;
 
+  bool IsSameOriginAsSrc(nsIPrincipal* aPrincipal) const;
+
   // WebIDL internal methods.
 
   JSObject* WrapObject(JSContext* aCx,

dom/security/featurepolicy/FeaturePolicyUtils.cpp

@@ -165,6 +165,7 @@ bool FeaturePolicyUtils::IsFeatureUnsafeAllowedAll(
   MOZ_ASSERT(policy);
 
   return policy->HasFeatureUnsafeAllowsAll(aFeatureName) &&
+         !policy->IsSameOriginAsSrc(aDocument->NodePrincipal()) &&
          !policy->AllowsFeatureExplicitlyInAncestorChain(
              aFeatureName, policy->DefaultOrigin()) &&
          !IsSameOriginAsTop(aDocument);