Attachment 342503 Details for Bug 186550 – Patch

[patch] Patch

bug-186550-20180611194534.patch (text/plain), 7.87 KB, created by youenn fablet on 2018-06-11 19:45:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-06-11 19:45:39 PDT

Size: 7.87 KB

patch

obsolete

>Subversion Revision: 232735
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 6f1d49b7a43ee108a119594ad73f88da92d8b01d..45b93b8993da0cd2913a7aacbf09070165791ccc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-06-11  Youenn Fablet  <youenn@apple.com>
>+
>+        Apply CSP checks before Content blocker checks in NetworkLoadChecker as done by CachedResourceLoader
>+        https://bugs.webkit.org/show_bug.cgi?id=186550
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Do CSP checks and URL upgrade before content blocker checks.
>+
>+        * NetworkProcess/NetworkLoadChecker.cpp:
>+        (WebKit::NetworkLoadChecker::checkRequest):
>+        (WebKit::NetworkLoadChecker::continueCheckingRequest):
>+
> 2018-06-11  Youenn Fablet  <youenn@apple.com>
> 
>         Improve error messages in case FetchEvent.respondWith has a rejected promise
>diff --git a/Source/WebCore/page/csp/ContentSecurityPolicy.h b/Source/WebCore/page/csp/ContentSecurityPolicy.h
>index aa134dcd453fff88fa80296052f87818cdade6bd..4da45d4bcaa12115013003c97fd3e803cd53400d 100644
>--- a/Source/WebCore/page/csp/ContentSecurityPolicy.h
>+++ b/Source/WebCore/page/csp/ContentSecurityPolicy.h
>@@ -162,7 +162,7 @@ public:
>     void setUpgradeInsecureRequests(bool);
>     bool upgradeInsecureRequests() const { return m_upgradeInsecureRequests; }
>     enum class InsecureRequestType { Load, FormSubmission, Navigation };
>-    void upgradeInsecureRequestIfNeeded(ResourceRequest&, InsecureRequestType) const;
>+    WEBCORE_EXPORT void upgradeInsecureRequestIfNeeded(ResourceRequest&, InsecureRequestType) const;
>     WEBCORE_EXPORT void upgradeInsecureRequestIfNeeded(URL&, InsecureRequestType) const;
> 
>     HashSet<SecurityOriginData> takeNavigationRequestsToUpgrade();
>diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>index d0f21256534a9be2de01838b9e0ba6965caa074d..7ab89e89a75f2120bd83d07ca662693bdd7b0c14 100644
>--- a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
>@@ -193,6 +193,17 @@ auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message
> 
> void NetworkLoadChecker::checkRequest(ResourceRequest&& request, ValidationHandler&& handler)
> {
>+    if (auto* contentSecurityPolicy = this->contentSecurityPolicy()) {
>+        if (isRedirected()) {
>+            auto type = m_options.mode == FetchOptions::Mode::Navigate ? ContentSecurityPolicy::InsecureRequestType::Navigation : ContentSecurityPolicy::InsecureRequestType::Load;
>+            contentSecurityPolicy->upgradeInsecureRequestIfNeeded(request, type);
>+        }
>+        if (!isAllowedByContentSecurityPolicy(request)) {
>+            handler(accessControlErrorForValidationHandler(ASCIILiteral { "Blocked by Content Security Policy." }));
>+            return;
>+        }
>+    }
>+
> #if ENABLE(CONTENT_EXTENSIONS)
>     processContentExtensionRulesForLoad(WTFMove(request), [this, handler = WTFMove(handler)](auto result) mutable {
>         if (!result.has_value()) {
>@@ -248,20 +259,6 @@ bool NetworkLoadChecker::isAllowedByContentSecurityPolicy(const ResourceRequest&
> 
> void NetworkLoadChecker::continueCheckingRequest(ResourceRequest&& request, ValidationHandler&& handler)
> {
>-    if (auto* contentSecurityPolicy = this->contentSecurityPolicy()) {
>-        if (isRedirected()) {
>-            URL url = request.url();
>-            auto type = m_options.mode == FetchOptions::Mode::Navigate ? ContentSecurityPolicy::InsecureRequestType::Navigation : ContentSecurityPolicy::InsecureRequestType::Load;
>-            contentSecurityPolicy->upgradeInsecureRequestIfNeeded(url, type);
>-            if (url != request.url())
>-                request.setURL(url);
>-        }
>-        if (!isAllowedByContentSecurityPolicy(request)) {
>-            handler(accessControlErrorForValidationHandler(ASCIILiteral { "Blocked by Content Security Policy." }));
>-            return;
>-        }
>-    }
>-
>     if (m_options.credentials == FetchOptions::Credentials::SameOrigin)
>         m_storedCredentialsPolicy = m_isSameOriginRequest && m_origin->canRequest(request.url()) ? StoredCredentialsPolicy::Use : StoredCredentialsPolicy::DoNotUse;
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 04d04b0e424b3fc90c6e9a69b714d52ff3676c32..18186eeff5e9fdba6cf0a19096efe3ef3fde65f7 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-11  Youenn Fablet  <youenn@apple.com>
>+
>+        Apply CSP checks before Content blocker checks in NetworkLoadChecker as done by CachedResourceLoader
>+        https://bugs.webkit.org/show_bug.cgi?id=186550
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/contentextensions/fetch-redirect-blocked-expected.txt: Added.
>+        * http/tests/contentextensions/fetch-redirect-blocked.html: Added.
>+        * http/tests/contentextensions/fetch-redirect-blocked.html.json: Added.
>+        * http/tests/contentextensions/resources/subresource-redirect.php:
>+
> 2018-06-11  Youenn Fablet  <youenn@apple.com>
> 
>         Improve error messages in case FetchEvent.respondWith has a rejected promise
>diff --git a/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked-expected.txt b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..6fcf5b27f113d30a2778a69ae4763271a79b676f
>--- /dev/null
>+++ b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked-expected.txt
>@@ -0,0 +1,6 @@
>+CONSOLE MESSAGE: Refused to connect to http://localhost:8000/resources/square128.png because it does not appear in the connect-src directive of the Content Security Policy.
>+CONSOLE MESSAGE: Blocked by Content Security Policy.
>+CONSOLE MESSAGE: Fetch API cannot load http://localhost:8000/resources/square128.png due to access control checks.
>+
>+PASS Ensure CSP happens before content blocker checks 
>+
>diff --git a/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2e1d4e8eebeb6f426d8936e52f519b9fd414e8f8
>--- /dev/null
>+++ b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html
>@@ -0,0 +1,13 @@
>+<meta http-equiv="Content-Security-Policy" content="connect-src 'self';">
>+<script src="/resources/testharness.js"></script>
>+<script src="/resources/testharnessreport.js"></script>
>+<script>
>+promise_test((test) => {
>+
>+    return fetch("resources/subresource-redirect.php", { mode : "no-cors" }).then(() => {
>+        assert_not_reached("fetch should fail");
>+    }, (e) => {
>+        assert_equals(e.message, "Blocked by Content Security Policy.");
>+    });
>+}, "Ensure CSP happens before content blocker checks");
>+</script>
>diff --git a/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html.json b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html.json
>new file mode 100644
>index 0000000000000000000000000000000000000000..8549148678e70e261bcb4f888a2265b48958f8cf
>--- /dev/null
>+++ b/LayoutTests/http/tests/contentextensions/fetch-redirect-blocked.html.json
>@@ -0,0 +1,10 @@
>+[
>+    {
>+        "action": {
>+            "type": "block"
>+        },
>+        "trigger": {
>+            "url-filter": ".*square"
>+        }
>+    }
>+]
>diff --git a/LayoutTests/http/tests/contentextensions/resources/subresource-redirect.php b/LayoutTests/http/tests/contentextensions/resources/subresource-redirect.php
>index 9b4035f654030489f8f6f9828894cfa7aab9e6ca..f0e9a2b093ec647914a52c85144a8aa9702c246c 100644
>--- a/LayoutTests/http/tests/contentextensions/resources/subresource-redirect.php
>+++ b/LayoutTests/http/tests/contentextensions/resources/subresource-redirect.php
>@@ -1,4 +1,4 @@
> <?php
>-  header('Location: http://127.0.0.1:8000/resources/square128.png');
>+  header('Location: http://localhost:8000/resources/square128.png');
>   header('HTTP/1.0 302 Found');
> ?>

Actions: View | Formatted Diff | Diff

Attachments on bug 186550: 342490 | 342495 | 342501 | 342503 | 342528