Skip to content

Commit

Permalink
Arbitrary cookie access via NetworkConnectionToWebProcess::cookiesForDOM
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=259040
rdar://107270673

Reviewed by Alex Christensen.

Currently, our `allowsFirstPartyForCookies` message checks will always pass if the given
URL can’t be parsed into a RegistrableDomain. This patch removes each of the FIXMEs in the
`allowsFirstPartyForCookies` functions which allow this.

260966@main previously removed most of these FIXMEs, but was reverted due to cached resources
causing the web process to crash when loaded. This is fixed by setting the first party for
cookies to the request in `CachedResourceLoader::requestResource`.

* Source/WebCore/loader/PingLoader.cpp:
(WebCore::PingLoader::sendViolationReport):
* Source/WebCore/loader/cache/CachedResourceLoader.cpp:
(WebCore::CachedResourceLoader::requestResource):
* Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:
(WebKit::NetworkConnectionToWebProcess::createSocketChannel):
(WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
(WebKit::NetworkConnectionToWebProcess::cookiesForDOM):
(WebKit::NetworkConnectionToWebProcess::cookiesForDOMAsync):
(WebKit::NetworkConnectionToWebProcess::setCookiesFromDOM):
(WebKit::NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue):
(WebKit::NetworkConnectionToWebProcess::getRawCookies):
(WebKit::NetworkConnectionToWebProcess::domCookiesForHost):
* Source/WebKit/NetworkProcess/NetworkProcess.cpp:
* Source/WebKit/NetworkProcess/NetworkProcess.h:
* Source/WebKit/Shared/AuxiliaryProcess.cpp:
(WebKit::AuxiliaryProcess::allowsFirstPartyForCookies):
* Source/WebKit/Shared/AuxiliaryProcess.h:
* Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:
(WebKit::WebLoaderStrategy::scheduleLoadFromNetworkProcess):
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:
(WebKit::PDFPlugin::getResourceBytesAtPosition):
* Source/WebKit/WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::Stream::start):
* Source/WebKit/WebProcess/WebProcess.cpp:
(WebKit::WebProcess::allowsFirstPartyForCookies):

Canonical link: https://commits.webkit.org/266074@main
  • Loading branch information
charliewolfe authored and JonWBedard committed Jul 14, 2023
1 parent 861b908 commit cb29a87
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 45 deletions.
7 changes: 3 additions & 4 deletions Source/WebCore/loader/PingLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,10 @@ void PingLoader::sendViolationReport(LocalFrame& frame, const URL& reportURL, Re
break;
}

bool removeCookies = true;
if (document.securityOrigin().isSameSchemeHostPort(SecurityOrigin::create(reportURL).get()))
removeCookies = false;
if (removeCookies)
if (document.firstPartyForCookies().isNull() || !document.securityOrigin().isSameSchemeHostPort(SecurityOrigin::create(reportURL).get()))
request.setAllowCookies(false);
else
request.setFirstPartyForCookies(document.firstPartyForCookies());

HTTPHeaderMap originalRequestHeader = request.httpHeaderFields();

Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/loader/cache/CachedResourceLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1110,8 +1110,10 @@ ResourceErrorOr<CachedResourceHandle<CachedResource>> CachedResourceLoader::requ

// See if we can use an existing resource from the cache.
CachedResourceHandle<CachedResource> resource;
if (auto* document = this->document())
if (auto* document = this->document()) {
request.setDomainForCachePartition(*document);
request.resourceRequest().setFirstPartyForCookies(document->firstPartyForCookies());
}

if (request.allowsCaching())
resource = memoryCache.resourceForRequest(request.resourceRequest(), page.sessionID());
Expand Down
19 changes: 10 additions & 9 deletions Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ void NetworkConnectionToWebProcess::didReceiveInvalidMessage(IPC::Connection&, I

void NetworkConnectionToWebProcess::createSocketChannel(const ResourceRequest& request, const String& protocol, WebSocketIdentifier identifier, WebPageProxyIdentifier webPageProxyID, std::optional<FrameIdentifier> frameID, std::optional<PageIdentifier> pageID, const ClientOrigin& clientOrigin, bool hadMainFrameMainResourcePrivateRelayed, bool allowPrivacyProxy, OptionSet<AdvancedPrivacyProtections> advancedPrivacyProtections, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, WebCore::StoredCredentialsPolicy storedCredentialsPolicy)
{
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, request.firstPartyForCookies()));
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { request.firstPartyForCookies() }));

ASSERT(!m_networkSocketChannels.contains(identifier));
if (auto channel = NetworkSocketChannel::create(*this, m_sessionID, request, protocol, identifier, webPageProxyID, frameID, pageID, clientOrigin, hadMainFrameMainResourcePrivateRelayed, allowPrivacyProxy, advancedPrivacyProtections, shouldRelaxThirdPartyCookieBlocking, storedCredentialsPolicy))
Expand Down Expand Up @@ -548,7 +548,8 @@ std::unique_ptr<ServiceWorkerFetchTask> NetworkConnectionToWebProcess::createFet

void NetworkConnectionToWebProcess::scheduleResourceLoad(NetworkResourceLoadParameters&& loadParameters, std::optional<NetworkResourceLoadIdentifier> existingLoaderToResume)
{
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, loadParameters.request.firstPartyForCookies()));
if (loadParameters.request.allowCookies())
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { loadParameters.request.firstPartyForCookies() }));

CONNECTION_RELEASE_LOG(Loading, "scheduleResourceLoad: (parentPID=%d, pageProxyID=%" PRIu64 ", webPageID=%" PRIu64 ", frameID=%" PRIu64 ", resourceID=%" PRIu64 ", existingLoaderToResume=%" PRIu64 ")", loadParameters.parentPID, loadParameters.webPageProxyID.toUInt64(), loadParameters.webPageID.toUInt64(), loadParameters.webFrameID.object().toUInt64(), loadParameters.identifier.toUInt64(), valueOrDefault(existingLoaderToResume).toUInt64());

Expand Down Expand Up @@ -771,7 +772,7 @@ void NetworkConnectionToWebProcess::registerURLSchemesAsCORSEnabled(Vector<Strin

void NetworkConnectionToWebProcess::cookiesForDOM(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, FrameIdentifier frameID, PageIdentifier pageID, IncludeSecureCookies includeSecureCookies, ApplyTrackingPrevention applyTrackingPrevention, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, CompletionHandler<void(String cookieString, bool secureCookiesAccessed)>&& completionHandler)
{
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty), completionHandler({ }, false));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }), completionHandler({ }, false));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand All @@ -788,7 +789,7 @@ void NetworkConnectionToWebProcess::cookiesForDOM(const URL& firstParty, const S

void NetworkConnectionToWebProcess::setCookiesFromDOM(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, WebCore::FrameIdentifier frameID, PageIdentifier pageID, ApplyTrackingPrevention applyTrackingPrevention, const String& cookieString, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking)
{
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty));
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand All @@ -804,7 +805,7 @@ void NetworkConnectionToWebProcess::setCookiesFromDOM(const URL& firstParty, con

void NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, std::optional<FrameIdentifier> frameID, std::optional<PageIdentifier> pageID, IncludeSecureCookies includeSecureCookies, ApplyTrackingPrevention applyTrackingPrevention, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, CompletionHandler<void(String, bool)>&& completionHandler)
{
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty), completionHandler({ }, false));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }), completionHandler({ }, false));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand All @@ -815,7 +816,7 @@ void NetworkConnectionToWebProcess::cookieRequestHeaderFieldValue(const URL& fir

void NetworkConnectionToWebProcess::getRawCookies(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, std::optional<FrameIdentifier> frameID, std::optional<PageIdentifier> pageID, ApplyTrackingPrevention applyTrackingPrevention, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, CompletionHandler<void(Vector<WebCore::Cookie>&&)>&& completionHandler)
{
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty), completionHandler({ }));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }), completionHandler({ }));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand Down Expand Up @@ -844,7 +845,7 @@ void NetworkConnectionToWebProcess::deleteCookie(const URL& url, const String& c

void NetworkConnectionToWebProcess::cookiesForDOMAsync(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, FrameIdentifier frameID, PageIdentifier pageID, IncludeSecureCookies includeSecureCookies, ApplyTrackingPrevention applyTrackingPrevention, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, WebCore::CookieStoreGetOptions&& options, CompletionHandler<void(std::optional<Vector<WebCore::Cookie>>&&)>&& completionHandler)
{
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty), completionHandler(std::nullopt));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }), completionHandler(std::nullopt));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand All @@ -861,7 +862,7 @@ void NetworkConnectionToWebProcess::cookiesForDOMAsync(const URL& firstParty, co

void NetworkConnectionToWebProcess::setCookieFromDOMAsync(const URL& firstParty, const SameSiteInfo& sameSiteInfo, const URL& url, WebCore::FrameIdentifier frameID, PageIdentifier pageID, ApplyTrackingPrevention applyTrackingPrevention, ShouldRelaxThirdPartyCookieBlocking shouldRelaxThirdPartyCookieBlocking, WebCore::Cookie&& cookie, CompletionHandler<void(bool)>&& completionHandler)
{
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, firstParty));
NETWORK_PROCESS_MESSAGE_CHECK(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { firstParty }));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand All @@ -881,7 +882,7 @@ void NetworkConnectionToWebProcess::domCookiesForHost(const URL& url, bool subsc
{
auto host = url.host().toString();
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(HashSet<String>::isValidValue(host), completionHandler({ }));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, url), completionHandler({ }));
NETWORK_PROCESS_MESSAGE_CHECK_COMPLETION(m_networkProcess->allowsFirstPartyForCookies(m_webProcessIdentifier, RegistrableDomain { url }), completionHandler({ }));

auto* networkStorageSession = storageSession();
if (!networkStorageSession)
Expand Down
14 changes: 6 additions & 8 deletions Source/WebKit/NetworkProcess/NetworkProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -421,16 +421,14 @@ void NetworkProcess::webProcessWillLoadWebArchive(WebCore::ProcessIdentifier pro
}).iterator->value.first = LoadedWebArchive::Yes;
}

bool NetworkProcess::allowsFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, const URL& firstParty)
{
return AuxiliaryProcess::allowsFirstPartyForCookies(firstParty, [&] {
RegistrableDomain firstPartyDomain(firstParty);
return allowsFirstPartyForCookies(processIdentifier, firstPartyDomain);
});
}

bool NetworkProcess::allowsFirstPartyForCookies(WebCore::ProcessIdentifier processIdentifier, const RegistrableDomain& firstPartyDomain)
{
#if PLATFORM(GTK)
// FIXME: This shouldn't be needed but is hit for some web socket tests on GTK.
if (firstPartyDomain.isEmpty())
return true;
#endif

if (!decltype(m_allowedFirstPartiesForCookies)::isValidKey(processIdentifier)) {
ASSERT_NOT_REACHED();
return false;
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/NetworkProcess/NetworkProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,6 @@ class NetworkProcess : public AuxiliaryProcess, private DownloadManager::Client,
void deleteWebsiteDataForOrigin(PAL::SessionID, OptionSet<WebsiteDataType>, const WebCore::ClientOrigin&, CompletionHandler<void()>&&);
void deleteWebsiteDataForOrigins(PAL::SessionID, OptionSet<WebsiteDataType>, const Vector<WebCore::SecurityOriginData>& origins, const Vector<String>& cookieHostNames, const Vector<String>& HSTSCacheHostnames, const Vector<RegistrableDomain>&, CompletionHandler<void()>&&);

bool allowsFirstPartyForCookies(WebCore::ProcessIdentifier, const URL&);
bool allowsFirstPartyForCookies(WebCore::ProcessIdentifier, const RegistrableDomain&);
void addAllowedFirstPartyForCookies(WebCore::ProcessIdentifier, WebCore::RegistrableDomain&&, LoadedWebArchive, CompletionHandler<void()>&&);
void webProcessWillLoadWebArchive(WebCore::ProcessIdentifier);
Expand Down
17 changes: 0 additions & 17 deletions Source/WebKit/Shared/AuxiliaryProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -251,25 +251,8 @@ void AuxiliaryProcess::didReceiveMemoryPressureEvent(bool isCritical)

#endif // !PLATFORM(COCOA)

bool AuxiliaryProcess::allowsFirstPartyForCookies(const URL& firstParty, Function<bool()>&& domainCheck)
{
// FIXME: This should probably not be necessary. If about:blank is the first party for cookies,
// we should set it to be the inherited origin then remove this exception.
if (firstParty.isAboutBlank())
return true;

if (firstParty.isNull())
return true; // FIXME: This shouldn't be allowed.

return domainCheck();
}

bool AuxiliaryProcess::allowsFirstPartyForCookies(const WebCore::RegistrableDomain& firstPartyDomain, HashSet<WebCore::RegistrableDomain>& allowedFirstPartiesForCookies)
{
// FIXME: This shouldn't be needed but it is hit sometimes at least with PDFs.
if (firstPartyDomain.isEmpty())
return true;

if (!std::remove_reference_t<decltype(allowedFirstPartiesForCookies)>::isValidValue(firstPartyDomain)) {
ASSERT_NOT_REACHED();
return false;
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/Shared/AuxiliaryProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ class AuxiliaryProcess : public IPC::Connection::Client, public IPC::MessageSend
// IPC::Connection::Client.
void didClose(IPC::Connection&) override;

bool allowsFirstPartyForCookies(const URL&, Function<bool()>&&);
bool allowsFirstPartyForCookies(const WebCore::RegistrableDomain&, HashSet<WebCore::RegistrableDomain>&);

private:
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ void WebLoaderStrategy::scheduleLoadFromNetworkProcess(ResourceLoader& resourceL
existingNetworkResourceLoadIdentifierToResume = std::exchange(m_existingNetworkResourceLoadIdentifierToResume, std::nullopt);
WEBLOADERSTRATEGY_RELEASE_LOG("scheduleLoad: Resource is being scheduled with the NetworkProcess (priority=%d, existingNetworkResourceLoadIdentifierToResume=%" PRIu64 ")", static_cast<int>(resourceLoader.request().priority()), valueOrDefault(existingNetworkResourceLoadIdentifierToResume).toUInt64());

if (frame && !frame->settings().siteIsolationEnabled() && !WebProcess::singleton().allowsFirstPartyForCookies(loadParameters.request.firstPartyForCookies()))
if (frame && !frame->settings().siteIsolationEnabled() && loadParameters.request.allowCookies() && !WebProcess::singleton().allowsFirstPartyForCookies(loadParameters.request.firstPartyForCookies()))
RELEASE_LOG_FAULT(IPC, "scheduleLoad: Process will terminate due to failed allowsFirstPartyForCookies check");

if (WebProcess::singleton().ensureNetworkProcessConnection().connection().send(Messages::NetworkConnectionToWebProcess::ScheduleResourceLoad(loadParameters, existingNetworkResourceLoadIdentifierToResume), 0) != IPC::Error::NoError) {
Expand Down
2 changes: 2 additions & 0 deletions Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1031,6 +1031,8 @@ static void dataProviderReleaseInfoCallback(void* info)
resourceRequest.setURL(m_view->mainResourceURL());
resourceRequest.setHTTPHeaderField(HTTPHeaderName::Range, makeString("bytes="_s, position, "-"_s, position + count - 1));
resourceRequest.setCachePolicy(ResourceRequestCachePolicy::DoNotUseAnyCache);
if (auto* document = coreFrame->document())
resourceRequest.setFirstPartyForCookies(document->topDocument().url());

#if !LOG_DISABLED
pdfLog(makeString("Scheduling a stream loader for request ", identifier, " (", count, " bytes at ", position, ")"));
Expand Down
3 changes: 3 additions & 0 deletions Source/WebKit/WebProcess/Plugins/PluginView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ void PluginView::Stream::start()
auto* frame = m_pluginView->frame();
ASSERT(frame);

if (auto* document = frame->document())
m_request.setFirstPartyForCookies(document->topDocument().url());

WebProcess::singleton().webLoaderStrategy().schedulePluginStreamLoad(*frame, *this, ResourceRequest {m_request}, [this, protectedThis = Ref { *this }](RefPtr<NetscapePlugInStreamLoader>&& loader) {
m_loader = WTFMove(loader);
});
Expand Down
4 changes: 1 addition & 3 deletions Source/WebKit/WebProcess/WebProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2359,9 +2359,7 @@ void WebProcess::addAllowedFirstPartyForCookies(WebCore::RegistrableDomain&& fir

bool WebProcess::allowsFirstPartyForCookies(const URL& firstParty)
{
return AuxiliaryProcess::allowsFirstPartyForCookies(firstParty, [&] {
return AuxiliaryProcess::allowsFirstPartyForCookies(WebCore::RegistrableDomain { firstParty }, m_allowedFirstPartiesForCookies);
});
return AuxiliaryProcess::allowsFirstPartyForCookies(WebCore::RegistrableDomain { firstParty }, m_allowedFirstPartiesForCookies);
}

} // namespace WebKit
Expand Down

0 comments on commit cb29a87

Please sign in to comment.