- Revision
- 246996
- Author
- [email protected]
- Date
- 2019-07-01 04:03:55 -0700 (Mon, 01 Jul 2019)
Log Message
Merge r246277 - [CSP] Blob URLs should inherit their CSP policy
https://bugs.webkit.org/show_bug.cgi?id=198579
<rdar://problem/51366878>
Reviewed by Brent Fulgham.
Source/WebCore:
As per <https://w3c.github.io/webappsec-csp/#security-inherit-csp> (Editor's Draft, 28 February 2019) blob
URLs should inherit their CSP policy from their parent (if they have one).
Test: http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html
http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html
* dom/Document.cpp:
(WebCore::Document::shouldInheritContentSecurityPolicyFromOwner const): Return true if the document's URL
is a Blob URL.
(WebCore::Document::initContentSecurityPolicy): Take a pointer to a ContentSecurityPolicy object that
represents the previous document's CSP. We only make us of this if the current URL is a Blob URL or a data
URL. Otherwise, do what we do now and take the policy from the owner frame.
* dom/Document.h:
* loader/DocumentWriter.cpp:
(WebCore::DocumentWriter::begin): Extend the lifetime of the previous document temporarily so that we can
pass its CSP to FrameLoader::didBeginDocument(). We need to do this extension because this function calls
FrameLoader::clear(), which can destroy the previous document and its ContentSecurityPolicy object. This
extension is also no different than if this function was called with a non-null ownerDocument except that
in that case it is the caller that extends the previous document's lifetime. Although it is tempting to
make use of ownerDocument to fix this bug by having the caller of begin() pass the previous document as
the ownerDocument when the new document's url (the one we are begin()ing) is a Blob URL. The ownerDocument
concept would privilege the Blob URL more than necessary; we only need to inherit the CSP policy from the
previous document for a Blob URL, not inherit the cookie URL or strict mixed content checking bit, etc.
We could make ContentSecurityPolicy ref-counted or even steal the ContentSecurityPolicy object from the
previous document. The latter is not of the question as a future enhancement, but the former seemed excessive
as a way to avoid extending the lifetime of the previous document because this would be the *only* call site
that actaully takes out a second ref of a ContentSecurityPolicy object. In general, shared ownership of
a ContentSecurityPolicy object does not make sense.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::didBeginDocument): Pass the specified content security policy through to
Document::initContentSecurityPolicy().
* loader/FrameLoader.h:
LayoutTests:
Add tests to ensure that a self navigation to a Blob or Data URL inherits its CSP policy from
its parent document.
* http/tests/security/contentSecurityPolicy/navigate-self-to-blob-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html: Added.
* http/tests/security/contentSecurityPolicy/navigate-self-to-data-url-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html: Added.
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/LayoutTests/ChangeLog 2019-07-01 11:03:55 UTC (rev 246996)
@@ -1,3 +1,19 @@
+2019-06-10 Daniel Bates <[email protected]>
+
+ [CSP] Blob URLs should inherit their CSP policy
+ https://bugs.webkit.org/show_bug.cgi?id=198579
+ <rdar://problem/51366878>
+
+ Reviewed by Brent Fulgham.
+
+ Add tests to ensure that a self navigation to a Blob or Data URL inherits its CSP policy from
+ its parent document.
+
+ * http/tests/security/contentSecurityPolicy/navigate-self-to-blob-expected.txt: Added.
+ * http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html: Added.
+ * http/tests/security/contentSecurityPolicy/navigate-self-to-data-url-expected.txt: Added.
+ * http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html: Added.
+
2019-06-05 Daniel Bates <[email protected]>
[CSP] Data URLs should inherit their CSP policy
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/ChangeLog 2019-07-01 11:03:55 UTC (rev 246996)
@@ -1,3 +1,44 @@
+2019-06-10 Daniel Bates <[email protected]>
+
+ [CSP] Blob URLs should inherit their CSP policy
+ https://bugs.webkit.org/show_bug.cgi?id=198579
+ <rdar://problem/51366878>
+
+ Reviewed by Brent Fulgham.
+
+ As per <https://w3c.github.io/webappsec-csp/#security-inherit-csp> (Editor's Draft, 28 February 2019) blob
+ URLs should inherit their CSP policy from their parent (if they have one).
+
+ Test: http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html
+ http/tests/security/contentSecurityPolicy/navigate-self-to-data-url.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::shouldInheritContentSecurityPolicyFromOwner const): Return true if the document's URL
+ is a Blob URL.
+ (WebCore::Document::initContentSecurityPolicy): Take a pointer to a ContentSecurityPolicy object that
+ represents the previous document's CSP. We only make us of this if the current URL is a Blob URL or a data
+ URL. Otherwise, do what we do now and take the policy from the owner frame.
+ * dom/Document.h:
+ * loader/DocumentWriter.cpp:
+ (WebCore::DocumentWriter::begin): Extend the lifetime of the previous document temporarily so that we can
+ pass its CSP to FrameLoader::didBeginDocument(). We need to do this extension because this function calls
+ FrameLoader::clear(), which can destroy the previous document and its ContentSecurityPolicy object. This
+ extension is also no different than if this function was called with a non-null ownerDocument except that
+ in that case it is the caller that extends the previous document's lifetime. Although it is tempting to
+ make use of ownerDocument to fix this bug by having the caller of begin() pass the previous document as
+ the ownerDocument when the new document's url (the one we are begin()ing) is a Blob URL. The ownerDocument
+ concept would privilege the Blob URL more than necessary; we only need to inherit the CSP policy from the
+ previous document for a Blob URL, not inherit the cookie URL or strict mixed content checking bit, etc.
+ We could make ContentSecurityPolicy ref-counted or even steal the ContentSecurityPolicy object from the
+ previous document. The latter is not of the question as a future enhancement, but the former seemed excessive
+ as a way to avoid extending the lifetime of the previous document because this would be the *only* call site
+ that actaully takes out a second ref of a ContentSecurityPolicy object. In general, shared ownership of
+ a ContentSecurityPolicy object does not make sense.
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::didBeginDocument): Pass the specified content security policy through to
+ Document::initContentSecurityPolicy().
+ * loader/FrameLoader.h:
+
2019-06-06 Brent Fulgham <[email protected]>
Avoid generating new XSLT-based document when already changing the document.
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.cpp (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.cpp 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.cpp 2019-07-01 11:03:55 UTC (rev 246996)
@@ -6009,12 +6009,13 @@
setSecurityOriginPolicy(ownerFrame->document()->securityOriginPolicy());
}
-bool Document::shouldInheritContentSecurityPolicyFromOwner() const
+// FIXME: The current criterion is stricter than <https://www.w3.org/TR/CSP3/#security-inherit-csp> (Editor's Draft, 28 February 2019).
+bool Document::shouldInheritContentSecurityPolicy() const
{
ASSERT(m_frame);
if (SecurityPolicy::shouldInheritSecurityOriginFromOwner(m_url))
return true;
- if (m_url.protocolIsData())
+ if (m_url.protocolIsData() || m_url.protocolIsBlob())
return true;
if (!isPluginDocument())
return false;
@@ -6026,7 +6027,7 @@
return openerFrame->document()->securityOrigin().canAccess(securityOrigin());
}
-void Document::initContentSecurityPolicy()
+void Document::initContentSecurityPolicy(ContentSecurityPolicy* previousPolicy)
{
// 1. Inherit Upgrade Insecure Requests
Frame* parentFrame = m_frame->tree().parent();
@@ -6034,19 +6035,27 @@
contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*parentFrame->document()->contentSecurityPolicy());
// 2. Inherit Content Security Policy (without copying Upgrade Insecure Requests state).
- if (!shouldInheritContentSecurityPolicyFromOwner())
+ if (!shouldInheritContentSecurityPolicy())
return;
- Frame* ownerFrame = parentFrame;
- if (!ownerFrame)
- ownerFrame = m_frame->loader().opener();
- if (!ownerFrame)
+ ContentSecurityPolicy* ownerPolicy = nullptr;
+ if (previousPolicy && (m_url.protocolIsData() || m_url.protocolIsBlob()))
+ ownerPolicy = previousPolicy;
+ if (!ownerPolicy) {
+ Frame* ownerFrame = parentFrame;
+ if (!ownerFrame)
+ ownerFrame = m_frame->loader().opener();
+ if (ownerFrame)
+ ownerPolicy = ownerFrame->document()->contentSecurityPolicy();
+ }
+ if (!ownerPolicy)
return;
- // FIXME: The CSP 3 spec. implies that only plugin documents delivered with a local scheme (e.g. blob, file, data)
- // should inherit a policy.
+ // FIXME: We are stricter than the CSP 3 spec. with regards to plugins: we prefer to inherit the full policy unless the plugin
+ // document is opened in a new window. The CSP 3 spec. implies that only plugin documents delivered with a local scheme (e.g. blob,
+ // file, data) should inherit a policy.
if (isPluginDocument() && m_frame->loader().opener())
- contentSecurityPolicy()->createPolicyForPluginDocumentFrom(*ownerFrame->document()->contentSecurityPolicy());
+ contentSecurityPolicy()->createPolicyForPluginDocumentFrom(*ownerPolicy);
else
- contentSecurityPolicy()->copyStateFrom(ownerFrame->document()->contentSecurityPolicy());
+ contentSecurityPolicy()->copyStateFrom(ownerPolicy);
}
bool Document::isContextThread() const
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.h (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.h 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/dom/Document.h 2019-07-01 11:03:55 UTC (rev 246996)
@@ -1132,7 +1132,7 @@
HashSet<SVGUseElement*> const svgUseElements() const { return m_svgUseElements; }
void initSecurityContext();
- void initContentSecurityPolicy();
+ void initContentSecurityPolicy(ContentSecurityPolicy* previousPolicy);
void updateURLForPushOrReplaceState(const URL&);
void statePopped(Ref<SerializedScriptValue>&&);
@@ -1544,7 +1544,7 @@
friend class IgnoreOpensDuringUnloadCountIncrementer;
friend class IgnoreDestructiveWriteCountIncrementer;
- bool shouldInheritContentSecurityPolicyFromOwner() const;
+ bool shouldInheritContentSecurityPolicy() const;
void updateTitleElement(Element& changingTitleElement);
void willDetachPage() final;
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/DocumentWriter.cpp (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/DocumentWriter.cpp 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/DocumentWriter.cpp 2019-07-01 11:03:55 UTC (rev 246996)
@@ -142,14 +142,12 @@
else
document->createDOMWindow();
- // Per <http://www.w3.org/TR/upgrade-insecure-requests/>, we need to retain an ongoing set of upgraded
- // requests in new navigation contexts. Although this information is present when we construct the
- // Document object, it is discard in the subsequent 'clear' statements below. So, we must capture it
- // so we can restore it.
- HashSet<SecurityOriginData> insecureNavigationRequestsToUpgrade;
- if (auto* existingDocument = m_frame->document())
- insecureNavigationRequestsToUpgrade = existingDocument->contentSecurityPolicy()->takeNavigationRequestsToUpgrade();
-
+ // Temporarily extend the lifetime of the existing document so that FrameLoader::clear() doesn't destroy it as
+ // we need to retain its ongoing set of upgraded requests in new navigation contexts per <http://www.w3.org/TR/upgrade-insecure-requests/>
+ // and we may also need to inherit its Content Security Policy in FrameLoader::didBeginDocument().
+ RefPtr<Document> existingDocument = m_frame->document();
+ auto* previousContentSecurityPolicy = existingDocument ? existingDocument->contentSecurityPolicy() : nullptr;
+
m_frame->loader().clear(document.ptr(), !shouldReuseDefaultView, !shouldReuseDefaultView);
clear();
@@ -164,7 +162,8 @@
m_frame->loader().setOutgoingReferrer(url);
m_frame->setDocument(document.copyRef());
- document->contentSecurityPolicy()->setInsecureNavigationRequestsToUpgrade(WTFMove(insecureNavigationRequestsToUpgrade));
+ if (previousContentSecurityPolicy)
+ document->contentSecurityPolicy()->setInsecureNavigationRequestsToUpgrade(previousContentSecurityPolicy->takeNavigationRequestsToUpgrade());
if (m_decoder)
document->setDecoder(m_decoder.get());
@@ -174,7 +173,7 @@
document->setStrictMixedContentMode(ownerDocument->isStrictMixedContentMode());
}
- m_frame->loader().didBeginDocument(dispatch);
+ m_frame->loader().didBeginDocument(dispatch, previousContentSecurityPolicy);
document->implicitOpen();
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.cpp (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.cpp 2019-07-01 11:03:55 UTC (rev 246996)
@@ -717,7 +717,7 @@
m_outgoingReferrer = url.strippedForUseAsReferrer();
}
-void FrameLoader::didBeginDocument(bool dispatch)
+void FrameLoader::didBeginDocument(bool dispatch, ContentSecurityPolicy* previousPolicy)
{
m_needsClear = true;
m_isComplete = false;
@@ -733,7 +733,7 @@
dispatchDidClearWindowObjectsInAllWorlds();
updateFirstPartyForCookies();
- m_frame.document()->initContentSecurityPolicy();
+ m_frame.document()->initContentSecurityPolicy(previousPolicy);
const Settings& settings = m_frame.settings();
m_frame.document()->cachedResourceLoader().setImagesEnabled(settings.areImagesEnabled());
Modified: releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.h (246995 => 246996)
--- releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.h 2019-07-01 11:03:50 UTC (rev 246995)
+++ releases/WebKitGTK/webkit-2.24/Source/WebCore/loader/FrameLoader.h 2019-07-01 11:03:55 UTC (rev 246996)
@@ -230,7 +230,7 @@
void didExplicitOpen();
// Callbacks from DocumentWriter
- void didBeginDocument(bool dispatchWindowObjectAvailable);
+ void didBeginDocument(bool dispatchWindowObjectAvailable, ContentSecurityPolicy* previousPolicy);
void receivedFirstData();
