From 3afcf306cb42009dfb505c89d60ed44937f3e877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caol=C3=A1n=20McNamara?= Date: Fri, 12 Jul 2024 14:49:20 +0100 Subject: [PATCH] use buildLocalPathToJail instead of direct JAILED_DOCUMENT_ROOT concat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e.g. download as pdf/save as pdf move buildLocalPathToJail to FileUtil instead of JailUtil given that the JailUtil code isn't built on mobile Signed-off-by: Caolán McNamara Change-Id: I8d727e9cdffc413c027bfb9dc6b0520a7d591b47 --- common/FileUtil.cpp | 37 +++++++++++++++++++++++++++++++++ common/FileUtil.hpp | 10 +++++++++ common/JailUtil.cpp | 37 --------------------------------- common/JailUtil.hpp | 10 --------- wsd/ClientRequestDispatcher.cpp | 4 ++-- wsd/ClientSession.cpp | 3 ++- wsd/DocumentBroker.cpp | 4 +++- wsd/Storage.cpp | 3 +-- 8 files changed, 55 insertions(+), 53 deletions(-) diff --git a/common/FileUtil.cpp b/common/FileUtil.cpp index f8f49fa10..9242a93ed 100644 --- a/common/FileUtil.cpp +++ b/common/FileUtil.cpp @@ -390,6 +390,43 @@ namespace FileUtil return (readFile(path, *data, maxSize) >= 0) ? std::move(data) : nullptr; } + std::string buildLocalPathToJail(bool usingMountNamespaces, std::string localStorePath, std::string localPath) + { + // Use where mountJail of kit/Kit.cpp mounts /tmp for this path *from* rather than + // where it is mounted *to*, so this process doesn't need the mount visible to it + if (usingMountNamespaces && !localPath.empty()) + { + Poco::Path jailPath(localPath); + const std::string jailPathDir = jailPath[0]; + if (jailPathDir == "tmp") + { + jailPath.popFrontDirectory(); + + Poco::Path localStorageDir(localStorePath); + localStorageDir.makeDirectory(); + const std::string jailId = localStorageDir[localStorageDir.depth() - 1]; + localStorageDir.popDirectory(); + + localStorageDir.pushDirectory(jailPathDir); + + std::string tmpMapping("cool-"); + tmpMapping.append(jailId); + + localStorageDir.pushDirectory(tmpMapping); + + localStorePath = localStorageDir.toString(); + + localPath = jailPath.toString(); + } + } + + // /chroot/jailId/user/doc/childId + const Poco::Path rootPath = Poco::Path(localStorePath, localPath); + Poco::File(rootPath).createDirectories(); + + return rootPath.toString(); + } + } // namespace FileUtil namespace diff --git a/common/FileUtil.hpp b/common/FileUtil.hpp index 1181b6119..634cbdd65 100644 --- a/common/FileUtil.hpp +++ b/common/FileUtil.hpp @@ -36,6 +36,16 @@ namespace FileUtil /// Create a secure, random directory path. std::string createRandomDir(const std::string& path); + /// return the local path to the jailPath under localJailRoot + /// localJailRoot /chroot/jailId + /// jailPath /tmp/user/doc/childId + /// with usingMountNamespaces false then simply return: + /// -> /chroot/jailId/tmp/user/doc/childId + /// otherwise replaces jailPath's in /tmp with the tmp dir that is mounted + /// from, e.g. return: + /// -> /chroot/tmp/cool-jailId/tmp/user/doc/childId + std::string buildLocalPathToJail(bool usingMountNamespaces, std::string localJailRoot, std::string jailPath); + // We work around some of the mess of using the same sources both on the server side and in unit // tests with conditional compilation based on BUILDING_TESTS. diff --git a/common/JailUtil.cpp b/common/JailUtil.cpp index 2389090a2..fbe80a0a6 100644 --- a/common/JailUtil.cpp +++ b/common/JailUtil.cpp @@ -114,43 +114,6 @@ bool enterUserNS(uid_t uid, gid_t gid) #endif } -std::string buildLocalPathToJail(bool usingMountNamespaces, std::string localStorePath, std::string localPath) -{ - // Use where mountJail of kit/Kit.cpp mounts /tmp for this path *from* rather than - // where it is mounted *to*, so this process doesn't need the mount visible to it - if (usingMountNamespaces && !localPath.empty()) - { - Poco::Path jailPath(localPath); - const std::string jailPathDir = jailPath[0]; - if (jailPathDir == "tmp") - { - jailPath.popFrontDirectory(); - - Poco::Path localStorageDir(localStorePath); - localStorageDir.makeDirectory(); - const std::string jailId = localStorageDir[localStorageDir.depth() - 1]; - localStorageDir.popDirectory(); - - localStorageDir.pushDirectory(jailPathDir); - - std::string tmpMapping("cool-"); - tmpMapping.append(jailId); - - localStorageDir.pushDirectory(tmpMapping); - - localStorePath = localStorageDir.toString(); - - localPath = jailPath.toString(); - } - } - - // /chroot/jailId/user/doc/childId - const Poco::Path rootPath = Poco::Path(localStorePath, localPath); - Poco::File(rootPath).createDirectories(); - - return rootPath.toString(); -} - bool coolmount(const std::string& arg, std::string source, std::string target) { source = Util::trim(source, '/'); diff --git a/common/JailUtil.hpp b/common/JailUtil.hpp index 4ba4ee2d0..de4df97d0 100644 --- a/common/JailUtil.hpp +++ b/common/JailUtil.hpp @@ -46,16 +46,6 @@ bool enterMountingNS(uid_t uid, gid_t gid); /// map root to uid/gid within that namespace. bool enterUserNS(uid_t uid, gid_t gid); -/// return the local path to the jailPath under localJailRoot -/// localJailRoot /chroot/jailId -/// jailPath /tmp/user/doc/childId -/// with usingMountNamespaces false then simply return: -/// -> /chroot/jailId/tmp/user/doc/childId -/// otherwise replaces jailPath's in /tmp with the tmp dir that is mounted -/// from, e.g. return: -/// -> /chroot/tmp/cool-jailId/tmp/user/doc/childId -std::string buildLocalPathToJail(bool usingMountNamespaces, std::string localJailRoot, std::string jailPath); - /// Bind mount a jail directory. bool bind(const std::string& source, const std::string& target); diff --git a/wsd/ClientRequestDispatcher.cpp b/wsd/ClientRequestDispatcher.cpp index 8e94bb6ed..d014e4f5f 100644 --- a/wsd/ClientRequestDispatcher.cpp +++ b/wsd/ClientRequestDispatcher.cpp @@ -1521,7 +1521,7 @@ void ClientRequestDispatcher::handlePostRequest(const RequestDetails& requestDet formName.find('/') == std::string::npos) { const std::string dirPath = - JailUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, COOLWSD::ChildRoot + formChildid, + FileUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, COOLWSD::ChildRoot + formChildid, JAILED_DOCUMENT_ROOT + std::string("insertfile")); const std::string fileName = dirPath + '/' + form.get("name"); LOG_INF("Perform insertfile: " << formChildid << ", " << formName @@ -1573,7 +1573,7 @@ void ClientRequestDispatcher::handlePostRequest(const RequestDetails& requestDet std::string decoded; Poco::URI::decode(url, decoded); - const Poco::Path filePath(JailUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, COOLWSD::ChildRoot + jailId, + const Poco::Path filePath(FileUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, COOLWSD::ChildRoot + jailId, JAILED_DOCUMENT_ROOT + decoded)); const std::string filePathAnonym = COOLWSD::anonymizeUrl(filePath.toString()); diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp index 2d1452fd3..4f2766022 100644 --- a/wsd/ClientSession.cpp +++ b/wsd/ClientSession.cpp @@ -1894,7 +1894,8 @@ bool ClientSession::handleKitToClientMessage(const std::shared_ptr& pay relative = relative.substr(1); // Rewrite file:// URLs to be visible to the outside world. - const Path path(docBroker->getJailRoot(), relative); + const Path path(FileUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, + docBroker->getJailRoot(), relative)); if (Poco::File(path).exists()) { if (!isConvertTo) diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 601e61c8c..12076dc04 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -3343,7 +3343,9 @@ bool DocumentBroker::handleInput(const std::shared_ptr& message) std::string decoded; Poco::URI::decode(url, decoded); - const std::string filePath(COOLWSD::ChildRoot + getJailId() + JAILED_DOCUMENT_ROOT + decoded); + const std::string filePath(FileUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, + COOLWSD::ChildRoot + getJailId(), + JAILED_DOCUMENT_ROOT + decoded)); std::ifstream ifs(filePath); const std::string svg((std::istreambuf_iterator(ifs)), diff --git a/wsd/Storage.cpp b/wsd/Storage.cpp index 832e329e6..fa95573b3 100644 --- a/wsd/Storage.cpp +++ b/wsd/Storage.cpp @@ -47,7 +47,6 @@ #include #include #include -#include #include #include #include @@ -82,7 +81,7 @@ std::string StorageBase::getLocalRootPath() const localPath.erase(0, 1); } - return JailUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, _localStorePath, std::move(localPath)); + return FileUtil::buildLocalPathToJail(COOLWSD::EnableMountNamespaces, _localStorePath, std::move(localPath)); } #endif