From 3feb35fbf88397e4155c08faedd1684dfc3b0b76 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 13:20:08 +0100 Subject: [PATCH] refactoring: simplify NaivePolicyDb::getGroups It affects dependencies, which mostly lose unnecessary ItemType parameter. Change-Id: I32056de6f2c8edf389909cd816a0de656b4f3830 --- src/internal/internal.cpp | 8 ++--- src/internal/naive_policy_checker.cpp | 57 ++++++++++++++++++++--------------- src/internal/naive_policy_checker.hpp | 25 +++++++-------- src/internal/naive_policy_db.cpp | 54 ++++++++++++++++++++++++--------- src/internal/naive_policy_db.hpp | 6 +++- 5 files changed, 94 insertions(+), 56 deletions(-) diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index d01fa78..6682767 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -102,7 +102,7 @@ int __internal_can_send(bool bus_type, tslog::log_verbose("Destination too long: ", destination, "\n"); return false; } - return static_cast(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::SEND)); + return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } int __internal_can_send_multi_dest(bool bus_type, @@ -121,7 +121,7 @@ int __internal_can_send_multi_dest(bool bus_type, while (destination[i]) { matcher.addName(destination[i++]); } - return static_cast(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::SEND)); + return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } int __internal_can_recv(bool bus_type, @@ -139,7 +139,7 @@ int __internal_can_recv(bool bus_type, tslog::log_verbose("Sender too long: ", sender, "\n"); return false; } - return static_cast(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::RECEIVE)); + return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } int __internal_can_recv_multi(bool bus_type, @@ -158,7 +158,7 @@ int __internal_can_recv_multi(bool bus_type, while (sender[i]) { matcher.addName(sender[i++]); } - return static_cast(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::RECEIVE)); + return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index b0a8fb2..c358b97 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -40,13 +40,27 @@ DecisionResult NaivePolicyChecker::parseDecision(const DecisionItem& decision, return DecisionResult::DENY; } +DecisionItem NaivePolicyChecker::checkItemAccess(bool bus_type, const MatchItemAccess& item) +{ + const NaivePolicyDb& policy_db = getPolicyDb(bus_type); + + DecisionItem ret = policy_db.getDecisionItem(PolicyType::CONTEXT, ContextType::MANDATORY, item); + // access rules can be defined only in default/mandatory context + // defining them elsewhere is considered as policy syntax error by dbus-daemon + // thus, no checking in user or group policies + if (ret.getDecision() == Decision::ANY) + ret = policy_db.getDecisionItem(PolicyType::CONTEXT, ContextType::DEFAULT, item); + + return ret; +} + DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t bus_owner, uid_t uid, gid_t gid, const char* const label) { - const auto &gids = *getPolicyDb(bus_type).getGroups(uid, gid, ItemType::ACCESS); - auto ret = checkItem(bus_type, uid, gid, MatchItemAccess(uid, gids), ItemType::ACCESS); + const auto &gids = *getPolicyDb(bus_type).getGroups(uid, gid); + auto ret = checkItemAccess(bus_type, MatchItemAccess(uid, gids)); if (ret.getDecision() == Decision::ANY) { if (bus_owner == uid) { ret = Decision::ALLOW; @@ -60,7 +74,7 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, gid_t gid, const char* const label, const char* const name) { - auto ret = checkItem(bus_type, uid, gid, name, ItemType::OWN); + auto ret = checkItem(bus_type, uid, gid, MatchItemOwn(name)); return parseDecision(ret, uid, label); } @@ -68,9 +82,8 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - MatchItemSend &matcher, - ItemType type) { - auto ret = checkItem(bus_type, uid, gid, matcher, type); + MatchItemSend &matcher) { + auto ret = checkItem(bus_type, uid, gid, matcher); return parseDecision(ret, uid, label); } @@ -78,41 +91,37 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - MatchItemReceive &matcher, - ItemType type) { - auto ret = checkItem(bus_type, uid, gid, matcher, type); + MatchItemReceive &matcher) { + auto ret = checkItem(bus_type, uid, gid, matcher); return parseDecision(ret, uid, label); } template -DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item, const ItemType type) { +DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item) { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); - DecisionItem ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::MANDATORY), item); + DecisionItem ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::MANDATORY), item); - // access rules can be defined only in default/mandatory context - // defining them elsewhere is considered as policy syntax error by dbus-daemon - if (type != ItemType::ACCESS) { - if (ret.getDecision() == Decision::ANY) - ret = policy_db.getDecisionItem(PolicyType::USER, PolicyTypeValue(uid), item); + if (ret.getDecision() == Decision::ANY) + ret = policy_db.getDecisionItem(PolicyType::USER, PolicyTypeValue(uid), item); + + if (ret.getDecision() == Decision::ANY) + ret = checkGroupPolicies(policy_db, uid, gid, item); - if (ret.getDecision() == Decision::ANY) - ret = checkGroupPolicies(policy_db, uid, gid, item, type); - } if (ret.getDecision() == Decision::ANY) - ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); + ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); return ret; } template -DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_db, uid_t uid, gid_t gid, const T& item, const ItemType type) { - const auto *sgroups = policy_db.getGroups(uid, gid, type); +DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_db, uid_t uid, gid_t gid, const T& item) { + const auto *sgroups = policy_db.getGroups(uid, gid); if (sgroups == nullptr) return Decision::ANY; - for (auto sgid : *sgroups) { - DecisionItem ret = policy_db.getDecisionItem(PolicyType::GROUP, PolicyTypeValue(sgid), item); + for (const auto sgid : *sgroups) { + DecisionItem ret = policy_db.getDecisionItem(PolicyType::GROUP, PolicyTypeValue(sgid), item); if (ret.getDecision() != Decision::ANY) return ret; diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index 4713e97..e5e4b79 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -57,13 +57,11 @@ namespace ldp_xml_parser uid_t uid, const char* label) const; - /** Checks type P policy for given item + /** Checks policy for a given own, send or receive item * \param[in] bus_type Bus type (system/session) * \param[in] uid User id * \param[in] gid User group id - * \param[in] label User label * \param[in] item Item to check - * \param[in] type Item type * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ @@ -71,15 +69,21 @@ namespace ldp_xml_parser DecisionItem checkItem(bool bus_type, uid_t uid, gid_t gid, - const T& item, - const ItemType type); + const T& item); + + /** Checks policy for a given access item + * \param[in] bus_type Bus type (system/session) + * \param[in] item Item to check + * \return Returns deny=0, allow=1 or cynara error + * \ingroup Implementation + */ + DecisionItem checkItemAccess(bool bus_type, const MatchItemAccess &item); template DecisionItem checkGroupPolicies(const NaivePolicyDb& policy_db, uid_t uid, gid_t gid, - const T& item, - const ItemType type); + const T& item); /** Provides db handle for parsing purposes */ @@ -130,7 +134,6 @@ namespace ldp_xml_parser * \param[in] gid User group id * \param[in] label User label * \param[in] matcher Structure with multiple names to check - * \param[in] type Item type * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ @@ -138,15 +141,13 @@ namespace ldp_xml_parser uid_t uid, gid_t gid, const char* const label, - MatchItemSend &matcher, - ItemType type); + MatchItemSend &matcher); DecisionResult check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - MatchItemReceive &matcher, - ItemType type); + MatchItemReceive &matcher); }; } diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index f0fdd39..304664e 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -19,17 +19,20 @@ namespace ldp_xml_parser { * Parameters: T - MatchItem* type * field - one of m_*_set fields from NaivePolicyDb * - * The below DEF_GET_POLICY_SET defines specialization for MatchPolicy template. + * The below DEF_GET_POLICY_SET defines specialization for MatchPolicy template, + * and a specialization of NaivePolicyDb::getPolicySet template function * * Specialization for MatchPolicy provides field 'policy_type' which specifies * a type of policy used within 'field' parameter, and field 'policy_set_type' * which specifies a type of the 'field' parameter. + * Specialization of NaivePolicyDb::getPolicySet returns the 'field'. * * For example: DEF_GET_POLICY_SET(MatchItemOwn, m_own_set) defines equivalent to * MatchPolicy { * typedef PolicyOwn policy_type; * typedef PolicySet policy_set_type; * } + * PolicySet &getPolicySet const; * * Thanks to this construction we do not need to manually specify PolicyOwn in functions that * know MatchItemOwn - it is inferred. @@ -40,6 +43,10 @@ namespace ldp_xml_parser { typedef decltype(field)::type policy_type; \ typedef decltype(field) policy_set_type; \ }; \ + template <> \ + auto NaivePolicyDb::getPolicySet() const -> decltype((field)) { \ + return field; \ + } DEF_GET_POLICY_SET(MatchItemOwn, m_own_set) DEF_GET_POLICY_SET(MatchItemSend, m_send_set) @@ -130,10 +137,6 @@ DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, return policy->getDecisionItem(item); } -template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemOwn &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemSend &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemReceive &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemAccess &item) const; template const P *NaivePolicyDb::getPolicy(const PolicyType policy_type, @@ -208,20 +211,14 @@ void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, } } -const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemType type) const +template +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid) const { - if (type == ItemType::OWN) { - return &m_own_set.getMapGroup(uid); - } - if (type == ItemType::ACCESS) { - return &m_access_set.getMapGroup(uid); - } - if (uid == getuid() && gid == getgid()) - return (type == ItemType::SEND) ? &m_send_set.getMapGroup(uid) : &m_receive_set.getMapGroup(uid); + return &getPolicySet().getMapGroup(uid); pthread_mutex_lock(&mutexGroup); - auto &vgid = (type == ItemType::SEND) ? m_send_set.getMapGroup(uid) : m_receive_set.getMapGroup(uid); + auto &vgid = getPolicySet().getMapGroup(uid); if (vgid.empty()) updateSupplementaryGroups(get_groups(uid, gid), uid, gid); @@ -233,6 +230,20 @@ const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const return &vgid; } +namespace ldp_xml_parser { +template <> +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t) const +{ + return &m_own_set.getMapGroup(uid); +} + +template <> +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t) const +{ + return &m_access_set.getMapGroup(uid); +} +} // namespace ldp_xml_parser + void NaivePolicyDb::initializeGroups(uid_t uid, gid_t gid) { pthread_mutex_lock(&mutexGroup); @@ -447,3 +458,16 @@ void NaivePolicyDb::PolicyAccess::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } + +/* Explicit instantiation is needed for used public template methods defined in this file. + */ +#define T_INSTANTIATION(T) \ + template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const; \ + template const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t, gid_t) const; + +T_INSTANTIATION(MatchItemOwn) +T_INSTANTIATION(MatchItemSend) +T_INSTANTIATION(MatchItemReceive) +T_INSTANTIATION(MatchItemAccess) + +#undef T_INSTANTIATION diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 72dcb5b..68d4dd2 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -149,12 +149,16 @@ namespace ldp_xml_parser template struct MatchPolicy; // provide policy_type in specializations + template + const typename MatchPolicy::policy_set_type &getPolicySet() const; + void updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const; void updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, gid_t gid) const; public: void printContent() const; - const VGid *getGroups(uid_t uid, gid_t gid, const ItemType type) const; + template + const VGid *getGroups(uid_t uid, gid_t gid) const; void initializeGroups(uid_t uid, gid_t gid); -- 2.7.4