From 75c156112a4919d77fba170f422e6ba4c234b6a1 Mon Sep 17 00:00:00 2001 From: Raine Makelainen Date: Sat, 20 Mar 2021 16:54:43 +0200 Subject: [PATCH] [launcherlib] Revert: Add checks for invoker. JB#53620 This reverts commit 88bf4689e4d1838c847fae25b448710e96b35278. --- src/launcherlib/booster.cpp | 10 +--- src/launcherlib/connection.cpp | 83 +--------------------------------- src/launcherlib/connection.h | 13 ------ 3 files changed, 2 insertions(+), 104 deletions(-) diff --git a/src/launcherlib/booster.cpp b/src/launcherlib/booster.cpp index 2d60948..131fad8 100644 --- a/src/launcherlib/booster.cpp +++ b/src/launcherlib/booster.cpp @@ -58,7 +58,6 @@ Booster::Booster() : m_spaceAvailable(0), m_bootMode(false) { - Connection::setMountNamespace(Connection::getMountNamespace(getpid())); } Booster::~Booster() @@ -237,15 +236,8 @@ 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)) + if(!m_connection->receiveApplicationData(m_appData)) { m_connection->close(); return false; diff --git a/src/launcherlib/connection.cpp b/src/launcherlib/connection.cpp index ac6aee3..71bd405 100644 --- a/src/launcherlib/connection.cpp +++ b/src/launcherlib/connection.cpp @@ -23,8 +23,6 @@ #include #include /* for getsockopt */ #include /* for chmod */ -#include -#include #include #include #include @@ -32,10 +30,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), @@ -77,7 +71,7 @@ int Connection::getFd() const return m_fd; } -bool Connection::accept(AppData*) +bool Connection::accept(AppData* appData) { if (!m_testMode) { @@ -111,47 +105,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) @@ -592,37 +545,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 || 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 62bf088..d22806b 100644 --- a/src/launcherlib/connection.h +++ b/src/launcherlib/connection.h @@ -83,15 +83,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. @@ -141,9 +132,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); @@ -172,7 +160,6 @@ private: gid_t m_gid; uid_t m_uid; - static std::pair s_mntNS; #ifdef UNIT_TEST friend class Ut_Connection;