Change privilege related Smack rules on cynara policy change 84/229384/11
authorZofia Abramowska <z.abramowska@samsung.com>
Tue, 7 Apr 2020 15:30:03 +0000 (17:30 +0200)
committerZofia Abramowska <z.abramowska@samsung.com>
Mon, 20 Apr 2020 10:38:46 +0000 (12:38 +0200)
When policy is updated recalculate privilege related Smack rules
for all running applications.

Change-Id: Ic6a0341399186d10404f1ce189217d963707e7be

packaging/security-manager.spec
src/client/CMakeLists.txt
src/common/CMakeLists.txt
src/common/include/service_impl.h
src/common/include/smack-rules.h
src/common/service_impl.cpp
src/common/smack-rules.cpp
src/server/CMakeLists.txt

index 0d50227..d489f22 100644 (file)
@@ -37,6 +37,7 @@ BuildRequires: pkgconfig(cynara-client-async)
 BuildRequires: pkgconfig(security-privilege-manager)
 BuildRequires: pkgconfig(openssl1.1)
 BuildRequires: pkgconfig(mount)
+BuildRequires: pkgconfig(capi-appfw-app-manager)
 BuildRequires: boost-devel
 %{?systemd_requires}
 
index 8862ca0..4dfe61c 100644 (file)
@@ -1,5 +1,6 @@
 PKG_CHECK_MODULES(CLIENT_DEP
     REQUIRED
+    capi-appfw-app-manager
     cynara-client-async
     libsmack
     libcap
index 257f0a9..b37a53e 100644 (file)
@@ -11,6 +11,7 @@ PKG_CHECK_MODULES(COMMON_DEP
     libtzplatform-config
     security-privilege-manager
     mount
+    capi-appfw-app-manager
     )
 
 IF(DPL_WITH_DLOG)
index 33a3d1d..ff98d51 100644 (file)
@@ -28,6 +28,8 @@
 
 #include <vector>
 
+#include <app_context.h>
+
 #include "channel.h"
 #include "credentials.h"
 #include "cynara.h"
@@ -440,6 +442,7 @@ private:
                               const Smack::Label &targetAppLabel,
                               const std::string &path);
 
+    static bool updateRunningAppSmackPolicy(app_context_h app_context, void *user_data);
     void updatePermissibleSet(uid_t uid, int type);
 
     Smack::Label getAppProcessLabel(const std::string &appName, const std::string &pkgName);
@@ -458,6 +461,7 @@ private:
     CynaraAdmin m_cynaraAdmin;
     PrivilegeGids m_privilegeGids;
     NSMountLogic m_NSMountLogic;
+    std::map<std::string, uid_t> m_appIdUidMap;
 };
 
 } /* namespace SecurityManager */
index 189611a..35f6d8d 100644 (file)
@@ -99,6 +99,23 @@ public:
         int authorId);
 
     /**
+     * Disable privilege-specific smack rules for given application
+     *
+     * Function creates privilege-specific smack rules using predefined templates.
+     * Rules are cleared from the kernel.
+     *
+     * @param[in] appProcessLabel - process label of the application
+     * @param[in] pkgName - package id of given application
+     * @param[in] authorId - author id of given application (if not applicable, set to -1)
+     * @param[in] privileges - a list of privileges to be disabled for given application
+     */
+    void disablePrivilegeRules(
+        const Smack::Label &appProcessLabel,
+        const std::string &pkgName,
+        int authorId,
+        const std::vector<std::string> &privileges);
+
+    /**
      * Uninstall package-specific smack rules.
      *
      * Function loads package-specific smack rules, revokes them from the kernel.
index e3186f8..a1c145a 100644 (file)
 #include <dirent.h>
 #include <cstring>
 #include <algorithm>
+#include <memory>
 
 #include <dpl/log/log.h>
 #include <dpl/errno_string.h>
 
+#include <app_manager.h>
 #include <sys/smack.h>
 
 #include <config.h>
@@ -1081,6 +1083,80 @@ int ServiceImpl::userDelete(const Credentials &creds, uid_t uidDeleted)
 
     return ret;
 }
+typedef std::map<std::string, std::vector<std::string>> AppsAllowedPrivilegesMap;
+
+namespace {
+
+struct AppManagerCallbackContext {
+    ServiceImpl *impl;
+    AppsAllowedPrivilegesMap *appsAllowedPrivileges;
+};
+
+std::string getAppIdFromContext(app_context_h app_context)
+{
+    char *appId;
+    int ret = app_context_get_app_id (app_context, &appId);
+    if (ret != APP_MANAGER_ERROR_NONE) {
+        LogError("Failed to get appId from app_context: " << ret);
+        return "";
+    }
+    std::unique_ptr<char, decltype(std::free) *> appIdPtr(appId, std::free);
+    std::string appIdStr(appId);
+    appIdPtr.release();
+    return appIdStr;
+}
+
+} // namespace anonymous
+
+bool ServiceImpl::updateRunningAppSmackPolicy(app_context_h app_context, void *user_data) {
+    AppManagerCallbackContext *context = reinterpret_cast<AppManagerCallbackContext*>(user_data);
+
+    std::string appId = getAppIdFromContext(app_context);
+    if (appId.empty()) {
+        LogDebug("No appId from running context");
+        return true;
+    }
+
+    LogDebug("Found " << appId << " running");
+
+    std::string pkgName;
+    context->impl->getPkgName(appId, pkgName);
+
+    bool isPkgHybrid = context->impl->m_privilegeDb.IsPackageHybrid(pkgName);
+
+    auto oldAllowedPrivs = context->appsAllowedPrivileges->operator[](appId);
+
+    auto it = context->impl->m_appIdUidMap.find(appId);
+    if (it == context->impl->m_appIdUidMap.end()) {
+        LogError("THIS IS VERY VERY BAD. RUNNING APPLICATION HAS NO SAVED UID!!!");
+        return true;
+    }
+    uid_t uid = it->second;
+    std::vector<std::string> newAllowedPrivs;
+    auto appProcessLabel = SmackLabels::generateProcessLabel(appId, pkgName, isPkgHybrid);
+    context->impl->getAppAllowedPrivileges(appProcessLabel, uid, newAllowedPrivs);
+
+    std::vector<std::string> inter;
+    std::sort(oldAllowedPrivs.begin(), oldAllowedPrivs.end());
+    std::sort(newAllowedPrivs.begin(), newAllowedPrivs.end());
+
+    std::vector<std::string> denied, allowed;
+    // Deny privileges which were in old allowed privs, but are not in new ones
+    std::set_difference(oldAllowedPrivs.begin(), oldAllowedPrivs.end(),
+                        newAllowedPrivs.begin(), newAllowedPrivs.end(),
+                        std::back_inserter(denied));
+    // Allow privileges which are in new allowed privs, but were not in old ones
+    std::set_difference(newAllowedPrivs.begin(), newAllowedPrivs.end(),
+                        oldAllowedPrivs.begin(), oldAllowedPrivs.end(),
+                        std::back_inserter(allowed));
+    int authorId;
+    context->impl->m_privilegeDb.GetPkgAuthorId(pkgName, authorId);
+
+    context->impl->m_smackRules.disablePrivilegeRules(appProcessLabel, pkgName, authorId, denied);
+    context->impl->m_smackRules.enablePrivilegeRules(appProcessLabel, pkgName, authorId, allowed);
+
+    return true;
+}
 
 int ServiceImpl::policyUpdate(const Credentials &creds, const std::vector<policy_entry> &policyEntries)
 {
@@ -1101,9 +1177,44 @@ int ServiceImpl::policyUpdate(const Credentials &creds, const std::vector<policy
             validatedPolicies.push_back(std::move(cyap));
         };
 
+        std::map<std::string, std::vector<std::string>> appsAllowedPrivileges;
+        std::vector<std::string> allPackages;
+
+        // TODO: Can this be optimized to: if policy entries contain either wildcard
+        // in place of privilege or if any of the privileges has smack mapping?
+        if (m_smackRules.isPrivilegeMappingEnabled()) {
+            // Get policy of all installed application so we can recalculate it after update
+            m_privilegeDb.GetAllPackages(allPackages);
+            for (auto &package : allPackages) {
+                std::vector<std::string> packageApps;
+                m_privilegeDb.GetPkgApps(package, packageApps);
+                bool isPkgHybrid = m_privilegeDb.IsPackageHybrid(package);
+                for (auto &app : packageApps) {
+                    auto it = m_appIdUidMap.find(app);
+                    if (it == m_appIdUidMap.end()) {
+                        // TODO: Move app -> uid mapping to database
+                        LogDebug("App " << app << " has never been run in security-manager instance");
+                        continue;
+                    }
+                    uid_t uid = it->second;
+                    auto appProcessLabel = SmackLabels::generateProcessLabel(app, package, isPkgHybrid);
+                    getAppAllowedPrivileges(appProcessLabel, uid, appsAllowedPrivileges[app]);
+                }
+            }
+        }
         // Apply updates
         m_cynaraAdmin.setPolicies(validatedPolicies);
 
+        if (m_smackRules.isPrivilegeMappingEnabled()) {
+            LogDebug("Updating privilege related Smack rules");
+            AppManagerCallbackContext context{this, &appsAllowedPrivileges};
+            int ret = app_manager_foreach_running_app_context(&ServiceImpl::updateRunningAppSmackPolicy,
+                                                              &context);
+            if (ret != APP_MANAGER_ERROR_NONE) {
+                LogError("Failed to update Smack policy for running apps");
+            }
+        }
+
         // Update mount namespaces
         // TODO: Don't update all users, apps and privileges, filter by policyEntries
         if (!m_NSMountLogic.check())
@@ -2148,6 +2259,7 @@ int ServiceImpl::prepareApp(const Credentials &creds, const std::string &appName
         std::vector<gid_t> &forbiddenGroups, std::vector<gid_t> &allowedGroups, std::vector<bool> &privPathsStatusVector)
 {
     LogDebug("Requested prepareApp for application " << appName);
+
     bool isHybrid;
     if (!m_privilegeDb.GetAppPkgInfo(appName, pkgName, isHybrid, enabledSharedRO))
         return SECURITY_MANAGER_ERROR_UNKNOWN;
@@ -2167,6 +2279,8 @@ int ServiceImpl::prepareApp(const Credentials &creds, const std::string &appName
     getPkgLabels(pkgName, pkgLabels);
 
     if (m_smackRules.isPrivilegeMappingEnabled()) {
+        m_appIdUidMap[appName] = creds.uid;
+
         m_smackRules.disableAllPrivilegeRules(label, pkgName, authorId);
         m_smackRules.enablePrivilegeRules(label, pkgName, authorId, allowedPrivileges);
     }
index ccba2e8..c205865 100644 (file)
@@ -237,15 +237,29 @@ void SmackRules::enablePrivilegeRules(
 }
 
 void SmackRules::disableAllPrivilegeRules(
+    const Smack::Label &appProcessLabel,
+    const std::string &pkgName,
+    int authorId)
+{
+    LogDebug("Disabling all privilege rules for " << appProcessLabel);
+    disablePrivilegeRules(appProcessLabel, pkgName, authorId, m_templateMgr.getAllMappedPrivs());
+}
+
+void SmackRules::disablePrivilegeRules(
         const Smack::Label &appProcessLabel,
         const std::string &pkgName,
-        int authorId)
+        int authorId,
+        const std::vector<std::string> &privileges)
 {
-    LogDebug("Disabling all privilege rules for " << appProcessLabel);
+    if (privileges.empty()) {
+        // No rules to apply
+        return;
+    }
+
+    LogDebug("Disabling privilege rules for " << appProcessLabel);
     SmackAccesses smackRules;
 
-    addPrivilegesRules(smackRules, appProcessLabel, pkgName, authorId,
-                       m_templateMgr.getAllMappedPrivs());
+    addPrivilegesRules(smackRules, appProcessLabel, pkgName, authorId, privileges);
 
     if (smack_check())
         smackRules.clear();
index 7f0ce3f..97f596e 100644 (file)
@@ -1,6 +1,7 @@
 PKG_CHECK_MODULES(SERVER_DEP
     REQUIRED
     libsystemd
+    capi-appfw-app-manager
     cynara-client
     mount
     )