From a791590b5fa2452c66508f301bb8452aff79fe58 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 7 Dec 2018 13:54:36 +0100 Subject: [PATCH 01/16] tests for send_destination_prefix Change-Id: Ic7943971b11eeae8dae55ccd50b884beb84dba0a --- Makefile.am | 5 +- ...libdbuspolicy1-send_destination_prefix-deny.cpp | 164 +++++++++++++++++++++ .../system.d/send_destination_prefix.test.conf | 99 +++++++++++++ 3 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 src/test-libdbuspolicy1-send_destination_prefix-deny.cpp create mode 100644 tests/default_deny/system.d/send_destination_prefix.test.conf diff --git a/Makefile.am b/Makefile.am index e7b63a1..35c5d37 100644 --- a/Makefile.am +++ b/Makefile.am @@ -83,7 +83,8 @@ TESTS = src/test-libdbuspolicy1-ownership \ src/test-libdbuspolicy1-ownership-deny \ src/test-libdbuspolicy1-signal \ src/test-libdbuspolicy1-method \ - src/test-libdbuspolicy1-access-deny + src/test-libdbuspolicy1-access-deny \ + src/test-libdbuspolicy1-send_destination_prefix-deny check_PROGRAMS = $(TESTS) @@ -92,6 +93,7 @@ src_test_libdbuspolicy1_ownership_deny_SOURCES = src/test-libdbuspolicy1-ownersh src_test_libdbuspolicy1_signal_SOURCES = src/test-libdbuspolicy1-signal.cpp src_test_libdbuspolicy1_method_SOURCES = src/test-libdbuspolicy1-method.cpp src_test_libdbuspolicy1_access_deny_SOURCES = src/test-libdbuspolicy1-access-deny.cpp +src_test_libdbuspolicy1_send_destination_prefix_deny_SOURCES = src/test-libdbuspolicy1-send_destination_prefix-deny.cpp noinst_LTLIBRARIES = src/libinternal.a src_libinternal_a_SOURCES =\ @@ -109,6 +111,7 @@ src_test_libdbuspolicy1_ownership_deny_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_signal_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_method_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_access_deny_LDADD = $(TESTS_LDADD) +src_test_libdbuspolicy1_send_destination_prefix_deny_LDADD = $(TESTS_LDADD) if ENABLE_STANDALONE_TESTS noinst_LTLIBRARIES += src/libinternalfortests.a diff --git a/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp new file mode 100644 index 0000000..c2d5a45 --- /dev/null +++ b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp @@ -0,0 +1,164 @@ +#include +#include +#include "internal/internal.h" +#include "internal/policy.hpp" +#include "libdbuspolicy1-private.h" + +struct Test { + bool expected_result; + uid_t user; + gid_t group; + const char *label; + const char *destination; + const char *path; + const char *interface; + const char *member; + ldp_xml_parser::MessageType type; +}; + +const int ROOT = 0; + +#define TC(expected_result, names) \ + {(expected_result), ROOT, ROOT, "User::Shell", (names), "/", "a.b", "d", ldp_xml_parser::MessageType::METHOD_CALL} + +/** + * This test set tests ability to parse xml db + * and check sending privilege in use cases + * checking send_destination_prefix + */ +struct Test tests[]={ + /* straight-forward tests - base allow */ + TC(true, "org.tizen.test.dest_prefix.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.f.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.apf"), + TC(false, "org.tizen.test.dest_prefix.apf.f.f.f.f"), + /* multiple names owned */ + TC(true, "org.tizen.test.dest_prefix.ao org.tizen.test.dest_prefix.ap.f"), + TC(true, "org.tizen.test.dest_prefix.ap.f org.tizen.test.dest_prefix.ao"), + TC(false, "org.tizen.test.dest_prefix.do org.tizen.test.dest_prefix.ap.f"), + TC(false, "org.tizen.test.dest_prefix.ap.f org.tizen.test.dest_prefix.do"), + /* target holes in default allow */ + TC(false, "org.tizen.test.dest_prefix.ap.1.d"), + TC(false, "org.tizen.test.dest_prefix.ap.1.dp"), + TC(false, "org.tizen.test.dest_prefix.ap.1.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.ap.1.dp.f.f.f.f org.tizen.test.dest_prefix.ao"), + TC(false, "org.tizen.test.dest_prefix.ap.1.dp.f.f.f.f org.tizen.test.dest_prefix.ap"), + TC(false, "org.tizen.test.dest_prefix.ao org.tizen.test.dest_prefix.ap.1.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.ap org.tizen.test.dest_prefix.ap.1.dp.f.f.f.f"), + /* target holes in holes in default allow */ + TC(true, "org.tizen.test.dest_prefix.ap.1.d.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.1.d.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.1.dp.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.1.dp.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.1.dp.a"), + /* check redefinitions in default allow */ + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.dp"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.ap"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.ap.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.ap.d"), + TC(true, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.a"), + TC(true, "org.tizen.test.dest_prefix.ap.2.apxdp.dp.ap.f.a"), + TC(true, "org.tizen.test.dest_prefix.ap.2.apxdp.f.f.f.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.2.apxdp.f.f.f.ap.f.f.f"), + /* totally cancelling previous definitions in default allow */ + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.dp"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.dp.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.dp.ap"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.dp.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.ap.3.dpxap.ap.dp.a"), + /* straight-forward tests - base deny */ + TC(false, "org.tizen.test.dest_prefix.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.f.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dpf"), + TC(false, "org.tizen.test.dest_prefix.dpf.f.f.f.f"), + /* multiple names owned */ + TC(false, "org.tizen.test.dest_prefix.do org.tizen.test.dest_prefix.dp.f"), + TC(false, "org.tizen.test.dest_prefix.dp.f org.tizen.test.dest_prefix.do"), + TC(false, "org.tizen.test.dest_prefix.ao org.tizen.test.dest_prefix.dp.f"), + TC(false, "org.tizen.test.dest_prefix.dp.f org.tizen.test.dest_prefix.ao"), + /* target holes in default deny */ + TC(true, "org.tizen.test.dest_prefix.dp.1.a"), + TC(true, "org.tizen.test.dest_prefix.dp.1.ap"), + TC(true, "org.tizen.test.dest_prefix.dp.1.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.dp.1.ap.f.f.f.f org.tizen.test.dest_prefix.do"), + TC(true, "org.tizen.test.dest_prefix.dp.1.ap.f.f.f.f org.tizen.test.dest_prefix.dp"), + TC(true, "org.tizen.test.dest_prefix.do org.tizen.test.dest_prefix.dp.1.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.dp org.tizen.test.dest_prefix.dp.1.ap.f.f.f.f"), + /* target holes in holes in default demy */ + TC(false, "org.tizen.test.dest_prefix.dp.1.a.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.1.a.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.1.ap.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.1.ap.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.1.ap.d"), + /* check redefinitions in default deny */ + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.ap"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.dp"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.dp.f.f.f.f"), + TC(true, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.dp.a"), + TC(false, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.d"), + TC(false, "org.tizen.test.dest_prefix.dp.2.dpxap.ap.dp.f.d"), + TC(false, "org.tizen.test.dest_prefix.dp.2.dpxap.f.f.f.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.2.dpxap.f.f.f.dp.f.f.f"), + /* totally cancelling previous definitions in default deny */ + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.ap"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.ap.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.ap.dp"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.ap.dp.f.f.f.f"), + TC(false, "org.tizen.test.dest_prefix.dp.3.apxdp.dp.ap.d"), + /* checking order in multiple names case */ + TC(false, "org.tizen.test.dest_prefix.ao org.tizen.test.dest_prefix.do"), + TC(true, "org.tizen.test.dest_prefix.ao.ao org.tizen.test.dest_prefix.do"), + TC(false, "org.tizen.test.dest_prefix.do org.tizen.test.dest_prefix.ao"), + TC(true, "org.tizen.test.dest_prefix.do org.tizen.test.dest_prefix.ao.ao"), +}; + +void test_print(struct Test* t, bool result) { + printf("uid = %lu, gid = %lu, label = %s, destination = %s, expected = %d, result = %d", + (unsigned long)t->user, (unsigned long)t->group, t->label, t->destination, (int)t->expected_result, (int)result); +} + +bool test() +{ + unsigned i = 0; + bool flag = true; + bool ret = true; + + __internal_init(false, "tests/default_deny/system.conf"); + + for (i = 0; i < sizeof(tests)/sizeof(struct Test); i++) { + ret = __internal_can_send(SYSTEM_BUS, tests[i].user, + tests[i].group, tests[i].label, tests[i].destination, + tests[i].path, tests[i].interface, tests[i].member, + (int)tests[i].type); + + if (tests[i].expected_result != ret) { + printf("[ERROR][%d] test failed: %d %d ", i, (int)((tests[i].expected_result)), ret); + test_print(&tests[i], ret); + printf("\n"); + flag = false; + } + } + return flag; +} + +int main() +{ + __internal_init_once(); + if (!test()) + return -1; + return 0; +} diff --git a/tests/default_deny/system.d/send_destination_prefix.test.conf b/tests/default_deny/system.d/send_destination_prefix.test.conf new file mode 100644 index 0000000..b0496d5 --- /dev/null +++ b/tests/default_deny/system.d/send_destination_prefix.test.conf @@ -0,0 +1,99 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + -- 2.7.4 From 96e88c43df1394824e0ffa3d2d19477359190449 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Fri, 18 Jan 2019 12:36:18 +0100 Subject: [PATCH 02/16] internal: make nondestructible global static_parser This should help with destructing global variable while using it in other thread. Multi-threaded daemons often kill the main thread before killing the other threads. This patch prevents instance memory release when the main thread exits. , and thus required to avoid crash on released memory. .#0 ldp_xml_parser::ItemBuilder::~ItemBuilder (this=0xb67a980c , __in_chrg=) at src/internal/policy.cpp:510 .#1 0xb67a5de8 in ldp_xml_parser::XmlParser::~XmlParser (this=0xb67a980c , __in_chrg=) at /usr/lib/gcc/armv7l-tizen-linux-gnueabi/6.2.1/include/c++/ext/new_allocator.h:110 .#2 0xb695806c in __cxa_finalize (d=0xb67a9438) at cxa_finalize.c:83 .#3 0xb679bcea in __do_global_dtors_aux () from /lib/libdbuspolicy1.so.1 .#4 0xb6fdfa1c in _dl_fini () at dl-fini.c:235 .#5 0xb6957a44 in __run_exit_handlers (status=, listp=, run_list_atexit=run_list_atexit@entry=true, run_dtors=run_dtors@entry=true) at exit.c:106 .#6 0xb6957b6c in __GI_exit (status=) at exit.c:137 .#7 0xb6940640 in __libc_start_main (main=0xbefffe44, argc=-1230635008, argv=0xb6940640 <__libc_start_main+280>, init=, fini=0x7f55f69c <__libc_csu_fini>, rtld_fini=0xb6fdf7e4 <_dl_fini>, stack_end=0xbefffe44) . at libc-start.c:323 .#8 0x7f5589e0 in _start () at ../sysdeps/arm/start.S:110 Change-Id: I0cc0a2623eee688b0498fccacb8a2bc219fd3a94 Signed-off-by: Hyotaek Shim --- src/internal/global_nodestruct.hpp | 38 +++++++++++++++++++++++++++++++++++ src/internal/internal.cpp | 4 ++-- src/internal/naive_policy_checker.hpp | 13 +----------- src/internal/xml_parser.cpp | 8 ++++---- src/internal/xml_parser.hpp | 4 +++- 5 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 src/internal/global_nodestruct.hpp diff --git a/src/internal/global_nodestruct.hpp b/src/internal/global_nodestruct.hpp new file mode 100644 index 0000000..6bc7fe7 --- /dev/null +++ b/src/internal/global_nodestruct.hpp @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2015-2019 Samsung Electronics Co., Ltd All Rights Reserved + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +/** + * \file + * \ingroup Implementation + */ + + +#ifndef GLOBAL_NODESTRUCT_HPP +#define GLOBAL_NODESTRUCT_HPP + +/** Declare a global variable that is constructed (via gcc's __attribute__((constructor))) but never destroyed. */ +#define DCL_NODESTRUCT_GLOBAL(TYPE,NAME)\ + namespace detail_NODESTRUCT_GLOBAL { alignas(TYPE) extern uint8_t NAME[sizeof(TYPE)]; }\ + /* awkwardly written to silence gcc's -Wstrict-aliasing because _Pragma("GCC diagnostic ignored \"-Wstrict-aliasing\"") is bugged */\ + static inline TYPE &NAME() { auto p = static_cast(&detail_NODESTRUCT_GLOBAL::NAME[0]); return *reinterpret_cast(p); }\ + namespace detailInit_NODESTRUCT_GLOBAL { __attribute__((constructor)) static void NAME() { new(&::NAME()) TYPE(); } } + +/** Define a global variable that is constructed (via gcc's __attribute__((constructor))) but never destroyed. */ +#define DEF_NODESTRUCT_GLOBAL(TYPE,NAME)\ + namespace detail_NODESTRUCT_GLOBAL { alignas(TYPE) uint8_t NAME[sizeof(TYPE)]; } + + +#endif // GLOBAL_NODESTRUCT_HPP diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index 8100483..78b8dc1 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -35,7 +35,7 @@ static const char* get_str(const char* const szstr) { int __internal_init(bool bus_type, const char* const config_name) { policy_checker().clearDb(bus_type); - auto err = ldp_xml_parser::static_parser.parsePolicy(bus_type, get_str(config_name)); + auto err = static_parser().parsePolicy(bus_type, get_str(config_name)); if (tslog::enabled()) memory_dump(bus_type); return err; @@ -64,7 +64,7 @@ void __internal_init_flush_logs() void __internal_init_sup_group(bool bus_type, uid_t uid, gid_t gid) { - ldp_xml_parser::static_parser.updateGroupPolicy(bus_type, uid, gid); + static_parser().updateGroupPolicy(bus_type, uid, gid); } void __internal_enter() diff --git a/src/internal/naive_policy_checker.hpp b/src/internal/naive_policy_checker.hpp index d4379cc..c15eab0 100644 --- a/src/internal/naive_policy_checker.hpp +++ b/src/internal/naive_policy_checker.hpp @@ -25,6 +25,7 @@ #include "policy.hpp" #include "naive_policy_db.hpp" +#include "global_nodestruct.hpp" namespace ldp_xml_parser { @@ -170,18 +171,6 @@ namespace ldp_xml_parser }; } -/** Declare a global variable that is constructed (via gcc's __attribute__((constructor))) but never destroyed. */ -#define DCL_NODESTRUCT_GLOBAL(TYPE,NAME)\ - namespace detail_NODESTRUCT_GLOBAL { alignas(TYPE) extern uint8_t NAME[sizeof(TYPE)]; }\ - /* awkwardly written to silence gcc's -Wstrict-aliasing because _Pragma("GCC diagnostic ignored \"-Wstrict-aliasing\"") is bugged */\ - static inline TYPE &NAME() { auto p = static_cast(&detail_NODESTRUCT_GLOBAL::NAME[0]); return *reinterpret_cast(p); }\ - namespace detailInit_NODESTRUCT_GLOBAL { __attribute__((constructor)) static void NAME() { new(&::NAME()) TYPE(); } } - -/** Define a global variable that is constructed (via gcc's __attribute__((constructor))) but never destroyed. */ -#define DEF_NODESTRUCT_GLOBAL(TYPE,NAME)\ - namespace detail_NODESTRUCT_GLOBAL { alignas(TYPE) uint8_t NAME[sizeof(TYPE)]; } - - DCL_NODESTRUCT_GLOBAL(ldp_xml_parser::NaivePolicyChecker, policy_checker) #endif diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index 25cc199..03d6a13 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -49,7 +49,7 @@ void parseAssert(bool condition) { void start_element_handler(void *data, const char *el, const char **attr) { (void)data; - XmlParser& parser = static_parser; + XmlParser& parser = static_parser(); try { parser.elementStart(el, attr); } catch (...) { @@ -60,7 +60,7 @@ void start_element_handler(void *data, const char *el, const char **attr) { void end_element_handler(void *data, const char *el) { (void)data; - XmlParser& parser = static_parser; + XmlParser& parser = static_parser(); try { parser.elementEnd(el); } catch (...) { @@ -71,7 +71,7 @@ void end_element_handler(void *data, const char *el) { void text_handler(void *data, const char *text, int len) { (void)data; - XmlParser& parser = static_parser; + XmlParser& parser = static_parser(); try { parser.text(text, len); } catch (...) { @@ -297,4 +297,4 @@ void XmlParser::getIncludedFiles(const std::string& parent_dir, const std::strin } } -ldp_xml_parser::XmlParser ldp_xml_parser::static_parser; +DEF_NODESTRUCT_GLOBAL(ldp_xml_parser::XmlParser, static_parser); diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp index 3cf7899..2369c61 100755 --- a/src/internal/xml_parser.hpp +++ b/src/internal/xml_parser.hpp @@ -24,6 +24,7 @@ #include #include #include "policy.hpp" +#include "global_nodestruct.hpp" namespace ldp_xml_parser { @@ -81,7 +82,8 @@ namespace ldp_xml_parser std::vector included_files; }; - extern XmlParser static_parser; } //namespace +DCL_NODESTRUCT_GLOBAL(ldp_xml_parser::XmlParser, static_parser) + #endif -- 2.7.4 From 625c027c0ad3528b1da6e9087b0bd7fd518d0f2b Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Tue, 22 Jan 2019 15:37:34 +0100 Subject: [PATCH 03/16] internal: make debug logs small to be more readable The logs take relatively large space within actual source code. This makes them smaller to help with avoiding distraction. Change-Id: Iafca91f5e30f4446e28215cafbf78325ac9287bd --- src/internal/naive_policy_checker.cpp | 16 ++------ src/internal/naive_policy_db.cpp | 20 +++------- src/internal/own_tree.cpp | 4 +- src/internal/policy.cpp | 73 ++++++++++++++++++++--------------- src/internal/policy.hpp | 20 +++++++--- 5 files changed, 64 insertions(+), 69 deletions(-) diff --git a/src/internal/naive_policy_checker.cpp b/src/internal/naive_policy_checker.cpp index 0e2405b..f719e7f 100755 --- a/src/internal/naive_policy_checker.cpp +++ b/src/internal/naive_policy_checker.cpp @@ -13,9 +13,7 @@ DEF_NODESTRUCT_GLOBAL(ldp_xml_parser::NaivePolicyChecker, policy_checker) static void __log_item(const MatchItemSR& item) { - char tmp[MAX_LOG_LINE]; - const char* i_str = item.toString(tmp); - std::cout << "checkpolicy for: " << i_str <getDecision().toString(tmp); - std::cout << "-read: " << i_str; - i_str = i->toString(tmp); - std::cout << " " << i_str <getDecision() << " " << i << std::endl; } if (i->match(item)) { if (tslog::verbose()) { - char tmp[MAX_LOG_LINE]; - const char* i_str = i->getDecision().toString(tmp); - std::cout << "-matched: " << i_str; - const char* i_str2 = i->toString(tmp); - std::cout << " " << i_str2 <getDecision() << " " << i << std::endl; } return i->getDecision(); } diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index 1898b99..177df28 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -16,11 +16,7 @@ using namespace ldp_xml_parser; template void log_item_add(const T* item) { if (tslog::enabled()) { - char tmp[MAX_LOG_LINE]; - const char* i_str = item->toString(tmp); - char tmp2[MAX_LOG_LINE]; - const char* d_str = item->getDecision().toString(tmp2); - std::cout << "Add item: " << i_str << ", decision: " << d_str << std::endl; + std::cout << "Add item: " << item << ", decision: " << item->getDecision() << std::endl; } } @@ -211,11 +207,8 @@ size_t NaivePolicyDb::PolicySR::getSize() const { } void NaivePolicyDb::PolicySR::printContent() const { - char str[MAX_LOG_LINE]; - for (const auto& i : m_items) { - i->toString(str); - std::cerr << str << std::endl; - } + for (const auto& i : m_items) + std::cerr << i << std::endl; } void NaivePolicyDb::PolicyOwn::addItem(ItemOwn* item) { @@ -265,11 +258,8 @@ size_t NaivePolicyDb::PolicyAccess::getSize() const } void NaivePolicyDb::PolicyAccess::printContent() const { - char str[MAX_LOG_LINE]; - for (const auto& i : m_items) { - i.toString(str); - std::cerr << str << std::endl; - } + for (const auto& i : m_items) + std::cerr << i << std::endl; } template diff --git a/src/internal/own_tree.cpp b/src/internal/own_tree.cpp index e285526..9ae9b19 100644 --- a/src/internal/own_tree.cpp +++ b/src/internal/own_tree.cpp @@ -74,10 +74,8 @@ DecisionItem OwnershipTree::getDecisionItem(const ItemOwn& item) const void OwnershipTree::printTreeLevel(const TreeNode& node, const std::string& indent) const { - char str[MAX_LOG_LINE]; - std::cerr << indent << "| " << node.__token << " (" << node.__children.size() << ") | " - << node.__own_decision_item.toString(str) << " " << node.__own_prefix_decision_item.toString(str) << std::endl; + << node.__own_decision_item << " " << node.__own_prefix_decision_item << std::endl; for (const auto& i : node.__children) printTreeLevel(*i.second, indent + "\t"); diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index b10f7c6..ce0e3ae 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -239,11 +239,6 @@ ItemType DecisionItem::getType() const { return ItemType::GENERIC; } -const char* DecisionItem::toString(char* str) const { - snprintf(str, MAX_LOG_LINE, "%s%s%s", __decision_to_str(__decision), __privilege == "" ? "" : ":", __privilege.c_str()); - return str; -} - size_t DecisionItem::getSize() const { return get_string_heap_allocated_memory(__privilege); } @@ -267,11 +262,6 @@ ItemType ItemOwn::getType() const { return ItemType::OWN; } -const char* ItemOwn::toString(char* str) const { - snprintf(str, MAX_LOG_LINE, "ItemOwn: service(%s), pref(%d)", __name == "" ? "*" : __name.c_str(), __is_prefix); - return str; -} - const char* ItemOwn::getName() const { if (__name == "") { return nullptr; @@ -344,11 +334,6 @@ ItemSendReceive::ItemSendReceive(const char* name, __direction(direction) { } -const char* ItemSendReceive::toString(char* str) const { - snprintf(str, MAX_LOG_LINE, "ItemSR: name(%s), inter(%s), member(%s), path(%s), type(%s), dir(%s)", __name.name, __interface, __member, __path, __message_type_to_str(__type), __message_dir_to_str(__direction)); - return str; -} - ItemSendReceive::~ItemSendReceive() { delete[] __interface; delete[] __member; @@ -460,12 +445,6 @@ bool ItemAccess::match(const MatchItemAccess& query) const return false; } -const char* ItemAccess::toString(char* str) const -{ - snprintf(str, MAX_LOG_LINE, "ItemAccess: type(%s), uid(%d), gid(%d)", __access_type_to_str(__type), __uid, __gid); - return str; -} - size_t ItemAccess::getSize() const { return __decision.getSize(); @@ -582,17 +561,6 @@ void ItemBuilder::addName(const char* name, bool prefix) { sr->__is_name_prefix = prefix; } -const char* MatchItemSR::toString(char* str) const { - char tmp[MAX_LOG_LINE]; - tmp[0] = 0; - for (int i = 0; i < names_num; i++) { - std::strncat(tmp, names[i].name, sizeof(tmp) - strlen(tmp) - 1); - std::strncat(tmp, " ", sizeof(tmp) - strlen(tmp) - 1); - } - snprintf(str, MAX_LOG_LINE, "matcher: services(%s), interface(%s), member(%s), path(%s), type(%s), direction(%s)", tmp, interface, member, path, __message_type_to_str(type), __message_dir_to_str(direction) ); - return str; -} - void ItemBuilder::addInterface(const char* interface) { ItemSendReceive* sr = getSendReceiveItem(); sr->__interface = duplicate(interface); @@ -647,3 +615,44 @@ PolicyTypeValue::PolicyTypeValue(ContextType type) : context(type) { PolicyTypeValue::PolicyTypeValue(uid_t us) : user(us) { } + +namespace ldp_xml_parser { + +std::ostream &operator<<(std::ostream& stream, const DecisionItem &di) +{ + return stream << __decision_to_str(di.__decision) << (di.__privilege.empty() ? "" : ":") << di.__privilege; +} + +std::ostream &operator<<(std::ostream& stream, const ItemOwn &item) +{ + return stream << "ItemOwn: service(" << (item.__name.empty() ? "*" : item.__name) << + "), pref(" << item.__is_prefix << ")"; +} + +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; + 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) << ")"; +} + +std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item) +{ + return stream << "ItemSR: name(" << item.__name.name << "), inter(" << item.__interface << + "), member(" << item.__member << "), path(" << item.__path << "), type(" << + __message_type_to_str(item.__type) << "), dir(" << __message_dir_to_str(item.__direction) << ")"; +} + +std::ostream &operator<<(std::ostream& stream, const ItemAccess &item) +{ + return stream << "ItemAccess: type(" << __access_type_to_str(item.__type) << "), uid(" << + item.__uid << "), gid(" << item.__gid << ")"; +} + +} diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index dcee2f5..536ad8d 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -27,7 +27,6 @@ #include #include -#define MAX_LOG_LINE 1024 /** Maximum tree node children. It is connected with proper characters which can be used in name.*/ #define MAX_CHILDREN 65 @@ -122,9 +121,11 @@ namespace ldp_xml_parser Decision getDecision() const; const char* getPrivilege() const; ItemType getType() const; - const char* toString(char* str) const; size_t getSize() const; + + friend std::ostream &operator<<(std::ostream& stream, const DecisionItem &di); }; + std::ostream &operator<<(std::ostream& stream, const DecisionItem &di); /** Class contains info about ownership policy item */ class ItemOwn { @@ -140,12 +141,14 @@ namespace ldp_xml_parser const char* privilege = NULL); bool match(const char* const name) const; ItemType getType() const; - const char* toString(char* str) const; const DecisionItem& getDecision() const; const char* getName() const; bool isPrefix() const; bool isMatchAll() const; + + friend std::ostream &operator<<(std::ostream& stream, const ItemOwn &item); }; + std::ostream &operator<<(std::ostream& stream, const ItemOwn &item); /** Name structure for send/receive policy */ struct NameSR { @@ -166,8 +169,10 @@ namespace ldp_xml_parser MatchItemSR(const char* i = NULL, const char* me = NULL, const char* p = NULL, MessageType t = MessageType::ANY, MessageDirection d = MessageDirection::ANY); void addName(const char* name); bool addNames(const char* name); - const char* toString(char* str) const; + + friend std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item); }; + std::ostream &operator<<(std::ostream& stream, const MatchItemSR &item); /** Class contains info about item send/receive */ class ItemSendReceive { @@ -192,10 +197,12 @@ namespace ldp_xml_parser bool match(const MatchItemSR& item) const; MessageDirection getDirection() const; ItemType getType() const; - const char* toString(char* str) const; const DecisionItem& getDecision() const; size_t getSize() const; + + friend std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item); }; + std::ostream &operator<<(std::ostream& stream, const ItemSendReceive &item); class MatchItemAccess { private: @@ -222,9 +229,10 @@ namespace ldp_xml_parser friend class ItemBuilder; const DecisionItem& getDecision() const; bool match(const MatchItemAccess& query) const; - const char* toString(char* str) const; size_t getSize() const; + friend std::ostream &operator<<(std::ostream& stream, const ItemAccess &item); }; + std::ostream &operator<<(std::ostream& stream, const ItemAccess &item); class NaivePolicyDb; -- 2.7.4 From aaa665533f504cd1442edd8f7da072af8c66c072 Mon Sep 17 00:00:00 2001 From: Adrian Szyndela Date: Wed, 23 Jan 2019 11:37:22 +0100 Subject: [PATCH 04/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 05/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 06/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 07/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 08/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 09/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 10/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 11/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 12/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 13/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 14/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 15/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 16/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