Prevent smack rules leaking during multi-app hybrid pkg uninstall

Package hybridity would be detected after database modifications and
change from 1 to 0 for the last application as a result, leading to
wrong process labels being considered (User::Pkg::$pkgName as opposed
to User::Pkg::$pkgName::App::$appName).

Hybridity is now checked ahead of time to prevent the issue.

Change-Id: Ibe08d443d5fe29d36dabd6df023123da82286d21

diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h
index f02453a15d11fcb4474a6db3660447fefee3d7c8..a0a031e09b9a479d6df1a76885d4521049504d77 100644 (file)
--- a/src/common/include/service_impl.h
+++ b/src/common/include/service_impl.h
@@ -50,7 +50,7 @@ struct InstallHelper {
 
 struct UninstallHelper {
     bool isUserPkgInstalled;
-    bool isOldPkgHybrid;
+    bool isPkgHybrid;
     bool removePkg;
     bool removeAuthor;
     int authorId;
@@ -394,7 +394,7 @@ private:
                                 app_inst_req &req,
                                 UninstallHelper &uh);
 
-    void appUninstallCynaraPolicies(std::string &processLabel,
+    void appUninstallCynaraPolicies(const std::string &processLabel,
                                     app_inst_req &req,
                                     UninstallHelper &uh);
 

diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp
index d902b250a0f7210f96fbba28d38025ebe8676abd..31527946f04c933922445799fc840af4686bc406 100644 (file)
--- a/src/common/service_impl.cpp
+++ b/src/common/service_impl.cpp
@@ -65,7 +65,7 @@ InstallHelper::InstallHelper() {
 
 UninstallHelper::UninstallHelper() {
     isUserPkgInstalled = false;
-    isOldPkgHybrid = false;
+    isPkgHybrid = false;
     removePkg = false;
     removeAuthor = false;
     authorId = 0;
@@ -995,7 +995,7 @@ void ServiceImpl::appUninstallPrivileges(app_inst_req::app &app, app_inst_req &r
     uh.removeApps.push_back(removeApp);
 }
 
-void ServiceImpl::appUninstallCynaraPolicies(std::string &processLabel, app_inst_req &req,
+void ServiceImpl::appUninstallCynaraPolicies(const std::string &processLabel, app_inst_req &req,
         UninstallHelper &ui)
 {
     LogDebug("Removing Cynara policy for: pkgName=" << req.pkgName
@@ -1020,9 +1020,9 @@ int ServiceImpl::appUninstallSmackRules(app_inst_req &req, UninstallHelper &uh)
         for (unsigned appIdx = 0; appIdx < req.apps.size(); ++appIdx) {
             if (uh.removeApps[appIdx]) {
                 const std::string &appName = req.apps[appIdx].appName;
-                std::string processLabel = getAppProcessLabel(appName, req.pkgName);
+                std::string processLabel = SmackLabels::generateProcessLabel(appName, req.pkgName, uh.isPkgHybrid);
                 LogDebug("Removing Smack rules for appName " << appName);
-                if (uh.isOldPkgHybrid || uh.removePkg || uh.isOldPkgHybrid != req.isHybrid) {
+                if (uh.removePkg || uh.isPkgHybrid || req.isHybrid) {
                     /*
                      * Nonhybrid apps have the same label, so revoking it is unnecessary
                      * unless whole package is being removed.
@@ -1077,12 +1077,10 @@ int ServiceImpl::appUninstallSmackRules(app_inst_req &req, UninstallHelper &uh)
 
 int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
 {
-    int ret;
-
     try {
         // initial checks
         int toRemove = 0;
-        ret = appUninstallInitialChecks(creds, req, toRemove);
+        int ret = appUninstallInitialChecks(creds, req, toRemove);
         if (ret != SECURITY_MANAGER_SUCCESS) {
             return ret;
         } else {
@@ -1099,6 +1097,7 @@ int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
         getPkgLabels(req.pkgName, uh.pkgLabels);
         m_privilegeDb.GetPackagesInfo(uh.pkgsInfo);
         getPkgsProcessLabels(uh.pkgsInfo, uh.pkgsProcessLabels);
+        uh.isPkgHybrid = m_privilegeDb.IsPackageHybrid(req.pkgName);
 
         LogDebug("Uninstalling pkg: " << req.pkgName << " with " << req.apps.size() << " apps");
         for (auto &app: req.apps) {
@@ -1111,14 +1110,10 @@ int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
         for (auto &app : req.apps) {
             if (app.appName.empty())
                 continue;
-            // Cynara client (Smack label) needs to be calculated before we'll remove app from privileges db
-            // Otherwise, last app being removed from pkg will not be able to determine correct Smack label
-            // if the pkg is hybrid
-            std::string processLabel = getAppProcessLabel(app.appName, req.pkgName);
             // [db] remove app
             appUninstallPrivileges(app, req, uh);
             // [cynara] update app policy
-            appUninstallCynaraPolicies(processLabel, req, uh);
+            appUninstallCynaraPolicies(SmackLabels::generateProcessLabel(app.appName, req.pkgName, uh.isPkgHybrid), req, uh);
         }
 
         // [db] commit
@@ -1129,8 +1124,7 @@ int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
         updatePermissibleSet(req.uid, req.installationType);
 
         // remove and merge Smack rules for apps and pkg
-        ret = appUninstallSmackRules(req, uh);
-        return ret;
+        return appUninstallSmackRules(req, uh);
     } catch (const PrivilegeDb::Exception::IOError &e) {
         LogError("Cannot access application database: " << e.DumpToString());
         return SECURITY_MANAGER_ERROR_SERVER_ERROR;