- Revision
- 281941
- Author
- cdu...@apple.com
- Date
- 2021-09-02 11:37:07 -0700 (Thu, 02 Sep 2021)
Log Message
[COOP] html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox.html WPT test is failing
https://bugs.webkit.org/show_bug.cgi?id=229716
Reviewed by Darin Adler.
LayoutTests/imported/w3c:
Rebaseline WPT test that is now passing.
* web-platform-tests/html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox-expected.txt:
Source/WebCore:
The COOP algorithm is relying on the response origin for some of the checks. We were computing the
response origin via `SecurityOrigin::create(response.url())`, which worked fine in most cases.
However, the response may contain a CSP header, which could set sandbox flags. If sandbox flags
are set, the response origin should be unique, not the origin of the response URL. This patch fixes
that.
No new tests, rebaselined existing WPT test.
* loader/DocumentLoader.cpp:
(WebCore::computeResponseOriginAndCOOP):
(WebCore::DocumentLoader::doCrossOriginOpenerHandlingOfResponse):
* loader/DocumentLoader.h:
(WebCore::DocumentLoader::contentSecurityPolicy const):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::didBeginDocument):
* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::copyStateFrom):
(WebCore::ContentSecurityPolicy::didReceiveHeaders):
* page/csp/ContentSecurityPolicy.h:
(WebCore::ContentSecurityPolicy::sandboxFlags const):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (281940 => 281941)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2021-09-02 18:37:07 UTC (rev 281941)
@@ -1,3 +1,14 @@
+2021-09-02 Chris Dumez <cdu...@apple.com>
+
+ [COOP] html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox.html WPT test is failing
+ https://bugs.webkit.org/show_bug.cgi?id=229716
+
+ Reviewed by Darin Adler.
+
+ Rebaseline WPT test that is now passing.
+
+ * web-platform-tests/html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox-expected.txt:
+
2021-09-02 Tim Nguyen <n...@apple.com>
Add more inert checks for selection-related functionality
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox-expected.txt (281940 => 281941)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox-expected.txt 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox-expected.txt 2021-09-02 18:37:07 UTC (rev 281941)
@@ -1,3 +1,3 @@
-FAIL coop-navigate-same-origin-csp-sandbox assert_equals: expected "Success: no opener" but got "Error: have opener"
+PASS coop-navigate-same-origin-csp-sandbox
Modified: trunk/Source/WebCore/ChangeLog (281940 => 281941)
--- trunk/Source/WebCore/ChangeLog 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/ChangeLog 2021-09-02 18:37:07 UTC (rev 281941)
@@ -1,5 +1,33 @@
2021-09-02 Chris Dumez <cdu...@apple.com>
+ [COOP] html/cross-origin-opener-policy/coop-navigate-same-origin-csp-sandbox.html WPT test is failing
+ https://bugs.webkit.org/show_bug.cgi?id=229716
+
+ Reviewed by Darin Adler.
+
+ The COOP algorithm is relying on the response origin for some of the checks. We were computing the
+ response origin via `SecurityOrigin::create(response.url())`, which worked fine in most cases.
+ However, the response may contain a CSP header, which could set sandbox flags. If sandbox flags
+ are set, the response origin should be unique, not the origin of the response URL. This patch fixes
+ that.
+
+ No new tests, rebaselined existing WPT test.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::computeResponseOriginAndCOOP):
+ (WebCore::DocumentLoader::doCrossOriginOpenerHandlingOfResponse):
+ * loader/DocumentLoader.h:
+ (WebCore::DocumentLoader::contentSecurityPolicy const):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::didBeginDocument):
+ * page/csp/ContentSecurityPolicy.cpp:
+ (WebCore::ContentSecurityPolicy::copyStateFrom):
+ (WebCore::ContentSecurityPolicy::didReceiveHeaders):
+ * page/csp/ContentSecurityPolicy.h:
+ (WebCore::ContentSecurityPolicy::sandboxFlags const):
+
+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>
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (281940 => 281941)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2021-09-02 18:37:07 UTC (rev 281941)
@@ -758,7 +758,7 @@
return true;
}
-static std::tuple<Ref<SecurityOrigin>, CrossOriginOpenerPolicy> computeResponseOriginAndCOOP(const ResourceResponse& response, const Document& document, const std::optional<NavigationAction::Requester>& requester)
+static std::tuple<Ref<SecurityOrigin>, CrossOriginOpenerPolicy> computeResponseOriginAndCOOP(const ResourceResponse& response, const Document& document, const std::optional<NavigationAction::Requester>& requester, ContentSecurityPolicy* responseCSP)
{
// Non-initial empty documents (about:blank) should inherit their cross-origin-opener-policy from the navigation's initiator top level document,
// if the initiator and its top level document are same-origin, or default (unsafe-none) otherwise.
@@ -766,7 +766,9 @@
if (SecurityPolicy::shouldInheritSecurityOriginFromOwner(response.url()) && requester)
return std::make_tuple(makeRef(requester->securityOrigin()), requester->securityOrigin().isSameOriginAs(requester->topOrigin()) ? requester->crossOriginOpenerPolicy() : CrossOriginOpenerPolicy { });
- return std::make_tuple(SecurityOrigin::create(response.url()), obtainCrossOriginOpenerPolicy(response, document));
+ // If the HTTP response contains a CSP header, it may set sandbox flags, which would cause the origin to become unique.
+ auto responseOrigin = responseCSP && responseCSP->sandboxFlags() != SandboxNone ? SecurityOrigin::createUnique() : SecurityOrigin::create(response.url());
+ return std::make_tuple(WTFMove(responseOrigin), obtainCrossOriginOpenerPolicy(response, document));
}
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-fetch (Step 12.5.6)
@@ -779,8 +781,14 @@
if (!m_frame->document() || !m_frame->document()->settings().crossOriginOpenerPolicyEnabled())
return true;
- auto [responseOrigin, responseCOOP] = computeResponseOriginAndCOOP(response, *m_frame->document(), m_triggeringAction.requester());
+ if (!response.httpHeaderField(HTTPHeaderName::ContentSecurityPolicy).isNull()) {
+ m_contentSecurityPolicy = makeUnique<ContentSecurityPolicy>(URL { response.url() }, nullptr);
+ m_contentSecurityPolicy->didReceiveHeaders(ContentSecurityPolicyResponseHeaders { response }, m_request.httpReferrer(), ContentSecurityPolicy::ReportParsingErrors::No);
+ } else
+ m_contentSecurityPolicy = nullptr;
+ auto [responseOrigin, responseCOOP] = computeResponseOriginAndCOOP(response, *m_frame->document(), m_triggeringAction.requester(), m_contentSecurityPolicy.get());
+
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-fetch (Step 12.5.6.2)
// If sandboxFlags is not empty and responseCOOP's value is not "unsafe-none", then set response to an appropriate network error and break.
if (responseCOOP.value != CrossOriginOpenerPolicyValue::UnsafeNone && frameLoader()->effectiveSandboxFlags() != SandboxNone) {
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (281940 => 281941)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2021-09-02 18:37:07 UTC (rev 281941)
@@ -423,6 +423,7 @@
bool lastNavigationWasAppInitiated() const { return m_lastNavigationWasAppInitiated; }
void setLastNavigationWasAppInitiated(bool lastNavigationWasAppInitiated) { m_lastNavigationWasAppInitiated = lastNavigationWasAppInitiated; }
+ ContentSecurityPolicy* contentSecurityPolicy() const { return m_contentSecurityPolicy.get(); }
std::optional<CrossOriginOpenerPolicy> crossOriginOpenerPolicy() const { return m_currentCoopEnforcementResult ? std::make_optional(m_currentCoopEnforcementResult->crossOriginOpenerPolicy) : std::nullopt; }
bool isContinuingLoadAfterResponsePolicyCheck() const { return m_isContinuingLoadAfterResponsePolicyCheck; }
@@ -628,6 +629,7 @@
ShouldOpenExternalURLsPolicy m_shouldOpenExternalURLsPolicy { ShouldOpenExternalURLsPolicy::ShouldNotAllow };
std::unique_ptr<ApplicationCacheHost> m_applicationCacheHost;
+ std::unique_ptr<ContentSecurityPolicy> m_contentSecurityPolicy;
#if ENABLE(CONTENT_FILTERING)
std::unique_ptr<ContentFilter> m_contentFilter;
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (281940 => 281941)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2021-09-02 18:37:07 UTC (rev 281941)
@@ -749,7 +749,11 @@
if (!dnsPrefetchControl.isEmpty())
m_frame.document()->parseDNSPrefetchControlHeader(dnsPrefetchControl);
- m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(ContentSecurityPolicyResponseHeaders(m_documentLoader->response()), referrer(), ContentSecurityPolicy::ReportParsingErrors::No);
+ // The DocumentLoader may have already parsed the CSP header to do some checks. If so, reuse the already parsed version instead of parsing again.
+ if (auto* contentSecurityPolicy = m_documentLoader->contentSecurityPolicy())
+ m_frame.document()->contentSecurityPolicy()->didReceiveHeaders(*contentSecurityPolicy, ContentSecurityPolicy::ReportParsingErrors::No);
+ else
+ 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()));
Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp (281940 => 281941)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp 2021-09-02 18:37:07 UTC (rev 281941)
@@ -173,6 +173,17 @@
m_httpStatusCode = headers.m_httpStatusCode;
}
+void ContentSecurityPolicy::didReceiveHeaders(const ContentSecurityPolicy& other, ReportParsingErrors reportParsingErrors)
+{
+ SetForScope<bool> isReportingEnabled(m_isReportingEnabled, reportParsingErrors == ReportParsingErrors::Yes);
+ for (auto& policy : other.m_policies)
+ didReceiveHeader(policy->header(), policy->headerType(), ContentSecurityPolicy::PolicyFrom::HTTPHeader, String { });
+ m_referrer = other.m_referrer;
+ m_httpStatusCode = other.m_httpStatusCode;
+ m_upgradeInsecureRequests = other.m_upgradeInsecureRequests;
+ m_insecureNavigationRequestsToUpgrade.add(other.m_insecureNavigationRequestsToUpgrade.begin(), other.m_insecureNavigationRequestsToUpgrade.end());
+}
+
void ContentSecurityPolicy::didReceiveHeader(const String& header, ContentSecurityPolicyHeaderType type, ContentSecurityPolicy::PolicyFrom policyFrom, String&& referrer, int httpStatusCode)
{
if (m_hasAPIPolicy)
Modified: trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h (281940 => 281941)
--- trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2021-09-02 18:35:22 UTC (rev 281940)
+++ trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h 2021-09-02 18:37:07 UTC (rev 281941)
@@ -84,6 +84,7 @@
WEBCORE_EXPORT ContentSecurityPolicyResponseHeaders responseHeaders() const;
enum ReportParsingErrors { No, Yes };
WEBCORE_EXPORT void didReceiveHeaders(const ContentSecurityPolicyResponseHeaders&, String&& referrer, ReportParsingErrors = ReportParsingErrors::Yes);
+ void didReceiveHeaders(const ContentSecurityPolicy&, ReportParsingErrors = ReportParsingErrors::Yes);
void didReceiveHeader(const String&, ContentSecurityPolicyHeaderType, ContentSecurityPolicy::PolicyFrom, String&& referrer, int httpStatusCode = 0);
bool allowScriptWithNonce(const String& nonce, bool overrideContentSecurityPolicy = false) const;
@@ -177,6 +178,8 @@
void setDocumentURL(URL& documentURL) { m_documentURL = documentURL; }
+ SandboxFlags sandboxFlags() const { return m_sandboxFlags; }
+
private:
void logToConsole(const String& message, const String& contextURL = String(), const WTF::OrdinalNumber& contextLine = WTF::OrdinalNumber::beforeFirst(), const WTF::OrdinalNumber& contextColumn = WTF::OrdinalNumber::beforeFirst(), JSC::JSGlobalObject* = nullptr) const;
void applyPolicyToScriptExecutionContext();
