Refactor security_manager_prepare_app() 69/166969/5
authorDariusz Michaluk <d.michaluk@samsung.com>
Fri, 12 Jan 2018 11:09:32 +0000 (12:09 +0100)
committerDariusz Michaluk <d.michaluk@samsung.com>
Wed, 17 Jan 2018 18:08:30 +0000 (19:08 +0100)
This change reduces the number of IPCs and SQL queries needed to smack label generation.
The goal is to reduce the application start time.

Change-Id: I2871a51b663b300836459b834d968f2d15cd47e0

src/client/client-security-manager.cpp
src/common/include/service_impl.h
src/common/service_impl.cpp
src/server/service/service.cpp

index 672bc43..571e4b9 100644 (file)
@@ -480,10 +480,10 @@ static int getPrivilegedGroups(std::vector<gid_t> &groups)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-static int getAppGroups(const std::string appName, std::vector<gid_t> &groups)
+static int getAppGroups(const std::string appProcessLabel, std::vector<gid_t> &groups)
 {
     ClientRequest request(SecurityModuleCall::APP_GET_GROUPS);
-    if (request.send(appName).failed()) {
+    if (request.send(appProcessLabel).failed()) {
         LogError("Failed to get list of groups from security-manager service.");
         return request.getStatus();
     }
@@ -582,10 +582,8 @@ inline static int label_for_self_internal()
     return 0;
 }
 
-static inline int security_manager_sync_threads_internal(const char *app_name)
+static inline int security_manager_sync_threads_internal(const std::string &app_label)
 {
-    LogDebug("security_manager_sync_threads_internal called for app_name: " << app_name);
-
     if (ATOMIC_INT_LOCK_FREE != 2) {
         LogError("std::atomic<int> is not always lock free");
         return SECURITY_MANAGER_ERROR_UNKNOWN;
@@ -595,9 +593,7 @@ static inline int security_manager_sync_threads_internal(const char *app_name)
     uid_t cur_tid = Syscall::gettid();
     pid_t cur_pid = getpid();
 
-    int ret = fetchLabelForProcess(app_name, g_app_label);
-    if (ret != SECURITY_MANAGER_SUCCESS)
-        return ret;
+    g_app_label = app_label;
     g_threads_count = 0;
     g_tid_attr_current_map.clear();
     g_smack_present = smack_check();
@@ -696,21 +692,12 @@ static inline int security_manager_sync_threads_internal(const char *app_name)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-SECURITY_MANAGER_API
-int security_manager_set_process_groups_from_appid(const char *app_name)
+static int security_manager_set_process_groups_internal(const std::string &app_label)
 {
     using namespace SecurityManager;
     int ret;
 
-    LogDebug("security_manager_set_process_groups_from_appid() called");
-
     return try_catch([&]() -> int {
-        //checking parameters
-
-        if (app_name == nullptr) {
-            LogError("app_name is NULL");
-            return SECURITY_MANAGER_ERROR_INPUT_PARAM;
-        }
 
         std::vector<gid_t> currentGroups;
         ret = getProcessGroups(currentGroups);
@@ -725,7 +712,7 @@ int security_manager_set_process_groups_from_appid(const char *app_name)
         LogDebug("All privileged supplementary groups count: " << privilegedGroups.size());
 
         std::vector<gid_t> allowedGroups;
-        ret = getAppGroups(app_name, allowedGroups);
+        ret = getAppGroups(app_label, allowedGroups);
         if (ret != SECURITY_MANAGER_SUCCESS)
             return ret;
         LogDebug("Allowed privileged supplementary groups count: " << allowedGroups.size());
@@ -745,6 +732,32 @@ int security_manager_set_process_groups_from_appid(const char *app_name)
 }
 
 SECURITY_MANAGER_API
+int security_manager_set_process_groups_from_appid(const char *app_name)
+{
+    using namespace SecurityManager;
+
+    LogDebug("security_manager_set_process_groups_from_appid() called");
+
+    return try_catch([&]() -> int {
+        //checking parameters
+
+        if (app_name == nullptr) {
+            LogError("app_name is NULL");
+            return SECURITY_MANAGER_ERROR_INPUT_PARAM;
+        }
+
+        std::string app_label;
+        int ret = fetchLabelForProcess(app_name, app_label);
+        if (ret != SECURITY_MANAGER_SUCCESS) {
+            LogError("Failed to generate smack label for appName: " << app_name);
+            return ret;
+        }
+
+        return security_manager_set_process_groups_internal(app_label);
+    });
+}
+
+SECURITY_MANAGER_API
 int security_manager_drop_process_privileges(void)
 {
     LogDebug("security_manager_drop_process_privileges() called");
@@ -774,17 +787,17 @@ int security_manager_drop_process_privileges(void)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-static int setupMountNamespace(const std::string appName, std::vector<std::pair<std::string, bool>> &privilegeStatusVector)
+static int setupMountNamespace(const std::string &appProcessLabel, std::vector<std::pair<std::string, bool>> &privilegeStatusVector)
 {
     ClientRequest request(SecurityModuleCall::APP_SETUP_NAMESPACE);
-    if (request.send(appName).failed())
+    if (request.send(appProcessLabel).failed())
         return request.getStatus();
 
     request.recv(privilegeStatusVector);
     return SECURITY_MANAGER_SUCCESS;
 }
 
-static inline int security_manager_create_namespace_internal(const char *app_name)
+static inline int security_manager_create_namespace_internal(const std::string &app_label)
 {
     if (!MountNS::isMountNamespaceEnabled())
         return SECURITY_MANAGER_SUCCESS;
@@ -798,9 +811,9 @@ static inline int security_manager_create_namespace_internal(const char *app_nam
         return ret;
 
     std::vector<std::pair<std::string, bool>> privilegeStatusVector;
-    ret = setupMountNamespace(app_name, privilegeStatusVector);
+    ret = setupMountNamespace(app_label, privilegeStatusVector);
     if (ret != SECURITY_MANAGER_SUCCESS) {
-        LogError("Failed to setup app namespace: " << security_manager_strerror(static_cast<lib_retcode>(ret)) << " App name:" << app_name);
+        LogError("Failed to setup app namespace: " << security_manager_strerror(static_cast<lib_retcode>(ret)));
         return ret;
     }
 
@@ -812,7 +825,7 @@ static inline int security_manager_create_namespace_internal(const char *app_nam
 
         for (auto &privilegePath : it->second) {
             if (FS::directoryStatus(privilegePath.dstPath) == 0) {
-                LogWarning("Not enforcing privilege " << it->first << " for application " << app_name << " : " <<
+                LogWarning("Not enforcing privilege " << it->first << " for application " << app_label << " : " <<
                            "directory " << privilegePath.dstPath << " doesn't exist");
                 continue;
             }
@@ -837,19 +850,26 @@ int security_manager_prepare_app(const char *app_name)
             return static_cast<int>(SECURITY_MANAGER_ERROR_INPUT_PARAM);
         }
 
-        int ret = security_manager_set_process_groups_from_appid(app_name);
+        std::string app_label;
+        int ret = fetchLabelForProcess(app_name, app_label);
+        if (ret != SECURITY_MANAGER_SUCCESS) {
+            LogError("Failed to generate smack label for appName: " << app_name);
+            return ret;
+        }
+
+        ret = security_manager_set_process_groups_internal(app_label);
         if (ret != SECURITY_MANAGER_SUCCESS) {
             LogError("Unable to setup process groups for application " << app_name);
             return ret;
         }
 
-        ret = security_manager_create_namespace_internal(app_name);
+        ret = security_manager_create_namespace_internal(app_label);
         if (ret != SECURITY_MANAGER_SUCCESS) {
             LogError("Unable to setup namespace for application " << app_name);
             return ret;
         }
 
-        ret = security_manager_sync_threads_internal(app_name);
+        ret = security_manager_sync_threads_internal(app_label);
         if (ret != SECURITY_MANAGER_SUCCESS) {
             LogError("Can't properly setup application threads (Smack label & capabilities) for application " << app_name);
             return ret;
index f9b941b..fc93373 100644 (file)
@@ -78,18 +78,18 @@ public:
 
     /**
     * Process query for supplementary groups allowed for the application.
-    * For given \ref appName and \ref uid, calculate allowed privileges that give
+    * For given \ref appProcessLabel and \ref uid, calculate allowed privileges that give
     * direct access to file system resources. For each permission Cynara will be
     * queried.
     * Returns set of group ids that are permitted.
     *
     * @param[in]  creds credentials of the requesting process
-    * @param[in]  appName application identifier
+    * @param[in]  appProcessLabel application identifier
     * @param[out] groups returned vector of allowed groups
     *
     * @return API return code, as defined in protocols.h
     */
-    int getAppGroups(const Credentials &creds, const std::string &appName,
+    int getAppGroups(const Credentials &creds, const std::string &appProcessLabel,
         std::vector<gid_t> &groups);
 
     /**
@@ -296,12 +296,12 @@ public:
      * Setup app namespace
      *
      * @param[in]  creds    credentials of the requesting process
-     * @param[in]  appName  application identifier
+     * @param[in]  appProcessLabel  application identifier
      * @param[out] privilegeStatusVector returned vector of privilege-status pair
      *
      * @return API return code, as defined in protocols.h
      */
-    int appSetupNamespace(const Credentials &creds, const std::string &appName,
+    int appSetupNamespace(const Credentials &creds, const std::string &appProcessLabel,
         std::vector<std::pair<std::string, bool>> &privilegeStatusVector);
 
     /**
index 8c669d9..86134cf 100644 (file)
@@ -967,12 +967,10 @@ int ServiceImpl::getPkgName(const std::string &appName, std::string &pkgName)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-int ServiceImpl::getAppGroups(const Credentials &creds, const std::string &appName,
+int ServiceImpl::getAppGroups(const Credentials &creds, const std::string &appProcessLabel,
     std::vector<gid_t> &groups)
 {
     try {
-        LogDebug("appName: " << appName);
-        std::string appProcessLabel = getAppProcessLabel(appName);
         LogDebug("smack label: " << appProcessLabel);
 
         std::vector<std::string> privileges;
@@ -1973,7 +1971,7 @@ int ServiceImpl::getClientPrivilegeLicense(
     return SECURITY_MANAGER_SUCCESS;
 }
 
-int ServiceImpl::appSetupNamespace(const Credentials &creds, const std::string &appName,
+int ServiceImpl::appSetupNamespace(const Credentials &creds, const std::string &appProcessLabel,
     std::vector<std::pair<std::string, bool>> &privilegeStatusVector)
 {
     int ret;
@@ -1983,7 +1981,6 @@ int ServiceImpl::appSetupNamespace(const Credentials &creds, const std::string &
     }
 
     try {
-        const std::string appProcessLabel = getAppProcessLabel(appName);
         const std::string uidStr = std::to_string(creds.uid);
         const std::string appsDir = MountNS::getUserAppsMountPointsPath(creds.uid);
         const std::string appFile = MountNS::getUserAppMountPointPath(creds.uid, appProcessLabel);
index 04dff6d..5d81500 100644 (file)
@@ -230,12 +230,12 @@ void Service::processGetPkgName(MessageBuffer &buffer, MessageBuffer &send)
 
 void Service::processGetAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds)
 {
-    std::string appName;
+    std::string appProcessLabel;
     std::vector<gid_t> groups;
     int ret;
 
-    Deserialization::Deserialize(buffer, appName);
-    ret = serviceImpl.getAppGroups(creds, appName, groups);
+    Deserialization::Deserialize(buffer, appProcessLabel);
+    ret = serviceImpl.getAppGroups(creds, appProcessLabel, groups);
     Serialization::Serialize(send, ret);
     if (ret == SECURITY_MANAGER_SUCCESS)
         Serialization::Serialize(send, groups);
@@ -462,11 +462,11 @@ void Service::processGetClientPrivilegeLicense(MessageBuffer &buffer, MessageBuf
 
 void Service::processAppSetupNamespace(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds)
 {
-    std::string appName;
+    std::string appProcessLabel;
     std::vector<std::pair<std::string, bool>> privilegeStatusVector;
 
-    Deserialization::Deserialize(buffer, appName);
-    int ret = serviceImpl.appSetupNamespace(creds, appName, privilegeStatusVector);
+    Deserialization::Deserialize(buffer, appProcessLabel);
+    int ret = serviceImpl.appSetupNamespace(creds, appProcessLabel, privilegeStatusVector);
     Serialization::Serialize(send, ret);
     if (ret == SECURITY_MANAGER_SUCCESS)
         Serialization::Serialize(send, privilegeStatusVector);