Optimize operations on file with list of Smack labels

There's no need to call DB and tz-platform-config for each
label of given user; it makes sense to re-use the fact
that update is called always on update/install/uninstall of precisely
specified package, so changes only affect labels of that package,
be it removal or addition to the set.

Change-Id: I88686341fc49186afe60ed9f86dbdb98c1258064

diff --git a/src/common/include/permissible-set.h b/src/common/include/permissible-set.h
index 1e53b19..9c2889a 100644 (file)
--- a/src/common/include/permissible-set.h
+++ b/src/common/include/permissible-set.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2016-2023 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -61,18 +61,20 @@ public:
 std::string getPermissibleFileLocation(uid_t uid, int installationType);
 
 /**
- * Update permissable file with current content of database
+ * Update permissible file, 1st removing some labels from the list and then,
+ * adding new labels to the list (in this particular order).
  * @throws FileLockError
  * @throws FileOpenError
  * @throws FileWriteError
  *
  * @param[in] uid user id
  * @param[in] installationType type of installation (global or local)
- * @param[in] labelsForUser set of labels permitted for given user
- * @return resulting true on success
+ * @param[in] labelsToAddForUser new labels permitted for given user to be added to file
+ * @param[in] labelsToRemoveForUser labels no longer for given user to be removed from file
  */
 void updatePermissibleFile(uid_t uid, int installationType,
-                           const std::vector<std::string> &labelsForUser);
+                           const std::vector<std::string> &labelsToAddForUser,
+                           const std::vector<std::string> &labelsToRemoveForUser);
 
 /**
  * Read labels from a file into a vector
@@ -82,7 +84,6 @@ void updatePermissibleFile(uid_t uid, int installationType,
  *
  * @param[in] nameFile path to the labels file
  * @param[out] appLabels vector to which application labels are added
- * @return SECURITY_MANAGER_SUCCESS or error code
  */
 void readLabelsFromPermissibleFile(const std::string &nameFile, std::vector<std::string> &appLabels);
 

diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h
index 7a9d6a0..0cb1e7c 100644 (file)
--- a/src/common/include/service_impl.h
+++ b/src/common/include/service_impl.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2022 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2014-2023 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -450,8 +450,6 @@ private:
         const MountNS::AppContext &appContext,
         const std::map<std::string, std::vector<std::string>> &appsAllowedPrivileges);
 
-    void updatePermissibleSet(uid_t uid, int type);
-
     Smack::Label getAppProcessLabel(const std::string &appName, const std::string &pkgName);
 
     Smack::Label getAppProcessLabel(const std::string &appName);

diff --git a/src/common/permissible-set.cpp b/src/common/permissible-set.cpp
index f99989e..cf53100 100644 (file)
--- a/src/common/permissible-set.cpp
+++ b/src/common/permissible-set.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2022 Samsung Electronics Co., Ltd. All rights reserved
+ * Copyright (c) 2016-2023 Samsung Electronics Co., Ltd. All rights reserved
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -97,9 +97,34 @@ static void markPermissibleFileValid(int fd, const std::string &nameFile, bool v
 }
 
 void updatePermissibleFile(uid_t uid, int installationType,
-                           const std::vector<std::string> &labelsForUser)
+                           const std::vector<std::string> &labelsToAddForUser,
+                           const std::vector<std::string> &labelsToRemoveForUser)
 {
     std::string nameFile = getPermissibleFileLocation(uid, installationType);
+    std::vector<std::string> labelsForUser;
+    {
+        std::ifstream fstream;
+        try {
+            openAndLockNameFile(nameFile, fstream);
+
+            std::string line;
+            while (std::getline(fstream, line)) {
+                bool flag = true;
+                for (auto &l : labelsToRemoveForUser)
+                    if (l == line) {
+                        flag = false;
+                        break;
+                    }
+                if (flag)
+                    labelsForUser.push_back(line);
+            }
+        } catch (const PermissibleSet::PermissibleSetException::FileOpenError &e) {
+            LogDebug("File doesn't exist yet, needs to be created - just continue; error:" << e.DumpToString());
+        }
+        for (auto &l : labelsToAddForUser)
+            labelsForUser.push_back(l);
+    }
+
     std::ofstream fstream;
     openAndLockNameFile(nameFile, fstream);
     markPermissibleFileValid(getFd(fstream), nameFile, false);

diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp
index b94c092..76a5602 100644 (file)
--- a/src/common/service_impl.cpp
+++ b/src/common/service_impl.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2022 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2014-2023 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -428,19 +428,6 @@ void ServiceImpl::getPkgLabels(const std::string &pkgName, Smack::Labels &pkgsLa
     }
 }
 
-void ServiceImpl::updatePermissibleSet(uid_t uid, int type)
-{
-    std::vector<std::string> userPkgs;
-    m_privilegeDb.GetUserPkgs(uid, userPkgs);
-    Smack::Labels labelsForUser;
-    for (const auto &pkg : userPkgs) {
-        Smack::Labels pkgLabels;
-        getPkgLabels(pkg, pkgLabels);
-        labelsForUser.insert(labelsForUser.end(), pkgLabels.begin(), pkgLabels.end());
-    }
-    PermissibleSet::updatePermissibleFile(uid, type, labelsForUser);
-}
-
 int ServiceImpl::appInstallInitialChecks(const Credentials &creds, app_inst_req &req)
 {
     if (!verifyAppDefinedPrivileges(req))
@@ -581,6 +568,7 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req)
         ih.isUserPkgInstalled = m_privilegeDb.IsUserPkgInstalled(req.pkgName, req.uid);
         ih.isOldPkgHybrid = ih.isUserPkgInstalled ? m_privilegeDb.IsPackageHybrid(req.pkgName) : req.isHybrid;
 
+
         if (ih.isUserPkgInstalled && ih.isOldPkgHybrid != req.isHybrid) {
             LogError("Application conflicts with existing one. " <<
                      "Flag isHybrid from the request is different than in the database. " <<
@@ -588,6 +576,11 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req)
             return SECURITY_MANAGER_ERROR_INPUT_PARAM;
         }
 
+        Smack::Labels oldLabels;
+
+        if (ih.isUserPkgInstalled)
+            getPkgLabels(req.pkgName, oldLabels);
+
         // [db] begin
         ScopedTransaction trans(m_privilegeDb);
 
@@ -607,8 +600,11 @@ int ServiceImpl::appInstall(const Credentials &creds, app_inst_req &req)
         trans.commit();
         LogDebug("Application installation commited to database");
 
+        Smack::Labels newLabels;
+        getPkgLabels(req.pkgName, newLabels);
+
         // update permissible set
-        updatePermissibleSet(req.uid, req.installationType);
+        PermissibleSet::updatePermissibleFile(req.uid, req.installationType, newLabels, oldLabels);
 
         // label paths
         ret = labelPaths(req.pkgPaths,
@@ -665,7 +661,10 @@ int ServiceImpl::appUpdate(const Credentials &creds, app_inst_req &req)
         ih.isUserPkgInstalled = m_privilegeDb.IsUserPkgInstalled(req.pkgName, req.uid);
         ih.isOldPkgHybrid = ih.isUserPkgInstalled ? m_privilegeDb.IsPackageHybrid(req.pkgName) : req.isHybrid;
 
+        Smack::Labels oldLabels;
+
         if (ih.isUserPkgInstalled) {
+            getPkgLabels(req.pkgName, oldLabels);
             // search for installed apps in the request
             std::vector<std::string> requestedApps;
             for (auto &app : req.apps) {
@@ -714,8 +713,12 @@ int ServiceImpl::appUpdate(const Credentials &creds, app_inst_req &req)
         trans.commit();
         LogDebug("Application installation commited to database");
 
+
+        Smack::Labels newLabels;
+        getPkgLabels(req.pkgName, newLabels);
+
         // update permissible set
-        updatePermissibleSet(req.uid, req.installationType);
+        PermissibleSet::updatePermissibleFile(req.uid, req.installationType, newLabels, oldLabels);
 
         // label paths
         ret = labelPaths(req.pkgPaths,
@@ -928,12 +931,15 @@ int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
                 return SECURITY_MANAGER_SUCCESS;
         }
 
+        Smack::Labels oldLabels;
+
         // Before we remove the app from the database, let's fetch all apps in the package
         // that this app belongs to, this will allow us to remove all rules within the
         // package that the app appears in
         UninstallHelper uh;
         m_privilegeDb.GetPkgAuthorHash(req.pkgName, uh.authorHash);
         getPkgLabels(req.pkgName, uh.pkgLabels);
+        oldLabels = uh.pkgLabels;
         uh.isPkgHybrid = m_privilegeDb.IsPackageHybrid(req.pkgName);
 
         LogDebug("Uninstalling pkg: " << req.pkgName << " with " << req.apps.size() << " apps");
@@ -963,8 +969,14 @@ int ServiceImpl::appUninstall(const Credentials &creds, app_inst_req &req)
         trans.commit();
         LogDebug("Application uninstallation commited to database");
 
+        Smack::Labels newLabels;
+        if(!uh.removePkg)
+            // only if we didn't remove entire pkg, some labels can stay
+            // in the pkg only some apps could have been removed
+            getPkgLabels(req.pkgName, newLabels);
+
         // update permissible set
-        updatePermissibleSet(req.uid, req.installationType);
+        PermissibleSet::updatePermissibleFile(req.uid, req.installationType, newLabels, oldLabels);
 
         // remove and merge Smack rules for apps and pkg
         return appUninstallSmackRules(req, uh);