[CSP] Policy of window opener not applied to about:blank window
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Dec 2016 17:27:25 +0000 (17:27 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Dec 2016 17:27:25 +0000 (17:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165531
<rdar://problem/29426639>

Reviewed by Brent Fulgham.

Source/WebCore:

Fixes an issue where the content security policy of the opener document was not applied to
an about:blank window.

An about:blank window inherits its security origin from its opener document. It should also
copy (inherit) the ContentSecurityPolicy from its opener document. When copying the ContentSecurityPolicy
state from the opener document to the about:blank document we must take care to avoid copying
any upgrade-insecure-request directive because new windows should not inherit it by definition.
With respect to upgrade-insecure-requests, new windows should only inherit the insecure navigation set
from their opener document.

Test: http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window.html

* dom/Document.cpp:
(WebCore::Document::initContentSecurityPolicy): Copy the ContentSecurityPolicy state from the
owner document to this document when it inherits its security origin from its owner. An about:blank
window is one example of a document that inherits its security origin from its owner.
* loader/WorkerThreadableLoader.cpp:
(WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge): Call ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
to copy the upgrade insecure requests state from the owner document to the worker now that
ContentSecurityPolicy::copyStateFrom() no longer does this.
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::copyStateFrom): Do not copy the upgrade insecure request state.
Callers are now responsible for calling ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
to copy this state.
* page/csp/ContentSecurityPolicyDirectiveList.cpp:
(WebCore::ContentSecurityPolicyDirectiveList::parse): Ignore directive upgrade-insecure-requests when
inheriting ContentSecurityPolicy state as this directive as the Upgrade Insecure Requests feature has
its own inheritance semantics that differ from the semantics of copying a ContentSecurityPolicy object.
* xml/XSLTProcessor.cpp:
(WebCore::XSLTProcessor::createDocumentFromSource): Call ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
to copy the upgrade insecure requests state from the original document to the transformed document now
that ContentSecurityPolicy::copyStateFrom() no longer does this.

LayoutTests:

Add a test to ensure that an about:blank window inherits the CSP policy of its
opener document.

* http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-expected.txt: Added.
* http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-blocked.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@209608 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/loader/WorkerThreadableLoader.cpp
Source/WebCore/page/csp/ContentSecurityPolicy.cpp
Source/WebCore/page/csp/ContentSecurityPolicyDirectiveList.cpp
Source/WebCore/xml/XSLTProcessor.cpp

index 224aa88..5515bd0 100644 (file)
@@ -1,3 +1,17 @@
+2016-12-09  Daniel Bates  <dabates@apple.com>
+
+        [CSP] Policy of window opener not applied to about:blank window
+        https://bugs.webkit.org/show_bug.cgi?id=165531
+        <rdar://problem/29426639>
+
+        Reviewed by Brent Fulgham.
+
+        Add a test to ensure that an about:blank window inherits the CSP policy of its
+        opener document.
+
+        * http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-expected.txt: Added.
+        * http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-blocked.html: Added.
+
 2016-12-09  Antoine Quint  <graouts@apple.com>
 
         [Modern Media Controls] Remaining time label first appears way to the left
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-expected.txt b/LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window-expected.txt
new file mode 100644 (file)
index 0000000..e62784b
--- /dev/null
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: Refused to load http://127.0.0.1:8000/security/resources/abe.png because it does not appear in the img-src directive of the Content Security Policy.
+PASS did not load image.
+
diff --git a/LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window.html b/LayoutTests/http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window.html
new file mode 100644 (file)
index 0000000..4a0732e
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta http-equiv="Content-Security-Policy" content="script-src 'self' 'unsafe-inline'; img-src 'none'">
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.setCanOpenWindows();
+    testRunner.setCloseRemainingWindowsWhenComplete(true);
+    testRunner.waitUntilDone();
+}
+
+window.addEventListener("message", function (messageEvent) {
+    document.getElementById("console").textContent = messageEvent.data + "\n";
+    if (window.testRunner)
+        testRunner.notifyDone();
+}, false);
+</script>
+</head>
+<body>
+<pre id="console"></pre>
+<script>
+let childWindow = window.open("about:blank");
+childWindow.document.write(`<img src="../resources/abe.png" onerror="window.opener.postMessage('PASS did not load image.', '*')" onload="window.opener.postMessage('FAIL did load image.', '*')">`);
+</script>
+</body>
+</html>
index d1636e4..64975f4 100644 (file)
@@ -1,3 +1,44 @@
+2016-12-09  Daniel Bates  <dabates@apple.com>
+
+        [CSP] Policy of window opener not applied to about:blank window
+        https://bugs.webkit.org/show_bug.cgi?id=165531
+        <rdar://problem/29426639>
+
+        Reviewed by Brent Fulgham.
+
+        Fixes an issue where the content security policy of the opener document was not applied to
+        an about:blank window.
+
+        An about:blank window inherits its security origin from its opener document. It should also
+        copy (inherit) the ContentSecurityPolicy from its opener document. When copying the ContentSecurityPolicy
+        state from the opener document to the about:blank document we must take care to avoid copying
+        any upgrade-insecure-request directive because new windows should not inherit it by definition.
+        With respect to upgrade-insecure-requests, new windows should only inherit the insecure navigation set
+        from their opener document.
+
+        Test: http/tests/security/contentSecurityPolicy/image-blocked-in-about-blank-window.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::initContentSecurityPolicy): Copy the ContentSecurityPolicy state from the
+        owner document to this document when it inherits its security origin from its owner. An about:blank
+        window is one example of a document that inherits its security origin from its owner.
+        * loader/WorkerThreadableLoader.cpp:
+        (WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge): Call ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
+        to copy the upgrade insecure requests state from the owner document to the worker now that
+        ContentSecurityPolicy::copyStateFrom() no longer does this.
+        * page/csp/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::copyStateFrom): Do not copy the upgrade insecure request state.
+        Callers are now responsible for calling ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
+        to copy this state.
+        * page/csp/ContentSecurityPolicyDirectiveList.cpp:
+        (WebCore::ContentSecurityPolicyDirectiveList::parse): Ignore directive upgrade-insecure-requests when
+        inheriting ContentSecurityPolicy state as this directive as the Upgrade Insecure Requests feature has
+        its own inheritance semantics that differ from the semantics of copying a ContentSecurityPolicy object.
+        * xml/XSLTProcessor.cpp:
+        (WebCore::XSLTProcessor::createDocumentFromSource): Call ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom()
+        to copy the upgrade insecure requests state from the original document to the transformed document now
+        that ContentSecurityPolicy::copyStateFrom() no longer does this.
+
 2016-12-09  Antoine Quint  <graouts@apple.com>
 
         [Modern Media Controls] Remaining time label first appears way to the left
index edc05e0..ba55665 100644 (file)
@@ -5202,19 +5202,19 @@ void Document::initSecurityContext()
 
 void Document::initContentSecurityPolicy()
 {
-    if (!m_frame->tree().parent())
+    Frame* parentFrame = m_frame->tree().parent();
+    if (parentFrame)
+        contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*parentFrame->document()->contentSecurityPolicy());
+
+    if (!shouldInheritSecurityOriginFromOwner(m_url) && !isPluginDocument())
         return;
 
-    if (!shouldInheritSecurityOriginFromOwner(m_url) && !isPluginDocument()) {
-        // 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
-        ASSERT(m_frame->tree().parent()->document());
-        contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*m_frame->tree().parent()->document()->contentSecurityPolicy());
+    Frame* ownerFrame = parentFrame;
+    if (!ownerFrame)
+        ownerFrame = m_frame->loader().opener();
+    if (!ownerFrame)
         return;
-    }
-    
-    contentSecurityPolicy()->copyStateFrom(m_frame->tree().parent()->document()->contentSecurityPolicy());
+    contentSecurityPolicy()->copyStateFrom(ownerFrame->document()->contentSecurityPolicy()); // Does not copy Upgrade Insecure Requests state.
 }
 
 bool Document::isContextThread() const
index 1455a2e..998af58 100644 (file)
@@ -111,6 +111,7 @@ WorkerThreadableLoader::MainThreadBridge::MainThreadBridge(ThreadableLoaderClien
     auto securityOriginCopy = securityOrigin->isolatedCopy();
     auto contentSecurityPolicyCopy = std::make_unique<ContentSecurityPolicy>(securityOriginCopy);
     contentSecurityPolicyCopy->copyStateFrom(contentSecurityPolicy);
+    contentSecurityPolicyCopy->copyUpgradeInsecureRequestStateFrom(*contentSecurityPolicy);
 
     auto optionsCopy = std::make_unique<LoaderTaskOptions>(options, request.httpReferrer().isNull() ? outgoingReferrer : request.httpReferrer(), WTFMove(securityOriginCopy));
 
index 2c1f219..cb78d06 100644 (file)
@@ -113,8 +113,6 @@ void ContentSecurityPolicy::copyStateFrom(const ContentSecurityPolicy* other)
     ASSERT(m_policies.isEmpty());
     for (auto& policy : other->m_policies)
         didReceiveHeader(policy->header(), policy->headerType(), ContentSecurityPolicy::PolicyFrom::Inherited);
-
-    copyUpgradeInsecureRequestStateFrom(*other);
 }
 
 void ContentSecurityPolicy::copyUpgradeInsecureRequestStateFrom(const ContentSecurityPolicy& other)
index 79db7c2..4bdc6f1 100644 (file)
@@ -307,19 +307,18 @@ void ContentSecurityPolicyDirectiveList::parse(const String& policy, ContentSecu
         String name, value;
         if (parseDirective(directiveBegin, position, name, value)) {
             ASSERT(!name.isEmpty());
-            switch (policyFrom) {
-            case ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta:
+            if (policyFrom == ContentSecurityPolicy::PolicyFrom::Inherited) {
+                if (equalIgnoringASCIICase(name, ContentSecurityPolicyDirectiveNames::upgradeInsecureRequests))
+                    continue;
+            } else if (policyFrom == ContentSecurityPolicy::PolicyFrom::HTTPEquivMeta) {
                 if (equalIgnoringASCIICase(name, ContentSecurityPolicyDirectiveNames::sandbox)
                     || equalIgnoringASCIICase(name, ContentSecurityPolicyDirectiveNames::reportURI)
                     || equalIgnoringASCIICase(name, ContentSecurityPolicyDirectiveNames::frameAncestors)) {
                     m_policy.reportInvalidDirectiveInHTTPEquivMeta(name);
-                    break;
+                    continue;
                 }
-                FALLTHROUGH;
-            default:
-                addDirective(name, value);
-                break;
             }
+            addDirective(name, value);
         }
 
         ASSERT(position == end || *position == ';');
index 53aaccd..c285eb6 100644 (file)
@@ -94,6 +94,7 @@ Ref<Document> XSLTProcessor::createDocumentFromSource(const String& sourceString
             result->setFirstPartyForCookies(oldDocument->firstPartyForCookies());
             result->setStrictMixedContentMode(oldDocument->isStrictMixedContentMode());
             result->contentSecurityPolicy()->copyStateFrom(oldDocument->contentSecurityPolicy());
+            result->contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*oldDocument->contentSecurityPolicy());
         }
 
         frame->setDocument(result.copyRef());