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 <r.krypa@samsung.com>

diff --git a/src/common/include/nsmount-logic.h b/src/common/include/nsmount-logic.h
index 3708b67..4e76f10 100644 (file)
--- a/src/common/include/nsmount-logic.h
+++ b/src/common/include/nsmount-logic.h
@@ -27,18 +27,16 @@
 
 #pragma once
 
-#include <cynara.h>
-#include <channel.h>
+#include <mutex>
+
+#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<Privilege,bool> PrivilegeStatus;
@@ -66,17 +64,27 @@ public:
 
     typedef std::vector<Entry> 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 (file)
--- 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 (file)
--- 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 <signal.h>
+
 #include <string>
 
 #include <filesystem.h>
 #include <protocols.h>
 #include <message-buffer.h>
 #include <mount-namespace.h>
+#include <worker.h>
 #include <dpl/serialization.h>
 
 #include <nsmount-logic.h>
@@ -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<std::mutex> 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 (file)
--- a/src/common/service_impl.cpp
+++ b/src/common/service_impl.cpp
@@ -43,7 +43,6 @@
 #include <sys/smack.h>
 
 #include <config.h>
-#include <nsmount-logic.h>
 #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<policy
         // Apply updates
         m_cynaraAdmin.setPolicies(validatedPolicies);
 
-        if (MountNS::isMountNamespaceEnabled())
-            NSMountLogic(m_cynara, m_channel).check();
-
+        // Update mount namespaces
+        // TODO: Don't update all users, apps and privileges, filter by policyEntries
+        m_NSMountLogic.check();
     } catch (const CynaraException::Base &e) {
         LogError("Error while updating Cynara rules: " << e.DumpToString());
         return SECURITY_MANAGER_ERROR_SERVER_ERROR;