From b51e9d489e2595749b3c032eb7ed17b150026d0c Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Fri, 12 Oct 2018 09:36:33 +0200 Subject: [PATCH] Bug 1496034 - Apply bz's comments to FeaturePolicy, r=bz --- dom/base/nsDocument.cpp | 24 ++-- dom/html/HTMLIFrameElement.cpp | 84 +++++++------- dom/html/HTMLIFrameElement.h | 15 ++- dom/html/nsGenericHTMLFrameElement.cpp | 2 - dom/html/nsGenericHTMLFrameElement.h | 4 - dom/security/featurepolicy/Feature.cpp | 56 +++++---- dom/security/featurepolicy/Feature.h | 16 +-- dom/security/featurepolicy/FeaturePolicy.cpp | 99 +++++++++++++--- dom/security/featurepolicy/FeaturePolicy.h | 27 ++--- .../featurepolicy/FeaturePolicyParser.cpp | 40 +++---- .../featurepolicy/FeaturePolicyParser.h | 9 +- .../featurepolicy/FeaturePolicyUtils.cpp | 74 ++++-------- .../featurepolicy/FeaturePolicyUtils.h | 29 +++-- .../test/gtest/TestFeaturePolicyParser.cpp | 66 ++++++----- ...policy-allowed-for-self.https.sub.html.ini | 1 - ...rame-policy-allowed-for-all.https.sub.html | 85 ++++++++++++-- ...ame-policy-allowed-for-self.https.sub.html | 109 +++++++++++++++--- ...ame-policy-allowed-for-some.https.sub.html | 109 +++++++++++++++--- ...e-policy-disallowed-for-all.https.sub.html | 85 ++++++++++++-- .../feature-policy/resources/featurepolicy.js | 12 +- 20 files changed, 640 insertions(+), 306 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 452471d6f9c0..522d605519e6 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -3024,24 +3024,16 @@ nsIDocument::InitFeaturePolicy(nsIChannel* aChannel) return NS_OK; } - nsAutoString origin; - nsresult rv = nsContentUtils::GetUTFOrigin(NodePrincipal(), origin); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - mFeaturePolicy->SetDefaultOrigin(origin); + mFeaturePolicy->SetDefaultOrigin(NodePrincipal()); RefPtr parentPolicy = nullptr; if (mDocumentContainer) { nsPIDOMWindowOuter* containerWindow = mDocumentContainer->GetWindow(); if (containerWindow) { nsCOMPtr node = containerWindow->GetFrameElementInternal(); - if (node) { - HTMLIFrameElement* iframe = HTMLIFrameElement::FromNode(node); - if (iframe) { - parentPolicy = iframe->Policy(); - } + HTMLIFrameElement* iframe = HTMLIFrameElement::FromNodeOrNull(node); + if (iframe) { + parentPolicy = iframe->Policy(); } } } @@ -3052,7 +3044,7 @@ nsIDocument::InitFeaturePolicy(nsIChannel* aChannel) } nsCOMPtr httpChannel; - rv = GetHttpChannelHelper(aChannel, getter_AddRefs(httpChannel)); + nsresult rv = GetHttpChannelHelper(aChannel, getter_AddRefs(httpChannel)); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -3067,8 +3059,7 @@ nsIDocument::InitFeaturePolicy(nsIChannel* aChannel) value); if (NS_SUCCEEDED(rv)) { mFeaturePolicy->SetDeclaredPolicy(this, NS_ConvertUTF8toUTF16(value), - origin, EmptyString(), - false /* 'src' enabled */); + NodePrincipal(), nullptr); } return NS_OK; @@ -10231,7 +10222,8 @@ nsIDocument::Policy() const MOZ_ASSERT(StaticPrefs::dom_security_featurePolicy_enabled()); // The policy is created when the document is initialized. We _must_ have a - // policy here. + // policy here even if the featurePolicy pref is off. If this assertion fails, + // it means that ::Policy() is called before ::StartDocumentLoad(). MOZ_ASSERT(mFeaturePolicy); return mFeaturePolicy; } diff --git a/dom/html/HTMLIFrameElement.cpp b/dom/html/HTMLIFrameElement.cpp index d10038bf1d43..54cedee5b828 100644 --- a/dom/html/HTMLIFrameElement.cpp +++ b/dom/html/HTMLIFrameElement.cpp @@ -8,6 +8,7 @@ #include "mozilla/dom/HTMLIFrameElementBinding.h" #include "mozilla/dom/FeaturePolicy.h" #include "mozilla/MappedDeclarations.h" +#include "mozilla/NullPrincipal.h" #include "mozilla/StaticPrefs.h" #include "nsMappedAttributes.h" #include "nsAttrValueInlines.h" @@ -22,6 +23,24 @@ NS_IMPL_NS_NEW_HTML_ELEMENT_CHECK_PARSER(IFrame) namespace mozilla { namespace dom { +NS_IMPL_CYCLE_COLLECTION_CLASS(HTMLIFrameElement) + +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(HTMLIFrameElement, + nsGenericHTMLFrameElement) + NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFeaturePolicy) +NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END + +NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLIFrameElement, + nsGenericHTMLFrameElement) + NS_IMPL_CYCLE_COLLECTION_UNLINK(mFeaturePolicy) +NS_IMPL_CYCLE_COLLECTION_UNLINK_END + +NS_IMPL_ADDREF_INHERITED(HTMLIFrameElement, nsGenericHTMLFrameElement) +NS_IMPL_RELEASE_INHERITED(HTMLIFrameElement, nsGenericHTMLFrameElement) + +NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(HTMLIFrameElement) +NS_INTERFACE_MAP_END_INHERITING(nsGenericHTMLFrameElement) + // static const DOMTokenListSupportedToken HTMLIFrameElement::sSupportedSandboxTokens[] = { #define SANDBOX_KEYWORD(string, atom, flags) string, @@ -34,9 +53,8 @@ HTMLIFrameElement::HTMLIFrameElement(already_AddRefed&& FromParser aFromParser) : nsGenericHTMLFrameElement(std::move(aNodeInfo), aFromParser) { - if (StaticPrefs::dom_security_featurePolicy_enabled()) { - mFeaturePolicy = new FeaturePolicy(this); - } + // We always need a featurePolicy, even if not exposed. + mFeaturePolicy = new FeaturePolicy(this); } HTMLIFrameElement::~HTMLIFrameElement() @@ -174,6 +192,7 @@ HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsAtom* aName, } if ((aName == nsGkAtoms::allow || aName == nsGkAtoms::src || + aName == nsGkAtoms::srcdoc || aName == nsGkAtoms::sandbox || aName == nsGkAtoms::allowpaymentrequest) && StaticPrefs::dom_security_featurePolicy_enabled()) { @@ -236,41 +255,29 @@ HTMLIFrameElement::Policy() const } nsresult -HTMLIFrameElement::GetFeaturePolicyDefaultOrigin(nsAString& aDefaultOrigin) const +HTMLIFrameElement::GetFeaturePolicyDefaultOrigin(nsIPrincipal** aPrincipal) const { - aDefaultOrigin.Truncate(); + nsCOMPtr principal; - nsresult rv; - nsAutoString src; - GetURIAttr(nsGkAtoms::src, nullptr, src); + if (HasAttr(kNameSpaceID_None, nsGkAtoms::srcdoc)) { + principal = NodePrincipal(); + } - nsCOMPtr nodeURI; - if (!src.IsEmpty()) { - nsCOMPtr baseURI = OwnerDoc()->GetBaseURI(); - - rv = NS_NewURI(getter_AddRefs(nodeURI), src, - OwnerDoc()->GetDocumentCharacterSet(), - baseURI); - if (NS_WARN_IF(NS_FAILED(rv))) { - nodeURI = nullptr; + if (!principal) { + nsCOMPtr nodeURI; + if (GetURIAttr(nsGkAtoms::src, nullptr, getter_AddRefs(nodeURI)) && + nodeURI) { + principal = + BasePrincipal::CreateCodebasePrincipal(nodeURI, + BasePrincipal::Cast(NodePrincipal())->OriginAttributesRef()); } } - if (!nodeURI) { - if (OwnerDoc()->GetSandboxFlags() & SANDBOXED_ORIGIN) { - return NS_OK; - } - - nodeURI = OwnerDoc()->GetDocumentURI(); + if (!principal) { + principal = NodePrincipal(); } - nsAutoString origin; - rv = nsContentUtils::GetUTFOrigin(nodeURI, origin); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - - aDefaultOrigin.Assign(origin); + principal.forget(aPrincipal); return NS_OK; } @@ -281,29 +288,22 @@ HTMLIFrameElement::RefreshFeaturePolicy() mFeaturePolicy->ResetDeclaredPolicy(); // The origin can change if 'src' attribute changes. - nsAutoString origin; - nsresult rv = GetFeaturePolicyDefaultOrigin(origin); + nsCOMPtr origin; + nsresult rv = GetFeaturePolicyDefaultOrigin(getter_AddRefs(origin)); if (NS_WARN_IF(NS_FAILED(rv))) { return; } + MOZ_ASSERT(origin); mFeaturePolicy->SetDefaultOrigin(origin); nsAutoString allow; GetAttr(nsGkAtoms::allow, allow); if (!allow.IsEmpty()) { - nsAutoString documentOrigin; - if (OwnerDoc()->GetSandboxFlags() ^ SANDBOXED_ORIGIN) { - nsresult rv = nsContentUtils::GetUTFOrigin(NodePrincipal(), documentOrigin); - if (NS_WARN_IF(NS_FAILED(rv))) { - return; - } - } - // Set or reset the FeaturePolicy directives. - mFeaturePolicy->SetDeclaredPolicy(OwnerDoc(), allow, documentOrigin, - origin, true /* 'src' enabled */); + mFeaturePolicy->SetDeclaredPolicy(OwnerDoc(), allow, NodePrincipal(), + origin); } mFeaturePolicy->InheritPolicy(OwnerDoc()->Policy()); diff --git a/dom/html/HTMLIFrameElement.h b/dom/html/HTMLIFrameElement.h index 998597d89139..7e625da38461 100644 --- a/dom/html/HTMLIFrameElement.h +++ b/dom/html/HTMLIFrameElement.h @@ -8,6 +8,7 @@ #define mozilla_dom_HTMLIFrameElement_h #include "mozilla/Attributes.h" +#include "mozilla/dom/FeaturePolicy.h" #include "nsGenericHTMLFrameElement.h" #include "nsDOMTokenList.h" @@ -23,8 +24,9 @@ public: NS_IMPL_FROMNODE_HTML_WITH_TAG(HTMLIFrameElement, iframe) // nsISupports - NS_INLINE_DECL_REFCOUNTING_INHERITED(HTMLIFrameElement, - nsGenericHTMLFrameElement) + NS_DECL_ISUPPORTS_INHERITED + NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(HTMLIFrameElement, + nsGenericHTMLFrameElement) // Element virtual bool IsInteractiveHTMLContent(bool aIgnoreTabindex) const override @@ -223,8 +225,13 @@ private: void RefreshFeaturePolicy(); + // Implements the declared-origin algorithm as described in Feature-Policy + // spec: https://wicg.github.io/feature-policy/#declared-origin + // If this iframe has a 'src' attribute, the origin will be the parsing of its + // value as URL. If the URL is invalid, or 'src' attribute doesn't exist, the + // origin will be the document's origin. nsresult - GetFeaturePolicyDefaultOrigin(nsAString& aDefaultOrigin) const; + GetFeaturePolicyDefaultOrigin(nsIPrincipal** aDefaultOrigin) const; /** * This function is called by AfterSetAttr and OnAttrSetButNotChanged. @@ -236,6 +243,8 @@ private: * @param aNotify Whether we plan to notify document observers. */ void AfterMaybeChangeAttr(int32_t aNamespaceID, nsAtom* aName, bool aNotify); + + RefPtr mFeaturePolicy; }; } // namespace dom diff --git a/dom/html/nsGenericHTMLFrameElement.cpp b/dom/html/nsGenericHTMLFrameElement.cpp index 54d112ccc834..3e9c3f9863b8 100644 --- a/dom/html/nsGenericHTMLFrameElement.cpp +++ b/dom/html/nsGenericHTMLFrameElement.cpp @@ -35,7 +35,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(nsGenericHTMLFrameElement, NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFrameLoader) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mOpenerWindow) NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mBrowserElementAPI) - NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFeaturePolicy) NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsGenericHTMLFrameElement, @@ -47,7 +46,6 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsGenericHTMLFrameElement, NS_IMPL_CYCLE_COLLECTION_UNLINK(mFrameLoader) NS_IMPL_CYCLE_COLLECTION_UNLINK(mOpenerWindow) NS_IMPL_CYCLE_COLLECTION_UNLINK(mBrowserElementAPI) - NS_IMPL_CYCLE_COLLECTION_UNLINK(mFeaturePolicy) NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_ISUPPORTS_CYCLE_COLLECTION_INHERITED(nsGenericHTMLFrameElement, diff --git a/dom/html/nsGenericHTMLFrameElement.h b/dom/html/nsGenericHTMLFrameElement.h index 2dc46d6f899a..5fabc7856c0d 100644 --- a/dom/html/nsGenericHTMLFrameElement.h +++ b/dom/html/nsGenericHTMLFrameElement.h @@ -10,7 +10,6 @@ #include "mozilla/Attributes.h" #include "mozilla/ErrorResult.h" #include "mozilla/dom/nsBrowserElement.h" -#include "mozilla/dom/FeaturePolicy.h" #include "nsFrameLoader.h" #include "nsGenericHTMLElement.h" @@ -128,9 +127,6 @@ protected: nsCOMPtr mSrcTriggeringPrincipal; - // Used by