Bug 1224694 - Unify and clean up initialization of CSP (r=sicking)
☠☠ backed out by 9bba3baa2ce7 ☠ ☠
authorChristoph Kerschbaumer <mozilla@christophkerschbaumer.com>
Wed, 13 Jan 2016 15:51:30 -0800
changeset 315030 f001a01c85d7979a9b9e44ee198587a772b06f46
parent 315029 9092fd77df19045999fb34c6db1744475c283e82
child 315031 f51b921e1ccfe4f341cebad569a599b2a9700111
push id5703
push userraliiev@mozilla.com
push dateMon, 07 Mar 2016 14:18:41 +0000
treeherdermozilla-beta@31e373ad5b5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs1224694
milestone46.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 1224694 - Unify and clean up initialization of CSP (r=sicking)
caps/BasePrincipal.cpp
caps/BasePrincipal.h
caps/nsIPrincipal.idl
caps/nsPrincipal.cpp
caps/nsSystemPrincipal.cpp
caps/nsSystemPrincipal.h
dom/base/nsDocument.cpp
dom/html/HTMLMetaElement.cpp
parser/html/nsHtml5TreeOpExecutor.cpp
--- a/caps/BasePrincipal.cpp
+++ b/caps/BasePrincipal.cpp
@@ -359,40 +359,65 @@ BasePrincipal::CheckMayLoad(nsIURI* aURI
 NS_IMETHODIMP
 BasePrincipal::GetCsp(nsIContentSecurityPolicy** aCsp)
 {
   NS_IF_ADDREF(*aCsp = mCSP);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-BasePrincipal::SetCsp(nsIContentSecurityPolicy* aCsp)
+BasePrincipal::EnsureCSP(nsIDOMDocument* aDocument,
+                         nsIContentSecurityPolicy** aCSP)
 {
   if (mCSP) {
-    return NS_ERROR_ALREADY_INITIALIZED;
+    // if there is a CSP already associated with this principal
+    // then just return that - do not overwrite it!!!
+    NS_IF_ADDREF(*aCSP = mCSP);
+    return NS_OK;
   }
 
-  mCSP = aCsp;
+  nsresult rv = NS_OK;
+  mCSP = do_CreateInstance("@mozilla.org/cspcontext;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Store the request context for violation reports
+  rv = aDocument ? mCSP->SetRequestContext(aDocument, nullptr)
+                 : mCSP->SetRequestContext(nullptr, this);
+  NS_ENSURE_SUCCESS(rv, rv);
+  NS_IF_ADDREF(*aCSP = mCSP);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP)
 {
   NS_IF_ADDREF(*aPreloadCSP = mPreloadCSP);
   return NS_OK;
 }
 
 NS_IMETHODIMP
-BasePrincipal::SetPreloadCsp(nsIContentSecurityPolicy* aPreloadCSP)
+BasePrincipal::EnsurePreloadCSP(nsIDOMDocument* aDocument,
+                                nsIContentSecurityPolicy** aPreloadCSP)
 {
   if (mPreloadCSP) {
-    return NS_ERROR_ALREADY_INITIALIZED;
+    // if there is a speculative CSP already associated with this principal
+    // then just return that - do not overwrite it!!!
+    NS_IF_ADDREF(*aPreloadCSP = mPreloadCSP);
+    return NS_OK;
   }
-  mPreloadCSP = aPreloadCSP;
+
+  nsresult rv = NS_OK;
+  mPreloadCSP = do_CreateInstance("@mozilla.org/cspcontext;1", &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // Store the request context for violation reports
+  rv = aDocument ? mPreloadCSP->SetRequestContext(aDocument, nullptr)
+                 : mPreloadCSP->SetRequestContext(nullptr, this);
+  NS_ENSURE_SUCCESS(rv, rv);
+  NS_IF_ADDREF(*aPreloadCSP = mPreloadCSP);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 BasePrincipal::GetCspJSON(nsAString& outCSPinJSON)
 {
   outCSPinJSON.Truncate();
   dom::CSPPolicies jsonPolicies;
--- a/caps/BasePrincipal.h
+++ b/caps/BasePrincipal.h
@@ -197,19 +197,19 @@ public:
   NS_IMETHOD GetOrigin(nsACString& aOrigin) final;
   NS_IMETHOD GetOriginNoSuffix(nsACString& aOrigin) final;
   NS_IMETHOD Equals(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD EqualsConsideringDomain(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD Subsumes(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD SubsumesConsideringDomain(nsIPrincipal* other, bool* _retval) final;
   NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, bool allowIfInheritsPrincipal) final;
   NS_IMETHOD GetCsp(nsIContentSecurityPolicy** aCsp) override;
-  NS_IMETHOD SetCsp(nsIContentSecurityPolicy* aCsp) override;
+  NS_IMETHOD EnsureCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP) override;
-  NS_IMETHOD SetPreloadCsp(nsIContentSecurityPolicy* aPreloadCSP) override;
+  NS_IMETHOD EnsurePreloadCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetCspJSON(nsAString& outCSPinJSON) override;
   NS_IMETHOD GetIsNullPrincipal(bool* aResult) override;
   NS_IMETHOD GetIsCodebasePrincipal(bool* aResult) override;
   NS_IMETHOD GetIsExpandedPrincipal(bool* aResult) override;
   NS_IMETHOD GetIsSystemPrincipal(bool* aResult) override;
   NS_IMETHOD GetJarPrefix(nsACString& aJarPrefix) final;
   NS_IMETHOD GetOriginAttributes(JSContext* aCx, JS::MutableHandle<JS::Value> aVal) final;
   NS_IMETHOD GetOriginSuffix(nsACString& aOriginSuffix) final;
--- a/caps/nsIPrincipal.idl
+++ b/caps/nsIPrincipal.idl
@@ -10,22 +10,23 @@
 %{C++
 struct JSPrincipals;
 #include "nsCOMPtr.h"
 #include "nsTArray.h"
 %}
 
 interface nsIURI;
 interface nsIContentSecurityPolicy;
+interface nsIDOMDocument;
 
 [ptr] native JSContext(JSContext);
 [ptr] native JSPrincipals(JSPrincipals);
 [ptr] native PrincipalArray(nsTArray<nsCOMPtr<nsIPrincipal> >);
 
-[scriptable, builtinclass, uuid(188fc4a2-3157-4956-a7a2-d674991770da)]
+[scriptable, builtinclass, uuid(d0391e86-1ad7-4ab0-bb7c-14d6d9967369)]
 interface nsIPrincipal : nsISerializable
 {
     /**
      * Returns whether the other principal is equivalent to this principal.
      * Principals are considered equal if they are the same principal, or
      * they have the same origin.
      */
     boolean equals(in nsIPrincipal other);
@@ -128,37 +129,50 @@ interface nsIPrincipal : nsISerializable
      * @throws NS_ERROR_DOM_BAD_URI if the load is not allowed.
      */
     void checkMayLoad(in nsIURI uri, in boolean report,
                       in boolean allowIfInheritsPrincipal);
 
     /**
      * A Content Security Policy associated with this principal.
      *
-     * Please note that if a csp was already set on the
-     * principal, then it should not be destroyed! Instead, the
-     * current csp should be quried and extended by
-     * calling AppendPolicy() on it.
+     * Use this function to query the associated CSP with this principal.
      */
-    [noscript] attribute nsIContentSecurityPolicy csp;
+    [noscript] readonly attribute nsIContentSecurityPolicy csp;
+
+    /*
+     * Use this function to query a CSP associated with this principal.
+     * If no CSP is associated with this principal then one is created
+     * internally and setRequestContext is called on the CSP using aDocument.
+     *
+     * Please note if aDocument is null, then setRequestContext on the
+     * CSP object is called using the current principal.
+     */
+    [noscript] nsIContentSecurityPolicy ensureCSP(in nsIDOMDocument aDocument);
 
     /**
      * A speculative Content Security Policy associated with this
      * principal. Set during speculative loading (preloading) and
      * used *only* for preloads.
      *
      * If you want to query the CSP associated with that principal,
      * then this is *not* what you want. Instead query 'csp'.
-     * 
-     * Please note that if a preloadCSP was already set on the
-     * principal, then it should not be destroyed! Instead, the
-     * current preloadCSP should be quried and extended by
-     * calling AppendPolicy() on it.
      */
-    [noscript] attribute nsIContentSecurityPolicy preloadCsp;
+    [noscript] readonly attribute nsIContentSecurityPolicy preloadCsp;
+
+    /*
+     * Use this function to query a speculative CSP associated with this
+     * principal. If no speculative CSP is associated with this principal
+     * then one is created internally and setRequestContext is called on
+     * the CSP using aDocument.
+     *
+     * Please note if aDocument is null, then setRequestContext on the
+     * speculative CSP object is called using the current principal.
+     */
+    [noscript] nsIContentSecurityPolicy ensurePreloadCSP(in nsIDOMDocument aDocument);
 
     /**
      * The CSP of the principal in JSON notation.
      * Note, that the CSP itself is not exposed to JS, but script
      * should be able to obtain a JSON representation of the CSP.
      */
     readonly attribute AString cspJSON;
 
--- a/caps/nsPrincipal.cpp
+++ b/caps/nsPrincipal.cpp
@@ -393,31 +393,25 @@ nsPrincipal::Read(nsIObjectInputStream* 
 
   PrincipalOriginAttributes attrs;
   bool ok = attrs.PopulateFromSuffix(suffix);
   NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
 
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // This may be null.
-  nsCOMPtr<nsIContentSecurityPolicy> csp = do_QueryInterface(supports, &rv);
+  // CSP might be null
+  mCSP = do_QueryInterface(supports, &rv);
+  if (mCSP) {
+    mCSP->SetRequestContext(nullptr, this);
+  }
 
   rv = Init(codebase, attrs);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = SetCsp(csp);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // need to link in the CSP context here (link in the URI of the protected
-  // resource).
-  if (csp) {
-    csp->SetRequestContext(nullptr, this);
-  }
-
   SetDomain(domain);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsPrincipal::Write(nsIObjectOutputStream* aStream)
 {
--- a/caps/nsSystemPrincipal.cpp
+++ b/caps/nsSystemPrincipal.cpp
@@ -65,31 +65,33 @@ nsSystemPrincipal::GetOriginInternal(nsA
 NS_IMETHODIMP
 nsSystemPrincipal::GetCsp(nsIContentSecurityPolicy** aCsp)
 {
   *aCsp = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsSystemPrincipal::SetCsp(nsIContentSecurityPolicy* aCsp)
+nsSystemPrincipal::EnsureCSP(nsIDOMDocument* aDocument,
+                             nsIContentSecurityPolicy** aCSP)
 {
   // CSP on a system principal makes no sense
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSystemPrincipal::GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP)
 {
   *aPreloadCSP = nullptr;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsSystemPrincipal::SetPreloadCsp(nsIContentSecurityPolicy* aPreloadCSP)
+nsSystemPrincipal::EnsurePreloadCSP(nsIDOMDocument* aDocument,
+                                    nsIContentSecurityPolicy** aPreloadCSP)
 {
   // CSP on a system principal makes no sense
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSystemPrincipal::GetDomain(nsIURI** aDomain)
 {
--- a/caps/nsSystemPrincipal.h
+++ b/caps/nsSystemPrincipal.h
@@ -25,19 +25,19 @@ class nsSystemPrincipal final : public m
 public:
   NS_DECL_NSISERIALIZABLE
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
   NS_IMETHOD GetHashValue(uint32_t* aHashValue) override;
   NS_IMETHOD GetURI(nsIURI** aURI) override;
   NS_IMETHOD GetDomain(nsIURI** aDomain) override;
   NS_IMETHOD SetDomain(nsIURI* aDomain) override;
   NS_IMETHOD GetCsp(nsIContentSecurityPolicy** aCsp) override;
-  NS_IMETHOD SetCsp(nsIContentSecurityPolicy* aCsp) override;
+  NS_IMETHOD EnsureCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP) override;
-  NS_IMETHOD SetPreloadCsp(nsIContentSecurityPolicy* aPreloadCSP) override;
+  NS_IMETHOD EnsurePreloadCSP(nsIDOMDocument* aDocument, nsIContentSecurityPolicy** aCSP) override;
   NS_IMETHOD GetBaseDomain(nsACString& aBaseDomain) override;
   nsresult GetOriginInternal(nsACString& aOrigin) override;
 
   nsSystemPrincipal() {}
 
   virtual void GetScriptLocation(nsACString &aStr) override;
 
 protected:
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -2780,29 +2780,18 @@ nsDocument::InitCSP(nsIChannel* aChannel
       MOZ_LOG(gCspPRLog, LogLevel::Debug, ("%s %s %s",
            "This document is sharing principal with another document.",
            "Since the document is an app, CSP was already set.",
            "Skipping attempt to set CSP."));
       return NS_OK;
     }
   }
 
-  csp = do_CreateInstance("@mozilla.org/cspcontext;1", &rv);
-
-  if (NS_FAILED(rv)) {
-    MOZ_LOG(gCspPRLog, LogLevel::Debug, ("Failed to create CSP object: %x", rv));
-    return rv;
-  }
-
-  // used as a "self" identifier for the CSP.
-  nsCOMPtr<nsIURI> selfURI;
-  aChannel->GetURI(getter_AddRefs(selfURI));
-
-  // Store the request context for violation reports
-  csp->SetRequestContext(this, nullptr);
+  rv = principal->EnsureCSP(this, getter_AddRefs(csp));
+  NS_ENSURE_SUCCESS(rv, rv);
 
   // ----- if the doc is an app and we want a default CSP, apply it.
   if (applyAppDefaultCSP) {
     csp->AppendPolicy(appDefaultCSP, false, false);
   }
 
   // ----- if the doc is an app and specifies a CSP in its manifest, apply it.
   if (applyAppManifestCSP) {
@@ -2842,24 +2831,17 @@ nsDocument::InitCSP(nsIChannel* aChannel
 
     if (NS_FAILED(rv) || !safeAncestry) {
       MOZ_LOG(gCspPRLog, LogLevel::Debug,
               ("CSP doesn't like frame's ancestry, not loading."));
       // stop!  ERROR page!
       aChannel->Cancel(NS_ERROR_CSP_FRAME_ANCESTOR_VIOLATION);
     }
   }
-
-  rv = principal->SetCsp(csp);
-  NS_ENSURE_SUCCESS(rv, rv);
-  MOZ_LOG(gCspPRLog, LogLevel::Debug,
-         ("Inserted CSP into principal %p", principal));
-
   ApplySettingsFromCSP(false);
-
   return NS_OK;
 }
 
 void
 nsDocument::StopDocumentLoad()
 {
   if (mParser) {
     mParserAborted = true;
--- a/dom/html/HTMLMetaElement.cpp
+++ b/dom/html/HTMLMetaElement.cpp
@@ -124,36 +124,23 @@ HTMLMetaElement::BindToTree(nsIDocument*
       
       nsAutoString content;
       rv = GetContent(content);
       NS_ENSURE_SUCCESS(rv, rv);
       content = nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(content);
 
       nsIPrincipal* principal = aDocument->NodePrincipal();
       nsCOMPtr<nsIContentSecurityPolicy> csp;
-      rv = principal->GetCsp(getter_AddRefs(csp));
+      nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(aDocument);
+      rv = principal->EnsureCSP(domDoc, getter_AddRefs(csp));
       NS_ENSURE_SUCCESS(rv, rv);
 
       // Multiple CSPs (delivered through either header of meta tag) need to be
       // joined together, see:
       // https://w3c.github.io/webappsec/specs/content-security-policy/#delivery-html-meta-element
-      if (!csp) {
-        csp = do_CreateInstance("@mozilla.org/cspcontext;1", &rv);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        // Store the request context so CSP can resolve 'self'
-        nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(aDocument);
-        rv = csp->SetRequestContext(domDoc, nullptr);
-        NS_ENSURE_SUCCESS(rv, rv);
-
-        // set the new CSP
-        rv = principal->SetCsp(csp);
-        NS_ENSURE_SUCCESS(rv, rv);
-      }
-
       rv = csp->AppendPolicy(content,
                              false, // csp via meta tag can not be report only
                              true); // delivered through the meta tag
       NS_ENSURE_SUCCESS(rv, rv);
 
       aDocument->ApplySettingsFromCSP(false);
     }
   }
--- a/parser/html/nsHtml5TreeOpExecutor.cpp
+++ b/parser/html/nsHtml5TreeOpExecutor.cpp
@@ -1023,31 +1023,19 @@ nsHtml5TreeOpExecutor::AddSpeculationCSP
   if (!CSPService::sCSPEnabled) {
     return;
   }
 
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
   nsIPrincipal* principal = mDocument->NodePrincipal();
   nsCOMPtr<nsIContentSecurityPolicy> preloadCsp;
-  nsresult rv = principal->GetPreloadCsp(getter_AddRefs(preloadCsp));
+  nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(mDocument);
+  nsresult rv = principal->EnsurePreloadCSP(domDoc, getter_AddRefs(preloadCsp));
   NS_ENSURE_SUCCESS_VOID(rv);
-  if (!preloadCsp) {
-    preloadCsp = do_CreateInstance("@mozilla.org/cspcontext;1", &rv);
-    NS_ENSURE_SUCCESS_VOID(rv);
-
-    // Store the request context for violation reports
-    nsCOMPtr<nsIDOMDocument> domDoc = do_QueryInterface(mDocument);
-    rv = preloadCsp->SetRequestContext(domDoc, nullptr);
-    NS_ENSURE_SUCCESS_VOID(rv);
-
-    // set the new csp
-    rv = principal->SetPreloadCsp(preloadCsp);
-    NS_ENSURE_SUCCESS_VOID(rv);
-  }
 
   // please note that meta CSPs and CSPs delivered through a header need
   // to be joined together.
   rv = preloadCsp->AppendPolicy(aCSP,
                                 false, // csp via meta tag can not be report only
                                 true); // delivered through the meta tag
   NS_ENSURE_SUCCESS_VOID(rv);