Bug 1631859 - Part 1: Fill out ancestor principals and outer window IDs for LoadInfo only in the parent, r=kmag,extension-reviewers

Keeping a list of ancestor principals in a LoadInfo object, that, at times,
exists in the content process, is not secure. Since ancestor principals are
only ever needed to create a list of frameAncestors, which, in turn, are only
ever accessed from the parent process, we can assemble lists of ancestor
principals and outer windowIDs whenever we are in the parent process and are
either 1) creating a LoadInfo object or 2) deserializing a LoadInfoArgs struct,
received from content process, into a LoadInfo object.

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

ipc/glue/BackgroundUtils.cpp

@@ -28,6 +28,8 @@
 #include "mozilla/dom/nsCSPUtils.h"
 #include "mozilla/dom/nsCSPContext.h"
 #include "mozilla/dom/BrowsingContext.h"
+#include "mozilla/dom/CanonicalBrowsingContext.h"
+#include "mozilla/dom/WindowGlobalParent.h"
 
 namespace mozilla {
 
@@ -452,14 +454,6 @@ nsresult LoadInfoToLoadInfoArgs(nsILoadInfo* aLoadInfo,
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  nsTArray<PrincipalInfo> ancestorPrincipals;
-  ancestorPrincipals.SetCapacity(aLoadInfo->AncestorPrincipals().Length());
-  for (const auto& principal : aLoadInfo->AncestorPrincipals()) {
-    rv =
-        PrincipalToPrincipalInfo(principal, ancestorPrincipals.AppendElement());
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
   Maybe<IPCClientInfo> ipcClientInfo;
   const Maybe<ClientInfo>& clientInfo = aLoadInfo->GetClientInfo();
   if (clientInfo.isSome()) {
@@ -532,10 +526,10 @@ nsresult LoadInfoToLoadInfoArgs(nsILoadInfo* aLoadInfo,
       aLoadInfo->GetIsThirdPartyContextToTopWindow(),
       aLoadInfo->GetIsFormSubmission(), aLoadInfo->GetSendCSPViolationEvents(),
       aLoadInfo->GetOriginAttributes(), redirectChainIncludingInternalRedirects,
-      redirectChain, ancestorPrincipals, aLoadInfo->AncestorOuterWindowIDs(),
-      ipcClientInfo, ipcReservedClientInfo, ipcInitialClientInfo, ipcController,
-      aLoadInfo->CorsUnsafeHeaders(), aLoadInfo->GetForcePreflight(),
-      aLoadInfo->GetIsPreflight(), aLoadInfo->GetLoadTriggeredFromExternal(),
+      redirectChain, {}, {}, ipcClientInfo, ipcReservedClientInfo,
+      ipcInitialClientInfo, ipcController, aLoadInfo->CorsUnsafeHeaders(),
+      aLoadInfo->GetForcePreflight(), aLoadInfo->GetIsPreflight(),
+      aLoadInfo->GetLoadTriggeredFromExternal(),
       aLoadInfo->GetServiceWorkerTaintingSynthesized(),
       aLoadInfo->GetDocumentHasUserInteracted(),
       aLoadInfo->GetDocumentHasLoaded(),
@@ -670,14 +664,19 @@ nsresult LoadInfoArgsToLoadInfo(
   }
 
   nsTArray<nsCOMPtr<nsIPrincipal>> ancestorPrincipals;
-  ancestorPrincipals.SetCapacity(loadInfoArgs.ancestorPrincipals().Length());
-  for (const PrincipalInfo& principalInfo : loadInfoArgs.ancestorPrincipals()) {
-    auto ancestorPrincipalOrErr = PrincipalInfoToPrincipal(principalInfo);
-    if (NS_WARN_IF(ancestorPrincipalOrErr.isErr())) {
-      return ancestorPrincipalOrErr.unwrapErr();
+  nsTArray<uint64_t> ancestorOuterWindowIDs;
+  if (XRE_IsParentProcess() &&
+      (nsContentUtils::InternalContentPolicyTypeToExternal(
+           loadInfoArgs.contentPolicyType()) !=
+       nsIContentPolicy::TYPE_DOCUMENT)) {
+    // Only fill out ancestor principals and outer window IDs when we
+    // are deserializing LoadInfoArgs to be LoadInfo for a subresource
+    RefPtr<BrowsingContext> parentBC =
+        BrowsingContext::Get(loadInfoArgs.browsingContextID());
+    if (parentBC) {
+      LoadInfo::ComputeAncestors(parentBC->Canonical(), ancestorPrincipals,
+                                 ancestorOuterWindowIDs);
     }
-    nsCOMPtr<nsIPrincipal> ancestorPrincipal = ancestorPrincipalOrErr.unwrap();
-    ancestorPrincipals.AppendElement(ancestorPrincipal.forget());
   }
 
   Maybe<ClientInfo> clientInfo;
@@ -757,10 +756,9 @@ nsresult LoadInfoArgsToLoadInfo(
       loadInfoArgs.isThirdPartyContextToTopWindow(),
       loadInfoArgs.isFormSubmission(), loadInfoArgs.sendCSPViolationEvents(),
       loadInfoArgs.originAttributes(), redirectChainIncludingInternalRedirects,
-      redirectChain, std::move(ancestorPrincipals),
-      loadInfoArgs.ancestorOuterWindowIDs(), loadInfoArgs.corsUnsafeHeaders(),
-      loadInfoArgs.forcePreflight(), loadInfoArgs.isPreflight(),
-      loadInfoArgs.loadTriggeredFromExternal(),
+      redirectChain, std::move(ancestorPrincipals), ancestorOuterWindowIDs,
+      loadInfoArgs.corsUnsafeHeaders(), loadInfoArgs.forcePreflight(),
+      loadInfoArgs.isPreflight(), loadInfoArgs.loadTriggeredFromExternal(),
       loadInfoArgs.serviceWorkerTaintingSynthesized(),
       loadInfoArgs.documentHasUserInteracted(),
       loadInfoArgs.documentHasLoaded(),

netwerk/base/LoadInfo.cpp

@@ -611,25 +611,9 @@ LoadInfo::LoadInfo(dom::WindowGlobalParent* aParentWGP,
       mLoadingEmbedderPolicy(nsILoadInfo::EMBEDDER_POLICY_NULL) {
   CanonicalBrowsingContext* parentBC = aParentWGP->BrowsingContext();
   MOZ_ASSERT(parentBC);
-  nsTArray<nsCOMPtr<nsIPrincipal>> ancestorPrincipals;
-  nsTArray<uint64_t> ancestorOuterWindowIDs;
-  CanonicalBrowsingContext* ancestorBC = parentBC;
-  RefPtr<WindowGlobalParent> topLevelWGP = aParentWGP->TopWindowContext();
+  ComputeAncestors(parentBC, mAncestorPrincipals, mAncestorOuterWindowIDs);
 
-  // Iterate over ancestor WindowGlobalParents, collecting principals and outer
-  // window IDs.
-  while (WindowGlobalParent* ancestorWGP =
-             ancestorBC->GetParentWindowContext()) {
-    nsCOMPtr<nsIPrincipal> parentPrincipal = ancestorWGP->DocumentPrincipal();
-    MOZ_ASSERT(parentPrincipal, "Ancestor principal is null");
-    ancestorPrincipals.AppendElement(parentPrincipal.forget());
-    ancestorOuterWindowIDs.AppendElement(ancestorWGP->OuterWindowId());
-    ancestorBC = ancestorWGP->BrowsingContext();
-  }
-  mAncestorPrincipals = std::move(ancestorPrincipals);
-  mAncestorOuterWindowIDs = std::move(ancestorOuterWindowIDs);
-  MOZ_DIAGNOSTIC_ASSERT(mAncestorPrincipals.Length() ==
-                        mAncestorOuterWindowIDs.Length());
+  RefPtr<WindowGlobalParent> topLevelWGP = aParentWGP->TopWindowContext();
 
   if (WindowGlobalParent* ancestorWGP = aParentWGP->GetParentWindowContext()) {
     mParentOuterWindowID = ancestorWGP->OuterWindowId();
@@ -935,6 +919,25 @@ LoadInfo::LoadInfo(
   mRedirectChain.SwapElements(aRedirectChain);
 }
 
+// static
+void LoadInfo::ComputeAncestors(
+    CanonicalBrowsingContext* aBC,
+    nsTArray<nsCOMPtr<nsIPrincipal>>& aAncestorPrincipals,
+    nsTArray<uint64_t>& aOuterWindowIDs) {
+  MOZ_ASSERT(aAncestorPrincipals.IsEmpty());
+  MOZ_ASSERT(aOuterWindowIDs.IsEmpty());
+  CanonicalBrowsingContext* ancestorBC = aBC;
+  // Iterate over ancestor WindowGlobalParents, collecting principals and outer
+  // window IDs.
+  while (WindowGlobalParent* ancestorWGP =
+             ancestorBC->GetParentWindowContext()) {
+    nsCOMPtr<nsIPrincipal> parentPrincipal = ancestorWGP->DocumentPrincipal();
+    MOZ_ASSERT(parentPrincipal, "Ancestor principal is null");
+    aAncestorPrincipals.AppendElement(parentPrincipal.forget());
+    aOuterWindowIDs.AppendElement(ancestorWGP->OuterWindowId());
+    ancestorBC = ancestorWGP->BrowsingContext();
+  }
+}
 void LoadInfo::ComputeIsThirdPartyContext(nsPIDOMWindowOuter* aOuterWindow) {
   nsContentPolicyType type =
       nsContentUtils::InternalContentPolicyTypeToExternal(

netwerk/base/LoadInfo.h

@@ -89,6 +89,14 @@ class LoadInfo final : public nsILoadInfo {
            nsContentPolicyType aContentPolicyType,
            nsSecurityFlags aSecurityFlags, uint32_t aSandboxFlags);
 
+  // Compute a list of ancestor principals and outer windowIDs.
+  // See methods AncestorPrincipals and AncestorOuterWindowIDs
+  // in nsILoadInfo.idl for details.
+  static void ComputeAncestors(
+      dom::CanonicalBrowsingContext* aBC,
+      nsTArray<nsCOMPtr<nsIPrincipal>>& aAncestorPrincipals,
+      nsTArray<uint64_t>& aOuterWindowIDs);
+
   // create an exact copy of the loadinfo
   already_AddRefed<nsILoadInfo> Clone() const;
 

netwerk/base/nsILoadInfo.idl

@@ -866,19 +866,30 @@ interface nsILoadInfo : nsISupports
   nsIRedirectHistoryEntryArray binaryRedirectChain();
 
   /**
-   * An array of nsIPrincipals which stores the principals of the parent frames,
-   * not including the frame loading this request.  The closest ancestor is at
-   * index zero and the top level ancestor is at the last index.
+   * This array is only filled out when we are in the parent process and we are
+   * creating a loadInfo object or deserializing LoadInfoArgs into LoadInfo,
+   * as we ever only need in the parent process.
    *
-   * The ancestorPrincipals[0] entry for an iframe load will be the principal of
-   * the iframe element's owner document.
-   * The ancestorPrincipals[0] entry for an image loaded in an iframe will be the
-   * principal of the iframe element's owner document.
+   * The array is meant to be a list of principals of the documents that the
+   * browsing context, corresponding to this loadInfo object, is "nested through" in
+   * the sense of
+   * <https://html.spec.whatwg.org/multipage/browsers.html#browsing-context-nested-through>.
+   * Note that the array does not include the principal corresponding to the frame
+   * loading this request. The closest ancestor is at index zero and the top level
+   * ancestor is at the last index.
    *
-   * See Document::AncestorPrincipals for more information.
+   * If this is a toplevel content browsing context (i.e. toplevel document in spec
+   * terms), the list is empty.
    *
-   * Please note that this array has the same lifetime as the
-   * loadInfo object - use with caution!
+   * Otherwise the array is a list for the document we're nested through (again in
+   * the spec sense), with the principal of that document prepended. The
+   * ancestorPrincipals[0] entry for an iframe load will be the principal of the
+   * iframe element's owner document. The ancestorPrincipals[0] entry for an image
+   * loaded in an iframe will be the principal of the iframe element's owner
+   * document. This matches the ordering specified for Location.ancestorOrigins.
+   *
+   * Please note that this array has the same lifetime as the loadInfo object - use
+   * with caution!
    */
   [noscript, notxpcom, nostdcall]
   PrincipalArrayRef AncestorPrincipals();

toolkit/components/extensions/test/mochitest/file_contains_iframe.html

@@ -0,0 +1,12 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+
+<iframe src="http://example.org/tests/toolkit/components/extensions/test/mochitest/file_contains_img.html">
+</iframe>
+
+</body>
+</html>

toolkit/components/extensions/test/mochitest/file_contains_img.html

@@ -0,0 +1,11 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+
+<img src="file_image_good.png"/>
+
+</body>
+</html>

toolkit/components/extensions/test/mochitest/mochitest-common.ini

@@ -5,6 +5,8 @@ support-files =
   file_WebNavigation_page2.html
   file_WebNavigation_page3.html
   file_WebRequest_page3.html
+  file_contains_img.html
+  file_contains_iframe.html
   file_image_bad.png
   file_image_good.png
   file_image_great.png
@@ -92,6 +94,7 @@ skip-if = os == 'android' || tsan # Times out on TSan intermittently, bug 161518
 skip-if = os == 'android' # Bug 1513544 Android does not support multiple windows.
 [test_ext_cookies_permissions_bad.html]
 [test_ext_cookies_permissions_good.html]
+[test_ext_embeddedimg_iframe_frameAncestors.html]
 [test_ext_exclude_include_globs.html]
 [test_ext_external_messaging.html]
 [test_ext_generate.html]

toolkit/components/extensions/test/mochitest/test_ext_embeddedimg_iframe_frameAncestors.html

@@ -0,0 +1,94 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Test checking webRequest.onBeforeRequest details object</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="/tests/SimpleTest/EventUtils.js"></script>
+  <script src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+
+<script type="text/javascript">
+"use strict";
+
+let expected = {
+  "file_contains_iframe.html": {
+    type: "main_frame",
+    frameAncestor_length: 0,
+  },
+  "file_contains_img.html": {
+    type: "sub_frame",
+    frameAncestor_length: 1,
+  },
+  "file_image_good.png": {
+    type: "image",
+    frameAncestor_length: 1,
+  }
+};
+
+function checkDetails(details) {
+  let url = new URL(details.url);
+  let filename = url.pathname.split("/").pop();
+  ok(expected.hasOwnProperty(filename), `Should be expecting a request for ${filename}`);
+  let expect = expected[filename];
+  is(expect.type, details.type, `${details.type} type matches`);
+  is(expect.frameAncestor_length, details.frameAncestors.length, "incorrect frameAncestors length");
+  if (filename == "file_contains_img.html") {
+    is(details.frameAncestors[0].frameId, details.parentFrameId);
+  } else if (filename == "file_image_good.png") {
+    if (SpecialPowers.useRemoteSubframes) {
+      // This will be fixed in upcoming patches
+      todo_is(details.frameAncestors[0].frameId, details.parentFrameId);
+    } else {
+      is(details.frameAncestors[0].frameId, details.parentFrameId);
+    }
+  }
+}
+
+add_task(async () => {
+  // Clear the image cache, since it gets in the way otherwise.
+  let imgTools = SpecialPowers.Cc["@mozilla.org/image/tools;1"].getService(SpecialPowers.Ci.imgITools);
+  let cache = imgTools.getImgCacheForDocument(document);
+  cache.clearCache(false);
+  await SpecialPowers.spawnChrome([], async () => {
+    Services.cache2.clear();
+  });
+
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      permissions: ["webRequest", "<all_urls>"],
+    },
+    background() {
+      browser.webRequest.onBeforeRequest.addListener(
+        details => {
+          browser.test.sendMessage("onBeforeRequest", details);
+        },
+        {
+          urls: [
+          "http://example.org/*/file_contains_img.html",
+          "http://mochi.test/*/file_contains_iframe.html",
+          "*://*/*.png",
+          ],
+        }
+      );
+    },
+  });
+
+  await extension.startup();
+  const FILE_URL = "http://mochi.test:8888/tests/toolkit/components/extensions/test/mochitest/file_contains_iframe.html";
+  let win = window.open(FILE_URL);
+  await new Promise(resolve => win.addEventListener("load", () => resolve(), {once: true}));
+
+  for (let i = 0; i < Object.keys(expected).length; i++) {
+    checkDetails(await extension.awaitMessage("onBeforeRequest"));
+  }
+
+  win.close();
+  await extension.unload();
+});
+
+</script>
+
+</body>
+</html>