From aaa665533f504cd1442edd8f7da072af8c66c072 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 23 Jan 2019 11:37:22 +0100 Subject: [PATCH 01/16] internal: replaced manual reverse iterator with STL-provided PolicyConstIterator's purpose is identical to vector::const_reverse_iterator from vector. This removes the unnecessary code, replacing it with the usage of vector::const_reverse_iterator. Change-Id: Ib3063eb6eb2a4fd23f8da8a2cce75c808abb3c00 --- src/internal/naive_policy_db.cpp | 33 --------------------------------- src/internal/naive_policy_db.hpp | 19 +++---------------- 2 files changed, 3 insertions(+), 49 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 177df28..cce03eb 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -162,39 +162,6 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, return this->getPolicy(m_access_set, policy_type, policy_type_value, policy); } - -NaivePolicyDb::PolicySR::PolicyConstIterator::PolicyConstIterator(const std::vector< ItemSendReceive const *> & items, int position) - : m_items(items), m_index(position) { -} - -ItemSendReceive const *const& NaivePolicyDb::PolicySR::PolicyConstIterator::operator*() const { - return m_items[m_index]; -} - - -typename NaivePolicyDb::PolicySR::PolicyConstIterator& NaivePolicyDb::PolicySR::PolicyConstIterator::operator++() { - if (m_index >= 0) - --m_index; - return *this; -} - - -bool NaivePolicyDb::PolicySR::PolicyConstIterator::operator!=(const PolicyConstIterator& it) const { - return m_index != it.m_index; -} - - -NaivePolicyDb::PolicySR::PolicyConstIterator NaivePolicyDb::PolicySR::begin() const { - int s = m_items.size() - 1; - return NaivePolicyDb::PolicySR::PolicyConstIterator(m_items, s); -} - - -NaivePolicyDb::PolicySR::PolicyConstIterator NaivePolicyDb::PolicySR::end() const { - return NaivePolicyDb::PolicySR::PolicyConstIterator(m_items, -1); -} - - void NaivePolicyDb::PolicySR::addItem(ItemSendReceive* item) { m_items.push_back(item); } diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 679b620..cedc27a 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -48,23 +48,10 @@ namespace ldp_xml_parser private: /** Vector with policy items */ std::vector m_items; - /** Iterator used to get access to policy items */ - class PolicyConstIterator { - private: - /** Vector with policy items */ - const std::vector& m_items; - /** Assistant iterator */ - int m_index; - public: - /** Public method allowing access to policy items */ - PolicyConstIterator(const std::vector& items, int position); - ItemSendReceive const *const& operator*() const; - PolicyConstIterator& operator++(); - bool operator!=(const PolicyConstIterator& it) const; - }; + typedef decltype(m_items)::const_reverse_iterator PolicyConstIterator; public: - PolicyConstIterator begin() const; - PolicyConstIterator end() const; + 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 -- 2.7.4 From 4432e59ea3b4bcae826796540863b2c3de774190 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 23 Jan 2019 15:45:38 +0100 Subject: [PATCH 02/16] internal: make logs less cluttering Many logs are under an if-clause, making them take more space than necessary. This replaces most of if-clause-and-logs with one-liners to improve readability. Change-Id: I7e9ada35160974c8cc38de1c6e0217de15912c6e --- src/internal/groups_proxy.cpp | 9 +++---- src/internal/internal.cpp | 6 ++--- src/internal/naive_policy_checker.cpp | 45 ++++++++++------------------------- src/internal/naive_policy_db.cpp | 25 +++++++------------ src/internal/policy.cpp | 14 ++++++++--- src/internal/policy.hpp | 3 +++ src/internal/tslog.hpp | 11 +++++++++ src/internal/xml_parser.cpp | 21 ++++++---------- 8 files changed, 58 insertions(+), 76 deletions(-) diff --git a/src/internal/groups_proxy.cpp b/src/internal/groups_proxy.cpp index fd5870b..e577037 100644 --- a/src/internal/groups_proxy.cpp +++ b/src/internal/groups_proxy.cpp @@ -15,13 +15,11 @@ std::vector get_groups(const uid_t uid, const gid_t gid) { struct passwd pwd; if (getpwuid_r(uid, &pwd, buf.data(), buf.size(), &user_pw)) { - if (tslog::enabled()) - std::cout << "getpwuid_r failed" << " uid:" << uid << " gid: " << gid << std::endl; + tslog::log("getpwuid_r failed uid:", uid, " gid: ", gid, "\n"); return {}; } if (!user_pw) { - if (tslog::enabled()) - std::cout << "getpwuid_r failed. no matching password record was found" << " uid:" << uid << " gid: " << gid << std::endl; + tslog::log("getpwuid_r failed. no matching password record was found uid:", uid, " gid: ", gid, "\n"); return {}; } @@ -36,8 +34,7 @@ std::vector get_groups(const uid_t uid, const gid_t gid) { auto groups = std::vector (ngroups); int r = getgrouplist(user_pw->pw_name, gid, groups.data(), &ngroups); if (r < 0) { - if (tslog::enabled()) - std::cout << "getgrouplist failed" << " uid:" << uid << " gid: " << gid << std::endl; + tslog::log("getgrouplist failed uid:", uid, " gid: ", gid, "\n"); groups.clear(); } return groups; diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index 78b8dc1..f15e81b 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -99,8 +99,7 @@ int __internal_can_send(bool bus_type, { ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::SEND); if (!matcher.addNames(destination)) { - if (tslog::verbose()) - std::cout << "Destination too long: " << destination << std::endl; + 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)); @@ -137,8 +136,7 @@ int __internal_can_recv(bool bus_type, { ldp_xml_parser::MatchItemSR matcher(interface, member, path, static_cast(type), ldp_xml_parser::MessageDirection::RECEIVE); if (!matcher.addNames(sender)) { - if (tslog::verbose()) - std::cout << "Sender too long: " << sender << std::endl; + 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)); diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index f719e7f..e6c33e2 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -11,12 +11,6 @@ using namespace ldp_xml_parser; DEF_NODESTRUCT_GLOBAL(ldp_xml_parser::NaivePolicyChecker, policy_checker) -static void __log_item(const MatchItemSR& item) -{ - std::cout << "checkpolicy for: " << item << std::endl; -} - - NaivePolicyDb& NaivePolicyChecker::getPolicyDb(bool type) { return m_bus_db[type]; } @@ -25,9 +19,8 @@ DecisionResult NaivePolicyChecker::parseDecision(const DecisionItem& decision, uid_t uid, const char* label) const { char uid_str[17]; - if (tslog::verbose()) { - std::cout << "----Decision made\n"; - } + tslog::log_verbose("----Decision made\n"); + switch (decision.getDecision()) { case Decision::ALLOW: @@ -87,17 +80,14 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySR& policy, const MatchItemSR& item) const { - if (tslog::verbose()) { - __log_item(item); - } + tslog::log_verbose("checkpolicy for: ", item, "\n"); + for (auto i : policy) { - if (tslog::verbose()) { - std::cout << "-read: " << i->getDecision() << " " << i << std::endl; - } + tslog::log_verbose("-read: ", i->getDecision(), " ", i, "\n"); + if (i->match(item)) { - if (tslog::verbose()) { - std::cout << "-matched: " << i->getDecision() << " " << i << std::endl; - } + tslog::log_verbose("-matched: ", i->getDecision(), " ", i, "\n"); + return i->getDecision(); } } @@ -106,25 +96,16 @@ DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySR& poli DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyOwn& policy, const ItemOwn& item) const { - if (tslog::verbose()) { - std::cout << "Checking policy for name: " << std::string(item.getName() ? item.getName() : "NULL") << std::endl; - } + tslog::log_verbose("Checking policy for name: ", std::string(item.getName() ? item.getName() : "NULL"), "\n"); - DecisionItem decision_item = policy.getDecisionItem(item); - return decision_item; + return policy.getDecisionItem(item); } DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyAccess& policy, const MatchItemAccess& item) const { - if (tslog::verbose()) { - std::cout << "Checking access policy for uid: " << item.getUid() << ", gids: "; - for (auto gid : item.getGids()) { - std::cout << gid << ", "; - } - std::cout << std::endl; - } - DecisionItem decision_item = policy.getDecisionItem(item); - return decision_item; + tslog::log_verbose("Checking access policy for ", item, "\n"); + + return policy.getDecisionItem(item); } template diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index cce03eb..25dbf08 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -15,9 +15,7 @@ using namespace ldp_xml_parser; template void log_item_add(const T* item) { - if (tslog::enabled()) { - std::cout << "Add item: " << item << ", decision: " << item->getDecision() << std::endl; - } + tslog::log("Add item: ", item, ", decision: ", item->getDecision(), "\n"); } void NaivePolicyDb::addItem(const PolicyType policy_type, @@ -256,24 +254,20 @@ bool NaivePolicyDb::getPolicy(const PolicySet

& set, const PolicyTypeValue policy_type_value, const P*& policy) const { - if (tslog::enabled()) - std::cout << "---policy_type ="; + tslog::log("---policy_type ="); switch (policy_type) { case PolicyType::CONTEXT: - if (tslog::enabled()) - std::cout << "CONTEXT =" << (int)policy_type_value.context << std::endl; + tslog::log("CONTEXT =", (int)policy_type_value.context, "\n"); policy = &set.context[static_cast(policy_type_value.context)]; return true; case PolicyType::USER: { - if (tslog::enabled()) - std::cout << "USER =" << (int)policy_type_value.user << std::endl; + tslog::log("USER =", (int)policy_type_value.user, "\n"); auto it = set.user.find(policy_type_value.user); if (it == set.user.end()) { - if (tslog::verbose()) - std::cout << "GetPolicy: Out of Range exception\n"; + tslog::log_verbose("GetPolicy: Out of Range exception\n"); return false; } policy = &(it->second); @@ -281,21 +275,18 @@ bool NaivePolicyDb::getPolicy(const PolicySet

& set, return true; case PolicyType::GROUP: { - if (tslog::enabled()) - std::cout << "GROUP = " << (int)policy_type_value.group << std::endl; + tslog::log("GROUP = ", (int)policy_type_value.group, "\n"); auto it = set.group.find(policy_type_value.group); if (it == set.group.end()) { - if (tslog::verbose()) - std::cout << "GetPolicy: Out of Range exception\n"; + tslog::log_verbose("GetPolicy: Out of Range exception\n"); return false; } policy = &(it->second); } return true; default: - if (tslog::enabled()) - std::cout << "NO POLICY\n"; + tslog::log("NO POLICY\n"); } return false; diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index ce0e3ae..07fb60c 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -302,9 +302,9 @@ bool MatchItemSR::addNames(const char* name) { char c; int len; j = i; - if (tslog::verbose() && !(isalnum(name[i]) || name[i] == ' ')) { - std::cout << "Wrong name("<< i <<"): " << name << std::endl; - } + if (!(isalnum(name[i]) || name[i] == ' ')) + tslog::log_verbose("Wrong name(", i, "): ", name, "\n"); + while ((c = name[i++]) && ' ' != c); if (!c) { --i; @@ -655,4 +655,12 @@ std::ostream &operator<<(std::ostream& stream, const ItemAccess &item) item.__uid << "), gid(" << item.__gid << ")"; } +std::ostream &operator<<(std::ostream& stream, const MatchItemAccess &item) +{ + stream << "uid: " << item.getUid() << ", gid: "; + for (auto gid : item.getGids()) + stream << gid << ", "; + return stream << std::endl; +} + } diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 536ad8d..b93b450 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -213,7 +213,10 @@ namespace ldp_xml_parser MatchItemAccess(const uid_t uid, const std::vector* gids); uid_t getUid() const; const std::vector& getGids() const; + + friend std::ostream &operator<<(std::ostream& stream, const MatchItemAccess &item); }; + std::ostream &operator<<(std::ostream& stream, const MatchItemAccess &item); /** DBus bus access constraint */ class ItemAccess { diff --git a/src/internal/tslog.hpp b/src/internal/tslog.hpp index f41c417..d6695a3 100644 --- a/src/internal/tslog.hpp +++ b/src/internal/tslog.hpp @@ -43,6 +43,17 @@ namespace tslog bool verbose(); void logError(const char *error); + + template + void log_to_stream(std::ostream &stream, const Args &...args) { + std::initializer_list{((void)(stream << args), 0)...}; + } + + template + void log(const Args &...args) { if (enabled()) log_to_stream(std::cout, args...); } + + template + void log_verbose(const Args &...args) { if (verbose()) log_to_stream(std::cout, args...); } } #endif diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index 03d6a13..9be353a 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -178,9 +178,7 @@ void XmlParser::elementEnd(const char *el) { } int XmlParser::parsePolicy(bool bus, const std::string& fname) { - if (tslog::enabled()) { - std::cout << "XmlParser::parsePolicy called with filename: " << fname << std::endl; - } + tslog::log("XmlParser::parsePolicy called with filename: ", fname, "\n"); curr_bus = bus; parsePolicyInternal(fname); return ret_code; @@ -242,11 +240,9 @@ std::unique_ptr file2str(const std::string& filename) { void XmlParser::parseXmlFile(const std::string& filename) { included_files.clear(); - if (tslog::enabled()) { - std::cout << "Processing: " << filename << " ..." << std::endl; - if (tslog::verbose()) - std::cout << "=== XML PARSING BEGIN === : " << filename << '\n'; - } + tslog::log("Processing: ", filename, " ...\n"); + tslog::log_verbose("=== XML PARSING BEGIN === : ", filename, '\n'); + curr_dir = getDir(filename); auto parser = std::unique_ptr>(XML_ParserCreate(nullptr), @@ -259,10 +255,7 @@ void XmlParser::parseXmlFile(const std::string& filename) { throw std::runtime_error(std::string("Error parsing xml file: ").append(XML_ErrorString(XML_GetErrorCode(parser.get()))).c_str()); } - if (tslog::enabled()) { - if (tslog::verbose()) - std::cout << "=== XML PARSING END ===\n\n"; - } + tslog::log_verbose("=== XML PARSING END ===\n\n"); } XmlParser::IncludeItem XmlParser::parseIncludeItem() { @@ -292,8 +285,8 @@ void XmlParser::getIncludedFiles(const std::string& parent_dir, const std::strin std::copy(files.begin(), files.end(), std::ostream_iterator(std::cout, "\n")); std::cout << '\n'; } - } else if (tslog::enabled()) { - std::cout << "could not open directory " << dname << '\n'; + } else { + tslog::log("could not open directory ", dname, '\n'); } } -- 2.7.4 From 20d0a9598ca25d93426188bc332f1ebef3583892 Mon Sep 17 00:00:00 2001 From: "sanghyeok.oh" Date: Mon, 28 Jan 2019 16:22:33 +0900 Subject: [PATCH 03/16] svace fix Change-Id: I33fb1fc35dd8f8edb6dc13545868120528a3885a Signed-off-by: sanghyeok.oh --- src/internal/policy.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 07fb60c..2effa15 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -331,7 +331,8 @@ ItemSendReceive::ItemSendReceive(const char* name, __member(member), __path(path), __type(type), - __direction(direction) { + __direction(direction), + __is_name_prefix(false) { } ItemSendReceive::~ItemSendReceive() { -- 2.7.4 From 31d7c97fe2cebad689df5b26cd7f6eedd0641890 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 8 Feb 2019 08:58:44 +0100 Subject: [PATCH 04/16] internal: fix mistaken getgid()->getuid() Change-Id: I97a0514e779c3a5efc3e0dc1aae3e770d8769102 --- src/internal/naive_policy_db.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 25dbf08..fe08543 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -345,7 +345,7 @@ std::vector* NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemTyp } static gid_t mygid = getgid(); - static uid_t myuid = getgid(); + static uid_t myuid = getuid(); if (uid == myuid && gid ==mygid) return (type == ItemType::SEND) ? &mapSendGroup[uid] : &mapRecvGroup[uid]; -- 2.7.4 From 0c8c526e92f520ad768f0336792f055ac29a7712 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Thu, 7 Feb 2019 13:50:16 +0100 Subject: [PATCH 05/16] tests: build tests in a single build Spec file makes two builds: one with --enable-tests, and second without tests. This was used to add a special function for changing creds and checking config file. This commit exports the behaviour which is different in both versions to separate files and uses those files for linking with proper versions. It also makes the special function not exported anymore, as it is not needed for static linking with test programs. Change-Id: I72cc44e35c17da85e73ac503bbd447a4ee24e6cc --- Makefile.am | 9 ++++++--- packaging/libdbuspolicy.spec | 9 ++------- src/dbuspolicy1/libdbuspolicy1.h | 1 + src/internal/internal.h | 1 + src/libdbuspolicy1-private.h | 12 +++++++++++- src/libdbuspolicy1.c | 30 +++++++++------------------- src/libdbuspolicy1.sym | 6 ------ src/primary-conf-files.c | 11 +++++++++++ src/stest_cynara.c | 4 ++-- src/stest_method_call.c | 42 ++++++++++++++++++++-------------------- src/stest_ownership.c | 24 +++++++++++------------ src/stest_signal.c | 6 +++--- src/test-helpers.c | 14 ++++++++++++++ 13 files changed, 93 insertions(+), 76 deletions(-) create mode 100644 src/primary-conf-files.c create mode 100644 src/test-helpers.c diff --git a/Makefile.am b/Makefile.am index 35c5d37..acc9fdc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -63,7 +63,8 @@ COMMON_SRC =\ src_libdbuspolicy1_la_SOURCES =\ $(COMMON_SRC) \ src/internal/cynara.cpp \ - src/internal/groups_proxy.cpp + src/internal/groups_proxy.cpp \ + src/primary-conf-files.c EXTRA_DIST += src/libdbuspolicy1.sym @@ -99,7 +100,8 @@ noinst_LTLIBRARIES = src/libinternal.a src_libinternal_a_SOURCES =\ $(COMMON_SRC) \ src/internal/cynara_mockup.cpp \ - src/internal/groups_mockup.cpp + src/internal/groups_mockup.cpp \ + src/primary-conf-files.c TESTS_LDADD = src/libinternal.a \ $(CYNARA_LIBS) \ @@ -118,7 +120,8 @@ noinst_LTLIBRARIES += src/libinternalfortests.a src_libinternalfortests_a_SOURCES =\ $(COMMON_SRC) \ src/internal/cynara.cpp \ - src/internal/groups_proxy.cpp + src/internal/groups_proxy.cpp \ + src/test-helpers.c runner_PROGRAMS = libdbuspolicy-tests libdbuspolicy_tests_SOURCES = src/test_runner.c diff --git a/packaging/libdbuspolicy.spec b/packaging/libdbuspolicy.spec index 2d4d9b7..a723b28 100644 --- a/packaging/libdbuspolicy.spec +++ b/packaging/libdbuspolicy.spec @@ -48,16 +48,11 @@ Doxygen documentation for libdbuspolicy. cp %{SOURCE1001} . %build -%reconfigure --libdir=%{_libdir} --prefix=/usr --enable-tests -make all-tests - -%reconfigure --libdir=%{_libdir} --prefix=/usr \ +%reconfigure --libdir=%{_libdir} --prefix=/usr --enable-tests \ %if 0%{?enable_doxygen:1} --enable-doxygen %endif - -make -make check +make all-tests check %install make DESTDIR=%{buildroot} install diff --git a/src/dbuspolicy1/libdbuspolicy1.h b/src/dbuspolicy1/libdbuspolicy1.h index d923a94..9333ddd 100644 --- a/src/dbuspolicy1/libdbuspolicy1.h +++ b/src/dbuspolicy1/libdbuspolicy1.h @@ -20,6 +20,7 @@ #define _LIBDBUSPOLICY1_H_ #include +#include #ifdef __cplusplus extern "C" { diff --git a/src/internal/internal.h b/src/internal/internal.h index 51ffec7..05c2b94 100755 --- a/src/internal/internal.h +++ b/src/internal/internal.h @@ -32,6 +32,7 @@ extern "C" { #include #include +#include #define KDBUS_CONN_MAX_NAMES 256 diff --git a/src/libdbuspolicy1-private.h b/src/libdbuspolicy1-private.h index 8fb623c..752f4b9 100644 --- a/src/libdbuspolicy1-private.h +++ b/src/libdbuspolicy1-private.h @@ -17,8 +17,9 @@ #ifndef _LIBDBUSPOLICY1_PRIVATE_H_ #define _LIBDBUSPOLICY1_PRIVATE_H_ -#include +#include #include +#include #include #include @@ -34,4 +35,13 @@ typedef uint8_t dbus_name_len; #define LOG_TAG "LIBDBUSPOLICY" #include +#ifdef __cplusplus +extern "C" { +#endif +const char *system_bus_conf_file_primary(); +const char *session_bus_conf_file_primary(); +#ifdef __cplusplus +} +#endif + #endif diff --git a/src/libdbuspolicy1.c b/src/libdbuspolicy1.c index 3d26e84..9b6eeef 100755 --- a/src/libdbuspolicy1.c +++ b/src/libdbuspolicy1.c @@ -48,18 +48,9 @@ #define UID_INVALID ((uid_t) -1) #define GID_INVALID ((gid_t) -1) -#ifdef LIBDBUSPOLICY_TESTS_API -#undef SYSTEM_BUS_CONF_FILE_PRIMARY -#undef SESSION_BUS_CONF_FILE_PRIMARY -#define SYSTEM_BUS_CONF_FILE_PRIMARY "/usr/lib/dbus-tests/configs/libdbuspolicy-tests/system.conf" -#define SESSION_BUS_CONF_FILE_PRIMARY "/usr/lib/dbus-tests/configs/libdbuspolicy-tests/session.conf" -#endif - #define POLICY_CONN_INFO_ALL (__u64)(KDBUS_ATTACH_CREDS | KDBUS_ATTACH_NAMES | KDBUS_ATTACH_SECLABEL) #define POLICY_CONN_INFO_NAME (__u64)(KDBUS_ATTACH_NAMES) -/** A process ID */ -typedef unsigned long dbus_pid_t; /** A user ID */ typedef unsigned long dbus_uid_t; /** A group ID */ @@ -80,6 +71,14 @@ struct udesc { char label[256]; } g_udesc; +void __dbuspolicy1_change_creds(uid_t uid, gid_t gid, const char* label) +{ + g_udesc.uid = uid; + g_udesc.gid = gid; + if (label) + strncpy(g_udesc.label, label, strlen(label) + 1); +} + static int kdbus_open_bus(const char *path) { return open(path, O_RDWR|O_NOCTTY|O_LARGEFILE|O_CLOEXEC); @@ -244,7 +243,7 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) dbuspolicy_init_once(); } if (!init_once[bus_type]) { - rp = __internal_init(bus_type, (bus_type == SYSTEM_BUS) ? SYSTEM_BUS_CONF_FILE_PRIMARY : SESSION_BUS_CONF_FILE_PRIMARY); + rp = __internal_init(bus_type, (bus_type == SYSTEM_BUS) ? system_bus_conf_file_primary() : session_bus_conf_file_primary()); if (rp < 0) rs = __internal_init(bus_type, (bus_type == SYSTEM_BUS) ? SYSTEM_BUS_CONF_FILE_SECONDARY : SESSION_BUS_CONF_FILE_SECONDARY); else @@ -292,17 +291,6 @@ DBUSPOLICY1_EXPORT void dbuspolicy1_free(void* configuration) configuration = configuration; } -#ifdef LIBDBUSPOLICY_TESTS_API -DBUSPOLICY1_EXPORT void __dbuspolicy1_change_creds(void* configuration, uid_t uid, gid_t gid, const char* label) -{ - (void)configuration; - g_udesc.uid = uid; - g_udesc.gid = gid; - if (label) - strncpy(g_udesc.label, label, strlen(label) + 1); -} -#endif - static bool configuration_bus_type(struct kconn const *configuration) { return configuration != g_conn; } union kdbus_cmd_union { diff --git a/src/libdbuspolicy1.sym b/src/libdbuspolicy1.sym index c472731..447f243 100644 --- a/src/libdbuspolicy1.sym +++ b/src/libdbuspolicy1.sym @@ -5,12 +5,6 @@ global: dbuspolicy1_check_in; dbuspolicy1_check_out; dbuspolicy1_can_own; - __dbuspolicy1_change_creds; local: *; }; - -LIBDBUSPOLICY1_1_TEST { -global: - __dbuspolicy1_change_creds; -}; diff --git a/src/primary-conf-files.c b/src/primary-conf-files.c new file mode 100644 index 0000000..aa90233 --- /dev/null +++ b/src/primary-conf-files.c @@ -0,0 +1,11 @@ +#include + +const char *system_bus_conf_file_primary() +{ + return SYSTEM_BUS_CONF_FILE_PRIMARY; +} + +const char *session_bus_conf_file_primary() +{ + return SESSION_BUS_CONF_FILE_PRIMARY; +} diff --git a/src/stest_cynara.c b/src/stest_cynara.c index daa8b24..61e8b6c 100755 --- a/src/stest_cynara.c +++ b/src/stest_cynara.c @@ -10,7 +10,7 @@ #define NEGATIVE_MATCH "negative" #define COUNT 1000 -void __dbuspolicy1_change_creds(void* configuration, uid_t uid, gid_t gid, const char* label); +void __dbuspolicy1_change_creds(uid_t uid, gid_t gid, const char* label); int main(int argc, char* argv[]) { @@ -27,7 +27,7 @@ int main(int argc, char* argv[]) c = dbuspolicy1_init("/sys/fs/kdbus/0-system/bus"); assert(c != NULL); - __dbuspolicy1_change_creds(c, 200, 0, "L"); + __dbuspolicy1_change_creds(200, 0, "L"); for (i = 0; i < COUNT; i++) { int result = dbuspolicy1_check_in(c, "cynara.destination", "cynara.sender", diff --git a/src/stest_method_call.c b/src/stest_method_call.c index dfda6e0..7b1dc93 100755 --- a/src/stest_method_call.c +++ b/src/stest_method_call.c @@ -5,7 +5,7 @@ #include #include "stest_common.h" -void __dbuspolicy1_change_creds(void* configuration, uid_t uid, gid_t gid, const char* label); +void __dbuspolicy1_change_creds(uid_t uid, gid_t gid, const char* label); int main(int argc, char* argv[]) { @@ -16,64 +16,64 @@ int main(int argc, char* argv[]) c = dbuspolicy1_init("/sys/fs/kdbus/0-system/bus"); assert(c != NULL); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test2", "org.test.test3", NULL, "org.test.Itest1", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test2", "org.test.test3", NULL, 0, 0, "", "org.test.Itest1", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test2", "org.test.test3", "", "org.test.Itest1", "DontDoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test2", "org.test.test3", "", 0, 0, "", "org.test.Itest1", "DontDoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test2", "org.test.test3", "", "org.test.Itest1", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test2", "org.test.test3", "", 0, 0, "", "org.test.Itest1", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test2", "org.test.test3", "", "org.test.Itest2", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test2", "org.test.test3", "", 0, 0, "", "org.test.Itest2", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test3", "org.test.test2", "", "org.test.Itest3", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test3", "org.test.test2", "", 5001, 100, "", "org.test.Itest3", "NotKnown", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test3", "org.test.test2", "", "org.test.Itest3", "DontDoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test3", "org.test.test2", "", 5001, 100, "", "org.test.Itest3", "DontDoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == false); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test3", "org.test.test2", "", "org.test.Itest3", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test3", "org.test.test2", "", 5001, 100, "" , "org.test.Itest3", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test3", "org.test.test2", "", "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test3", "org.test.test2", "", 5001, 100, NULL, "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test9", "org.test.test10", "", "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test9", "org.test.test10", "", 5001, 100, NULL, "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "org.test.test10", "org.test.test9", "", "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 5001, 100, NULL); + __dbuspolicy1_change_creds(5001, 100, NULL); tassert(dbuspolicy1_check_in(c, "org.test.test10", "org.test.test9", "", 0, 0, NULL, "org.test.Itest4", "DoIt", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0) == true); return 0; diff --git a/src/stest_ownership.c b/src/stest_ownership.c index 85fe6b3..4baa558 100755 --- a/src/stest_ownership.c +++ b/src/stest_ownership.c @@ -5,7 +5,7 @@ #include #include "stest_common.h" -void __dbuspolicy1_change_creds(void* configuration, uid_t uid, gid_t gid, const char* label); +void __dbuspolicy1_change_creds(uid_t uid, gid_t gid, const char* label); int main(int argc, char* argv[]) { @@ -16,37 +16,37 @@ int main(int argc, char* argv[]) c = dbuspolicy1_init("/sys/fs/kdbus/0-system/bus"); assert(c != NULL); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_can_own(c, "org.test.test1")); - __dbuspolicy1_change_creds(c, 5009, 0, NULL); + __dbuspolicy1_change_creds(5009, 0, NULL); tassert(dbuspolicy1_can_own(c, "org.test.test1")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test2")); - __dbuspolicy1_change_creds(c, 5009, 0, NULL); + __dbuspolicy1_change_creds(5009, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test2")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test3")); - __dbuspolicy1_change_creds(c, 5009, 0, NULL); + __dbuspolicy1_change_creds(5009, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test3")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test4")); - __dbuspolicy1_change_creds(c, 5009, 0, NULL); + __dbuspolicy1_change_creds(5009, 0, NULL); tassert(dbuspolicy1_can_own(c, "org.test.test4")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(!dbuspolicy1_can_own(c, "org.test.test5")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_can_own(c, "org.test.test6")); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_can_own(c, "org.test.test7")); tassert(dbuspolicy1_can_own(c, "a.b.c")); diff --git a/src/stest_signal.c b/src/stest_signal.c index 66c4472..76bca69 100755 --- a/src/stest_signal.c +++ b/src/stest_signal.c @@ -5,7 +5,7 @@ #include #include "stest_common.h" -void __dbuspolicy1_change_creds(void* configuration, uid_t uid, gid_t gid, const char* label); +void __dbuspolicy1_change_creds(uid_t uid, gid_t gid, const char* label); int main(int argc, char* argv[]) { @@ -16,10 +16,10 @@ int main(int argc, char* argv[]) c = dbuspolicy1_init("/sys/fs/kdbus/0-system/bus"); assert(c != NULL); - __dbuspolicy1_change_creds(c, 0, 0, NULL); + __dbuspolicy1_change_creds(0, 0, NULL); tassert(dbuspolicy1_check_out(c, "", "bli.bla.blubb test.test1 test.tes3", "", "/an/object/path", "", DBUSPOLICY_MESSAGE_TYPE_SIGNAL, "", 0, 0) == true); - __dbuspolicy1_change_creds(c, 5010, 0, NULL); + __dbuspolicy1_change_creds(5010, 0, NULL); tassert(dbuspolicy1_check_out(c, "", "bli.bla.blubb", "", "/an/object/path", "", DBUSPOLICY_MESSAGE_TYPE_SIGNAL, "", 0, 0) == false); return 0; diff --git a/src/test-helpers.c b/src/test-helpers.c new file mode 100644 index 0000000..1eb4687 --- /dev/null +++ b/src/test-helpers.c @@ -0,0 +1,14 @@ +#include +#include + +#include "libdbuspolicy1-private.h" + +const char *system_bus_conf_file_primary() +{ + return "/usr/lib/dbus-tests/configs/libdbuspolicy-tests/system.conf"; +} + +const char *session_bus_conf_file_primary() +{ + return "/usr/lib/dbus-tests/configs/libdbuspolicy-tests/session.conf"; +} -- 2.7.4 From be861f92ec8ef8ccb017789bd4077dc88cc5e835 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 8 Feb 2019 15:43:49 +0100 Subject: [PATCH 06/16] tests: make tests passing again Do not confuse with 'make tests great again'. But at least they should work and give results now. Change-Id: I7f0c69f329e7ef1a1813c23eb51387494c717226 --- src/stest_cynara.c | 2 +- src/stest_signal.c | 2 +- tests/default_allow/system.d/cynara.test.conf | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/stest_cynara.c b/src/stest_cynara.c index 61e8b6c..b910ca9 100755 --- a/src/stest_cynara.c +++ b/src/stest_cynara.c @@ -30,7 +30,7 @@ int main(int argc, char* argv[]) __dbuspolicy1_change_creds(200, 0, "L"); for (i = 0; i < COUNT; i++) { - int result = dbuspolicy1_check_in(c, "cynara.destination", "cynara.sender", + int result = dbuspolicy1_check_in(c, "org.freedesktop.systemd1", "cynara.sender", "L", 200, 0, NULL, "cynara.interface", "cynara.method", DBUSPOLICY_MESSAGE_TYPE_METHOD_CALL, "", 0, 0); assert(result == desired_result); diff --git a/src/stest_signal.c b/src/stest_signal.c index 76bca69..1fe2f9e 100755 --- a/src/stest_signal.c +++ b/src/stest_signal.c @@ -17,7 +17,7 @@ int main(int argc, char* argv[]) assert(c != NULL); __dbuspolicy1_change_creds(0, 0, NULL); - tassert(dbuspolicy1_check_out(c, "", "bli.bla.blubb test.test1 test.tes3", "", "/an/object/path", "", DBUSPOLICY_MESSAGE_TYPE_SIGNAL, "", 0, 0) == true); + tassert(dbuspolicy1_check_out(c, "", "org.test.test3", "", "/an/object/path", "", DBUSPOLICY_MESSAGE_TYPE_SIGNAL, "", 0, 0) == true); __dbuspolicy1_change_creds(5010, 0, NULL); tassert(dbuspolicy1_check_out(c, "", "bli.bla.blubb", "", "/an/object/path", "", DBUSPOLICY_MESSAGE_TYPE_SIGNAL, "", 0, 0) == false); diff --git a/tests/default_allow/system.d/cynara.test.conf b/tests/default_allow/system.d/cynara.test.conf index e988385..76c6166 100644 --- a/tests/default_allow/system.d/cynara.test.conf +++ b/tests/default_allow/system.d/cynara.test.conf @@ -4,7 +4,7 @@ - + -- 2.7.4 From c289c011d826b612cee785ca1013334d0f00e15a Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 1 Feb 2019 12:10:29 +0100 Subject: [PATCH 07/16] refactoring: replaced snprintf with to_string Moved string conversion to cynara.cpp. Eliminated snprintf, char array in favor of single call to to_string(). Reworked ifs to switch. Change-Id: Ib57f459183328f55a16412af84214e77f7750f58 --- src/internal/cynara.cpp | 25 +++++++++++++------------ src/internal/cynara.hpp | 5 ++--- src/internal/cynara_mockup.cpp | 8 ++++---- src/internal/naive_policy_checker.cpp | 25 +++++++++++-------------- 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/src/internal/cynara.cpp b/src/internal/cynara.cpp index be3548f..0d1ab30 100644 --- a/src/internal/cynara.cpp +++ b/src/internal/cynara.cpp @@ -3,6 +3,7 @@ #include #include #include +#include using namespace ldp_cynara; @@ -21,28 +22,28 @@ bool Cynara::init() { static pthread_mutex_t __mutex = PTHREAD_MUTEX_INITIALIZER; static Cynara c; -CynaraResult Cynara::check(const char* label, const char* privilege, const char* uid) { +CynaraResult Cynara::check(const char* label, const char* privilege, uid_t uid) { const char* _label = ""; - const char* _uid = ""; const char* _privilege = ""; - CynaraResult ret; + CynaraResult ret = CynaraResult::DENY; if (label) _label = label; if (privilege) _privilege = privilege; - if (uid) - _uid = uid; pthread_mutex_lock(&__mutex); if (!c.init()) { ret = CynaraResult::ERROR_INIT; } else { - int r = cynara_check(c.__cynara, _label, c.__session, _uid, _privilege); - if (r == CYNARA_API_ACCESS_ALLOWED) - ret = CynaraResult::ALLOW; - else if (r == CYNARA_API_ACCESS_DENIED) - ret = CynaraResult::DENY; - else - ret = CynaraResult::ERROR_CHECK; + int r; + try { + r = cynara_check(c.__cynara, _label, c.__session, std::to_string(uid).c_str(), _privilege); + if (r == CYNARA_API_ACCESS_ALLOWED) + ret = CynaraResult::ALLOW; + else if (r == CYNARA_API_ACCESS_DENIED) + ret = CynaraResult::DENY; + else + ret = CynaraResult::ERROR_CHECK; + } catch (const std::bad_alloc &) {} } pthread_mutex_unlock(&__mutex); return ret; diff --git a/src/internal/cynara.hpp b/src/internal/cynara.hpp index 510b850..468138c 100644 --- a/src/internal/cynara.hpp +++ b/src/internal/cynara.hpp @@ -20,8 +20,7 @@ #include #include #include - -#include +#include namespace ldp_cynara { enum class CynaraResult : uint8_t { @@ -39,7 +38,7 @@ namespace ldp_cynara { bool init(); public: - static CynaraResult check(const char* label, const char* privilege, const char* uid); + static CynaraResult check(const char* label, const char* privilege, uid_t uid); }; } //namespace #endif diff --git a/src/internal/cynara_mockup.cpp b/src/internal/cynara_mockup.cpp index cfa7bd1..0ba81e0 100644 --- a/src/internal/cynara_mockup.cpp +++ b/src/internal/cynara_mockup.cpp @@ -6,7 +6,7 @@ using namespace ldp_cynara; -CynaraResult Cynara::check(const char* label, const char* privilege, const char* uid) { +CynaraResult Cynara::check(const char* label, const char* privilege, uid_t uid) { (void)label; if (privilege == nullptr) { return CynaraResult::ALLOW; @@ -14,20 +14,20 @@ CynaraResult Cynara::check(const char* label, const char* privilege, const char* if (strcmp(label, "System::Privileged") == 0) { return CynaraResult::ALLOW; } - if (strcmp(uid, "9999") == 0) { + if (uid == 9999) { if (strcmp(privilege, "http://tizen.org/privilege/packagemanager.admin") == 0) { return CynaraResult::ALLOW; } return CynaraResult::DENY; } if (strcmp(privilege, "privilege1") == 0) { - if (strcmp(uid, "9991") == 0 || strcmp(uid, "9993") == 0) { + if (uid == 9991 || uid == 9993) { return CynaraResult::ALLOW; } return CynaraResult::DENY; } if (strcmp(privilege, "privilege2") == 0) { - if (strcmp(uid, "9992") == 0 || strcmp(uid, "9993") == 0) { + if (uid == 9992 || uid == 9993) { return CynaraResult::ALLOW; } return CynaraResult::DENY; diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index e6c33e2..389b7db 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -18,27 +18,24 @@ NaivePolicyDb& NaivePolicyChecker::getPolicyDb(bool type) { DecisionResult NaivePolicyChecker::parseDecision(const DecisionItem& decision, uid_t uid, const char* label) const { - char uid_str[17]; tslog::log_verbose("----Decision made\n"); switch (decision.getDecision()) { case Decision::ALLOW: return DecisionResult::ALLOW; - case Decision::ANY: - case Decision::DENY: - return DecisionResult::DENY; case Decision::CHECK: - { - std::snprintf(uid_str, sizeof(uid_str) - 1, "%lu", (unsigned long)uid); - ldp_cynara::CynaraResult ret = ldp_cynara::Cynara::check(label, decision.getPrivilege(), uid_str); - if (ret == ldp_cynara::CynaraResult::ALLOW) - return DecisionResult::ALLOW; - else if (ret == ldp_cynara::CynaraResult::DENY) - return DecisionResult::DENY; - else - return DecisionResult::CYNARA_ERROR; - } + switch (ldp_cynara::Cynara::check(label, decision.getPrivilege(), uid)) + { + case ldp_cynara::CynaraResult::ALLOW: + return DecisionResult::ALLOW; + case ldp_cynara::CynaraResult::DENY: + return DecisionResult::DENY; + default: + return DecisionResult::CYNARA_ERROR; + } + default: + break; } return DecisionResult::DENY; } -- 2.7.4 From 3eb3d5ee631af8ba770b315b0f7c035c222e1f8e Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 1 Feb 2019 15:29:14 +0100 Subject: [PATCH 08/16] refactoring: replace char* with C++ strings Change-Id: I2554b13d0d00ed1b8ce5dbdfba7cb5f9cb6db6c7 --- src/internal/policy.cpp | 145 +++++++++++++++++------------------------------- src/internal/policy.hpp | 32 ++++------- 2 files changed, 61 insertions(+), 116 deletions(-) diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 2effa15..d3f2855 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -217,7 +217,7 @@ DecisionItem::DecisionItem(Decision decision, const char* privilege) void DecisionItem::setPrivilege(const char* privilege) { if (privilege == nullptr) { - __privilege = ""; + __privilege.clear(); } else { __privilege = privilege; } @@ -229,7 +229,7 @@ Decision DecisionItem::getDecision() const { } const char* DecisionItem::getPrivilege() const { - if (__privilege == "") { + if (__privilege.empty()) { return nullptr; } return __privilege.c_str(); @@ -252,7 +252,7 @@ ItemOwn::ItemOwn(const char* name, void ItemOwn::setName(const char* name) { if (name == nullptr) { - __name = ""; + __name.clear(); } else { __name = name; } @@ -263,7 +263,7 @@ ItemType ItemOwn::getType() const { } const char* ItemOwn::getName() const { - if (__name == "") { + if (__name.empty()) { return nullptr; } return __name.c_str(); @@ -274,45 +274,40 @@ bool ItemOwn::isPrefix() const { } bool ItemOwn::isMatchAll() const { - return __name == ""; + return __name.empty(); } const DecisionItem& ItemOwn::getDecision() const { return __decision; } -NameSR::NameSR(const char* m, int l) : name(m), len(l) -{ -} - MatchItemSR::MatchItemSR(const char* i, const char* me, const char* p, MessageType t, MessageDirection d) - : names_num(0), interface(i), member(me), path(p), type(t), direction(d) { + : names_num(0), type(t), direction(d) { + if (i) interface = i; + if (me) member = me; + if (p) path = p; } void MatchItemSR::addName(const char* name) { - names[names_num++] = NameSR(name, std::strlen(name)); + names[names_num++] = boost::string_ref(name); } bool MatchItemSR::addNames(const char* name) { - int i = 0; - int j = 0; if (name) { - while (name[i] && names_num < KDBUS_CONN_MAX_NAMES + 1) { - char c; - int len; - j = i; - if (!(isalnum(name[i]) || name[i] == ' ')) - tslog::log_verbose("Wrong name(", i, "): ", name, "\n"); - - while ((c = name[i++]) && ' ' != c); - if (!c) { - --i; - len = i-j; - } else { - len = i-j-1; - } - names[names_num++] = NameSR(name + j, len); + boost::string_ref current(name); + while (!current.empty() && names_num < KDBUS_CONN_MAX_NAMES + 1) { + + if (!(isalnum(current[0]) || current[0] == ' ')) + tslog::log_verbose("Wrong name(", current, "): ", name, "\n"); + + auto space = current.find_first_of(" "); + names[names_num++] = current.substr(0, space); + + if (space != current.npos) // skip the space + current = current.substr(space+1); + else + current.clear(); } if (names_num >= KDBUS_CONN_MAX_NAMES + 1) return false; @@ -320,31 +315,13 @@ bool MatchItemSR::addNames(const char* name) { return true; } -ItemSendReceive::ItemSendReceive(const char* name, - const char* interface, - const char* member, - const char* path, - MessageType type, +ItemSendReceive::ItemSendReceive(MessageType type, MessageDirection direction) - : __name(NameSR(name, name?std::strlen(name):0)), - __interface(interface), - __member(member), - __path(path), - __type(type), + : __type(type), __direction(direction), __is_name_prefix(false) { } -ItemSendReceive::~ItemSendReceive() { - delete[] __interface; - delete[] __member; - delete[] __path; - - if (__name.len > 0) { - delete[] __name.name; - } -} - bool ItemSendReceive::match(const MatchItemSR& item) const { if (__type != MessageType::ANY && __type != item.type) return false; @@ -352,23 +329,23 @@ bool ItemSendReceive::match(const MatchItemSR& item) const { if (__direction != item.direction) return false; - if (__interface && item.interface && std::strcmp(__interface, item.interface)) + if (!__interface.empty() && !item.interface.empty() && __interface != item.interface) return false; - if (__path && item.path && std::strcmp(__path, item.path)) + if (!__path.empty() && !item.path.empty() && __path != item.path) return false; - if (__member && item.member && std::strcmp(__member, item.member)) + if (!__member.empty() && !item.member.empty() && __member != item.member) return false; - if (__name.len > 0) { + if (!__name.empty()) { for (int i = 0; i < item.names_num; i++) { - if (!boost::algorithm::starts_with(item.names[i].name, __name.name)) + if (!boost::algorithm::starts_with(item.names[i], __name)) continue; - if (item.names[i].len == __name.len) + if (item.names[i].length() == __name.length()) return true; - else if (__is_name_prefix && item.names[i].name[__name.len] == '.') + else if (__is_name_prefix && item.names[i][__name.length()] == '.') return true; } return false; @@ -396,12 +373,12 @@ const DecisionItem& ItemSendReceive::getDecision() const { size_t ItemSendReceive::getSize() const { size_t size = __decision.getSize(); - if (__interface) - size += strlen(__interface) + 1; - if (__member) - size += strlen(__member) + 1; - if (__path) - size += strlen(__path) + 1; + if (!__interface.empty()) + size += __interface.length() + 1; + if (!__member.empty()) + size += __member.length() + 1; + if (!__path.empty()) + size += __path.length() + 1; return size; } @@ -500,22 +477,6 @@ void ItemBuilder::reset() { __current_access = ItemAccess(); } -char* ItemBuilder::duplicate(const char* str) { - char* ret; - int i = 0; - int len; - - if (!str) - return NULL; - - len = strlen(str) + 1; - ret = new char[len]; - for (i = 0; i < len; i++) - ret[i] = str[i]; - - return ret; -} - void ItemBuilder::generateItem(NaivePolicyDb& db, PolicyType& policy_type, PolicyTypeValue& policy_type_value) { switch (__current_item_type) { case ItemType::OWN: @@ -548,33 +509,29 @@ void ItemBuilder::addOwner(const char* owner) { void ItemBuilder::addName(const char* name, bool prefix) { ItemSendReceive* sr = getSendReceiveItem(); - if (sr->__name.len > 0) { - delete[] sr->__name.name; - sr->__name.len = 0; - } - if (!name) { - sr->__name.name = NULL; - } else { - sr->__name.name = duplicate(name); - sr->__name.len = std::strlen(name); - } + if (name) + sr->__name = name; + else + sr->__name.clear(); + sr->__is_name_prefix = prefix; } void ItemBuilder::addInterface(const char* interface) { + assert(interface); ItemSendReceive* sr = getSendReceiveItem(); - sr->__interface = duplicate(interface); + sr->__interface = interface; } void ItemBuilder::addMember(const char* member) { - ItemSendReceive* sr = getSendReceiveItem(); - sr->__member = duplicate(member); + assert(member); + getSendReceiveItem()->__member = member; } void ItemBuilder::addPath(const char* path) { - ItemSendReceive* sr = getSendReceiveItem(); - sr->__path = duplicate(path); + assert(path); + getSendReceiveItem()->__path = path; } void ItemBuilder::addMessageType(MessageType type) { @@ -634,7 +591,7 @@ std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item) { stream << "matcher: services("; for (int i = 0; i < item.names_num; i++) { - stream << item.names[i].name; + stream << item.names[i]; if (i != item.names_num -1) stream << " "; } @@ -645,7 +602,7 @@ std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item) std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item) { - return stream << "ItemSR: name(" << item.__name.name << "), inter(" << item.__interface << + 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) << ")"; } diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index b93b450..293e973 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -23,6 +23,7 @@ #ifndef _POLICY_H #define _POLICY_H +#include #include #include #include @@ -150,20 +151,13 @@ namespace ldp_xml_parser }; std::ostream &operator<<(std::ostream& stream, const ItemOwn &item); - /** Name structure for send/receive policy */ - struct NameSR { - const char* name; - int len; - NameSR(const char* m = NULL, int l = 0); - }; - /** Struct which allows to contain multiple connection names then compared in s/r policy checker */ struct MatchItemSR { int names_num; - NameSR names[KDBUS_CONN_MAX_NAMES+1]; - const char* interface; - const char* member; - const char* path; + boost::string_ref names[KDBUS_CONN_MAX_NAMES+1]; + boost::string_ref interface; + boost::string_ref member; + boost::string_ref path; MessageType type; MessageDirection direction; MatchItemSR(const char* i = NULL, const char* me = NULL, const char* p = NULL, MessageType t = MessageType::ANY, MessageDirection d = MessageDirection::ANY); @@ -178,22 +172,17 @@ namespace ldp_xml_parser class ItemSendReceive { private: DecisionItem __decision; - NameSR __name; - const char* __interface; - const char* __member; - const char* __path; + std::string __name; + std::string __interface; + std::string __member; + std::string __path; MessageType __type; MessageDirection __direction; bool __is_name_prefix; public: friend class ItemBuilder; - ItemSendReceive(const char* name = NULL, - const char* interface = NULL, - const char* member = NULL, - const char* path = NULL, - MessageType type = MessageType::ANY, + ItemSendReceive(MessageType type = MessageType::ANY, MessageDirection direction = MessageDirection::ANY); - ~ItemSendReceive(); bool match(const MatchItemSR& item) const; MessageDirection getDirection() const; ItemType getType() const; @@ -251,7 +240,6 @@ namespace ldp_xml_parser ItemOwn* getOwnItem(); ItemSendReceive* getSendReceiveItem(); ItemAccess* getAccessItem(); - char* duplicate(const char* str); public: ItemBuilder(); ~ItemBuilder(); -- 2.7.4 From 5e2f8df7d05aa39daab0c467289d73be171dd2fe Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 1 Feb 2019 15:54:47 +0100 Subject: [PATCH 09/16] refactoring: remove params from ItemSendReceive() Params for ItemSendReceive are never used. Change-Id: I0c4c6bbaf68a4f65472267b3498c14f335561d12 --- src/internal/policy.cpp | 6 ++---- src/internal/policy.hpp | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index d3f2855..0316fb7 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -315,10 +315,8 @@ bool MatchItemSR::addNames(const char* name) { return true; } -ItemSendReceive::ItemSendReceive(MessageType type, - MessageDirection direction) - : __type(type), - __direction(direction), +ItemSendReceive::ItemSendReceive() + : __type(MessageType::ANY), __is_name_prefix(false) { } diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 293e973..0c3a04b 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -181,8 +181,7 @@ namespace ldp_xml_parser bool __is_name_prefix; public: friend class ItemBuilder; - ItemSendReceive(MessageType type = MessageType::ANY, - MessageDirection direction = MessageDirection::ANY); + ItemSendReceive(); bool match(const MatchItemSR& item) const; MessageDirection getDirection() const; ItemType getType() const; -- 2.7.4 From 1f49b077f973c359e968eb54482707cc8b3ee3e3 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 1 Feb 2019 16:11:29 +0100 Subject: [PATCH 10/16] refactoring: introduce MatchItemOwn for matching Change-Id: I0c01db965a1b4d61cea0ac7f0b825d93cb331ed9 --- src/internal/naive_policy_checker.cpp | 6 +++--- src/internal/naive_policy_checker.hpp | 2 +- src/internal/naive_policy_db.cpp | 2 +- src/internal/naive_policy_db.hpp | 2 +- src/internal/own_tree.cpp | 7 +++---- src/internal/own_tree.hpp | 2 +- src/internal/policy.cpp | 5 +++++ src/internal/policy.hpp | 11 +++++++++++ 8 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 389b7db..431f88e 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -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); } @@ -91,9 +91,9 @@ DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySR& poli return Decision::ANY; } -DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyOwn& policy, const ItemOwn& item) const +DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyOwn& policy, const MatchItemOwn& item) const { - tslog::log_verbose("Checking policy for name: ", std::string(item.getName() ? item.getName() : "NULL"), "\n"); + tslog::log_verbose("Checking policy for name: ", std::string(item.getName().empty() ? "NULL" : item.getName()), "\n"); return policy.getDecisionItem(item); } diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index c15eab0..807083b 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -62,7 +62,7 @@ namespace ldp_xml_parser * \ingroup Implementation */ DecisionItem checkPolicy(const NaivePolicyDb::PolicyOwn& policy, - const ItemOwn& item) const; + const MatchItemOwn& item) const; /** Checks access policy for given item * \param[in] policy Policy to check diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index fe08543..52d8d78 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -185,7 +185,7 @@ void NaivePolicyDb::PolicyOwn::printContent() const ownership_tree.printTree(); } -DecisionItem NaivePolicyDb::PolicyOwn::getDecisionItem(const ItemOwn& item) const +DecisionItem NaivePolicyDb::PolicyOwn::getDecisionItem(const MatchItemOwn& item) const { return ownership_tree.getDecisionItem(item); } diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index cedc27a..9fd8164 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -71,7 +71,7 @@ namespace ldp_xml_parser * \param[in] item Item to add to policy */ void addItem(ItemOwn* item); - DecisionItem getDecisionItem(const ItemOwn& item) const; + DecisionItem getDecisionItem(const MatchItemOwn& item) const; void printContent() const; size_t getSize() const; }; diff --git a/src/internal/own_tree.cpp b/src/internal/own_tree.cpp index 9ae9b19..b9cc3d2 100644 --- a/src/internal/own_tree.cpp +++ b/src/internal/own_tree.cpp @@ -61,14 +61,13 @@ void OwnershipTree::addItem(ItemOwn* item) { __root->add(tokens, item->getDecision(), item->isPrefix()); } -DecisionItem OwnershipTree::getDecisionItem(const ItemOwn& item) const +DecisionItem OwnershipTree::getDecisionItem(const MatchItemOwn& item) const { - if (item.getName() == nullptr) { + if (item.getName().length() == 0) { return Decision::DENY; } - std::string name = item.getName(); - auto tokens = tokenize(name); + auto tokens = tokenize(item.getName()); return __root->getDecisionItem(tokens); } diff --git a/src/internal/own_tree.hpp b/src/internal/own_tree.hpp index 669a3e8..b052fea 100644 --- a/src/internal/own_tree.hpp +++ b/src/internal/own_tree.hpp @@ -32,7 +32,7 @@ namespace ldp_xml_parser public: OwnershipTree(); void addItem(ItemOwn* item); - DecisionItem getDecisionItem(const ItemOwn& item) const; + DecisionItem getDecisionItem(const MatchItemOwn& item) const; void printTree() const; size_t getSize() const; diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 0316fb7..d9fe047 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -585,6 +585,11 @@ std::ostream &operator<<(std::ostream& stream, const ItemOwn &item) "), pref(" << item.__is_prefix << ")"; } +std::ostream &operator<<(std::ostream& stream, const MatchItemOwn &item) +{ + return stream << (item._name.empty() ? "NULL" : item._name); +} + std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item) { stream << "matcher: services("; diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 0c3a04b..e72d8b3 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -128,6 +128,17 @@ namespace ldp_xml_parser }; std::ostream &operator<<(std::ostream& stream, const DecisionItem &di); + class MatchItemOwn { + private: + std::string _name; + public: + MatchItemOwn(const char *name) : _name(name) {} + const std::string &getName() const { return _name; } + + friend std::ostream &operator<<(std::ostream& stream, const MatchItemOwn &item); + }; + std::ostream &operator<<(std::ostream& stream, const MatchItemOwn &item); + /** Class contains info about ownership policy item */ class ItemOwn { private: -- 2.7.4 From ceeb3a08fea964d871b5ea179e11a9a3abc27417 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 09:00:53 +0100 Subject: [PATCH 11/16] refactoring: move PolicySet methods into proper class Change-Id: Ia9c72791fd592017ac3e0284f1e72f753f398f10 --- src/internal/naive_policy_db.cpp | 228 +++++++++++++++++++-------------------- src/internal/naive_policy_db.hpp | 55 +++++----- 2 files changed, 135 insertions(+), 148 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 52d8d78..46446cd 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -24,12 +24,12 @@ void NaivePolicyDb::addItem(const PolicyType policy_type, log_item_add(item); const MessageDirection dir = item->getDirection(); if (dir == MessageDirection::SEND) { - addItem(m_send_set, policy_type, policy_type_value, item); + m_send_set.addItem(policy_type, policy_type_value, item); } else if (dir == MessageDirection::RECEIVE) { - addItem(m_receive_set, policy_type, policy_type_value, item); + m_receive_set.addItem(policy_type, policy_type_value, item); } else { - addItem(m_send_set, policy_type, policy_type_value, item); - addItem(m_receive_set, policy_type, policy_type_value, item); + m_send_set.addItem(policy_type, policy_type_value, item); + m_receive_set.addItem(policy_type, policy_type_value, item); } } @@ -37,44 +37,21 @@ void NaivePolicyDb::addItem(const PolicyType policy_type, const PolicyTypeValue policy_type_value, ItemOwn* const item) { log_item_add(item); - addItem(m_own_set, policy_type, policy_type_value, 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) { log_item_add(item); - addItem(m_access_set, policy_type, policy_type_value, item); + m_access_set.addItem(policy_type, policy_type_value, item); } -template -size_t NaivePolicyDb::getSetSize(const PolicySet& set) const -{ - size_t size = sizeof(set); - - for (const auto e : { ContextType::DEFAULT, ContextType::MANDATORY }) - size += set.context[static_cast(e)].getSize(); - - size += set.user.size() * sizeof(typename decltype(set.user)::value_type); - for (const auto& i : set.user) - size += i.second.getSize(); - - size += set.group.size() * sizeof(typename decltype(set.group)::value_type); - for (const auto& i : set.group) - size += i.second.getSize(); - - return size; -} - -template size_t NaivePolicyDb::getSetSize(const PolicySet& set) const; -template size_t NaivePolicyDb::getSetSize(const PolicySet& set) const; -template size_t NaivePolicyDb::getSetSize(const PolicySet& set) const; - void NaivePolicyDb::printContent() const { #define PRINT_SET(x) \ std::cerr << std::endl << "----" #x "----" << std::endl; \ - printSet(x); + (x).printSet(); PRINT_SET(m_own_set) PRINT_SET(m_send_set) @@ -95,46 +72,125 @@ void NaivePolicyDb::printContent() const #undef PRINT_MAP } -void NaivePolicyDb::printMap(const std::map>& map) const +/****************** NaivePolicyDb::PolicySet ************************/ +template +size_t NaivePolicyDb::PolicySet

::getSetSize() const { - int size = sizeof(map) + map.size() * sizeof(std::remove_reference::type::value_type); + size_t size = sizeof(*this); - for (const auto& i : map) { - size += i.second.capacity() * sizeof(gid_t); - std:: cerr << "gid: " << i.first << " |"; - for (auto j : i.second) - std::cerr << " " << j << ","; - std::cerr << std::endl; - } + for (const auto e : { ContextType::DEFAULT, ContextType::MANDATORY }) + size += context[static_cast(e)].getSize(); - std::cerr << "Memory consumption: " << size << " B" << std::endl; + 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::printSet(const PolicySet& set) const +template +void NaivePolicyDb::PolicySet

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

::getPolicy(const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + const P*& policy) 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; + 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 false; + } + policy = &(it->second); + } + return true; + 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 false; + } + policy = &(it->second); + } + return true; + default: + tslog::log("NO POLICY\n"); + } + + return false; +} + +template template +void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, + const PolicyTypeValue policy_type_value, + T* const item) { + switch (policy_type) { + case PolicyType::CONTEXT: + context[static_cast(policy_type_value.context)].addItem(item); + break; + case PolicyType::USER: + user[policy_type_value.user].addItem(item); + break; + case PolicyType::GROUP: + group[policy_type_value.group].addItem(item); + break; + case PolicyType::NONE: + assert(false); + break; + } } -template void NaivePolicyDb::printSet(const PolicySet& set) const; -template void NaivePolicyDb::printSet(const PolicySet& set) const; -template void NaivePolicyDb::printSet(const PolicySet& set) const; +void NaivePolicyDb::printMap(const std::map>& map) const +{ + int size = sizeof(map) + map.size() * sizeof(std::remove_reference::type::value_type); + + for (const auto& i : map) { + 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; +} 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 this->getPolicy(m_own_set, policy_type, policy_type_value, policy); + return m_own_set.getPolicy(policy_type, policy_type_value, policy); } bool NaivePolicyDb::getPolicy(const ItemType item_type, @@ -143,9 +199,9 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, const NaivePolicyDb::PolicySR*& policy) const { switch (item_type) { case ItemType::SEND: - return this->getPolicy(m_send_set, policy_type, policy_type_value, policy); + return m_send_set.getPolicy(policy_type, policy_type_value, policy); case ItemType::RECEIVE: - return this->getPolicy(m_receive_set, policy_type, policy_type_value, policy); + return m_receive_set.getPolicy(policy_type, policy_type_value, policy); default: assert(false); return false; @@ -157,7 +213,7 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyTypeValue policy_type_value, const NaivePolicyDb::PolicyAccess*& policy) const { assert(item_type == ItemType::ACCESS); - return this->getPolicy(m_access_set, policy_type, policy_type_value, policy); + return m_access_set.getPolicy(policy_type, policy_type_value, policy); } void NaivePolicyDb::PolicySR::addItem(ItemSendReceive* item) { @@ -226,72 +282,6 @@ void NaivePolicyDb::PolicyAccess::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } - -template -void NaivePolicyDb::addItem(S& set, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - T* const item) { - switch (policy_type) { - case PolicyType::CONTEXT: - set.context[static_cast(policy_type_value.context)].addItem(item); - break; - case PolicyType::USER: - set.user[policy_type_value.user].addItem(item); - break; - case PolicyType::GROUP: - set.group[policy_type_value.group].addItem(item); - break; - case PolicyType::NONE: - assert(false); - break; - } -} - -template -bool NaivePolicyDb::getPolicy(const PolicySet

& set, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const P*& policy) const -{ - tslog::log("---policy_type ="); - - switch (policy_type) { - case PolicyType::CONTEXT: - tslog::log("CONTEXT =", (int)policy_type_value.context, "\n"); - policy = &set.context[static_cast(policy_type_value.context)]; - return true; - case PolicyType::USER: - { - tslog::log("USER =", (int)policy_type_value.user, "\n"); - - auto it = set.user.find(policy_type_value.user); - if (it == set.user.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return false; - } - policy = &(it->second); - } - return true; - case PolicyType::GROUP: - { - tslog::log("GROUP = ", (int)policy_type_value.group, "\n"); - - auto it = set.group.find(policy_type_value.group); - if (it == set.group.end()) { - tslog::log_verbose("GetPolicy: Out of Range exception\n"); - return false; - } - policy = &(it->second); - } - return true; - default: - tslog::log("NO POLICY\n"); - } - - return false; -} - void NaivePolicyDb::updateSupplementaryGroups(uid_t uid, gid_t gid, const ItemType type) const { auto vsend = &mapSendGroup[uid]; diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 9fd8164..c5626b7 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -148,6 +148,32 @@ namespace ldp_xml_parser 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 + * \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* const 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; + + size_t getSetSize() const; }; /** Set of ownership policies */ @@ -159,36 +185,7 @@ namespace ldp_xml_parser /** Set of bus access policies */ PolicySet m_access_set; - /** 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 - */ - template - void addItem(S& set, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - T* const 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 - */ - template - bool getPolicy(const PolicySet

& set, - const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - const P*& policy) const; - public: - template - size_t getSetSize(const PolicySet& set) const; - template - void printSet(const PolicySet& set) const; void printMap(const std::map>& map) const; void printContent() const; }; -- 2.7.4 From 5bd9984343392affc13464540da33e57b4fe642e Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 10:12:47 +0100 Subject: [PATCH 12/16] refactoring: make common interface for Policy classes Change-Id: I02efdddbafcce4bbbd8e7f7630cececaa1b362ed --- src/internal/naive_policy_checker.cpp | 11 +-------- src/internal/naive_policy_db.cpp | 46 +++++++++++++++++++++++------------ src/internal/naive_policy_db.hpp | 5 +++- 3 files changed, 35 insertions(+), 27 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 431f88e..54d0786 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -79,16 +79,7 @@ DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicySR& poli { tslog::log_verbose("checkpolicy for: ", item, "\n"); - for (auto i : policy) { - 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; + return policy.getDecisionItem(item); } DecisionItem NaivePolicyChecker::checkPolicy(const NaivePolicyDb::PolicyOwn& policy, const MatchItemOwn& item) const diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 46446cd..8945077 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -170,6 +170,36 @@ void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, } } +/****************** NaivePolicyDb::PolicySR ************************/ +void NaivePolicyDb::PolicySR::addItem(ItemSendReceive* item) { + m_items.push_back(item); +} + +size_t NaivePolicyDb::PolicySR::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::PolicySR::printContent() const { + for (const auto& i : m_items) + std::cerr << i << std::endl; +} + +DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSR &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 NaivePolicyDb::printMap(const std::map>& map) const { int size = sizeof(map) + map.size() * sizeof(std::remove_reference::type::value_type); @@ -216,22 +246,6 @@ bool NaivePolicyDb::getPolicy(const ItemType item_type, return m_access_set.getPolicy(policy_type, policy_type_value, policy); } -void NaivePolicyDb::PolicySR::addItem(ItemSendReceive* item) { - m_items.push_back(item); -} - -size_t NaivePolicyDb::PolicySR::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::PolicySR::printContent() const { - for (const auto& i : m_items) - std::cerr << i << std::endl; -} - void NaivePolicyDb::PolicyOwn::addItem(ItemOwn* item) { ownership_tree.addItem(item); } diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index c5626b7..aa1c9a1 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -49,14 +49,17 @@ namespace ldp_xml_parser /** Vector with policy items */ std::vector m_items; typedef decltype(m_items)::const_reverse_iterator PolicyConstIterator; - public: + 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(ItemSendReceive* item); + + DecisionItem getDecisionItem(const MatchItemSR &item) const; void printContent() const; size_t getSize() const; }; -- 2.7.4 From 9729784ff4030927278cb03f3cb8d2fdc6a67167 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 12:09:25 +0100 Subject: [PATCH 13/16] refactoring: group maps moved into Policies Change-Id: I5def57a4ed74a2a72ab76a527091c4e56d5c1b67 --- src/internal/naive_policy_checker.cpp | 4 +- src/internal/naive_policy_db.cpp | 126 +++++++++++++++++----------------- src/internal/naive_policy_db.hpp | 28 ++++---- src/internal/policy.cpp | 4 +- src/internal/policy.hpp | 4 +- 5 files changed, 85 insertions(+), 81 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 54d0786..8652dec 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -45,7 +45,7 @@ DecisionResult NaivePolicyChecker::check(bool bus_type, uid_t uid, gid_t gid, const char* const label) { - const auto* gids = getPolicyDb(bus_type).getGroups(uid, gid, ItemType::ACCESS); + const auto &gids = *getPolicyDb(bus_type).getGroups(uid, gid, ItemType::ACCESS); auto ret = checkItem(bus_type, uid, gid, MatchItemAccess(uid, gids), ItemType::ACCESS); if (ret.getDecision() == Decision::ANY) { if (bus_owner == uid) { @@ -125,7 +125,7 @@ DecisionItem NaivePolicyChecker::checkItem(bool bus_type, uid_t uid, gid_t gid, template DecisionItem NaivePolicyChecker::checkGroupPolicies(const NaivePolicyDb& policy_db, uid_t uid, gid_t gid, const T& item, const ItemType type) { - auto sgroups = policy_db.getGroups(uid, gid, type); + const auto *sgroups = policy_db.getGroups(uid, gid, type); if (sgroups == nullptr) return Decision::ANY; const P* curr_policy = nullptr; diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 8945077..6b2946b 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -62,12 +62,12 @@ void NaivePolicyDb::printContent() const #define PRINT_MAP(x) \ std::cerr << std::endl << "----" #x "----" << std::endl; \ - printMap(x); + (x).printMap(); - PRINT_MAP(mapOwnGroup); - PRINT_MAP(mapSendGroup); - PRINT_MAP(mapRecvGroup); - PRINT_MAP(mapAccessGroup); + PRINT_MAP(m_own_set); + PRINT_MAP(m_send_set); + PRINT_MAP(m_receive_set); + PRINT_MAP(m_access_set); #undef PRINT_MAP } @@ -93,6 +93,22 @@ size_t NaivePolicyDb::PolicySet

::getSetSize() const } 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 { for (const auto e : {ContextType::DEFAULT, ContextType::MANDATORY}) @@ -200,21 +216,6 @@ DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSR &item) c return Decision::ANY; } -void NaivePolicyDb::printMap(const std::map>& map) const -{ - int size = sizeof(map) + map.size() * sizeof(std::remove_reference::type::value_type); - - for (const auto& i : map) { - 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; -} - bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyType policy_type, const PolicyTypeValue policy_type_value, @@ -296,86 +297,85 @@ void NaivePolicyDb::PolicyAccess::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } -void NaivePolicyDb::updateSupplementaryGroups(uid_t uid, gid_t gid, const ItemType type) const + +void NaivePolicyDb::updateSupplementaryGroups(const VGid &groups, uid_t uid, gid_t gid) const { - auto vsend = &mapSendGroup[uid]; - auto vrecv = &mapRecvGroup[uid]; - auto vown = (type == ItemType::GENERIC || type == ItemType::OWN) ? &mapOwnGroup[uid] : nullptr; - auto vaccess = &mapAccessGroup[uid]; + auto &vsend = m_send_set.getMapGroup(uid); + auto &vrecv = m_receive_set.getMapGroup(uid); + auto &vaccess = m_access_set.getMapGroup(uid); - const std::vector groups = get_groups(uid, gid); if (groups.empty()) { - (*vsend).push_back(gid); - (*vrecv).push_back(gid); - (*vaccess).push_back(gid); - if (vown != nullptr) - (*vown).push_back(gid); + 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); + 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 + vrecv.push_back(group); + vaccess.push_back(group); // no filtering, it will be used once } - if ((*vsend).size() == 0) - (*vsend).push_back(-1); - if ((*vrecv).size() == 0) - (*vrecv).push_back(-1); - if ((*vaccess).size() == 0) - (*vaccess).push_back(-1); - if (type == ItemType::GENERIC || type == ItemType::OWN) { + 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); + vown.push_back(group); } - - if ((*vown).size() == 0) - (*vown).push_back(-1); } } -std::vector* NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemType type) const +const NaivePolicyDb::VGid *NaivePolicyDb::getGroups(uid_t uid, gid_t gid, const ItemType type) const { if (type == ItemType::OWN) { - return &mapOwnGroup[uid]; + return &m_own_set.getMapGroup(uid); } if (type == ItemType::ACCESS) { - return &mapAccessGroup[uid]; + return &m_access_set.getMapGroup(uid); } - static gid_t mygid = getgid(); - static uid_t myuid = getuid(); - - if (uid == myuid && gid ==mygid) - return (type == ItemType::SEND) ? &mapSendGroup[uid] : &mapRecvGroup[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) ? &mapSendGroup[uid] : &mapRecvGroup[uid]; + auto &vgid = (type == ItemType::SEND) ? m_send_set.getMapGroup(uid) : m_receive_set.getMapGroup(uid); - if ((*vgid).size() == 0) - updateSupplementaryGroups(uid, gid, type); + if (vgid.empty()) + updateSupplementaryGroups(get_groups(uid, gid), uid, gid); pthread_mutex_unlock(&mutexGroup); - if ((*vgid)[0] == (gid_t)-1) + if (vgid[0] == (gid_t)-1) return nullptr; - return vgid; + return &vgid; } void NaivePolicyDb::initializeGroups(uid_t uid, gid_t gid) { pthread_mutex_lock(&mutexGroup); - mapSendGroup.clear(); - mapRecvGroup.clear(); - mapOwnGroup.clear(); - mapAccessGroup.clear(); + 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); - updateSupplementaryGroups(uid, gid, ItemType::GENERIC); pthread_mutex_unlock(&mutexGroup); } diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index aa1c9a1..ef06bd3 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -31,17 +31,6 @@ namespace ldp_xml_parser { /** Database class, contains policies for ownership and send/receive */ class NaivePolicyDb { - private: - mutable std::map> mapOwnGroup; - mutable std::map> mapSendGroup; - mutable std::map> mapRecvGroup; - mutable std::map> mapAccessGroup; - mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; - - void updateSupplementaryGroups(uid_t uid, gid_t gid, const ItemType type) const; - public: - std::vector* getGroups(uid_t uid, gid_t gid, const ItemType type) const; - void initializeGroups(uid_t uid, gid_t gid); public: /** Class containing policy with send/receive rules */ class PolicySR { @@ -142,8 +131,11 @@ namespace ldp_xml_parser ItemAccess* const item); private: + typedef std::vector VGid; + template class PolicySet { + mutable std::map mapGroup; public: /** Policies for all contexts */ P context[static_cast(ContextType::MAX)]; @@ -176,7 +168,11 @@ namespace ldp_xml_parser 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 */ @@ -188,9 +184,17 @@ namespace ldp_xml_parser /** Set of bus access policies */ PolicySet m_access_set; + /* A mutex for mapGroups within sets */ + mutable pthread_mutex_t mutexGroup = PTHREAD_MUTEX_INITIALIZER; + + 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 printMap(const std::map>& map) const; void printContent() const; + + const VGid *getGroups(uid_t uid, gid_t gid, const ItemType type) const; + + void initializeGroups(uid_t uid, gid_t gid); }; } #endif diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index d9fe047..5379452 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -426,7 +426,7 @@ size_t ItemAccess::getSize() const return __decision.getSize(); } -MatchItemAccess::MatchItemAccess(const uid_t uid, const std::vector* gids) +MatchItemAccess::MatchItemAccess(const uid_t uid, const std::vector &gids) : __uid(uid), __gids(gids) { } @@ -438,7 +438,7 @@ uid_t MatchItemAccess::getUid() const const std::vector& MatchItemAccess::getGids() const { - return *__gids; + return __gids; } ItemOwn* ItemBuilder::getOwnItem() { diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index e72d8b3..a6c5c8a 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -206,10 +206,10 @@ namespace ldp_xml_parser class MatchItemAccess { private: const uid_t __uid; - const std::vector* __gids; + const std::vector &__gids; public: - MatchItemAccess(const uid_t uid, const std::vector* gids); + MatchItemAccess(const uid_t uid, const std::vector &gids); uid_t getUid() const; const std::vector& getGids() const; -- 2.7.4 From 3c0b3b67e7629287b7347e4d67a08408fb9f0784 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 12:31:30 +0100 Subject: [PATCH 14/16] refactoring: move addItem() declarations This is a move towards making single public and single private section in NaivePolicyDb. Change-Id: Ib300da409a4442b1578f3a48ae888c1982de24fa --- src/internal/naive_policy_db.hpp | 49 ++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index ef06bd3..862985b 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -108,28 +108,6 @@ namespace ldp_xml_parser const PolicyTypeValue policy_type_value, const PolicyAccess*& policy) const; - /** Adds item to ownership 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, - ItemOwn* const item); - - /** Adds item to send/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); - - void addItem(const PolicyType policy_type, - const PolicyTypeValue policy_type_value, - ItemAccess* const item); - private: typedef std::vector VGid; @@ -195,6 +173,33 @@ namespace ldp_xml_parser const VGid *getGroups(uid_t uid, gid_t gid, const ItemType type) const; void initializeGroups(uid_t uid, gid_t gid); + + /** Adds item to ownership 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, + ItemOwn* const item); + + /** Adds item to send/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); + + /** Adds item to access 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, + ItemAccess* const item); }; } #endif -- 2.7.4 From df114c3a36232fa4f123f0108f81d3f63b3a0e19 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 12:56:49 +0100 Subject: [PATCH 15/16] refactoring: introduce Send/Receive split types This will help with getting rid of some unnecessary switches/ifs and types. Change-Id: I784e133063c78f2b14ee50c81d526253ec173f0c --- src/internal/naive_policy_db.cpp | 17 +++++++++++------ src/internal/naive_policy_db.hpp | 19 ++++++++++++++----- src/internal/policy.cpp | 12 ++++++++++++ src/internal/policy.hpp | 33 +++++++++++++++++++++++++++++++-- 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 6b2946b..a4f54a7 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -186,24 +186,28 @@ void NaivePolicyDb::PolicySet

::addItem(const PolicyType policy_type, } } -/****************** NaivePolicyDb::PolicySR ************************/ -void NaivePolicyDb::PolicySR::addItem(ItemSendReceive* item) { +/****************** NaivePolicyDb::PolicySRBase ************************/ +template +void NaivePolicyDb::PolicySRBase::addItem(TI* item) { m_items.push_back(item); } -size_t NaivePolicyDb::PolicySR::getSize() const { - size_t size = m_items.capacity() * sizeof(decltype(m_items)::value_type); +template +size_t NaivePolicyDb::PolicySRBase::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; } -void NaivePolicyDb::PolicySR::printContent() const { +template +void NaivePolicyDb::PolicySRBase::printContent() const { for (const auto& i : m_items) std::cerr << i << std::endl; } -DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSR &item) const { +template +DecisionItem NaivePolicyDb::PolicySRBase::getDecisionItem(const TM &item) const { for (auto i : *this) { tslog::log_verbose("-read: ", i->getDecision(), " ", i, "\n"); @@ -215,6 +219,7 @@ DecisionItem NaivePolicyDb::PolicySR::getDecisionItem(const MatchItemSR &item) c } return Decision::ANY; } +template DecisionItem NaivePolicyDb::PolicySRBase::getDecisionItem(const MatchItemSR &item) const; bool NaivePolicyDb::getPolicy(const ItemType item_type, const PolicyType policy_type, diff --git a/src/internal/naive_policy_db.hpp b/src/internal/naive_policy_db.hpp index 862985b..b8489cc 100755 --- a/src/internal/naive_policy_db.hpp +++ b/src/internal/naive_policy_db.hpp @@ -33,11 +33,12 @@ namespace ldp_xml_parser class NaivePolicyDb { public: /** Class containing policy with send/receive rules */ - class PolicySR { + template + class PolicySRBase { private: /** Vector with policy items */ - std::vector m_items; - typedef decltype(m_items)::const_reverse_iterator PolicyConstIterator; + 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(); } @@ -46,12 +47,20 @@ namespace ldp_xml_parser /** Adds given item to policy. * \param[in] item Item to add to policy */ - void addItem(ItemSendReceive* item); + void addItem(TI* item); - DecisionItem getDecisionItem(const MatchItemSR &item) const; + DecisionItem getDecisionItem(const TM &item) const; void printContent() const; size_t getSize() const; }; + class PolicySend : public PolicySRBase { + public: + }; + class PolicyReceive : public PolicySRBase { + public: + }; + + typedef PolicySRBase PolicySR; /** Class containing policy with ownership rules */ class PolicyOwn { diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 5379452..c3f43ad 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -603,6 +603,18 @@ std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item) __message_dir_to_str(item.direction) << ")"; } +std::ostream &operator<<(std::ostream& stream, const MatchItemSend &item) +{ + stream << "MatchItemSend"; + return stream << static_cast(item); +} + +std::ostream &operator<<(std::ostream& stream, const MatchItemReceive &item) +{ + stream << "MatchItemReceive"; + return stream << static_cast(item); +} + std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item) { return stream << ": name(" << item.__name << "), inter(" << item.__interface << diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index a6c5c8a..32037e7 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -27,6 +27,7 @@ #include #include #include +#include /** Maximum tree node children. It is connected with proper characters which can be used in name.*/ #define MAX_CHILDREN 65 @@ -171,7 +172,7 @@ namespace ldp_xml_parser boost::string_ref path; MessageType type; MessageDirection direction; - MatchItemSR(const char* i = NULL, const char* me = NULL, const char* p = NULL, MessageType t = MessageType::ANY, MessageDirection d = MessageDirection::ANY); + MatchItemSR(const char* i, const char* me, const char* p, MessageType t, MessageDirection d); void addName(const char* name); bool addNames(const char* name); @@ -179,9 +180,21 @@ namespace ldp_xml_parser }; std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item); + class MatchItemSend : public MatchItemSR { + using MatchItemSR::MatchItemSR; + friend std::ostream &operator<<(std::ostream& stream, const MatchItemSend &item); + }; + std::ostream &operator<<(std::ostream& stream, const MatchItemSend &item); + + class MatchItemReceive : public MatchItemSR { + using MatchItemSR::MatchItemSR; + friend std::ostream &operator<<(std::ostream& stream, const MatchItemReceive &item); + }; + std::ostream &operator<<(std::ostream& stream, const MatchItemReceive &item); + /** Class contains info about item send/receive */ class ItemSendReceive { - private: + protected: DecisionItem __decision; std::string __name; std::string __interface; @@ -203,6 +216,22 @@ namespace ldp_xml_parser }; std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item); + template + class ItemSR : public ItemSendReceive { + public: + bool match(const M &item) const { + return ItemSendReceive::match(item); + } + + friend std::ostream &operator<<(std::ostream& stream, const ItemSR &item) { + stream << typeid(item).name(); + return stream << static_cast(item); + } + }; + + typedef ItemSR ItemSend; + typedef ItemSR ItemReceive; + class MatchItemAccess { private: const uid_t __uid; -- 2.7.4 From cda9c5d302fc9a5a14c1ccac130304ca10ce8682 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Mon, 4 Feb 2019 15:20:47 +0100 Subject: [PATCH 16/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