Refactoring Smack-related code for exception-based error handling 66/36066/4
authorRafal Krypa <r.krypa@samsung.com>
Mon, 2 Mar 2015 11:43:48 +0000 (12:43 +0100)
committerRafal Krypa <r.krypa@samsung.com>
Mon, 2 Mar 2015 15:19:54 +0000 (16:19 +0100)
Smack functions were incoherent with rest of security-manager with regard
to error handling. Functions and methods returned bool value to indicate
their success. This patch changes this schema to use exceptions for error
handling.

Change-Id: If4ec3cac6b63bb411b13a4eb8d9b553e7b5d1c86
Signed-off-by: Rafal Krypa <r.krypa@samsung.com>
src/client/client-security-manager.cpp
src/common/include/smack-exceptions.h [new file with mode: 0644]
src/common/include/smack-labels.h
src/common/include/smack-rules.h
src/common/service_impl.cpp
src/common/smack-labels.cpp
src/common/smack-rules.cpp

index 5b9323a..74a6b30 100644 (file)
@@ -363,7 +363,9 @@ int security_manager_set_process_label_from_appid(const char *app_id)
     if (smack_smackfs_path() == NULL)
         return SECURITY_MANAGER_SUCCESS;
 
-    if (SecurityManager::generateAppLabel(std::string(app_id), appLabel) == false) {
+    try {
+        appLabel = SecurityManager::SmackLabels::generateAppLabel(app_id);
+    } catch (...) {
         LogError("Failed to generate smack label for appId: " << app_id);
         return SECURITY_MANAGER_API_ERROR_NO_SUCH_OBJECT;
     }
diff --git a/src/common/include/smack-exceptions.h b/src/common/include/smack-exceptions.h
new file mode 100644 (file)
index 0000000..5ea2abb
--- /dev/null
@@ -0,0 +1,43 @@
+/*
+ *  Copyright (c) 2015 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *  Contact: Rafal Krypa <r.krypa@samsung.com>
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License
+ */
+/**
+ * @file        smack-exceptions.h
+ * @author      Rafal Krypa <r.krypa@samsung.com>
+ * @version     1.0
+ * @brief       Declaration of Smack-specific exceptions
+ *
+ */
+#ifndef _SMACK_EXCEPTIONS_H_
+#define _SMACK_EXCEPTIONS_H_
+
+#include <dpl/exception.h>
+
+namespace SecurityManager {
+
+class SmackException {
+public:
+    DECLARE_EXCEPTION_TYPE(SecurityManager::Exception, Base)
+    DECLARE_EXCEPTION_TYPE(Base, LibsmackError)
+    DECLARE_EXCEPTION_TYPE(Base, FileError)
+    DECLARE_EXCEPTION_TYPE(Base, InvalidLabel)
+    DECLARE_EXCEPTION_TYPE(Base, InvalidPathType)
+};
+
+} // namespace SecurityManager
+
+#endif /* _SMACK_EXCEPTIONS_H_ */
index ce48a28..f4f15f7 100644 (file)
 
 #include <string>
 #include <utility>
-
-#include "security-manager.h"
+#include <smack-exceptions.h>
+#include <security-manager.h>
 
 namespace SecurityManager {
+namespace SmackLabels {
 
 /**
  * Sets Smack labels on a directory and its contents, recursively.
@@ -41,12 +42,22 @@ namespace SecurityManager {
  * @param pathType[in] type of path to setup. See description of
  *         app_install_path_type in security-manager.h for details
  *
- * @return true on success, false on error.
  */
-bool setupPath(const std::string &appId, const std::string &path,
+void setupPath(const std::string &appId, const std::string &path,
     app_install_path_type pathType);
 
 /**
+ * Sets Smack labels on a <ROOT_APP>/<pkg_id> and <ROOT_APP>/<pkg_id>/<app_id>
+ * non-recursively
+ *
+ * @param pkgId[in] package identifier
+ * @param appId[in] application's identifier
+ * @param basePath[in] <ROOT_APP> path
+ */
+void setupCorrectPath(const std::string &pkgId, const std::string &appId,
+        const std::string &basePath);
+
+/**
  * Generates application name for a label fetched from Cynara
  *
  * @param[in] label string to fetch application name for
@@ -55,34 +66,22 @@ bool setupPath(const std::string &appId, const std::string &path,
 std::string generateAppNameFromLabel(const std::string &label);
 
 /**
- * Sets Smack labels on a <ROOT_APP>/<pkg_id> and <ROOT_APP>/<pkg_id>/<app_id>
- * non-recursively
- *
- * @param pkgID[in] package identifier
- * @param appID[in] application's identifier
- * @param path[in] <ROOT_APP> path
- */
-bool setupCorrectPath(const std::string &pkgID, const std::string &appID,
-        const std::string &path);
-
-/**
- * Generates label for an application with a specific application ID
- * read from @ref appId and assigns it to @ref label.
+ * Generates label for an application with an application ID read from @ref appId.
  *
  * @param[in] appId application's identifier
- * @param[out] label string in which application's label will be stored
- * @return true on success, false on error.
+ * @return resulting Smack label
 */
-bool generateAppLabel(const std::string &appId, std::string &label);
+std::string generateAppLabel(const std::string &appId);
+
 /**
- * Generates label for an application with a package ID
- * read from @ref appPkgId and assigns it to @ref label.
+ * Generates label for an application with a package ID read from @ref pkgId.
  *
  * @param[in] pkgId
- * @param[out] label
- * @return true on success, false on error.
+ * @return resulting Smack label
  */
-bool generatePkgLabel(const std::string &pkgId, std::string &label);
+std::string generatePkgLabel(const std::string &pkgId);
+
+} // namespace SmackLabels
 } // namespace SecurityManager
 
 #endif /* _SMACK_LABELS_H_ */
index 5f5dae8..91446a7 100644 (file)
@@ -27,6 +27,7 @@
 
 #include <vector>
 #include <string>
+#include <smack-exceptions.h>
 
 struct smack_accesses;
 
@@ -38,18 +39,18 @@ public:
     SmackRules();
     virtual ~SmackRules();
 
-    bool add(const std::string &subject, const std::string &object,
+    void add(const std::string &subject, const std::string &object,
             const std::string &permissions);
-    bool addModify(const std::string &subject, const std::string &object,
+    void addModify(const std::string &subject, const std::string &object,
             const std::string &allowPermissions, const std::string &denyPermissions);
-    bool loadFromFile(const std::string &path);
-    bool addFromTemplate(const std::vector<std::string> &templateRules,
+    void loadFromFile(const std::string &path);
+    void addFromTemplate(const std::vector<std::string> &templateRules,
         const std::string &appId, const std::string &pkgId);
-    bool addFromTemplateFile(const std::string &appId, const std::string &pkgId);
+    void addFromTemplateFile(const std::string &appId, const std::string &pkgId);
 
-    bool apply() const;
-    bool clear() const;
-    bool saveToFile(const std::string &path) const;
+    void apply() const;
+    void clear() const;
+    void saveToFile(const std::string &path) const;
 
     /**
      * Create cross dependencies for all applications in a package
@@ -58,9 +59,9 @@ public:
      * correct permissions to shared data.
      *
      * @param[in] pkgContents - a list of all applications inside this package
-     * @return true on success, false on error
      */
-    bool generatePackageCrossDeps(const std::vector<std::string> &pkgContents);
+    void generatePackageCrossDeps(const std::vector<std::string> &pkgContents);
+
     /**
      * Install package-specific smack rules.
      *
@@ -70,9 +71,8 @@ public:
      * @param[in] appId - application id that is beeing installed
      * @param[in] pkgId - package id that the application is in
      * @param[in] pkgContents - a list of all applications in the package
-     * @return true on success, false on error
      */
-    static bool installApplicationRules(const std::string &appId, const std::string &pkgId,
+    static void installApplicationRules(const std::string &appId, const std::string &pkgId,
         const std::vector<std::string> &pkgContents);
     /**
      * Uninstall package-specific smack rules.
@@ -81,13 +81,13 @@ public:
      * and removes them from the persistent storage.
      *
      * @param[in] pkgId - package identifier
-     * @return true if smack rule file has been uninstalled or didn't exist
-     *         false otherwise
      */
-    static bool uninstallPackageRules(const std::string &pkgId);
+    static void uninstallPackageRules(const std::string &pkgId);
+
     /* FIXME: Remove this function if real pkgId instead of "User" label will be used
      * in generateAppLabel(). */
-    static bool addMissingRulesFix();
+    static void addMissingRulesFix();
+
     /**
     * Uninstall application-specific smack rules.
     *
@@ -97,10 +97,10 @@ public:
     * @param[in] appId - application id
     * @param[in] pkgId - package id that the application belongs to
     * @param[in] appsInPkg - a list of other applications in the same package id that the application belongs to
-    * @return true if smack rules have been removed false otherwise
     */
-    static bool uninstallApplicationRules(const std::string &appId, const std::string &pkgId,
+    static void uninstallApplicationRules(const std::string &appId, const std::string &pkgId,
             std::vector<std::string> appsInPkg);
+
     /**
      * Update package specific rules
      *
@@ -110,19 +110,21 @@ public:
      *
      * @param[in] pkgId - id of the package to update
      * @param[in] pkgContents - a list of all applications in the package
-     * @return true in case of success false otherwise
      */
-    static bool updatePackageRules(const std::string &pkgId, const std::vector<std::string> &pkgContents);
+    static void updatePackageRules(const std::string &pkgId, const std::vector<std::string> &pkgContents);
+
 private:
     /**
      * Create a path for package rules
      *
      */
     static std::string getPackageRulesFilePath(const std::string &pkgId);
+
     /**
      * Create a path for application rules
      */
     static std::string getApplicationRulesFilePath(const std::string &appId);
+
     /**
      * Uninstall rules inside a specified file path
      *
@@ -130,9 +132,8 @@ private:
      * rules in the file specified by path
      *
      * @param[in] path - path to the file that contains the rules
-     * @return true in case of success false otherwise
      */
-    static bool uninstallRules (const std::string &path);
+    static void uninstallRules (const std::string &path);
 
     smack_accesses *m_handle;
 };
index 9576faa..8828b29 100644 (file)
@@ -64,7 +64,6 @@ static inline int validatePolicy(policy_entry &policyEntry, std::string uidStr,
         policyEntry.user = uidStr;
     };
 
-    std::string client;
     int level;
 
     if (policyEntry.currentLevel.empty()) { //for admin
@@ -116,13 +115,10 @@ static inline int validatePolicy(policy_entry &policyEntry, std::string uidStr,
         policyEntry.user = CYNARA_ADMIN_WILDCARD;
     if (!policyEntry.privilege.compare(SECURITY_MANAGER_ANY))
         policyEntry.privilege = CYNARA_ADMIN_WILDCARD;
-    if (policyEntry.appId.compare(SECURITY_MANAGER_ANY))
-        generateAppLabel(policyEntry.appId, client);
-    else
-        client = CYNARA_ADMIN_WILDCARD;
 
     cyap = std::move(CynaraAdminPolicy(
-        client,
+        policyEntry.appId.compare(SECURITY_MANAGER_ANY) ?
+            SmackLabels::generateAppLabel(policyEntry.appId) : CYNARA_ADMIN_WILDCARD,
         policyEntry.user,
         policyEntry.privilege,
         level,
@@ -252,6 +248,9 @@ int appInstall(const app_inst_req &req, uid_t uid)
     std::string uidstr;
     bool isCorrectPath = false;
     std::string appPath;
+    std::string appLabel;
+    std::string pkgLabel;
+
     if (uid) {
         if (uid != req.uid) {
             LogError("User " << uid <<
@@ -269,15 +268,6 @@ int appInstall(const app_inst_req &req, uid_t uid)
         return SECURITY_MANAGER_API_ERROR_AUTHENTICATION_FAILED;
     }
 
-    std::string smackLabel;
-    if (!generateAppLabel(req.appId, smackLabel)) {
-        LogError("Cannot generate Smack label for application: " << req.appId);
-        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
-    }
-
-    LogDebug("Install parameters: appId: " << req.appId << ", pkgId: " << req.pkgId
-            << ", generated smack label: " << smackLabel);
-
     // create null terminated array of strings for permissions
     std::unique_ptr<const char *[]> pp_permissions(new const char* [req.privileges.size() + 1]);
     for (size_t i = 0; i < req.privileges.size(); ++i) {
@@ -288,8 +278,13 @@ int appInstall(const app_inst_req &req, uid_t uid)
 
     try {
         std::vector<std::string> oldAppPrivileges;
+
+        appLabel = SmackLabels::generateAppLabel(req.appId);
+        /* NOTE: we don't use pkgLabel here, but generate it for pkgId validation */
+        pkgLabel = SmackLabels::generatePkgLabel(req.pkgId);
         LogDebug("Install parameters: appId: " << req.appId << ", pkgId: " << req.pkgId
-                 << ", uidstr " << uidstr << ", generated smack label: " << smackLabel);
+                 << ", uidstr " << uidstr
+                 << ", app label: " << appLabel << ", pkg label: " << pkgLabel);
 
         PrivilegeDb::getInstance().BeginTransaction();
         std::string pkg;
@@ -304,7 +299,7 @@ int appInstall(const app_inst_req &req, uid_t uid)
         PrivilegeDb::getInstance().UpdateAppPrivileges(req.appId, uid, req.privileges);
         /* Get all application ids in the package to generate rules withing the package */
         PrivilegeDb::getInstance().GetAppIdsForPkgId(req.pkgId, pkgContents);
-        CynaraAdmin::getInstance().UpdateAppPolicy(smackLabel, uidstr, oldAppPrivileges,
+        CynaraAdmin::getInstance().UpdateAppPolicy(appLabel, uidstr, oldAppPrivileges,
                                          req.privileges);
         PrivilegeDb::getInstance().CommitTransaction();
         LogDebug("Application installation commited to database");
@@ -319,35 +314,36 @@ int appInstall(const app_inst_req &req, uid_t uid)
         PrivilegeDb::getInstance().RollbackTransaction();
         LogError("Error while setting Cynara rules for application: " << e.DumpToString());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    } catch (const SmackException::InvalidLabel &e) {
+        PrivilegeDb::getInstance().RollbackTransaction();
+        LogError("Error while generating Smack labels: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
     } catch (const std::bad_alloc &e) {
         PrivilegeDb::getInstance().RollbackTransaction();
         LogError("Memory allocation while setting Cynara rules for application: " << e.what());
-        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+        return SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY;
     }
 
-    if (isCorrectPath)
-        if (!setupCorrectPath(req.pkgId, req.appId, appPath)) {
-            LogError("Can't setup <APP_ROOT> dirs");
-            return false;
-        }
-
-    // register paths
-    for (const auto &appPath : req.appPaths) {
-        const std::string &path = appPath.first;
-        app_install_path_type pathType = static_cast<app_install_path_type>(appPath.second);
-        int result = setupPath(req.appId, path, pathType);
-
-        if (!result) {
-            LogError("setupPath() failed");
-            return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    try {
+        if (isCorrectPath)
+            SmackLabels::setupCorrectPath(req.pkgId, req.appId, appPath);
+
+        // register paths
+        for (const auto &appPath : req.appPaths) {
+            const std::string &path = appPath.first;
+            app_install_path_type pathType = static_cast<app_install_path_type>(appPath.second);
+            SmackLabels::setupPath(req.appId, path, pathType);
         }
-    }
 
-    LogDebug("Adding Smack rules for new appId: " << req.appId << " with pkgId: "
-            << req.pkgId << ". Applications in package: " << pkgContents.size());
-    if (!SmackRules::installApplicationRules(req.appId, req.pkgId, pkgContents)) {
-        LogError("Failed to apply package-specific smack rules");
-        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+        LogDebug("Adding Smack rules for new appId: " << req.appId << " with pkgId: "
+                << req.pkgId << ". Applications in package: " << pkgContents.size());
+        SmackRules::installApplicationRules(req.appId, req.pkgId, pkgContents);
+    } catch (const SmackException::Base &e) {
+        LogError("Error while applying Smack policy for application: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SETTING_FILE_LABEL_FAILED;
+    } catch (const std::bad_alloc &e) {
+        LogError("Memory allocation error: " << e.what());
+        return SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY;
     }
 
     return SECURITY_MANAGER_API_SUCCESS;
@@ -373,15 +369,10 @@ int appUninstall(const std::string &appId, uid_t uid)
             PrivilegeDb::getInstance().RollbackTransaction();
             appExists = false;
         } else {
-
+            smackLabel = SmackLabels::generateAppLabel(appId);
             LogDebug("Uninstall parameters: appId: " << appId << ", pkgId: " << pkgId
                      << ", uidstr " << uidstr << ", generated smack label: " << smackLabel);
 
-            if (!generateAppLabel(appId, smackLabel)) {
-                LogError("Cannot generate Smack label for package: " << pkgId);
-                return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
-            }
-
             /* 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 withing the
                 package that the app appears in */
@@ -405,26 +396,32 @@ int appUninstall(const std::string &appId, uid_t uid)
         PrivilegeDb::getInstance().RollbackTransaction();
         LogError("Error while setting Cynara rules for application: " << e.DumpToString());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    } catch (const SmackException::InvalidLabel &e) {
+        PrivilegeDb::getInstance().RollbackTransaction();
+        LogError("Error while generating Smack labels: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
     } catch (const std::bad_alloc &e) {
         PrivilegeDb::getInstance().RollbackTransaction();
         LogError("Memory allocation while setting Cynara rules for application: " << e.what());
-        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+        return SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY;
     }
 
-    if (appExists) {
-
-        if (removePkg) {
-            LogDebug("Removing Smack rules for deleted pkgId " << pkgId);
-            if (!SmackRules::uninstallPackageRules(pkgId)) {
-                LogError("Error on uninstallation of package-specific smack rules");
-                return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    try {
+        if (appExists) {
+            if (removePkg) {
+                LogDebug("Removing Smack rules for deleted pkgId " << pkgId);
+                SmackRules::uninstallPackageRules(pkgId);
             }
+
+            LogDebug("Removing smack rules for deleted appId " << appId);
+            SmackRules::uninstallApplicationRules(appId, pkgId, pkgContents);
         }
-        LogDebug ("Removing smack rules for deleted appId " << appId);
-        if (!SmackRules::uninstallApplicationRules(appId, pkgId, pkgContents)) {
-            LogError("Error during uninstallation of application-specific smack rules");
-            return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
-        }
+    } catch (const SmackException::Base &e) {
+        LogError("Error while removing Smack rules for application: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SETTING_FILE_LABEL_FAILED;
+    } catch (const std::bad_alloc &e) {
+        LogError("Memory allocation error: " << e.what());
+        return SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY;
     }
 
     return SECURITY_MANAGER_API_SUCCESS;
@@ -464,10 +461,8 @@ int getAppGroups(const std::string &appId, uid_t uid, pid_t pid, std::unordered_
             return SECURITY_MANAGER_API_ERROR_NO_SUCH_OBJECT;
         }
         LogDebug("pkgId: " << pkgId);
-        if (!generatePkgLabel(pkgId, smackLabel)) {
-            LogError("Cannot generate Smack label for pkgId: " << pkgId);
-            return SECURITY_MANAGER_API_ERROR_NO_SUCH_OBJECT;
-        }
+
+        smackLabel = SmackLabels::generatePkgLabel(pkgId);
         LogDebug("smack label: " << smackLabel);
 
         std::vector<std::string> privileges;
@@ -506,6 +501,9 @@ int getAppGroups(const std::string &appId, uid_t uid, pid_t pid, std::unordered_
     } catch (const CynaraException::Base &e) {
         LogError("Error while querying Cynara for permissions: " << e.DumpToString());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    } catch (const SmackException::InvalidLabel &e) {
+        LogError("Error while generating Smack labels: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
     } catch (const std::bad_alloc &e) {
         LogError("Memory allocation failed: " << e.what());
         return SECURITY_MANAGER_API_ERROR_OUT_OF_MEMORY;
@@ -637,14 +635,9 @@ int getConfiguredPolicy(bool forAdmin, const policy_entry &filter, uid_t uid, pi
         std::vector<CynaraAdminPolicy> listOfPolicies;
 
         //convert appId to smack label
-        std::string appLabel;
-        if (!filter.appId.compare(SECURITY_MANAGER_ANY))
-            appLabel = CYNARA_ADMIN_ANY;
-        else
-            generateAppLabel(filter.appId, appLabel);
-
-        std::string user = (!filter.user.compare(SECURITY_MANAGER_ANY))? CYNARA_ADMIN_ANY: filter.user;
-        std::string privilege = (!filter.privilege.compare(SECURITY_MANAGER_ANY))? CYNARA_ADMIN_ANY: filter.privilege;
+        std::string appLabel = filter.appId.compare(SECURITY_MANAGER_ANY) ? SmackLabels::generateAppLabel(filter.appId) : CYNARA_ADMIN_ANY;
+        std::string user = filter.user.compare(SECURITY_MANAGER_ANY) ? filter.user : CYNARA_ADMIN_ANY;
+        std::string privilege = filter.privilege.compare(SECURITY_MANAGER_ANY) ? filter.privilege : CYNARA_ADMIN_ANY;
 
         LogDebug("App: " << filter.appId << ", Label: " << appLabel);
 
@@ -688,7 +681,7 @@ int getConfiguredPolicy(bool forAdmin, const policy_entry &filter, uid_t uid, pi
 
             policy_entry pe;
 
-            pe.appId = strcmp(policy.client, CYNARA_ADMIN_WILDCARD) ? generateAppNameFromLabel(policy.client) : SECURITY_MANAGER_ANY;
+            pe.appId = strcmp(policy.client, CYNARA_ADMIN_WILDCARD) ? SmackLabels::generateAppNameFromLabel(policy.client) : SECURITY_MANAGER_ANY;
             pe.user =  strcmp(policy.user, CYNARA_ADMIN_WILDCARD) ? policy.user : SECURITY_MANAGER_ANY;
             pe.privilege = strcmp(policy.privilege, CYNARA_ADMIN_WILDCARD) ? policy.privilege : pe.privilege = SECURITY_MANAGER_ANY;
             pe.currentLevel = CynaraAdmin::getInstance().convertToPolicyDescription(policy.result);
@@ -718,6 +711,9 @@ int getConfiguredPolicy(bool forAdmin, const policy_entry &filter, uid_t uid, pi
     } catch (const CynaraException::Base &e) {
         LogError("Error while listing Cynara rules: " << e.DumpToString());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    } catch (const SmackException::InvalidLabel &e) {
+        LogError("Error while generating Smack labels: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
     } catch (const std::bad_alloc &e) {
         LogError("Memory allocation error while listing Cynara rules: " << e.what());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
@@ -780,10 +776,9 @@ int getPolicy(const policy_entry &filter, uid_t uid, pid_t pid, const std::strin
 
             for (const std::string &appId : listOfApps) {
                 LogDebug("App: " << appId);
-                std::string smackLabelForApp;
+                std::string smackLabelForApp = SmackLabels::generateAppLabel(appId);
                 std::vector<std::string> listOfPrivileges;
 
-                generateAppLabel(appId, smackLabelForApp);
                 // FIXME: also fetch privileges of global applications
                 PrivilegeDb::getInstance().GetAppPrivileges(appId, uid, listOfPrivileges);
 
@@ -834,6 +829,9 @@ int getPolicy(const policy_entry &filter, uid_t uid, pid_t pid, const std::strin
     } catch (const CynaraException::Base &e) {
         LogError("Error while listing Cynara rules: " << e.DumpToString());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
+    } catch (const SmackException::InvalidLabel &e) {
+        LogError("Error while generating Smack labels: " << e.DumpToString());
+        return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
     } catch (const std::bad_alloc &e) {
         LogError("Memory allocation error while listing Cynara rules: " << e.what());
         return SECURITY_MANAGER_API_ERROR_SERVER_ERROR;
index 2e822a1..feef9f7 100644 (file)
 #include "smack-labels.h"
 
 namespace SecurityManager {
+namespace SmackLabels {
 
 /* Const defined below is used to label files accessible to apps only for reading */
 const char *const LABEL_FOR_APP_RO_PATH = "User::Home";
 
-enum class FileDecision {
-    SKIP = 0,
-    LABEL = 1,
-    ERROR = -1
-};
+typedef std::function<bool(const FTSENT*)> LabelDecisionFn;
 
-typedef std::function<FileDecision(const FTSENT*)> LabelDecisionFn;
-
-static FileDecision labelAll(const FTSENT *ftsent __attribute__((unused)))
+static bool labelAll(const FTSENT *ftsent __attribute__((unused)))
 {
-    return FileDecision::LABEL;
+    return true;
 }
 
-static FileDecision labelDirs(const FTSENT *ftsent)
+static bool labelDirs(const FTSENT *ftsent)
 {
     // label only directories
-    if (S_ISDIR(ftsent->fts_statp->st_mode))
-        return FileDecision::LABEL;
-    return FileDecision::SKIP;
+    return (S_ISDIR(ftsent->fts_statp->st_mode));
 }
 
-static FileDecision labelExecs(const FTSENT *ftsent)
+static bool labelExecs(const FTSENT *ftsent)
 {
     // LogDebug("Mode = " << ftsent->fts_statp->st_mode); // this could be helpfull in debugging
     // label only regular executable files
-    if (S_ISREG(ftsent->fts_statp->st_mode) && (ftsent->fts_statp->st_mode & S_IXUSR))
-        return FileDecision::LABEL;
-    return FileDecision::SKIP;
+    return (S_ISREG(ftsent->fts_statp->st_mode) && (ftsent->fts_statp->st_mode & S_IXUSR));
 }
 
-static inline bool pathSetSmack(const char *path, const std::string &label,
+static inline void pathSetSmack(const char *path, const std::string &label,
         const char *xattr_name)
 {
-    if (lsetxattr(path, xattr_name, label.c_str(), label.length(), 0) != 0)
-        return false;
-
-    return true;
+    if (lsetxattr(path, xattr_name, label.c_str(), label.length(), 0)) {
+        LogError("lsetxattr failed.");
+        ThrowMsg(SmackException::FileError, "lsetxattr failed.");
+    }
 }
 
-static bool dirSetSmack(const std::string &path, const std::string &label,
+static void dirSetSmack(const std::string &path, const std::string &label,
         const char *xattr_name, LabelDecisionFn fn)
 {
     char *const path_argv[] = {const_cast<char *>(path.c_str()), NULL};
     FTSENT *ftsent;
-    FileDecision ret;
 
     std::unique_ptr<FTS, std::function<void(FTS*)> > fts(
             fts_open(path_argv, FTS_PHYSICAL | FTS_NOCHDIR, NULL),
             fts_close);
 
-    if (fts.get() == NULL) {
+    if (!fts) {
         LogError("fts_open failed.");
-        return false;
+        ThrowMsg(SmackException::FileError, "fts_open failed.");
     }
 
     while ((ftsent = fts_read(fts.get())) != NULL) {
         /* Check for error (FTS_ERR) or failed stat(2) (FTS_NS) */
         if (ftsent->fts_info == FTS_ERR || ftsent->fts_info == FTS_NS) {
             LogError("FTS_ERR error or failed stat(2) (FTS_NS)");
-            return false;
+            ThrowMsg(SmackException::FileError, "FTS_ERR error or failed stat(2) (FTS_NS)");
         }
 
         /* avoid to tag directories two times */
         if (ftsent->fts_info == FTS_D)
             continue;
 
-        ret = fn(ftsent);
-        if (ret == FileDecision::ERROR) {
-            LogError("fn(ftsent) failed.");
-            return false;
-        }
-
-        if (ret == FileDecision::LABEL) {
-            if (!pathSetSmack(ftsent->fts_path, label, xattr_name)) {
-                LogError("pathSetSmack failed.");
-                return false;
-            }
-        }
-
+        if (fn(ftsent))
+            pathSetSmack(ftsent->fts_path, label, xattr_name);
     }
 
     /* If last call to fts_read() set errno, we need to return error. */
     if ((errno != 0) && (ftsent == NULL)) {
         LogError("Last errno from fts_read: " << strerror(errno));
-        return false;
+        ThrowMsg(SmackException::FileError, "Last errno from fts_read: " << strerror(errno));
     }
-    return true;
 }
 
-static bool labelDir(const std::string &path, const std::string &label,
+static void labelDir(const std::string &path, const std::string &label,
         bool set_transmutable, bool set_executables)
 {
-    bool ret = true;
-
     // setting access label on everything in given directory and below
-    ret = dirSetSmack(path, label, XATTR_NAME_SMACK, labelAll);
-    if (!ret) {
-        LogError("dirSetSmack failed (access label)");
-        return ret;
-    }
+    dirSetSmack(path, label, XATTR_NAME_SMACK, labelAll);
 
-    if (set_transmutable) {
-        // setting transmute on dirs
-        ret = dirSetSmack(path, "TRUE", XATTR_NAME_SMACKTRANSMUTE, labelDirs);
-        if (!ret) {
-            LogError("dirSetSmack failed (transmute)");
-            return ret;
-        }
-    }
+    // setting transmute on dirs
+    if (set_transmutable)
+        dirSetSmack(path, "TRUE", XATTR_NAME_SMACKTRANSMUTE, labelDirs);
 
-    if (set_executables) {
-        ret = dirSetSmack(path, label, XATTR_NAME_SMACKEXEC, &labelExecs);
-        if (!ret)
-        {
-            LogError("dirSetSmack failed (execs).");
-            return ret;
-        }
-    }
-
-    return ret;
+    // setting SMACK64EXEC labels
+    if (set_executables)
+        dirSetSmack(path, label, XATTR_NAME_SMACKEXEC, &labelExecs);
 }
 
-bool setupPath(const std::string &appId, const std::string &path,
+void setupPath(const std::string &appId, const std::string &path,
     app_install_path_type pathType)
 {
     std::string label;
@@ -174,8 +134,7 @@ bool setupPath(const std::string &appId, const std::string &path,
     switch (pathType) {
     case SECURITY_MANAGER_PATH_PRIVATE:
     case SECURITY_MANAGER_PATH_RW:
-        if (!generateAppLabel(appId, label))
-            return false;
+        label = generateAppLabel(appId);
         label_executables = true;
         label_transmute = false;
         break;
@@ -192,62 +151,38 @@ bool setupPath(const std::string &appId, const std::string &path,
         break;
     default:
         LogError("Path type not known.");
-        return false;
+        Throw(SmackException::InvalidPathType);
     }
     return labelDir(path, label, label_transmute, label_executables);
 }
 
-std::string generateAppNameFromLabel(const std::string &label)
+void setupCorrectPath(const std::string &pkgId, const std::string &appId, const std::string &basePath)
 {
-    //TODO: Fix when a label generating mechanism is ready
-    return label;
+    std::string pkgPath = basePath + "/" + pkgId;
+    std::string appPath = pkgPath + "/" + appId;
+
+    pathSetSmack(pkgPath.c_str(), generatePkgLabel(pkgId), XATTR_NAME_SMACK);
+    pathSetSmack(appPath.c_str(), generateAppLabel(appId), XATTR_NAME_SMACK);
+    pathSetSmack(appPath.c_str(), "TRUE", XATTR_NAME_SMACKTRANSMUTE);
 }
 
-bool setupCorrectPath(const std::string &pkgId, const std::string &appId, const std::string &appPath)
+std::string generateAppNameFromLabel(const std::string &label)
 {
-    std::string tmpPath;
-    std::string label;
-
-    tmpPath.clear();
-    tmpPath = appPath + "/" + pkgId;
-
-    label.clear();
-    generatePkgLabel(pkgId, label);
-
-    if (!pathSetSmack(tmpPath.c_str(), label, XATTR_NAME_SMACK)) {
-        LogError("pathSetSmack failed (access label) on: " << tmpPath);
-        return false;
-    }
-
-    label.clear();
-    generateAppLabel(appId, label);
-    tmpPath += "/" + appId;
-
-    if (!pathSetSmack(tmpPath.c_str(), label, XATTR_NAME_SMACK)) {
-        LogError("pathSetSmack failed (access label) on: " << tmpPath);
-        return false;
-    }
-
-    if (!pathSetSmack(tmpPath.c_str(), "TRUE", XATTR_NAME_SMACKTRANSMUTE)) {
-        LogError("pathSetSmack failed (transmute) on: " << tmpPath);
-        return false;
-    }
-
-    return true;
+    //TODO: Fix when a label generating mechanism is ready
+    return label;
 }
 
-bool generateAppLabel(const std::string &appId, std::string &label)
+std::string generateAppLabel(const std::string &appId)
 {
     (void) appId;
-    label = "User";
-    return true;
+    return "User";
 }
 
-bool generatePkgLabel(const std::string &pkgId, std::string &label)
+std::string generatePkgLabel(const std::string &pkgId)
 {
     (void) pkgId;
-    label = "User";
-    return (true);
+    return "User";
 }
 
+} // namespace SmackLabels
 } // namespace SecurityManager
index b2dceb9..ace23ba 100644 (file)
@@ -32,6 +32,7 @@
 #include <fstream>
 #include <cstring>
 #include <sstream>
+#include <memory>
 
 #include <dpl/log/log.h>
 #include <tzplatform_config.h>
@@ -58,86 +59,85 @@ SmackRules::~SmackRules() {
     smack_accesses_free(m_handle);
 }
 
-bool SmackRules::add(const std::string &subject, const std::string &object,
+void SmackRules::add(const std::string &subject, const std::string &object,
         const std::string &permissions)
 {
-    return 0 == smack_accesses_add(m_handle, subject.c_str(), object.c_str(), permissions.c_str());
+    if (smack_accesses_add(m_handle, subject.c_str(), object.c_str(), permissions.c_str()))
+        ThrowMsg(SmackException::LibsmackError, "smack_accesses_add");
 }
 
-bool SmackRules::addModify(const std::string &subject, const std::string &object,
+void SmackRules::addModify(const std::string &subject, const std::string &object,
         const std::string &allowPermissions, const std::string &denyPermissions)
 {
-    return 0 == smack_accesses_add_modify(m_handle, subject.c_str(), object.c_str(), allowPermissions.c_str(), denyPermissions.c_str());
+    if (smack_accesses_add_modify(m_handle, subject.c_str(), object.c_str(), allowPermissions.c_str(), denyPermissions.c_str()))
+        ThrowMsg(SmackException::LibsmackError, "smack_accesses_add_modify");
 }
 
-bool SmackRules::clear() const
+void SmackRules::clear() const
 {
-    return 0 == smack_accesses_clear(m_handle);
+    if (smack_accesses_clear(m_handle))
+        ThrowMsg(SmackException::LibsmackError, "smack_accesses_clear");
 }
 
-bool SmackRules::apply() const
+void SmackRules::apply() const
 {
-    return 0 == smack_accesses_apply(m_handle);
+    if (smack_accesses_apply(m_handle))
+        ThrowMsg(SmackException::LibsmackError, "smack_accesses_apply");
+
 }
 
-bool SmackRules::loadFromFile(const std::string &path)
+void SmackRules::loadFromFile(const std::string &path)
 {
     int fd;
-    bool ret = true;
 
     fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_RDONLY));
     if (fd == -1) {
         LogError("Failed to open file: " << path);
-        return false;
+        ThrowMsg(SmackException::FileError, "Failed to open file: " << path);
     }
 
     if (smack_accesses_add_from_file(m_handle, fd)) {
         LogError("Failed to load smack rules from file: " << path);
-        ret = false;
+        ThrowMsg(SmackException::LibsmackError, "Failed to load smack rules from file: " << path);
     }
 
     if (close(fd) == -1) {
         // don't change the return code, the descriptor should be closed despite the error.
         LogWarning("Error while closing the file: " << path << ", error: " << strerror(errno));
     }
-
-    return ret;
 }
 
-bool SmackRules::saveToFile(const std::string &path) const
+void SmackRules::saveToFile(const std::string &path) const
 {
     int fd;
-    bool ret = true;
 
     fd = TEMP_FAILURE_RETRY(open(path.c_str(), O_CREAT | O_WRONLY | O_TRUNC, 0644));
     if (fd == -1) {
         LogError("Failed to create file: " << path);
-        return false;
+        ThrowMsg(SmackException::FileError, "Failed to create file: " << path);
     }
 
     if (smack_accesses_save(m_handle, fd)) {
         LogError("Failed to save rules to file: " << path);
         unlink(path.c_str());
-        ret = false;
+        ThrowMsg(SmackException::LibsmackError, "Failed to save rules to file: " << path);
     }
 
     if (close(fd) == -1) {
         if (errno == EIO) {
             LogError("I/O Error occured while closing the file: " << path << ", error: " << strerror(errno));
             unlink(path.c_str());
-            return false;
+            ThrowMsg(SmackException::FileError, "I/O Error occured while closing the file: " << path << ", error: " << strerror(errno));
         } else {
             // non critical error
             // don't change the return code, the descriptor should be closed despite the error.
             LogWarning("Error while closing the file: " << path << ", error: " << strerror(errno));
         }
     }
-
-    return ret;
 }
 
 
-bool SmackRules::addFromTemplateFile(const std::string &appId,
+void SmackRules::addFromTemplateFile(const std::string &appId,
         const std::string &pkgId)
 {
     std::vector<std::string> templateRules;
@@ -146,7 +146,7 @@ bool SmackRules::addFromTemplateFile(const std::string &appId,
 
     if (!templateRulesFile.is_open()) {
         LogError("Cannot open rules template file: " << APP_RULES_TEMPLATE_FILE_PATH);
-        return false;
+        ThrowMsg(SmackException::FileError, "Cannot open rules template file: " << APP_RULES_TEMPLATE_FILE_PATH);
     }
 
     while (std::getline(templateRulesFile, line)) {
@@ -155,13 +155,13 @@ bool SmackRules::addFromTemplateFile(const std::string &appId,
 
     if (templateRulesFile.bad()) {
         LogError("Error reading template file: " << APP_RULES_TEMPLATE_FILE_PATH);
-        return false;
+        ThrowMsg(SmackException::FileError, "Error reading template file: " << APP_RULES_TEMPLATE_FILE_PATH);
     }
 
-    return addFromTemplate(templateRules, appId, pkgId);
+    addFromTemplate(templateRules, appId, pkgId);
 }
 
-bool SmackRules::addFromTemplate(const std::vector<std::string> &templateRules,
+void SmackRules::addFromTemplate(const std::vector<std::string> &templateRules,
         const std::string &appId, const std::string &pkgId)
 {
     for (auto rule : templateRules) {
@@ -174,47 +174,26 @@ bool SmackRules::addFromTemplate(const std::vector<std::string> &templateRules,
 
         if (stream.fail() || !stream.eof()) {
             LogError("Invalid rule template: " << rule);
-            return false;
+            ThrowMsg(SmackException::FileError, "Invalid rule template: " << rule);
         }
 
-        if (subject == SMACK_APP_LABEL_TEMPLATE) {
-            if (!generateAppLabel(appId, subject)) {
-                LogError("Failed to generate app label from appId: " << appId);
-                return false;
-            }
-        }
+        if (subject == SMACK_APP_LABEL_TEMPLATE)
+            subject = SmackLabels::generateAppLabel(appId);
 
-        if (subject == SMACK_PKG_LABEL_TEMPLATE) {
-            if (!generatePkgLabel(pkgId, object)) {
-                LogError("Failed to generate pkg label from pkgid: " << pkgId);
-                return false;
-            }
-        }
+        if (subject == SMACK_PKG_LABEL_TEMPLATE)
+             subject = SmackLabels::generatePkgLabel(pkgId);
 
-        if (object == SMACK_APP_LABEL_TEMPLATE) {
-            if (!generateAppLabel(appId, object)) {
-                LogError("Failed to generate app label from appId: " << appId);
-                return false;
-            }
-        }
+        if (object == SMACK_APP_LABEL_TEMPLATE)
+            object = SmackLabels::generateAppLabel(appId);
 
-        if (object == SMACK_PKG_LABEL_TEMPLATE) {
-            if (!generatePkgLabel(pkgId, object)) {
-                LogError("Failed to generate pkg label from pkgId: " << pkgId);
-                return false;
-            }
-        }
+        if (object == SMACK_PKG_LABEL_TEMPLATE)
+            object = SmackLabels::generatePkgLabel(pkgId);
 
-        if (!add(subject, object, permissions)) {
-            LogError("Failed to add rule: " << subject << " " << object << " " << permissions);
-            return false;
-        }
+        add(subject, object, permissions);
     }
-
-    return true;
 }
 
-bool SmackRules::generatePackageCrossDeps(const std::vector<std::string> &pkgContents)
+void SmackRules::generatePackageCrossDeps(const std::vector<std::string> &pkgContents)
 {
     LogDebug ("Generating cross-package rules");
 
@@ -226,24 +205,12 @@ bool SmackRules::generatePackageCrossDeps(const std::vector<std::string> &pkgCon
             if (object == subject)
                 continue;
 
-            if (generateAppLabel(subject, subjectLabel) && generateAppLabel(object, objectLabel)) {
-                LogDebug ("Trying to add rule subject: " << subjectLabel << " object: " << objectLabel
-                            << " perms: " << appsInPackagePerms);
-                if (!add (subjectLabel, objectLabel, appsInPackagePerms)) {
-                    LogError ("Can't add in-package rule for subject: "
-                                << subject << " and object: " << object);
-                    return false;
-                }
-            }
-            else {
-                LogError ("Failed to created smack labels for subject: "
-                            << subject << " and object: " << object);
-                return false;
-            }
+            subjectLabel = SmackLabels::generateAppLabel(subject);
+            objectLabel = SmackLabels::generateAppLabel(object);
+            LogDebug ("Trying to add rule subject: " << subjectLabel << " object: " << objectLabel << " perms: " << appsInPackagePerms);
+            add(subjectLabel, objectLabel, appsInPackagePerms);
         }
     }
-
-    return true;
 }
 
 std::string SmackRules::getPackageRulesFilePath(const std::string &pkgId)
@@ -258,163 +225,96 @@ std::string SmackRules::getApplicationRulesFilePath(const std::string &appId)
     return path;
 }
 
-bool SmackRules::installApplicationRules(const std::string &appId, const std::string &pkgId,
-        const std::vector<std::string> &pkgContents) {
-    try {
-        SmackRules smackRules;
-        std::string appPath = getApplicationRulesFilePath(appId);
-
-        if (!smackRules.addFromTemplateFile(appId, pkgId)) {
-            LogError("Failed to load smack rules for appId: " << appId << " with pkgId: " << pkgId);
-            return false;
-        }
-
-        if (smack_smackfs_path() != NULL && !smackRules.apply()) {
-            LogError("Failed to apply application rules to kernel [app]");
-            return false;
-        }
+void SmackRules::installApplicationRules(const std::string &appId, const std::string &pkgId,
+        const std::vector<std::string> &pkgContents)
+{
+    SmackRules smackRules;
+    std::string appPath = getApplicationRulesFilePath(appId);
 
-        if (!smackRules.saveToFile(appPath)) {
-            smackRules.clear();
-            return false;
-        }
+    smackRules.addFromTemplateFile(appId, pkgId);
 
-        if (!updatePackageRules(pkgId, pkgContents))
-        {
-            return false;
-        }
+    if (smack_smackfs_path() != NULL)
+        smackRules.apply();
 
-        return true;
-    } catch (const std::bad_alloc &e) {
-        LogError("Out of memory while trying to install smack rules for appId: "
-                << appId << "in pkgId: " << pkgId);
-        return false;
-    }
+    smackRules.saveToFile(appPath);
+    updatePackageRules(pkgId, pkgContents);
 }
 
-bool SmackRules::updatePackageRules(const std::string &pkgId, const std::vector<std::string> &pkgContents)
+void SmackRules::updatePackageRules(const std::string &pkgId, const std::vector<std::string> &pkgContents)
 {
-    try {
-        SmackRules smackRules;
-        std::string pkgPath = getPackageRulesFilePath(pkgId);
-
-        if (!smackRules.generatePackageCrossDeps(pkgContents))
-        {
-            LogError("Failed to create application in-package cross dependencies");
-            return false;
-        }
+    SmackRules smackRules;
+    std::string pkgPath = getPackageRulesFilePath(pkgId);
 
-        if (smack_smackfs_path() != NULL && !smackRules.apply()) {
-             LogError("Failed to apply application rules to kernel [pkg]");
-             return false;
-         }
+    smackRules.generatePackageCrossDeps(pkgContents);
 
-         if (!smackRules.saveToFile(pkgPath)) {
-             smackRules.clear();
-             return false;
-         }
+    if (smack_smackfs_path() != NULL)
+        smackRules.apply();
 
-         return true;
-    } catch (const std::bad_alloc &e) {
-        LogError("Out of memory while trying to install smack rules for pkgId: " << pkgId);
-        return false;
-    }
+    smackRules.saveToFile(pkgPath);
 }
 
 /* FIXME: Remove this function if real pkgId instead of "User" label will be used
  * in generateAppLabel(). */
-bool SmackRules::addMissingRulesFix()
+void SmackRules::addMissingRulesFix()
 {
-    DIR *dir;
     struct dirent *ent;
-    SmackRules rules;
+
     std::string path(tzplatform_mkpath(TZ_SYS_SMACK, "accesses.d"));
+    std::unique_ptr<DIR, std::function<int(DIR*)>> dir(opendir(path.c_str()), closedir);
+    if (!dir)
+        ThrowMsg(SmackException::FileError, "opendir");
 
-    dir = opendir(path.c_str());
-    if (dir != NULL) {
-        while ((ent = readdir(dir))) {
-            if (ent->d_type == DT_REG) {
-                rules.loadFromFile(tzplatform_mkpath3(TZ_SYS_SMACK, "accesses.d/", ent->d_name));
-                // Do not check error here. If this fails we can't do anything anyway.
-            }
+    while ((ent = readdir(dir.get()))) {
+        SmackRules rules;
+        if (ent->d_type == DT_REG) {
+            rules.loadFromFile(tzplatform_mkpath3(TZ_SYS_SMACK, "accesses.d/", ent->d_name));
+            // Do not check error here. If this fails we can't do anything anyway.
         }
         rules.apply();
     }
-    else
-        return false;
-
-    closedir(dir);
-
-    return true;
 }
 
-bool SmackRules::uninstallPackageRules(const std::string &pkgId)
+void SmackRules::uninstallPackageRules(const std::string &pkgId)
 {
-    if (!uninstallRules(getPackageRulesFilePath(pkgId)))
-    {
-        LogError("Failed to uninstall application rules for pkgId: " << pkgId);
-        return false;
-    }
-
-    return true;
+    uninstallRules(getPackageRulesFilePath(pkgId));
 }
 
-bool SmackRules::uninstallApplicationRules(const std::string &appId,
+void SmackRules::uninstallApplicationRules(const std::string &appId,
         const std::string &pkgId, std::vector<std::string> pkgContents)
 {
-    if (!uninstallRules (getApplicationRulesFilePath(appId)))
-    {
-        LogError("Failed to uninstall application rules for appId: " << appId);
-        return false;
-    }
-
-    if (!updatePackageRules(pkgId, pkgContents))
-    {
-        LogError("failed to update package rules for appId: " << appId
-                << " pkgId: " << pkgId);
-        return false;
-    }
+    uninstallRules(getApplicationRulesFilePath(appId));
+    updatePackageRules(pkgId, pkgContents);
 
     // FIXME: Reloading all rules:
     SmackRules::addMissingRulesFix();
-
-    return true;
 }
 
-bool SmackRules::uninstallRules(const std::string &path)
+void SmackRules::uninstallRules(const std::string &path)
 {
     if (access(path.c_str(), F_OK) == -1) {
         if (errno == ENOENT) {
             LogWarning("Smack rules not found in file: " << path);
-            return true;
+            return;
         }
 
         LogWarning("Cannot access smack rules path: " << path);
-        return false;
+        ThrowMsg(SmackException::FileError, "Cannot access smack rules path: " << path);
     }
 
     try {
         SmackRules rules;
-        if (rules.loadFromFile(path)) {
-            if (smack_smackfs_path() != NULL && !rules.clear()) {
-                LogWarning("Failed to clear smack kernel rules from file: " << path);
-                // don't stop uninstallation
-            }
-        } else {
-            LogWarning("Failed to load rules from file: " << path);
-            // don't stop uninstallation
-        }
+        rules.loadFromFile(path);
+        if (smack_smackfs_path())
+            rules.clear();
+    } catch (const SmackException::Base &e) {
+        LogWarning("Failed to clear smack kernel rules from file: " << path);
+        // don't stop uninstallation
+    }
 
-        if (unlink(path.c_str()) == -1) {
-            LogError("Failed to remove smack rules file: " << path);
-            return false;
-        }
-    } catch (const std::bad_alloc &e) {
-        LogError("Out of memory while trying to uninstall smack rules from path: " << path);
-        return false;
+    if (unlink(path.c_str()) == -1) {
+        LogWarning("Failed to remove smack rules file: " << path);
+        ThrowMsg(SmackException::FileError, "Failed to remove smack rules file: " << path);
     }
-    return true;
 }
 
 } // namespace SecurityManager
-