Attachment 348015 Details for Bug 188852 – Patch

[patch] Patch

bug-188852-20180824085422.patch (text/plain), 19.84 KB, created by Brent Fulgham on 2018-08-24 08:54:23 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Brent Fulgham

Created: 2018-08-24 08:54:23 PDT

Size: 19.84 KB

patch

obsolete

>Subversion Revision: 235132
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 57b3451b1bc4b1035edcd9a6fa043238ac6e5a6d..ab93032d8f8709d745ef929c81142bc0a99b5144 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-08-22  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Improve compatibility with hyperlink auditing spec
>+        https://bugs.webkit.org/show_bug.cgi?id=188852
>+        <rdar://problem/42572559>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Implement the optional behavior to ignore hyperlink auditing directives. If the resource being
>+        pinged has been previously flagged as prevalent, ignore the ping request. If the target URL
>+        has been granted Storage Access API permissions, allow the ping.
>+
>+        Provide these same protections to Beacon calls, and block synchronous XHR requests to prevalent
>+        domains to discourage abuse of that protocol.
>+
>+        Test: http/tests/resourceLoadStatistics/beacon-to-prevalent-domain.html
>+              http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html
>+              http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain.html
>+
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        (WebKit::NetworkResourceLoader::loadIsForPrevalentDomain const):
>+        (WebKit::NetworkResourceLoader::start):
>+        * NetworkProcess/NetworkResourceLoader.h:
>+        * NetworkProcess/PingLoad.cpp:
>+        (WebKit::PingLoad::PingLoad):
>+        (WebKit::PingLoad::loadIsForPrevalentDomain const):
>+        (WebKit::PingLoad::willPerformHTTPRedirection):
>+        * NetworkProcess/PingLoad.h:
>+
> 2018-08-21  Brent Fulgham  <bfulgham@apple.com>
> 
>         Remove experimental affiliated domain code now that StorageAccess API is available
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>index 909acc43892a199da089d2837be25f3c969ac6d0..7e56ad44f0756770db6cb059b159ffe34275302f 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>@@ -51,6 +51,7 @@
> #include <WebCore/HTTPParsers.h>
> #include <WebCore/NetworkLoadMetrics.h>
> #include <WebCore/ProtectionSpace.h>
>+#include <WebCore/PublicSuffix.h>
> #include <WebCore/SameSiteInfo.h>
> #include <WebCore/SecurityOrigin.h>
> #include <WebCore/SharedBuffer.h>
>@@ -162,6 +163,23 @@ bool NetworkResourceLoader::canUseCachedRedirect(const ResourceRequest& request)
>     return true;
> }
> 
>+bool NetworkResourceLoader::loadIsForPrevalentDomain(const URL& url) const
>+{
>+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
>+    if (auto* networkSession = SessionTracker::networkSession(m_parameters.sessionID)) {
>+        ASSERT(m_parameters.sourceOrigin);
>+        URL sourceOrigin(URL(), m_parameters.sourceOrigin->host());
>+        sourceOrigin.setProtocol(m_parameters.sourceOrigin->protocol());
>+        
>+        return networkSession->networkStorageSession().shouldBlockCookies(sourceOrigin, url, m_parameters.webFrameID, m_parameters.webPageID);
>+    }
>+    
>+    ASSERT_NOT_REACHED();
>+#endif
>+    
>+    return false;
>+}
>+
> bool NetworkResourceLoader::isSynchronous() const
> {
>     return !!m_synchronousLoadData;
>@@ -181,6 +199,11 @@ void NetworkResourceLoader::start()
>     ASSERT(!m_wasStarted);
>     m_wasStarted = true;
> 
>+    if (isSynchronous() && loadIsForPrevalentDomain(this->m_parameters.request.url())) {
>+        didFailLoading(ResourceError { String { }, 0, this->m_parameters.request.url(), "Synchronous XHR request to prevalent domain"_s, ResourceError::Type::AccessControl });
>+        return;
>+    }
>+    
>     if (m_networkLoadChecker) {
>         m_networkLoadChecker->check(ResourceRequest { originalRequest() }, this, [this] (auto&& result) {
>             if (!result.has_value()) {
>@@ -578,6 +601,11 @@ void NetworkResourceLoader::didBlockAuthenticationChallenge()
> 
> void NetworkResourceLoader::willSendRedirectedRequest(ResourceRequest&& request, ResourceRequest&& redirectRequest, ResourceResponse&& redirectResponse)
> {
>+    if (isSynchronous() && loadIsForPrevalentDomain(request.url())) {
>+        didFailLoading(ResourceError { String { }, 0, request.url(), "Redirection to prevalent domain"_s, ResourceError::Type::AccessControl });
>+        return;
>+    }
>+
>     ++m_redirectCount;
> 
>     if (redirectResponse.source() == ResourceResponse::Source::Network && canUseCachedRedirect(request))
>diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>index d8977482fcf22ff08fc3dfc5db372af9f2320cba..ec14dcefda72abb4dcf2cc6c01dd7e71b4bf893c 100644
>--- a/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>+++ b/Source/WebKit/NetworkProcess/NetworkResourceLoader.h
>@@ -125,6 +125,7 @@ private:
> 
>     bool canUseCache(const WebCore::ResourceRequest&) const;
>     bool canUseCachedRedirect(const WebCore::ResourceRequest&) const;
>+    bool loadIsForPrevalentDomain(const WebCore::URL&) const;
> 
>     void tryStoreAsCacheEntry();
>     void retrieveCacheEntry(const WebCore::ResourceRequest&);
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.cpp b/Source/WebKit/NetworkProcess/PingLoad.cpp
>index bdb4123c17527b676a692f2d9ccafd137878ae4f..e51f7b232abee446dc4635e3a01d0b17f2962d93 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.cpp
>+++ b/Source/WebKit/NetworkProcess/PingLoad.cpp
>@@ -31,6 +31,7 @@
> #include "NetworkLoadChecker.h"
> #include "SessionTracker.h"
> #include "WebErrors.h"
>+#include <WebCore/PublicSuffix.h>
> 
> #define RELEASE_LOG_IF_ALLOWED(fmt, ...) RELEASE_LOG_IF(m_parameters.sessionID.isAlwaysOnLoggingAllowed(), Network, "%p - PingLoad::" fmt, this, ##__VA_ARGS__)
> 
>@@ -55,6 +56,11 @@ PingLoad::PingLoad(NetworkResourceLoadParameters&& parameters, WTF::CompletionHa
>     // Set a very generous timeout, just in case.
>     m_timeoutTimer.startOneShot(60000_s);
> 
>+    if (loadIsForPrevalentDomain(m_parameters.request.url())) {
>+        didFinish(ResourceError { String { }, 0, m_parameters.request.url(), "Ping request to prevalent domain"_s, ResourceError::Type::AccessControl });
>+        return;
>+    }
>+
>     m_networkLoadChecker->check(ResourceRequest { m_parameters.request }, nullptr, [this] (auto&& result) {
>         if (!result.has_value()) {
>             this->didFinish(result.error());
>@@ -73,6 +79,23 @@ PingLoad::~PingLoad()
>     }
> }
> 
>+bool PingLoad::loadIsForPrevalentDomain(const URL& url) const
>+{
>+#if HAVE(CFNETWORK_STORAGE_PARTITIONING)
>+    if (auto* networkSession = SessionTracker::networkSession(m_parameters.sessionID)) {
>+        ASSERT(m_parameters.sourceOrigin);
>+        URL sourceOrigin(URL(), m_parameters.sourceOrigin->host());
>+        sourceOrigin.setProtocol(m_parameters.sourceOrigin->protocol());
>+
>+        return networkSession->networkStorageSession().shouldBlockCookies(sourceOrigin, url, m_parameters.webFrameID, m_parameters.webPageID);
>+    }
>+
>+    ASSERT_NOT_REACHED();
>+#endif
>+
>+    return false;
>+}
>+    
> void PingLoad::didFinish(const ResourceError& error, const ResourceResponse& response)
> {
>     m_completionHandler(error, response);
>@@ -93,6 +116,11 @@ void PingLoad::loadRequest(ResourceRequest&& request)
> 
> void PingLoad::willPerformHTTPRedirection(ResourceResponse&& redirectResponse, ResourceRequest&& request, RedirectCompletionHandler&& completionHandler)
> {
>+    if (loadIsForPrevalentDomain(request.url())) {
>+        didFinish(ResourceError { String { }, 0, request.url(), "Redirection to prevalent domain"_s, ResourceError::Type::AccessControl });
>+        return;
>+    }
>+
>     m_networkLoadChecker->checkRedirection(ResourceRequest { }, WTFMove(request), WTFMove(redirectResponse), nullptr, [this, completionHandler = WTFMove(completionHandler)] (auto&& result) mutable {
>         if (!result.has_value()) {
>             completionHandler({ });
>diff --git a/Source/WebKit/NetworkProcess/PingLoad.h b/Source/WebKit/NetworkProcess/PingLoad.h
>index e47bf1733121b710beb5e5b3ff1ae61d8c4ebffe..98d25065a995387af6289a3bd69308e759fe1239 100644
>--- a/Source/WebKit/NetworkProcess/PingLoad.h
>+++ b/Source/WebKit/NetworkProcess/PingLoad.h
>@@ -67,7 +67,9 @@ private:
>     void loadRequest(WebCore::ResourceRequest&&);
> 
>     void didFinish(const WebCore::ResourceError& = { }, const WebCore::ResourceResponse& response = { });
>-    
>+
>+    bool loadIsForPrevalentDomain(const URL&) const;
>+
>     NetworkResourceLoadParameters m_parameters;
>     WTF::CompletionHandler<void(const WebCore::ResourceError&, const WebCore::ResourceResponse&)> m_completionHandler;
>     RefPtr<NetworkDataTask> m_task;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b54f89ec9ec11605cccaaa5fe035bd400cba76f7..8e526232b6d2695c9e91353f9771e84e20d26325 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-08-23  Brent Fulgham  <bfulgham@apple.com>
>+
>+        Improve compatibility with hyperlink auditing spec
>+        https://bugs.webkit.org/show_bug.cgi?id=188852
>+        <rdar://problem/42572559>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/resourceLoadStatistics/beacon-to-prevalent-domain-expected.txt: Added.
>+        * http/tests/resourceLoadStatistics/beacon-to-prevalent-domain.html: Added.
>+        * http/tests/resourceLoadStatistics/ping-to-prevalent-domain-expected.txt: Added.
>+        * http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html: Added.
>+        * platform/ios/TestExpectations: Skip tests on iOS since it requires a working event sender.
>+
> 2018-08-21  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
> 
>         Support "name" option for dedicated workers
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain-expected.txt b/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3f6c643ded9bd741fbeee2de61a5d1a7893d0cf5
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain-expected.txt
>@@ -0,0 +1,14 @@
>+CONSOLE MESSAGE: Beacon API cannot load http://localhost:8000/blink/sendbeacon/resources/save-beacon.php?name=prevalent-domain. Ping request to prevalent domain
>+Tests that navigator.sendBeacon() operations are not performed for prevalent domains.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS Localhost was marked as a prevalent domain.
>+PASS navigator.sendBeacon("http://localhost:8000/blink/sendbeacon/resources/save-beacon.php?name=prevalent-domain", "CrossOrigin"); is true
>+PASS Beacon not sent
>+PASS 
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain.html b/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..aa33c255b46d62f5aded81dbed836253807b3000
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/beacon-to-prevalent-domain.html
>@@ -0,0 +1,44 @@
>+<!DOCTYPE HTML>
>+<html>
>+<head>
>+<script src="/js-test-resources/js-test.js"></script>
>+<script>
>+description("Tests that navigator.sendBeacon() operations are not performed for prevalent domains.");
>+
>+window.jsTestIsAsync = true;
>+
>+function test() {
>+    if (window.testRunner) {
>+        testRunner.dumpAsText();
>+        testRunner.waitUntilDone();
>+    }
>+
>+    testRunner.setStatisticsPrevalentResource("http://localhost", true, function() {
>+        if (!testRunner.isStatisticsPrevalentResource("http://localhost"))
>+            testFailed("Host did not get set as prevalent resource.");
>+
>+        testPassed("Localhost was marked as a prevalent domain.");
>+        
>+        testRunner.statisticsUpdateCookieBlocking(function() {
>+            shouldBeTrue('navigator.sendBeacon("http://localhost:8000/blink/sendbeacon/resources/save-beacon.php?name=prevalent-domain", "CrossOrigin");');
>+            var xhr = new XMLHttpRequest();
>+            xhr.open("GET", "../blink/sendbeacon/resources/check-beacon.php?name=prevalent-domain");
>+            xhr.onload = function () {
>+                var lines = xhr.responseText.split("\n");
>+                for (var i in lines)
>+                    testPassed(lines[i]);
>+                finishJSTest();
>+            };
>+            xhr.onerror = function () {
>+                testFailed("Unable to fetch beacon status");
>+                finishJSTest();
>+            };
>+            xhr.send();
>+        });
>+    });
>+}
>+</script>
>+</head>
>+<body onload="test();">
>+</body>
>+</html>
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain-expected.txt b/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..b4b214b37aae341625c554b9c6c88595f32d0346
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain-expected.txt
>@@ -0,0 +1,20 @@
>+Tests that ping operations are not performed for prevalent domains.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+PASS Localhost was marked as a prevalent domain.
>+This test follows a link with a ping attribute where the ping URL is a prevalent domain.  
>+
>+--------
>+Frame: 'link_frame'
>+--------
>+Link with ping was clicked.
>+
>+--------
>+Frame: 'result_frame'
>+--------
>+Ping not received - timed out.
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html b/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..88b33e714fb0da3ffffcf4f904dbb654b03a6a61
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html
>@@ -0,0 +1,69 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="/js-test-resources/js-test.js"></script>
>+<script>
>+description("Tests that ping operations are not performed for prevalent domains.");
>+
>+if (window.testRunner && window.internals) {
>+    testRunner.dumpAsText();
>+    testRunner.dumpChildFramesAsText();
>+    internals.settings.setHyperlinkAuditingEnabled(true);
>+    testRunner.waitUntilDone();
>+}
>+
>+function loadLinkWithPing() {
>+    testRunner.setStatisticsPrevalentResource("http://localhost", true, function() {
>+        if (!testRunner.isStatisticsPrevalentResource("http://localhost"))
>+            testFailed("Host did not get set as prevalent resource.");
>+
>+        testPassed("Localhost was marked as a prevalent domain.");
>+
>+        testRunner.statisticsUpdateCookieBlocking(function() {
>+            var iframe = document.getElementById("link_frame");
>+            var iframeDoc = iframe.contentDocument || iframe.contentWindow.document;
>+            iframeDoc.write('' +
>+                '<img src="../contentextensions/resources/delete-ping.php?test=ping-to-prevalent-resource" ' + 
>+                    'onerror="parent.clickOnLinkWithPing();">' +
>+                '<a id="a" ' +
>+                    'href="../contentextensions/resources/check-ping.html" ' + // check-ping.html calls showPingResult()
>+                    'ping="http://localhost:8000/contentextensions/resources/save-ping.php?test=ping-to-prevalent-resource"> ' +
>+                    'Link with ping' +
>+                '</a>'
>+            );
>+        });
>+    });
>+}
>+
>+function clickOnLinkWithPing() {
>+    var iframe = document.getElementById("link_frame");
>+    var iframeDoc = iframe.contentDocument;
>+    if (window.eventSender) {
>+        var a = iframeDoc.getElementById("a");
>+        var x = iframe.offsetLeft + a.offsetLeft + 2;
>+        var y = iframe.offsetTop + a.offsetTop + 2;
>+        eventSender.mouseMoveTo(x, y);
>+        eventSender.mouseDown();
>+        eventSender.mouseUp();
>+    }
>+}
>+
>+function showPingResult() {
>+    var iframe = document.getElementById("result_frame");
>+    iframe.onload = function() {
>+        if (window.testRunner) { testRunner.notifyDone(); }
>+    }
>+    iframe.src = "../contentextensions/resources/get-ping-data.php?test=ping-to-prevalent-resource&timeout_ms=1000";
>+    // Why timeout_ms=1000:
>+    // To pass the test, the ping shouldn't arrive, so we need to
>+    // timeout at some point. We don't have to wait too long because
>+    // the console message can tell us whether the ping was blocked.
>+}
>+</script>
>+</head>
>+<body onload="loadLinkWithPing();">
>+This test follows a link with a ping attribute where the ping URL is a prevalent domain.
>+<iframe id="link_frame" name="link_frame"></iframe>
>+<iframe id="result_frame" name="result_frame"></iframe>
>+</body>
>+</html>
>\ No newline at end of file
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain-expected.txt b/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..1c3410dff79376ae9982ebf1588631b1d3a5b3b0
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain-expected.txt
>@@ -0,0 +1,13 @@
>+CONSOLE MESSAGE: Synchronous XHR request to prevalent domain
>+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi due to access control checks.
>+Tests that synchronous XHR operations are not performed for prevalent domains.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+PASS Localhost was marked as a prevalent domain.
>+PASS Synchronous error: NetworkError:  A network error occurred.
>+
>diff --git a/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain.html b/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..fb66b14f09c89436ffbe3851bbd90618b1be2b1e
>--- /dev/null
>+++ b/LayoutTests/http/tests/resourceLoadStatistics/sync-xhr-to-prevalent-domain.html
>@@ -0,0 +1,36 @@
>+<!DOCTYPE html>
>+<html>
>+<body>
>+<script src="/js-test-resources/js-test.js"></script>
>+<script>
>+description("Tests that synchronous XHR operations are not performed for prevalent domains.");
>+
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    testRunner.waitUntilDone();
>+}
>+
>+onload = function() {
>+    testRunner.setStatisticsPrevalentResource("http://localhost", true, function() {
>+        if (!testRunner.isStatisticsPrevalentResource("http://localhost"))
>+            testFailed("Host did not get set as prevalent resource.");
>+
>+
>+        testPassed("Localhost was marked as a prevalent domain.");
>+
>+        testRunner.statisticsUpdateCookieBlocking(function() {
>+            var xhr = new XMLHttpRequest();
>+            xhr.open("GET", "http://localhost:8000/xmlhttprequest/resources/access-control-basic-allow-star.cgi", false);
>+            try {
>+                xhr.send();
>+            } catch (error) {
>+                testPassed("Synchronous error: " + error);
>+            }
>+            if (window.testRunner)
>+                testRunner.notifyDone();
>+        });
>+    });
>+};
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index 45f51f894c1a31bc5a8f7b15e358a3ca71b4f03a..3b617777934c97d9b7120b978e7c8a1976d040ff 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -965,6 +965,7 @@ http/tests/navigation/keyboard-events-during-provisional-navigation.html [ Skip
> http/tests/navigation/keyboard-events-during-provisional-subframe-navigation.html [ Skip ]
> http/tests/resourceLoadStatistics/prevalent-resource-handled-keydown.html [ Skip ]
> http/tests/resourceLoadStatistics/prevalent-resource-unhandled-keydown.html [ Skip ]
>+http/tests/resourceLoadStatistics/ping-to-prevalent-domain.html [ Skip ]
> media/audio-playback-volume-changes-with-restrictions-and-user-gestures.html [ Skip ]
> media/audio-playback-volume-changes-with-restrictions.html [ Skip ]
> media/picture-in-picture-interruption.html [ Skip ]

Actions: View | Formatted Diff | Diff

Attachments on bug 188852: 347840 | 347842 | 347851 | 347866 | 347929 | 347945 | 347949 | 347954 | 347975 | 347977 | 348015 | 388786