Bug 1224694 - Unify and clean up initialization of CSP (r=sicking)
authorChristoph Kerschbaumer <mozilla@christophkerschbaumer.com>
Thu, 14 Jan 2016 13:21:31 -0800
changeset 315216 febf0e69c996e1a6678e6d7877e6ef07e0011b2c
parent 315215 4a9aa8e075958ba033e5fd618a4241d147d2e67f
child 315217 4f911c4f9072420068e2d3a8f7277c8d69d6baf8
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
dom/security/nsCSPContext.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,29 +393,24 @@ 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);
-
   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);
+  mCSP = do_QueryInterface(supports, &rv);
+  // make sure setRequestContext is called after Init(),
+  // to make sure  the principals URI been initalized.
+  if (mCSP) {
+    mCSP->SetRequestContext(nullptr, this);
   }
 
   SetDomain(domain);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
--- 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/dom/security/nsCSPContext.cpp
+++ b/dom/security/nsCSPContext.cpp
@@ -405,17 +405,19 @@ nsCSPContext::reportInlineViolation(nsCo
   nsCOMPtr<nsISupportsCString> selfICString(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
   if (selfICString) {
     selfICString->SetData(nsDependentCString("self"));
   }
   nsCOMPtr<nsISupports> selfISupports(do_QueryInterface(selfICString));
 
   // use selfURI as the sourceFile
   nsAutoCString sourceFile;
-  mSelfURI->GetSpec(sourceFile);
+  if (mSelfURI) {
+    mSelfURI->GetSpec(sourceFile);
+  }
 
   nsAutoString codeSample(aContent);
   // cap the length of the script sample at 40 chars
   if (codeSample.Length() > 40) {
     codeSample.Truncate(40);
     codeSample.AppendLiteral("...");
   }
   AsyncReportViolation(selfISupports,                      // aBlockedContentSource
--- 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);