From bd010625093b73290150622daf9c6b3a5d0d7344 Mon Sep 17 00:00:00 2001 From: Rafal Krypa Date: Fri, 15 Dec 2017 09:25:25 +0100 Subject: [PATCH] Refactoring: make NSMountLogic class responsible for Channel and MntMonitor NSMountLogic class will now be solely responsible for making updates to mount namespaces of running applications. It's single instance will be persistent in ServiceImpl class. NSMountLogic now owns Channel for communicating with the Worker process and sends requests for mount updates. It also listens to mount events from MntMonitor and sends appropriate requests to worker. All required synchronization should be done in NSMountLogic. NSMountLogic::check() method needs to be thread-safe because it may be called concurrently from ServiceImpl and from MntMonitor thread. Change-Id: I8cb4be25e5f9c8da4360d7ddff34993836f9f169 Signed-off-by: Rafal Krypa --- src/common/include/nsmount-logic.h | 34 +++++++++++++-------- src/common/include/service_impl.h | 7 ++--- src/common/nsmount-logic.cpp | 62 ++++++++++++++++++++++++++++++++------ src/common/service_impl.cpp | 22 +++----------- 4 files changed, 81 insertions(+), 44 deletions(-) diff --git a/src/common/include/nsmount-logic.h b/src/common/include/nsmount-logic.h index 3708b67..4e76f10 100644 --- a/src/common/include/nsmount-logic.h +++ b/src/common/include/nsmount-logic.h @@ -27,18 +27,16 @@ #pragma once -#include -#include +#include + +#include "cynara.h" +#include "channel.h" +#include "mount-monitor.h" namespace SecurityManager { class NSMountLogic { public: - NSMountLogic(Cynara &cynara, Channel &channel) - : m_cynara(cynara) - , m_channel(channel) - {} - struct Entry : public ISerializable { typedef std::string Privilege; typedef std::pair PrivilegeStatus; @@ -66,17 +64,27 @@ public: typedef std::vector EntryVector; + void RegisterChannel(Channel channel) { m_channel = std::move(channel); } + bool check(); + void mntEventCallback(MntDiffEntry &e); + + bool enabled(); + + NSMountLogic(Cynara &cynara); + virtual ~NSMountLogic(); -protected: - void readFiles(void); - void cynaraCheck(void); - bool sendJobs(); - EntryVector m_entryVector; +private: + void readFiles(EntryVector &entryVector); + void cynaraCheck(EntryVector &entryVector); + bool sendJobs(EntryVector &entryVector); + Cynara &m_cynara; - Channel &m_channel; + Channel m_channel; + MntMonitor *m_mntMonitor = nullptr; + std::mutex m_mutex; }; } // namespace SecurityManager diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h index fc2e360..d82cad3 100644 --- a/src/common/include/service_impl.h +++ b/src/common/include/service_impl.h @@ -31,7 +31,7 @@ #include "channel.h" #include "credentials.h" #include "cynara.h" -#include "mount-monitor.h" +#include "nsmount-logic.h" #include "security-manager.h" #include "smack-rules.h" #include "protocols.h" @@ -308,7 +308,7 @@ public: * @param[in] channel * */ - void RegisterChannel(Channel channel) { m_channel = std::move(channel); } + void RegisterChannel(Channel channel) { m_NSMountLogic.RegisterChannel(std::move(channel)); } private: bool authenticate(const Credentials &creds, const std::string &privilege); @@ -374,8 +374,7 @@ private: PrivilegeDb m_privilegeDb; CynaraAdmin m_cynaraAdmin; PrivilegeGids m_privilegeGids; - Channel m_channel; - MntMonitor *m_mntMonitor; + NSMountLogic m_NSMountLogic; }; } /* namespace SecurityManager */ diff --git a/src/common/nsmount-logic.cpp b/src/common/nsmount-logic.cpp index 7bf0250..ee1e388 100644 --- a/src/common/nsmount-logic.cpp +++ b/src/common/nsmount-logic.cpp @@ -21,12 +21,16 @@ * @version 1.0 * @brief Logic for modifying up existing mount namespace. */ + +#include + #include #include #include #include #include +#include #include #include @@ -60,7 +64,7 @@ auto toNumber = [](const std::string &input) namespace SecurityManager { -void NSMountLogic::readFiles(void) +void NSMountLogic::readFiles(EntryVector &entryVector) { auto users = FS::getSubDirectoriesFromDirectory(MountNS::getUsersAppsMountPointsPath(), true); auto last = std::remove_if(users.begin(), users.end(), notNumber); @@ -70,13 +74,13 @@ void NSMountLogic::readFiles(void) std::string dir = MountNS::getUserAppsMountPointsPath(toNumber(user)); if (FS::directoryStatus(dir) > 0) for (auto &smackLabel : FS::getFilesFromDirectory(dir)) - m_entryVector.emplace_back(toNumber(user), smackLabel); + entryVector.emplace_back(toNumber(user), smackLabel); } } -void NSMountLogic::cynaraCheck(void) +void NSMountLogic::cynaraCheck(EntryVector &entryVector) { - for (auto &entry : m_entryVector) { + for (auto &entry : entryVector) { if (entry.smackLabel.empty()) continue; @@ -89,11 +93,11 @@ void NSMountLogic::cynaraCheck(void) } } -bool NSMountLogic::sendJobs(void) +bool NSMountLogic::sendJobs(EntryVector &entryVector) { int status; MessageBuffer send, recv; - Serialization::Serialize(send, m_entryVector); + Serialization::Serialize(send, entryVector); if (!m_channel.write(send)) { LogError("Could not send data to worker!"); return false; @@ -106,12 +110,25 @@ bool NSMountLogic::sendJobs(void) return status == 0; } +bool NSMountLogic::enabled() +{ + static bool enabled = MountNS::isMountNamespaceEnabled(); + + return enabled; +} + bool NSMountLogic::check() { + if (!enabled()) + return true; + + // Critical section + std::lock_guard guard(m_mutex); try { - readFiles(); - cynaraCheck(); - return sendJobs(); + EntryVector entryVector; + readFiles(entryVector); + cynaraCheck(entryVector); + return sendJobs(entryVector); } catch (const MessageBuffer::Exception::Base &e) { LogError("Worker connection failed: " << e.DumpToString()); } catch (const FS::Exception::Base &e) { @@ -120,7 +137,32 @@ bool NSMountLogic::check() return false; } +void NSMountLogic::mntEventCallback(MntDiffEntry &e) +{ + if (!enabled()) + return; + + // TODO: react only on mount events on directories related to a privilege + // TODO: filter out mount events that are generated by security-manager itself + if (e.m_type == MntDiffEntry::Type::MOUNT || e.m_type == MntDiffEntry::Type::UMOUNT) + check(); +} + +NSMountLogic::NSMountLogic(Cynara &cynara) : + m_cynara(cynara) +{ + if (!enabled()) + return; + + m_mntMonitor = new MntMonitor( + std::bind(&NSMountLogic::mntEventCallback, this, std::placeholders::_1)); +} + NSMountLogic::~NSMountLogic() -{} +{ + if (m_mntMonitor != nullptr) + delete m_mntMonitor; + m_channel.closeAll(); +} } // namespace SecurityManager diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index 7142d68..5c6c41f 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -43,7 +43,6 @@ #include #include -#include #include "protocols.h" #include "privilege_db.h" #include "cynara.h" @@ -156,27 +155,16 @@ bool verifyAppDefinedPrivileges(app_inst_req &req) } // end of anonymous namespace -ServiceImpl::ServiceImpl() +ServiceImpl::ServiceImpl() : + m_NSMountLogic(m_cynara) { PrivilegeGids::GroupPrivileges group_privileges; m_privilegeDb.GetGroupsRelatedPrivileges(group_privileges); m_privilegeGids.init(group_privileges); - - if (MountNS::isMountNamespaceEnabled()) - m_mntMonitor = new MntMonitor([this](MntDiffEntry &e) { - // TODO: react only on mount events on directories related to a privilege - // TODO: filter out mount events that are generated by security-manager itself - if (e.m_type == MntDiffEntry::Type::MOUNT || e.m_type == MntDiffEntry::Type::UMOUNT) - NSMountLogic(m_cynara, m_channel).check(); - }); - else - m_mntMonitor = nullptr; } ServiceImpl::~ServiceImpl() { - if (m_mntMonitor != nullptr) - delete m_mntMonitor; } int ServiceImpl::validatePolicy(const Credentials &creds, policy_entry &policyEntry, CynaraAdminPolicy &cyap) @@ -1129,9 +1117,9 @@ int ServiceImpl::policyUpdate(const Credentials &creds, const std::vector