From: Rafal Krypa Date: Wed, 16 Sep 2015 09:03:51 +0000 (+0200) Subject: Rewrite and fix CynaraAdmin::SetPolicies X-Git-Tag: accepted/tizen/mobile/20150920.232640^0 X-Git-Url: http://review.tizen.org/git/?p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git;a=commitdiff_plain;h=d11d29c3f7402482072695dfa47ae80605076d3d Rewrite and fix CynaraAdmin::SetPolicies Method CynaraAdmin::Setpolicies, updating Cynara policy for an application, was previously written to accept two vectors of privileges: previously enabled privileges and privileges that should be enabled. Vectors were used to calculate privileges to disable and privileges to enable in Cynara. It required that both vectors are sorted and without duplicates. Callers of this method fetched privileges from data base, which provides sorting and unification. This was broken in commit 626f947e0bb6fd90d4c20fd914981d5b752ab1e6 (Change smack labeling to be appId based). The second vector was taken directly from application installation request, that wasn't necessarily sorted or unique. This method can be simplified now withot the need for sorted vectors. In fact only one vector is necessarry now, because cynara-admin provides support for listing policies (it didn't when the method was initially written). Now it only takes vector of privileges that should be enabled, in arbitrary order, that may contain duplicates. It lists previously enabled privileges directly from Cynara, calculates the difference and sends updated policies back to Cynara. Change-Id: I15ca331cf5f46ae43c7665977df7eb4d3c7e986c Signed-off-by: Rafal Krypa --- diff --git a/src/common/cynara.cpp b/src/common/cynara.cpp index 30f0777..d1bbef0 100644 --- a/src/common/cynara.cpp +++ b/src/common/cynara.cpp @@ -22,6 +22,7 @@ */ #include +#include #include "cynara.h" #include @@ -286,53 +287,35 @@ void CynaraAdmin::SetPolicies(const std::vector &policies) void CynaraAdmin::UpdateAppPolicy( const std::string &label, const std::string &user, - const std::vector &oldPrivileges, - const std::vector &newPrivileges) + const std::vector &privileges) { - std::vector policies; + std::unordered_set privilegesSet(privileges.begin(), privileges.end()); - // Perform sort-merge join on oldPrivileges and newPrivileges. - // Assume that they are already sorted and without duplicates. - auto oldIter = oldPrivileges.begin(); - auto newIter = newPrivileges.begin(); + std::vector policies; + CynaraAdmin::getInstance().ListPolicies( + CynaraAdmin::Buckets.at(Bucket::MANIFESTS), + label, user, CYNARA_ADMIN_ANY, policies); - while (oldIter != oldPrivileges.end() && newIter != newPrivileges.end()) { - int compare = oldIter->compare(*newIter); - if (compare == 0) { + // Compare previous policies with set of new requested privileges + for (auto &policy : policies) { + if (privilegesSet.erase(policy.privilege)) { + // privilege was found and removed from the set, keeping policy LogDebug("(user = " << user << " label = " << label << ") " << - "keeping privilege " << *newIter); - ++oldIter; - ++newIter; - continue; - } else if (compare < 0) { - LogDebug("(user = " << user << " label = " << label << ") " << - "removing privilege " << *oldIter); - policies.push_back(CynaraAdminPolicy(label, user, *oldIter, - static_cast(CynaraAdminPolicy::Operation::Delete), - Buckets.at(Bucket::MANIFESTS))); - ++oldIter; + "keeping privilege " << policy.privilege); } else { + // privilege was not found in the set, deleting policy + policy.result = static_cast(CynaraAdminPolicy::Operation::Delete); LogDebug("(user = " << user << " label = " << label << ") " << - "adding privilege " << *newIter); - policies.push_back(CynaraAdminPolicy(label, user, *newIter, - static_cast(CynaraAdminPolicy::Operation::Allow), - Buckets.at(Bucket::MANIFESTS))); - ++newIter; + "removing privilege " << policy.privilege); } } - for (; oldIter != oldPrivileges.end(); ++oldIter) { - LogDebug("(user = " << user << " label = " << label << ") " << - "removing privilege " << *oldIter); - policies.push_back(CynaraAdminPolicy(label, user, *oldIter, - static_cast(CynaraAdminPolicy::Operation::Delete), - Buckets.at(Bucket::MANIFESTS))); - } - - for (; newIter != newPrivileges.end(); ++newIter) { + // Add policies for privileges that weren't previously enabled + // Those that were previously enabled are now removed from privilegesSet + for (const auto &privilege : privilegesSet) { LogDebug("(user = " << user << " label = " << label << ") " << - "adding privilege " << *newIter); - policies.push_back(CynaraAdminPolicy(label, user, *newIter, + "adding privilege " << privilege); + policies.push_back(CynaraAdminPolicy(label, user, privilege, static_cast(CynaraAdminPolicy::Operation::Allow), Buckets.at(Bucket::MANIFESTS))); } diff --git a/src/common/include/cynara.h b/src/common/include/cynara.h index aa0ed60..c6a77f3 100644 --- a/src/common/include/cynara.h +++ b/src/common/include/cynara.h @@ -118,26 +118,18 @@ public: void SetPolicies(const std::vector &policies); /** - * Update Cynara policies for the package and the user, using two vectors - * of privileges: privileges set before (and already enabled in Cynara) - * and new privileges, to be set in Cynara. + * Update Cynara policies for the application and the user. * Difference will be calculated, removing old unneeded privileges and * adding new, previously not enabled privileges. * Caller must have permission to access Cynara administrative socket. * * @param label application Smack label * @param user user identifier - * @param oldPrivileges previously enabled privileges for the package. - * Must be sorted and without duplicates. - * @param newPrivileges currently enabled privileges for the package. - * Must be sorted and without duplicates. + * @param privileges currently enabled privileges * - * TODO: drop oldPrivileges argument and get them directly from Cynara. - * Appropriate Cynara interface is needed first. */ void UpdateAppPolicy(const std::string &label, const std::string &user, - const std::vector &oldPrivileges, - const std::vector &newPrivileges); + const std::vector &privileges); /** * Depending on user type, create link between MAIN bucket and appropriate diff --git a/src/common/include/master-req.h b/src/common/include/master-req.h index 9890728..15b1fdc 100644 --- a/src/common/include/master-req.h +++ b/src/common/include/master-req.h @@ -39,16 +39,12 @@ namespace MasterReq { * * @param[in] appID Application ID * @param[in] uidstr String containing user identifier - * @param[in] oldPkgPrivileges Previously enabled privileges for the package, - * Must be sorted and without duplicates - * @param[in] newPkgPrivileges Currently enabled privileges for the package, - * Must be sorted and without duplicates + * @param[in] privileges Currently enabled privileges for the application * * @see CynaraAdmin::UpdateAppPolicy */ int CynaraPolicyUpdate(const std::string &appId, const std::string &uidstr, - const std::vector &oldPkgPrivileges, - const std::vector &newPkgPrivileges); + const std::vector &privileges); /** * Forwards Cynara user initialization to Master service. diff --git a/src/common/master-req.cpp b/src/common/master-req.cpp index f6526b3..b2c2783 100644 --- a/src/common/master-req.cpp +++ b/src/common/master-req.cpp @@ -32,16 +32,15 @@ namespace SecurityManager { namespace MasterReq { -int CynaraPolicyUpdate(const std::string &appId, const std::string &uidstr, - const std::vector &oldPkgPrivileges, - const std::vector &newPkgPrivileges) +int CynaraPolicyUpdate(const std::string &appId, const std::string &uidstr, + const std::vector &privileges) { int ret; MessageBuffer sendBuf, retBuf; Serialization::Serialize(sendBuf, static_cast(MasterSecurityModuleCall::CYNARA_UPDATE_POLICY), - appId, uidstr, oldPkgPrivileges, newPkgPrivileges); + appId, uidstr, privileges); ret = sendToServer(MASTER_SERVICE_SOCKET, sendBuf.Pop(), retBuf); if (ret == SECURITY_MANAGER_API_SUCCESS) diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index 469f6a4..fb0a0e6 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -292,8 +292,6 @@ int ServiceImpl::appInstall(const app_inst_req &req, uid_t uid, bool isSlave) } try { - std::vector oldAppPrivileges; - appLabel = zoneSmackLabelGenerate(SmackLabels::generateAppLabel(req.appId), zoneId); /* NOTE: we don't use pkgLabel here, but generate it for pkgId validation */ pkgLabel = zoneSmackLabelGenerate(SmackLabels::generatePkgLabel(req.pkgId), zoneId); @@ -309,23 +307,21 @@ int ServiceImpl::appInstall(const app_inst_req &req, uid_t uid, bool isSlave) PrivilegeDb::getInstance().RollbackTransaction(); return SECURITY_MANAGER_API_ERROR_INPUT_PARAM; } - PrivilegeDb::getInstance().GetAppPrivileges(req.appId, uid, oldAppPrivileges); + PrivilegeDb::getInstance().AddApplication(req.appId, req.pkgId, 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); if (isSlave) { - int ret = MasterReq::CynaraPolicyUpdate(req.appId, uidstr, oldAppPrivileges, - req.privileges); + int ret = MasterReq::CynaraPolicyUpdate(req.appId, uidstr, req.privileges); if (ret != SECURITY_MANAGER_API_SUCCESS) { PrivilegeDb::getInstance().RollbackTransaction(); LogError("Error while processing request on master: " << ret); return ret; } } else { - CynaraAdmin::getInstance().UpdateAppPolicy(appLabel, uidstr, oldAppPrivileges, - req.privileges); + CynaraAdmin::getInstance().UpdateAppPolicy(appLabel, uidstr, req.privileges); } PrivilegeDb::getInstance().CommitTransaction(); @@ -408,8 +404,6 @@ int ServiceImpl::appUninstall(const std::string &appId, uid_t uid, bool isSlave) } try { - std::vector oldAppPrivileges; - PrivilegeDb::getInstance().BeginTransaction(); if (!PrivilegeDb::getInstance().GetAppPkgId(appId, pkgId)) { LogWarning("Application " << appId << @@ -425,21 +419,18 @@ int ServiceImpl::appUninstall(const std::string &appId, uid_t uid, bool isSlave) that this app belongs to, this will allow us to remove all rules withing the package that the app appears in */ PrivilegeDb::getInstance().GetAppIdsForPkgId(pkgId, pkgContents); - PrivilegeDb::getInstance().GetAppPrivileges(appId, uid, oldAppPrivileges); PrivilegeDb::getInstance().UpdateAppPrivileges(appId, uid, std::vector()); PrivilegeDb::getInstance().RemoveApplication(appId, uid, removePkg); if (isSlave) { - int ret = MasterReq::CynaraPolicyUpdate(appId, uidstr, oldAppPrivileges, - std::vector()); + int ret = MasterReq::CynaraPolicyUpdate(appId, uidstr, std::vector()); if (ret != SECURITY_MANAGER_API_SUCCESS) { PrivilegeDb::getInstance().RollbackTransaction(); LogError("Error while processing request on master: " << ret); return ret; } } else { - CynaraAdmin::getInstance().UpdateAppPolicy(smackLabel, uidstr, oldAppPrivileges, - std::vector()); + CynaraAdmin::getInstance().UpdateAppPolicy(smackLabel, uidstr, std::vector()); } PrivilegeDb::getInstance().CommitTransaction(); @@ -883,6 +874,7 @@ int ServiceImpl::getPolicy(const policy_entry &filter, uid_t uid, pid_t pid, con std::vector listOfPrivileges; // FIXME: also fetch privileges of global applications + // FIXME: fetch privileges from cynara, drop PrivilegeDb::GetAppPrivileges PrivilegeDb::getInstance().GetAppPrivileges(appId, uid, listOfPrivileges); if (filter.privilege.compare(SECURITY_MANAGER_ANY)) { diff --git a/src/server/service/master-service.cpp b/src/server/service/master-service.cpp index 2440419..770820e 100644 --- a/src/server/service/master-service.cpp +++ b/src/server/service/master-service.cpp @@ -170,19 +170,17 @@ void MasterService::processCynaraUpdatePolicy(MessageBuffer &buffer, MessageBuff int ret = SECURITY_MANAGER_API_ERROR_SERVER_ERROR; std::string appId; std::string uidstr; - std::vector oldAppPrivileges, newAppPrivileges; - std::string appLabel, newLabel; + std::string appLabel; + std::vector privileges; Deserialization::Deserialize(buffer, appId); Deserialization::Deserialize(buffer, uidstr); - Deserialization::Deserialize(buffer, oldAppPrivileges); - Deserialization::Deserialize(buffer, newAppPrivileges); + Deserialization::Deserialize(buffer, privileges); appLabel = zoneSmackLabelGenerate(SmackLabels::generateAppLabel(appId), zoneId); try { - CynaraAdmin::getInstance().UpdateAppPolicy(appLabel, uidstr, oldAppPrivileges, - newAppPrivileges); + CynaraAdmin::getInstance().UpdateAppPolicy(appLabel, uidstr, privileges); } catch (const CynaraException::Base &e) { LogError("Error while setting Cynara rules for application: " << e.DumpToString()); goto out;