- Revision
- 249700
- Author
- alanc...@apple.com
- Date
- 2019-09-09 20:20:08 -0700 (Mon, 09 Sep 2019)
Log 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
Modified Paths
Diff
Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/ChangeLog 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog 2019-09-10 03:20:08 UTC (rev 249700)
@@ -1,5 +1,73 @@
2019-09-09 Kocsen Chung <kocsen_ch...@apple.com>
+ 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
+
+ 2019-09-09 Per Arne Vollan <pvol...@apple.com>
+
+ [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 Kocsen Chung <kocsen_ch...@apple.com>
+
Cherry-pick r249624. rdar://problem/55202935
Marking up a note on iOS results in a PDF with no contents
Modified: branches/safari-608-branch/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp 2019-09-10 03:20:08 UTC (rev 249700)
@@ -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: branches/safari-608-branch/Source/WebKit/UIProcess/WebPageProxy.cpp (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-09-10 03:20:08 UTC (rev 249700)
@@ -1055,14 +1055,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))
@@ -1076,11 +1077,12 @@
willAcquireUniversalFileReadSandboxExtension(process);
return;
}
-#endif
+#else
if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
willAcquireUniversalFileReadSandboxExtension(process);
return;
}
+#endif
#if PLATFORM(COCOA)
if (!linkedOnOrAfter(SDKVersion::FirstWithoutUnconditionalUniversalSandboxExtension))
@@ -1090,8 +1092,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)
@@ -1149,7 +1158,23 @@
addPlatformLoadParameters(loadParameters);
+#if HAVE(SANDBOX_ISSUE_READ_EXTENSION_TO_PROCESS_BY_PID)
+ if (processIdentifier() || !url.isLocalFile())
+ process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
+ 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), m_pageID);
+ }
+#else
process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
+#endif
process->responsivenessTimer().start();
}
@@ -1196,13 +1221,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_pageID);
+ else
+ m_process->send(Messages::WebPage::LoadRequestWaitingForPID(loadParameters, resourceDirectoryPath), m_pageID);
+#else
m_process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
+#endif
m_process->responsivenessTimer().start();
return navigation;
Modified: branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2019-09-10 03:20:08 UTC (rev 249700)
@@ -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: branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.h (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.h 2019-09-10 03:20:08 UTC (rev 249700)
@@ -1309,6 +1309,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: branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (249699 => 249700)
--- branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2019-09-10 03:20:05 UTC (rev 249699)
+++ branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2019-09-10 03:20:08 UTC (rev 249700)
@@ -165,6 +165,7 @@
LoadURLInFrame(URL url, String referrer, uint64_t frameID)
LoadDataInFrame(IPC::DataReference data, String MIMEType, String encodingName, URL baseURL, uint64_t frameID)
LoadRequest(struct WebKit::LoadParameters loadParameters)
+ LoadRequestWaitingForPID(struct WebKit::LoadParameters loadParameters, String sandboxExtensionPath)
LoadData(struct WebKit::LoadParameters loadParameters)
LoadAlternateHTML(struct WebKit::LoadParameters loadParameters)
