WebPageProxy should use the right path for sandbox extension
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jun 2019 20:36:57 +0000 (20:36 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jun 2019 20:36:57 +0000 (20:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198902
<rdar://problem/50772810>

Reviewed by Geoffrey Garen.

Source/WebKit:

Store the sandbox path, if any, in UIProcess for each page in its PageLoadState.
This allows proper sandbox handling for reload after crash and process swap cases.
Store the sandbox path, if any, in the b/w list so that the sandbox path can be properly computed
during b/f navigation works.

Add SPI for test purposes to check what is the current sandbox path.

* Shared/WebBackForwardListItem.h:
(WebKit::WebBackForwardListItem::resourceDirectoryURL const):
(WebKit::WebBackForwardListItem::setResourceDirectoryURL):
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _resourceDirectoryURL]):
* UIProcess/API/Cocoa/WKWebViewPrivate.h:
* UIProcess/PageLoadState.cpp:
(WebKit::PageLoadState::resourceDirectoryURL const):
(WebKit::PageLoadState::setPendingAPIRequestURL):
* UIProcess/PageLoadState.h:
(WebKit::PageLoadState::setPendingAPIRequestURL):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::reload):
(WebKit::WebPageProxy::backForwardAddItem):
(WebKit::WebPageProxy::backForwardGoToItemShared):
(WebKit::WebPageProxy::currentResourceDirectoryURL const):
* UIProcess/WebPageProxy.h:

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm:
(TEST):
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

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

12 files changed:
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebBackForwardListItem.h
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
Source/WebKit/UIProcess/PageLoadState.cpp
Source/WebKit/UIProcess/PageLoadState.h
Source/WebKit/UIProcess/WebBackForwardList.cpp
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

index eea4bd2..7cb8947 100644 (file)
@@ -1,3 +1,39 @@
+2019-06-21  Youenn Fablet  <youenn@apple.com>
+
+        WebPageProxy should use the right path for sandbox extension
+        https://bugs.webkit.org/show_bug.cgi?id=198902
+        <rdar://problem/50772810>
+
+        Reviewed by Geoffrey Garen.
+
+        Store the sandbox path, if any, in UIProcess for each page in its PageLoadState.
+        This allows proper sandbox handling for reload after crash and process swap cases.
+        Store the sandbox path, if any, in the b/w list so that the sandbox path can be properly computed
+        during b/f navigation works.
+
+        Add SPI for test purposes to check what is the current sandbox path.
+
+        * Shared/WebBackForwardListItem.h:
+        (WebKit::WebBackForwardListItem::resourceDirectoryURL const):
+        (WebKit::WebBackForwardListItem::setResourceDirectoryURL):
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _resourceDirectoryURL]):
+        * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+        * UIProcess/PageLoadState.cpp:
+        (WebKit::PageLoadState::resourceDirectoryURL const):
+        (WebKit::PageLoadState::setPendingAPIRequestURL):
+        * UIProcess/PageLoadState.h:
+        (WebKit::PageLoadState::setPendingAPIRequestURL):
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::maybeInitializeSandboxExtensionHandle):
+        (WebKit::WebPageProxy::loadRequestWithNavigationShared):
+        (WebKit::WebPageProxy::loadFile):
+        (WebKit::WebPageProxy::reload):
+        (WebKit::WebPageProxy::backForwardAddItem):
+        (WebKit::WebPageProxy::backForwardGoToItemShared):
+        (WebKit::WebPageProxy::currentResourceDirectoryURL const):
+        * UIProcess/WebPageProxy.h:
+
 2019-06-21  Antoine Quint  <graouts@apple.com>
 
         [iOS] Compatibility mouse events aren't prevented by calling preventDefault() on pointerdown
index 5b2d2e0..3b9b0bf 100644 (file)
@@ -67,6 +67,9 @@ public:
     const String& url() const { return m_itemState.pageState.mainFrameState.urlString; }
     const String& title() const { return m_itemState.pageState.title; }
 
+    const URL& resourceDirectoryURL() const { return m_resourceDirectoryURL; }
+    void setResourceDirectoryURL(URL&& url) { m_resourceDirectoryURL = WTFMove(url); }
+
     bool itemIsInSameDocument(const WebBackForwardListItem&) const;
     bool itemIsClone(const WebBackForwardListItem&);
 
@@ -87,6 +90,7 @@ private:
     void removeSuspendedPageFromProcessPool();
 
     BackForwardListItemState m_itemState;
+    URL m_resourceDirectoryURL;
     WebCore::PageIdentifier m_pageID;
     WebCore::ProcessIdentifier m_lastProcessIdentifier;
     WeakPtr<SuspendedPageProxy> m_suspendedPage;
index d11b57a..bbeaef4 100644 (file)
@@ -982,6 +982,11 @@ static void validate(WKWebViewConfiguration *configuration)
     return [NSURL _web_URLWithWTFString:_page->pageLoadState().activeURL()];
 }
 
+- (NSURL *)_resourceDirectoryURL
+{
+    return _page->currentResourceDirectoryURL();
+}
+
 - (BOOL)isLoading
 {
     return _page->pageLoadState().isLoading();
index c675bff..723a34d 100644 (file)
@@ -176,6 +176,8 @@ typedef NS_OPTIONS(NSUInteger, _WKRectEdge) {
 
 @property (nonatomic, readonly, getter=_isShowingNavigationGestureSnapshot) BOOL _showingNavigationGestureSnapshot;
 
+@property (nonatomic, readonly) NSURL *_resourceDirectoryURL WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+
 - (void)_close;
 
 - (void)_updateWebsitePolicies:(_WKWebsitePolicies *)websitePolicies WK_API_AVAILABLE(macos(10.13), ios(11.0));
index 10185a4..0cf6a0d 100644 (file)
@@ -239,10 +239,16 @@ const String& PageLoadState::pendingAPIRequestURL() const
     return m_committedState.pendingAPIRequestURL;
 }
 
-void PageLoadState::setPendingAPIRequestURL(const Transaction::Token& token, const String& pendingAPIRequestURL)
+const URL& PageLoadState::resourceDirectoryURL() const
+{
+    return m_committedState.resourceDirectoryURL;
+}
+
+void PageLoadState::setPendingAPIRequestURL(const Transaction::Token& token, const String& pendingAPIRequestURL, const URL& resourceDirectoryURL)
 {
     ASSERT_UNUSED(token, &token.m_pageLoadState == this);
     m_uncommittedState.pendingAPIRequestURL = pendingAPIRequestURL;
+    m_uncommittedState.resourceDirectoryURL = resourceDirectoryURL;
 }
 
 void PageLoadState::clearPendingAPIRequestURL(const Transaction::Token& token)
index dbeaa9a..2076918 100644 (file)
@@ -27,6 +27,7 @@
 #define PageLoadState_h
 
 #include "WebCertificateInfo.h"
+#include <wtf/URL.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebKit {
@@ -140,8 +141,10 @@ public:
 
     WebCertificateInfo* certificateInfo() const { return m_committedState.certificateInfo.get(); }
 
+    const URL& resourceDirectoryURL() const;
+
     const String& pendingAPIRequestURL() const;
-    void setPendingAPIRequestURL(const Transaction::Token&, const String&);
+    void setPendingAPIRequestURL(const Transaction::Token&, const String& pendingAPIRequestURL, const URL& resourceDirectoryPath = { });
     void clearPendingAPIRequestURL(const Transaction::Token&);
 
     void didStartProvisionalLoad(const Transaction::Token&, const String& url, const String& unreachableURL);
@@ -213,6 +216,8 @@ private:
 
         String title;
 
+        URL resourceDirectoryURL;
+
         bool canGoBack;
         bool canGoForward;
 
index a652618..8275cf6 100644 (file)
@@ -434,6 +434,7 @@ void WebBackForwardList::restoreFromState(BackForwardListState backForwardListSt
     Vector<Ref<WebBackForwardListItem>> items;
     items.reserveInitialCapacity(backForwardListState.items.size());
 
+    // FIXME: Enable restoring resourceDirectoryURL.
     for (auto& backForwardListItemState : backForwardListState.items) {
         backForwardListItemState.identifier = { Process::identifier(), ObjectIdentifier<BackForwardItemIdentifier::ItemIdentifierType>::generate() };
         items.uncheckedAppend(WebBackForwardListItem::create(WTFMove(backForwardListItemState), m_page->pageID()));
index 1302a84..deb2ada 100644 (file)
@@ -1048,19 +1048,29 @@ bool WebPageProxy::tryClose()
     return false;
 }
 
-bool WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, SandboxExtension::Handle& sandboxExtensionHandle)
+void WebPageProxy::maybeInitializeSandboxExtensionHandle(WebProcessProxy& process, const URL& url, const URL& resourceDirectoryURL, SandboxExtension::Handle& sandboxExtensionHandle)
 {
     if (!url.isLocalFile())
-        return false;
+        return;
+
+    if (!resourceDirectoryURL.isEmpty()) {
+        if (process.hasAssumedReadAccessToURL(resourceDirectoryURL))
+            return;
+
+        if (SandboxExtension::createHandle(resourceDirectoryURL.fileSystemPath(), SandboxExtension::Type::ReadOnly, sandboxExtensionHandle)) {
+            m_process->assumeReadAccessToBaseURL(*this, resourceDirectoryURL);
+            return;
+        }
+    }
 
     if (process.hasAssumedReadAccessToURL(url))
-        return false;
+        return;
 
     // Inspector resources are in a directory with assumed access.
     ASSERT_WITH_SECURITY_IMPLICATION(!WebKit::isInspectorPage(*this));
 
-    SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle);
-    return true;
+    if (SandboxExtension::createHandle("/", SandboxExtension::Type::ReadOnly, sandboxExtensionHandle))
+        willAcquireUniversalFileReadSandboxExtension(process);
 }
 
 #if !PLATFORM(COCOA)
@@ -1114,9 +1124,8 @@ void WebPageProxy::loadRequestWithNavigationShared(Ref<WebProcessProxy>&& proces
     loadParameters.lockHistory = navigation.lockHistory();
     loadParameters.lockBackForwardList = navigation.lockBackForwardList();
     loadParameters.clientRedirectSourceForHistory = navigation.clientRedirectSourceForHistory();
-    bool createdExtension = maybeInitializeSandboxExtensionHandle(process, url, loadParameters.sandboxExtensionHandle);
-    if (createdExtension)
-        willAcquireUniversalFileReadSandboxExtension(process);
+    maybeInitializeSandboxExtensionHandle(process, url, m_pageLoadState.resourceDirectoryURL(), loadParameters.sandboxExtensionHandle);
+
     addPlatformLoadParameters(loadParameters);
 
     process->send(Messages::WebPage::LoadRequest(loadParameters), m_pageID);
@@ -1156,7 +1165,7 @@ RefPtr<API::Navigation> WebPageProxy::loadFile(const String& fileURLString, cons
 
     auto transaction = m_pageLoadState.transaction();
 
-    m_pageLoadState.setPendingAPIRequestURL(transaction, fileURLString);
+    m_pageLoadState.setPendingAPIRequestURL(transaction, fileURLString, resourceDirectoryURL);
 
     String resourceDirectoryPath = resourceDirectoryURL.fileSystemPath();
 
@@ -1336,9 +1345,7 @@ RefPtr<API::Navigation> WebPageProxy::reload(OptionSet<WebCore::ReloadOption> op
         m_pageLoadState.setPendingAPIRequestURL(transaction, url);
 
         // We may not have an extension yet if back/forward list was reinstated after a WebProcess crash or a browser relaunch
-        bool createdExtension = maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), sandboxExtensionHandle);
-        if (createdExtension)
-            willAcquireUniversalFileReadSandboxExtension(m_process);
+        maybeInitializeSandboxExtensionHandle(m_process, URL(URL(), url), currentResourceDirectoryURL(), sandboxExtensionHandle);
     }
 
     if (!hasRunningProcess())
@@ -5714,6 +5721,7 @@ void WebPageProxy::requestDOMPasteAccess(const WebCore::IntRect& elementRect, co
 void WebPageProxy::backForwardAddItem(BackForwardListItemState&& itemState)
 {
     auto item = WebBackForwardListItem::create(WTFMove(itemState), pageID());
+    item->setResourceDirectoryURL(currentResourceDirectoryURL());
     m_backForwardList->addItem(WTFMove(item));
 }
 
@@ -5736,9 +5744,7 @@ void WebPageProxy::backForwardGoToItemShared(Ref<WebProcessProxy>&& process, con
         return completionHandler({ });
 
     SandboxExtension::Handle sandboxExtensionHandle;
-    bool createdExtension = maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), sandboxExtensionHandle);
-    if (createdExtension)
-        willAcquireUniversalFileReadSandboxExtension(process);
+    maybeInitializeSandboxExtensionHandle(process, URL(URL(), item->url()), item->resourceDirectoryURL(),  sandboxExtensionHandle);
     m_backForwardList->goToItem(*item);
     completionHandler(WTFMove(sandboxExtensionHandle));
 }
@@ -6789,6 +6795,16 @@ String WebPageProxy::currentURL() const
     return url;
 }
 
+URL WebPageProxy::currentResourceDirectoryURL() const
+{
+    auto resourceDirectoryURL = m_pageLoadState.resourceDirectoryURL();
+    if (!resourceDirectoryURL.isEmpty())
+        return resourceDirectoryURL;
+    if (auto* item = m_backForwardList->currentItem())
+        return item->resourceDirectoryURL();
+    return { };
+}
+
 void WebPageProxy::processDidTerminate(ProcessTerminationReason reason)
 {
     if (reason != ProcessTerminationReason::NavigationSwap)
index aaa58d1..457fadf 100644 (file)
@@ -1556,6 +1556,8 @@ public:
 
     void requestStorageSpace(uint64_t frameID, const String& originIdentifier, const String& databaseName, const String& displayName, uint64_t currentQuota, uint64_t currentOriginUsage, uint64_t currentDatabaseUsage, uint64_t expectedUsage, WTF::CompletionHandler<void(uint64_t)>&&);
 
+    URL currentResourceDirectoryURL() const;
+
 private:
     WebPageProxy(PageClient&, WebProcessProxy&, WebCore::PageIdentifier, Ref<API::PageConfiguration>&&);
     void platformInitialize();
@@ -1892,7 +1894,7 @@ private:
     void setPluginComplexTextInputState(uint64_t pluginComplexTextInputIdentifier, uint64_t complexTextInputState);
 #endif
 
-    bool maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL&, SandboxExtension::Handle&);
+    void maybeInitializeSandboxExtensionHandle(WebProcessProxy&, const URL& url, const URL& resourceDirectoryURL, SandboxExtension::Handle&);
 
 #if USE(AUTOMATIC_TEXT_REPLACEMENT)
     void toggleSmartInsertDelete();
index 8448377..8f9237c 100644 (file)
@@ -1,3 +1,15 @@
+2019-06-21  Youenn Fablet  <youenn@apple.com>
+
+        WebPageProxy should use the right path for sandbox extension
+        https://bugs.webkit.org/show_bug.cgi?id=198902
+        <rdar://problem/50772810>
+
+        Reviewed by Geoffrey Garen.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/LoadFileURL.mm:
+        (TEST):
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2019-06-21  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [WPE][GTK] Bump minimum versions of GLib, GTK, libsoup, ATK, GStreamer, and Cairo
index 24f229b..dd1fdc3 100644 (file)
@@ -29,6 +29,7 @@
 #import "Test.h"
 #import "TestNavigationDelegate.h"
 #import <WebKit/WebKit.h>
+#import <WebKit/WebViewPrivate.h>
 #import <wtf/RetainPtr.h>
 
 
@@ -42,6 +43,7 @@ TEST(WKWebView, LoadTwoFiles)
     NSURL *file = [[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
     [webView loadFileURL:file allowingReadAccessToURL:file.URLByDeletingLastPathComponent];
     [delegate waitForDidFinishNavigation];
+    EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, file.URLByDeletingLastPathComponent.path);
 
     // Load a second file: resource that is in a completely different directory from the above
     file = [[NSBundle mainBundle] URLForResource:@"simple2" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
@@ -49,8 +51,18 @@ TEST(WKWebView, LoadTwoFiles)
     [[NSFileManager defaultManager] createDirectoryAtURL:targetURL.URLByDeletingLastPathComponent withIntermediateDirectories:YES attributes:nil error:nil];
     [[NSFileManager defaultManager] removeItemAtURL:targetURL error:nil];
     [[NSFileManager defaultManager] copyItemAtURL:file toURL:targetURL error:nil];
-    
+
     // If this second load succeeds (e.g. doesn't timeout due to a sandbox violation) the test passes
     [webView loadFileURL:targetURL allowingReadAccessToURL:targetURL.URLByDeletingLastPathComponent];
     [delegate waitForDidFinishNavigation];
+    EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, targetURL.URLByDeletingLastPathComponent.path);
+
+    [webView _killWebContentProcessAndResetState];
+    [webView reload];
+    [delegate waitForDidFinishNavigation];
+    EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, targetURL.URLByDeletingLastPathComponent.path);
+
+    [webView goBack];
+    [delegate waitForDidFinishNavigation];
+    EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, file.URLByDeletingLastPathComponent.path);
 }
index 6217c98..b532a6d 100644 (file)
@@ -6295,3 +6295,33 @@ TEST(ProcessSwap, SuspendAllMediaPlayback)
     [webView synchronouslyLoadTestPageNamed:@"video-with-audio"];
     TestWebKitAPI::Util::run(&notPlaying);
 }
+
+TEST(ProcessSwap, PassSandboxExtension)
+{
+    auto processPoolConfiguration = psonProcessPoolConfiguration();
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    configuration.get().mediaTypesRequiringUserActionForPlayback = WKAudiovisualMediaTypeNone;
+#if TARGET_OS_IPHONE
+    configuration.get().allowsInlineMediaPlayback = YES;
+#endif
+    [configuration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [configuration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
+
+    auto navigationDelegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+    [webView setNavigationDelegate:navigationDelegate.get()];
+
+    [webView loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]]];
+    TestWebKitAPI::Util::run(&done);
+    done = false;
+
+    NSURL *file = [[NSBundle mainBundle] URLForResource:@"autoplay-with-controls" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"];
+    [webView loadFileURL:file allowingReadAccessToURL:file.URLByDeletingLastPathComponent];
+    [webView waitForMessage:@"loaded"];
+
+    EXPECT_WK_STREQ(webView.get()._resourceDirectoryURL.path, file.URLByDeletingLastPathComponent.path);
+}