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

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 (file)
--- 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<gid_t> 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<gid_t> 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<gid_t> 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<gid_t> 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<gid_t> &allowedGids) const
-{
-    int ret;
-    std::unordered_set<gid_t> 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<gid_t> 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 (file)
--- 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<gid_t> &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 (file)
--- 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<void(void)>& 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 (file)
--- 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 <functional>
+
 #include <app_install_helper.h>
 #include <synchronization_pipe.h>
 
 class ScopedAppLauncher final {
 public:
-    ScopedAppLauncher(const AppInstallHelper& app);
+    explicit ScopedAppLauncher(const AppInstallHelper& app,
+                               const std::function<void(void)>& 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 (file)
--- 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";