Improve security_manager_prepare_app() performance 51/207351/2
authorDariusz Michaluk <d.michaluk@samsung.com>
Fri, 31 May 2019 13:10:55 +0000 (15:10 +0200)
committerDariusz Michaluk <d.michaluk@samsung.com>
Mon, 3 Jun 2019 16:42:20 +0000 (18:42 +0200)
This commit merges getPrivilegedGroups() and getAppGroups() into one client request.

Change-Id: I77b42773845b264794398af7995bba087320689d

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

index cef785c..8c2a4f7 100644 (file)
@@ -494,27 +494,16 @@ static int setProcessGroups(const std::vector<gid_t> &groups)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-static int getPrivilegedGroups(std::vector<gid_t> &groups)
+static int getPrivilegedAndAppGroups(const std::string &appProcessLabel, std::vector<gid_t> &privilegedGroups, std::vector<gid_t> &appGroups)
 {
     ClientRequest request(SecurityModuleCall::GROUPS_GET);
-    if (request.send().failed()) {
-        LogError("Failed to get list of groups from security-manager service.");
-        return request.getStatus();
-    }
-
-    request.recv(groups);
-    return SECURITY_MANAGER_SUCCESS;
-}
 
-static int getAppGroups(const std::string appProcessLabel, std::vector<gid_t> &groups)
-{
-    ClientRequest request(SecurityModuleCall::APP_GET_GROUPS);
     if (request.send(appProcessLabel).failed()) {
         LogError("Failed to get list of groups from security-manager service.");
         return request.getStatus();
     }
 
-    request.recv(groups);
+    request.recv(privilegedGroups, appGroups);
     return SECURITY_MANAGER_SUCCESS;
 }
 
@@ -731,16 +720,11 @@ static int security_manager_set_process_groups_internal(const std::string &app_l
             return ret;
         LogDebug("Current supplementary groups count: " << currentGroups.size());
 
-        std::vector<gid_t> privilegedGroups;
-        ret = getPrivilegedGroups(privilegedGroups);
+        std::vector<gid_t> privilegedGroups, allowedGroups;
+        ret = getPrivilegedAndAppGroups(app_label, privilegedGroups, allowedGroups);
         if (ret != SECURITY_MANAGER_SUCCESS)
             return ret;
         LogDebug("All privileged supplementary groups count: " << privilegedGroups.size());
-
-        std::vector<gid_t> allowedGroups;
-        ret = getAppGroups(app_label, allowedGroups);
-        if (ret != SECURITY_MANAGER_SUCCESS)
-            return ret;
         LogDebug("Allowed privileged supplementary groups count: " << allowedGroups.size());
 
         std::unordered_set<gid_t> groupsSet(currentGroups.begin(), currentGroups.end());
index 5c61ad4..0275c0e 100644 (file)
@@ -108,7 +108,6 @@ enum class SecurityModuleCall
     APP_UPDATE,
     APP_UNINSTALL,
     APP_GET_PKG_NAME,
-    APP_GET_GROUPS,
     APP_APPLY_PRIVATE_SHARING,
     APP_DROP_PRIVATE_SHARING,
     USER_ADD,
index d213f1e..a3bd92f 100644 (file)
@@ -112,22 +112,6 @@ public:
     int getPkgName(const std::string &appName, std::string &pkgName);
 
     /**
-    * Process query for supplementary groups allowed for the application.
-    * 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]  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 &appProcessLabel,
-        std::vector<gid_t> &groups);
-
-    /**
     * Process user adding request.
     *
     * @param[in] creds credentials of the requesting process
@@ -186,20 +170,28 @@ public:
     /**
     * Process getting policy descriptions list.
     *
-    * @param[in] descriptions empty vector for descriptions strings
+    * @param[out] descriptions returned vector of descriptions strings
     *
     * @return API return code, as defined in protocols.h
     */
     int policyGetDesc(std::vector<std::string> &descriptions);
 
     /**
-     * Process getting resources group list.
-     *
-     * @param[out] groups empty vector for group strings
-     *
-     * @return API return code, as defined in protocols.h
-     */
-    int policyGetGroups(std::vector<gid_t> &groups);
+    * Process query for resources group list and supplementary groups allowed for the application.
+    * 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]  appProcessLabel application identifier
+    * @param[out] privilegedGroups returned vector of privileged groups
+    * @param[out] appGroups returned vector of allowed groups
+    *
+    * @return API return code, as defined in protocols.h
+    */
+    int getPrivilegedAndAppGroups(const Credentials &creds, const std::string &appProcessLabel,
+        std::vector<gid_t> &privilegedGroups, std::vector<gid_t> &appGroups);
 
     /**
      * Receive groups connected with uid and add them
index d8d9f9e..8362481 100644 (file)
@@ -1175,56 +1175,6 @@ int ServiceImpl::getPkgName(const std::string &appName, std::string &pkgName)
     return SECURITY_MANAGER_SUCCESS;
 }
 
-int ServiceImpl::getAppGroups(const Credentials &creds, const std::string &appProcessLabel,
-    std::vector<gid_t> &groups)
-{
-    try {
-        LogDebug("smack label: " << appProcessLabel);
-
-        std::vector<std::string> privileges;
-
-        std::string uidStr = std::to_string(creds.uid);
-        m_cynaraAdmin.getAppPolicy(appProcessLabel, uidStr, privileges);
-        m_cynaraAdmin.getAppPolicy(appProcessLabel, CYNARA_ADMIN_WILDCARD, privileges);
-        m_cynaraAdmin.getAppPolicy(CYNARA_ADMIN_WILDCARD, CYNARA_ADMIN_WILDCARD, privileges);
-
-        vectorRemoveDuplicates(privileges);
-
-        std::string pidStr = std::to_string(creds.pid);
-        for (const auto &privilege : privileges) {
-            auto &pgids = m_privilegeGids.getGids(privilege);
-
-            LogDebug("Considering privilege " << privilege << " with " <<
-                     pgids.size() << " groups assigned");
-
-            if (pgids.empty())
-                continue;
-
-            if (m_cynara.check(appProcessLabel, privilege, uidStr, pidStr)) {
-                groups.insert(groups.end(), pgids.begin(), pgids.end());
-                LogDebug("Cynara allowed, adding groups");
-            } else {
-                LogDebug("Cynara denied, not adding groups");
-            }
-        }
-        vectorRemoveDuplicates(groups);
-    } catch (const PrivilegeDb::Exception::Base &e) {
-        LogError("Database error: " << e.DumpToString());
-        return SECURITY_MANAGER_ERROR_SERVER_ERROR;
-    } catch (const CynaraException::Base &e) {
-        LogError("Error while querying Cynara for permissions: " << e.DumpToString());
-        return SECURITY_MANAGER_ERROR_SERVER_ERROR;
-    } catch (const SmackException::InvalidLabel &e) {
-        LogError("Error while generating Smack labels: " << e.DumpToString());
-        return SECURITY_MANAGER_ERROR_SERVER_ERROR;
-    } catch (const std::bad_alloc &e) {
-        LogError("Memory allocation failed: " << e.what());
-        return SECURITY_MANAGER_ERROR_MEMORY;
-    }
-
-    return SECURITY_MANAGER_SUCCESS;
-}
-
 int ServiceImpl::userAdd(const Credentials &creds, uid_t uidAdded, int userType)
 {
     if (!authenticate(creds, PRIVILEGE_USER_ADMIN)) {
@@ -1620,19 +1570,51 @@ int ServiceImpl::policyGetDesc(std::vector<std::string> &levels)
     return ret;
 }
 
-int ServiceImpl::policyGetGroups(std::vector<gid_t> &groups)
+int ServiceImpl::getPrivilegedAndAppGroups(const Credentials &creds, const std::string &appProcessLabel,
+    std::vector<gid_t> &privilegedGroups, std::vector<gid_t> &appGroups)
 {
-    int ret = SECURITY_MANAGER_SUCCESS;
-
     try {
+        LogDebug("smack label: " << appProcessLabel);
+
         auto &gids = m_privilegeGids.getGids();
-        groups.insert(groups.end(), gids.begin(), gids.end());
-    } catch (const PrivilegeDb::Exception::Base &e) {
-        LogError("Error while getting groups from database: " << e.DumpToString());
+        privilegedGroups.insert(privilegedGroups.end(), gids.begin(), gids.end());
+
+        std::vector<std::string> privileges;
+
+        std::string uidStr = std::to_string(creds.uid);
+        m_cynaraAdmin.getAppPolicy(appProcessLabel, uidStr, privileges);
+        m_cynaraAdmin.getAppPolicy(appProcessLabel, CYNARA_ADMIN_WILDCARD, privileges);
+        m_cynaraAdmin.getAppPolicy(CYNARA_ADMIN_WILDCARD, CYNARA_ADMIN_WILDCARD, privileges);
+
+        vectorRemoveDuplicates(privileges);
+
+        std::string pidStr = std::to_string(creds.pid);
+        for (const auto &privilege : privileges) {
+            const auto &pgids = m_privilegeGids.getGids(privilege);
+
+            LogDebug("Considering privilege " << privilege << " with " <<
+                     pgids.size() << " groups assigned");
+
+            if (pgids.empty())
+                continue;
+
+            if (m_cynara.check(appProcessLabel, privilege, uidStr, pidStr)) {
+                appGroups.insert(appGroups.end(), pgids.begin(), pgids.end());
+                LogDebug("Cynara allowed, adding groups");
+            } else {
+                LogDebug("Cynara denied, not adding groups");
+            }
+        }
+        vectorRemoveDuplicates(appGroups);
+    } catch (const CynaraException::Base &e) {
+        LogError("Error while querying Cynara for permissions: " << e.DumpToString());
         return SECURITY_MANAGER_ERROR_SERVER_ERROR;
+    } catch (const std::bad_alloc &e) {
+        LogError("Memory allocation failed: " << e.what());
+        return SECURITY_MANAGER_ERROR_MEMORY;
     }
 
-    return ret;
+    return SECURITY_MANAGER_SUCCESS;
 }
 
 int ServiceImpl::policyGroupsForUid(uid_t uid, std::vector<gid_t> &groups)
index 5805cf0..1c697b0 100644 (file)
@@ -104,15 +104,6 @@ private:
      */
     void processGetPkgName(MessageBuffer &buffer, MessageBuffer &send);
 
-    /**
-     * Process getting permitted group ids for app id
-     *
-     * @param  buffer Raw received data buffer
-     * @param  send   Raw data buffer to be sent
-     * @param  creds  credentials of the requesting process
-     */
-    void processGetAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds);
-
     void processUserAdd(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds);
 
     void processUserDelete(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds);
@@ -160,11 +151,13 @@ private:
     void processPolicyGetDesc(MessageBuffer &send);
 
     /**
-     * Process getting groups bound with privileges
+     * Process getting groups bound with privileges and permitted group ids for app id
      *
+     * @param  buffer Raw received data buffer
      * @param  send   Raw data buffer to be sent
+     * @param  creds  credentials of the requesting process
      */
-    void processGroupsGet(MessageBuffer &send);
+    void processGetPrivilegedAndAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds);
 
     /**
      * Process getting groups bound with privileges for given uid
index d881866..043f4a8 100644 (file)
@@ -93,10 +93,6 @@ bool Service::processOne(const ConnectionID &conn, MessageBuffer &buffer,
                     LogDebug("call_type: SecurityModuleCall::APP_GET_PKG_NAME");
                     processGetPkgName(buffer, send);
                     break;
-                case SecurityModuleCall::APP_GET_GROUPS:
-                    LogDebug("call_type: SecurityModuleCall::APP_GET_GROUPS");
-                    processGetAppGroups(buffer, send, creds);
-                    break;
                 case SecurityModuleCall::USER_ADD:
                     LogDebug("call_type: SecurityModuleCall::USER_ADD");
                     processUserAdd(buffer, send, creds);
@@ -127,7 +123,7 @@ bool Service::processOne(const ConnectionID &conn, MessageBuffer &buffer,
                     break;
                 case SecurityModuleCall::GROUPS_GET:
                     LogDebug("call_type: SecurityModuleCall::GROUPS_GET");
-                    processGroupsGet(send);
+                    processGetPrivilegedAndAppGroups(buffer, send, creds);
                     break;
                 case SecurityModuleCall::GROUPS_FOR_UID:
                     processGroupsForUid(buffer, send);
@@ -265,19 +261,6 @@ void Service::processGetPkgName(MessageBuffer &buffer, MessageBuffer &send)
         Serialization::Serialize(send, pkgName);
 }
 
-void Service::processGetAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds)
-{
-    std::string appProcessLabel;
-    std::vector<gid_t> groups;
-    int ret;
-
-    Deserialization::Deserialize(buffer, appProcessLabel);
-    ret = serviceImpl.getAppGroups(creds, appProcessLabel, groups);
-    Serialization::Serialize(send, ret);
-    if (ret == SECURITY_MANAGER_SUCCESS)
-        Serialization::Serialize(send, groups);
-}
-
 void Service::processUserAdd(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds)
 {
     int ret;
@@ -362,14 +345,16 @@ void Service::processPolicyGetDesc(MessageBuffer &send)
     }
 }
 
-void Service::processGroupsGet(MessageBuffer &send)
+void Service::processGetPrivilegedAndAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds)
 {
-    std::vector<gid_t> groups;
-    int ret = serviceImpl.policyGetGroups(groups);
+    std::string appProcessLabel;
+    std::vector<gid_t> privilegedGroups, appGroups;
 
+    Deserialization::Deserialize(buffer, appProcessLabel);
+    int ret = serviceImpl.getPrivilegedAndAppGroups(creds, appProcessLabel, privilegedGroups, appGroups);
     Serialization::Serialize(send, ret);
     if (ret == SECURITY_MANAGER_SUCCESS) {
-        Serialization::Serialize(send, groups);
+        Serialization::Serialize(send, privilegedGroups, appGroups);
     }
 }