From: Zofia Abramowska Date: Tue, 11 Mar 2025 10:52:39 +0000 (+0100) Subject: security-manager: Change path handling in AppInstallHelper X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5a47d442357fefb5fca56e8a39ce78ab596be65b;p=platform%2Fcore%2Ftest%2Fsecurity-tests.git 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 cee8488e..670f4ed8 100644 --- a/src/common/app_install_helper.cpp +++ b/src/common/app_install_helper.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include +#include #include #include #include @@ -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 2f74d5ee..fca3b740 100644 --- a/src/common/app_install_helper.h +++ b/src/common/app_install_helper.h @@ -24,10 +24,11 @@ #include #include #include -#include +#include #include #include +#include #include "dac.h" struct Access { @@ -37,7 +38,7 @@ struct Access { struct AppInstallHelper { - using TypePathsMap = std::map>; + using TypePathsMap = std::map>; using TypePathMap = std::map; 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 875036e3..68ce61bc 100644 --- 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 1f2229c1..d318edfc 100644 --- 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)