From 760954e5abbb1978b247d006dc3eb4f2eae747dc Mon Sep 17 00:00:00 2001 From: Krzysztof Jackiewicz Date: Thu, 16 Apr 2020 22:18:49 +0200 Subject: [PATCH] Group privilege check refactoring Make the checking function a passive one. Do not change process suplementary groups in it. Modify ScopedAppLauncher to perform the test in launched app. Test group setting api in a separate test. Change-Id: Iccc20810dad0b667f0f4007701bd0c99e5c99f83 --- .../common/app_install_helper_ext.cpp | 77 +++++++++++----------- .../common/app_install_helper_ext.h | 3 +- .../common/scoped_app_launcher.cpp | 4 +- .../common/scoped_app_launcher.h | 5 +- src/security-manager-tests/test_cases.cpp | 38 ++++++++++- 5 files changed, 83 insertions(+), 44 deletions(-) diff --git a/src/security-manager-tests/common/app_install_helper_ext.cpp b/src/security-manager-tests/common/app_install_helper_ext.cpp index cec5434..734a09f 100644 --- a/src/security-manager-tests/common/app_install_helper_ext.cpp +++ b/src/security-manager-tests/common/app_install_helper_ext.cpp @@ -122,22 +122,50 @@ void AppInstallHelperExt::checkDeniedPrivileges(const PrivilegeVector &deniedPri checkPrivileges({}, deniedPrivs); } -void AppInstallHelperExt::checkPrivilegeGroups(const PrivilegeVector &allowedPrivs) const +void AppInstallHelperExt::checkGroupPrivileges(const PrivilegeVector &expectedPrivs) const { static PolicyConfiguration policy; - const auto allowed_groups = policy.privToGroup(allowedPrivs); - RUNNER_ASSERT_MSG(allowed_groups.size() == allowedPrivs.size(), + + // get expected groups + auto expectedGids = policy.groupToGid(policy.privToGroup(expectedPrivs)); + RUNNER_ASSERT_MSG(expectedGids.size() == expectedPrivs.size(), "Some privileges given were not found in the policy"); + std::sort(expectedGids.begin(), expectedGids.end()); - std::vector allowed_gids; - for (const auto &groupName : allowed_groups) { - errno = 0; - struct group* grp = getgrnam(groupName.c_str()); - RUNNER_ASSERT_ERRNO_MSG(grp, "Group: " << groupName << " not found"); - allowed_gids.push_back(grp->gr_gid); - } + // get current process groups + int ret = getgroups(0, nullptr); + RUNNER_ASSERT_MSG(ret != -1, "Unable to get supplementary groups"); - checkGids(allowed_gids); + std::vector actualGids(ret); + ret = getgroups(ret, actualGids.data()); + RUNNER_ASSERT_MSG(ret != -1, "Unable to get supplementary groups"); + + // remove groups unrelated to privileges + const auto allPrivGids = policy.getGid(); + auto notPrivGid = [&](gid_t gid){ + return std::find(allPrivGids.begin(), allPrivGids.end(), gid) == allPrivGids.end(); + }; + actualGids.erase(std::remove_if(actualGids.begin(), actualGids.end(), notPrivGid), + actualGids.end()); + std::sort(actualGids.begin(), actualGids.end()); + + // expected but not allowed + std::vector notAllowedGids; + std::set_difference(expectedGids.begin(), expectedGids.end(), + actualGids.begin(), actualGids.end(), + std::back_inserter(notAllowedGids)); + + RUNNER_ASSERT_MSG(notAllowedGids.empty(), + notAllowedGids.size() << " expected groups were not assigned"); + + // allowed but not expected + std::vector notDeniedGids; + std::set_difference(actualGids.begin(), actualGids.end(), + expectedGids.begin(), expectedGids.end(), + std::back_inserter(notDeniedGids)); + + RUNNER_ASSERT_MSG(notDeniedGids.empty(), + notDeniedGids.size() << " unexpected groups were assigned"); } void AppInstallHelperExt:: checkSmackPrivileges(const PrivilegeVector &allowedPrivs, @@ -257,31 +285,4 @@ void AppInstallHelperExt::checkAppIdExistence(bool expected) const } } -void AppInstallHelperExt::checkGids(const std::vector &allowedGids) const -{ - int ret; - std::unordered_set referenceGids(allowedGids.begin(), allowedGids.end()); - - // Reset supplementary groups - ret = setgroups(0, NULL); - RUNNER_ASSERT_MSG(ret != -1, "Unable to set supplementary groups"); - - Api::setProcessGroups(m_appName); - - ret = getgroups(0, nullptr); - RUNNER_ASSERT_MSG(ret != -1, "Unable to get supplementary groups"); - - std::vector actualGids(ret); - ret = getgroups(ret, actualGids.data()); - RUNNER_ASSERT_MSG(ret != -1, "Unable to get supplementary groups"); - - for (const auto &gid : actualGids) { - RUNNER_ASSERT_MSG(referenceGids.count(gid) > 0, - "Application shouldn't get access to group " << gid); - referenceGids.erase(gid); - } - - RUNNER_ASSERT_MSG(referenceGids.empty(), "Application didn't get access to some groups"); -} - } // namespace SecurityManagerTest diff --git a/src/security-manager-tests/common/app_install_helper_ext.h b/src/security-manager-tests/common/app_install_helper_ext.h index 5b89d11..fd50ae7 100644 --- a/src/security-manager-tests/common/app_install_helper_ext.h +++ b/src/security-manager-tests/common/app_install_helper_ext.h @@ -32,7 +32,7 @@ public: void checkPrivileges(const PrivilegeVector &allowedPrivs, const PrivilegeVector &deniedPrivs) const; void checkDeniedPrivileges(const PrivilegeVector &deniedPrivs) const; - void checkPrivilegeGroups(const PrivilegeVector &allowedPrivs) const; + void checkGroupPrivileges(const PrivilegeVector &allowedPrivs) const; void checkSmackPrivileges(const PrivilegeVector &allowedPrivs, const PrivilegeVector &deniedPrivs = {}) const; void checkAfterInstall() const; @@ -44,7 +44,6 @@ private: void checkPkgSmackRulesAfterUninstall() const; void checkHybridAppSmackRulesAterUninstall() const; void checkAppIdExistence(bool expected) const; - void checkGids(const std::vector &allowedGids) const; }; } // namespace SecurityManagerTest diff --git a/src/security-manager-tests/common/scoped_app_launcher.cpp b/src/security-manager-tests/common/scoped_app_launcher.cpp index 50a9587..bde5681 100644 --- a/src/security-manager-tests/common/scoped_app_launcher.cpp +++ b/src/security-manager-tests/common/scoped_app_launcher.cpp @@ -22,7 +22,8 @@ using namespace SecurityManagerTest; -ScopedAppLauncher::ScopedAppLauncher(const AppInstallHelper& app) : +ScopedAppLauncher::ScopedAppLauncher(const AppInstallHelper& app, + const std::function& runInAppContext) : m_uid(app.getUID()), m_appId(app.getAppId()) { @@ -39,6 +40,7 @@ ScopedAppLauncher::ScopedAppLauncher(const AppInstallHelper& app) : "launcher failed"); Api::prepareAppCandidate(); Api::prepareApp(app.getAppId().c_str()); + runInAppContext(); } catch (...) { m_syncPipe.post(); throw; diff --git a/src/security-manager-tests/common/scoped_app_launcher.h b/src/security-manager-tests/common/scoped_app_launcher.h index 5a43149..133e7c8 100644 --- a/src/security-manager-tests/common/scoped_app_launcher.h +++ b/src/security-manager-tests/common/scoped_app_launcher.h @@ -15,12 +15,15 @@ */ #pragma once +#include + #include #include class ScopedAppLauncher final { public: - ScopedAppLauncher(const AppInstallHelper& app); + explicit ScopedAppLauncher(const AppInstallHelper& app, + const std::function& runInAppContext = []{}); ~ScopedAppLauncher(); ScopedAppLauncher(const ScopedAppLauncher&) = delete; diff --git a/src/security-manager-tests/test_cases.cpp b/src/security-manager-tests/test_cases.cpp index b009a93..c46e6e0 100644 --- a/src/security-manager-tests/test_cases.cpp +++ b/src/security-manager-tests/test_cases.cpp @@ -136,7 +136,7 @@ RUNNER_TEST(security_manager_01d_app_install_complicated_dir_tree) check_path(sharedRODir, getSharedROPathLabel()); } -RUNNER_TEST(security_manager_02_app_install_uninstall_full) +RUNNER_CHILD_TEST(security_manager_02_app_install_uninstall_full) { const PrivilegeVector defaultPrivs = { PRIV_INTERNAL_AUDIO, @@ -160,7 +160,9 @@ RUNNER_TEST(security_manager_02_app_install_uninstall_full) app.checkAfterInstall(); app.checkDeniedPrivileges(someDeniedPrivs); - app.checkPrivilegeGroups(defaultAllowedPrivs); + { + ScopedAppLauncher launcher(app, [&]{ app.checkGroupPrivileges(defaultAllowedPrivs); }); + } check_path(app.getPrivateDir(), generatePathRWLabel(app.getPkgId())); check_path(app.getPrivateRODir(), generatePathROLabel(app.getPkgId()), false); @@ -171,6 +173,38 @@ RUNNER_TEST(security_manager_02_app_install_uninstall_full) app.checkAfterUninstall(); } +RUNNER_CHILD_TEST(security_manager_02a_set_process_groups) +{ + const PrivilegeVector defaultPrivs = { + PRIV_INTERNAL_AUDIO, + PRIV_INTERNAL_DISPLAY, + PRIV_INTERNAL_VIDEO, + }; + const PrivilegeVector allowedPrivs = {PRIV_CAMERA, PRIV_MEDIASTORAGE}; + + auto defaultAllowedPrivs = defaultPrivs; + defaultAllowedPrivs.insert(defaultAllowedPrivs.end(), allowedPrivs.begin(), allowedPrivs.end()); + + AppInstallHelperExt app("sm_test_02a"); + app.addPrivileges(allowedPrivs); + { + ScopedInstaller appInstall(app); + + app.checkAfterInstall(); + + pid_t pid = fork(); + RUNNER_ASSERT_ERRNO_MSG(pid >= 0, "Fork failed"); + if (pid != 0) { + waitPid(pid); + } else { + Api::setProcessGroups(app.getAppId()); + app.checkGroupPrivileges(defaultAllowedPrivs); + exit(0); + } + } + app.checkAfterUninstall(); +} + RUNNER_CHILD_TEST_SMACK(security_manager_03_set_label_from_appid) { std::string expectedSockLabel = "labelExpectedOnlyFromSocket"; -- 2.7.4