From cda9c5d302fc9a5a14c1ccac130304ca10ce8682 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 15:20:47 +0100 Subject: [PATCH 01/16] refactoring: get rid of unnecessary MessageDirection Things done to accomplish removing MessageDirection: - used ItemSend/ItemReceive instead of ItemSendReceive - used MatchItemSend/MatchItemReceive instead of MatchItemSR - used PolicySend/PolicyReceive instead of PolicySR - database no longer contains pointers Change-Id: I1c5957dad2181a6c1c42dee0e77be3e8b0ccc471 --- src/internal/internal.cpp | 8 ++-- src/internal/naive_policy_checker.cpp | 28 +++++++++--- src/internal/naive_policy_checker.hpp | 25 +++++++++-- src/internal/naive_policy_db.cpp | 83 ++++++++++++++++++----------------- src/internal/naive_policy_db.hpp | 46 ++++++++++++------- src/internal/policy.cpp | 78 ++++++++++---------------------- src/internal/policy.hpp | 25 +++-------- src/test-libdbuspolicy1-method.cpp | 5 +++ 8 files changed, 154 insertions(+), 144 deletions(-) diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index f15e81b..d01fa78 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -97,7 +97,7 @@ int __internal_can_send(bool bus_type, const char* const member, int type) { - ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::SEND); + ldp_xml_parser::MatchItemSend matcher(interface, member, path, static_cast(type)); if (!matcher.addNames(destination)) { tslog::log_verbose("Destination too long: ", destination, "\n"); return false; @@ -116,7 +116,7 @@ int __internal_can_send_multi_dest(bool bus_type, int type) { int i = 0; - ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::SEND); + ldp_xml_parser::MatchItemSend matcher(interface, member, path, static_cast(type)); if (destination) while (destination[i]) { matcher.addName(destination[i++]); @@ -134,7 +134,7 @@ int __internal_can_recv(bool bus_type, const char* const member, int type) { - ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::RECEIVE); + ldp_xml_parser::MatchItemReceive matcher(interface, member, path, static_cast(type)); if (!matcher.addNames(sender)) { tslog::log_verbose("Sender too long: ", sender, "\n"); return false; @@ -153,7 +153,7 @@ int __internal_can_recv_multi(bool bus_type, int type) { int i = 0; - ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::RECEIVE); + ldp_xml_parser::MatchItemReceive matcher(interface, member, path, static_cast(type)); if (sender) while (sender[i]) { matcher.addName(sender[i++]); diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 8652dec..2eda4e0 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -68,16 +68,34 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - MatchItemSR& matcher, + MatchItemSend &matcher, ItemType type) { - auto ret = checkItem(bus_type, uid, gid, matcher, type); + auto ret = checkItem(bus_type, uid, gid, matcher, type); return parseDecision(ret, uid, label); } -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySR& policy, - const MatchItemSR& item) const +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); + return parseDecision(ret, uid, label); +} + +DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySend &policy, + const MatchItemSend &item) const +{ + tslog::log_verbose("Checking send policy for: ", item, "\n"); + + return policy.getDecisionItem(item); +} + +DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyReceive &policy, + const MatchItemReceive &item) const { - tslog::log_verbose("checkpolicy for: ", item, "\n"); + tslog::log_verbose("Checking receive policy for: ", item, "\n"); return policy.getDecisionItem(item); } diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index 807083b..efee9da 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -45,15 +45,25 @@ namespace ldp_xml_parser */ NaivePolicyDb& getPolicyDb(bool type); - /** Checks send/receive policy for given item + /** Checks send policy for given item + * \param[in] policy Policy to check + * \param[in] item Item to check + * \return Returns decision retrieved from policy + * \ingroup Implementation + * \callgraph + */ + DecisionItem checkPolicy(const NaivePolicyDb::PolicySend &policy, + const MatchItemSend &item) const; + + /** Checks receive policy for given item * \param[in] policy Policy to check * \param[in] item Item to check * \return Returns decision retrieved from policy * \ingroup Implementation * \callgraph */ - DecisionItem checkPolicy(const NaivePolicyDb::PolicySR& policy, - const MatchItemSR& item) const; + DecisionItem checkPolicy(const NaivePolicyDb::PolicyReceive &policy, + const MatchItemReceive &item) const; /** Checks ownership policy for given item * \param[in] policy Policy to check @@ -166,7 +176,14 @@ namespace ldp_xml_parser uid_t uid, gid_t gid, const char* const label, - MatchItemSR& matcher, + MatchItemSend &matcher, + ItemType type); + + DecisionResult check(bool bus_type, + uid_t uid, + gid_t gid, + const char* const label, + MatchItemReceive &matcher, ItemType type); }; } diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index a4f54a7..265e31d 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -14,35 +14,34 @@ using namespace ldp_xml_parser; template -void log_item_add(const T* item) { - tslog::log("Add item: ", item, ", decision: ", item->getDecision(), "\n"); +void log_item_add(const T &item) { + tslog::log("Add item: ", item, ", decision: ", item.getDecision(), "\n"); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemSendReceive* const item) { + ItemSend &item) { log_item_add(item); - const MessageDirection dir = item->getDirection(); - if (dir == MessageDirection::SEND) { - m_send_set.addItem(policy_type, policy_type_value, item); - } else if (dir == MessageDirection::RECEIVE) { - m_receive_set.addItem(policy_type, policy_type_value, item); - } else { - m_send_set.addItem(policy_type, policy_type_value, item); - m_receive_set.addItem(policy_type, policy_type_value, item); - } + m_send_set.addItem(policy_type, policy_type_value, item); +} + +void NaivePolicyDb::addItem(const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + ItemReceive &item) { + log_item_add(item); + m_receive_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemOwn* const item) { + ItemOwn &item) { log_item_add(item); m_own_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemAccess* const item) { + ItemAccess &item) { log_item_add(item); m_access_set.addItem(policy_type, policy_type_value, item); } @@ -169,7 +168,7 @@ bool NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, template template void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - T* const item) { + T &item) { switch (policy_type) { case PolicyType::CONTEXT: context[static_cast(policy_type_value.context)].addItem(item); @@ -186,40 +185,41 @@ void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, } } -/****************** NaivePolicyDb::PolicySRBase ************************/ +/****************** NaivePolicyDb::PolicySR ************************/ template -void NaivePolicyDb::PolicySRBase::addItem(TI* item) { - m_items.push_back(item); +void NaivePolicyDb::PolicySR::addItem(TI &item) { + m_items.push_back(std::move(item)); } template -size_t NaivePolicyDb::PolicySRBase::getSize() const { +size_t NaivePolicyDb::PolicySR::getSize() const { size_t size = m_items.capacity() * sizeof(typename decltype(m_items)::value_type); for (const auto& i : m_items) - size += i->getSize(); + size += i.getSize(); return size; } template -void NaivePolicyDb::PolicySRBase::printContent() const { +void NaivePolicyDb::PolicySR::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } template -DecisionItem NaivePolicyDb::PolicySRBase::getDecisionItem(const TM &item) const { +DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const TM &item) const { for (auto i : *this) { - tslog::log_verbose("-read: ", i->getDecision(), " ", i, "\n"); + tslog::log_verbose("-read: ", i.getDecision(), " ", i, "\n"); - if (i->match(item)) { - tslog::log_verbose("-matched: ", i->getDecision(), " ", i, "\n"); + if (i.match(item)) { + tslog::log_verbose("-matched: ", i.getDecision(), " ", i, "\n"); - return i->getDecision(); + return i.getDecision(); } } return Decision::ANY; } -template DecisionItem NaivePolicyDb::PolicySRBase::getDecisionItem(const MatchItemSR &item) const; +template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSend &item) const; +template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemReceive &item) const; bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyType policy_type, @@ -232,16 +232,17 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyType policy_type, const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicySR*& policy) const { - switch (item_type) { - case ItemType::SEND: - return m_send_set.getPolicy(policy_type, policy_type_value, policy); - case ItemType::RECEIVE: - return m_receive_set.getPolicy(policy_type, policy_type_value, policy); - default: - assert(false); - return false; - } + const NaivePolicyDb::PolicySend*& policy) const { + assert(item_type == ItemType::SEND); + return m_send_set.getPolicy(policy_type, policy_type_value, policy); +} + +bool NaivePolicyDb::getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const NaivePolicyDb::PolicyReceive*& policy) const { + assert(item_type == ItemType::RECEIVE); + return m_receive_set.getPolicy(policy_type, policy_type_value, policy); } bool NaivePolicyDb::getPolicy(const ItemType item_type, @@ -252,8 +253,8 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, return m_access_set.getPolicy(policy_type, policy_type_value, policy); } -void NaivePolicyDb::PolicyOwn::addItem(ItemOwn* item) { - ownership_tree.addItem(item); +void NaivePolicyDb::PolicyOwn::addItem(ItemOwn &item) { + ownership_tree.addItem(&item); } void NaivePolicyDb::PolicyOwn::printContent() const @@ -285,9 +286,9 @@ DecisionItem NaivePolicyDb::PolicyAccess::getDecisionItem(const MatchItemAccess& return ret; } -void NaivePolicyDb::PolicyAccess::addItem(const ItemAccess* item) +void NaivePolicyDb::PolicyAccess::addItem(ItemAccess &item) { - m_items.push_back(*item); + m_items.push_back(std::move(item)); } size_t NaivePolicyDb::PolicyAccess::getSize() const diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index b8489cc..1892f22 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -34,10 +34,10 @@ namespace ldp_xml_parser public: /** Class containing policy with send/receive rules */ template - class PolicySRBase { + class PolicySR { private: /** Vector with policy items */ - std::vector m_items; + std::vector m_items; typedef typename decltype(m_items)::const_reverse_iterator PolicyConstIterator; PolicyConstIterator begin() const { return m_items.rbegin(); } @@ -47,21 +47,19 @@ namespace ldp_xml_parser /** Adds given item to policy. * \param[in] item Item to add to policy */ - void addItem(TI* item); + void addItem(TI &item); DecisionItem getDecisionItem(const TM &item) const; void printContent() const; size_t getSize() const; }; - class PolicySend : public PolicySRBase { + class PolicySend : public PolicySR { public: }; - class PolicyReceive : public PolicySRBase { + class PolicyReceive : public PolicySR { public: }; - typedef PolicySRBase PolicySR; - /** Class containing policy with ownership rules */ class PolicyOwn { private: @@ -71,7 +69,7 @@ namespace ldp_xml_parser /** Adds given item to tree by retrieving its name, decision and checking is it prefix. * \param[in] item Item to add to policy */ - void addItem(ItemOwn* item); + void addItem(ItemOwn &item); DecisionItem getDecisionItem(const MatchItemOwn& item) const; void printContent() const; size_t getSize() const; @@ -83,7 +81,7 @@ namespace ldp_xml_parser std::vector m_items; public: - void addItem(const ItemAccess* item); + void addItem(ItemAccess &item); DecisionItem getDecisionItem(const MatchItemAccess& item) const; void printContent() const; size_t getSize() const; @@ -110,7 +108,12 @@ namespace ldp_xml_parser bool getPolicy(const ItemType item_type, const PolicyType policy_type, const PolicyTypeValue policy_type_value, - const PolicySR*& policy) const; + const PolicySend*& policy) const; + + bool getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const PolicyReceive*& policy) const; bool getPolicy(const ItemType item_type, const PolicyType policy_type, @@ -140,7 +143,7 @@ namespace ldp_xml_parser template void addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - T* const item); + T &item); /** Gets requested policy * \param[in] set Set to add item to @@ -165,9 +168,9 @@ namespace ldp_xml_parser /** Set of ownership policies */ PolicySet m_own_set; /** Set of send policies */ - PolicySet m_send_set; + PolicySet m_send_set; /** Set of receive policies */ - PolicySet m_receive_set; + PolicySet m_receive_set; /** Set of bus access policies */ PolicySet m_access_set; @@ -190,16 +193,25 @@ namespace ldp_xml_parser */ void addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemOwn* const item); + ItemOwn &item); + + /** Adds item to send policy + * \param[in] policy_type Policy type + * \param[in] policy_type_value Policy type value + * \param[in] item Item to add + */ + void addItem(const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + ItemSend &item); - /** Adds item to send/receive policy + /** Adds item to receive policy * \param[in] policy_type Policy type * \param[in] policy_type_value Policy type value * \param[in] item Item to add */ void addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemSendReceive* const item); + ItemReceive &item); /** Adds item to access policy * \param[in] policy_type Policy type @@ -208,7 +220,7 @@ namespace ldp_xml_parser */ void addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, - ItemAccess* const item); + ItemAccess &item); }; } #endif diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index c3f43ad..cb850b0 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -20,7 +20,6 @@ using namespace ldp_xml_parser; static const char* message_type[] = { "ANY", "METHOD_CALL", "METHOD_RETURN", "ERROR", "SIGNAL"}; -static const char* message_dir[] = { "ANY", "SEND", "RECEIVE"}; static const char* message_decision[] = {"NO_DECISION", "ALLOW", "DENY", "CHECK"}; static const char* message_access[] = {"USER", "GROUP", "ALL_USERS", "ALL_GROUPS"}; @@ -55,10 +54,6 @@ static inline const char* __message_type_to_str(MessageType type) { return message_type[static_cast(type)]; } -static inline const char* __message_dir_to_str(MessageDirection type) { - return message_dir[static_cast(type)]; -} - static inline const char* __decision_to_str(Decision dec) { return message_decision[static_cast(dec)]; } @@ -170,9 +165,9 @@ void DbAdapter::parseRuleAttribute(const char *name, const char *value) value = nullptr; if (field_has(name, "send_")) { - __builder.addDirection(MessageDirection::SEND); + __builder.setItemType(ItemType::SEND); } else if (field_has(name, "receive_")) { - __builder.addDirection(MessageDirection::RECEIVE); + __builder.setItemType(ItemType::RECEIVE); } else if (std::strcmp(name, "own") == 0) { __builder.addOwner(value); __builder.setPrefix(false); @@ -235,10 +230,6 @@ const char* DecisionItem::getPrivilege() const { return __privilege.c_str(); } -ItemType DecisionItem::getType() const { - return ItemType::GENERIC; -} - size_t DecisionItem::getSize() const { return get_string_heap_allocated_memory(__privilege); } @@ -258,10 +249,6 @@ void ItemOwn::setName(const char* name) { } } -ItemType ItemOwn::getType() const { - return ItemType::OWN; -} - const char* ItemOwn::getName() const { if (__name.empty()) { return nullptr; @@ -281,8 +268,8 @@ const DecisionItem& ItemOwn::getDecision() const { return __decision; } -MatchItemSR::MatchItemSR(const char* i, const char* me, const char* p, MessageType t, MessageDirection d) - : names_num(0), type(t), direction(d) { +MatchItemSR::MatchItemSR(const char* i, const char* me, const char* p, MessageType t) + : names_num(0), type(t) { if (i) interface = i; if (me) member = me; if (p) path = p; @@ -324,9 +311,6 @@ bool ItemSendReceive::match(const MatchItemSR& item) const { if (__type != MessageType::ANY && __type != item.type) return false; - if (__direction != item.direction) - return false; - if (!__interface.empty() && !item.interface.empty() && __interface != item.interface) return false; @@ -352,18 +336,6 @@ bool ItemSendReceive::match(const MatchItemSR& item) const { return true; } -ItemType ItemSendReceive::getType() const { - if (__direction == MessageDirection::SEND) - return ItemType::SEND; - else - return ItemType::RECEIVE; -} - - -MessageDirection ItemSendReceive::getDirection() const { - return __direction; -} - const DecisionItem& ItemSendReceive::getDecision() const { return __decision; } @@ -447,11 +419,11 @@ ItemOwn* ItemBuilder::getOwnItem() { } ItemSendReceive* ItemBuilder::getSendReceiveItem() { - if (!__current_sr) { - __current_sr = new ItemSendReceive(); - } - __current_item_type = ItemType::SEND; - return __current_sr; + if (__current_item_type == ItemType::SEND) + return &__current_send; + + assert(__current_item_type == ItemType::RECEIVE); + return &__current_receive; } ItemAccess* ItemBuilder::getAccessItem() { @@ -459,18 +431,17 @@ ItemAccess* ItemBuilder::getAccessItem() { return &__current_access; } -ItemBuilder::ItemBuilder() : __current_item_type(ItemType::GENERIC), __current_sr(NULL) { +ItemBuilder::ItemBuilder() : __current_item_type(ItemType::GENERIC) { } ItemBuilder::~ItemBuilder(){ - if (__current_sr) - delete __current_sr; } void ItemBuilder::reset() { __decision.__decision = Decision::ANY; __decision.setPrivilege(nullptr); - __current_sr = nullptr; + __current_send = ItemSend(); + __current_receive = ItemReceive(); __current_own = ItemOwn(); __current_access = ItemAccess(); } @@ -479,18 +450,19 @@ void ItemBuilder::generateItem(NaivePolicyDb& db, PolicyType& policy_type, Polic switch (__current_item_type) { case ItemType::OWN: __current_own.__decision = __decision; - db.addItem(policy_type, policy_type_value, &__current_own); + db.addItem(policy_type, policy_type_value, __current_own); break; case ItemType::SEND: + __current_send.__decision = __decision; + db.addItem(policy_type, policy_type_value, __current_send); + break; case ItemType::RECEIVE: - if (__current_sr) { - __current_sr->__decision = __decision; - db.addItem(policy_type, policy_type_value, __current_sr); - } + __current_receive.__decision = __decision; + db.addItem(policy_type, policy_type_value, __current_receive); break; case ItemType::ACCESS: __current_access.__decision = __decision; - db.addItem(policy_type, policy_type_value, &__current_access); + db.addItem(policy_type, policy_type_value, __current_access); break; case ItemType::GENERIC: //should never happen @@ -537,9 +509,8 @@ void ItemBuilder::addMessageType(MessageType type) { sr->__type = type; } -void ItemBuilder::addDirection(MessageDirection direction) { - ItemSendReceive* sr = getSendReceiveItem(); - sr->__direction = direction; +void ItemBuilder::setItemType(ItemType type) { + __current_item_type = type; } void ItemBuilder::addUserAccess(const char* user) { @@ -592,15 +563,14 @@ std::ostream &operator<<(std::ostream& stream, const MatchItemOwn &item) std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item) { - stream << "matcher: services("; + stream << ": services("; for (int i = 0; i < item.names_num; i++) { stream << item.names[i]; if (i != item.names_num -1) stream << " "; } return stream << "), interface(" << item.interface << "), member(" << item.member << - "), path(" << item.path << "), type(" << __message_type_to_str(item.type) << "), direction(" << - __message_dir_to_str(item.direction) << ")"; + "), path(" << item.path << "), type(" << __message_type_to_str(item.type) << ")"; } std::ostream &operator<<(std::ostream& stream, const MatchItemSend &item) @@ -619,7 +589,7 @@ std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item) { return stream << ": name(" << item.__name << "), inter(" << item.__interface << "), member(" << item.__member << "), path(" << item.__path << "), type(" << - __message_type_to_str(item.__type) << "), dir(" << __message_dir_to_str(item.__direction) << ")"; + __message_type_to_str(item.__type) << ")"; } std::ostream &operator<<(std::ostream& stream, const ItemAccess &item) diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 32037e7..1e11645 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -43,13 +43,6 @@ namespace ldp_xml_parser SIGNAL }; - /** Sent message direction */ - enum class MessageDirection : uint8_t { - ANY, - SEND, - RECEIVE - }; - /** Item type. (different items have different content of class */ enum class ItemType : uint8_t { GENERIC, @@ -122,7 +115,6 @@ namespace ldp_xml_parser DecisionItem(Decision decision = Decision::ANY, const char* privilege = NULL); Decision getDecision() const; const char* getPrivilege() const; - ItemType getType() const; size_t getSize() const; friend std::ostream &operator<<(std::ostream& stream, const DecisionItem &di); @@ -153,7 +145,6 @@ namespace ldp_xml_parser Decision decision = Decision::ANY, const char* privilege = NULL); bool match(const char* const name) const; - ItemType getType() const; const DecisionItem& getDecision() const; const char* getName() const; bool isPrefix() const; @@ -171,14 +162,12 @@ namespace ldp_xml_parser boost::string_ref member; boost::string_ref path; MessageType type; - MessageDirection direction; - MatchItemSR(const char* i, const char* me, const char* p, MessageType t, MessageDirection d); + MatchItemSR(const char* i, const char* me, const char* p, MessageType t); void addName(const char* name); bool addNames(const char* name); friend std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item); }; - std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item); class MatchItemSend : public MatchItemSR { using MatchItemSR::MatchItemSR; @@ -201,14 +190,11 @@ namespace ldp_xml_parser std::string __member; std::string __path; MessageType __type; - MessageDirection __direction; bool __is_name_prefix; + bool match(const MatchItemSR& item) const; + ItemSendReceive(); public: friend class ItemBuilder; - ItemSendReceive(); - bool match(const MatchItemSR& item) const; - MessageDirection getDirection() const; - ItemType getType() const; const DecisionItem& getDecision() const; size_t getSize() const; @@ -273,7 +259,8 @@ namespace ldp_xml_parser DecisionItem __decision; ItemType __current_item_type; ItemOwn __current_own; - ItemSendReceive* __current_sr; + ItemSend __current_send; + ItemReceive __current_receive; ItemAccess __current_access; ItemOwn* getOwnItem(); @@ -292,7 +279,7 @@ namespace ldp_xml_parser void addMember(const char* member); void addPath(const char* path); void addMessageType(MessageType type); - void addDirection(MessageDirection direction); + void setItemType(ItemType type); void addPrivilege(const char* privilege); void addDecision(Decision decision); void setPrefix(bool value); diff --git a/src/test-libdbuspolicy1-method.cpp b/src/test-libdbuspolicy1-method.cpp index 615183d..3c2e16d 100644 --- a/src/test-libdbuspolicy1-method.cpp +++ b/src/test-libdbuspolicy1-method.cpp @@ -8,6 +8,11 @@ using namespace ldp_xml_parser; +enum MessageDirection { + RECEIVE, + SEND +}; + struct MethodTest { bool expected_result; uid_t user; -- 2.7.4 From 3c249e0ba168918fa1876bb55fea50eee7e95e3c Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 16:06:33 +0100 Subject: [PATCH 02/16] refactoring: make parsing lazy We don't need to reset all the potential items on every rule. This commit resets items on first encounter of given type. Change-Id: I9bd1abe38516c188dd295697109bfe1a337eaf0b --- src/internal/policy.cpp | 75 ++++++++++++++++++++++++++++++++----------------- src/internal/policy.hpp | 6 +++- 2 files changed, 54 insertions(+), 27 deletions(-) diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index cb850b0..6b73ddf 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -149,7 +149,6 @@ void DbAdapter::parseRule(const char *name, const char **attr) if (str2decision.find(std::string(name)) == str2decision.end()) { return; } - __builder.reset(); __builder.addDecision(str2decision.at(std::string(name))); for (int i = 0; attr[i]; i += 2) { @@ -165,9 +164,9 @@ void DbAdapter::parseRuleAttribute(const char *name, const char *value) value = nullptr; if (field_has(name, "send_")) { - __builder.setItemType(ItemType::SEND); + __builder.setSendItem(); } else if (field_has(name, "receive_")) { - __builder.setItemType(ItemType::RECEIVE); + __builder.setReceiveItem(); } else if (std::strcmp(name, "own") == 0) { __builder.addOwner(value); __builder.setPrefix(false); @@ -413,12 +412,32 @@ const std::vector& MatchItemAccess::getGids() const return __gids; } -ItemOwn* ItemBuilder::getOwnItem() { - __current_item_type = ItemType::OWN; +void ItemBuilder::setOwnItem() { + if (__current_item_type != ItemType::OWN) { + __current_own = ItemOwn(); + __current_item_type = ItemType::OWN; + } +} + +ItemOwn *ItemBuilder::getOwnItem() { return &__current_own; } -ItemSendReceive* ItemBuilder::getSendReceiveItem() { +void ItemBuilder::setSendItem() { + if (__current_item_type != ItemType::SEND) { + __current_send = ItemSend(); + __current_item_type = ItemType::SEND; + } +} + +void ItemBuilder::setReceiveItem() { + if (__current_item_type != ItemType::RECEIVE) { + __current_receive = ItemReceive(); + __current_item_type = ItemType::RECEIVE; + } +} + +ItemSendReceive *ItemBuilder::getSendReceiveItem() { if (__current_item_type == ItemType::SEND) return &__current_send; @@ -426,24 +445,30 @@ ItemSendReceive* ItemBuilder::getSendReceiveItem() { return &__current_receive; } -ItemAccess* ItemBuilder::getAccessItem() { - __current_item_type = ItemType::ACCESS; +void ItemBuilder::setAccessItem() { + if (__current_item_type != ItemType::ACCESS) { + __current_access = ItemAccess(); + __current_item_type = ItemType::ACCESS; + } +} + +ItemAccess *ItemBuilder::getAccessItem() { return &__current_access; } -ItemBuilder::ItemBuilder() : __current_item_type(ItemType::GENERIC) { +ItemBuilder::ItemBuilder() + : __current_item_type(ItemType::GENERIC) +{ + reset(); } -ItemBuilder::~ItemBuilder(){ +ItemBuilder::~ItemBuilder() { } void ItemBuilder::reset() { __decision.__decision = Decision::ANY; __decision.setPrivilege(nullptr); - __current_send = ItemSend(); - __current_receive = ItemReceive(); - __current_own = ItemOwn(); - __current_access = ItemAccess(); + __current_item_type = ItemType::GENERIC; } void ItemBuilder::generateItem(NaivePolicyDb& db, PolicyType& policy_type, PolicyTypeValue& policy_type_value) { @@ -465,16 +490,17 @@ void ItemBuilder::generateItem(NaivePolicyDb& db, PolicyType& policy_type, Polic db.addItem(policy_type, policy_type_value, __current_access); break; case ItemType::GENERIC: - //should never happen - assert(false); + // do nothing + // this is for rules like + // libdbuspolicy does not support that break; } reset(); } void ItemBuilder::addOwner(const char* owner) { - ItemOwn* o = getOwnItem(); - o->setName(owner); + setOwnItem(); + getOwnItem()->setName(owner); } void ItemBuilder::addName(const char* name, bool prefix) { @@ -505,19 +531,16 @@ void ItemBuilder::addPath(const char* path) { } void ItemBuilder::addMessageType(MessageType type) { - ItemSendReceive* sr = getSendReceiveItem(); - sr->__type = type; -} - -void ItemBuilder::setItemType(ItemType type) { - __current_item_type = type; + getSendReceiveItem()->__type = type; } void ItemBuilder::addUserAccess(const char* user) { + setAccessItem(); getAccessItem()->setUser(user); } void ItemBuilder::addGroupAccess(const char* group) { + setAccessItem(); getAccessItem()->setGroup(group); } @@ -530,8 +553,8 @@ void ItemBuilder::addDecision(Decision decision) { } void ItemBuilder::setPrefix(bool value) { - ItemOwn* o = getOwnItem(); - o->__is_prefix = value; + assert(__current_item_type == ItemType::OWN); + getOwnItem()->__is_prefix = value; } PolicyTypeValue::PolicyTypeValue() : context(ContextType::DEFAULT) { diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 1e11645..19781d2 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -258,6 +258,7 @@ namespace ldp_xml_parser private: DecisionItem __decision; ItemType __current_item_type; + ItemOwn __current_own; ItemSend __current_send; ItemReceive __current_receive; @@ -271,6 +272,10 @@ namespace ldp_xml_parser ~ItemBuilder(); void generateItem(NaivePolicyDb& db, PolicyType& policy_type, PolicyTypeValue& policy_type_value); void reset(); + void setOwnItem(); + void setSendItem(); + void setReceiveItem(); + void setAccessItem(); void addUser(const char* name); void addGroup(const char* name); void addOwner(const char* owner); @@ -279,7 +284,6 @@ namespace ldp_xml_parser void addMember(const char* member); void addPath(const char* path); void addMessageType(MessageType type); - void setItemType(ItemType type); void addPrivilege(const char* privilege); void addDecision(Decision decision); void setPrefix(bool value); -- 2.7.4 From 3937a6e478190a57caaeef1c3b39e831316d204f Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 16:25:11 +0100 Subject: [PATCH 03/16] refactoring: make own_tree more const-correct Change-Id: I4c6f7cb72f743e597f6fa295cb4731e01cfcda55 --- src/internal/naive_policy_db.cpp | 2 +- src/internal/own_tree.cpp | 14 +++++++------- src/internal/own_tree.hpp | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 265e31d..e754cf6 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -254,7 +254,7 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, } void NaivePolicyDb::PolicyOwn::addItem(ItemOwn &item) { - ownership_tree.addItem(&item); + ownership_tree.addItem(item); } void NaivePolicyDb::PolicyOwn::printContent() const diff --git a/src/internal/own_tree.cpp b/src/internal/own_tree.cpp index b9cc3d2..be19ca7 100644 --- a/src/internal/own_tree.cpp +++ b/src/internal/own_tree.cpp @@ -47,18 +47,18 @@ std::deque tokenize(const std::string& item_name) return elements; } -void OwnershipTree::addItem(ItemOwn* item) { - if (item->isMatchAll()) { - if (item->getDecision().getDecision() != Decision::ANY){ - __root->setDecisionItem(item->getDecision(), true); +void OwnershipTree::addItem(const ItemOwn &item) { + if (item.isMatchAll()) { + if (item.getDecision().getDecision() != Decision::ANY){ + __root->setDecisionItem(item.getDecision(), true); } return; } - assert(item->getName() != nullptr); - std::string name = item->getName(); + assert(item.getName() != nullptr); + std::string name = item.getName(); auto tokens = tokenize(name); - __root->add(tokens, item->getDecision(), item->isPrefix()); + __root->add(tokens, item.getDecision(), item.isPrefix()); } DecisionItem OwnershipTree::getDecisionItem(const MatchItemOwn& item) const diff --git a/src/internal/own_tree.hpp b/src/internal/own_tree.hpp index b052fea..4f986b3 100644 --- a/src/internal/own_tree.hpp +++ b/src/internal/own_tree.hpp @@ -31,7 +31,7 @@ namespace ldp_xml_parser class OwnershipTree{ public: OwnershipTree(); - void addItem(ItemOwn* item); + void addItem(const ItemOwn &item); DecisionItem getDecisionItem(const MatchItemOwn& item) const; void printTree() const; size_t getSize() const; -- 2.7.4 From 004dbfcc86e21c9d25850bf7426cd7fdfe5bcba4 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 10:20:26 +0100 Subject: [PATCH 04/16] refactoring: group methods from same classes Change-Id: Id6f8124fc7b5423ee4b9922b017f153b99b249b4 --- src/internal/naive_policy_db.cpp | 235 ++++++++++++++++++++------------------- 1 file changed, 120 insertions(+), 115 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index e754cf6..442e0c6 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -13,11 +13,14 @@ using namespace ldp_xml_parser; -template +namespace { +template void log_item_add(const T &item) { tslog::log("Add item: ", item, ", decision: ", item.getDecision(), "\n"); } +} +/********************* NaivePolicyDb **********************/ void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemSend &item) { @@ -71,6 +74,120 @@ void NaivePolicyDb::printContent() const #undef PRINT_MAP } +bool NaivePolicyDb::getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const NaivePolicyDb::PolicyOwn*& policy) const { + assert(item_type == ItemType::OWN); + return m_own_set.getPolicy(policy_type, policy_type_value, policy); +} + +bool NaivePolicyDb::getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const NaivePolicyDb::PolicySend*& policy) const { + assert(item_type == ItemType::SEND); + return m_send_set.getPolicy(policy_type, policy_type_value, policy); +} + +bool NaivePolicyDb::getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const NaivePolicyDb::PolicyReceive*& policy) const { + assert(item_type == ItemType::RECEIVE); + return m_receive_set.getPolicy(policy_type, policy_type_value, policy); +} + +bool NaivePolicyDb::getPolicy(const ItemType item_type, + const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const NaivePolicyDb::PolicyAccess*& policy) const { + assert(item_type == ItemType::ACCESS); + return m_access_set.getPolicy(policy_type, policy_type_value, policy); +} + +void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const +{ + auto &vsend = m_send_set.getMapGroup(uid); + auto &vrecv = m_receive_set.getMapGroup(uid); + auto &vaccess = m_access_set.getMapGroup(uid); + + if (groups.empty()) { + vsend.push_back(gid); + vrecv.push_back(gid); + vaccess.push_back(gid); + return; + } + + /* insert supplementary group */ + for (const auto group : groups) { + if (m_send_set.group.find(group) != m_send_set.group.end()) + vsend.push_back(group); + if (m_receive_set.group.find(group) != m_receive_set.group.end()) + vrecv.push_back(group); + vaccess.push_back(group); // no filtering, it will be used once + } + + if (vsend.empty()) + vsend.push_back(-1); + if (vrecv.empty()) + vrecv.push_back(-1); +} + +void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, gid_t gid) const +{ + auto &vown = m_own_set.getMapGroup(uid); + if (groups.empty()) { + vown.push_back(gid); + } else { + for (const auto group : groups) { + if (m_own_set.group.find(group) != m_own_set.group.end()) + vown.push_back(group); + } + } +} + +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemType type) 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); + + pthread_mutex_lock(&mutexGroup); + auto &vgid = (type == ItemType::SEND) ? m_send_set.getMapGroup(uid) : m_receive_set.getMapGroup(uid); + + if (vgid.empty()) + updateSupplementaryGroups(get_groups(uid, gid), uid, gid); + pthread_mutex_unlock(&mutexGroup); + + if (vgid[0] == (gid_t)-1) + return nullptr; + + return &vgid; +} + +void NaivePolicyDb::initializeGroups(uid_t uid, gid_t gid) +{ + pthread_mutex_lock(&mutexGroup); + + m_send_set.clearMapGroup(); + m_receive_set.clearMapGroup(); + m_own_set.clearMapGroup(); + m_access_set.clearMapGroup(); + + auto groups = get_groups(uid, gid); + updateSupplementaryGroups(groups, uid, gid); + updateSupplementaryGroupsOwn(groups, uid, gid); + + pthread_mutex_unlock(&mutexGroup); +} + /****************** NaivePolicyDb::PolicySet ************************/ template size_t NaivePolicyDb::PolicySet

::getSetSize() const @@ -221,38 +338,7 @@ DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const TM &item) co template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSend &item) const; template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemReceive &item) const; -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyOwn*& policy) const { - assert(item_type == ItemType::OWN); - return m_own_set.getPolicy(policy_type, policy_type_value, policy); -} - -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicySend*& policy) const { - assert(item_type == ItemType::SEND); - return m_send_set.getPolicy(policy_type, policy_type_value, policy); -} - -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyReceive*& policy) const { - assert(item_type == ItemType::RECEIVE); - return m_receive_set.getPolicy(policy_type, policy_type_value, policy); -} - -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyAccess*& policy) const { - assert(item_type == ItemType::ACCESS); - return m_access_set.getPolicy(policy_type, policy_type_value, policy); -} - +/****************** NaivePolicyDb::PolicyOwn ************************/ void NaivePolicyDb::PolicyOwn::addItem(ItemOwn &item) { ownership_tree.addItem(item); } @@ -271,6 +357,7 @@ size_t NaivePolicyDb::PolicyOwn::getSize() const { return ownership_tree.getSize(); } +/****************** NaivePolicyDb::PolicyAccess ************************/ DecisionItem NaivePolicyDb::PolicyAccess::getDecisionItem(const MatchItemAccess& query) const { // All group and user rules are applied in the order in which they appear in config, so: @@ -303,85 +390,3 @@ void NaivePolicyDb::PolicyAccess::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } - -void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const -{ - auto &vsend = m_send_set.getMapGroup(uid); - auto &vrecv = m_receive_set.getMapGroup(uid); - auto &vaccess = m_access_set.getMapGroup(uid); - - if (groups.empty()) { - vsend.push_back(gid); - vrecv.push_back(gid); - vaccess.push_back(gid); - return; - } - - /* insert supplementary group */ - for (const auto group : groups) { - if (m_send_set.group.find(group) != m_send_set.group.end()) - vsend.push_back(group); - if (m_receive_set.group.find(group) != m_receive_set.group.end()) - vrecv.push_back(group); - vaccess.push_back(group); // no filtering, it will be used once - } - - if (vsend.empty()) - vsend.push_back(-1); - if (vrecv.empty()) - vrecv.push_back(-1); -} - -void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, gid_t gid) const -{ - auto &vown = m_own_set.getMapGroup(uid); - if (groups.empty()) { - vown.push_back(gid); - } else { - for (const auto group : groups) { - if (m_own_set.group.find(group) != m_own_set.group.end()) - vown.push_back(group); - } - } -} - -const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemType type) 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); - - pthread_mutex_lock(&mutexGroup); - auto &vgid = (type == ItemType::SEND) ? m_send_set.getMapGroup(uid) : m_receive_set.getMapGroup(uid); - - if (vgid.empty()) - updateSupplementaryGroups(get_groups(uid, gid), uid, gid); - pthread_mutex_unlock(&mutexGroup); - - if (vgid[0] == (gid_t)-1) - return nullptr; - - return &vgid; -} - -void NaivePolicyDb::initializeGroups(uid_t uid, gid_t gid) -{ - pthread_mutex_lock(&mutexGroup); - - m_send_set.clearMapGroup(); - m_receive_set.clearMapGroup(); - m_own_set.clearMapGroup(); - m_access_set.clearMapGroup(); - - auto groups = get_groups(uid, gid); - updateSupplementaryGroups(groups, uid, gid); - updateSupplementaryGroupsOwn(groups, uid, gid); - - pthread_mutex_unlock(&mutexGroup); -} -- 2.7.4 From da18c3f00d5ddddb083d7345dad33aad9485ac25 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 11:13:37 +0100 Subject: [PATCH 05/16] refactoring: make Policy classes more private Change-Id: If2724b9f8de024da28e04710a81ba8435dcdf515 --- src/internal/naive_policy_checker.cpp | 67 ++++++++--------------------------- src/internal/naive_policy_checker.hpp | 38 -------------------- src/internal/naive_policy_db.cpp | 25 +++++++++++++ src/internal/naive_policy_db.hpp | 10 ++++++ 4 files changed, 50 insertions(+), 90 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 2eda4e0..05fc74b 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -84,60 +84,24 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, return parseDecision(ret, uid, label); } -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySend &policy, - const MatchItemSend &item) const -{ - tslog::log_verbose("Checking send policy for: ", item, "\n"); - - return policy.getDecisionItem(item); -} - -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyReceive &policy, - const MatchItemReceive &item) const -{ - tslog::log_verbose("Checking receive policy for: ", item, "\n"); - - return policy.getDecisionItem(item); -} - -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyOwn& policy, const MatchItemOwn& item) const -{ - tslog::log_verbose("Checking policy for name: ", std::string(item.getName().empty() ? "NULL" : item.getName()), "\n"); - - return policy.getDecisionItem(item); -} - -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyAccess& policy, const MatchItemAccess& item) const -{ - tslog::log_verbose("Checking access policy for ", item, "\n"); - - return policy.getDecisionItem(item); -} - template DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item, const ItemType type) { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); - DecisionItem ret = Decision::ANY; - const P* curr_policy = nullptr; - if (ret.getDecision() == Decision::ANY) { - if (policy_db.getPolicy(type, PolicyType::CONTEXT, PolicyTypeValue(ContextType::MANDATORY), curr_policy)) - ret = checkPolicy(*curr_policy, item); - } + + DecisionItem ret = policy_db.getDecisionItem(type, 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) { - if (policy_db.getPolicy(type, PolicyType::USER, PolicyTypeValue(uid), curr_policy)) - ret = checkPolicy(*curr_policy, item); - } - if (ret.getDecision() == Decision::ANY) { + if (ret.getDecision() == Decision::ANY) + ret = policy_db.getDecisionItem(type, PolicyType::USER, PolicyTypeValue(uid), item); + + if (ret.getDecision() == Decision::ANY) ret = checkGroupPolicies(policy_db, uid, gid, item, type); - } - } - if (ret.getDecision() == Decision::ANY) { - if (policy_db.getPolicy(type, PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), curr_policy)) - ret = checkPolicy(*curr_policy, item); } + if (ret.getDecision() == Decision::ANY) + ret = policy_db.getDecisionItem(type, PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); + return ret; } @@ -146,13 +110,12 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ const auto *sgroups = policy_db.getGroups(uid, gid, type); if (sgroups == nullptr) return Decision::ANY; - const P* curr_policy = nullptr; + for (auto sgid : *sgroups) { - if (policy_db.getPolicy(type, PolicyType::GROUP, PolicyTypeValue(sgid), curr_policy)) { - DecisionItem ret = checkPolicy(*curr_policy, item); - if (ret.getDecision() != Decision::ANY) - return ret; - } + DecisionItem ret = policy_db.getDecisionItem(type, PolicyType::GROUP, PolicyTypeValue(sgid), item); + + if (ret.getDecision() != Decision::ANY) + return ret; } return Decision::ANY; } diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index efee9da..7d6c655 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -45,44 +45,6 @@ namespace ldp_xml_parser */ NaivePolicyDb& getPolicyDb(bool type); - /** Checks send policy for given item - * \param[in] policy Policy to check - * \param[in] item Item to check - * \return Returns decision retrieved from policy - * \ingroup Implementation - * \callgraph - */ - DecisionItem checkPolicy(const NaivePolicyDb::PolicySend &policy, - const MatchItemSend &item) const; - - /** Checks receive policy for given item - * \param[in] policy Policy to check - * \param[in] item Item to check - * \return Returns decision retrieved from policy - * \ingroup Implementation - * \callgraph - */ - DecisionItem checkPolicy(const NaivePolicyDb::PolicyReceive &policy, - const MatchItemReceive &item) const; - - /** Checks ownership policy for given item - * \param[in] policy Policy to check - * \param[in] item Item to check - * \return Returns decision retrieved from policy - * \ingroup Implementation - */ - DecisionItem checkPolicy(const NaivePolicyDb::PolicyOwn& policy, - const MatchItemOwn& item) const; - - /** Checks access policy for given item - * \param[in] policy Policy to check - * \param[in] item Item to check - * \return Returns decision retrieved from policy - * \ingroup Implementation - */ - DecisionItem checkPolicy(const NaivePolicyDb::PolicyAccess& policy, - const MatchItemAccess& item) const; - /** Parses delivered decision. In case of Decision::CHECK calls cynara. * \param[in] decision Decision from checkers * \param[in] uid User id diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 442e0c6..fc30042 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -20,6 +20,13 @@ void log_item_add(const T &item) { } } +// Definition of constexpr static member requires redeclaration at namespace level. +// Deprecated with C++17. +constexpr const char *NaivePolicyDb::PolicyOwn::name; +constexpr const char *NaivePolicyDb::PolicySend::name; +constexpr const char *NaivePolicyDb::PolicyReceive::name; +constexpr const char *NaivePolicyDb::PolicyAccess::name; + /********************* NaivePolicyDb **********************/ void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, @@ -74,6 +81,24 @@ void NaivePolicyDb::printContent() const #undef PRINT_MAP } +template +DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, + PolicyType policy_type, + PolicyTypeValue policy_type_value, + const T &item) const { + const P *curr_policy = nullptr; + if (!getPolicy(type, policy_type, policy_type_value, curr_policy)) + return Decision::ANY; + + tslog::log_verbose("Checking ", P::name, " policy for: ", item, "\n"); + + return curr_policy->getDecisionItem(item); +} +template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemOwn &item) const; +template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemSend &item) const; +template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemReceive &item) const; +template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemAccess &item) const; + bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyType policy_type, const PolicyTypeValue policy_type_value, diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 1892f22..f117876 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -55,9 +55,11 @@ namespace ldp_xml_parser }; class PolicySend : public PolicySR { public: + static constexpr const char *name = "send"; }; class PolicyReceive : public PolicySR { public: + static constexpr const char *name = "receive"; }; /** Class containing policy with ownership rules */ @@ -73,6 +75,8 @@ namespace ldp_xml_parser DecisionItem getDecisionItem(const MatchItemOwn& item) const; void printContent() const; size_t getSize() const; + + static constexpr const char *name = "own"; }; /** Class containing policy with access rules */ @@ -85,6 +89,8 @@ namespace ldp_xml_parser DecisionItem getDecisionItem(const MatchItemAccess& item) const; void printContent() const; size_t getSize() const; + + static constexpr const char *name = "access"; }; /** Gets policy with ownership rules from DB @@ -221,6 +227,10 @@ namespace ldp_xml_parser void addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemAccess &item); + + template + DecisionItem getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, + const T &item) const; }; } #endif -- 2.7.4 From eee77963d6c4aa27e506b9ea22d309179898c32b Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 11:37:12 +0100 Subject: [PATCH 06/16] refactoring: simplify NaivePolicyDb::getPolicy() Change-Id: I3be0a3682dd7be9510a0145d16cd3b750ee129c9 --- src/internal/naive_policy_checker.cpp | 8 ++-- src/internal/naive_policy_db.cpp | 85 +++++++++++++++++------------------ src/internal/naive_policy_db.hpp | 53 ++++------------------ 3 files changed, 52 insertions(+), 94 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 05fc74b..12244cd 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -88,19 +88,19 @@ template DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item, const ItemType type) { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); - DecisionItem ret = policy_db.getDecisionItem(type, 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(type, PolicyType::USER, PolicyTypeValue(uid), item); + ret = policy_db.getDecisionItem(PolicyType::USER, PolicyTypeValue(uid), item); if (ret.getDecision() == Decision::ANY) ret = checkGroupPolicies(policy_db, uid, gid, item, type); } if (ret.getDecision() == Decision::ANY) - ret = policy_db.getDecisionItem(type, PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); + ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); return ret; } @@ -112,7 +112,7 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ return Decision::ANY; for (auto sgid : *sgroups) { - DecisionItem ret = policy_db.getDecisionItem(type, PolicyType::GROUP, PolicyTypeValue(sgid), item); + DecisionItem ret = policy_db.getDecisionItem(PolicyType::GROUP, PolicyTypeValue(sgid), item); if (ret.getDecision() != Decision::ANY) return ret; diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index fc30042..34a07fa 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -82,53 +82,52 @@ void NaivePolicyDb::printContent() const } template -DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, - PolicyType policy_type, +DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const { - const P *curr_policy = nullptr; - if (!getPolicy(type, policy_type, policy_type_value, curr_policy)) + auto policy = getPolicy

(policy_type, policy_type_value); + if (nullptr == policy) return Decision::ANY; tslog::log_verbose("Checking ", P::name, " policy for: ", item, "\n"); - return curr_policy->getDecisionItem(item); + return policy->getDecisionItem(item); } -template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemOwn &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemSend &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemReceive &item) const; -template DecisionItem NaivePolicyDb::getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, const MatchItemAccess &item) const; +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; -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyOwn*& policy) const { - assert(item_type == ItemType::OWN); - return m_own_set.getPolicy(policy_type, policy_type_value, policy); +template +const P *NaivePolicyDb::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { + return nullptr; +} + +namespace ldp_xml_parser { +template <> +const NaivePolicyDb::PolicyOwn *NaivePolicyDb::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { + return m_own_set.getPolicy(policy_type, policy_type_value); } -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicySend*& policy) const { - assert(item_type == ItemType::SEND); - return m_send_set.getPolicy(policy_type, policy_type_value, policy); +template <> +const NaivePolicyDb::PolicySend *NaivePolicyDb::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { + return m_send_set.getPolicy(policy_type, policy_type_value); } -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyReceive*& policy) const { - assert(item_type == ItemType::RECEIVE); - return m_receive_set.getPolicy(policy_type, policy_type_value, policy); +template <> +const NaivePolicyDb::PolicyReceive *NaivePolicyDb::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { + return m_receive_set.getPolicy(policy_type, policy_type_value); } -bool NaivePolicyDb::getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const NaivePolicyDb::PolicyAccess*& policy) const { - assert(item_type == ItemType::ACCESS); - return m_access_set.getPolicy(policy_type, policy_type_value, policy); +template <> +const NaivePolicyDb::PolicyAccess *NaivePolicyDb::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { + return m_access_set.getPolicy(policy_type, policy_type_value); +} } void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const @@ -265,17 +264,15 @@ void NaivePolicyDb::PolicySet

::printSet() const } template -bool NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const P*& policy) const +const P *NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const { tslog::log("---policy_type ="); switch (policy_type) { case PolicyType::CONTEXT: tslog::log("CONTEXT =", (int)policy_type_value.context, "\n"); - policy = &context[static_cast(policy_type_value.context)]; - return true; + return &context[static_cast(policy_type_value.context)]; case PolicyType::USER: { tslog::log("USER =", (int)policy_type_value.user, "\n"); @@ -283,11 +280,10 @@ bool NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, auto it = user.find(policy_type_value.user); if (it == user.end()) { tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return false; + return nullptr; } - policy = &(it->second); + return &(it->second); } - return true; case PolicyType::GROUP: { tslog::log("GROUP = ", (int)policy_type_value.group, "\n"); @@ -295,16 +291,15 @@ bool NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, auto it = group.find(policy_type_value.group); if (it == group.end()) { tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return false; + return nullptr; } - policy = &(it->second); + return &(it->second); } - return true; default: tslog::log("NO POLICY\n"); } - return false; + return nullptr; } template template diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index f117876..93c0378 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -93,40 +93,11 @@ namespace ldp_xml_parser static constexpr const char *name = "access"; }; - /** Gets policy with ownership rules from DB - * \param[in] policy_type Policy type - * \param[in] policy_type_value Policy type value - * \param[out] policy Received policy - * \return True if there is such policy, false elsewhere - */ - bool getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const PolicyOwn*& policy) const; - - /** Gets policy with send/receive rules from DB - * \param[in] item_type Item Type - * \param[in] policy_type Policy type - * \param[in] policy_type_value Policy type value - * \param[out] policy Received policy - * \return True if there is such policy, false elsewhere - */ - bool getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const PolicySend*& policy) const; - - bool getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const PolicyReceive*& policy) const; - - bool getPolicy(const ItemType item_type, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const PolicyAccess*& policy) const; - private: + template + const P *getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const; + typedef std::vector VGid; template @@ -151,17 +122,6 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, T &item); - /** Gets requested policy - * \param[in] set Set to add item to - * \param[in] policy_type Policy type - * \param[in] policy_type_value Policy type value - * \param[out] policy Received policy - * \return False if there is no such policy, true elsewhere - */ - bool getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const P*& policy) const; - void printSet() const; void printMap() const; @@ -169,6 +129,9 @@ namespace ldp_xml_parser VGid &getMapGroup(uid_t uid) const { return mapGroup[uid]; } void clearMapGroup() { mapGroup.clear(); } + + const P *getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value) const; }; /** Set of ownership policies */ @@ -229,7 +192,7 @@ namespace ldp_xml_parser ItemAccess &item); template - DecisionItem getDecisionItem(ItemType type, PolicyType policy_type, PolicyTypeValue policy_type_value, + DecisionItem getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const; }; } -- 2.7.4 From 7bad686cd639c791e14480f1ec8856f1e99e550c Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 12:05:53 +0100 Subject: [PATCH 07/16] refactoring: eliminate template params in getDecisionItem Affects dependencies, which also lost unnecessary params. Policy classes can be now private, it allows single private and single public section in NaivePolicyDb. Change-Id: I5f01d2e43f2ffd3f58ef2aa76b1cd06d83365b7b --- src/internal/naive_policy_checker.cpp | 22 +++++++-------- src/internal/naive_policy_checker.hpp | 4 +-- src/internal/naive_policy_db.cpp | 51 ++++++++++++++++++++++++++++++----- src/internal/naive_policy_db.hpp | 9 ++++--- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 12244cd..b0a8fb2 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -46,7 +46,7 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, 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); + auto ret = checkItem(bus_type, uid, gid, MatchItemAccess(uid, gids), ItemType::ACCESS); if (ret.getDecision() == Decision::ANY) { if (bus_owner == uid) { ret = Decision::ALLOW; @@ -60,7 +60,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, name, ItemType::OWN); return parseDecision(ret, uid, label); } @@ -70,7 +70,7 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, const char* const label, MatchItemSend &matcher, ItemType type) { - auto ret = checkItem(bus_type, uid, gid, matcher, type); + auto ret = checkItem(bus_type, uid, gid, matcher, type); return parseDecision(ret, uid, label); } @@ -80,39 +80,39 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, const char* const label, MatchItemReceive &matcher, ItemType type) { - auto ret = checkItem(bus_type, uid, gid, matcher, type); + auto ret = checkItem(bus_type, uid, gid, matcher, type); return parseDecision(ret, uid, label); } -template +template DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item, const ItemType type) { 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); + ret = policy_db.getDecisionItem(PolicyType::USER, PolicyTypeValue(uid), item); if (ret.getDecision() == Decision::ANY) - ret = checkGroupPolicies(policy_db, uid, gid, item, type); + 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 +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); if (sgroups == nullptr) return Decision::ANY; for (auto sgid : *sgroups) { - DecisionItem ret = policy_db.getDecisionItem(PolicyType::GROUP, PolicyTypeValue(sgid), item); + 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 7d6c655..4713e97 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -67,14 +67,14 @@ namespace ldp_xml_parser * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ - template + template DecisionItem checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item, const ItemType type); - template + template DecisionItem checkGroupPolicies(const NaivePolicyDb& policy_db, uid_t uid, gid_t gid, diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 34a07fa..f0fdd39 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -13,6 +13,43 @@ using namespace ldp_xml_parser; +namespace ldp_xml_parser { + +/* Tie MatchItems with proper policy sets. + * Parameters: T - MatchItem* type + * field - one of m_*_set fields from NaivePolicyDb + * + * The below DEF_GET_POLICY_SET defines specialization for MatchPolicy template. + * + * 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. + * + * For example: DEF_GET_POLICY_SET(MatchItemOwn, m_own_set) defines equivalent to + * MatchPolicy { + * typedef PolicyOwn policy_type; + * typedef PolicySet policy_set_type; + * } + * + * Thanks to this construction we do not need to manually specify PolicyOwn in functions that + * know MatchItemOwn - it is inferred. + * + */ +#define DEF_GET_POLICY_SET(T, field) \ + template <> struct NaivePolicyDb::MatchPolicy { \ + typedef decltype(field)::type policy_type; \ + typedef decltype(field) policy_set_type; \ + }; \ + +DEF_GET_POLICY_SET(MatchItemOwn, m_own_set) +DEF_GET_POLICY_SET(MatchItemSend, m_send_set) +DEF_GET_POLICY_SET(MatchItemReceive, m_receive_set) +DEF_GET_POLICY_SET(MatchItemAccess, m_access_set) + +#undef DEF_GET_POLICY_SET + +} // namespace ldp_xml_parser + namespace { template void log_item_add(const T &item) { @@ -81,22 +118,22 @@ void NaivePolicyDb::printContent() const #undef PRINT_MAP } -template +template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const { - auto policy = getPolicy

(policy_type, policy_type_value); + auto policy = getPolicy::policy_type>(policy_type, policy_type_value); if (nullptr == policy) return Decision::ANY; - tslog::log_verbose("Checking ", P::name, " policy for: ", item, "\n"); + tslog::log_verbose("Checking ", MatchPolicy::policy_type::name, " policy for: ", item, "\n"); 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 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, diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 93c0378..72dcb5b 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -31,7 +31,7 @@ namespace ldp_xml_parser { /** Database class, contains policies for ownership and send/receive */ class NaivePolicyDb { - public: + private: /** Class containing policy with send/receive rules */ template class PolicySR { @@ -93,7 +93,6 @@ namespace ldp_xml_parser static constexpr const char *name = "access"; }; - private: template const P *getPolicy(const PolicyType policy_type, const PolicyTypeValue policy_type_value) const; @@ -104,6 +103,7 @@ namespace ldp_xml_parser class PolicySet { mutable std::map mapGroup; public: + typedef P type; /** Policies for all contexts */ P context[static_cast(ContextType::MAX)]; /** Map with policies for different users */ @@ -146,6 +146,9 @@ namespace ldp_xml_parser /* A mutex for mapGroups within sets */ mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; + template + struct MatchPolicy; // provide policy_type in specializations + 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: @@ -191,7 +194,7 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, ItemAccess &item); - template + template DecisionItem getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const; }; -- 2.7.4 From 3feb35fbf88397e4155c08faedd1684dfc3b0b76 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 13:20:08 +0100 Subject: [PATCH 08/16] 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 From 10cdd899509b71b23c107a6a2adb891433e5966d Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 13:57:15 +0100 Subject: [PATCH 09/16] refactoring: eliminate NaivePolicyDb::getPolicy() Change-Id: I4378947432a01974f27ebf115e785556e52155d5 --- src/internal/naive_policy_db.cpp | 34 +--------------------------------- src/internal/naive_policy_db.hpp | 4 ---- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 304664e..8fb4609 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -129,7 +129,7 @@ template DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, const T &item) const { - auto policy = getPolicy::policy_type>(policy_type, policy_type_value); + auto policy = getPolicySet().getPolicy(policy_type, policy_type_value); if (nullptr == policy) return Decision::ANY; @@ -138,38 +138,6 @@ DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, return policy->getDecisionItem(item); } -template -const P *NaivePolicyDb::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const { - return nullptr; -} - -namespace ldp_xml_parser { -template <> -const NaivePolicyDb::PolicyOwn *NaivePolicyDb::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const { - return m_own_set.getPolicy(policy_type, policy_type_value); -} - -template <> -const NaivePolicyDb::PolicySend *NaivePolicyDb::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const { - return m_send_set.getPolicy(policy_type, policy_type_value); -} - -template <> -const NaivePolicyDb::PolicyReceive *NaivePolicyDb::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const { - return m_receive_set.getPolicy(policy_type, policy_type_value); -} - -template <> -const NaivePolicyDb::PolicyAccess *NaivePolicyDb::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const { - return m_access_set.getPolicy(policy_type, policy_type_value); -} -} - void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const { auto &vsend = m_send_set.getMapGroup(uid); diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 68d4dd2..29782f3 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -93,10 +93,6 @@ namespace ldp_xml_parser static constexpr const char *name = "access"; }; - template - const P *getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const; - typedef std::vector VGid; template -- 2.7.4 From 1bc99fe16be86a2de7d8fcc33f5fb13aafb6c4ca Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 14:17:09 +0100 Subject: [PATCH 10/16] refactoring: templatize NaivePolicyChecker::check This converts check() methods for MatchItemOwn, MatchItemSend and MatchItemReceive to single template method. The MatchItemAccess check() method stays in place as it requires different params. Change-Id: Ibbc2c0498bb9289f6dbce5ffd3206abf93115d17 --- src/internal/internal.cpp | 12 +++++++----- src/internal/naive_policy_checker.cpp | 26 ++++++-------------------- src/internal/naive_policy_checker.hpp | 28 ++++------------------------ 3 files changed, 17 insertions(+), 49 deletions(-) diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index 6682767..f142cca 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -28,6 +28,8 @@ #include "internal.h" #include "tslog.hpp" +using namespace ldp_xml_parser; + static const char* get_str(const char* const szstr) { return (szstr != NULL) ? szstr : ""; } @@ -97,7 +99,7 @@ int __internal_can_send(bool bus_type, const char* const member, int type) { - ldp_xml_parser::MatchItemSend matcher(interface, member, path, static_cast(type)); + MatchItemSend matcher(interface, member, path, static_cast(type)); if (!matcher.addNames(destination)) { tslog::log_verbose("Destination too long: ", destination, "\n"); return false; @@ -116,7 +118,7 @@ int __internal_can_send_multi_dest(bool bus_type, int type) { int i = 0; - ldp_xml_parser::MatchItemSend matcher(interface, member, path, static_cast(type)); + MatchItemSend matcher(interface, member, path, static_cast(type)); if (destination) while (destination[i]) { matcher.addName(destination[i++]); @@ -134,7 +136,7 @@ int __internal_can_recv(bool bus_type, const char* const member, int type) { - ldp_xml_parser::MatchItemReceive matcher(interface, member, path, static_cast(type)); + MatchItemReceive matcher(interface, member, path, static_cast(type)); if (!matcher.addNames(sender)) { tslog::log_verbose("Sender too long: ", sender, "\n"); return false; @@ -153,7 +155,7 @@ int __internal_can_recv_multi(bool bus_type, int type) { int i = 0; - ldp_xml_parser::MatchItemReceive matcher(interface, member, path, static_cast(type)); + MatchItemReceive matcher(interface, member, path, static_cast(type)); if (sender) while (sender[i]) { matcher.addName(sender[i++]); @@ -168,5 +170,5 @@ int __internal_can_own(bool bus_type, const char* const label, const char* const service) { - return static_cast(policy_checker().check(bus_type, user, group, label, service)); + return static_cast(policy_checker().check(bus_type, user, group, label, MatchItemOwn(service))); } diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index c358b97..2ba566d 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -69,32 +69,18 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, return parseDecision(ret, uid, label); } +template DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - const char* const name) { - auto ret = checkItem(bus_type, uid, gid, MatchItemOwn(name)); - return parseDecision(ret, uid, label); -} - -DecisionResult NaivePolicyChecker::check(bool bus_type, - uid_t uid, - gid_t gid, - const char* const label, - MatchItemSend &matcher) { - auto ret = checkItem(bus_type, uid, gid, matcher); - return parseDecision(ret, uid, label); -} - -DecisionResult NaivePolicyChecker::check(bool bus_type, - uid_t uid, - gid_t gid, - const char* const label, - MatchItemReceive &matcher) { - auto ret = checkItem(bus_type, uid, gid, matcher); + const T &matchItem) { + auto ret = checkItem(bus_type, uid, gid, matchItem); return parseDecision(ret, uid, label); } +template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemOwn &); +template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemSend &); +template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemReceive &); template DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item) { diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index e5e4b79..a1b0cdc 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -113,41 +113,21 @@ namespace ldp_xml_parser gid_t gid, const char* const label); - /** Checks ownership policy for given item + /** Checks send/receive/ownership policy for given 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] name Name to own + * \param[in] matchItem MatchItem to check * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ + template DecisionResult check(bool bus_type, uid_t uid, gid_t gid, const char* const label, - const char* const name); - - /** Checks send/receive policy for given 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] matcher Structure with multiple names to check - * \return Returns deny=0, allow=1 or cynara error - * \ingroup Implementation - */ - DecisionResult check(bool bus_type, - uid_t uid, - gid_t gid, - const char* const label, - MatchItemSend &matcher); - - DecisionResult check(bool bus_type, - uid_t uid, - gid_t gid, - const char* const label, - MatchItemReceive &matcher); + const T &matchItem); }; } -- 2.7.4 From 3271ca977e1e40afddeb85f10afac99ab6f330df Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 15:53:06 +0100 Subject: [PATCH 11/16] refactoring: prepare for PolicySet customization PolicySets are composed of context, user, and group policies. However, PolicySet for PolicyAccess does not need user and group policies. This commit prepares base classes for PolicySets that will allow handy customization of behaviour and composition. Change-Id: Idde9daf06e2caa2975d899b99f091ece49b66113 --- src/internal/naive_policy_db.cpp | 137 +++++++++++++++++++++++++-------------- src/internal/naive_policy_db.hpp | 40 ++++++++++-- 2 files changed, 124 insertions(+), 53 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 8fb4609..134c8bb 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -153,9 +153,9 @@ void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid /* insert supplementary group */ for (const auto group : groups) { - if (m_send_set.group.find(group) != m_send_set.group.end()) + if (m_send_set.getPolicyGroup(group) != nullptr) vsend.push_back(group); - if (m_receive_set.group.find(group) != m_receive_set.group.end()) + if (m_receive_set.getPolicyGroup(group) != nullptr) vrecv.push_back(group); vaccess.push_back(group); // no filtering, it will be used once } @@ -173,7 +173,7 @@ void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, vown.push_back(gid); } else { for (const auto group : groups) { - if (m_own_set.group.find(group) != m_own_set.group.end()) + if (m_own_set.getPolicyGroup(group) != nullptr) vown.push_back(group); } } @@ -228,28 +228,93 @@ void NaivePolicyDb::initializeGroups(uid_t uid, gid_t gid) pthread_mutex_unlock(&mutexGroup); } -/****************** NaivePolicyDb::PolicySet ************************/ +/****************** NaivePolicyDb::UserAndGroupContext **************/ template -size_t NaivePolicyDb::PolicySet

::getSetSize() const -{ - size_t size = sizeof(*this); - - for (const auto e : { ContextType::DEFAULT, ContextType::MANDATORY }) - size += context[static_cast(e)].getSize(); - - size += user.size() * sizeof(typename decltype(user)::value_type); +size_t NaivePolicyDb::UserAndGroupContext

::getSize() const { + size_t size = user.size() * sizeof(typename decltype(user)::value_type); for (const auto& i : user) size += i.second.getSize(); size += group.size() * sizeof(typename decltype(group)::value_type); for (const auto& i : group) size += i.second.getSize(); - return size; } template -void NaivePolicyDb::PolicySet

::printMap() const +void NaivePolicyDb::UserAndGroupContext

::printContent() const { + for (const auto& i : user) + i.second.printContent(); + + for (const auto& i : group) + i.second.printContent(); +} + +template +const P *NaivePolicyDb::UserAndGroupContext

::getPolicyUser(uid_t uid) const { + tslog::log("USER =", uid, "\n"); + + auto it = user.find(uid); + if (it == user.end()) { + tslog::log_verbose("GetPolicy: Out of Range exception\n"); + return nullptr; + } + return &(it->second); +} + +template +const P *NaivePolicyDb::UserAndGroupContext

::getPolicyGroup(gid_t gid) const { + tslog::log("GROUP = ", gid, "\n"); + + auto it = group.find(gid); + if (it == group.end()) { + tslog::log_verbose("GetPolicy: Out of Range exception\n"); + return nullptr; + } + return &(it->second); +} + +template template +void NaivePolicyDb::UserAndGroupContext

::addItemUser(uid_t uid, T &item) +{ + user[uid].addItem(item); +} + +template template +void NaivePolicyDb::UserAndGroupContext

::addItemGroup(gid_t gid, T &item) +{ + group[gid].addItem(item); +} + +/****************** NaivePolicyDb::NoUserAndGroupContext ************/ +template +void NaivePolicyDb::NoUserAndGroupContext::addItemUser(uid_t, T &) +{ + assert(false); +} + +template +void NaivePolicyDb::NoUserAndGroupContext::addItemGroup(gid_t, T &) +{ + assert(false); +} + +/****************** NaivePolicyDb::PolicySet ************************/ +template +size_t NaivePolicyDb::PolicySet::getSetSize() const +{ + size_t size = sizeof(*this); + + for (const auto e : { ContextType::DEFAULT, ContextType::MANDATORY }) + size += context[static_cast(e)].getSize(); + + size += UG::getSize(); + + return size; +} + +template +void NaivePolicyDb::PolicySet::printMap() const { int size = sizeof(mapGroup) + mapGroup.size() * sizeof(typename std::remove_reference::type::value_type); @@ -264,23 +329,19 @@ void NaivePolicyDb::PolicySet

::printMap() const std::cerr << "Memory consumption: " << size << " B" << std::endl; } -template -void NaivePolicyDb::PolicySet

::printSet() const +template +void NaivePolicyDb::PolicySet::printSet() const { for (const auto e : {ContextType::DEFAULT, ContextType::MANDATORY}) context[static_cast(e)].printContent(); - for (const auto& i : user) - i.second.printContent(); - - for (const auto& i : group) - i.second.printContent(); + UG::printContent(); std::cerr << "Memory consumption: " << getSetSize() << " B" << std::endl; } -template -const P *NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, +template +const P *NaivePolicyDb::PolicySet::getPolicy(const PolicyType policy_type, const PolicyTypeValue policy_type_value) const { tslog::log("---policy_type ="); @@ -290,27 +351,9 @@ const P *NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, tslog::log("CONTEXT =", (int)policy_type_value.context, "\n"); return &context[static_cast(policy_type_value.context)]; case PolicyType::USER: - { - tslog::log("USER =", (int)policy_type_value.user, "\n"); - - auto it = user.find(policy_type_value.user); - if (it == user.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return nullptr; - } - return &(it->second); - } + return UG::getPolicyUser(policy_type_value.user); case PolicyType::GROUP: - { - tslog::log("GROUP = ", (int)policy_type_value.group, "\n"); - - auto it = group.find(policy_type_value.group); - if (it == group.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return nullptr; - } - return &(it->second); - } + return UG::getPolicyGroup(policy_type_value.group); default: tslog::log("NO POLICY\n"); } @@ -318,8 +361,8 @@ const P *NaivePolicyDb::PolicySet

::getPolicy(const PolicyType policy_type, return nullptr; } -template template -void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, +template template +void NaivePolicyDb::PolicySet::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, T &item) { switch (policy_type) { @@ -327,10 +370,10 @@ void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, context[static_cast(policy_type_value.context)].addItem(item); break; case PolicyType::USER: - user[policy_type_value.user].addItem(item); + UG::addItemUser(policy_type_value.user, item); break; case PolicyType::GROUP: - group[policy_type_value.group].addItem(item); + UG::addItemGroup(policy_type_value.group, item); break; case PolicyType::NONE: assert(false); diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 29782f3..ba94966 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -93,19 +93,47 @@ namespace ldp_xml_parser static constexpr const char *name = "access"; }; + template + class UserAndGroupContext { + protected: + /** Map with policies for different users */ + std::map user; + /** Map with policies for different groups */ + std::map group; + + template + void addItemUser(uid_t uid, T &item); + template + void addItemGroup(gid_t gid, T &item); + + void printContent() const; + size_t getSize() const; + + public: + const P *getPolicyUser(uid_t) const; + const P *getPolicyGroup(gid_t) const; + }; + + class NoUserAndGroupContext { + protected: + template + void addItemUser(uid_t uid, T &item); + template + void addItemGroup(gid_t gid, T &item); + + void printContent() const {} + size_t getSize() const { return 0; } + }; + typedef std::vector VGid; - template - class PolicySet { + template > + class PolicySet : public UG { mutable std::map mapGroup; public: typedef P type; /** Policies for all contexts */ P context[static_cast(ContextType::MAX)]; - /** Map with policies for different users */ - std::map user; - /** Map with policies for different groups */ - std::map group; /** Adds given item to policy * \param[in] set Set to add item to -- 2.7.4 From 9de132605eb189251191836215f2772bf7f532b8 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 16:09:40 +0100 Subject: [PATCH 12/16] refactoring: simplify PolicySet This splits artificial context array into separate contextMandatory and contextDefault. Change-Id: I05a0b3a66d2f3370f73b6b4992de4a911db331f4 --- src/internal/naive_policy_db.cpp | 53 ++++++++++++++++++++++++++++------------ src/internal/naive_policy_db.hpp | 14 +++++++---- 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 134c8bb..70b61eb 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -252,7 +252,7 @@ void NaivePolicyDb::UserAndGroupContext

::printContent() const { template const P *NaivePolicyDb::UserAndGroupContext

::getPolicyUser(uid_t uid) const { - tslog::log("USER =", uid, "\n"); + tslog::log("---policy_type = USER =", uid, "\n"); auto it = user.find(uid); if (it == user.end()) { @@ -264,7 +264,7 @@ const P *NaivePolicyDb::UserAndGroupContext

::getPolicyUser(uid_t uid) const { template const P *NaivePolicyDb::UserAndGroupContext

::getPolicyGroup(gid_t gid) const { - tslog::log("GROUP = ", gid, "\n"); + tslog::log("---policy_type = GROUP = ", gid, "\n"); auto it = group.find(gid); if (it == group.end()) { @@ -305,11 +305,10 @@ size_t NaivePolicyDb::PolicySet::getSetSize() const { size_t size = sizeof(*this); - for (const auto e : { ContextType::DEFAULT, ContextType::MANDATORY }) - size += context[static_cast(e)].getSize(); + size += contextMandatory.getSize(); + size += contextDefault.getSize(); size += UG::getSize(); - return size; } @@ -332,30 +331,52 @@ void NaivePolicyDb::PolicySet::printMap() const template void NaivePolicyDb::PolicySet::printSet() const { - for (const auto e : {ContextType::DEFAULT, ContextType::MANDATORY}) - context[static_cast(e)].printContent(); + contextDefault.printContent(); + contextMandatory.printContent(); UG::printContent(); std::cerr << "Memory consumption: " << getSetSize() << " B" << std::endl; } -template +template +const P *NaivePolicyDb::PolicySet::getPolicyContextMandatory() const { + tslog::log("---policy_type = CONTEXT = MANDATORY\n"); + return &contextMandatory; +} + +template +const P *NaivePolicyDb::PolicySet::getPolicyContextDefault() const { + tslog::log("---policy_type = CONTEXT = DEFAULT\n"); + return &contextDefault; +} + +template +P &NaivePolicyDb::PolicySet::getPolicyContext(ContextType type) +{ + if (ContextType::MANDATORY == type) + return contextMandatory; + + assert(ContextType::DEFAULT == type); + return contextDefault; +} + +template const P *NaivePolicyDb::PolicySet::getPolicy(const PolicyType policy_type, const PolicyTypeValue policy_type_value) const { - tslog::log("---policy_type ="); - switch (policy_type) { case PolicyType::CONTEXT: - tslog::log("CONTEXT =", (int)policy_type_value.context, "\n"); - return &context[static_cast(policy_type_value.context)]; + if (ContextType::MANDATORY == policy_type_value.context) + return getPolicyContextMandatory(); + assert(ContextType::DEFAULT == policy_type_value.context); + return getPolicyContextDefault(); case PolicyType::USER: return UG::getPolicyUser(policy_type_value.user); case PolicyType::GROUP: return UG::getPolicyGroup(policy_type_value.group); default: - tslog::log("NO POLICY\n"); + tslog::log("---policy_type = NO POLICY\n"); } return nullptr; @@ -363,11 +384,11 @@ const P *NaivePolicyDb::PolicySet::getPolicy(const PolicyType policy_type template template void NaivePolicyDb::PolicySet::addItem(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - T &item) { + const PolicyTypeValue policy_type_value, + T &item) { switch (policy_type) { case PolicyType::CONTEXT: - context[static_cast(policy_type_value.context)].addItem(item); + getPolicyContext(policy_type_value.context).addItem(item); break; case PolicyType::USER: UG::addItemUser(policy_type_value.user, item); diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index ba94966..cc14b68 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -129,14 +129,16 @@ namespace ldp_xml_parser template > class PolicySet : public UG { + /** Policies for all contexts */ + P contextMandatory; + P contextDefault; + + P &getPolicyContext(ContextType type); + mutable std::map mapGroup; public: typedef P type; - /** Policies for all contexts */ - P context[static_cast(ContextType::MAX)]; - /** Adds given item to policy - * \param[in] set Set to add item to * \param[in] policy_type Policy type * \param[in] policy_type_value Policy type value * \param[in] item Item to add @@ -146,8 +148,10 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, T &item); - void printSet() const; + const P *getPolicyContextMandatory() const; + const P *getPolicyContextDefault() const; + void printSet() const; void printMap() const; size_t getSetSize() const; -- 2.7.4 From 20e460bdb40a42037580bedeb8dae7f17b02c296 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 5 Feb 2019 17:27:43 +0100 Subject: [PATCH 13/16] refactoring: remove user and group from access PolicySet Additionally, this adds specialized method for getting different policies to get rid of artificial parameterization with PolicyType and PolicyTypeValue. Change-Id: Ib22191aafc8d2823cb52af9b5e61ccf31d207acf --- src/internal/naive_policy_checker.cpp | 12 +++--- src/internal/naive_policy_db.cpp | 77 +++++++++++++++++++++-------------- src/internal/naive_policy_db.hpp | 27 ++++++++---- 3 files changed, 71 insertions(+), 45 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 2ba566d..82fdb2c 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -44,12 +44,12 @@ DecisionItem NaivePolicyChecker::checkItemAccess(bool bus_type, const MatchItemA { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); - DecisionItem ret = policy_db.getDecisionItem(PolicyType::CONTEXT, ContextType::MANDATORY, item); + DecisionItem ret = policy_db.getDecisionItemContextMandatory(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); + ret = policy_db.getDecisionItemContextDefault(item); return ret; } @@ -86,16 +86,16 @@ template 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.getDecisionItemContextMandatory(item); if (ret.getDecision() == Decision::ANY) - ret = policy_db.getDecisionItem(PolicyType::USER, PolicyTypeValue(uid), item); + ret = policy_db.getDecisionItemUser(uid, item); if (ret.getDecision() == Decision::ANY) ret = checkGroupPolicies(policy_db, uid, gid, item); if (ret.getDecision() == Decision::ANY) - ret = policy_db.getDecisionItem(PolicyType::CONTEXT, PolicyTypeValue(ContextType::DEFAULT), item); + ret = policy_db.getDecisionItemContextDefault(item); return ret; } @@ -107,7 +107,7 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ return Decision::ANY; for (const auto sgid : *sgroups) { - DecisionItem ret = policy_db.getDecisionItem(PolicyType::GROUP, PolicyTypeValue(sgid), item); + DecisionItem ret = policy_db.getDecisionItemGroup(sgid, item); if (ret.getDecision() != Decision::ANY) return ret; diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 70b61eb..a982bcd 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -40,7 +40,7 @@ namespace ldp_xml_parser { */ #define DEF_GET_POLICY_SET(T, field) \ template <> struct NaivePolicyDb::MatchPolicy { \ - typedef decltype(field)::type policy_type; \ + typedef decltype(field)::policy_type policy_type; \ typedef decltype(field) policy_set_type; \ }; \ template <> \ @@ -125,19 +125,47 @@ void NaivePolicyDb::printContent() const #undef PRINT_MAP } -template -DecisionItem NaivePolicyDb::getDecisionItem(PolicyType policy_type, - PolicyTypeValue policy_type_value, - const T &item) const { - auto policy = getPolicySet().getPolicy(policy_type, policy_type_value); +template +DecisionItem NaivePolicyDb::getDecisionItem(const T &item, + const P *(PS::*f)(Args ... args) const, + Args ...args) const +{ + const auto &policySet = getPolicySet(); + typedef typename std::remove_reference::type PolicySetType; + + auto policy = (policySet.*f)(args...); if (nullptr == policy) return Decision::ANY; - tslog::log_verbose("Checking ", MatchPolicy::policy_type::name, " policy for: ", item, "\n"); + tslog::log_verbose("Checking ", PolicySetType::policy_type::name, " policy for: ", item, "\n"); return policy->getDecisionItem(item); } +template +DecisionItem NaivePolicyDb::getDecisionItemContextMandatory(const T &item) const +{ + return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyContextMandatory); +} + +template +DecisionItem NaivePolicyDb::getDecisionItemContextDefault(const T &item) const +{ + return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyContextDefault); +} + +template +DecisionItem NaivePolicyDb::getDecisionItemUser(uid_t uid, const T &item) const +{ + return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyUser, uid); +} + +template +DecisionItem NaivePolicyDb::getDecisionItemGroup(gid_t gid, const T &item) const +{ + return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyGroup, gid); +} + void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const { auto &vsend = m_send_set.getMapGroup(uid); @@ -361,27 +389,6 @@ P &NaivePolicyDb::PolicySet::getPolicyContext(ContextType type) return contextDefault; } -template -const P *NaivePolicyDb::PolicySet::getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const -{ - switch (policy_type) { - case PolicyType::CONTEXT: - if (ContextType::MANDATORY == policy_type_value.context) - return getPolicyContextMandatory(); - assert(ContextType::DEFAULT == policy_type_value.context); - return getPolicyContextDefault(); - case PolicyType::USER: - return UG::getPolicyUser(policy_type_value.user); - case PolicyType::GROUP: - return UG::getPolicyGroup(policy_type_value.group); - default: - tslog::log("---policy_type = NO POLICY\n"); - } - - return nullptr; -} - template template void NaivePolicyDb::PolicySet::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, @@ -435,8 +442,6 @@ DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const TM &item) co } return Decision::ANY; } -template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSend &item) const; -template DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemReceive &item) const; /****************** NaivePolicyDb::PolicyOwn ************************/ void NaivePolicyDb::PolicyOwn::addItem(ItemOwn &item) { @@ -494,7 +499,8 @@ void NaivePolicyDb::PolicyAccess::printContent() const { /* 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 DecisionItem NaivePolicyDb::getDecisionItemContextMandatory(const T &) const; \ + template DecisionItem NaivePolicyDb::getDecisionItemContextDefault(const T &) const; \ template const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t, gid_t) const; T_INSTANTIATION(MatchItemOwn) @@ -502,4 +508,13 @@ T_INSTANTIATION(MatchItemSend) T_INSTANTIATION(MatchItemReceive) T_INSTANTIATION(MatchItemAccess) +#define T_INSTANTIATION2(T) \ + template DecisionItem NaivePolicyDb::getDecisionItemUser(uid_t, const T &) const; \ + template DecisionItem NaivePolicyDb::getDecisionItemGroup(gid_t, const T &) const; + +T_INSTANTIATION2(MatchItemOwn) +T_INSTANTIATION2(MatchItemSend) +T_INSTANTIATION2(MatchItemReceive) + #undef T_INSTANTIATION +#undef T_INSTANTIATION2 diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index cc14b68..2e6fbc2 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -137,7 +137,7 @@ namespace ldp_xml_parser mutable std::map mapGroup; public: - typedef P type; + typedef P policy_type; /** Adds given item to policy * \param[in] policy_type Policy type * \param[in] policy_type_value Policy type value @@ -157,9 +157,6 @@ namespace ldp_xml_parser VGid &getMapGroup(uid_t uid) const { return mapGroup[uid]; } void clearMapGroup() { mapGroup.clear(); } - - const P *getPolicy(const PolicyType policy_type, - const PolicyTypeValue policy_type_value) const; }; /** Set of ownership policies */ @@ -169,7 +166,7 @@ namespace ldp_xml_parser /** Set of receive policies */ PolicySet m_receive_set; /** Set of bus access policies */ - PolicySet m_access_set; + PolicySet m_access_set; /* A mutex for mapGroups within sets */ mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; @@ -180,12 +177,29 @@ namespace ldp_xml_parser template const typename MatchPolicy::policy_set_type &getPolicySet() const; + template ::policy_type, + typename PS = typename MatchPolicy::policy_set_type, + typename ...Args> + DecisionItem getDecisionItem(const T &item, + const P *(PS::*f)(Args ... args) const, + Args ...args) 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; template + DecisionItem getDecisionItemContextMandatory(const T &item) const; + template + DecisionItem getDecisionItemContextDefault(const T &item) const; + template + DecisionItem getDecisionItemUser(uid_t uid, const T &item) const; + template + DecisionItem getDecisionItemGroup(gid_t gid, const T &item) const; + + template const VGid *getGroups(uid_t uid, gid_t gid) const; void initializeGroups(uid_t uid, gid_t gid); @@ -226,9 +240,6 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, ItemAccess &item); - template - DecisionItem getDecisionItem(PolicyType policy_type, PolicyTypeValue policy_type_value, - const T &item) const; }; } #endif -- 2.7.4 From 3e018e22f5c130483981efa2ccfebe9fefb1742c Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 6 Feb 2019 08:39:37 +0100 Subject: [PATCH 14/16] refactoring: make pimpl in naive_policy_db naive_policy_db got a bit bloated, including several internal classes etc. To clean things up this introduces impl pattern, moving all the private stuff into implementation file, leaving only public nonvirtual interface. Change-Id: I32dd22bd219826994a2f52baa12aa64649ac0af2 --- src/internal/naive_policy_checker.cpp | 2 +- src/internal/naive_policy_db.cpp | 761 ++++++++++++++++++---------------- src/internal/naive_policy_db.hpp | 161 +------ 3 files changed, 423 insertions(+), 501 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 82fdb2c..e9add38 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -117,7 +117,7 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ void NaivePolicyChecker::clearDb(bool bus_type) { - m_bus_db[bus_type] = NaivePolicyDb(); + m_bus_db[bus_type].clear(); } void NaivePolicyChecker::printContent(const bool bus_type) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index a982bcd..cd686de 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -1,20 +1,371 @@ +#include "own_tree.hpp" #include "naive_policy_db.hpp" #include "cynara.hpp" #include "groups_proxy.hpp" #include "tslog.hpp" -#include -#include #include +#include /** * \file * \ingroup Implementation */ -using namespace ldp_xml_parser; - namespace ldp_xml_parser { +/****************** PolicySR: a base for PolicySend and PolicyReceive ************************/ +/** Class containing policy with send/receive rules */ +template +class PolicySR { +private: + /** Vector with policy items */ + std::vector m_items; + typedef typename decltype(m_items)::const_reverse_iterator PolicyConstIterator; +public: + PolicyConstIterator begin() const { return m_items.rbegin(); } + PolicyConstIterator end() const { return m_items.rend(); } + + /** Adds given item to policy. + * \param[in] item Item to add to policy + */ + void addItem(TI &item) { m_items.push_back(std::move(item)); } + + DecisionItem getDecisionItem(const TM &item) const { + for (auto i : *this) { + tslog::log_verbose("-read: ", i.getDecision(), " ", i, "\n"); + + if (i.match(item)) { + tslog::log_verbose("-matched: ", i.getDecision(), " ", i, "\n"); + + return i.getDecision(); + } + } + return Decision::ANY; + } + + void printContent() const { + for (const auto& i : m_items) + std::cerr << i << std::endl; + } + + size_t getSize() const { + size_t size = m_items.capacity() * sizeof(typename decltype(m_items)::value_type); + for (const auto& i : m_items) + size += i.getSize(); + return size; + } +}; + +/****************** PolicySend ************************/ +class PolicySend : public PolicySR { +public: + static constexpr const char *name = "send"; +}; +constexpr const char *PolicySend::name; + +/****************** PolicyReceive ************************/ +class PolicyReceive : public PolicySR { +public: + static constexpr const char *name = "receive"; +}; +constexpr const char *PolicyReceive::name; + +/****************** PolicyOwn ************************/ +/** Class containing policy with ownership rules */ +class PolicyOwn { +private: + class OwnershipTree ownership_tree; + +public: + /** Adds given item to tree by retrieving its name, decision and checking is it prefix. + * \param[in] item Item to add to policy + */ + void addItem(const ItemOwn &item) { ownership_tree.addItem(item); } + + DecisionItem getDecisionItem(const MatchItemOwn& item) const { + return ownership_tree.getDecisionItem(item); + } + + void printContent() const { ownership_tree.printTree(); } + + size_t getSize() const { return ownership_tree.getSize(); } + + static constexpr const char *name = "own"; +}; +constexpr const char *PolicyOwn::name; + +/****************** PolicyAccess ************************/ +/** Class containing policy with access rules */ +class PolicyAccess { +private: + std::vector m_items; + +public: + void addItem(const ItemAccess &item) { m_items.push_back(std::move(item)); } + + DecisionItem getDecisionItem(const MatchItemAccess& query) const { + // All group and user rules are applied in the order in which they appear in config, so: + // - a subsequent matching group rule overrides all previous matched user and group rules, + // - a subsequent matching user rule overrides all previous matched user and group rules. + // (according to how dbus-daemon actually works, not stated clearly in documentation) + DecisionItem ret = Decision::ANY; + for (const auto& item : m_items) { + if (item.match(query)) { + ret = item.getDecision(); + } + } + return ret; + } + + void printContent() const { + for (const auto& i : m_items) + std::cerr << i << std::endl; + } + + size_t getSize() const { + size_t size = m_items.capacity() * sizeof(decltype(m_items)::value_type); + for (const auto & i : m_items) + size += i.getSize(); + return size; + } + + static constexpr const char *name = "access"; +}; +constexpr const char *PolicyAccess::name; + +/* The following two are building blocks for PolicySet class. + * They specify if a given PolicySet class supports user and group policies. + */ +/****************** UserAndGroupContext ************************/ +template +class UserAndGroupContext { +protected: + /** Map with policies for different users */ + std::map user; + /** Map with policies for different groups */ + std::map group; + + template + void addItemUser(uid_t uid, T &item) { user[uid].addItem(item); } + + template + void addItemGroup(gid_t gid, T &item) { group[gid].addItem(item); } + + void printContent() const { + for (const auto& i : user) + i.second.printContent(); + + for (const auto& i : group) + i.second.printContent(); + } + + size_t getSize() const { + size_t size = user.size() * sizeof(typename decltype(user)::value_type); + for (const auto& i : user) + size += i.second.getSize(); + + size += group.size() * sizeof(typename decltype(group)::value_type); + for (const auto& i : group) + size += i.second.getSize(); + return size; + } + +public: + const P *getPolicyUser(uid_t uid) const { + tslog::log("---policy_type = USER =", uid, "\n"); + + auto it = user.find(uid); + if (it == user.end()) { + tslog::log_verbose("GetPolicy: Out of Range exception\n"); + return nullptr; + } + return &(it->second); + } + + const P *getPolicyGroup(gid_t gid) const { + tslog::log("---policy_type = GROUP = ", gid, "\n"); + + auto it = group.find(gid); + if (it == group.end()) { + tslog::log_verbose("GetPolicy: Out of Range exception\n"); + return nullptr; + } + return &(it->second); + } +}; + +/****************** NoUserAndGroupContext ************************/ +class NoUserAndGroupContext { +protected: + template + void addItemUser(uid_t, T &) { assert(false); } + + template + void addItemGroup(gid_t, T &) { assert(false); } + + void printContent() const {} + size_t getSize() const { return 0; } +}; + +template> +class PolicySet : public UG { + /** Policies for all contexts */ + P contextMandatory; + P contextDefault; + + P &getPolicyContext(ContextType type) { + if (ContextType::MANDATORY == type) + return contextMandatory; + + assert(ContextType::DEFAULT == type); + return contextDefault; + } + + mutable std::map mapGroup; +public: + typedef P policy_type; + /** Adds given item to policy + * \param[in] policy_type Policy type + * \param[in] policy_type_value Policy type value + * \param[in] item Item to add + */ + template + void addItem(const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + T &item) { + switch (policy_type) { + case PolicyType::CONTEXT: + getPolicyContext(policy_type_value.context).addItem(item); + break; + case PolicyType::USER: + UG::addItemUser(policy_type_value.user, item); + break; + case PolicyType::GROUP: + UG::addItemGroup(policy_type_value.group, item); + break; + case PolicyType::NONE: + assert(false); + break; + } + } + + const P *getPolicyContextMandatory() const { + tslog::log("---policy_type = CONTEXT = MANDATORY\n"); + return &contextMandatory; + } + + const P *getPolicyContextDefault() const { + tslog::log("---policy_type = CONTEXT = DEFAULT\n"); + return &contextDefault; + } + + void printSet() const { + contextDefault.printContent(); + contextMandatory.printContent(); + + UG::printContent(); + + std::cerr << "Memory consumption: " << getSetSize() << " B" << std::endl; + } + + void printMap() const { + int size = sizeof(mapGroup); + size += mapGroup.size() * sizeof(typename std::remove_reference::type::value_type); + + for (const auto& i : mapGroup) { + size += i.second.capacity() * sizeof(gid_t); + std:: cerr << "gid: " << i.first << " |"; + for (auto j : i.second) + std::cerr << " " << j << ","; + std::cerr << std::endl; + } + + std::cerr << "Memory consumption: " << size << " B" << std::endl; + } + + size_t getSetSize() const { + size_t size = sizeof(*this); + + size += contextMandatory.getSize(); + size += contextDefault.getSize(); + + size += UG::getSize(); + return size; + } + + NaivePolicyDb::VGid &getMapGroup(uid_t uid) const { return mapGroup[uid]; } + void clearMapGroup() { mapGroup.clear(); } +}; + +/****************** NaivePolicyDbImpl ************************/ +class NaivePolicyDb::NaivePolicyDbImpl { +public: + /** Set of ownership policies */ + PolicySet m_own_set; + /** Set of send policies */ + PolicySet m_send_set; + /** Set of receive policies */ + PolicySet m_receive_set; + /** Set of bus access policies */ + PolicySet m_access_set; + + /* A mutex for mapGroups within sets */ + mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; + + template + struct MatchPolicy; // provide policy_type in specializations + + template + const typename MatchPolicy::policy_set_type &getPolicySet() const; + + template ::policy_type, + typename PS = typename MatchPolicy::policy_set_type, + typename ...Args> + DecisionItem getDecisionItem(const T &item, + const P *(PS::*f)(Args ... args) const, + Args ...args) const; + + void updateSupplementaryGroups(const NaivePolicyDb::VGid &groups, uid_t uid, gid_t gid) const { + auto &vsend = m_send_set.getMapGroup(uid); + auto &vrecv = m_receive_set.getMapGroup(uid); + auto &vaccess = m_access_set.getMapGroup(uid); + + if (groups.empty()) { + vsend.push_back(gid); + vrecv.push_back(gid); + vaccess.push_back(gid); + return; + } + + /* insert supplementary group */ + for (const auto group : groups) { + if (m_send_set.getPolicyGroup(group) != nullptr) + vsend.push_back(group); + if (m_receive_set.getPolicyGroup(group) != nullptr) + vrecv.push_back(group); + vaccess.push_back(group); // no filtering, it will be used once + } + + if (vsend.empty()) + vsend.push_back(-1); + if (vrecv.empty()) + vrecv.push_back(-1); + } + + void updateSupplementaryGroupsOwn(const NaivePolicyDb::VGid &groups, uid_t uid, gid_t gid) const { + auto &vown = m_own_set.getMapGroup(uid); + if (groups.empty()) { + vown.push_back(gid); + } else { + for (const auto group : groups) { + if (m_own_set.getPolicyGroup(group) != nullptr) + vown.push_back(group); + } + } + } +}; + /* Tie MatchItems with proper policy sets. * Parameters: T - MatchItem* type * field - one of m_*_set fields from NaivePolicyDb @@ -32,19 +383,19 @@ namespace ldp_xml_parser { * typedef PolicyOwn policy_type; * typedef PolicySet policy_set_type; * } - * PolicySet &getPolicySet const; + * PolicySet &getPolicySet const; * * Thanks to this construction we do not need to manually specify PolicyOwn in functions that * know MatchItemOwn - it is inferred. * */ #define DEF_GET_POLICY_SET(T, field) \ - template <> struct NaivePolicyDb::MatchPolicy { \ + template <> struct NaivePolicyDb::NaivePolicyDbImpl::MatchPolicy { \ typedef decltype(field)::policy_type policy_type; \ typedef decltype(field) policy_set_type; \ }; \ template <> \ - auto NaivePolicyDb::getPolicySet() const -> decltype((field)) { \ + auto NaivePolicyDb::NaivePolicyDbImpl::getPolicySet() const -> decltype((field)) { \ return field; \ } @@ -55,7 +406,22 @@ DEF_GET_POLICY_SET(MatchItemAccess, m_access_set) #undef DEF_GET_POLICY_SET -} // namespace ldp_xml_parser +template +DecisionItem NaivePolicyDb::NaivePolicyDbImpl::getDecisionItem(const T &item, + const P *(PS::*f)(Args ... args) const, + Args ...args) const +{ + const auto &policySet = getPolicySet(); + typedef typename std::remove_reference::type PolicySetType; + + auto policy = (policySet.*f)(args...); + if (nullptr == policy) + return Decision::ANY; + + tslog::log_verbose("Checking ", PolicySetType::policy_type::name, " policy for: ", item, "\n"); + + return policy->getDecisionItem(item); +} namespace { template @@ -64,40 +430,33 @@ void log_item_add(const T &item) { } } -// Definition of constexpr static member requires redeclaration at namespace level. -// Deprecated with C++17. -constexpr const char *NaivePolicyDb::PolicyOwn::name; -constexpr const char *NaivePolicyDb::PolicySend::name; -constexpr const char *NaivePolicyDb::PolicyReceive::name; -constexpr const char *NaivePolicyDb::PolicyAccess::name; - /********************* NaivePolicyDb **********************/ void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemSend &item) { log_item_add(item); - m_send_set.addItem(policy_type, policy_type_value, item); + pimpl->m_send_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemReceive &item) { log_item_add(item); - m_receive_set.addItem(policy_type, policy_type_value, item); + pimpl->m_receive_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemOwn &item) { log_item_add(item); - m_own_set.addItem(policy_type, policy_type_value, item); + pimpl->m_own_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemAccess &item) { log_item_add(item); - m_access_set.addItem(policy_type, policy_type_value, item); + pimpl->m_access_set.addItem(policy_type, policy_type_value, item); } void NaivePolicyDb::printContent() const @@ -106,10 +465,10 @@ void NaivePolicyDb::printContent() const std::cerr << std::endl << "----" #x "----" << std::endl; \ (x).printSet(); - PRINT_SET(m_own_set) - PRINT_SET(m_send_set) - PRINT_SET(m_receive_set) - PRINT_SET(m_access_set) + PRINT_SET(pimpl->m_own_set) + PRINT_SET(pimpl->m_send_set) + PRINT_SET(pimpl->m_receive_set) + PRINT_SET(pimpl->m_access_set) #undef PRINT_SET @@ -117,108 +476,62 @@ void NaivePolicyDb::printContent() const std::cerr << std::endl << "----" #x "----" << std::endl; \ (x).printMap(); - PRINT_MAP(m_own_set); - PRINT_MAP(m_send_set); - PRINT_MAP(m_receive_set); - PRINT_MAP(m_access_set); + PRINT_MAP(pimpl->m_own_set); + PRINT_MAP(pimpl->m_send_set); + PRINT_MAP(pimpl->m_receive_set); + PRINT_MAP(pimpl->m_access_set); #undef PRINT_MAP } -template -DecisionItem NaivePolicyDb::getDecisionItem(const T &item, - const P *(PS::*f)(Args ... args) const, - Args ...args) const -{ - const auto &policySet = getPolicySet(); - typedef typename std::remove_reference::type PolicySetType; - - auto policy = (policySet.*f)(args...); - if (nullptr == policy) - return Decision::ANY; - - tslog::log_verbose("Checking ", PolicySetType::policy_type::name, " policy for: ", item, "\n"); - - return policy->getDecisionItem(item); -} - template DecisionItem NaivePolicyDb::getDecisionItemContextMandatory(const T &item) const { - return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyContextMandatory); + return pimpl->getDecisionItem(item, &NaivePolicyDbImpl::MatchPolicy::policy_set_type::getPolicyContextMandatory); } template DecisionItem NaivePolicyDb::getDecisionItemContextDefault(const T &item) const { - return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyContextDefault); + return pimpl->getDecisionItem(item, &NaivePolicyDbImpl::MatchPolicy::policy_set_type::getPolicyContextDefault); } template DecisionItem NaivePolicyDb::getDecisionItemUser(uid_t uid, const T &item) const { - return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyUser, uid); + return pimpl->getDecisionItem(item, &NaivePolicyDbImpl::MatchPolicy::policy_set_type::getPolicyUser, uid); } template DecisionItem NaivePolicyDb::getDecisionItemGroup(gid_t gid, const T &item) const { - return getDecisionItem(item, &MatchPolicy::policy_set_type::getPolicyGroup, gid); + return pimpl->getDecisionItem(item, &NaivePolicyDbImpl::MatchPolicy::policy_set_type::getPolicyGroup, gid); } -void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const +template <> +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t) const { - auto &vsend = m_send_set.getMapGroup(uid); - auto &vrecv = m_receive_set.getMapGroup(uid); - auto &vaccess = m_access_set.getMapGroup(uid); - - if (groups.empty()) { - vsend.push_back(gid); - vrecv.push_back(gid); - vaccess.push_back(gid); - return; - } - - /* insert supplementary group */ - for (const auto group : groups) { - if (m_send_set.getPolicyGroup(group) != nullptr) - vsend.push_back(group); - if (m_receive_set.getPolicyGroup(group) != nullptr) - vrecv.push_back(group); - vaccess.push_back(group); // no filtering, it will be used once - } - - if (vsend.empty()) - vsend.push_back(-1); - if (vrecv.empty()) - vrecv.push_back(-1); + return &pimpl->m_own_set.getMapGroup(uid); } -void NaivePolicyDb::updateSupplementaryGroupsOwn(const VGid &groups, uid_t uid, gid_t gid) const +template <> +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t) const { - auto &vown = m_own_set.getMapGroup(uid); - if (groups.empty()) { - vown.push_back(gid); - } else { - for (const auto group : groups) { - if (m_own_set.getPolicyGroup(group) != nullptr) - vown.push_back(group); - } - } + return &pimpl->m_access_set.getMapGroup(uid); } template const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid) const { if (uid == getuid() && gid == getgid()) - return &getPolicySet().getMapGroup(uid); + return &pimpl->getPolicySet().getMapGroup(uid); - pthread_mutex_lock(&mutexGroup); - auto &vgid = getPolicySet().getMapGroup(uid); + pthread_mutex_lock(&pimpl->mutexGroup); + auto &vgid = pimpl->getPolicySet().getMapGroup(uid); if (vgid.empty()) - updateSupplementaryGroups(get_groups(uid, gid), uid, gid); - pthread_mutex_unlock(&mutexGroup); + pimpl->updateSupplementaryGroups(get_groups(uid, gid), uid, gid); + pthread_mutex_unlock(&pimpl->mutexGroup); if (vgid[0] == (gid_t)-1) return nullptr; @@ -226,275 +539,29 @@ 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); + pthread_mutex_lock(&pimpl->mutexGroup); - m_send_set.clearMapGroup(); - m_receive_set.clearMapGroup(); - m_own_set.clearMapGroup(); - m_access_set.clearMapGroup(); + pimpl->m_send_set.clearMapGroup(); + pimpl->m_receive_set.clearMapGroup(); + pimpl->m_own_set.clearMapGroup(); + pimpl->m_access_set.clearMapGroup(); auto groups = get_groups(uid, gid); - updateSupplementaryGroups(groups, uid, gid); - updateSupplementaryGroupsOwn(groups, uid, gid); + pimpl->updateSupplementaryGroups(groups, uid, gid); + pimpl->updateSupplementaryGroupsOwn(groups, uid, gid); - pthread_mutex_unlock(&mutexGroup); + pthread_mutex_unlock(&pimpl->mutexGroup); } -/****************** NaivePolicyDb::UserAndGroupContext **************/ -template -size_t NaivePolicyDb::UserAndGroupContext

::getSize() const { - size_t size = user.size() * sizeof(typename decltype(user)::value_type); - for (const auto& i : user) - size += i.second.getSize(); - - size += group.size() * sizeof(typename decltype(group)::value_type); - for (const auto& i : group) - size += i.second.getSize(); - return size; -} - -template -void NaivePolicyDb::UserAndGroupContext

::printContent() const { - for (const auto& i : user) - i.second.printContent(); - - for (const auto& i : group) - i.second.printContent(); +void NaivePolicyDb::clear() { + pimpl = std::unique_ptr{new NaivePolicyDbImpl}; } -template -const P *NaivePolicyDb::UserAndGroupContext

::getPolicyUser(uid_t uid) const { - tslog::log("---policy_type = USER =", uid, "\n"); +NaivePolicyDb::NaivePolicyDb() : pimpl{new NaivePolicyDbImpl} { } - auto it = user.find(uid); - if (it == user.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return nullptr; - } - return &(it->second); -} - -template -const P *NaivePolicyDb::UserAndGroupContext

::getPolicyGroup(gid_t gid) const { - tslog::log("---policy_type = GROUP = ", gid, "\n"); - - auto it = group.find(gid); - if (it == group.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return nullptr; - } - return &(it->second); -} - -template template -void NaivePolicyDb::UserAndGroupContext

::addItemUser(uid_t uid, T &item) -{ - user[uid].addItem(item); -} - -template template -void NaivePolicyDb::UserAndGroupContext

::addItemGroup(gid_t gid, T &item) -{ - group[gid].addItem(item); -} - -/****************** NaivePolicyDb::NoUserAndGroupContext ************/ -template -void NaivePolicyDb::NoUserAndGroupContext::addItemUser(uid_t, T &) -{ - assert(false); -} - -template -void NaivePolicyDb::NoUserAndGroupContext::addItemGroup(gid_t, T &) -{ - assert(false); -} - -/****************** NaivePolicyDb::PolicySet ************************/ -template -size_t NaivePolicyDb::PolicySet::getSetSize() const -{ - size_t size = sizeof(*this); - - size += contextMandatory.getSize(); - size += contextDefault.getSize(); - - size += UG::getSize(); - return size; -} - -template -void NaivePolicyDb::PolicySet::printMap() const -{ - int size = sizeof(mapGroup) + mapGroup.size() * sizeof(typename std::remove_reference::type::value_type); - - for (const auto& i : mapGroup) { - size += i.second.capacity() * sizeof(gid_t); - std:: cerr << "gid: " << i.first << " |"; - for (auto j : i.second) - std::cerr << " " << j << ","; - std::cerr << std::endl; - } - - std::cerr << "Memory consumption: " << size << " B" << std::endl; -} - -template -void NaivePolicyDb::PolicySet::printSet() const -{ - contextDefault.printContent(); - contextMandatory.printContent(); - - UG::printContent(); - - std::cerr << "Memory consumption: " << getSetSize() << " B" << std::endl; -} - -template -const P *NaivePolicyDb::PolicySet::getPolicyContextMandatory() const { - tslog::log("---policy_type = CONTEXT = MANDATORY\n"); - return &contextMandatory; -} - -template -const P *NaivePolicyDb::PolicySet::getPolicyContextDefault() const { - tslog::log("---policy_type = CONTEXT = DEFAULT\n"); - return &contextDefault; -} - -template -P &NaivePolicyDb::PolicySet::getPolicyContext(ContextType type) -{ - if (ContextType::MANDATORY == type) - return contextMandatory; - - assert(ContextType::DEFAULT == type); - return contextDefault; -} - -template template -void NaivePolicyDb::PolicySet::addItem(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - T &item) { - switch (policy_type) { - case PolicyType::CONTEXT: - getPolicyContext(policy_type_value.context).addItem(item); - break; - case PolicyType::USER: - UG::addItemUser(policy_type_value.user, item); - break; - case PolicyType::GROUP: - UG::addItemGroup(policy_type_value.group, item); - break; - case PolicyType::NONE: - assert(false); - break; - } -} - -/****************** NaivePolicyDb::PolicySR ************************/ -template -void NaivePolicyDb::PolicySR::addItem(TI &item) { - m_items.push_back(std::move(item)); -} - -template -size_t NaivePolicyDb::PolicySR::getSize() const { - size_t size = m_items.capacity() * sizeof(typename decltype(m_items)::value_type); - for (const auto& i : m_items) - size += i.getSize(); - return size; -} - -template -void NaivePolicyDb::PolicySR::printContent() const { - for (const auto& i : m_items) - std::cerr << i << std::endl; -} - -template -DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const TM &item) const { - for (auto i : *this) { - tslog::log_verbose("-read: ", i.getDecision(), " ", i, "\n"); - - if (i.match(item)) { - tslog::log_verbose("-matched: ", i.getDecision(), " ", i, "\n"); - - return i.getDecision(); - } - } - return Decision::ANY; -} - -/****************** NaivePolicyDb::PolicyOwn ************************/ -void NaivePolicyDb::PolicyOwn::addItem(ItemOwn &item) { - ownership_tree.addItem(item); -} - -void NaivePolicyDb::PolicyOwn::printContent() const -{ - ownership_tree.printTree(); -} - -DecisionItem NaivePolicyDb::PolicyOwn::getDecisionItem(const MatchItemOwn& item) const -{ - return ownership_tree.getDecisionItem(item); -} - -size_t NaivePolicyDb::PolicyOwn::getSize() const { - return ownership_tree.getSize(); -} - -/****************** NaivePolicyDb::PolicyAccess ************************/ -DecisionItem NaivePolicyDb::PolicyAccess::getDecisionItem(const MatchItemAccess& query) const -{ - // All group and user rules are applied in the order in which they appear in config, so: - // - a subsequent matching group rule overrides all previous matched user and group rules, - // - a subsequent matching user rule overrides all previous matched user and group rules. - // (according to how dbus-daemon actually works, not stated clearly in documentation) - DecisionItem ret = Decision::ANY; - for (const auto& item : m_items) { - if (item.match(query)) { - ret = item.getDecision(); - } - } - return ret; -} - -void NaivePolicyDb::PolicyAccess::addItem(ItemAccess &item) -{ - m_items.push_back(std::move(item)); -} - -size_t NaivePolicyDb::PolicyAccess::getSize() const -{ - size_t size = m_items.capacity() * sizeof(decltype(m_items)::value_type); - for (const auto & i : m_items) - size += i.getSize(); - return size; -} - -void NaivePolicyDb::PolicyAccess::printContent() const { - for (const auto& i : m_items) - std::cerr << i << std::endl; -} +NaivePolicyDb::~NaivePolicyDb() = default; /* Explicit instantiation is needed for used public template methods defined in this file. */ @@ -518,3 +585,5 @@ T_INSTANTIATION2(MatchItemReceive) #undef T_INSTANTIATION #undef T_INSTANTIATION2 + +} // namespace ldp_xml_parser diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 2e6fbc2..f489b0a 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -16,11 +16,9 @@ #ifndef _NAIVE_DB_H #define _NAIVE_DB_H -#include +#include #include #include "policy.hpp" -#include "own_tree.hpp" - /** * \file @@ -32,162 +30,16 @@ namespace ldp_xml_parser /** Database class, contains policies for ownership and send/receive */ class NaivePolicyDb { private: - /** Class containing policy with send/receive rules */ - template - class PolicySR { - private: - /** Vector with policy items */ - std::vector m_items; - typedef typename decltype(m_items)::const_reverse_iterator PolicyConstIterator; - - PolicyConstIterator begin() const { return m_items.rbegin(); } - PolicyConstIterator end() const { return m_items.rend(); } - public: - - /** Adds given item to policy. - * \param[in] item Item to add to policy - */ - void addItem(TI &item); - - DecisionItem getDecisionItem(const TM &item) const; - void printContent() const; - size_t getSize() const; - }; - class PolicySend : public PolicySR { - public: - static constexpr const char *name = "send"; - }; - class PolicyReceive : public PolicySR { - public: - static constexpr const char *name = "receive"; - }; - - /** Class containing policy with ownership rules */ - class PolicyOwn { - private: - class OwnershipTree ownership_tree; - - public: - /** Adds given item to tree by retrieving its name, decision and checking is it prefix. - * \param[in] item Item to add to policy - */ - void addItem(ItemOwn &item); - DecisionItem getDecisionItem(const MatchItemOwn& item) const; - void printContent() const; - size_t getSize() const; - - static constexpr const char *name = "own"; - }; - - /** Class containing policy with access rules */ - class PolicyAccess { - private: - std::vector m_items; - - public: - void addItem(ItemAccess &item); - DecisionItem getDecisionItem(const MatchItemAccess& item) const; - void printContent() const; - size_t getSize() const; - - static constexpr const char *name = "access"; - }; - - template - class UserAndGroupContext { - protected: - /** Map with policies for different users */ - std::map user; - /** Map with policies for different groups */ - std::map group; - - template - void addItemUser(uid_t uid, T &item); - template - void addItemGroup(gid_t gid, T &item); - - void printContent() const; - size_t getSize() const; - - public: - const P *getPolicyUser(uid_t) const; - const P *getPolicyGroup(gid_t) const; - }; - - class NoUserAndGroupContext { - protected: - template - void addItemUser(uid_t uid, T &item); - template - void addItemGroup(gid_t gid, T &item); + class NaivePolicyDbImpl; - void printContent() const {} - size_t getSize() const { return 0; } - }; + std::unique_ptr pimpl; + public: typedef std::vector VGid; - template > - class PolicySet : public UG { - /** Policies for all contexts */ - P contextMandatory; - P contextDefault; - - P &getPolicyContext(ContextType type); - - mutable std::map mapGroup; - public: - typedef P policy_type; - /** Adds given item to policy - * \param[in] policy_type Policy type - * \param[in] policy_type_value Policy type value - * \param[in] item Item to add - */ - template - void addItem(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - T &item); - - const P *getPolicyContextMandatory() const; - const P *getPolicyContextDefault() const; - - void printSet() const; - void printMap() const; - size_t getSetSize() const; - - VGid &getMapGroup(uid_t uid) const { return mapGroup[uid]; } - void clearMapGroup() { mapGroup.clear(); } - }; - - /** Set of ownership policies */ - PolicySet m_own_set; - /** Set of send policies */ - PolicySet m_send_set; - /** Set of receive policies */ - PolicySet m_receive_set; - /** Set of bus access policies */ - PolicySet m_access_set; + NaivePolicyDb(); + ~NaivePolicyDb(); - /* A mutex for mapGroups within sets */ - mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; - - template - struct MatchPolicy; // provide policy_type in specializations - - template - const typename MatchPolicy::policy_set_type &getPolicySet() const; - - template ::policy_type, - typename PS = typename MatchPolicy::policy_set_type, - typename ...Args> - DecisionItem getDecisionItem(const T &item, - const P *(PS::*f)(Args ... args) const, - Args ...args) 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; template @@ -240,6 +92,7 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, ItemAccess &item); + void clear(); }; } #endif -- 2.7.4 From 118785437b30f4f05a8bfa0a6f5479087ca65bf3 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 1 Feb 2019 09:27:34 +0100 Subject: [PATCH 15/16] refactoring: remove duplicate db access method This removes duplicated db access method from naive_policy_checker. Additionally, friend declaration is removed for DbAdapter, the access method made public, and code that uses removed method is reworked. Change-Id: Ifac510e13ba0001390c25d5584c7500c89d665be --- src/internal/internal.cpp | 2 +- src/internal/naive_policy_checker.cpp | 9 +++++++-- src/internal/naive_policy_checker.hpp | 19 +++++++------------ src/internal/policy.cpp | 7 +------ src/internal/policy.hpp | 1 - src/internal/xml_parser.hpp | 4 ---- 6 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index f142cca..d38f277 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -66,7 +66,7 @@ void __internal_init_flush_logs() void __internal_init_sup_group(bool bus_type, uid_t uid, gid_t gid) { - static_parser().updateGroupPolicy(bus_type, uid, gid); + policy_checker().updateGroupDb(bus_type, uid, gid); } void __internal_enter() diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index e9add38..e1ef7c0 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -115,12 +115,17 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ return Decision::ANY; } +void NaivePolicyChecker::updateGroupDb(bool bus_type, uid_t uid, gid_t gid) +{ + getPolicyDb(bus_type).initializeGroups(uid, gid); +} + void NaivePolicyChecker::clearDb(bool bus_type) { - m_bus_db[bus_type].clear(); + getPolicyDb(bus_type).clear(); } void NaivePolicyChecker::printContent(const bool bus_type) { - db(bus_type).printContent(); + getPolicyDb(bus_type).printContent(); } diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index a1b0cdc..5dc5355 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -38,13 +38,6 @@ namespace ldp_xml_parser /** Policy databases for system and session bus */ NaivePolicyDb m_bus_db[2]; - /** Retrieves policy db - * \param[in] type Type of database (system/session bus) - * \return Returns reference to chosen bus policy db - * \ingroup Implementation - */ - NaivePolicyDb& getPolicyDb(bool type); - /** Parses delivered decision. In case of Decision::CHECK calls cynara. * \param[in] decision Decision from checkers * \param[in] uid User id @@ -85,18 +78,20 @@ namespace ldp_xml_parser gid_t gid, const T& item); - /** Provides db handle for parsing purposes + public: + /** Retrieves policy db + * \param[in] type Type of database (system/session bus) + * \return Returns reference to chosen bus policy db */ - inline NaivePolicyDb &db(bool sessionBus) { return m_bus_db[sessionBus]; } - - friend class DbAdapter; // give adapters access to db() + NaivePolicyDb& getPolicyDb(bool type); - public: /** Clears all db data, useful for reloading configuration * during testing. */ void clearDb(bool bus_type); + void updateGroupDb(bool bus_type, uid_t uid, gid_t gid); + /** Prints to stderr the structures and the amount of their memory */ void printContent(const bool bus_type); diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 6b73ddf..0530194 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -155,7 +155,7 @@ void DbAdapter::parseRule(const char *name, const char **attr) parseRuleAttribute(attr[i], attr[i + 1]); } - __builder.generateItem(policy_checker().db(curr_bus), policy_type, policy_type_value); + __builder.generateItem(policy_checker().getPolicyDb(curr_bus), policy_type, policy_type_value); } void DbAdapter::parseRuleAttribute(const char *name, const char *value) @@ -197,11 +197,6 @@ void DbAdapter::parseRuleAttribute(const char *name, const char *value) __builder.addMessageType(__str_to_message_type(value)); } -void DbAdapter::updateGroupDb(bool bus, uid_t uid, gid_t gid) -{ - policy_checker().db(bus).initializeGroups(uid, gid); -} - DecisionItem::DecisionItem(Decision decision, const char* privilege) : __decision(decision) { diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 19781d2..e0d852a 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -305,7 +305,6 @@ namespace ldp_xml_parser public: void parsePolicy(bool bus, const char **attr); void parseRule(const char *name, const char **attr); - void updateGroupDb(bool bus, uid_t uid, gid_t gid); }; } #endif diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp index 2369c61..d309afd 100755 --- a/src/internal/xml_parser.hpp +++ b/src/internal/xml_parser.hpp @@ -35,10 +35,6 @@ namespace ldp_xml_parser /** Parses given config file for declared bus type */ int parsePolicy(bool bus, const std::string& fname); - void updateGroupPolicy(bool bus, uid_t uid, gid_t gid) { - __adapter.updateGroupDb(bus, uid, gid); - } - void elementStart(const char *el, const char **attr); void elementEnd(const char *el); -- 2.7.4 From 7b3249ef3ca59930b3435137374fbd6f6ac3e49c Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 6 Feb 2019 11:28:07 +0100 Subject: [PATCH 16/16] refactoring: make bus type a type instead of bool Change-Id: Idadba8b5d9aa5cb7b801be89b08699c53767905f --- src/internal/internal.cpp | 18 ++++++++--------- src/internal/internal.h | 23 +++++++++++++--------- src/internal/naive_policy_checker.cpp | 20 +++++++++---------- src/internal/naive_policy_checker.hpp | 14 ++++++------- src/internal/xml_parser.cpp | 4 ++-- src/internal/xml_parser.hpp | 4 ++-- src/libdbuspolicy1-private.h | 3 --- src/libdbuspolicy1.c | 19 ++++++++---------- src/test-libdbuspolicy1-access-deny.cpp | 5 +---- src/test-libdbuspolicy1-method.cpp | 6 +++--- src/test-libdbuspolicy1-ownership-deny.cpp | 4 ++-- src/test-libdbuspolicy1-ownership.cpp | 4 ++-- ...libdbuspolicy1-send_destination_prefix-deny.cpp | 2 +- src/test-libdbuspolicy1-signal.cpp | 4 ++-- 14 files changed, 63 insertions(+), 67 deletions(-) diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index d38f277..a655f48 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -34,7 +34,7 @@ static const char* get_str(const char* const szstr) { return (szstr != NULL) ? szstr : ""; } -int __internal_init(bool bus_type, const char* const config_name) +int __internal_init(BusType bus_type, const char* const config_name) { policy_checker().clearDb(bus_type); auto err = static_parser().parsePolicy(bus_type, get_str(config_name)); @@ -45,7 +45,7 @@ int __internal_init(bool bus_type, const char* const config_name) pthread_mutex_t g_mutex = PTHREAD_MUTEX_INITIALIZER; -void memory_dump(bool bus_type) +void memory_dump(BusType bus_type) { policy_checker().printContent(bus_type); } @@ -64,7 +64,7 @@ void __internal_init_flush_logs() } } -void __internal_init_sup_group(bool bus_type, uid_t uid, gid_t gid) +void __internal_init_sup_group(BusType bus_type, uid_t uid, gid_t gid) { policy_checker().updateGroupDb(bus_type, uid, gid); } @@ -80,7 +80,7 @@ void __internal_exit() pthread_mutex_unlock(&g_mutex); } -int __internal_can_open(bool bus_type, +int __internal_can_open(BusType bus_type, uid_t bus_owner, uid_t user, gid_t group, @@ -89,7 +89,7 @@ int __internal_can_open(bool bus_type, return static_cast(policy_checker().check(bus_type, bus_owner, user, group, label)); } -int __internal_can_send(bool bus_type, +int __internal_can_send(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -107,7 +107,7 @@ int __internal_can_send(bool bus_type, return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } -int __internal_can_send_multi_dest(bool bus_type, +int __internal_can_send_multi_dest(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -126,7 +126,7 @@ int __internal_can_send_multi_dest(bool bus_type, return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } -int __internal_can_recv(bool bus_type, +int __internal_can_recv(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -144,7 +144,7 @@ int __internal_can_recv(bool bus_type, return static_cast(policy_checker().check(bus_type, user, group, label, matcher)); } -int __internal_can_recv_multi(bool bus_type, +int __internal_can_recv_multi(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -164,7 +164,7 @@ int __internal_can_recv_multi(bool bus_type, } -int __internal_can_own(bool bus_type, +int __internal_can_own(BusType bus_type, const uid_t user, const gid_t group, const char* const label, diff --git a/src/internal/internal.h b/src/internal/internal.h index 05c2b94..30ff4fd 100755 --- a/src/internal/internal.h +++ b/src/internal/internal.h @@ -36,11 +36,16 @@ extern "C" { #define KDBUS_CONN_MAX_NAMES 256 +typedef enum { + SYSTEM_BUS = 0, + SESSION_BUS = 1 +} BusType; + /** Initializes policies from given policy configuration file name * \param[in] bus_type Bus type (system/session) * \param[in] config_name Configuration file name */ -int __internal_init(bool bus_type, const char* const config_name); +int __internal_init(BusType bus_type, const char* const config_name); /** Inits tslog. */ void __internal_init_once(void); @@ -48,7 +53,7 @@ void __internal_init_once(void); extern pthread_mutex_t g_mutex; /** Dumps memory */ -void memory_dump(bool bus_type); +void memory_dump(BusType bus_type); /** Flushes logs. */ void __internal_init_flush_logs(void); @@ -56,7 +61,7 @@ void __internal_init_flush_logs(void); /** Initializes supplementary groups for current process * \param[in] bus_type Bus type (system/session) */ -void __internal_init_sup_group(bool bus_type, uid_t uid, gid_t gid); +void __internal_init_sup_group(BusType bus_type, uid_t uid, gid_t gid); /** Enables logger mutex */ void __internal_enter(void); @@ -65,7 +70,7 @@ void __internal_enter(void); void __internal_exit(void); /** checks if user can open dbus bus */ -int __internal_can_open(bool bus_type, +int __internal_can_open(BusType bus_type, uid_t bus_owner, uid_t user, gid_t group, @@ -83,7 +88,7 @@ int __internal_can_open(bool bus_type, * \param[in] type Message type * \return 1 on allow, 0 on deny, negative error code otherwise */ -int __internal_can_send(bool bus_type, +int __internal_can_send(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -105,7 +110,7 @@ int __internal_can_send(bool bus_type, * \param[in] type Message type * \return 1 on allow, 0 on deny, negative error code otherwise */ -int __internal_can_send_multi_dest(bool bus_type, +int __internal_can_send_multi_dest(BusType bus_type, const uid_t user, const gid_t group, const char* const label, @@ -127,7 +132,7 @@ int __internal_can_send_multi_dest(bool bus_type, * \param[in] type Message type * \return 1 on allow, 0 on deny, negative error code otherwise */ -int __internal_can_recv(bool bus_type, +int __internal_can_recv(BusType bus_type, uid_t user, gid_t group, const char* const label, @@ -149,7 +154,7 @@ int __internal_can_recv(bool bus_type, * \param[in] type Message type * \return 1 on allow, 0 on deny, negative error code otherwise */ -int __internal_can_recv_multi(bool bus_type, +int __internal_can_recv_multi(BusType bus_type, uid_t user, gid_t group, const char* const label, @@ -167,7 +172,7 @@ int __internal_can_recv_multi(bool bus_type, * \param[in] service Name to own * \return 1 on allow, 0 on deny, negative error code otherwise */ -int __internal_can_own(bool bus_type, +int __internal_can_own(BusType bus_type, uid_t user, gid_t group, const char* const label, diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index e1ef7c0..d94925f 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -40,7 +40,7 @@ DecisionResult NaivePolicyChecker::parseDecision(const DecisionItem& decision, return DecisionResult::DENY; } -DecisionItem NaivePolicyChecker::checkItemAccess(bool bus_type, const MatchItemAccess& item) +DecisionItem NaivePolicyChecker::checkItemAccess(BusType bus_type, const MatchItemAccess& item) { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); @@ -54,7 +54,7 @@ DecisionItem NaivePolicyChecker::checkItemAccess(bool bus_type, const MatchItemA return ret; } -DecisionResult NaivePolicyChecker::check(bool bus_type, +DecisionResult NaivePolicyChecker::check(BusType bus_type, uid_t bus_owner, uid_t uid, gid_t gid, @@ -70,7 +70,7 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, } template -DecisionResult NaivePolicyChecker::check(bool bus_type, +DecisionResult NaivePolicyChecker::check(BusType bus_type, uid_t uid, gid_t gid, const char* const label, @@ -78,12 +78,12 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, auto ret = checkItem(bus_type, uid, gid, matchItem); return parseDecision(ret, uid, label); } -template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemOwn &); -template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemSend &); -template DecisionResult NaivePolicyChecker::check(bool, uid_t, gid_t, const char *, const MatchItemReceive &); +template DecisionResult NaivePolicyChecker::check(BusType, uid_t, gid_t, const char *, const MatchItemOwn &); +template DecisionResult NaivePolicyChecker::check(BusType, uid_t, gid_t, const char *, const MatchItemSend &); +template DecisionResult NaivePolicyChecker::check(BusType, uid_t, gid_t, const char *, const MatchItemReceive &); template -DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, const T& item) { +DecisionItem NaivePolicyChecker::checkItem(BusType bus_type, uid_t uid, gid_t gid, const T& item) { const NaivePolicyDb& policy_db = getPolicyDb(bus_type); DecisionItem ret = policy_db.getDecisionItemContextMandatory(item); @@ -115,17 +115,17 @@ DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_ return Decision::ANY; } -void NaivePolicyChecker::updateGroupDb(bool bus_type, uid_t uid, gid_t gid) +void NaivePolicyChecker::updateGroupDb(BusType bus_type, uid_t uid, gid_t gid) { getPolicyDb(bus_type).initializeGroups(uid, gid); } -void NaivePolicyChecker::clearDb(bool bus_type) +void NaivePolicyChecker::clearDb(BusType bus_type) { getPolicyDb(bus_type).clear(); } -void NaivePolicyChecker::printContent(const bool bus_type) +void NaivePolicyChecker::printContent(BusType bus_type) { getPolicyDb(bus_type).printContent(); } diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index 5dc5355..ea8898d 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -59,7 +59,7 @@ namespace ldp_xml_parser * \ingroup Implementation */ template - DecisionItem checkItem(bool bus_type, + DecisionItem checkItem(BusType bus_type, uid_t uid, gid_t gid, const T& item); @@ -70,7 +70,7 @@ namespace ldp_xml_parser * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ - DecisionItem checkItemAccess(bool bus_type, const MatchItemAccess &item); + DecisionItem checkItemAccess(BusType bus_type, const MatchItemAccess &item); template DecisionItem checkGroupPolicies(const NaivePolicyDb& policy_db, @@ -88,12 +88,12 @@ namespace ldp_xml_parser /** Clears all db data, useful for reloading configuration * during testing. */ - void clearDb(bool bus_type); + void clearDb(BusType bus_type); - void updateGroupDb(bool bus_type, uid_t uid, gid_t gid); + void updateGroupDb(BusType bus_type, uid_t uid, gid_t gid); /** Prints to stderr the structures and the amount of their memory */ - void printContent(const bool bus_type); + void printContent(BusType bus_type); /** Checks access/open policy for given item * \param[in] bus_type Bus type (system/session) @@ -102,7 +102,7 @@ namespace ldp_xml_parser * \return Returns deny=0, allow=1 or cynara error * \ingroup Implementation */ - DecisionResult check(bool bus_type, + DecisionResult check(BusType bus_type, uid_t bus_owner, uid_t uid, gid_t gid, @@ -118,7 +118,7 @@ namespace ldp_xml_parser * \ingroup Implementation */ template - DecisionResult check(bool bus_type, + DecisionResult check(BusType bus_type, uid_t uid, gid_t gid, const char* const label, diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index 9be353a..caf98be 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -81,7 +81,7 @@ void text_handler(void *data, const char *text, int len) { } bool XmlParser::isMainConfFile(const std::string& filename) { - switch ((int)curr_bus) { + switch (curr_bus) { case SYSTEM_BUS: return (filename == SYSTEM_BUS_CONF_FILE_PRIMARY || filename == SYSTEM_BUS_CONF_FILE_SECONDARY); case SESSION_BUS: @@ -177,7 +177,7 @@ void XmlParser::elementEnd(const char *el) { } } -int XmlParser::parsePolicy(bool bus, const std::string& fname) { +int XmlParser::parsePolicy(BusType bus, const std::string& fname) { tslog::log("XmlParser::parsePolicy called with filename: ", fname, "\n"); curr_bus = bus; parsePolicyInternal(fname); diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp index d309afd..eba16ad 100755 --- a/src/internal/xml_parser.hpp +++ b/src/internal/xml_parser.hpp @@ -33,7 +33,7 @@ namespace ldp_xml_parser { public: /** Parses given config file for declared bus type */ - int parsePolicy(bool bus, const std::string& fname); + int parsePolicy(BusType bus_type, const std::string& fname); void elementStart(const char *el, const char **attr); @@ -70,7 +70,7 @@ namespace ldp_xml_parser bool ignore_always; bool ignore_missing; - bool curr_bus; + BusType curr_bus; std::string current_text; std::string curr_dir; int ret_code; diff --git a/src/libdbuspolicy1-private.h b/src/libdbuspolicy1-private.h index 752f4b9..55daccc 100644 --- a/src/libdbuspolicy1-private.h +++ b/src/libdbuspolicy1-private.h @@ -24,9 +24,6 @@ #include #include -#define SYSTEM_BUS 0 -#define SESSION_BUS 1 - #define DBUSPOLICY1_EXPORT __attribute__ ((visibility("default"))) typedef uint8_t dbus_name_len; diff --git a/src/libdbuspolicy1.c b/src/libdbuspolicy1.c index 9b6eeef..94fc189 100755 --- a/src/libdbuspolicy1.c +++ b/src/libdbuspolicy1.c @@ -84,7 +84,7 @@ static int kdbus_open_bus(const char *path) return open(path, O_RDWR|O_NOCTTY|O_LARGEFILE|O_CLOEXEC); } -static int kdbus_hello(bool bus_type, uint64_t hello_flags, uint64_t attach_flags_send, uint64_t attach_flags_recv) +static int kdbus_hello(BusType bus_type, uint64_t hello_flags, uint64_t attach_flags_send, uint64_t attach_flags_recv) { struct kdbus_cmd_hello* cmd; struct kdbus_cmd_free cmd_free; @@ -167,7 +167,7 @@ static bool dbuspolicy_init_once(void) return false; } -static int bus_path_resolve(const char *bus_path, char *resolved_path, unsigned resolved_path_size, unsigned int *bus_type, uid_t *bus_owner) +static int bus_path_resolve(const char *bus_path, char *resolved_path, unsigned resolved_path_size, BusType *bus_type, uid_t *bus_owner) { char *p; const char user_suffix[] = "-user/bus"; @@ -221,7 +221,7 @@ static bool init_once[2] = {false, false}; DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) { - unsigned int bus_type = -1; + BusType bus_type = SESSION_BUS; char resolved_path[PATH_MAX] = { 0 }; int rp, rs; uid_t bus_owner = 0; @@ -234,9 +234,6 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) return NULL; } - if (bus_type) - bus_type = SESSION_BUS; - pthread_mutex_lock(&g_mutex); if (!init_once_done) { init_once_done = true; @@ -291,7 +288,7 @@ DBUSPOLICY1_EXPORT void dbuspolicy1_free(void* configuration) configuration = configuration; } -static bool configuration_bus_type(struct kconn const *configuration) { return configuration != g_conn; } +static BusType configuration_bus_type(struct kconn const *configuration) { return configuration == g_conn ? SYSTEM_BUS : SESSION_BUS; } union kdbus_cmd_union { struct kdbus_cmd_info cmd_info; @@ -309,7 +306,7 @@ struct kdbus_cmd_param { union kdbus_cmd_union cmd; }; -int kdbus_get_conn_info(bool bus_type, const char *destination, struct kdbus_cmd_param *info, __u64 flags) +int kdbus_get_conn_info(BusType bus_type, const char *destination, struct kdbus_cmd_param *info, __u64 flags) { char const *label = NULL; const char** k_names = info->k_names; @@ -408,7 +405,7 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_out(void* configuration, (void)requested_reply; int r; - bool bus_type = configuration_bus_type(configuration); + BusType bus_type = configuration_bus_type(configuration); struct kdbus_cmd_param info = { .free_offset = false, .empty_names = true @@ -501,7 +498,7 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_in(void* configuration, (void)requested_reply; int r; - bool bus_type = configuration_bus_type(configuration); + BusType bus_type = configuration_bus_type(configuration); struct kdbus_cmd_param info = { .free_offset = false, .empty_names = true @@ -542,7 +539,7 @@ end: DBUSPOLICY1_EXPORT int dbuspolicy1_can_own(void* configuration, const char* const service) { int r; - bool bus_type = configuration_bus_type(configuration); + BusType bus_type = configuration_bus_type(configuration); __internal_enter(); r = __internal_can_own(bus_type, g_udesc.uid, g_udesc.gid, g_udesc.label, service); __internal_exit(); diff --git a/src/test-libdbuspolicy1-access-deny.cpp b/src/test-libdbuspolicy1-access-deny.cpp index 2dc29c2..5ea0002 100644 --- a/src/test-libdbuspolicy1-access-deny.cpp +++ b/src/test-libdbuspolicy1-access-deny.cpp @@ -3,9 +3,6 @@ #include "internal/internal.h" #include -#define SYSTEM_BUS 0 -#define SESSION_BUS 1 - struct AccessTest { bool expected_result; uid_t user; @@ -73,7 +70,7 @@ void print_test(const struct AccessTest* t, bool result) { (unsigned long)t->user, (unsigned long)t->group, t->label, (int)t->expected_result, (int)result); } -void run_tests_for_bus(bool bus_type, const std::vector& test_setup, int& i, bool& passed) { +void run_tests_for_bus(BusType bus_type, const std::vector& test_setup, int& i, bool& passed) { for (const auto& test : test_setup) { __internal_init_sup_group(bus_type, test.user, test.group); bool res = __internal_can_open(bus_type, bus_owner, test.user, test.group, test.label); diff --git a/src/test-libdbuspolicy1-method.cpp b/src/test-libdbuspolicy1-method.cpp index 3c2e16d..d7db435 100644 --- a/src/test-libdbuspolicy1-method.cpp +++ b/src/test-libdbuspolicy1-method.cpp @@ -64,13 +64,13 @@ bool method_test() { unsigned i = 0; bool flag = true; bool ret = true; - __internal_init(false, "tests/default_allow/system.conf"); + __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf"); for (i = 0; i < sizeof(method_tests)/sizeof(struct MethodTest); i++) { if (method_tests[i].recv_send == MessageDirection::SEND) { - ret = __internal_can_send(false, method_tests[i].user, method_tests[i].group, method_tests[i].label, method_tests[i].name, method_tests[i].path, method_tests[i].interface, method_tests[i].member, static_cast(method_tests[i].type)); + ret = __internal_can_send(SYSTEM_BUS, method_tests[i].user, method_tests[i].group, method_tests[i].label, method_tests[i].name, method_tests[i].path, method_tests[i].interface, method_tests[i].member, static_cast(method_tests[i].type)); } else if (method_tests[i].recv_send == MessageDirection::RECEIVE) { - ret = __internal_can_recv(false, method_tests[i].user, method_tests[i].group, method_tests[i].label, method_tests[i].name, method_tests[i].path, method_tests[i].interface, method_tests[i].member, static_cast(method_tests[i].type)); + ret = __internal_can_recv(SYSTEM_BUS, method_tests[i].user, method_tests[i].group, method_tests[i].label, method_tests[i].name, method_tests[i].path, method_tests[i].interface, method_tests[i].member, static_cast(method_tests[i].type)); } if ( (int)((method_tests[i].expected_result)) != ret) { printf("[ERROR][%d] method test failed: %d %d ", i, (int)((method_tests[i].expected_result)), ret); diff --git a/src/test-libdbuspolicy1-ownership-deny.cpp b/src/test-libdbuspolicy1-ownership-deny.cpp index 872161e..f52688c 100644 --- a/src/test-libdbuspolicy1-ownership-deny.cpp +++ b/src/test-libdbuspolicy1-ownership-deny.cpp @@ -130,9 +130,9 @@ bool ownership_test() { unsigned i = 0; bool flag = true; bool ret = true; - __internal_init(false, "tests/default_deny/system.conf"); + __internal_init(SYSTEM_BUS, "tests/default_deny/system.conf"); for (i = 0; i < sizeof(ownership_tests)/sizeof(struct OwnershipTest); i++) { - ret = __internal_can_own(false, ownership_tests[i].user, ownership_tests[i].group, ownership_tests[i].label, ownership_tests[i].service); + ret = __internal_can_own(SYSTEM_BUS, ownership_tests[i].user, ownership_tests[i].group, ownership_tests[i].label, ownership_tests[i].service); if ( (int)((ownership_tests[i].expected_result)) != ret) { printf("[ERROR][%d] ownership test failed: %d %d ", i, (int)((ownership_tests[i].expected_result)), ret); ownershipTest_print(&ownership_tests[i], ret); diff --git a/src/test-libdbuspolicy1-ownership.cpp b/src/test-libdbuspolicy1-ownership.cpp index ae8d238..5a32425 100644 --- a/src/test-libdbuspolicy1-ownership.cpp +++ b/src/test-libdbuspolicy1-ownership.cpp @@ -55,9 +55,9 @@ bool ownership_test() { unsigned i = 0; bool flag = true; bool ret = true; - __internal_init(false, "tests/default_allow/system.conf"); + __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf"); for (i = 0; i < sizeof(ownership_tests)/sizeof(struct OwnershipTest); i++) { - ret = __internal_can_own(false, ownership_tests[i].user, ownership_tests[i].group, ownership_tests[i].label, ownership_tests[i].service); + ret = __internal_can_own(SYSTEM_BUS, ownership_tests[i].user, ownership_tests[i].group, ownership_tests[i].label, ownership_tests[i].service); if ( (int)((ownership_tests[i].expected_result)) != ret) { printf("[ERROR][%d] ownership test failed: %d %d ", i, (int)((ownership_tests[i].expected_result)), ret); ownershipTest_print(&ownership_tests[i], ret); diff --git a/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp index c2d5a45..2f2fbe5 100644 --- a/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp +++ b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp @@ -137,7 +137,7 @@ bool test() bool flag = true; bool ret = true; - __internal_init(false, "tests/default_deny/system.conf"); + __internal_init(SYSTEM_BUS, "tests/default_deny/system.conf"); for (i = 0; i < sizeof(tests)/sizeof(struct Test); i++) { ret = __internal_can_send(SYSTEM_BUS, tests[i].user, diff --git a/src/test-libdbuspolicy1-signal.cpp b/src/test-libdbuspolicy1-signal.cpp index e0c11cc..27069e5 100644 --- a/src/test-libdbuspolicy1-signal.cpp +++ b/src/test-libdbuspolicy1-signal.cpp @@ -31,9 +31,9 @@ bool signal_test() { unsigned i = 0; bool flag = true; bool ret = true; - __internal_init(false, "tests/default_allow/system.conf"); + __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf"); for (i = 0; i < sizeof(signal_tests)/sizeof(struct SignalTest); i++) { - ret = __internal_can_send(false, signal_tests[i].user, signal_tests[i].group, signal_tests[i].label, signal_tests[i].dest, NULL, signal_tests[i].interface, NULL, DBUSPOLICY_MESSAGE_TYPE_SIGNAL); + ret = __internal_can_send(SYSTEM_BUS, signal_tests[i].user, signal_tests[i].group, signal_tests[i].label, signal_tests[i].dest, NULL, signal_tests[i].interface, NULL, DBUSPOLICY_MESSAGE_TYPE_SIGNAL); if ( (int)((signal_tests[i].expected_result)) != ret) { printf("[ERROR][%d] signal test failed: %d %d ", i, (int)((signal_tests[i].expected_result)), ret); signalTest_print(&signal_tests[i], ret); -- 2.7.4