⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 249700 in webkit


Ignore:
Timestamp:
Sep 9, 2019, 8:20:08 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r249649. rdar://problem/55198071

[macOS] Pid is sometimes invalid when creating sandbox extensions by pid.
​https://bugs.webkit.org/show_bug.cgi?id=201543
<rdar://problem/54733465>

Reviewed by Brent Fulgham.

There is a race condition when starting a load of a local file, where the WebContent process has not finished
launching yet, and its pid is not available. When we try to create a sandbox extension by using the pid of the
WebContent process, it is not available in the cases where the WebContent process has just launched and has not
finished launching yet. This patch creates a new dummy Web page message, 'LoadRequestWaitingForPID', which will
be sent instead of a normal 'LoadRequest' message, and only when the WebContent process has not finished
launching. When the WebContent process has finished launching, and we are about to actually send the pending
messages, we can detect that a 'LoadRequestWaitingForPID' has been appended for sending, and replace it with a
normal 'LoadReqest' message where we have created the sandbox extension issue with a valid pid. The message
'LoadRequestWaitingForPID' is never intended to reach the WebContent process, it is just there to replace with
a normal 'LoadRequest' message with a new sandbox extension. In the implementation of the message handler on
the WebContent process side, we assert that the method is never called. This patch makes sure the ordering of
the Web page messages are the same, even when we modify the message.

  • UIProcess/AuxiliaryProcessProxy.cpp: (WebKit::AuxiliaryProcessProxy::didFinishLaunching):
  • UIProcess/WebPageProxy.cpp: (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle): (WebKit::WebPageProxy::loadRequestWithNavigationShared): (WebKit::WebPageProxy::loadFile):
  • WebProcess/WebPage/WebPage.cpp: (WebKit::WebPage::fileLoadRequest):
  • WebProcess/WebPage/WebPage.h:
  • WebProcess/WebPage/WebPage.messages.in:

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@249649 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-608-branch/Source/WebKit
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608-branch/Source/WebKit/ChangeLog

    r249699 r249700  
     12019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r249649. rdar://problem/55198071
     4
     5    [macOS] Pid is sometimes invalid when creating sandbox extensions by pid.
     6    https://bugs.webkit.org/show_bug.cgi?id=201543
     7    <rdar://problem/54733465>
     8   
     9    Reviewed by Brent Fulgham.
     10   
     11    There is a race condition when starting a load of a local file, where the WebContent process has not finished
     12    launching yet, and its pid is not available. When we try to create a sandbox extension by using the pid of the
     13    WebContent process, it is not available in the cases where the WebContent process has just launched and has not
     14    finished launching yet. This patch creates a new dummy Web page message, 'LoadRequestWaitingForPID', which will
     15    be sent instead of a normal 'LoadRequest' message, and only when the WebContent process has not finished
     16    launching. When the WebContent process has finished launching, and we are about to actually send the pending
     17    messages, we can detect that a 'LoadRequestWaitingForPID' has been appended for sending, and replace it with a
     18    normal 'LoadReqest' message where we have created the sandbox extension issue with a valid pid. The message
     19    'LoadRequestWaitingForPID' is never intended to reach the WebContent process, it is just there to replace with
     20    a normal 'LoadRequest' message with a new sandbox extension. In the implementation of the message handler on
     21    the WebContent process side, we assert that the method is never called. This patch makes sure the ordering of
     22    the Web page messages are the same, even when we modify the message.
     23   
     24    * UIProcess/AuxiliaryProcessProxy.cpp:
     25    (WebKit::AuxiliaryProcessProxy::didFinishLaunching):
     26    * UIProcess/WebPageProxy.cpp:
     27    (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
     28    (WebKit::WebPageProxy::loadRequestWithNavigationShared):
     29    (WebKit::WebPageProxy::loadFile):
     30    * WebProcess/WebPage/WebPage.cpp:
     31    (WebKit::WebPage::fileLoadRequest):
     32    * WebProcess/WebPage/WebPage.h:
     33    * WebProcess/WebPage/WebPage.messages.in:
     34   
     35    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249649 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     36
     37    2019-09-09  Per Arne Vollan  <pvollan@apple.com>
     38
     39            [macOS] Pid is sometimes invalid when creating sandbox extensions by pid.
     40            https://bugs.webkit.org/show_bug.cgi?id=201543
     41            <rdar://problem/54733465>
     42
     43            Reviewed by Brent Fulgham.
     44
     45            There is a race condition when starting a load of a local file, where the WebContent process has not finished
     46            launching yet, and its pid is not available. When we try to create a sandbox extension by using the pid of the
     47            WebContent process, it is not available in the cases where the WebContent process has just launched and has not
     48            finished launching yet. This patch creates a new dummy Web page message, 'LoadRequestWaitingForPID', which will
     49            be sent instead of a normal 'LoadRequest' message, and only when the WebContent process has not finished
     50            launching. When the WebContent process has finished launching, and we are about to actually send the pending
     51            messages, we can detect that a 'LoadRequestWaitingForPID' has been appended for sending, and replace it with a
     52            normal 'LoadReqest' message where we have created the sandbox extension issue with a valid pid. The message
     53            'LoadRequestWaitingForPID' is never intended to reach the WebContent process, it is just there to replace with
     54            a normal 'LoadRequest' message with a new sandbox extension. In the implementation of the message handler on
     55            the WebContent process side, we assert that the method is never called. This patch makes sure the ordering of
     56            the Web page messages are the same, even when we modify the message.
     57
     58            * UIProcess/AuxiliaryProcessProxy.cpp:
     59            (WebKit::AuxiliaryProcessProxy::didFinishLaunching):
     60            * UIProcess/WebPageProxy.cpp:
     61            (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
     62            (WebKit::WebPageProxy::loadRequestWithNavigationShared):
     63            (WebKit::WebPageProxy::loadFile):
     64            * WebProcess/WebPage/WebPage.cpp:
     65            (WebKit::WebPage::fileLoadRequest):
     66            * WebProcess/WebPage/WebPage.h:
     67            * WebProcess/WebPage/WebPage.messages.in:
     68
    1692019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    270

  • branches/safari-608-branch/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp

    r242303 r249700  
    2828
    2929#include "AuxiliaryProcessMessages.h"
     30#include "LoadParameters.h"
     31#include "WebPageMessages.h"
    3032#include <wtf/RunLoop.h>
    3133
    … …  
    176178        std::unique_ptr<IPC::Encoder> message = WTFMove(m_pendingMessages[i].first);
    177179        OptionSet<IPC::SendOption> sendOptions = m_pendingMessages[i].second;
     180#if HAVE(SANDBOX_ISSUE_MACH_EXTENSION_TO_PROCESS_BY_PID)
     181        if (message->messageName() == "LoadRequestWaitingForPID") {
     182            auto buffer = message->buffer();
     183            auto bufferSize = message->bufferSize();
     184            std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
     185            LoadParameters loadParameters;
     186            String sandboxExtensionPath;
     187            if (decoder->decode(loadParameters) && decoder->decode(sandboxExtensionPath)) {
     188                SandboxExtension::createHandleForReadByPid(sandboxExtensionPath, processIdentifier(), loadParameters.sandboxExtensionHandle);
     189                send(Messages::WebPage::LoadRequest(loadParameters), decoder->destinationID());
     190                continue;
     191            }
     192        }
     193#endif
    178194        m_connection->sendMessage(WTFMove(message), sendOptions);
    179195    }

  • branches/safari-608-branch/Source/WebKit/UIProcess/WebPageProxy.cpp

    r249691 r249700  
    10561056#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
    10571057        if (SandboxExtension::createHandleForReadByPid(resourceDirectoryURL.fileSystemPath(), process.processIdentifier(), sandboxExtensionHandle)) {
    1058             m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
     1058            process.assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
    10591059            return;
    10601060        }
    1061 #endif
     1061#else
    10621062        if (SandboxExtension::createHandle(resourceDirectoryURL.fileSystemPath(), SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
    1063             m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
     1063            process.assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
    10641064            return;
    10651065        }
     1066#endif
    10661067    }
    10671068
    … …  
    10771078        return;
    10781079    }
    1079 #endif
     1080#else
    10801081    if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
    10811082        willAcquireUniversalFileReadSandboxExtension(process);
    10821083        return;
    10831084    }
     1085#endif
    10841086
    10851087#if PLATFORM(COCOA)
    … …  
    10911093    auto baseURL = URL(URL(), url.baseAsString());
    10921094    auto basePath = baseURL.fileSystemPath();
    1093     if (!basePath.isNull() && SandboxExtension::createHandle(basePath, SandboxExtension::Type::ReadOnly, sandboxExtensionHandle))
    1094         m_process->assumeReadAccessToBaseURL(*this, baseURL);
     1095    if (basePath.isNull())
     1096        return;
     1097#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
     1098    if (SandboxExtension::createHandleForReadByPid(basePath, process.processIdentifier(), sandboxExtensionHandle))
     1099        process.assumeReadAccessToBaseURL(*this, baseURL);
     1100#else
     1101    if (SandboxExtension::createHandle(basePath, SandboxExtension::Type::ReadOnly, sandboxExtensionHandle))
     1102        process.assumeReadAccessToBaseURL(*this, baseURL);
     1103#endif
    10951104}
    10961105
    … …  
    11501159    addPlatformLoadParameters(loadParameters);
    11511160
     1161#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
     1162    if (processIdentifier() || !url.isLocalFile())
     1163        process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
     1164    else {
     1165        String sandboxExtensionPath;
     1166        if (!m_pageLoadState.resourceDirectoryURL().isEmpty()) {
     1167            sandboxExtensionPath = m_pageLoadState.resourceDirectoryURL().fileSystemPath();
     1168            process->assumeReadAccessToBaseURL(*this, m_pageLoadState.resourceDirectoryURL());
     1169        } else {
     1170            sandboxExtensionPath = "/";
     1171            willAcquireUniversalFileReadSandboxExtension(process);
     1172        }
     1173        process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, sandboxExtensionPath), m_pageID);
     1174    }
     1175#else
    11521176    process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
     1177#endif
    11531178    process->responsivenessTimer().start();
    11541179}
    … …  
    11971222    loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
    11981223#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
    1199     if (!SandboxExtension::createHandleForReadByPid(resourceDirectoryPath, processIdentifier(), loadParameters.sandboxExtensionHandle))
    1200 #endif
     1224    SandboxExtension::createHandleForReadByPid(resourceDirectoryPath, processIdentifier(), loadParameters.sandboxExtensionHandle);
     1225#else
    12011226    SandboxExtension::createHandle(resourceDirectoryPath, SandboxExtension::Type::ReadOnly, loadParameters.sandboxExtensionHandle);
     1227#endif
    12021228    addPlatformLoadParameters(loadParameters);
    12031229
    12041230    m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
     1231#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
     1232    if (processIdentifier())
     1233        m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
     1234    else
     1235        m_process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, resourceDirectoryPath), m_pageID);
     1236#else
    12051237    m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
     1238#endif
    12061239    m_process->responsivenessTimer().start();
    12071240

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r249012 r249700  
    15541554}
    15551555
     1556// LoadRequestWaitingForPID should never be sent to the WebProcess. It must always be converted to a LoadRequest message.
     1557NO_RETURN void WebPage::loadRequestWaitingForPID(LoadParameters&&, const String&)
     1558{
     1559    RELEASE_ASSERT_NOT_REACHED();
     1560}
     1561
    15561562void WebPage::loadDataImpl(uint64_t navigationID, bool shouldTreatAsContinuingLoad, Optional<WebsitePoliciesData>&& websitePolicies, Ref<SharedBuffer>&& sharedBuffer, const String& MIMEType, const String& encodingName, const URL& baseURL, const URL& unreachableURL, const UserData& userData, ShouldOpenExternalURLsPolicy shouldOpenExternalURLsPolicy)
    15571563{

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.h

    r249012 r249700  
    13101310    void platformDidReceiveLoadParameters(const LoadParameters&);
    13111311    void loadRequest(LoadParameters&&);
     1312    void loadRequestWaitingForPID(LoadParameters&&, const String&);
    13121313    void loadData(LoadParameters&&);
    13131314    void loadAlternateHTML(LoadParameters&&);

  • branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.messages.in

    r247982 r249700  
    166166    LoadDataInFrame(IPC::DataReference data, String MIMEType, String encodingName, URL baseURL, uint64_t frameID)
    167167    LoadRequest(struct WebKit::LoadParameters loadParameters)
     168    LoadRequestWaitingForPID(struct WebKit::LoadParameters loadParameters, String sandboxExtensionPath)
    168169    LoadData(struct WebKit::LoadParameters loadParameters)
    169170    LoadAlternateHTML(struct WebKit::LoadParameters loadParameters)
Note: See TracChangeset for help on using the changeset viewer.