Title: [281935] trunk
Revision
281935
Author
cdu...@apple.com
Date
2021-09-02 10:25:34 -0700 (Thu, 02 Sep 2021)

Log Message

[COOP] Cross origin isolation doesn't happen when going from an HTTP URL to a HTTPS one with COOP+COEP
https://bugs.webkit.org/show_bug.cgi?id=229745
<rdar://problem/82630927>

Reviewed by Alex Christensen.

Source/WebCore:

When calling obtainCrossOriginOpenerPolicy() / obtainCrossOriginEmbedderPolicy() to get the COOP / COEP
policy from a resource response, we should check if the response's URL is potentially trustworty to
determine if COOP/COEP apply, instead of relying on the current context's isSecureContext().

When navigating from a HTTP URL to a HTTPS one with COOP+COEP, we obviously want cross-origin isolation
to happen, even if the source context is not a secure context.

Test: http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html

* loader/CrossOriginEmbedderPolicy.cpp:
(WebCore::obtainCrossOriginEmbedderPolicy):
* loader/CrossOriginEmbedderPolicy.h:
* loader/CrossOriginOpenerPolicy.cpp:
(WebCore::obtainCrossOriginOpenerPolicy):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::didBeginDocument):
* workers/WorkerScriptLoader.cpp:
(WebCore::WorkerScriptLoader::loadSynchronously):
(WebCore::WorkerScriptLoader::loadAsynchronously):
(WebCore::WorkerScriptLoader::didReceiveResponse):
* workers/WorkerScriptLoader.h:

Source/WebKit:

* NetworkProcess/NetworkResourceLoader.cpp:
(WebKit::NetworkResourceLoader::shouldInterruptNavigationForCrossOriginEmbedderPolicy):
(WebKit::NetworkResourceLoader::shouldInterruptWorkerLoadForCrossOriginEmbedderPolicy):
* NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp:
(WebKit::ServiceWorkerSoftUpdateLoader::processResponse):

LayoutTests:

Add layout test coverage.

* http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https-expected.txt: Added.
* http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html: Added.
* http/wpt/cross-origin-opener-policy/resources/non-secure-to-secure-context-navigation-popup.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (281934 => 281935)


--- trunk/LayoutTests/ChangeLog	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/LayoutTests/ChangeLog	2021-09-02 17:25:34 UTC (rev 281935)
@@ -1,3 +1,17 @@
+2021-09-02  Chris Dumez  <cdu...@apple.com>
+
+        [COOP] Cross origin isolation doesn't happen when going from an HTTP URL to a HTTPS one with COOP+COEP
+        https://bugs.webkit.org/show_bug.cgi?id=229745
+        <rdar://problem/82630927>
+
+        Reviewed by Alex Christensen.
+
+        Add layout test coverage.
+
+        * http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https-expected.txt: Added.
+        * http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html: Added.
+        * http/wpt/cross-origin-opener-policy/resources/non-secure-to-secure-context-navigation-popup.html: Added.
+
 2021-09-02  Eric Hutchison  <ehutchi...@apple.com>
 
         [iOS] fast/mediastream/get-display-media-capabilities.html is a flaky failure and crash.

Added: trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https-expected.txt (0 => 281935)


--- trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https-expected.txt	2021-09-02 17:25:34 UTC (rev 281935)
@@ -0,0 +1,3 @@
+
+PASS Make sure that COOP causes a browsing context group switch when navigating from a secure context to a non-secure one
+

Added: trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html (0 => 281935)


--- trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html	                        (rev 0)
+++ trunk/LayoutTests/http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html	2021-09-02 17:25:34 UTC (rev 281935)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset=utf-8>
+<script src=""
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+async_test((t) => {
+    let bc = new BroadcastChannel('non-secure-to-secure-context-navigation');
+    bc._onmessage_ = t.step_func((event) => {
+        assert_true(win.closed, "Window should be closed");
+        assert_equals(event.data, "does_not_have_opener");
+        t.done();
+    });
+
+    win = open("resources/non-secure-to-secure-context-navigation-popup.html");
+    win._onload_ = () => {
+        win._onload_ = null;
+        // Navigate to HTTPS with  COOP=same-origin.
+        win.location.href = "" + "?secure=1&pipe=header(Cross-Origin-Opener-Policy,same-origin)";
+    };
+}, "Make sure that COOP causes a browsing context group switch when navigating from a secure context to a non-secure one");
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/http/wpt/cross-origin-opener-policy/resources/non-secure-to-secure-context-navigation-popup.html (0 => 281935)


--- trunk/LayoutTests/http/wpt/cross-origin-opener-policy/resources/non-secure-to-secure-context-navigation-popup.html	                        (rev 0)
+++ trunk/LayoutTests/http/wpt/cross-origin-opener-policy/resources/non-secure-to-secure-context-navigation-popup.html	2021-09-02 17:25:34 UTC (rev 281935)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+const urlParams = new URLSearchParams(location.search);
+const isSecureContext = urlParams.get("secure") == "1";
+if (window.internals && !isSecureContext)
+    internals.markContextAsInsecure();
+
+if (isSecureContext) {
+    let bc = new BroadcastChannel('non-secure-to-secure-context-navigation');
+    bc.postMessage(window.opener ? "has_opener" : "does_not_have_opener");
+}
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (281934 => 281935)


--- trunk/Source/WebCore/ChangeLog	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/ChangeLog	2021-09-02 17:25:34 UTC (rev 281935)
@@ -1,3 +1,33 @@
+2021-09-02  Chris Dumez  <cdu...@apple.com>
+
+        [COOP] Cross origin isolation doesn't happen when going from an HTTP URL to a HTTPS one with COOP+COEP
+        https://bugs.webkit.org/show_bug.cgi?id=229745
+        <rdar://problem/82630927>
+
+        Reviewed by Alex Christensen.
+
+        When calling obtainCrossOriginOpenerPolicy() / obtainCrossOriginEmbedderPolicy() to get the COOP / COEP
+        policy from a resource response, we should check if the response's URL is potentially trustworty to
+        determine if COOP/COEP apply, instead of relying on the current context's isSecureContext().
+
+        When navigating from a HTTP URL to a HTTPS one with COOP+COEP, we obviously want cross-origin isolation
+        to happen, even if the source context is not a secure context.
+
+        Test: http/wpt/cross-origin-opener-policy/non-secure-to-secure-context-navigation.https.html
+
+        * loader/CrossOriginEmbedderPolicy.cpp:
+        (WebCore::obtainCrossOriginEmbedderPolicy):
+        * loader/CrossOriginEmbedderPolicy.h:
+        * loader/CrossOriginOpenerPolicy.cpp:
+        (WebCore::obtainCrossOriginOpenerPolicy):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::didBeginDocument):
+        * workers/WorkerScriptLoader.cpp:
+        (WebCore::WorkerScriptLoader::loadSynchronously):
+        (WebCore::WorkerScriptLoader::loadAsynchronously):
+        (WebCore::WorkerScriptLoader::didReceiveResponse):
+        * workers/WorkerScriptLoader.h:
+
 2021-09-02  Alex Christensen  <achristen...@webkit.org>
 
         Remove optimistic assertion from r281860

Modified: trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.cpp (281934 => 281935)


--- trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -35,7 +35,7 @@
 namespace WebCore {
 
 // https://html.spec.whatwg.org/multipage/origin.html#obtain-an-embedder-policy
-CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse& response, IsSecureContext isSecureContext)
+CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse& response, const ScriptExecutionContext* context)
 {
     auto parseCOEPHeader = [&response](HTTPHeaderName headerName, auto& value, auto& reportingEndpoint) {
         auto coepParsingResult = parseStructuredFieldValue(response.httpHeaderField(headerName));
@@ -46,8 +46,10 @@
     };
 
     CrossOriginEmbedderPolicy policy;
-    if (isSecureContext == IsSecureContext::No)
+    if (context && !context->settingsValues().crossOriginEmbedderPolicyEnabled)
         return policy;
+    if (!SecurityOrigin::create(response.url())->isPotentiallyTrustworthy())
+        return policy;
 
     parseCOEPHeader(HTTPHeaderName::CrossOriginEmbedderPolicy, policy.value, policy.reportingEndpoint);
     parseCOEPHeader(HTTPHeaderName::CrossOriginEmbedderPolicyReportOnly, policy.reportOnlyValue, policy.reportOnlyReportingEndpoint);
@@ -54,16 +56,6 @@
     return policy;
 }
 
-CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse& response, const ScriptExecutionContext& context)
-{
-    if (!context.settingsValues().crossOriginEmbedderPolicyEnabled)
-        return { };
-
-    // FIXME: about:blank should be marked as secure as per https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url.
-    auto isSecureContext = context.isSecureContext() || context.url() == aboutBlankURL() || context.url().isEmpty() ? IsSecureContext::Yes : IsSecureContext::No;
-    return obtainCrossOriginEmbedderPolicy(response, isSecureContext);
-}
-
 CrossOriginEmbedderPolicy CrossOriginEmbedderPolicy::isolatedCopy() const &
 {
     return {

Modified: trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.h (281934 => 281935)


--- trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.h	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/loader/CrossOriginEmbedderPolicy.h	2021-09-02 17:25:34 UTC (rev 281935)
@@ -93,10 +93,7 @@
     }};
 }
 
-CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse&, const ScriptExecutionContext&);
+WEBCORE_EXPORT CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse&, const ScriptExecutionContext*);
 WEBCORE_EXPORT void addCrossOriginEmbedderPolicyHeaders(ResourceResponse&, const CrossOriginEmbedderPolicy&);
 
-enum class IsSecureContext : bool { No, Yes };
-WEBCORE_EXPORT CrossOriginEmbedderPolicy obtainCrossOriginEmbedderPolicy(const ResourceResponse&, IsSecureContext);
-
 } // namespace WebCore

Modified: trunk/Source/WebCore/loader/CrossOriginOpenerPolicy.cpp (281934 => 281935)


--- trunk/Source/WebCore/loader/CrossOriginOpenerPolicy.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/loader/CrossOriginOpenerPolicy.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -54,7 +54,7 @@
     std::optional<CrossOriginEmbedderPolicy> coep;
     auto ensureCOEP = [&coep, &response, &context]() -> CrossOriginEmbedderPolicy& {
         if (!coep)
-            coep = obtainCrossOriginEmbedderPolicy(response, context);
+            coep = obtainCrossOriginEmbedderPolicy(response, &context);
         return *coep;
     };
     auto parseCOOP = [&response, &ensureCOEP](HTTPHeaderName headerName, auto& value, auto& reportingEndpoint) {
@@ -75,11 +75,8 @@
     };
 
     CrossOriginOpenerPolicy policy;
-    if (!context.settingsValues().crossOriginOpenerPolicyEnabled)
+    if (!context.settingsValues().crossOriginOpenerPolicyEnabled || !SecurityOrigin::create(response.url())->isPotentiallyTrustworthy())
         return policy;
-    // FIXME: about:blank should be marked as secure as per https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-url.
-    if (!context.isSecureContext() && context.url() != aboutBlankURL() && !context.url().isEmpty())
-        return policy;
 
     parseCOOP(HTTPHeaderName::CrossOriginOpenerPolicy, policy.value, policy.reportingEndpoint);
     parseCOOP(HTTPHeaderName::CrossOriginOpenerPolicyReportOnly, policy.reportOnlyValue, policy.reportOnlyReportingEndpoint);

Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (281934 => 281935)


--- trunk/Source/WebCore/loader/FrameLoader.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -752,7 +752,7 @@
         m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(ContentSecurityPolicyResponseHeaders(m_documentLoader->response()), referrer(), ContentSecurityPolicy::ReportParsingErrors::No);
 
         if (m_frame.document()->url().protocolIsInHTTPFamily() || m_frame.document()->url().protocolIsBlob())
-            m_frame.document()->setCrossOriginEmbedderPolicy(obtainCrossOriginEmbedderPolicy(m_documentLoader->response(), *m_frame.document()));
+            m_frame.document()->setCrossOriginEmbedderPolicy(obtainCrossOriginEmbedderPolicy(m_documentLoader->response(), m_frame.document()));
 
         String referrerPolicy = m_documentLoader->response().httpHeaderField(HTTPHeaderName::ReferrerPolicy);
         if (!referrerPolicy.isNull())

Modified: trunk/Source/WebCore/workers/WorkerScriptLoader.cpp (281934 => 281935)


--- trunk/Source/WebCore/workers/WorkerScriptLoader.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/workers/WorkerScriptLoader.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -58,7 +58,7 @@
 
     m_url = url;
     m_destination = FetchOptions::Destination::Script;
-    m_isSecureContext = workerGlobalScope.isSecureContext();
+    m_isCOEPEnabled = scriptExecutionContext->settingsValues().crossOriginEmbedderPolicyEnabled;
 
 #if ENABLE(SERVICE_WORKER)
     bool isServiceWorkerGlobalScope = is<ServiceWorkerGlobalScope>(workerGlobalScope);
@@ -117,7 +117,7 @@
     m_client = &client;
     m_url = scriptRequest.url();
     m_destination = fetchOptions.destination;
-    m_isSecureContext = scriptExecutionContext.isSecureContext();
+    m_isCOEPEnabled = scriptExecutionContext.settingsValues().crossOriginEmbedderPolicyEnabled;
 
     ASSERT(scriptRequest.httpMethod() == "GET");
 
@@ -193,7 +193,8 @@
     m_responseSource = response.source();
     m_isRedirected = response.isRedirected();
     m_contentSecurityPolicy = ContentSecurityPolicyResponseHeaders { response };
-    m_crossOriginEmbedderPolicy = obtainCrossOriginEmbedderPolicy(response, m_isSecureContext ? IsSecureContext::Yes : IsSecureContext::No);
+    if (m_isCOEPEnabled)
+        m_crossOriginEmbedderPolicy = obtainCrossOriginEmbedderPolicy(response, nullptr);
     m_referrerPolicy = response.httpHeaderField(HTTPHeaderName::ReferrerPolicy);
     if (m_client)
         m_client->didReceiveResponse(identifier, response);

Modified: trunk/Source/WebCore/workers/WorkerScriptLoader.h (281934 => 281935)


--- trunk/Source/WebCore/workers/WorkerScriptLoader.h	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebCore/workers/WorkerScriptLoader.h	2021-09-02 17:25:34 UTC (rev 281935)
@@ -114,7 +114,7 @@
     bool m_failed { false };
     bool m_finishing { false };
     bool m_isRedirected { false };
-    bool m_isSecureContext { false };
+    bool m_isCOEPEnabled { false };
     ResourceResponse::Source m_responseSource { ResourceResponse::Source::Unknown };
     ResourceError m_error;
 };

Modified: trunk/Source/WebKit/ChangeLog (281934 => 281935)


--- trunk/Source/WebKit/ChangeLog	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebKit/ChangeLog	2021-09-02 17:25:34 UTC (rev 281935)
@@ -1,3 +1,17 @@
+2021-09-02  Chris Dumez  <cdu...@apple.com>
+
+        [COOP] Cross origin isolation doesn't happen when going from an HTTP URL to a HTTPS one with COOP+COEP
+        https://bugs.webkit.org/show_bug.cgi?id=229745
+        <rdar://problem/82630927>
+
+        Reviewed by Alex Christensen.
+
+        * NetworkProcess/NetworkResourceLoader.cpp:
+        (WebKit::NetworkResourceLoader::shouldInterruptNavigationForCrossOriginEmbedderPolicy):
+        (WebKit::NetworkResourceLoader::shouldInterruptWorkerLoadForCrossOriginEmbedderPolicy):
+        * NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp:
+        (WebKit::ServiceWorkerSoftUpdateLoader::processResponse):
+
 2021-09-02  Tim Nguyen  <n...@apple.com>
 
         Add more inert checks for selection-related functionality

Modified: trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp (281934 => 281935)


--- trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -602,7 +602,7 @@
 
     // https://html.spec.whatwg.org/multipage/origin.html#check-a-navigation-response's-adherence-to-its-embedder-policy
     if (m_parameters.parentCrossOriginEmbedderPolicy.value != WebCore::CrossOriginEmbedderPolicyValue::UnsafeNone && m_parameters.sourceOrigin) {
-        auto responseCOEP = WebCore::obtainCrossOriginEmbedderPolicy(response, m_parameters.sourceOrigin->isPotentiallyTrustworthy() ? IsSecureContext::Yes : IsSecureContext::No);
+        auto responseCOEP = WebCore::obtainCrossOriginEmbedderPolicy(response, nullptr);
         if (responseCOEP.value != WebCore::CrossOriginEmbedderPolicyValue::RequireCORP) {
             String errorMessage = makeString("Refused to display '", response.url().stringCenterEllipsizedToLength(), "' in a frame because of Cross-Origin-Embedder-Policy.");
             send(Messages::WebPage::AddConsoleMessage { m_parameters.webFrameID,  MessageSource::Security, MessageLevel::Error, errorMessage, coreIdentifier() }, m_parameters.webPageID);
@@ -618,7 +618,7 @@
         return false;
 
     if (m_parameters.crossOriginEmbedderPolicy.value != WebCore::CrossOriginEmbedderPolicyValue::UnsafeNone && m_parameters.sourceOrigin) {
-        auto responseCOEP = WebCore::obtainCrossOriginEmbedderPolicy(response, m_parameters.sourceOrigin->isPotentiallyTrustworthy() ? IsSecureContext::Yes : IsSecureContext::No);
+        auto responseCOEP = WebCore::obtainCrossOriginEmbedderPolicy(response, nullptr);
         if (responseCOEP.value != WebCore::CrossOriginEmbedderPolicyValue::RequireCORP) {
             String errorMessage = makeString("Refused to load '", response.url().stringCenterEllipsizedToLength(), "' worker because of Cross-Origin-Embedder-Policy.");
             send(Messages::WebPage::AddConsoleMessage { m_parameters.webFrameID,  MessageSource::Security, MessageLevel::Error, errorMessage, coreIdentifier() }, m_parameters.webPageID);

Modified: trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp (281934 => 281935)


--- trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp	2021-09-02 17:23:32 UTC (rev 281934)
+++ trunk/Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerSoftUpdateLoader.cpp	2021-09-02 17:25:34 UTC (rev 281935)
@@ -170,7 +170,7 @@
 
     m_contentSecurityPolicy = ContentSecurityPolicyResponseHeaders { response };
     // Service workers are always secure contexts.
-    m_crossOriginEmbedderPolicy = obtainCrossOriginEmbedderPolicy(response, IsSecureContext::Yes);
+    m_crossOriginEmbedderPolicy = obtainCrossOriginEmbedderPolicy(response, nullptr);
     m_referrerPolicy = response.httpHeaderField(HTTPHeaderName::ReferrerPolicy);
     m_responseEncoding = response.textEncodingName();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to