From: Konrad Lipinski Date: Thu, 19 Dec 2019 14:44:33 +0000 (+0100) Subject: prepare_app: refactor supplementary group assignment X-Git-Tag: submit/tizen/20200123.073443~2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=132ab9fca9af38f8aa4512d00ae2c685e07f9ffc;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git prepare_app: refactor supplementary group assignment * use a stack array for syscalls * stream forbiddenGroups = privilegedGroups \ allowedGroups instead of privilegedGroups, making IPC thinner Change-Id: I343af0052fd90f1ed4fd37d41b7b8c7a1a5a7858 --- diff --git a/src/client/client-security-manager.cpp b/src/client/client-security-manager.cpp index 38c65c13..635fdcb9 100644 --- a/src/client/client-security-manager.cpp +++ b/src/client/client-security-manager.cpp @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -452,46 +451,8 @@ int security_manager_set_process_label_from_appid(const char *app_name) return SECURITY_MANAGER_SUCCESS; } -static int getProcessGroups(std::vector &groups) -{ - int ret = getgroups(0, nullptr); - if (ret == -1) { - LogError("Unable to get number of current supplementary groups: " << - GetErrnoString(errno)); - return SECURITY_MANAGER_ERROR_UNKNOWN; - } - int cnt = ret; - - auto groupsPtr = makeUnique(cnt); - if (!groupsPtr) { - LogError("Memory allocation failed."); - return SECURITY_MANAGER_ERROR_MEMORY; - } - - ret = getgroups(cnt, groupsPtr.get()); - if (ret == -1) { - LogError("Unable to get list of current supplementary groups: " << - GetErrnoString(errno)); - return SECURITY_MANAGER_ERROR_UNKNOWN; - } - - groups.assign(groupsPtr.get(), groupsPtr.get() + cnt); - return SECURITY_MANAGER_SUCCESS; -} - -static int setProcessGroups(const std::vector &groups) -{ - int ret = setgroups(groups.size(), groups.data()); - if (ret == -1) { - LogError("Unable to set list of current supplementary groups: " << - GetErrnoString(errno)); - return SECURITY_MANAGER_ERROR_UNKNOWN; - } - - return SECURITY_MANAGER_SUCCESS; -} - -static int getPrivilegedAndAppGroups(const std::string &appName, std::vector &privilegedGroups, std::vector &appGroups) +// respective group vectors come sorted +static int fetchForbiddenAndAllowedGroups(const std::string &appName, std::vector &forbiddenGroups, std::vector &allowedGroups) { ClientRequest request(SecurityModuleCall::GROUPS_GET); @@ -500,12 +461,12 @@ static int getPrivilegedAndAppGroups(const std::string &appName, std::vector &privilegedGroups, std::vector &appGroups, + std::string &pkgName, bool &enabledSharedRO, std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privilegeStatusVector) { ClientRequest request(SecurityModuleCall::PREPARE_APP); @@ -514,7 +475,7 @@ static int prepareAppInitialSetupAndFetch(const std::string &appName, const Moun return request.getStatus(); } - request.recv(privilegedGroups, appGroups, privilegeStatusVector, label, pkgName, enabledSharedRO); + request.recv(forbiddenGroups, allowedGroups, privilegeStatusVector, label, pkgName, enabledSharedRO); return SECURITY_MANAGER_SUCCESS; } @@ -697,33 +658,74 @@ err: return SECURITY_MANAGER_ERROR_UNKNOWN; } -static int security_manager_set_process_groups_internal(const std::vector &privilegedGroups, std::vector &allowedGroups) +// respective group vectors come sorted +static int security_manager_set_process_groups_internal(const std::vector &forbiddenGroups, const std::vector &allowedGroups) { - using namespace SecurityManager; + const size_t forbiddenGroupsSize = forbiddenGroups.size(); + const size_t allowedGroupsSize = allowedGroups.size(); + size_t fi = 0, ai = 0, size = 0; + gid_t forbg = -1, allog = -1; + int ret; + gid_t grp[NGROUPS_MAX+1]; - return try_catch([&]() -> int { + ret = getgroups(sizeof grp / sizeof *grp, grp); + if (ret < 0) { + LogError("Unable to get list of current supplementary groups: " << + GetErrnoString(errno)); + return SECURITY_MANAGER_ERROR_UNKNOWN; + } - std::vector currentGroups; - int ret = getProcessGroups(currentGroups); - if (ret != SECURITY_MANAGER_SUCCESS) - return ret; - LogDebug("Current supplementary groups count: " << currentGroups.size()); + LogDebug("Current supplementary groups count: " << ret); + LogDebug("All privileged supplementary groups count: " << forbiddenGroupsSize); + LogDebug("Allowed privileged supplementary groups count: " << allowedGroupsSize); + + std::sort(grp, grp+ret); + + if (forbiddenGroupsSize) + forbg = forbiddenGroups[fi++]; + if (allowedGroupsSize) + allog = allowedGroups[ai++]; + + // remove grp elements already in either forbiddenGroups or allowedGroups, in place + for (int i = 0; i < ret; i++) { + const auto outg = grp[i]; - LogDebug("All privileged supplementary groups count: " << privilegedGroups.size()); - LogDebug("Allowed privileged supplementary groups count: " << allowedGroups.size()); + using ugid_t = std::make_unsigned_t; - std::unordered_set groupsSet(currentGroups.begin(), currentGroups.end()); // Remove all groups that are mapped to privileges, so if app is not granted // the privilege, the group will be dropped from current process - for (gid_t group : privilegedGroups) - groupsSet.erase(group); + while (ugid_t(forbg) < ugid_t(outg)) + forbg = fi < forbiddenGroupsSize ? forbiddenGroups[fi++] : -1; + if (outg == forbg) + continue; + + // Skip allowed groups too - they'll be added at the end + while (ugid_t(allog) < ugid_t(outg)) + allog = ai < allowedGroupsSize ? allowedGroups[ai++] : -1; + if (outg == allog) + continue; + + grp[size++] = outg; + } + + if (size + allowedGroupsSize > NGROUPS_MAX) { + LogError("Too many supplementary groups"); + return SECURITY_MANAGER_ERROR_UNKNOWN; + } - // Re-add those privileged groups that an app is entitled to - groupsSet.insert(allowedGroups.begin(), allowedGroups.end()); - LogDebug("Final supplementary groups count: " << groupsSet.size()); + // Append allowedGroups + memcpy(grp + size, allowedGroups.data(), allowedGroupsSize * sizeof *grp); + size += allowedGroupsSize; - return setProcessGroups(std::vector(groupsSet.begin(), groupsSet.end())); - }); + LogDebug("Final supplementary groups count: " << size); + + if (setgroups(size, grp) < 0) { + LogError("Unable to set list of current supplementary groups: " << + GetErrnoString(errno)); + return SECURITY_MANAGER_ERROR_UNKNOWN; + } + + return SECURITY_MANAGER_SUCCESS; } SECURITY_MANAGER_API @@ -741,14 +743,14 @@ int security_manager_set_process_groups_from_appid(const char *app_name) return SECURITY_MANAGER_ERROR_INPUT_PARAM; } - std::vector privilegedGroups, allowedGroups; - int ret = getPrivilegedAndAppGroups(app_name, privilegedGroups, allowedGroups); + std::vector forbiddenGroups, allowedGroups; + int ret = fetchForbiddenAndAllowedGroups(app_name, forbiddenGroups, allowedGroups); if (ret != SECURITY_MANAGER_SUCCESS) { LogError("Failed to generate smack label for appName: " << app_name); return ret; } - return security_manager_set_process_groups_internal(privilegedGroups, allowedGroups); + return security_manager_set_process_groups_internal(forbiddenGroups, allowedGroups); }); } @@ -902,17 +904,17 @@ int security_manager_prepare_app(const char *app_name) std::string appLabel, pkgName; bool enabledSharedRO; - std::vector privilegedGroups, allowedGroups; + std::vector forbiddenGroups, allowedGroups; std::vector privilegeStatusVector; auto privilegePathMap = MountNS::getPrivilegePathMap(getuid()); - int ret = prepareAppInitialSetupAndFetch(app_name, privilegePathMap, appLabel, pkgName, enabledSharedRO, privilegedGroups, - allowedGroups, privilegeStatusVector); + int ret = prepareAppInitialSetupAndFetch(app_name, privilegePathMap, appLabel, pkgName, enabledSharedRO, + forbiddenGroups, allowedGroups, privilegeStatusVector); if (ret != SECURITY_MANAGER_SUCCESS) { LogError("Failed to get app info for appName: " << app_name); return ret; } - ret = security_manager_set_process_groups_internal(privilegedGroups, allowedGroups); + ret = security_manager_set_process_groups_internal(forbiddenGroups, allowedGroups); if (ret != SECURITY_MANAGER_SUCCESS) { LogError("Unable to setup process groups for application " << app_name); return ret; diff --git a/src/common/include/privilege-gids.h b/src/common/include/privilege-gids.h index d4c89881..65bfd889 100644 --- a/src/common/include/privilege-gids.h +++ b/src/common/include/privilege-gids.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2017-2020 Samsung Electronics Co., Ltd. All rights reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,14 +39,14 @@ public: // get gids by privilege const std::vector& getGids(const std::string& privilege) const; - // get all gids + // get all gids (sorted) const std::vector& getGids() const; // get all privileges const std::vector& getPrivileges() const; private: std::unordered_map> m_priv2gids; - std::vector m_gids; + std::vector m_gids; // sorted std::vector m_privileges; }; diff --git a/src/common/include/service_impl.h b/src/common/include/service_impl.h index 7fd917cd..80c0a825 100644 --- a/src/common/include/service_impl.h +++ b/src/common/include/service_impl.h @@ -183,13 +183,13 @@ public: * * @param[in] creds credentials of the requesting process * @param[in] appProcessLabel application name - * @param[out] privilegedGroups returned vector of privileged groups - * @param[out] appGroups returned vector of allowed groups + * @param[out] forbiddenGroups returned sorted vector of forbidden groups + * @param[out] allowedGroups returned sorted vector of allowed groups * * @return API return code, as defined in protocols.h */ - int getPrivilegedAndAppGroups(const Credentials &creds, const std::string &appProcessLabel, - std::vector &privilegedGroups, std::vector &appGroups); + int getForbiddenAndAllowedGroups(const Credentials &creds, const std::string &appProcessLabel, + std::vector &forbiddenGroups, std::vector &allowedGroups); /** * Receive groups connected with uid and add them @@ -360,15 +360,15 @@ public: * @param[out] label generated label * @param[out] pkgName application package name * @param[out] enabledSharedRO placeholder for check shared_ro result - * @param[out] privilegedGroups returned vector of privileged groups - * @param[out] appGroups returned vector of allowed groups - * @param[out] privilegeStatusVector returned result of privilege queries + * @param[out] forbiddenGroups sorted vector of forbidden groups + * @param[out] allowedGroups sorted vector of allowed groups + * @param[out] privilegeStatusVector results of respective privilege queries * * @return API return code, as defined in protocols.h */ int prepareApp(const Credentials &creds, const std::string &appName, const std::vector &privilegeVector, std::string &label, std::string &pkgName, bool &enabledSharedRO, - std::vector &privilegedGroups, std::vector &appGroups, std::vector &privilegeStatusVector); + std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privilegeStatusVector); private: int appInstallInitialChecks(const Credentials &creds, diff --git a/src/common/privilege-gids.cpp b/src/common/privilege-gids.cpp index 6dfc0d47..0ba427d3 100644 --- a/src/common/privilege-gids.cpp +++ b/src/common/privilege-gids.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017 Samsung Electronics Co., Ltd All Rights Reserved + * Copyright (c) 2017-2020 Samsung Electronics Co., Ltd. All rights reserved * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -52,7 +52,7 @@ void PrivilegeGids::init(const GroupPrivileges &group_privs) } // remove duplicates - vectorRemoveDuplicates(m_gids); + vectorRemoveDuplicates(m_gids); // sorted vectorRemoveDuplicates(m_privileges); } @@ -68,7 +68,7 @@ const std::vector& PrivilegeGids::getGids(const std::string &privilege) c const std::vector& PrivilegeGids::getGids() const { - return m_gids; + return m_gids; // sorted } const std::vector& PrivilegeGids::getPrivileges() const diff --git a/src/common/service_impl.cpp b/src/common/service_impl.cpp index ac5eb929..3d5e0cb8 100644 --- a/src/common/service_impl.cpp +++ b/src/common/service_impl.cpp @@ -1401,15 +1401,12 @@ int ServiceImpl::policyGetDesc(std::vector &levels) return ret; } -int ServiceImpl::getPrivilegedAndAppGroups(const Credentials &creds, const std::string &appProcessLabel, - std::vector &privilegedGroups, std::vector &appGroups) +int ServiceImpl::getForbiddenAndAllowedGroups(const Credentials &creds, const std::string &appProcessLabel, + std::vector &forbiddenGroups, std::vector &allowedGroups) { try { LogDebug("smack label: " << appProcessLabel); - auto &gids = m_privilegeGids.getGids(); - privilegedGroups.insert(privilegedGroups.end(), gids.begin(), gids.end()); - std::vector privileges; std::string uidStr = std::to_string(creds.uid); @@ -1430,13 +1427,18 @@ int ServiceImpl::getPrivilegedAndAppGroups(const Credentials &creds, const std:: continue; if (m_cynara.check(appProcessLabel, privilege, uidStr, pidStr)) { - appGroups.insert(appGroups.end(), pgids.begin(), pgids.end()); + allowedGroups.insert(allowedGroups.end(), pgids.begin(), pgids.end()); LogDebug("Cynara allowed, adding groups"); } else { LogDebug("Cynara denied, not adding groups"); } } - vectorRemoveDuplicates(appGroups); + vectorRemoveDuplicates(allowedGroups); // sorted + + auto &gids = m_privilegeGids.getGids(); // sorted + forbiddenGroups.reserve(gids.size()); + std::set_difference(gids.begin(), gids.end(), allowedGroups.begin(), allowedGroups.end(), + std::back_inserter(forbiddenGroups)); // sorted } catch (const CynaraException::Base &e) { LogError("Error while querying Cynara for permissions: " << e.DumpToString()); return SECURITY_MANAGER_ERROR_SERVER_ERROR; @@ -2125,7 +2127,7 @@ std::string ServiceImpl::getProcessLabel(const std::string &appName) int ServiceImpl::prepareApp(const Credentials &creds, const std::string &appName, const std::vector &privilegeVector, std::string &label, std::string &pkgName, bool &enabledSharedRO, - std::vector &privilegedGroups, std::vector &appGroups, std::vector &privilegeStatusVector) + std::vector &forbiddenGroups, std::vector &allowedGroups, std::vector &privilegeStatusVector) { LogDebug("Requested prepareApp for application " << appName); bool isHybrid; @@ -2133,7 +2135,7 @@ int ServiceImpl::prepareApp(const Credentials &creds, const std::string &appName return SECURITY_MANAGER_ERROR_UNKNOWN; label = SmackLabels::generateProcessLabel(appName, pkgName, isHybrid); - int ret = getPrivilegedAndAppGroups(creds, label, privilegedGroups, appGroups); + int ret = getForbiddenAndAllowedGroups(creds, label, forbiddenGroups, allowedGroups); return ret != SECURITY_MANAGER_SUCCESS ? ret : appSetupNamespace(creds, label, privilegeVector, privilegeStatusVector); } diff --git a/src/server/service/include/service.h b/src/server/service/include/service.h index 4935f4e3..3834e8e0 100644 --- a/src/server/service/include/service.h +++ b/src/server/service/include/service.h @@ -157,7 +157,7 @@ private: * @param send Raw data buffer to be sent * @param creds credentials of the requesting process */ - void processGetPrivilegedAndAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds); + void processGetForbiddenAndAllowedGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds); /** * Process getting groups bound with privileges for given uid diff --git a/src/server/service/service.cpp b/src/server/service/service.cpp index 5cf78383..e29a8075 100644 --- a/src/server/service/service.cpp +++ b/src/server/service/service.cpp @@ -124,7 +124,7 @@ bool Service::processOne(const ConnectionID &conn, MessageBuffer &buffer, break; case SecurityModuleCall::GROUPS_GET: LogDebug("call_type: SecurityModuleCall::GROUPS_GET"); - processGetPrivilegedAndAppGroups(buffer, send, creds); + processGetForbiddenAndAllowedGroups(buffer, send, creds); break; case SecurityModuleCall::GROUPS_FOR_UID: processGroupsForUid(buffer, send); @@ -345,16 +345,16 @@ void Service::processPolicyGetDesc(MessageBuffer &send) } } -void Service::processGetPrivilegedAndAppGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds) +void Service::processGetForbiddenAndAllowedGroups(MessageBuffer &buffer, MessageBuffer &send, const Credentials &creds) { std::string appName; - std::vector privilegedGroups, appGroups; + std::vector forbiddenGroups, allowedGroups; Deserialization::Deserialize(buffer, appName); - int ret = serviceImpl.getPrivilegedAndAppGroups(creds, serviceImpl.getProcessLabel(appName), privilegedGroups, appGroups); + int ret = serviceImpl.getForbiddenAndAllowedGroups(creds, serviceImpl.getProcessLabel(appName), forbiddenGroups, allowedGroups); Serialization::Serialize(send, ret); if (ret == SECURITY_MANAGER_SUCCESS) { - Serialization::Serialize(send, privilegedGroups, appGroups); + Serialization::Serialize(send, forbiddenGroups, allowedGroups); } } @@ -494,14 +494,14 @@ void Service::prepareApp(MessageBuffer &buffer, MessageBuffer &send, const Crede std::string appName, pkgName, label; bool enabledSharedRO; std::vector privilegeVector; - std::vector privilegedGroups, appGroups; + std::vector forbiddenGroups, allowedGroups; std::vector privilegeStatusVector; Deserialization::Deserialize(buffer, appName, privilegeVector); int ret = serviceImpl.prepareApp(creds, appName, privilegeVector, - label, pkgName, enabledSharedRO, privilegedGroups, appGroups, privilegeStatusVector); + label, pkgName, enabledSharedRO, forbiddenGroups, allowedGroups, privilegeStatusVector); Serialization::Serialize(send, ret); if (ret == SECURITY_MANAGER_SUCCESS) - Serialization::Serialize(send, privilegedGroups, appGroups, privilegeStatusVector, label, pkgName, enabledSharedRO); + Serialization::Serialize(send, forbiddenGroups, allowedGroups, privilegeStatusVector, label, pkgName, enabledSharedRO); } } // namespace SecurityManager