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

diff --git a/Makefile.am b/Makefile.am
index 12b53f3..e7b63a1 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- a/src/dbuspolicy1/libdbuspolicy1.h
+++ b/src/dbuspolicy1/libdbuspolicy1.h
@@ -25,6 +25,12 @@
 extern "C" {
 #endif
 
+#define LOG_TAG "LIBDBUSPOLICY"
+#include <dlog.h>
+
+#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 (file)
--- 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 (file)
--- 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 (file)
--- a/src/internal/xml_parser.cpp
+++ b/src/internal/xml_parser.cpp
@@ -13,6 +13,7 @@
 #include <expat.h>
 #include <memory>
 #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<std::string> 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 (executable)
--- 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 (executable)
--- 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;
        }