Bug 1224694 - Unify and clean up initialization of CSP (r=sicking)

caps/BasePrincipal.cpp

@@ -364,13 +364,25 @@ BasePrincipal::GetCsp(nsIContentSecurityPolicy** aCsp)
 }
 
 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;
 }
 
@@ -382,12 +394,25 @@ BasePrincipal::GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP)
 }
 
 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;
 }
 

caps/BasePrincipal.h

@@ -202,9 +202,9 @@ public:
   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;

caps/nsIPrincipal.idl

@@ -15,12 +15,13 @@ struct JSPrincipals;
 
 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
 {
     /**
@@ -133,12 +134,19 @@ interface nsIPrincipal : nsISerializable
     /**
      * 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
@@ -147,13 +155,19 @@ interface nsIPrincipal : nsISerializable
      *
      * 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.

caps/nsPrincipal.cpp

@@ -398,21 +398,15 @@ nsPrincipal::Read(nsIObjectInputStream* aStream)
   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;

caps/nsSystemPrincipal.cpp

@@ -70,7 +70,8 @@ nsSystemPrincipal::GetCsp(nsIContentSecurityPolicy** aCsp)
 }
 
 NS_IMETHODIMP
-nsSystemPrincipal::SetCsp(nsIContentSecurityPolicy* aCsp)
+nsSystemPrincipal::EnsureCSP(nsIDOMDocument* aDocument,
+                             nsIContentSecurityPolicy** aCSP)
 {
   // CSP on a system principal makes no sense
   return NS_OK;
@@ -84,7 +85,8 @@ nsSystemPrincipal::GetPreloadCsp(nsIContentSecurityPolicy** aPreloadCSP)
 }
 
 NS_IMETHODIMP
-nsSystemPrincipal::SetPreloadCsp(nsIContentSecurityPolicy* aPreloadCSP)
+nsSystemPrincipal::EnsurePreloadCSP(nsIDOMDocument* aDocument,
+                                    nsIContentSecurityPolicy** aPreloadCSP)
 {
   // CSP on a system principal makes no sense
   return NS_OK;

caps/nsSystemPrincipal.h

@@ -30,9 +30,9 @@ public:
   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;
 

dom/base/nsDocument.cpp

@@ -2785,19 +2785,8 @@ nsDocument::InitCSP(nsIChannel* aChannel)
     }
   }
 
-  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) {
@@ -2847,14 +2836,7 @@ nsDocument::InitCSP(nsIChannel* aChannel)
       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;
 }
 

dom/html/HTMLMetaElement.cpp

@@ -129,26 +129,13 @@ HTMLMetaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent,
 
       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

parser/html/nsHtml5TreeOpExecutor.cpp

@@ -1028,21 +1028,9 @@ nsHtml5TreeOpExecutor::AddSpeculationCSP(const nsAString& aCSP)
 
   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.