Bug 1412345: Downgrade expanded principals before inheriting. r=bz,krizsa
authorKris Maglione <maglione.k@gmail.com>
Thu, 02 Nov 2017 19:56:27 -0700
changeset 443438 4350a326a49805c6138aabd0ed68136498bf97cd
parent 443437 8dfcfe1bd1557366bf49eb97b66f43a64b459c36
child 443439 83f0e033450d1dce9a9ce96b37b465ae50dacdb6
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz, krizsa
bugs1412345
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1412345: Downgrade expanded principals before inheriting. r=bz,krizsa There are several ways that expanded principals can be used as triggering principals for requests. While that works fine for security checks, it also sometimes causes them to be inherited, and used as result principals in contexts where expanded principals aren't allowed. This patch changes our inheritance behavior so that expanded principals are downgraded to the most appropriate constituent principal when they would otherwise be inherited. The logic for choosing the most appropriate principal is a bit suspect, and may eventually need to be changed to always select the last whitelist principal, but I chose it to preserve the current principal downgrade behavior used by XMLHttpRequest for the time being. MozReview-Commit-ID: 9fvAKr2e2fa
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/ExpandedPrincipal.cpp
caps/ExpandedPrincipal.h
caps/nsIPrincipal.idl
caps/nsScriptSecurityManager.cpp
dom/base/nsDocument.cpp
dom/jsurl/nsJSProtocolHandler.cpp
dom/xhr/XMLHttpRequestMainThread.cpp
netwerk/base/LoadInfo.cpp
netwerk/base/nsILoadInfo.idl
toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -8,16 +8,17 @@
 
 #include "nsDocShell.h"
 #include "nsIContentSecurityPolicy.h"
 #include "nsIObjectInputStream.h"
 #include "nsIObjectOutputStream.h"
 #include "nsIStandardURL.h"
 
 #include "ContentPrincipal.h"
+#include "ExpandedPrincipal.h"
 #include "nsNetUtil.h"
 #include "nsIURIWithPrincipal.h"
 #include "NullPrincipal.h"
 #include "nsScriptSecurityManager.h"
 #include "nsServiceManagerUtils.h"
 
 #include "mozilla/dom/ChromeUtils.h"
 #include "mozilla/dom/CSPDictionariesBinding.h"
@@ -352,16 +353,27 @@ bool
 BasePrincipal::AddonHasPermission(const nsAtom* aPerm)
 {
   if (auto policy = AddonPolicy()) {
     return policy->HasPermission(aPerm);
   }
   return false;
 }
 
+nsIPrincipal*
+BasePrincipal::PrincipalToInherit(nsIURI* aRequestedURI,
+                                  bool aAllowIfInheritsPrincipal)
+{
+  if (Is<ExpandedPrincipal>()) {
+    return As<ExpandedPrincipal>()->PrincipalToInherit(aRequestedURI,
+                                                       aAllowIfInheritsPrincipal);
+  }
+  return this;
+}
+
 already_AddRefed<BasePrincipal>
 BasePrincipal::CreateCodebasePrincipal(nsIURI* aURI,
                                        const OriginAttributes& aAttrs)
 {
   MOZ_ASSERT(aURI);
 
   nsAutoCString originNoSuffix;
   nsresult rv =
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -120,16 +120,28 @@ public:
 
   // Call these to avoid the cost of virtual dispatch.
   inline bool FastEquals(nsIPrincipal* aOther);
   inline bool FastEqualsConsideringDomain(nsIPrincipal* aOther);
   inline bool FastSubsumes(nsIPrincipal* aOther);
   inline bool FastSubsumesConsideringDomain(nsIPrincipal* aOther);
   inline bool FastSubsumesConsideringDomainIgnoringFPD(nsIPrincipal* aOther);
 
+  // Returns the principal to inherit when a caller with this principal loads
+  // the given URI.
+  //
+  // For most principal types, this returns the principal itself. For expanded
+  // principals, it returns the first sub-principal which subsumes the given URI
+  // (or, if no URI is given, the last whitelist principal).
+  //
+  // The aAllowIfInheritsPrincipal argument is passed through to CheckMayLoad()
+  // to determine which consistituent principals may load the requested URI.
+  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr,
+                                   bool aAllowIfInheritsPrincipal = true);
+
   /**
    * Returns true if this principal's CSP should override a document's CSP for
    * loads that it triggers. Currently true only for expanded principals which
    * subsume the document principal.
    */
   bool OverridesCSP(nsIPrincipal* aDocumentPrincipal)
   {
     return mKind == eExpandedPrincipal && FastSubsumes(aDocumentPrincipal);
--- a/caps/ExpandedPrincipal.cpp
+++ b/caps/ExpandedPrincipal.cpp
@@ -175,16 +175,31 @@ ExpandedPrincipal::AddonHasPermission(co
   for (size_t i = 0; i < mPrincipals.Length(); ++i) {
     if (BasePrincipal::Cast(mPrincipals[i])->AddonHasPermission(aPerm)) {
       return true;
     }
   }
   return false;
 }
 
+nsIPrincipal*
+ExpandedPrincipal::PrincipalToInherit(nsIURI* aRequestedURI,
+                                      bool aAllowIfInheritsPrincipal)
+{
+  if (aRequestedURI) {
+    for (const auto& principal : mPrincipals) {
+      if (NS_SUCCEEDED(principal->CheckMayLoad(aRequestedURI, false,
+                                               aAllowIfInheritsPrincipal))) {
+        return principal;
+      }
+    }
+  }
+  return mPrincipals.LastElement();
+}
+
 nsresult
 ExpandedPrincipal::GetScriptLocation(nsACString& aStr)
 {
   aStr.AssignLiteral("[Expanded Principal [");
   for (size_t i = 0; i < mPrincipals.Length(); ++i) {
     if (i != 0) {
       aStr.AppendLiteral(", ");
     }
--- a/caps/ExpandedPrincipal.h
+++ b/caps/ExpandedPrincipal.h
@@ -32,16 +32,21 @@ public:
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   NS_IMETHOD GetAddonId(nsAString& aAddonId) override;
   virtual bool AddonHasPermission(const nsAtom* aPerm) override;
   virtual nsresult GetScriptLocation(nsACString &aStr) override;
 
+  // Returns the principal to inherit when this principal requests the given
+  // URL. See BasePrincipal::PrincipalToInherit.
+  nsIPrincipal* PrincipalToInherit(nsIURI* aRequestedURI = nullptr,
+                                   bool aAllowIfInheritsPrincipal = true);
+
 protected:
   explicit ExpandedPrincipal(nsTArray<nsCOMPtr<nsIPrincipal>> &aWhiteList);
 
   virtual ~ExpandedPrincipal();
 
   bool SubsumesInternal(nsIPrincipal* aOther,
                         DocumentDomainConsideration aConsideration) override;
 
--- a/caps/nsIPrincipal.idl
+++ b/caps/nsIPrincipal.idl
@@ -332,14 +332,20 @@ interface nsIPrincipal : nsISerializable
  * content and a well defined set of other domains, without the risk of
  * leaking out a system principal to the content. See: Bug 734891
  */
 [uuid(f3e177Df-6a5e-489f-80a7-2dd1481471d8)]
 interface nsIExpandedPrincipal : nsISupports
 {
   /**
    * An array of principals that the expanded principal subsumes.
+   *
+   * When an expanded principal is used as a triggering principal for a
+   * request that inherits a security context, one of its constitutent
+   * principals is inherited rather than the expanded principal itself. The
+   * last principal in the whitelist is the default principal to inherit.
+   *
    * Note: this list is not reference counted, it is shared, so
    * should not be changed and should only be used ephemerally.
    */
   [noscript, notxpcom, nostdcall]
   PrincipalArray WhiteList();
 };
--- a/caps/nsScriptSecurityManager.cpp
+++ b/caps/nsScriptSecurityManager.cpp
@@ -225,20 +225,19 @@ InheritAndSetCSPOnPrincipalIfNeeded(nsIC
 
   bool isSrcDoc = URISpec.EqualsLiteral("about:srcdoc");
   bool isData = (NS_SUCCEEDED(uri->SchemeIs("data", &isData)) && isData);
 
   if (!isSrcDoc && !isData) {
     return;
   }
 
-  nsCOMPtr<nsIPrincipal> principalToInherit = loadInfo->PrincipalToInherit();
-  if (!principalToInherit) {
-    principalToInherit = loadInfo->TriggeringPrincipal();
-  }
+  nsCOMPtr<nsIPrincipal> principalToInherit =
+    loadInfo->FindPrincipalToInherit(aChannel);
+
   nsCOMPtr<nsIContentSecurityPolicy> originalCSP;
   principalToInherit->GetCsp(getter_AddRefs(originalCSP));
   if (!originalCSP) {
     return;
   }
 
   // if the principalToInherit had a CSP, add it to the before
   // created NullPrincipal (unless it already has one)
@@ -259,20 +258,18 @@ nsresult
 nsScriptSecurityManager::GetChannelResultPrincipal(nsIChannel* aChannel,
                                                    nsIPrincipal** aPrincipal,
                                                    bool aIgnoreSandboxing)
 {
   NS_PRECONDITION(aChannel, "Must have channel!");
   // Check whether we have an nsILoadInfo that says what we should do.
   nsCOMPtr<nsILoadInfo> loadInfo = aChannel->GetLoadInfo();
   if (loadInfo && loadInfo->GetForceInheritPrincipalOverruleOwner()) {
-    nsCOMPtr<nsIPrincipal> principalToInherit = loadInfo->PrincipalToInherit();
-    if (!principalToInherit) {
-      principalToInherit = loadInfo->TriggeringPrincipal();
-    }
+    nsCOMPtr<nsIPrincipal> principalToInherit =
+      loadInfo->FindPrincipalToInherit(aChannel);
     principalToInherit.forget(aPrincipal);
     return NS_OK;
   }
 
   nsCOMPtr<nsISupports> owner;
   aChannel->GetOwner(getter_AddRefs(owner));
   if (owner) {
     CallQueryInterface(owner, aPrincipal);
@@ -294,39 +291,36 @@ nsScriptSecurityManager::GetChannelResul
       // Check if SEC_FORCE_INHERIT_PRINCIPAL was dropped because of
       // sandboxing:
       if (loadInfo->GetLoadingSandboxed() &&
         loadInfo->GetForceInheritPrincipalDropped()) {
         forceInherit = true;
       }
     }
     if (forceInherit) {
-      nsCOMPtr<nsIPrincipal> principalToInherit = loadInfo->PrincipalToInherit();
-      if (!principalToInherit) {
-        principalToInherit = loadInfo->TriggeringPrincipal();
-      }
+      nsCOMPtr<nsIPrincipal> principalToInherit =
+        loadInfo->FindPrincipalToInherit(aChannel);
       principalToInherit.forget(aPrincipal);
       return NS_OK;
     }
 
     auto securityMode = loadInfo->GetSecurityMode();
     // The data: inheritance flags should only apply to the initial load,
     // not to loads that it might have redirected to.
     if (loadInfo->RedirectChain().IsEmpty() &&
         (securityMode == nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS ||
          securityMode == nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS ||
          securityMode == nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS)) {
 
       nsCOMPtr<nsIURI> uri;
       nsresult rv = NS_GetFinalChannelURI(aChannel, getter_AddRefs(uri));
       NS_ENSURE_SUCCESS(rv, rv);
-      nsCOMPtr<nsIPrincipal> principalToInherit = loadInfo->PrincipalToInherit();
-      if (!principalToInherit) {
-        principalToInherit = loadInfo->TriggeringPrincipal();
-      }
+
+      nsCOMPtr<nsIPrincipal> principalToInherit =
+        loadInfo->FindPrincipalToInherit(aChannel);
       bool inheritForAboutBlank = loadInfo->GetAboutBlankInherits();
 
       if (nsContentUtils::ChannelShouldInheritPrincipal(principalToInherit,
                                                         uri,
                                                         inheritForAboutBlank,
                                                         false)) {
         principalToInherit.forget(aPrincipal);
         return NS_OK;
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -2500,20 +2500,20 @@ nsDocument::MaybeDowngradePrincipal(nsIP
     return nullptr;
   }
 
   // We can't load a document with an expanded principal. If we're given one,
   // automatically downgrade it to the last principal it subsumes (which is the
   // extension principal, in the case of extension content scripts).
   auto* basePrin = BasePrincipal::Cast(aPrincipal);
   if (basePrin->Is<ExpandedPrincipal>()) {
+    MOZ_DIAGNOSTIC_ASSERT(false, "Should never try to create a document with "
+                                 "an expanded principal");
+
     auto* expanded = basePrin->As<ExpandedPrincipal>();
-
-    MOZ_ASSERT(expanded->WhiteList().Length() > 0);
-
     return do_AddRef(expanded->WhiteList().LastElement());
   }
 
   if (!sChromeInContentPrefCached) {
     sChromeInContentPrefCached = true;
     Preferences::AddBoolVarCache(&sChromeInContentAllowed,
                                  kChromeInContentPref, false);
   }
--- a/dom/jsurl/nsJSProtocolHandler.cpp
+++ b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -153,20 +153,17 @@ nsresult nsJSThunk::EvaluateScript(nsICh
     // Get principal of code for execution
     nsCOMPtr<nsISupports> owner;
     aChannel->GetOwner(getter_AddRefs(owner));
     nsCOMPtr<nsIPrincipal> principal = do_QueryInterface(owner);
     if (!principal) {
         nsCOMPtr<nsILoadInfo> loadInfo;
         aChannel->GetLoadInfo(getter_AddRefs(loadInfo));
         if (loadInfo && loadInfo->GetForceInheritPrincipal()) {
-            principal = loadInfo->PrincipalToInherit();
-            if (!principal) {
-                principal = loadInfo->TriggeringPrincipal();
-            }
+            principal = loadInfo->FindPrincipalToInherit(aChannel);
         } else {
             // No execution without a principal!
             NS_ASSERTION(!owner, "Non-principal owner?");
             NS_WARNING("No principal to execute JS with");
             return NS_ERROR_DOM_RETVAL_UNDEFINED;
         }
     }
 
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2495,42 +2495,16 @@ XMLHttpRequestMainThread::CreateChannel(
 
     // Set the initiator type
     nsCOMPtr<nsITimedChannel> timedChannel(do_QueryInterface(httpChannel));
     if (timedChannel) {
       timedChannel->SetInitiatorType(NS_LITERAL_STRING("xmlhttprequest"));
     }
   }
 
-  // Using the provided principal as the triggeringPrincipal is fine, since we
-  // want to be able to access any of the origins that the principal has access
-  // to during the security checks, but we don't want a document to inherit an
-  // expanded principal, so in that case we need to select the principal in the
-  // expanded principal's whitelist that can load our URL as principalToInherit.
-  nsCOMPtr<nsIPrincipal> resultingDocumentPrincipal(mPrincipal);
-  nsCOMPtr<nsIExpandedPrincipal> ep = do_QueryInterface(mPrincipal);
-  if (ep) {
-    MOZ_ASSERT(!(secFlags & nsILoadInfo::SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS));
-    bool dataInherits = (secFlags &
-      (nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
-       nsILoadInfo::SEC_REQUIRE_CORS_DATA_INHERITS)) != 0;
-    for (const auto& principal : ep->WhiteList()) {
-      if (NS_SUCCEEDED(principal->CheckMayLoad(mRequestURL, false, dataInherits))) {
-        resultingDocumentPrincipal = principal;
-        break;
-      }
-    }
-  }
-
-  nsCOMPtr<nsILoadInfo> loadInfo = mChannel->GetLoadInfo();
-  if (loadInfo) {
-    rv = loadInfo->SetPrincipalToInherit(resultingDocumentPrincipal);
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
   return NS_OK;
 }
 
 void
 XMLHttpRequestMainThread::MaybeLowerChannelPriority()
 {
   nsCOMPtr<nsIDocument> doc = GetDocumentIfCurrent();
   if (!doc) {
--- a/netwerk/base/LoadInfo.cpp
+++ b/netwerk/base/LoadInfo.cpp
@@ -503,16 +503,36 @@ LoadInfo::SetPrincipalToInherit(nsIPrinc
 }
 
 nsIPrincipal*
 LoadInfo::PrincipalToInherit()
 {
   return mPrincipalToInherit;
 }
 
+nsIPrincipal*
+LoadInfo::FindPrincipalToInherit(nsIChannel* aChannel)
+{
+  if (mPrincipalToInherit) {
+    return mPrincipalToInherit;
+  }
+
+  nsCOMPtr<nsIURI> uri = mResultPrincipalURI;
+  if (!uri) {
+    Unused << aChannel->GetOriginalURI(getter_AddRefs(uri));
+  }
+
+  bool dataInherits = mSecurityFlags & (SEC_REQUIRE_SAME_ORIGIN_DATA_INHERITS |
+                                        SEC_ALLOW_CROSS_ORIGIN_DATA_INHERITS |
+                                        SEC_REQUIRE_CORS_DATA_INHERITS);
+
+  auto prin = BasePrincipal::Cast(mTriggeringPrincipal);
+  return prin->PrincipalToInherit(uri, dataInherits);
+}
+
 NS_IMETHODIMP
 LoadInfo::GetSandboxedLoadingPrincipal(nsIPrincipal** aPrincipal)
 {
   if (!(mSecurityFlags & nsILoadInfo::SEC_SANDBOXED)) {
     *aPrincipal = nullptr;
     return NS_OK;
   }
 
--- a/netwerk/base/nsILoadInfo.idl
+++ b/netwerk/base/nsILoadInfo.idl
@@ -2,16 +2,17 @@
  * vim: ft=cpp tw=78 sw=2 et ts=2 sts=2 cin
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsISupports.idl"
 #include "nsIContentPolicy.idl"
 
+interface nsIChannel;
 interface nsIDOMDocument;
 interface nsINode;
 interface nsIPrincipal;
 interface nsIRedirectHistoryEntry;
 interface nsIURI;
 %{C++
 #include "nsTArray.h"
 #include "mozilla/BasePrincipal.h"
@@ -295,16 +296,23 @@ interface nsILoadInfo : nsISupports
 
   /**
    * A C++-friendly version of principalToInherit.
    */
   [noscript, notxpcom, nostdcall, binaryname(PrincipalToInherit)]
   nsIPrincipal binaryPrincipalToInherit();
 
   /**
+   * Finds the correct principal to inherit for the given channel, based on
+   * the values of PrincipalToInherit and TriggeringPrincipal.
+   */
+  [noscript, notxpcom, nostdcall]
+  nsIPrincipal FindPrincipalToInherit(in nsIChannel aChannel);
+
+  /**
    * This is the ownerDocument of the LoadingNode. Unless the LoadingNode
    * is a Document, in which case the LoadingDocument is the same as the
    * LoadingNode.
    *
    * For top-level loads, and for loads originating from workers, the
    * LoadingDocument is null. When the LoadingDocument is not null, the
    * LoadingPrincipal is set to the principal of the LoadingDocument.
    */
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_triggeringPrincipal.js
@@ -259,16 +259,31 @@ function toHTML(test, opts) {
 function injectElements(tests, baseOpts) {
   window.addEventListener("load", () => {
     // Basic smoke test to check that SVG images do not try to create a document
     // with an expanded principal, which would cause a crash.
     let img = document.createElement("img");
     img.src = "data:image/svg+xml,%3Csvg%2F%3E";
     document.body.appendChild(img);
 
+    let rand = Math.random();
+
+    // Basic smoke test to check that we don't try to create stylesheets with an
+    // expanded principal, which would cause a crash when loading font sets.
+    let link = document.createElement("link");
+    link.rel = "stylesheet";
+    link.href = "data:text/css;base64," + btoa(`
+      @font-face {
+          font-family: "DoesNotExist${rand}";
+          src: url("fonts/DoesNotExist.${rand}.woff") format("woff");
+          font-weight: normal;
+          font-style: normal;
+      }`);
+    document.head.appendChild(link);
+
     let overrideOpts = opts => Object.assign({}, baseOpts, opts);
     let opts = baseOpts;
 
     // Build the full element with setAttr, then inject.
     for (let test of tests) {
       let {elem, srcElem, src} = createElement(test, opts);
       srcElem.setAttribute(test.srcAttr, src);
       document.body.appendChild(elem);