Timing-Allow-Origin works with 302

https://bugs.webkit.org/show_bug.cgi?id=272682 rdar://126531139 Reviewed by Alex Christensen. We move the TAO check from platform specific NetworkDataTask implementations to NetworkLoadChecker. This allows us to implement the algorithm as defined in fetch, including checking the response tainting. This aligns behavior with Chrome and Firefox. For top level navigation, we were using the source origin, but we should use the top origin for top level navigations, as top level navigations are same origin. * LayoutTests/http/wpt/resource-timing/resources/rt-utilities.sub.js: (addACAOHeader): * LayoutTests/http/wpt/resource-timing/rt-cors-2-expected.txt: Added. * LayoutTests/http/wpt/resource-timing/rt-cors-2.html: Added. * LayoutTests/http/wpt/resource-timing/rt-cors-2.js: Added. (assertAlways): (assertRedirectWithDisallowedTimingData): (assertDisallowedTimingData): (promise_test): * Source/WebKit/NetworkProcess/NetworkDataTask.h: (WebKit::NetworkDataTask::setTimingAllowFailedFlag): * Source/WebKit/NetworkProcess/NetworkLoad.cpp: (WebKit::NetworkLoad::setTimingAllowFailedFlag): * Source/WebKit/NetworkProcess/NetworkLoad.h: * Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp: (WebKit::NetworkLoadChecker::validateResponse): (WebKit::NetworkLoadChecker::checkTAO): * Source/WebKit/NetworkProcess/NetworkLoadChecker.h: (WebKit::NetworkLoadChecker::timingAllowFailedFlag const): (WebKit::NetworkLoadChecker::isSameOriginRequest const): * Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp: (WebKit::NetworkResourceLoader::didReceiveResponse): (WebKit::NetworkResourceLoader::didFinishLoading): (WebKit::NetworkResourceLoader::willSendRedirectedRequestInternal): * Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h: * Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm: (WebKit::NetworkDataTaskCocoa::setTimingAllowFailedFlag): (WebKit::NetworkDataTaskCocoa::checkTAO): Deleted. * Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm: (-[WKNetworkSessionDelegate URLSession:task:willPerformHTTPRedirection:newRequest:completionHandler:]): (-[WKNetworkSessionDelegate URLSession:dataTask:didReceiveResponse:completionHandler:]): * Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp: (WebKit::NetworkDataTaskCurl::updateNetworkLoadMetrics): (WebKit::NetworkDataTaskCurl::setTimingAllowFailedFlag): * Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h: * Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp: (WebKit::NetworkDataTaskSoup::didSendRequest): (WebKit::NetworkDataTaskSoup::setTimingAllowFailedFlag): * Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h: Canonical link: https://commits.webkit.org/278448@main
WebKit · May 7, 2024 · 6a2c5a3 · 6a2c5a3
1 parent c47f18c
commit 6a2c5a3
Show file tree

Hide file tree

Showing 17 changed files with 161 additions and 28 deletions.
diff --git a/LayoutTests/http/wpt/resource-timing/resources/rt-utilities.sub.js b/LayoutTests/http/wpt/resource-timing/resources/rt-utilities.sub.js
@@ -37,6 +37,10 @@ function crossOriginURL(key, path) {
     return `http://localhost:{{ports[http][1]}}/${path}?${key}`;
 }
 
+function simpleCrossOriginURLBase() {
+    return `http://localhost:{{ports[http][1]}}`;
+}
+
 function urlWithRedirectTo(url) {
     return `/common/redirect.py?location=${encodeURIComponent(url)}`;
 }
@@ -45,6 +49,12 @@ function addPipeHeaders(url, headers) {
     return `${url}&pipe=${headers.join("|")}`;
 }
 
+function addACAOHeader(url) {
+    return addPipeHeaders(url, [
+        `header(Access-Control-Allow-Origin,*)`,
+    ]);
+}
+
 function addTimingAllowOriginHeader(url, origin) {
     if (!origin) throw `Invalid origin: ${origin}`;
     return addPipeHeaders(url, [

diff --git a/LayoutTests/http/wpt/resource-timing/rt-cors-2-expected.txt b/LayoutTests/http/wpt/resource-timing/rt-cors-2-expected.txt
@@ -0,0 +1,5 @@
+Resource Timing: CORS requests
+
+
+PASS Cross-origin redirection with TAO to same origin loads without TAO must have filtered timing data
+
diff --git a/LayoutTests/http/wpt/resource-timing/rt-cors-2.html b/LayoutTests/http/wpt/resource-timing/rt-cors-2.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>Resource Timing - CORS requests</title>
+<link rel="help" href="https://w3c.github.io/resource-timing/#cross-origin-resources">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/rt-utilities.sub.js"></script>
+</head>
+<body>
+<h1>Resource Timing: CORS requests</h1>
+<div id="log"></div>
+<script src="rt-cors-2.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/http/wpt/resource-timing/rt-cors-2.js b/LayoutTests/http/wpt/resource-timing/rt-cors-2.js
@@ -0,0 +1,52 @@
+function assertAlways(entry) {
+    assert_equals(entry.workerStart, 0, "entry should not have a workerStart time");
+    assert_equals(entry.secureConnectionStart, 0, "entry should not have a secureConnectionStart time");
+
+    assert_not_equals(entry.startTime, 0, "entry should have a non-0 fetchStart time");
+    assert_not_equals(entry.fetchStart, 0, "entry should have a non-0 startTime time");
+    assert_not_equals(entry.responseEnd, 0, "entry should have a non-0 responseEnd time");
+
+    assert_greater_than_equal(entry.fetchStart, entry.startTime, "fetchStart after startTime");
+    assert_greater_than_equal(entry.responseEnd, entry.fetchStart, "responseEnd after fetchStart");
+}
+
+function assertRedirectWithDisallowedTimingData(entry) {
+    assertAlways(entry);
+    assert_equals(entry.redirectStart, 0, "entry should not have a redirectStart time");
+    assert_equals(entry.redirectEnd, 0, "entry should not have a redirectEnd time");
+    assert_equals(entry.startTime, entry.fetchStart, "entry startTime should have matched redirectStart but it was disallowed so it should match fetchStart");
+}
+
+function assertDisallowedTimingData(entry) {
+    // These attributes must be zero:
+    // https://w3c.github.io/resource-timing/#cross-origin-resources
+    const keys = [
+        "redirectStart",
+        "redirectEnd",
+        "domainLookupStart",
+        "domainLookupEnd",
+        "connectStart",
+        "connectEnd",
+        "requestStart",
+        "responseStart",
+        "secureConnectionStart",
+    ];
+    for (let key of keys)
+        assert_equals(entry[key], 0, `entry ${key} must be zero for Cross Origin resource without passing Timing-Allow-Origin check`);
+}
+
+promise_test(function(t) {
+    let promise = observeResources(1).then(([entry]) => {
+        assertRedirectWithDisallowedTimingData(entry);
+        assertDisallowedTimingData(entry);
+    });
+
+    let sameOriginURL = uniqueDataURL("redirect-cross-origin-to-same-origin");
+    sameOriginURL = addACAOHeader(sameOriginURL);
+    const urlRedirect = urlWithRedirectTo(sameOriginURL);
+    const urlWithoutTAO = simpleCrossOriginURLBase() + urlRedirect;
+    const url = addTimingAllowOriginHeader(urlWithoutTAO, location.origin);
+
+    fetch(url);
+    return promise;
+}, "Cross-origin redirection with TAO to same origin loads without TAO must have filtered timing data");
diff --git a/Source/WebKit/NetworkProcess/NetworkDataTask.h b/Source/WebKit/NetworkProcess/NetworkDataTask.h
@@ -157,6 +157,8 @@ class NetworkDataTask : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<N
     const NetworkSession* networkSession() const;
     NetworkSession* networkSession();
 
+    virtual void setTimingAllowFailedFlag() { }
+
 protected:
     NetworkDataTask(NetworkSession&, NetworkDataTaskClient&, const WebCore::ResourceRequest&, WebCore::StoredCredentialsPolicy, bool shouldClearReferrerOnHTTPSToHTTPRedirect, bool dataTaskIsForMainFrameNavigation);
 

diff --git a/Source/WebKit/NetworkProcess/NetworkLoad.cpp b/Source/WebKit/NetworkProcess/NetworkLoad.cpp
@@ -323,6 +323,12 @@ void NetworkLoad::setH2PingCallback(const URL& url, CompletionHandler<void(Expec
         completionHandler(makeUnexpected(internalError(url)));
 }
 
+void NetworkLoad::setTimingAllowFailedFlag()
+{
+    if (m_task)
+        m_task->setTimingAllowFailedFlag();
+}
+
 String NetworkLoad::attributedBundleIdentifier(WebPageProxyIdentifier pageID)
 {
     if (m_task)

diff --git a/Source/WebKit/NetworkProcess/NetworkLoad.h b/Source/WebKit/NetworkProcess/NetworkLoad.h
@@ -81,6 +81,8 @@ class NetworkLoad final : public NetworkDataTaskClient {
     String description() const;
     void setH2PingCallback(const URL&, CompletionHandler<void(Expected<WTF::Seconds, WebCore::ResourceError>&&)>&&);
 
+    void setTimingAllowFailedFlag();
+
 private:
     // NetworkDataTaskClient
     void willPerformHTTPRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, RedirectCompletionHandler&&) final;

diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp b/Source/WebKit/NetworkProcess/NetworkLoadChecker.cpp
@@ -70,6 +70,9 @@ NetworkLoadChecker::NetworkLoadChecker(NetworkProcess& networkProcess, NetworkRe
     , m_schemeRegistry(schemeRegistry)
     , m_networkResourceLoader(networkResourceLoader)
 {
+    if (m_requestLoadType == LoadType::MainFrame)
+        m_origin = m_topOrigin;
+
     m_isSameOriginRequest = isSameOrigin(m_url, m_origin.get());
     switch (options.credentials) {
     case FetchOptions::Credentials::Include:
@@ -207,6 +210,15 @@ ResourceError NetworkLoadChecker::validateResponse(const ResourceRequest& reques
     if (response.containsInvalidHTTPHeaders())
         return badResponseHeadersError(request.url());
 
+    auto scope = makeScopeExit([&] {
+        if (!checkTAO(response)) {
+            if (auto metrics = response.takeNetworkLoadMetrics()) {
+                metrics->failsTAOCheck = true;
+                response.setDeprecatedNetworkLoadMetrics(WTFMove(metrics));
+            }
+        }
+    });
+
     if (m_redirectCount)
         response.setRedirected(true);
 
@@ -238,8 +250,10 @@ ResourceError NetworkLoadChecker::validateResponse(const ResourceRequest& reques
     ASSERT(m_options.mode == FetchOptions::Mode::Cors);
 
     // If we have a 304, the cached response is in WebProcess so we let WebProcess do the CORS check on the cached response.
-    if (response.httpStatusCode() == httpStatus304NotModified)
+    if (response.httpStatusCode() == httpStatus304NotModified) {
+        response.setTainting(ResourceResponse::Tainting::Cors);
         return { };
+    }
 
     auto result = passesAccessControlCheck(response, m_storedCredentialsPolicy, *origin(), m_networkResourceLoader.get());
     if (!result)
@@ -249,6 +263,30 @@ ResourceError NetworkLoadChecker::validateResponse(const ResourceRequest& reques
     return { };
 }
 
+// https://fetch.spec.whatwg.org/#concept-tao-check
+bool NetworkLoadChecker::checkTAO(const ResourceResponse& response)
+{
+    if (m_timingAllowFailedFlag)
+        return false;
+
+    if (m_origin) {
+        const auto& timingAllowOriginString = response.httpHeaderField(HTTPHeaderName::TimingAllowOrigin);
+        for (auto originWithSpace : StringView(timingAllowOriginString).split(',')) {
+            auto origin = originWithSpace.trim(isASCIIWhitespaceWithoutFF<UChar>);
+            if (origin == "*"_s || origin == m_origin->toString())
+                return true;
+        }
+    }
+
+    if (m_options.mode == FetchOptions::Mode::Navigate && !m_isSameOriginRequest) {
+        m_timingAllowFailedFlag = true;
+        return false;
+    }
+
+    m_timingAllowFailedFlag = response.tainting() != ResourceResponse::Tainting::Basic;
+    return !m_timingAllowFailedFlag;
+}
+
 auto NetworkLoadChecker::accessControlErrorForValidationHandler(String&& message) -> RequestOrRedirectionTripletOrError
 {
     return ResourceError { String { }, 0, m_url, WTFMove(message), ResourceError::Type::AccessControl };

diff --git a/Source/WebKit/NetworkProcess/NetworkLoadChecker.h b/Source/WebKit/NetworkProcess/NetworkLoadChecker.h
@@ -116,6 +116,8 @@ class NetworkLoadChecker : public CanMakeWeakPtr<NetworkLoadChecker> {
 
     const WebCore::FetchOptions& options() const { return m_options; }
 
+    bool timingAllowFailedFlag() const { return m_timingAllowFailedFlag; }
+
 private:
     WebCore::ContentSecurityPolicy* contentSecurityPolicy();
     const WebCore::OriginAccessPatterns& originAccessPatterns() const;
@@ -149,6 +151,8 @@ class NetworkLoadChecker : public CanMakeWeakPtr<NetworkLoadChecker> {
 
     RefPtr<WebCore::SecurityOrigin> parentOrigin() const { return m_parentOrigin; }
 
+    bool checkTAO(const WebCore::ResourceResponse&);
+
     WebCore::FetchOptions m_options;
     WebCore::StoredCredentialsPolicy m_storedCredentialsPolicy;
     bool m_allowPrivacyProxy;
@@ -188,6 +192,8 @@ class NetworkLoadChecker : public CanMakeWeakPtr<NetworkLoadChecker> {
     LoadType m_requestLoadType;
     RefPtr<NetworkSchemeRegistry> m_schemeRegistry;
     WeakPtr<NetworkResourceLoader> m_networkResourceLoader;
+
+    bool m_timingAllowFailedFlag { false };
 };
 
 }
diff --git a/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp b/Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
@@ -926,6 +926,8 @@ void NetworkResourceLoader::didReceiveResponse(ResourceResponse&& receivedRespon
             });
             return completionHandler(PolicyAction::Ignore);
         }
+        if (m_networkLoad && m_networkLoadChecker->timingAllowFailedFlag())
+            m_networkLoad->setTimingAllowFailedFlag();
     }
 
     initializeReportingEndpoints(m_response);
@@ -1056,6 +1058,8 @@ void NetworkResourceLoader::didReceiveBuffer(const WebCore::FragmentedSharedBuff
 
 void NetworkResourceLoader::didFinishLoading(const NetworkLoadMetrics& networkLoadMetrics)
 {
+    ASSERT(!m_networkLoadChecker || networkLoadMetrics.failsTAOCheck == m_networkLoadChecker->timingAllowFailedFlag());
+
     LOADER_RELEASE_LOG("didFinishLoading: (numBytesReceived=%zd, hasCacheEntryForValidation=%d)", m_numBytesReceived, !!m_cacheEntryForValidation);
 
     if (shouldCaptureExtraNetworkLoadMetrics())
@@ -1248,6 +1252,9 @@ void NetworkResourceLoader::willSendRedirectedRequestInternal(ResourceRequest&&
                 return completionHandler({ });
             }
 
+            if (m_networkLoad && m_networkLoadChecker && m_networkLoadChecker->timingAllowFailedFlag())
+                m_networkLoad->setTimingAllowFailedFlag();
+
             LOADER_RELEASE_LOG("willSendRedirectedRequest: NetworkLoadChecker::checkRedirection is done");
             if (m_parameters.options.redirect == FetchOptions::Redirect::Manual) {
                 this->didFinishWithRedirectResponse(WTFMove(result->request), WTFMove(result->redirectRequest), WTFMove(result->redirectResponse));

diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.h
@@ -104,6 +104,8 @@ class NetworkDataTaskCocoa final : public NetworkDataTask, public NetworkTaskCoc
     NSURLSessionTask* task() const final;
     WebCore::StoredCredentialsPolicy storedCredentialsPolicy() const final { return m_storedCredentialsPolicy; }
 
+    void setTimingAllowFailedFlag() final;
+
     WeakPtr<SessionWrapper> m_sessionWrapper;
     RefPtr<SandboxExtension> m_sandboxExtension;
     RetainPtr<NSURLSessionDataTask> m_task;

diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm
@@ -680,19 +680,9 @@ static void adjustNetworkServiceTypeIfNecessary(WebCore::ResourceRequestRequeste
 
 #endif // ENABLE(INSPECTOR_NETWORK_THROTTLING)
 
-void NetworkDataTaskCocoa::checkTAO(const WebCore::ResourceResponse& response)
+void NetworkDataTaskCocoa::setTimingAllowFailedFlag()
 {
-    if (networkLoadMetrics().failsTAOCheck)
-        return;
-
-    RefPtr<WebCore::SecurityOrigin> origin;
-    if (isTopLevelNavigation())
-        origin = WebCore::SecurityOrigin::create(firstRequest().url());
-    else
-        origin = m_sourceOrigin;
-
-    if (origin)
-        networkLoadMetrics().failsTAOCheck = !passesTimingAllowOriginCheck(response, *origin);
+    networkLoadMetrics().failsTAOCheck = true;
 }
 
 NSURLSessionTask* NetworkDataTaskCocoa::task() const

diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm
@@ -630,7 +630,6 @@ - (void)URLSession:(NSURLSession *)session task:(NSURLSessionTask *)task willPer
             ASSERT_NOT_REACHED();
 
         WebCore::ResourceResponse resourceResponse(response);
-        networkDataTask->checkTAO(resourceResponse);
 
         networkDataTask->willPerformHTTPRedirection(WTFMove(resourceResponse), request, [session = networkDataTask->networkSession(), completionHandler = makeBlockPtr(completionHandler), taskIdentifier, shouldIgnoreHSTS](auto&& request) {
 #if !LOG_DISABLED
@@ -1096,8 +1095,6 @@ - (void)URLSession:(NSURLSession *)session dataTask:(NSURLSessionDataTask *)data
         // all the fields when sending the response to the WebContent process over IPC.
         resourceResponse.disableLazyInitialization();
 
-        networkDataTask->checkTAO(resourceResponse);
-
         resourceResponse.setDeprecatedNetworkLoadMetrics(WebCore::copyTimingData(taskMetrics, networkDataTask->networkLoadMetrics()));
 
         networkDataTask->didReceiveResponse(WTFMove(resourceResponse), negotiatedLegacyTLS, privateRelayed, [completionHandler = makeBlockPtr(completionHandler), taskIdentifier](WebCore::PolicyAction policyAction) {

diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.cpp
@@ -593,18 +593,17 @@ void NetworkDataTaskCurl::updateNetworkLoadMetrics(WebCore::NetworkLoadMetrics&
     if (!m_startTime)
         m_startTime = networkLoadMetrics.fetchStart;
 
-    if (!m_failsTAOCheck) {
-        RefPtr<SecurityOrigin> origin = isTopLevelNavigation() ? SecurityOrigin::create(firstRequest().url()) : m_sourceOrigin;
-        if (origin)
-            m_failsTAOCheck = !passesTimingAllowOriginCheck(m_response, *origin);
-    }
-
     networkLoadMetrics.redirectStart = m_startTime;
     networkLoadMetrics.redirectCount = m_redirectCount;
     networkLoadMetrics.failsTAOCheck = m_failsTAOCheck;
     networkLoadMetrics.hasCrossOriginRedirect = m_hasCrossOriginRedirect;
 }
 
+void NetworkDataTaskCurl::setTimingAllowFailedFlag()
+{
+    m_failsTAOCheck = true;
+}
+
 void NetworkDataTaskCurl::setPendingDownloadLocation(const String& filename, SandboxExtension::Handle&& sandboxExtensionHandle, bool allowOverwrite)
 {
     NetworkDataTask::setPendingDownloadLocation(filename, WTFMove(sandboxExtensionHandle), allowOverwrite);

diff --git a/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h b/Source/WebKit/NetworkProcess/curl/NetworkDataTaskCurl.h
@@ -67,6 +67,7 @@ class NetworkDataTaskCurl final : public NetworkDataTask, public WebCore::CurlRe
     void resume() override;
     void invalidateAndCancel() override;
     NetworkDataTask::State state() const override;
+    void setTimingAllowFailedFlag() final;
 
     Ref<WebCore::CurlRequest> createCurlRequest(WebCore::ResourceRequest&&, RequestStatus = RequestStatus::NewRequest);
     void curlDidSendData(WebCore::CurlRequest&, unsigned long long, unsigned long long) override;

diff --git a/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp b/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.cpp
@@ -460,15 +460,14 @@ void NetworkDataTaskSoup::didSendRequest(GRefPtr<GInputStream>&& inputStream)
     m_networkLoadMetrics.responseStart = MonotonicTime::now();
 #endif
 
-    if (!m_networkLoadMetrics.failsTAOCheck) {
-        RefPtr<SecurityOrigin> origin = isTopLevelNavigation() ? SecurityOrigin::create(firstRequest().url()) : m_sourceOrigin;
-        if (origin)
-            m_networkLoadMetrics.failsTAOCheck = !passesTimingAllowOriginCheck(m_response, *origin);
-    }
-
     dispatchDidReceiveResponse();
 }
 
+void NetworkDataTaskSoup::setTimingAllowFailedFlag()
+{
+    m_networkLoadMetrics.failsTAOCheck = true;
+}
+
 void NetworkDataTaskSoup::dispatchDidReceiveResponse()
 {
     ASSERT(!m_response.isNull());

diff --git a/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h b/Source/WebKit/NetworkProcess/soup/NetworkDataTaskSoup.h
@@ -54,6 +54,7 @@ class NetworkDataTaskSoup final : public NetworkDataTask {
     void resume() override;
     void invalidateAndCancel() override;
     NetworkDataTask::State state() const override;
+    void setTimingAllowFailedFlag() final;
 
     void setPendingDownloadLocation(const String&, SandboxExtension::Handle&&, bool /*allowOverwrite*/) override;
     String suggestedFilename() const override;