- Revision
- 249649
- Author
- [email protected]
- Date
- 2019-09-09 10:44:33 -0700 (Mon, 09 Sep 2019)
Log Message
[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:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (249648 => 249649)
--- trunk/Source/WebKit/ChangeLog 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/ChangeLog 2019-09-09 17:44:33 UTC (rev 249649)
@@ -1,3 +1,35 @@
+2019-09-09 Per Arne Vollan <[email protected]>
+
+ [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:
+
2019-09-09 Youenn Fablet <[email protected]>
Remove ServiceWorkerProcessProxy
Modified: trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (249648 => 249649)
--- trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-09 17:44:33 UTC (rev 249649)
@@ -27,6 +27,8 @@
#include "AuxiliaryProcessProxy.h"
#include "AuxiliaryProcessMessages.h"
+#include "LoadParameters.h"
+#include "WebPageMessages.h"
#include <wtf/RunLoop.h>
namespace WebKit {
@@ -175,6 +177,20 @@
for (size_t i = 0; i < m_pendingMessages.size(); ++i) {
std::unique_ptr<IPC::Encoder> message = WTFMove(m_pendingMessages[i].first);
OptionSet<IPC::SendOption> sendOptions = m_pendingMessages[i].second;
+#if HAVE(SANDBOX_ISSUE_MACH_EXTENSION_TO_PROCESS_BY_PID)
+ if (message->messageName() == "LoadRequestWaitingForPID") {
+ auto buffer = message->buffer();
+ auto bufferSize = message->bufferSize();
+ std::unique_ptr<IPC::Decoder> decoder = makeUnique<IPC::Decoder>(buffer, bufferSize, nullptr, Vector<IPC::Attachment> { });
+ LoadParameters loadParameters;
+ String sandboxExtensionPath;
+ if (decoder->decode(loadParameters) && decoder->decode(sandboxExtensionPath)) {
+ SandboxExtension::createHandleForReadByPid(sandboxExtensionPath, processIdentifier(), loadParameters.sandboxExtensionHandle);
+ send(Messages::WebPage::LoadRequest(loadParameters), decoder->destinationID());
+ continue;
+ }
+ }
+#endif
m_connection->sendMessage(WTFMove(message), sendOptions);
}
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (249648 => 249649)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-09-09 17:44:33 UTC (rev 249649)
@@ -1071,14 +1071,15 @@
#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
if (SandboxExtension::createHandleForReadByPid(resourceDirectoryURL.fileSystemPath(), process.processIdentifier(), sandboxExtensionHandle)) {
- m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
+ process.assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
return;
}
-#endif
+#else
if (SandboxExtension::createHandle(resourceDirectoryURL.fileSystemPath(), SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
- m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
+ process.assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
return;
}
+#endif
}
if (process.hasAssumedReadAccessToURL(url))
@@ -1092,11 +1093,12 @@
willAcquireUniversalFileReadSandboxExtension(process);
return;
}
-#endif
+#else
if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
willAcquireUniversalFileReadSandboxExtension(process);
return;
}
+#endif
#if PLATFORM(COCOA)
if (!linkedOnOrAfter(SDKVersion::FirstWithoutUnconditionalUniversalSandboxExtension))
@@ -1106,8 +1108,15 @@
// We failed to issue an universal file read access sandbox, fall back to issuing one for the base URL instead.
auto baseURL = URL(URL(), url.baseAsString());
auto basePath = baseURL.fileSystemPath();
- if (!basePath.isNull() && SandboxExtension::createHandle(basePath, SandboxExtension::Type::ReadOnly, sandboxExtensionHandle))
- m_process->assumeReadAccessToBaseURL(*this, baseURL);
+ if (basePath.isNull())
+ return;
+#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
+ if (SandboxExtension::createHandleForReadByPid(basePath, process.processIdentifier(), sandboxExtensionHandle))
+ process.assumeReadAccessToBaseURL(*this, baseURL);
+#else
+ if (SandboxExtension::createHandle(basePath, SandboxExtension::Type::ReadOnly, sandboxExtensionHandle))
+ process.assumeReadAccessToBaseURL(*this, baseURL);
+#endif
}
#if !PLATFORM(COCOA)
@@ -1165,7 +1174,23 @@
addPlatformLoadParameters(loadParameters);
+#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
+ if (processIdentifier() || !url.isLocalFile())
+ process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
+ else {
+ String sandboxExtensionPath;
+ if (!m_pageLoadState.resourceDirectoryURL().isEmpty()) {
+ sandboxExtensionPath = m_pageLoadState.resourceDirectoryURL().fileSystemPath();
+ process->assumeReadAccessToBaseURL(*this, m_pageLoadState.resourceDirectoryURL());
+ } else {
+ sandboxExtensionPath = "/";
+ willAcquireUniversalFileReadSandboxExtension(process);
+ }
+ process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, sandboxExtensionPath), webPageID);
+ }
+#else
process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
+#endif
process->responsivenessTimer().start();
}
@@ -1212,13 +1237,21 @@
loadParameters.shouldOpenExternalURLsPolicy = ShouldOpenExternalURLsPolicy::ShouldNotAllow;
loadParameters.userData = UserData(process().transformObjectsToHandles(userData).get());
#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
- if (!SandboxExtension::createHandleForReadByPid(resourceDirectoryPath, processIdentifier(), loadParameters.sandboxExtensionHandle))
+ SandboxExtension::createHandleForReadByPid(resourceDirectoryPath, processIdentifier(), loadParameters.sandboxExtensionHandle);
+#else
+ SandboxExtension::createHandle(resourceDirectoryPath, SandboxExtension::Type::ReadOnly, loadParameters.sandboxExtensionHandle);
#endif
- SandboxExtension::createHandle(resourceDirectoryPath, SandboxExtension::Type::ReadOnly, loadParameters.sandboxExtensionHandle);
addPlatformLoadParameters(loadParameters);
m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
+#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
+ if (processIdentifier())
+ m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_webPageID);
+ else
+ m_process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, resourceDirectoryPath), m_webPageID);
+#else
m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_webPageID);
+#endif
m_process->responsivenessTimer().start();
return navigation;
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (249648 => 249649)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-09 17:44:33 UTC (rev 249649)
@@ -1553,6 +1553,12 @@
ASSERT(!m_pendingWebsitePolicies);
}
+// LoadRequestWaitingForPID should never be sent to the WebProcess. It must always be converted to a LoadRequest message.
+NO_RETURN void WebPage::loadRequestWaitingForPID(LoadParameters&&, const String&)
+{
+ RELEASE_ASSERT_NOT_REACHED();
+}
+
void 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)
{
SendStopResponsivenessTimer stopper;
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (249648 => 249649)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-09-09 17:44:33 UTC (rev 249649)
@@ -1317,6 +1317,7 @@
void tryClose();
void platformDidReceiveLoadParameters(const LoadParameters&);
void loadRequest(LoadParameters&&);
+ void loadRequestWaitingForPID(LoadParameters&&, const String&);
void loadData(LoadParameters&&);
void loadAlternateHTML(LoadParameters&&);
void navigateToPDFLinkWithSimulatedClick(const String& url, WebCore::IntPoint documentPoint, WebCore::IntPoint screenPoint);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (249648 => 249649)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2019-09-09 17:18:25 UTC (rev 249648)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2019-09-09 17:44:33 UTC (rev 249649)
@@ -165,6 +165,7 @@
LoadURLInFrame(URL url, String referrer, WebCore::FrameIdentifier frameID)
LoadDataInFrame(IPC::DataReference data, String MIMEType, String encodingName, URL baseURL, WebCore::FrameIdentifier frameID)
LoadRequest(struct WebKit::LoadParameters loadParameters)
+ LoadRequestWaitingForPID(struct WebKit::LoadParameters loadParameters, String sandboxExtensionPath)
LoadData(struct WebKit::LoadParameters loadParameters)
LoadAlternateHTML(struct WebKit::LoadParameters loadParameters)
