security-manager: Change path handling in AppInstallHelper

* properly remove created files
* keep paths in unordered set to eliminate duplicates
* fix tests that drop privileges in the same process
  as AppInstallHelper will be removing its paths

Change-Id: Ie737ef88058c63c3e1ecc868bd4f88b8eeb6797a

diff --git a/src/common/app_install_helper.cpp b/src/common/app_install_helper.cpp
index cee8488eba42aeb77f4a845947a2a948ed2a678c..670f4ed8032f04b14aed04d0d7b61af662ea6aba 100644 (file)
--- a/src/common/app_install_helper.cpp
+++ b/src/common/app_install_helper.cpp
@@ -14,6 +14,8 @@
  *    limitations under the License.
  */
 
+#include <filesystem>
+#include <system_error>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/smack.h>
@@ -208,7 +210,7 @@ bool AppInstallHelper::createFile(app_install_path_type smType, const std::strin
 
     auto ret = fchown(fd, m_uidGid, FILE_GROUP);
     close(fd);
-    m_fileTypeMap[smType].push_back(path);
+    m_fileTypeMap[smType].insert(path);
     return ret == 0;
 }
 
@@ -216,7 +218,7 @@ bool AppInstallHelper::createDir(app_install_path_type smType, const std::string
     mktreeSafe(path, 0777);
     // Dont pass base pkg dirs to SM, because transmute will be forced on RO subdirs
     if (!isBasePath)
-        m_dirTypeMap[smType].push_back(path);
+        m_dirTypeMap[smType].insert(path);
     if (chown(path.c_str(), m_uidGid, FILE_GROUP) == 0)
         return true;
 
@@ -252,7 +254,7 @@ void AppInstallHelper::createDirLink(app_install_path_type smType, const std::st
     createInstallDir(rType);
     std::string linkPath = getPath(smType, PathType::DIR, i, rType);
     if (symlink(dest.c_str(), linkPath.c_str()) == 0) {
-        m_fileTypeMap[smType].push_back(linkPath);
+        m_fileTypeMap[smType].insert(linkPath);
         chown(linkPath.c_str(), m_uidGid, FILE_GROUP);
     }
 }
@@ -452,21 +454,12 @@ const AppInstallHelper::TypePathsMap& AppInstallHelper::getFilesMap() const {
 }
 
 void AppInstallHelper::removePaths() {
-    for (const auto &oneTypePaths : m_dirTypeMap)
-            for (const auto& path : oneTypePaths.second)
-                rmdir(path.c_str());
-
-    m_dirTypeMap.clear();
-
-    for (const auto &oneTypePaths : m_fileTypeMap)
-            for (const auto& path : oneTypePaths.second)
-                unlink(path.c_str());
-
-    m_fileTypeMap.clear();
-
-    for (auto& rootInfo : m_rootPaths) {
-        if (rootInfo.second.isCreated)
-            rmdir(rootInfo.second.path.c_str());
+    for (auto &rootInfo : m_rootPaths) {
+        if (rootInfo.second.isCreated) {
+            std::error_code ec;
+            std::filesystem::remove_all(rootInfo.second.path, ec);
+            RUNNER_ASSERT_ERRNO_MSG(!ec, "Failed to remove root paths of app");
+        }
         rootInfo.second.isCreated = false;
     }
 }

diff --git a/src/common/app_install_helper.h b/src/common/app_install_helper.h
index 2f74d5ee5b1ba9117a64a7db4183c803ee79bae4..fca3b74078aae1002db0f7a469a20885ea3e6dd5 100644 (file)
--- a/src/common/app_install_helper.h
+++ b/src/common/app_install_helper.h
@@ -24,10 +24,11 @@
 #include <optional>
 #include <string>
 #include <utility>
-#include <vector>
+#include <unordered_set>
 
 #include <security-manager-types.h>
 #include <app_def_privilege.h>
+#include <dpl/test/safe_cleanup.h>
 #include "dac.h"
 
 struct Access {
@@ -37,7 +38,7 @@ struct Access {
 
 struct AppInstallHelper {
 
-    using TypePathsMap = std::map<app_install_path_type, std::vector<std::string>>;
+    using TypePathsMap = std::map<app_install_path_type, std::unordered_set<std::string>>;
     using TypePathMap = std::map<app_install_path_type, std::string>;
 
     AppInstallHelper(const std::string &appNamePrefix,
@@ -147,7 +148,7 @@ struct AppInstallHelper {
     void revokeRules() const;
     virtual ~AppInstallHelper() {
         if (m_creatorPid == getpid())
-            removePaths();
+            SafeCleanup::run([this] { removePaths(); });
     }
 
 #ifndef SMACK_ENABLED

diff --git a/src/security-manager-tests/test_cases.cpp b/src/security-manager-tests/test_cases.cpp
index 875036e3743489bf403649d413f10a48665b6753..68ce61bc46b4d7c18583ed9e40c6b30d2b402f84 100644 (file)
--- a/src/security-manager-tests/test_cases.cpp
+++ b/src/security-manager-tests/test_cases.cpp
@@ -1157,14 +1157,16 @@ RUNNER_CHILD_TEST(security_manager_25g_local_user_set_install_type_local)
     app.setInstallType(SM_APP_INSTALL_LOCAL);
     app.addPrivileges(allowedPrivs);
 
-    RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(testUser.getUid(), testUser.getGid()) == 0,
-                            "drop_root_privileges failed");
-    {
-        ScopedInstaller appInstall(app);
-        app.checkAfterInstall();
-        app.checkDeniedPrivileges(someDeniedPrivs);
-    }
-    app.checkAfterUninstall();
+    runInChildParentWait([&] {
+        RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(testUser.getUid(), testUser.getGid()) == 0,
+                                "drop_root_privileges failed");
+        {
+            ScopedInstaller appInstall(app);
+            app.checkAfterInstall();
+            app.checkDeniedPrivileges(someDeniedPrivs);
+        }
+        app.checkAfterUninstall();
+    });
 }
 
 RUNNER_CHILD_TEST(security_manager_25h_local_path_global_install)
@@ -1209,16 +1211,18 @@ RUNNER_CHILD_TEST(security_manager_25j_global_path_local_install)
     AppInstallHelper appGlobal("sm_test_25");
     appGlobal.createPrivateDir();
 
-    RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(testUser.getUid(), testUser.getGid()) == 0,
-                            "drop_root_privileges failed");
+    runInChildParentWait([&] {
+        RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(testUser.getUid(), testUser.getGid()) == 0,
+                                "drop_root_privileges failed");
 
-    InstallRequest invalidReq;
-    invalidReq.setAppId(appLocal.getAppId());
-    invalidReq.setPkgId(appLocal.getPkgId());
-    invalidReq.addPath(appGlobal.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
-    invalidReq.setInstallType(SM_APP_INSTALL_LOCAL);
+        InstallRequest invalidReq;
+        invalidReq.setAppId(appLocal.getAppId());
+        invalidReq.setPkgId(appLocal.getPkgId());
+        invalidReq.addPath(appGlobal.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
+        invalidReq.setInstallType(SM_APP_INSTALL_LOCAL);
 
-    Api::install(invalidReq, SECURITY_MANAGER_ERROR_NOT_PATH_OWNER);
+        Api::install(invalidReq, SECURITY_MANAGER_ERROR_NOT_PATH_OWNER);
+    });
 }
 
 RUNNER_CHILD_TEST(security_manager_26_hybrid_pkg_uninstall_artifacts_check)

diff --git a/src/security-manager-tests/test_cases_register_paths.cpp b/src/security-manager-tests/test_cases_register_paths.cpp
index 1f2229c1daa73ef1370c4b6382c40bbd55e5ea7b..d318edfccd49f308e239ad32e57987ab88138d02 100644 (file)
--- a/src/security-manager-tests/test_cases_register_paths.cpp
+++ b/src/security-manager-tests/test_cases_register_paths.cpp
@@ -147,15 +147,17 @@ RUNNER_CHILD_TEST(security_manager_60_path_req_as_user_positive)
     AppInstallHelper app("sm_test_60", user.getUid());
     ScopedInstaller appInstall(app);
 
-    RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(user.getUid(), user.getGid()) == 0,
-                            "drop_root_privileges failed");
-    app.createPrivateDir();
-    PathsRequest preq;
-    preq.setPkgId(app.getPkgId());
-    preq.setUid(user.getUid());
-    preq.addPath(app.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
+    runInChildParentWait([&] {
+        RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(user.getUid(), user.getGid()) == 0,
+                                "drop_root_privileges failed");
+        app.createPrivateDir();
+        PathsRequest preq;
+        preq.setPkgId(app.getPkgId());
+        preq.setUid(user.getUid());
+        preq.addPath(app.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
 
-    Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+        Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+    });
 }
 
 RUNNER_CHILD_TEST(security_manager_60a_path_req_as_user_positive_realpath_check)
@@ -166,20 +168,22 @@ RUNNER_CHILD_TEST(security_manager_60a_path_req_as_user_positive_realpath_check)
     AppInstallHelper app("sm_test_60a", user.getUid());
     ScopedInstaller appInstall(app);
 
-    RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(user.getUid(), user.getGid()) == 0,
-                            "drop_root_privileges failed");
+    runInChildParentWait([&] {
+        RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(user.getUid(), user.getGid()) == 0,
+                                "drop_root_privileges failed");
 
-    app.createPrivateDir();
-    PathsRequest preq;
-    preq.setPkgId(app.getPkgId());
-    preq.setUid(user.getUid());
-    std::string privPath = "/opt/.././" + app.getPrivateDir();
-    size_t pos = privPath.find_last_of("/");
-    std::string lastElem = privPath.substr(pos + 1);
+        app.createPrivateDir();
+        PathsRequest preq;
+        preq.setPkgId(app.getPkgId());
+        preq.setUid(user.getUid());
+        std::string privPath = "/opt/.././" + app.getPrivateDir();
+        size_t pos = privPath.find_last_of("/");
+        std::string lastElem = privPath.substr(pos + 1);
 
-    preq.addPath(privPath + "/../" + lastElem, SECURITY_MANAGER_PATH_RW);
+        preq.addPath(privPath + "/../" + lastElem, SECURITY_MANAGER_PATH_RW);
 
-    Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+        Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+    });
 }
 
 RUNNER_CHILD_TEST(security_manager_61_path_req_different_user)
@@ -312,17 +316,19 @@ RUNNER_CHILD_TEST(security_manager_64b_path_req_as_local_as_global_user)
 
     app.createPrivateDir();
 
-    RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(TzPlatformConfig::getGlobalUserId(),
-                                                 TzPlatformConfig::getGlobalGroupId()) == 0,
-                            "drop_root_privileges failed");
+    runInChildParentWait([&] {
+        RUNNER_ASSERT_ERRNO_MSG(drop_root_privileges(TzPlatformConfig::getGlobalUserId(),
+                                                     TzPlatformConfig::getGlobalGroupId()) == 0,
+                                "drop_root_privileges failed");
 
-    PathsRequest preq;
-    preq.setPkgId(app.getPkgId());
-    preq.setUid(app.getUID());
-    preq.setInstallType(SM_APP_INSTALL_LOCAL);
-    preq.addPath(app.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
+        PathsRequest preq;
+        preq.setPkgId(app.getPkgId());
+        preq.setUid(app.getUID());
+        preq.setInstallType(SM_APP_INSTALL_LOCAL);
+        preq.addPath(app.getPrivateDir(), SECURITY_MANAGER_PATH_RW);
 
-    Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+        Api::registerPaths(preq, (lib_retcode)SECURITY_MANAGER_SUCCESS);
+    });
 }
 
 RUNNER_TEST(security_manager_66_path_req_check_labels)