refactoring: simplify NaivePolicyDb::getGroups 41/199141/9
authorAdrian Szyndela <adrian.s@samsung.com>
Tue, 5 Feb 2019 12:20:08 +0000 (13:20 +0100)
committerAdrian Szyndela <adrian.s@samsung.com>
Tue, 12 Feb 2019 14:39:23 +0000 (15:39 +0100)
It affects dependencies, which mostly lose unnecessary ItemType
parameter.

Change-Id: I32056de6f2c8edf389909cd816a0de656b4f3830

src/internal/internal.cpp
src/internal/naive_policy_checker.cpp
src/internal/naive_policy_checker.hpp
src/internal/naive_policy_db.cpp
src/internal/naive_policy_db.hpp

index d01fa78..6682767 100755 (executable)
@@ -102,7 +102,7 @@ int __internal_can_send(bool bus_type,
                tslog::log_verbose("Destination too long: ", destination, "\n");
                return false;
        }
-       return static_cast<int>(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::SEND));
+       return static_cast<int>(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<int>(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::SEND));
+       return static_cast<int>(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<int>(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::RECEIVE));
+       return static_cast<int>(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<int>(policy_checker().check(bus_type, user, group, label, matcher, ldp_xml_parser::ItemType::RECEIVE));
+       return static_cast<int>(policy_checker().check(bus_type, user, group, label, matcher));
 }
 
 
index b0a8fb2..c358b97 100755 (executable)
@@ -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<MatchItemAccess>(bus_type, uid, gid, MatchItemAccess(uid, gids), ItemType::ACCESS);
+       const auto &gids = *getPolicyDb(bus_type).getGroups<MatchItemAccess>(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<MatchItemOwn>(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<MatchItemSend>(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<MatchItemReceive>(bus_type, uid, gid, matcher, type);
+                                                          MatchItemReceive &matcher) {
+       auto ret = checkItem(bus_type, uid, gid, matcher);
        return parseDecision(ret, uid, label);
 }
 
 template<typename T>
-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<T>(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<T>(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<T>(policy_db, uid, gid, item);
 
-               if (ret.getDecision() == Decision::ANY)
-                       ret = checkGroupPolicies<T>(policy_db, uid, gid, item, type);
-       }
        if (ret.getDecision() == Decision::ANY)
-               ret = policy_db.getDecisionItem<T>(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item);
+               ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item);
 
        return ret;
 }
 
 template<typename T>
-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<T>(uid, gid);
        if (sgroups == nullptr)
                return Decision::ANY;
 
-       for (auto sgid : *sgroups) {
-               DecisionItem ret = policy_db.getDecisionItem<T>(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;
index 4713e97..e5e4b79 100644 (file)
@@ -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<typename T>
                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);
        };
 }
 
index f0fdd39..304664e 100755 (executable)
@@ -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<T> template.
+ * The below DEF_GET_POLICY_SET defines specialization for MatchPolicy<T> template,
+ * and a specialization of NaivePolicyDb::getPolicySet template function
  *
  * Specialization for MatchPolicy<T> 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<MatchItemOwn> {
  *             typedef PolicyOwn policy_type;
  *             typedef PolicySet<PolicyOwn> policy_set_type;
  * }
+ * PolicySet<PolicyOwn> &getPolicySet<PolicyOwn> 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<T>() 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<MatchItemOwn>(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemOwn &item) const;
-template DecisionItem NaivePolicyDb::getDecisionItem<MatchItemSend>(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemSend &item) const;
-template DecisionItem NaivePolicyDb::getDecisionItem<MatchItemReceive>(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemReceive &item) const;
-template DecisionItem NaivePolicyDb::getDecisionItem<MatchItemAccess>(PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemAccess &item) const;
 
 template <typename P>
 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 <typename T>
+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<T>().getMapGroup(uid);
 
        pthread_mutex_lock(&mutexGroup);
-       auto &vgid = (type == ItemType::SEND) ? m_send_set.getMapGroup(uid) : m_receive_set.getMapGroup(uid);
+       auto &vgid = getPolicySet<T>().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<MatchItemOwn>(uid_t uid, gid_t) const
+{
+       return &m_own_set.getMapGroup(uid);
+}
+
+template <>
+const NaivePolicyDb::VGid *NaivePolicyDb::getGroups<MatchItemAccess>(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<T>(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const; \
+       template const NaivePolicyDb::VGid *NaivePolicyDb::getGroups<T>(uid_t, gid_t) const;
+
+T_INSTANTIATION(MatchItemOwn)
+T_INSTANTIATION(MatchItemSend)
+T_INSTANTIATION(MatchItemReceive)
+T_INSTANTIATION(MatchItemAccess)
+
+#undef T_INSTANTIATION
index 72dcb5b..68d4dd2 100755 (executable)
@@ -149,12 +149,16 @@ namespace ldp_xml_parser
                template <typename T>
                struct MatchPolicy; // provide policy_type in specializations
 
+               template <typename T>
+               const typename MatchPolicy<T>::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 <typename T>
+               const VGid *getGroups(uid_t uid, gid_t gid) const;
 
                void initializeGroups(uid_t uid, gid_t gid);