Unique origins should not be Potentially Trustworthy
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Mar 2020 13:15:52 +0000 (13:15 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 16 Mar 2020 13:15:52 +0000 (13:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209049

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

* web-platform-tests/service-workers/service-worker/interfaces-window.https-expected.txt:

Source/WebCore:

Unique origins should not be considered trustworthy as per https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy.

Test: http/tests/security/iframe-unique-origin.https.html

* dom/Document.cpp:
(WebCore::Document::isSecureContext const):
Removed check for top level origins as we make all unique origins not trusted.
* page/SecurityOrigin.cpp:

Source/WebKit:

* WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
(WebKit::WebFrameLoaderClient::hasNavigatedAwayFromAppBoundDomain):
We should only check this for the main frame since this is tied to the page.

Tools:

* TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp:
(TestWebKitAPI::TEST_F):

LayoutTests:

* editing/async-clipboard/resources/sanitize-when-reading-markup-iframe.html: Added.
* editing/async-clipboard/sanitize-when-reading-markup.html:
Updating test to use HTTP instead of data URL for iFrame since clipboard is SecureContext.
* http/tests/security/iframe-unique-origin.https-expected.txt: Added.
* http/tests/security/iframe-unique-origin.https.html: Added.

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

14 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/async-clipboard/resources/sanitize-when-reading-markup-iframe.html [new file with mode: 0644]
LayoutTests/editing/async-clipboard/sanitize-when-reading-markup.html
LayoutTests/http/tests/security/iframe-unique-origin.https-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/iframe-unique-origin.https.html [new file with mode: 0644]
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/interfaces-window.https-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/SecurityOrigin.cpp
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp

index 1b326e1..afa4e71 100644 (file)
@@ -1,3 +1,16 @@
+2020-03-16  youenn fablet  <youenn@apple.com>
+
+        Unique origins should not be Potentially Trustworthy
+        https://bugs.webkit.org/show_bug.cgi?id=209049
+
+        Reviewed by Darin Adler.
+
+        * editing/async-clipboard/resources/sanitize-when-reading-markup-iframe.html: Added.
+        * editing/async-clipboard/sanitize-when-reading-markup.html:
+        Updating test to use HTTP instead of data URL for iFrame since clipboard is SecureContext.
+        * http/tests/security/iframe-unique-origin.https-expected.txt: Added.
+        * http/tests/security/iframe-unique-origin.https.html: Added.
+
 2020-03-16  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [Cairo][SVG] marker-mid isn't shown on a joint of rectilinearly connected line-to path segments
diff --git a/LayoutTests/editing/async-clipboard/resources/sanitize-when-reading-markup-iframe.html b/LayoutTests/editing/async-clipboard/resources/sanitize-when-reading-markup-iframe.html
new file mode 100644 (file)
index 0000000..5c98da6
--- /dev/null
@@ -0,0 +1,12 @@
+<button id='copy' style='font-size: 40px; text-align: center;'>Click to copy</button>
+<script>
+    const markup1 = `<script>console.log('This script tag should be sanitized out.')</${'script'}><p onclick='javascript:void()'>Hello world 1</p>`;
+    const markup2 = `<p style='display: none;'>You should not see this text.</p><span>Hello world 2</span>`;
+    copy.addEventListener('click', async () => {
+        await navigator.clipboard.write([
+            new ClipboardItem({ 'text/html' : markup1 }),
+            new ClipboardItem({ 'text/html' : markup2 })
+        ]);
+        parent.postMessage('finished-copying', '*');
+    });
+</script>
index 8b01831..2a01927 100644 (file)
         addEventListener("load", runTest);
     </script>
     <body>
-        <iframe src="data:text/html,
-            <button id='copy' style='font-size: 40px; text-align: center;'>Click to copy</button>
-            <script>
-                const markup1 = `<script>console.log('This script tag should be sanitized out.')</${'script'}><p onclick='javascript:void()'>Hello world 1</p>`;
-                const markup2 = `<p style='display: none;'>You should not see this text.</p><span>Hello world 2</span>`;
-                copy.addEventListener('click', async () => {
-                    await navigator.clipboard.write([
-                        new ClipboardItem({ 'text/html' : markup1 }),
-                        new ClipboardItem({ 'text/html' : markup2 })
-                    ]);
-                    parent.postMessage('finished-copying', '*');
-                });
-            </script>"></iframe>
+        <iframe src="resources/sanitize-when-reading-markup-iframe.html"></iframe>
         <button>Click to paste</button>
         <p id="description"></p>
         <p id="console"></p>
diff --git a/LayoutTests/http/tests/security/iframe-unique-origin.https-expected.txt b/LayoutTests/http/tests/security/iframe-unique-origin.https-expected.txt
new file mode 100644 (file)
index 0000000..7ef22e9
--- /dev/null
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/http/tests/security/iframe-unique-origin.https.html b/LayoutTests/http/tests/security/iframe-unique-origin.https.html
new file mode 100644 (file)
index 0000000..a933532
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    window.onmessage = (event) => {
+        document.body.innerHTML = event.data === false ? "PASS" : "FAIL";
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }
+
+    window.onload = () => {
+        var url = 'data:text/html,<scri' + 'pt>parent.postMessage(window.isSecureContext, "*");</scri' + 'pt>';
+        document.body.innerHTML = "<iframe src='" + url + "'></iframe>";
+    };
+</script>
index 68c9320..0abb8fd 100644 (file)
@@ -1,5 +1,14 @@
 2020-03-16  youenn fablet  <youenn@apple.com>
 
+        Unique origins should not be Potentially Trustworthy
+        https://bugs.webkit.org/show_bug.cgi?id=209049
+
+        Reviewed by Darin Adler.
+
+        * web-platform-tests/service-workers/service-worker/interfaces-window.https-expected.txt:
+
+2020-03-16  youenn fablet  <youenn@apple.com>
+
         Remove the use of empty WebRTC sources for receiver tracks
         https://bugs.webkit.org/show_bug.cgi?id=209061
 
index fb040c8..6c6aa46 100644 (file)
@@ -1,7 +1,7 @@
 
 
 PASS test setup (worker registration) 
-FAIL navigator.serviceWorker is not available in a data: iframe assert_false: navigator.serviceWorker should not be defined in iframe expected false got true
+PASS navigator.serviceWorker is not available in a data: iframe 
 PASS ServiceWorker includes AbstractWorker: member names are unique 
 PASS WorkerGlobalScope includes WindowOrWorkerGlobalScope: member names are unique 
 PASS ServiceWorker interface: existence and properties of interface object 
index 9c41a20..0d12cf4 100644 (file)
@@ -1,5 +1,21 @@
 2020-03-16  youenn fablet  <youenn@apple.com>
 
+        Unique origins should not be Potentially Trustworthy
+        https://bugs.webkit.org/show_bug.cgi?id=209049
+
+        Reviewed by Darin Adler.
+
+        Unique origins should not be considered trustworthy as per https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy.
+
+        Test: http/tests/security/iframe-unique-origin.https.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::isSecureContext const):
+        Removed check for top level origins as we make all unique origins not trusted.
+        * page/SecurityOrigin.cpp:
+
+2020-03-16  youenn fablet  <youenn@apple.com>
+
         Remove the use of empty WebRTC sources for receiver tracks
         https://bugs.webkit.org/show_bug.cgi?id=209061
 
index 6879fd1..f65673a 100644 (file)
 #include "HTMLDocument.h"
 #include "HTMLElementFactory.h"
 #include "HTMLFormControlElement.h"
+#include "HTMLFrameElement.h"
 #include "HTMLFrameOwnerElement.h"
 #include "HTMLFrameSetElement.h"
 #include "HTMLHeadElement.h"
 #include "HTMLHtmlElement.h"
+#include "HTMLIFrameElement.h"
 #include "HTMLImageElement.h"
 #include "HTMLInputElement.h"
 #include "HTMLLinkElement.h"
@@ -6035,21 +6037,38 @@ bool Document::isContextThread() const
     return isMainThread();
 }
 
+// https://w3c.github.io/webappsec-secure-contexts/#is-url-trustworthy
+static bool isURLPotentiallyTrustworthy(const URL& url)
+{
+    if (url.protocolIsAbout())
+        return equalIgnoringASCIICase(url.string(), WTF::blankURL()) || equalLettersIgnoringASCIICase(url.string(), "about:srcdoc");
+    if (url.protocolIsData())
+        return true;
+    return SecurityOrigin::create(url)->isPotentiallyTrustworthy();
+}
+
+// https://w3c.github.io/webappsec-secure-contexts/#is-settings-object-contextually-secure step 5.3 and 5.4
+static inline bool isDocumentSecure(const Document& document)
+{
+    if (document.isSandboxed(SandboxOrigin))
+        return isURLPotentiallyTrustworthy(document.url());
+    return document.securityOrigin().isPotentiallyTrustworthy();
+}
+
+// https://w3c.github.io/webappsec-secure-contexts/#is-settings-object-contextually-secure
 bool Document::isSecureContext() const
 {
     if (!m_frame)
         return true;
     if (!RuntimeEnabledFeatures::sharedFeatures().secureContextChecksEnabled())
         return true;
-    if (!securityOrigin().isPotentiallyTrustworthy())
-        return false;
+
     for (auto* frame = m_frame->tree().parent(); frame; frame = frame->tree().parent()) {
-        if (!frame->document()->securityOrigin().isPotentiallyTrustworthy())
+        if (!isDocumentSecure(*frame->document()))
             return false;
     }
-    if (topOrigin().isUnique())
-        return false;
-    return true;
+
+    return isDocumentSecure(*this);
 }
 
 void Document::updateURLForPushOrReplaceState(const URL& url)
index 6d847eb..0c33f26 100644 (file)
@@ -174,7 +174,7 @@ SecurityOrigin::SecurityOrigin()
     : m_data { emptyString(), emptyString(), WTF::nullopt }
     , m_domain { emptyString() }
     , m_isUnique { true }
-    , m_isPotentiallyTrustworthy { true }
+    , m_isPotentiallyTrustworthy { false }
 {
 }
 
index 70ff6e0..68f7024 100644 (file)
@@ -1,3 +1,14 @@
+2020-03-16  youenn fablet  <youenn@apple.com>
+
+        Unique origins should not be Potentially Trustworthy
+        https://bugs.webkit.org/show_bug.cgi?id=209049
+
+        Reviewed by Darin Adler.
+
+        * WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:
+        (WebKit::WebFrameLoaderClient::hasNavigatedAwayFromAppBoundDomain):
+        We should only check this for the main frame since this is tied to the page.
+
 2020-03-16  Rob Buis  <rbuis@igalia.com>
 
         Simplify ChromeClient.createWindow
index 3ad00d9..0e87561 100644 (file)
@@ -1925,6 +1925,9 @@ void WebFrameLoaderClient::finishedLoadingApplicationManifest(uint64_t callbackI
 
 bool WebFrameLoaderClient::hasNavigatedAwayFromAppBoundDomain()
 {
+    if (!m_frame->isMainFrame())
+        return false;
+
     auto* webPage = m_frame->page();
     if (!webPage)
         return false;
index 9239252..60f5a45 100644 (file)
@@ -1,3 +1,13 @@
+2020-03-16  youenn fablet  <youenn@apple.com>
+
+        Unique origins should not be Potentially Trustworthy
+        https://bugs.webkit.org/show_bug.cgi?id=209049
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WebCore/SecurityOrigin.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2020-03-15  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         KeyedDecoderGeneric fails to allocate Vector while decoding broken data
index bf88878..ae03ba9 100644 (file)
@@ -152,10 +152,6 @@ TEST_F(SecurityOriginTest, IsPotentiallyTrustworthy)
     EXPECT_TRUE(SecurityOrigin::createFromString("blob:http://localhost/3D45F36F-C126-493A-A8AA-457FA495247B")->isPotentiallyTrustworthy());
     EXPECT_TRUE(SecurityOrigin::createFromString("blob:http://[::1]/3D45F36F-C126-493A-A8AA-457FA495247B")->isPotentiallyTrustworthy());
     EXPECT_TRUE(SecurityOrigin::createFromString("blob:https://example.com/3D45F36F-C126-493A-A8AA-457FA495247B")->isPotentiallyTrustworthy());
-    EXPECT_TRUE(SecurityOrigin::createFromString("data:,a")->isPotentiallyTrustworthy());
-    EXPECT_TRUE(SecurityOrigin::createFromString("about:")->isPotentiallyTrustworthy());
-    EXPECT_TRUE(SecurityOrigin::createFromString("about:blank")->isPotentiallyTrustworthy());
-    EXPECT_TRUE(SecurityOrigin::createFromString("about:srcdoc")->isPotentiallyTrustworthy());
     EXPECT_TRUE(SecurityOrigin::createFromString("wss://example.com")->isPotentiallyTrustworthy());
     EXPECT_TRUE(SecurityOrigin::createFromString("https://example.com")->isPotentiallyTrustworthy());
     EXPECT_TRUE(SecurityOrigin::createFromString("http://127.0.0.0")->isPotentiallyTrustworthy());
@@ -181,6 +177,12 @@ TEST_F(SecurityOriginTest, IsPotentiallyTrustworthy)
     EXPECT_FALSE(SecurityOrigin::createFromString("ws://example.com")->isPotentiallyTrustworthy());
     EXPECT_FALSE(SecurityOrigin::createFromString("blob:http://example.com/3D45F36F-C126-493A-A8AA-457FA495247B")->isPotentiallyTrustworthy());
     EXPECT_FALSE(SecurityOrigin::createFromString("dummy:a")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("blob:null/3D45F36F-C126-493A-A8AA-457FA495247B")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("data:,a")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("about:")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("about:blank")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("about:srcdoc")->isPotentiallyTrustworthy());
+    EXPECT_FALSE(SecurityOrigin::createFromString("javascript:srcdoc")->isPotentiallyTrustworthy());
 }
 
 TEST_F(SecurityOriginTest, IsRegistrableDomainSuffix)