[webkit-changes] [WebKit/WebKit] bbe099: Cherry-pick 272448.87 at safari-7618-branch (b70a943d...

[Chris Dumez]

  Branch: refs/heads/webkitglib/2.44
  Home:   https://github.com/WebKit/WebKit
  Commit: bbe099964156ba500bc7c7031c8e65ca5606081c
      https://github.com/WebKit/WebKit/commit/bbe099964156ba500bc7c7031c8e65ca5606081c
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2024-03-15 (Fri, 15 Mar 2024)

  Changed paths:
    M Source/WebCore/workers/service/server/SWServer.cpp
    M Source/WebCore/workers/service/server/SWServerToContextConnection.cpp
    M Source/WebCore/workers/service/server/SWServerWorker.cpp
    M Source/WebCore/workers/service/server/SWServerWorker.h
    M Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp
    M Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.cpp
    M Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorker.h
    M Source/WebKit/NetworkProcess/SharedWorker/WebSharedWorkerServer.cpp
    M Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm

  Log Message:
  -----------
  Cherry-pick 272448.87 at safari-7618-branch (b70a943d3745). https://bugs.webkit.org/show_bug.cgi?id=267298

    Regression: Third-party service worker processes get killed when trying to do loads
    https://bugs.webkit.org/show_bug.cgi?id=267298
    rdar://120587179

    Reviewed by Youenn Fablet.

    We recently added origin IPC message checks to make sure that WebProcesses can
    only access cookies from specific origins. Due to a bug in our service worker
    code, the network process would allow the wrong origin in the case of
    third-party service workers (it would allow the client origin instead of the
    top origin). As a result, the network process would kill the third-party
    service worker process as soon as it would try to attempt a load. This was
    occurring on https://boingboing.net/2024/01/06/stanley-water-bottle-madness-grips-america.html
    for example, with the tiktok.com service worker.

    When the network process needs to launch a service worker process, it sends an
    EstablishRemoteWorkerContextConnectionToNetworkProcess IPC to the UIProcess,
    with a given registrable domain. The UIProcess would use this registrable
    domain to select a suitable WebProcess or launch one. When launching a new
    process, the UIProcess would send an IPC to the network process telling it this
    new WebProcess is allowed to access cookies under the given registrable domain.

    Both from the process selection point of view and from the network process
    cookie access point of view, we expect this registrable domain to be the top
    origin's registrable domain. As a result, in the example above, the third-party
    tiktok.com service worker under boingboing.net, would be expected to use a
    "boingboing.net" WebProcess and only have access to cookies under the
    "boingboing.net" first party.

    However, our service worker logic was using the registrable domain of the
    service worker script URL instead. As a result, we would select a "tiktok.com"
    WebProcess for the service worker process, which was wrong from a site
    isolation perspective (since top-level tiktok.com would share the same process
    as tiktok.com under boingboing.net). Also, the network process would allow
    access to top-level tiktok.com cookies if the WebProcess requested them, which
    was wrong too.

    Later on, the service worker would try to do a load. The network request would
    request use "boingboing.net" as firstParty for cookies, which is correct.
    However, the network process would reject such load, since the process is only
    allowed to use "tiktok.com" as first party for cookies. It would then kill the
    service worker process for good measure since it would assume it is compromised.

    To address the issue, we now properly use the registrable domain of the top
    level origin when sending the EstablishRemoteWorkerContextConnectionToNetworkProcess
    IPC for and service worker connection selection in general. I updated the shared
    worker code as well to maintain consistency.

    Note that in order to write an API test for this, I had to restore the service
    worker from disk first. When the service worker is newly registered by JS, we
    would first tell the network process to allow the wrong client origin. However,
    a later IPC to tell the network process to also allow the top level origin. As
    a result, newly registered third-party service workers would not get terminated
    which is why our tests were passing. Those service worker origins were allowed
    access to cookies they shouldn't have access to though so there was a security
    issue still for them.
    When restoring the service worker from disk though, we'd only send a single IPC
    to the network process telling it to allow the original of the service worker
    script URL. This allowed me to write a test and explains the service worker
    processes terminations on boingboing.net.

    * Source/WebCore/workers/service/server/SWServer.cpp:
    (WebCore::SWServer::addRegistrationFromStore):
    (WebCore::SWServer::scheduleJob):
    (WebCore::SWServer::tryInstallContextData):
    (WebCore::SWServer::runServiceWorkerIfNecessary):
    (WebCore::SWServer::markAllWorkersForRegistrableDomainAsTerminated):
    (WebCore::SWServer::removeContextConnectionIfPossible):
    (WebCore::SWServer::fireFunctionalEvent):
    * Source/WebCore/workers/service/server/SWServerToContextConnection.cpp:
    (WebCore::SWServerToContextConnection::terminateWhenPossible):
    * Source/WebCore/workers/service/server/SWServerWorker.cpp:
    (WebCore::m_lastNavigationWasAppInitiated):
    (WebCore::SWServerWorker::contextConnection):
    (WebCore::SWServerWorker::terminationIfPossibleTimerFired):
    * Source/WebCore/workers/service/server/SWServerWorker.h:
    (WebCore::SWServerWorker::topRegistrableDomain const):
    (WebCore::SWServerWorker::registrableDomain const): Deleted.
    * Source/WebKit/NetworkProcess/ServiceWorker/WebSWServerConnection.cpp:
    (WebKit::WebSWServerConnection::startFetch):
    * Tools/TestWebKitAPI/Tests/WebKitCocoa/ServiceWorkerBasic.mm:
    (-[SWMessageHandlerForRestoreFromDiskTest resetExpectedMessage:]):

    Canonical link: https://commits.webkit.org/272448.87@safari-7618-branch

Canonical link: https://commits.webkit.org/274313.94@webkitglib/2.44



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications