From 9c6f72b395b0d990287ea3053d3095e520496f53 Mon Sep 17 00:00:00 2001 From: Christoph Kerschbaumer Date: Wed, 13 Jan 2016 15:51:30 -0800 Subject: [PATCH] Bug 1224694 - Unify and clean up initialization of CSP (r=sicking) --- caps/BasePrincipal.cpp | 37 +++++++++++++++++++++----- caps/BasePrincipal.h | 4 +-- caps/nsIPrincipal.idl | 38 ++++++++++++++++++--------- caps/nsPrincipal.cpp | 16 ++++------- caps/nsSystemPrincipal.cpp | 6 +++-- caps/nsSystemPrincipal.h | 4 +-- dom/base/nsDocument.cpp | 22 ++-------------- dom/html/HTMLMetaElement.cpp | 17 ++---------- parser/html/nsHtml5TreeOpExecutor.cpp | 16 ++--------- 9 files changed, 76 insertions(+), 84 deletions(-) diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index 6db72f8e6cb..052571d6995 100644 --- a/caps/BasePrincipal.cpp +++ b/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; } diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index 16c68b1a7d8..b4b9cfd88df 100644 --- a/caps/BasePrincipal.h +++ b/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; diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index 982cb6496c6..98a97bba110 100644 --- a/caps/nsIPrincipal.idl +++ b/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 >); -[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. diff --git a/caps/nsPrincipal.cpp b/caps/nsPrincipal.cpp index 50f63874ad1..f5834730816 100644 --- a/caps/nsPrincipal.cpp +++ b/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 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; diff --git a/caps/nsSystemPrincipal.cpp b/caps/nsSystemPrincipal.cpp index e8f52f1869d..1711f367f6b 100644 --- a/caps/nsSystemPrincipal.cpp +++ b/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; diff --git a/caps/nsSystemPrincipal.h b/caps/nsSystemPrincipal.h index 03e9175f1a4..8c57196c760 100644 --- a/caps/nsSystemPrincipal.h +++ b/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; diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 4d054f00dab..59e68394132 100644 --- a/dom/base/nsDocument.cpp +++ b/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 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; } diff --git a/dom/html/HTMLMetaElement.cpp b/dom/html/HTMLMetaElement.cpp index 63880cff960..43371db34fb 100644 --- a/dom/html/HTMLMetaElement.cpp +++ b/dom/html/HTMLMetaElement.cpp @@ -129,26 +129,13 @@ HTMLMetaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsIPrincipal* principal = aDocument->NodePrincipal(); nsCOMPtr csp; - rv = principal->GetCsp(getter_AddRefs(csp)); + nsCOMPtr 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 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 diff --git a/parser/html/nsHtml5TreeOpExecutor.cpp b/parser/html/nsHtml5TreeOpExecutor.cpp index c9659f1b8d8..72c710edba8 100644 --- a/parser/html/nsHtml5TreeOpExecutor.cpp +++ b/parser/html/nsHtml5TreeOpExecutor.cpp @@ -1028,21 +1028,9 @@ nsHtml5TreeOpExecutor::AddSpeculationCSP(const nsAString& aCSP) nsIPrincipal* principal = mDocument->NodePrincipal(); nsCOMPtr preloadCsp; - nsresult rv = principal->GetPreloadCsp(getter_AddRefs(preloadCsp)); + nsCOMPtr 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 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.