Bug 1532207: Do not query the CSP from the principal within LoadFrame, but rather do not even set the Principal if it does not override the CSP within nsContentUtils::GetAttrTriggeringPrincipal. r=bzbarsky
authorChristoph Kerschbaumer <ckerschb@christophkerschbaumer.com>
Thu, 14 Mar 2019 11:39:04 +0000
changeset 521870 f101c9664b87
parent 521869 5da37483c3e2
child 521871 d8b7c29dcee0
push id10870
push usernbeleuzu@mozilla.com
push dateFri, 15 Mar 2019 20:00:07 +0000
treeherdermozilla-beta@c594aee5b7a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1532207
milestone67.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 1532207: Do not query the CSP from the principal within LoadFrame, but rather do not even set the Principal if it does not override the CSP within nsContentUtils::GetAttrTriggeringPrincipal. r=bzbarsky Differential Revision: https://phabricator.services.mozilla.com/D22121
dom/base/nsContentUtils.cpp
dom/base/nsFrameLoader.cpp
dom/base/nsFrameLoader.h
--- a/dom/base/nsContentUtils.cpp
+++ b/dom/base/nsContentUtils.cpp
@@ -1985,20 +1985,27 @@ nsIPrincipal* nsContentUtils::GetAttrTri
 
   // If the subject principal is the same as the content principal, or no
   // explicit subject principal was provided, we don't need to do any further
   // checks. Just return the content principal.
   if (contentPrin == aSubjectPrincipal || !aSubjectPrincipal) {
     return contentPrin;
   }
 
-  // If the attribute value is empty, it's not an absolute URL, so don't bother
-  // with more expensive checks.
-  if (!aAttrValue.IsEmpty() &&
-      IsAbsoluteURL(NS_ConvertUTF16toUTF8(aAttrValue))) {
+  // Only use the subject principal if the URL string we are going to end up
+  // fetching is under the control of that principal, which is never the case
+  // for relative URLs.
+  if (aAttrValue.IsEmpty() ||
+      !IsAbsoluteURL(NS_ConvertUTF16toUTF8(aAttrValue))) {
+    return contentPrin;
+  }
+
+  // Only use the subject principal as the attr triggering principal if it
+  // should override the CSP of the node's principal.
+  if (BasePrincipal::Cast(aSubjectPrincipal)->OverridesCSP(contentPrin)) {
     return aSubjectPrincipal;
   }
 
   return contentPrin;
 }
 
 // static
 bool nsContentUtils::IsAbsoluteURL(const nsACString& aURL) {
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -274,38 +274,47 @@ nsFrameLoader* nsFrameLoader::Create(
 
 void nsFrameLoader::LoadFrame(bool aOriginalSrc) {
   if (NS_WARN_IF(!mOwnerContent)) {
     return;
   }
 
   nsAutoString src;
   nsCOMPtr<nsIPrincipal> principal;
+  nsCOMPtr<nsIContentSecurityPolicy> csp;
 
   bool isSrcdoc = mOwnerContent->IsHTMLElement(nsGkAtoms::iframe) &&
                   mOwnerContent->HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc);
   if (isSrcdoc) {
     src.AssignLiteral("about:srcdoc");
     principal = mOwnerContent->NodePrincipal();
+    // Currently the NodePrincipal holds the CSP for a document. After
+    // Bug 965637 we can query the CSP from mOwnerContent->OwnerDoc()
+    // instead of mOwnerContent->NodePrincipal().
+    mOwnerContent->NodePrincipal()->GetCsp(getter_AddRefs(csp));
   } else {
-    GetURL(src, getter_AddRefs(principal));
+    GetURL(src, getter_AddRefs(principal), getter_AddRefs(csp));
 
     src.Trim(" \t\n\r");
 
     if (src.IsEmpty()) {
       // If the frame is a XUL element and has the attribute 'nodefaultsrc=true'
       // then we will not use 'about:blank' as fallback but return early without
       // starting a load if no 'src' attribute is given (or it's empty).
       if (mOwnerContent->IsXULElement() &&
           mOwnerContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::nodefaultsrc,
                                      nsGkAtoms::_true, eCaseMatters)) {
         return;
       }
       src.AssignLiteral("about:blank");
       principal = mOwnerContent->NodePrincipal();
+      // Currently the NodePrincipal holds the CSP for a document. After
+      // Bug 965637 we can query the CSP from mOwnerContent->OwnerDoc()
+      // instead of mOwnerContent->NodePrincipal().
+      mOwnerContent->NodePrincipal()->GetCsp(getter_AddRefs(csp));
     }
   }
 
   Document* doc = mOwnerContent->OwnerDoc();
   if (doc->IsStaticDocument()) {
     return;
   }
 
@@ -322,17 +331,17 @@ void nsFrameLoader::LoadFrame(bool aOrig
 
   // If the URI was malformed, try to recover by loading about:blank.
   if (rv == NS_ERROR_MALFORMED_URI) {
     rv = NS_NewURI(getter_AddRefs(uri), NS_LITERAL_STRING("about:blank"),
                    encoding, base_uri);
   }
 
   if (NS_SUCCEEDED(rv)) {
-    rv = LoadURI(uri, principal, aOriginalSrc);
+    rv = LoadURI(uri, principal, csp, aOriginalSrc);
   }
 
   if (NS_FAILED(rv)) {
     FireErrorEvent();
   }
 }
 
 void nsFrameLoader::FireErrorEvent() {
@@ -343,16 +352,17 @@ void nsFrameLoader::FireErrorEvent() {
       new LoadBlockingAsyncEventDispatcher(
           mOwnerContent, NS_LITERAL_STRING("error"), CanBubble::eNo,
           ChromeOnlyDispatch::eNo);
   loadBlockingAsyncDispatcher->PostDOMEvent();
 }
 
 nsresult nsFrameLoader::LoadURI(nsIURI* aURI,
                                 nsIPrincipal* aTriggeringPrincipal,
+                                nsIContentSecurityPolicy* aCsp,
                                 bool aOriginalSrc) {
   if (!aURI) return NS_ERROR_INVALID_POINTER;
   NS_ENSURE_STATE(!mDestroyCalled && mOwnerContent);
   MOZ_ASSERT(
       aTriggeringPrincipal,
       "Must have an explicit triggeringPrincipal to nsFrameLoader::LoadURI.");
 
   mLoadingOriginalSrc = aOriginalSrc;
@@ -360,20 +370,22 @@ nsresult nsFrameLoader::LoadURI(nsIURI* 
   nsCOMPtr<Document> doc = mOwnerContent->OwnerDoc();
 
   nsresult rv;
   rv = CheckURILoad(aURI, aTriggeringPrincipal);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mURIToLoad = aURI;
   mTriggeringPrincipal = aTriggeringPrincipal;
+  mCsp = aCsp;
   rv = doc->InitializeFrameLoader(this);
   if (NS_FAILED(rv)) {
     mURIToLoad = nullptr;
     mTriggeringPrincipal = nullptr;
+    mCsp = nullptr;
   }
   return rv;
 }
 
 nsresult nsFrameLoader::ReallyStartLoading() {
   nsresult rv = ReallyStartLoadingInternal();
   if (NS_FAILED(rv)) {
     FireErrorEvent();
@@ -435,32 +447,31 @@ nsresult nsFrameLoader::ReallyStartLoadi
   // is very important; needed to prevent XSS attacks on documents loaded in
   // subframes!
   if (mTriggeringPrincipal) {
     loadState->SetTriggeringPrincipal(mTriggeringPrincipal);
   } else {
     loadState->SetTriggeringPrincipal(mOwnerContent->NodePrincipal());
   }
 
-  // Expanded Principals override the CSP of the document, hence we first check
-  // if the triggeringPrincipal overrides the document's principal. If so, let's
-  // query the CSP from that Principal, otherwise we use the document's CSP.
-  // Note that even after Bug 965637, Expanded Principals will hold their own
-  // CSP.
-  nsCOMPtr<nsIContentSecurityPolicy> csp;
-  if (BasePrincipal::Cast(loadState->TriggeringPrincipal())
-          ->OverridesCSP(mOwnerContent->NodePrincipal())) {
-    loadState->TriggeringPrincipal()->GetCsp(getter_AddRefs(csp));
-  } else {
+  // If we have an explicit CSP, we set it. If not, we only query it from
+  // the NodePrincipal in case there was no explicit triggeringPrincipal.
+  // Otherwise it's possible that the original triggeringPrincipal did not
+  // have a CSP which causes the CSP on the Principal and explicit CSP
+  // to be out of sync.
+  if (mCsp) {
+    loadState->SetCsp(mCsp);
+  } else if (!mTriggeringPrincipal) {
     // Currently the NodePrincipal holds the CSP for a document. After
     // Bug 965637 we can query the CSP from mOwnerContent->OwnerDoc()
     // instead of mOwnerContent->NodePrincipal().
+    nsCOMPtr<nsIContentSecurityPolicy> csp;
     mOwnerContent->NodePrincipal()->GetCsp(getter_AddRefs(csp));
+    loadState->SetCsp(csp);
   }
-  loadState->SetCsp(csp);
 
   nsCOMPtr<nsIURI> referrer;
 
   nsAutoString srcdoc;
   bool isSrcdoc =
       mOwnerContent->IsHTMLElement(nsGkAtoms::iframe) &&
       mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::srcdoc, srcdoc);
 
@@ -2187,35 +2198,46 @@ nsresult nsFrameLoader::MaybeCreateDocSh
   nsCOMPtr<nsIObserverService> os = services::GetObserverService();
   if (os) {
     os->NotifyObservers(ToSupports(this), "inprocess-browser-shown", nullptr);
   }
 
   return NS_OK;
 }
 
-void nsFrameLoader::GetURL(nsString& aURI,
-                           nsIPrincipal** aTriggeringPrincipal) {
+void nsFrameLoader::GetURL(nsString& aURI, nsIPrincipal** aTriggeringPrincipal,
+                           nsIContentSecurityPolicy** aCsp) {
   aURI.Truncate();
+  // Within this function we default to using the NodePrincipal as the
+  // triggeringPrincipal and the CSP of the document, currently stored
+  // on the NodePrincipal. After Bug 965637 we can query the CSP from
+  // mOwnerContent->OwnerDoc() instead of mOwnerContent->NodePrincipal().
+  // Expanded Principals however override the CSP of the document, hence
+  // if frame->GetSrcTriggeringPrincipal() returns a valid principal, we
+  // have to query the CSP from that Principal. Note that even after
+  // Bug 965637, Expanded Principals will hold their own CSP.
+  nsCOMPtr<nsIPrincipal> triggeringPrincipal = mOwnerContent->NodePrincipal();
+  nsCOMPtr<nsIContentSecurityPolicy> csp;
+  mOwnerContent->NodePrincipal()->GetCsp(getter_AddRefs(csp));
 
   if (mOwnerContent->IsHTMLElement(nsGkAtoms::object)) {
     mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::data, aURI);
-    nsCOMPtr<nsIPrincipal> prin = mOwnerContent->NodePrincipal();
-    prin.forget(aTriggeringPrincipal);
   } else {
     mOwnerContent->GetAttr(kNameSpaceID_None, nsGkAtoms::src, aURI);
     if (RefPtr<nsGenericHTMLFrameElement> frame =
             do_QueryObject(mOwnerContent)) {
-      nsCOMPtr<nsIPrincipal> prin = frame->GetSrcTriggeringPrincipal();
-      prin.forget(aTriggeringPrincipal);
-    } else {
-      nsCOMPtr<nsIPrincipal> prin = mOwnerContent->NodePrincipal();
-      prin.forget(aTriggeringPrincipal);
+      nsCOMPtr<nsIPrincipal> srcPrincipal = frame->GetSrcTriggeringPrincipal();
+      if (srcPrincipal) {
+        triggeringPrincipal = srcPrincipal;
+        triggeringPrincipal->GetCsp(getter_AddRefs(csp));
+      }
     }
   }
+  triggeringPrincipal.forget(aTriggeringPrincipal);
+  csp.forget(aCsp);
 }
 
 nsresult nsFrameLoader::CheckForRecursiveLoad(nsIURI* aURI) {
   nsresult rv;
 
   MOZ_ASSERT(!IsRemoteFrame(),
              "Shouldn't call CheckForRecursiveLoad on remote frames.");
 
--- a/dom/base/nsFrameLoader.h
+++ b/dom/base/nsFrameLoader.h
@@ -145,19 +145,23 @@ class nsFrameLoader final : public nsStu
   /**
    * Loads the specified URI in this frame. Behaves identically to loadFrame,
    * except that this method allows specifying the URI to load.
    *
    * @param aURI The URI to load.
    * @param aTriggeringPrincipal The triggering principal for the load. May be
    *        null, in which case the node principal of the owner content will be
    *        used.
+   * @param aCsp The CSP to be used for the load. That is not the CSP to be
+   *        applied to subresources within the frame, but to the iframe load
+   *        itself. E.g. if the CSP holds upgrade-insecure-requests the the
+   *        frame load is upgraded from http to https.
    */
   nsresult LoadURI(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal,
-                   bool aOriginalSrc);
+                   nsIContentSecurityPolicy* aCsp, bool aOriginalSrc);
 
   /**
    * Destroy the frame loader and everything inside it. This will
    * clear the weak owner content reference.
    */
   void Destroy();
 
   void ActivateRemoteFrame(mozilla::ErrorResult& aRv);
@@ -342,17 +346,18 @@ class nsFrameLoader final : public nsStu
 
   /**
    * Applies a new set of sandbox flags. These are merged with the sandbox
    * flags from our owning content's owning document with a logical OR, this
    * ensures that we can only add restrictions and never remove them.
    */
   void ApplySandboxFlags(uint32_t sandboxFlags);
 
-  void GetURL(nsString& aURL, nsIPrincipal** aTriggeringPrincipal);
+  void GetURL(nsString& aURL, nsIPrincipal** aTriggeringPrincipal,
+              nsIContentSecurityPolicy** aCsp);
 
   // Properly retrieves documentSize of any subdocument type.
   nsresult GetWindowDimensions(nsIntRect& aRect);
 
   virtual mozilla::dom::ProcessMessageManager* GetProcessMessageManager()
       const override;
 
   // public because a callback needs these.
@@ -436,16 +441,17 @@ class nsFrameLoader final : public nsStu
   enum TabParentChange { eTabParentRemoved, eTabParentChanged };
   void MaybeUpdatePrimaryTabParent(TabParentChange aChange);
 
   nsresult PopulateUserContextIdFromAttribute(mozilla::OriginAttributes& aAttr);
 
   RefPtr<nsDocShell> mDocShell;
   nsCOMPtr<nsIURI> mURIToLoad;
   nsCOMPtr<nsIPrincipal> mTriggeringPrincipal;
+  nsCOMPtr<nsIContentSecurityPolicy> mCsp;
   mozilla::dom::Element* mOwnerContent;  // WEAK
 
   // After the frameloader has been removed from the DOM but before all of the
   // messages from the frame have been received, we keep a strong reference to
   // our <browser> element.
   RefPtr<mozilla::dom::Element> mOwnerContentStrong;
 
   // Stores the root frame of the subdocument while the subdocument is being