From f24871adb2f01ef3f7cbd8a53e60939518045587 Mon Sep 17 00:00:00 2001 From: Simo Piiroinen Date: Fri, 28 May 2021 13:20:33 +0300 Subject: [PATCH] Revert "[launcherlib] Add checks for invoker" This reverts commit 88bf4689e4d1838c847fae25b448710e96b35278. --- src/launcherlib/booster.cpp | 8 ---- src/launcherlib/connection.cpp | 84 +--------------------------------- src/launcherlib/connection.h | 13 ------ 3 files changed, 2 insertions(+), 103 deletions(-) diff --git a/src/launcherlib/booster.cpp b/src/launcherlib/booster.cpp index 61c4f6c..f4008e4 100644 --- a/src/launcherlib/booster.cpp +++ b/src/launcherlib/booster.cpp @@ -61,7 +61,6 @@ Booster::Booster() : m_boostedApplication("default"), m_bootMode(false) { - Connection::setMountNamespace(Connection::getMountNamespace(getpid())); } Booster::~Booster() @@ -240,13 +239,6 @@ bool Booster::receiveDataFromInvoker(int socketFd) // Accept a new invocation. if (m_connection->accept(m_appData)) { - // Check that the caller is allowed to invoke apps - if (!m_connection->isPermitted()) - { - m_connection->close(); - return false; - } - // Receive application data from the invoker if (!m_connection->receiveApplicationData(m_appData)) { diff --git a/src/launcherlib/connection.cpp b/src/launcherlib/connection.cpp index a883885..4ca2b5e 100644 --- a/src/launcherlib/connection.cpp +++ b/src/launcherlib/connection.cpp @@ -25,8 +25,6 @@ #include #include /* for getsockopt */ #include /* for chmod */ -#include -#include #include #include #include @@ -34,10 +32,6 @@ #include #include -#define INVOKER_PATH "/usr/bin/invoker" - -std::pair Connection::s_mntNS = std::pair(); - Connection::Connection(int socketFd, bool testMode) : m_testMode(testMode), m_fd(-1), @@ -79,8 +73,9 @@ int Connection::getFd() const return m_fd; } -bool Connection::accept(AppData*) +bool Connection::accept(AppData *appData) { + (void)appData; // unused if (!m_testMode) { m_fd = ::accept(m_curSocket, NULL, NULL); @@ -113,47 +108,6 @@ void Connection::close() } } - -bool Connection::isPermitted() -{ - // Check that caller is in the same namespace and it is invoker - // and not something else. This is done to avoid sandboxed app - // that sees the socket from escaping the sandbox through booster - if (m_fd != -1) - { - pid_t pid = peerPid(); - if (pid == 0) - { - Logger::logError("Connection: %s: getting peer pid failed", __FUNCTION__); - } - else - { - // There is a possibility for a race here: if caller can exit - // and then invoke a process with the correct binary path and - // pid at the right time they could fool us - - if (!s_mntNS.first || getMountNamespace(pid) != s_mntNS) - { - Logger::logWarning("Connection: %s: invocation from %s (%d) came from different namespace", __FUNCTION__, getExecutablePath(pid), pid); - } - else - { - std::string executablePath = getExecutablePath(pid); - if (executablePath != INVOKER_PATH) - { - Logger::logWarning("Connection: %s: executable %s (%d) is not invoker", __FUNCTION__, executablePath, pid); - } - else - { - Logger::logDebug("Connection: %s: invoker (%d) called, allowing", __FUNCTION__, pid); - return true; - } - } - } - } - return false; -} - bool Connection::sendMsg(uint32_t msg) { if (!m_testMode) @@ -594,37 +548,3 @@ pid_t Connection::peerPid() return cr.pid; } - -void Connection::setMountNamespace(std::pair mntNS) -{ - s_mntNS = mntNS; -} - -std::pair Connection::getMountNamespace(pid_t pid) -{ - struct stat sb; - std::ostringstream path; - path << "/proc/" << pid << "/ns/mnt"; - if (stat(path.str().c_str(), &sb) == -1) - { - Logger::logError("Connection: %s: stat failed for pid %d: %s", __FUNCTION__, pid, strerror(errno)); - return std::pair(); - } - return std::pair(sb.st_dev, sb.st_ino); -} - -std::string Connection::getExecutablePath(pid_t pid) -{ - std::ostringstream path; - char exe[PATH_MAX]; - ssize_t len; - static_assert(sizeof(INVOKER_PATH) < sizeof(exe)); - path << "/proc/" << pid << "/exe"; - len = readlink(path.str().c_str(), exe, sizeof(exe)); - if (len < 0 || (size_t)len >= sizeof(exe)) - { - Logger::logError("Connection: %s: readlink failed for pid %d: %s", __FUNCTION__, pid, strerror(errno)); - return std::string(); - } - return std::string(exe, len); -} diff --git a/src/launcherlib/connection.h b/src/launcherlib/connection.h index 2fcb49e..6216094 100644 --- a/src/launcherlib/connection.h +++ b/src/launcherlib/connection.h @@ -84,15 +84,6 @@ public: //! \brief Send application exit value bool sendExitValue(int value); - //! \brief Check if caller is permitted to invoke apps - bool isPermitted(); - - //! \brief Get device id and inode number pair of a mount namespace - static std::pair getMountNamespace(pid_t pid); - - //! \brief Set required mount namespace for invoker processes - static void setMountNamespace(std::pair mntNS); - private: /*! \brief Receive actions. @@ -142,9 +133,6 @@ private: //! Send process pid bool sendPid(pid_t pid); - //! Helper method: Get executable path for pid - static std::string getExecutablePath(pid_t pid); - //! Send message to a socket. This is a virtual to help unit testing. virtual bool sendMsg(uint32_t msg); @@ -173,7 +161,6 @@ private: gid_t m_gid; uid_t m_uid; - static std::pair s_mntNS; #ifdef UNIT_TEST friend class Ut_Connection;