From af5eef6979e770bd7501bd808f18483ba01e02fd Mon Sep 17 00:00:00 2001 From: Monika Zielinska Date: Thu, 9 Aug 2018 13:08:33 +0200 Subject: [PATCH] add error logs to bus initializing and policy parsing Added dlog error logs to dbuspolicy1_init and policy parsing for easier debugging. Changed behavior when parsing: exceptions thrown while parsing main system.conf/session.conf files cause interruption of parsing. Exceptions from parsing other .conf files are logged, but parsing is continued. Change-Id: I615d1a49fb552518d0040df1d875ba20ca6839f7 --- Makefile.am | 20 +++++++++------ configure.ac | 4 +++ packaging/libdbuspolicy.spec | 1 + src/dbuspolicy1/libdbuspolicy1.h | 6 +++++ src/internal/tslog.cpp | 9 +++++++ src/internal/tslog.hpp | 2 ++ src/internal/xml_parser.cpp | 54 ++++++++++++++++++++++++---------------- src/internal/xml_parser.hpp | 3 +++ src/libdbuspolicy1.c | 19 ++++++++------ 9 files changed, 83 insertions(+), 35 deletions(-) diff --git a/Makefile.am b/Makefile.am index 12b53f3..e7b63a1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -9,12 +9,14 @@ AM_CPPFLAGS = \ -include $(top_builddir)/config.h \ -DSYSCONFDIR=\""$(sysconfdir)"\" \ $(CYNARA_CFLAGS) \ + $(DLOG_CFLAGS) \ -I${top_srcdir}/src AM_CFLAGS = ${my_CFLAGS} \ -fvisibility=hidden \ -ffunction-sections \ $(CYNARA_CFLAGS) \ + $(DLOG_CFLAGS) \ -fdata-sections AM_LDFLAGS = \ @@ -23,6 +25,8 @@ AM_LDFLAGS = \ -Wl,-z,nodelete \ $(CYNARA_CFLAGS) \ $(CYNARA_LIBS)\ + $(DLOG_CFLAGS) \ + $(DLOG_LIBS) \ -pthread \ -lexpat @@ -66,6 +70,7 @@ EXTRA_DIST += src/libdbuspolicy1.sym src_libdbuspolicy1_la_LDFLAGS = $(AM_LDFLAGS) \ -version-info $(LIBDBUSPOLICY1_CURRENT):$(LIBDBUSPOLICY1_REVISION):$(LIBDBUSPOLICY1_AGE) \ $(CYNARA_LIBS) \ + $(DLOG_LIBS) \ -Wl,--version-script=$(top_srcdir)/src/libdbuspolicy1.sym src_libdbuspolicy1_la_DEPENDENCIES = ${top_srcdir}/src/libdbuspolicy1.sym @@ -94,8 +99,9 @@ src_libinternal_a_SOURCES =\ src/internal/cynara_mockup.cpp \ src/internal/groups_mockup.cpp -TESTS_LDADD = $(CYNARA_LIBS) \ - src/libinternal.a \ +TESTS_LDADD = src/libinternal.a \ + $(CYNARA_LIBS) \ + $(DLOG_LIBS) \ -lexpat src_test_libdbuspolicy1_ownership_LDADD = $(TESTS_LDADD) @@ -125,11 +131,11 @@ stest_signal_SOURCES = src/stest_signal.c src/stest_common.c stest_cynara_SOURCES = src/stest_cynara.c src/stest_common.c stest_memory_SOURCES = src/stest_memory.c -stest_ownership_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) -stest_method_call_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) -stest_signal_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) -stest_cynara_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) -stest_memory_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) +stest_ownership_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) $(DLOG_LIBS) +stest_method_call_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) $(DLOG_LIBS) +stest_signal_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) $(DLOG_LIBS) +stest_cynara_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) $(DLOG_LIBS) +stest_memory_LDADD = src/libinternalfortests.a -lexpat -lstdc++ $(CYNARA_LIBS) $(DLOG_LIBS) all-tests:: $(alonetest_PROGRAMS) $(runner_PROGRAMS) endif diff --git a/configure.ac b/configure.ac index 6d6dbf4..be90ee8 100644 --- a/configure.ac +++ b/configure.ac @@ -91,10 +91,14 @@ AC_SUBST([my_CXXFLAGS]) PKG_CHECK_MODULES([CYNARA], [cynara-client >= 0.4.2 cynara-session >= 0.4.2], [AC_DEFINE([ENABLE_CYNARA], [1], [Define to enable Cynara privilege checks in libdbuspolicy])], [AC_MSG_ERROR([libcynara-client-async and cynara-session are required to enable Cynara integration])]) +PKG_CHECK_MODULES([DLOG], [dlog]) AC_SUBST([CYNARA_CFLAGS]) AC_SUBST([CYNARA_LIBS]) +AC_SUBST([DLOG_CFLAGS]) +AC_SUBST([DLOG_LIBS]) + AC_CONFIG_HEADERS(config.h) AC_CONFIG_FILES([ Makefile diff --git a/packaging/libdbuspolicy.spec b/packaging/libdbuspolicy.spec index 18ebe04..ad34f6d 100644 --- a/packaging/libdbuspolicy.spec +++ b/packaging/libdbuspolicy.spec @@ -9,6 +9,7 @@ Source1001: %{name}.manifest BuildRequires: boost-devel BuildRequires: libexpat-devel BuildRequires: pkgconfig(cynara-client) +BuildRequires: pkgconfig(dlog) %package devel Summary: Helper library for fine-grained userspace policy handling-development package diff --git a/src/dbuspolicy1/libdbuspolicy1.h b/src/dbuspolicy1/libdbuspolicy1.h index d923a94..ff7f6a8 100644 --- a/src/dbuspolicy1/libdbuspolicy1.h +++ b/src/dbuspolicy1/libdbuspolicy1.h @@ -25,6 +25,12 @@ extern "C" { #endif +#define LOG_TAG "LIBDBUSPOLICY" +#include + +#define SYSTEM_BUS 0 +#define SESSION_BUS 1 + #define SYSTEM_BUS_CONF_FILE_PRIMARY "/usr/share/dbus-1/system.conf" #define SESSION_BUS_CONF_FILE_PRIMARY "/usr/share/dbus-1/session.conf" diff --git a/src/internal/tslog.cpp b/src/internal/tslog.cpp index cd6d18e..38d62b5 100644 --- a/src/internal/tslog.cpp +++ b/src/internal/tslog.cpp @@ -4,6 +4,7 @@ */ #include "tslog.hpp" +#include "dbuspolicy1/libdbuspolicy1.h" namespace tslog { int8_t g_verbosity; @@ -20,3 +21,11 @@ void tslog::init() { bool tslog::enabled() { return g_verbosity >= 0; } bool tslog::verbose() { return g_verbosity > 0; } + +void tslog::logError(const char *error) +{ + if (tslog::enabled()) { + std::cout << error << std::endl; + } + LOGE("%s", error); +} \ No newline at end of file diff --git a/src/internal/tslog.hpp b/src/internal/tslog.hpp index 62aea11..f41c417 100644 --- a/src/internal/tslog.hpp +++ b/src/internal/tslog.hpp @@ -41,6 +41,8 @@ namespace tslog /** Checks if verbosity is enabled */ bool verbose(); + + void logError(const char *error); } #endif diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index ddb360b..914da82 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -13,6 +13,7 @@ #include #include #include "tslog.hpp" +#include "dbuspolicy1/libdbuspolicy1.h" using namespace ldp_xml_parser; @@ -32,12 +33,6 @@ std::string getDir(std::string path) { return ret; } -void logError(const char* error) { - if (tslog::enabled()) { - std::cout << "Error parsing xml: " << error << std::endl; - } -} - class XmlParser::IncludeItem { public: std::string filename; @@ -58,7 +53,7 @@ void start_element_handler(void *data, const char *el, const char **attr) { try { parser.elementStart(el, attr); } catch (...) { - logError("Exception occurred while processing start element."); + tslog::logError("Exception occurred while processing start element."); parser.setRetCode(-1); } } @@ -69,7 +64,7 @@ void end_element_handler(void *data, const char *el) { try { parser.elementEnd(el); } catch (...) { - logError("Exception occurred while processing end element."); + tslog::logError("Exception occurred while processing end element."); parser.setRetCode(-1); } } @@ -80,11 +75,21 @@ void text_handler(void *data, const char *text, int len) { try { parser.text(text, len); } catch (...) { - logError("Exception occurred while processing element text."); + tslog::logError("Exception occurred while processing element text."); parser.setRetCode(-1); } } +bool XmlParser::isMainConfFile(const std::string& filename) { + switch ((int)curr_bus) { + case SYSTEM_BUS: + return (filename == SYSTEM_BUS_CONF_FILE_PRIMARY || filename == SYSTEM_BUS_CONF_FILE_SECONDARY); + case SESSION_BUS: + return (filename == SESSION_BUS_CONF_FILE_PRIMARY || filename == SESSION_BUS_CONF_FILE_SECONDARY); + } + return false; +} + void XmlParser::setRetCode(int ret) { ret_code = ret; } @@ -152,7 +157,7 @@ void XmlParser::elementEnd(const char *el) { if (item.ignore_missing) { return; } - logError(std::string("Missing required policy file: ").append(item.filename).c_str()); + tslog::logError(std::string("Missing required policy file: ").append(item.filename).c_str()); ret_code = -1; return; } @@ -177,22 +182,29 @@ int XmlParser::parsePolicy(bool bus, const std::string& fname) { std::cout << "XmlParser::parsePolicy called with filename: " << fname << std::endl; } curr_bus = bus; - ret_code = 0; + parsePolicyInternal(fname); + return ret_code; +} +void XmlParser::parsePolicyInternal(const std::string& filename) { + ret_code = 0; try { - parsePolicyInternal(fname); + parseXmlFile(filename); } catch (const std::runtime_error& ex) { - logError(std::string("Exception occured while parsing: ").append(ex.what()).c_str()); - ret_code = -1; + tslog::logError(std::string("Error parsing xml: Exception occured: ").append(ex.what()).c_str()); + if (isMainConfFile(filename)) { + tslog::logError("Exiting parsing"); + ret_code = -1; + return; + } } catch (...) { - logError("Exception occurred while parsing."); - ret_code = -1; + tslog::logError("Error parsing xml: Exception occured: "); + if (isMainConfFile(filename)) { + tslog::logError("Exiting parsing"); + ret_code = -1; + return; + } } - return ret_code; -} - -void XmlParser::parsePolicyInternal(const std::string& filename) { - parseXmlFile(filename); std::vector curr_included_files = included_files; // deep copy for (const auto& included_file : curr_included_files) { parsePolicyInternal(included_file); diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp index 87789e0..3cf7899 100755 --- a/src/internal/xml_parser.hpp +++ b/src/internal/xml_parser.hpp @@ -52,6 +52,9 @@ namespace ldp_xml_parser /** Adapter which allows to access parsed policies */ DbAdapter __adapter; + /** Decides whether a filename describes one of main system.conf/session.conf files */ + bool isMainConfFile(const std::string& filename); + /** Parses config file and all files included in it (recursively) */ void parsePolicyInternal(const std::string& filename); diff --git a/src/libdbuspolicy1.c b/src/libdbuspolicy1.c index 957636f..539179b 100755 --- a/src/libdbuspolicy1.c +++ b/src/libdbuspolicy1.c @@ -41,9 +41,6 @@ #define KDBUS_SYSTEM_BUS_PATH "/sys/fs/kdbus/0-system/bus" #define KDBUS_POOL_SIZE (1024UL * 1024UL) -#define SYSTEM_BUS 0 -#define SESSION_BUS 1 - #define CONNECTION_LABEL "libdbuspolicy1-kdbus" #define ALIGN8(l) (((l) + 7) & ~7) @@ -233,8 +230,10 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) _Static_assert(SYSTEM_BUS == 0, "SYSTEM_BUS not 0"); _Static_assert(SESSION_BUS == 1, "SESSION_BUS not 1"); - if (bus_path_resolve(bus_path, resolved_path, sizeof(resolved_path), &bus_type, &bus_owner) < 0) + if (bus_path_resolve(bus_path, resolved_path, sizeof(resolved_path), &bus_type, &bus_owner) < 0) { + LOGE("Error resolving bus ath: %s", bus_path); return NULL; + } if (bus_type) bus_type = SESSION_BUS; @@ -256,14 +255,20 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init(const char *bus_path) __internal_init_sup_group(bus_type, g_udesc.uid, g_udesc.gid); - if (__internal_can_open(bus_type, bus_owner, g_udesc.uid, g_udesc.gid, g_udesc.label) <= 0) + if (__internal_can_open(bus_type, bus_owner, g_udesc.uid, g_udesc.gid, g_udesc.label) <= 0) { + LOGE("Error: %lu isn't allowed", g_udesc.uid); goto err; + } - if ((g_conn[bus_type].fd = kdbus_open_bus(resolved_path)) < 0) + if ((g_conn[bus_type].fd = kdbus_open_bus(resolved_path)) < 0) { + LOGE("Error opening bus: %s", resolved_path); goto err; + } - if (kdbus_hello(bus_type, 0, _KDBUS_ATTACH_ALL, 0) < 0) + if (kdbus_hello(bus_type, 0, _KDBUS_ATTACH_ALL, 0) < 0) { + LOGE("Error: bus hello failed"); goto err_close; + } init_once[bus_type] = true; } -- 2.7.4