From 460682009933a36e77e2cd9887ecd3866db3eaad Mon Sep 17 00:00:00 2001 From: Aleksy Barcz Date: Fri, 18 May 2018 14:35:32 +0200 Subject: [PATCH] Use expat SAX parser instead of boost::ptree (DOM) Modified xml parsing methods to use expat parser, which is much faster than boost::property_tree (initialization time decreased by about 30%). Added missing includes after the removal of ptree. Change-Id: I8973fe7ad7bbfe7928ef261fe07305f54e640b76 --- Makefile.am | 14 ++- packaging/libdbuspolicy.spec | 1 + src/internal/internal.cpp | 6 +- src/internal/internal.h | 3 + src/internal/naive_policy_db.cpp | 2 + src/internal/policy.cpp | 95 +++++++-------- src/internal/policy.hpp | 15 ++- src/internal/xml_parser.cpp | 244 +++++++++++++++++++++++++++++---------- src/internal/xml_parser.hpp | 31 ++++- 9 files changed, 277 insertions(+), 134 deletions(-) diff --git a/Makefile.am b/Makefile.am index c270a44..b2e5f02 100644 --- a/Makefile.am +++ b/Makefile.am @@ -23,7 +23,8 @@ AM_LDFLAGS = \ -Wl,-z,nodelete \ $(CYNARA_CFLAGS) \ $(CYNARA_LIBS)\ - -pthread + -pthread \ + -lexpat SED_PROCESS = \ $(AM_V_GEN)$(MKDIR_P) $(dir $@) && $(SED) \ @@ -94,7 +95,8 @@ src_libinternal_a_SOURCES =\ src/internal/groups_mockup.cpp TESTS_LDADD = $(CYNARA_LIBS) \ - src/libinternal.a + src/libinternal.a \ + -lexpat src_test_libdbuspolicy1_ownership_LDADD = $(TESTS_LDADD) src_test_libdbuspolicy1_ownership_deny_LDADD = $(TESTS_LDADD) @@ -122,10 +124,10 @@ stest_method_call_SOURCES = src/stest_method_call.c src/stest_common.c stest_signal_SOURCES = src/stest_signal.c src/stest_common.c stest_cynara_SOURCES = src/stest_cynara.c src/stest_common.c -stest_ownership_LDADD = src/libinternalfortests.a -lstdc++ $(CYNARA_LIBS) -stest_method_call_LDADD = src/libinternalfortests.a -lstdc++ $(CYNARA_LIBS) -stest_signal_LDADD = src/libinternalfortests.a -lstdc++ $(CYNARA_LIBS) -stest_cynara_LDADD = src/libinternalfortests.a -lstdc++ $(CYNARA_LIBS) +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) all-tests:: $(alonetest_PROGRAMS) $(runner_PROGRAMS) endif diff --git a/packaging/libdbuspolicy.spec b/packaging/libdbuspolicy.spec index 725c1b8..18ebe04 100644 --- a/packaging/libdbuspolicy.spec +++ b/packaging/libdbuspolicy.spec @@ -7,6 +7,7 @@ Release: 0 Source: %{name}-%{version}.tar.gz Source1001: %{name}.manifest BuildRequires: boost-devel +BuildRequires: libexpat-devel BuildRequires: pkgconfig(cynara-client) %package devel diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp index 4d4bf42..c7fd165 100755 --- a/src/internal/internal.cpp +++ b/src/internal/internal.cpp @@ -34,8 +34,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); - ldp_xml_parser::XmlParser p; - auto err = p.parsePolicy(bus_type, get_str(config_name)); + auto err = ldp_xml_parser::static_parser.parsePolicy(bus_type, get_str(config_name)); return err; } @@ -57,8 +56,7 @@ void __internal_init_flush_logs() void __internal_init_sup_group(bool bus_type, uid_t uid, gid_t gid) { - ldp_xml_parser::XmlParser p; - p.updateGroupPolicy(bus_type, uid, gid); + ldp_xml_parser::static_parser.updateGroupPolicy(bus_type, uid, gid); } void __internal_enter() diff --git a/src/internal/internal.h b/src/internal/internal.h index 00db97b..9811a2c 100755 --- a/src/internal/internal.h +++ b/src/internal/internal.h @@ -30,6 +30,9 @@ extern "C" { #endif +#include +#include + #define KDBUS_CONN_MAX_NAMES 256 /** Initializes policies from given policy configuration file name diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp index e9f413b..680ebe2 100755 --- a/src/internal/naive_policy_db.cpp +++ b/src/internal/naive_policy_db.cpp @@ -2,7 +2,9 @@ #include "cynara.hpp" #include "groups_proxy.hpp" #include "tslog.hpp" +#include #include +#include /** * \file diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp index 04a7d64..99aaece 100755 --- a/src/internal/policy.cpp +++ b/src/internal/policy.cpp @@ -12,9 +12,10 @@ #include #include #include +#include +#include using namespace ldp_xml_parser; -using boost::property_tree::ptree; static const char* message_type[] = { "ANY", "METHOD_CALL", "METHOD_RETURN", "ERROR", "SIGNAL"}; static const char* message_dir[] = { "ANY", "SEND", "RECEIVE"}; @@ -89,8 +90,8 @@ static gid_t convertToGid(const char* group) { return gg->gr_gid; } -static bool field_has(const ptree::value_type& v, const std::string& substr) { - return (v.first.find(substr) != std::string::npos); +static bool field_has(const std::string& str, const std::string& substr) { + return (str.find(substr) != std::string::npos); } static const std::map str2decision{ @@ -98,40 +99,24 @@ static const std::map str2decision{ {"deny", Decision::DENY}, {"check", Decision::CHECK}}; -void DbAdapter::parsePolicy(bool bus, const ptree& pt) +void DbAdapter::parsePolicy(bool bus, const char **attr) { - PolicyType policy_type = PolicyType::NONE; - PolicyTypeValue policy_type_value; - - // we must read policy attrs before parsing rules, as ptree API - // doesn't guarantee that will occur before other - // child elements - const auto xmlattr = pt.get_child_optional(""); - if (xmlattr) { - for (const auto& va : *xmlattr) { - parsePolicyAttribute(va, policy_type, policy_type_value); - } - } - if (policy_type == PolicyType::NONE) { - return; - } - for (const auto& v : pt) { - if (str2decision.find(v.first) != str2decision.end()) { - parseRule(v); - __builder.generateItem(policy_checker().db(bus), policy_type, policy_type_value); - } + policy_type = PolicyType::NONE; + curr_bus = bus; + + for (int i = 0; attr[i]; i += 2) { + parsePolicyAttribute(attr[i], attr[i + 1], policy_type, policy_type_value); } } -void DbAdapter::parsePolicyAttribute(const ptree::value_type& v, PolicyType& policy_type, PolicyTypeValue& policy_type_value) +void DbAdapter::parsePolicyAttribute(const char* name, const char* value, PolicyType& policy_type, PolicyTypeValue& policy_type_value) { //possible values (from dbus-daemon man): // context="(default|mandatory)" // at_console="(true|false)" (deprecated and not used on targets, we ignore it) // user="username or userid" // group="group name or gid" - const char* value = v.second.data().c_str(); - if (v.first == "context") { + if (std::strcmp(name, "context") == 0) { if (std::strcmp(value, "mandatory") == 0 ) { policy_type = PolicyType::CONTEXT; policy_type_value.context = ContextType::MANDATORY; @@ -139,63 +124,67 @@ void DbAdapter::parsePolicyAttribute(const ptree::value_type& v, PolicyType& pol policy_type = PolicyType::CONTEXT; policy_type_value.context = ContextType::DEFAULT; } - } else if (v.first == "user") { + } else if (std::strcmp(name, "user") == 0) { policy_type = PolicyType::USER; policy_type_value.user = convertToUid(value); - } else if (v.first == "group") { + } else if (std::strcmp(name, "group") == 0) { policy_type = PolicyType::GROUP; policy_type_value.group = convertToGid(value); } } -void DbAdapter::parseRule(const ptree::value_type& v) +void DbAdapter::parseRule(const char *name, const char **attr) { + if (policy_type == PolicyType::NONE) { + return; + } + if (str2decision.find(std::string(name)) == str2decision.end()) { + return; + } __builder.reset(); - __builder.addDecision(str2decision.at(v.first)); + __builder.addDecision(str2decision.at(std::string(name))); - const auto xmlattr = v.second.get_child_optional(""); - if (xmlattr) { - for (const auto& va : *xmlattr) { - parseRuleAttribute(va); - } + for (int i = 0; attr[i]; i += 2) { + parseRuleAttribute(attr[i], attr[i + 1]); } + + __builder.generateItem(policy_checker().db(curr_bus), policy_type, policy_type_value); } -void DbAdapter::parseRuleAttribute(const ptree::value_type& v) +void DbAdapter::parseRuleAttribute(const char *name, const char *value) { - const char* value = nullptr; - if (v.second.data() != "*") - value = v.second.data().c_str(); + if (std::strcmp(value, "*") == 0) + value = nullptr; - if (field_has(v, "send_")) { + if (field_has(name, "send_")) { __builder.addDirection(MessageDirection::SEND); - } else if (field_has(v, "receive_")) { + } else if (field_has(name, "receive_")) { __builder.addDirection(MessageDirection::RECEIVE); - } else if (v.first == "own") { + } else if (std::strcmp(name, "own") == 0) { __builder.addOwner(value); __builder.setPrefix(false); - } else if (v.first == "own_prefix") { + } else if (std::strcmp(name, "own_prefix") == 0) { __builder.addOwner(value); __builder.setPrefix(true); - } else if (v.first == "privilege") { + } else if (std::strcmp(name, "privilege") == 0) { __builder.addPrivilege(value); - } else if (v.first == "user") { + } else if (std::strcmp(name, "user") == 0) { __builder.addUserAccess(value); - } else if (v.first == "group") { + } else if (std::strcmp(name, "group") == 0) { __builder.addGroupAccess(value); } - if (field_has(v, "_destination")) + if (field_has(name, "_destination")) __builder.addName(value); - else if (field_has(v, "_sender")) + else if (field_has(name, "_sender")) __builder.addName(value); - else if (field_has(v, "_path")) + else if (field_has(name, "_path")) __builder.addPath(value); - else if (field_has(v, "_interface")) + else if (field_has(name, "_interface")) __builder.addInterface(value); - else if (field_has(v, "_member")) + else if (field_has(name, "_member")) __builder.addMember(value); - else if (field_has(v, "_type")) + else if (field_has(name, "_type")) __builder.addMessageType(__str_to_message_type(value)); } diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp index 67af749..34239e6 100755 --- a/src/internal/policy.hpp +++ b/src/internal/policy.hpp @@ -23,10 +23,9 @@ #ifndef _POLICY_H #define _POLICY_H -#include -#include #include #include +#include #define MAX_LOG_LINE 1024 /** Maximum tree node children. It is connected with proper characters which can be used in name.*/ @@ -261,12 +260,16 @@ namespace ldp_xml_parser class DbAdapter { private: ItemBuilder __builder; - void parsePolicyAttribute(const boost::property_tree::ptree::value_type& v, PolicyType&, PolicyTypeValue&); - void parseRule(const boost::property_tree::ptree::value_type& v); - void parseRuleAttribute(const boost::property_tree::ptree::value_type& v); + PolicyType policy_type; + PolicyTypeValue policy_type_value; + bool curr_bus; + + void parsePolicyAttribute(const char* name, const char* value, PolicyType& policy_type, PolicyTypeValue& policy_type_value); + void parseRuleAttribute(const char *name, const char *value); public: - void parsePolicy(bool bus, const boost::property_tree::ptree& pt); + void parsePolicy(bool bus, const char **attr); + void parseRule(const char *name, const char **attr); void updateGroupDb(bool bus, uid_t uid, gid_t gid); }; } diff --git a/src/internal/xml_parser.cpp b/src/internal/xml_parser.cpp index febe723..60aa5ad 100644 --- a/src/internal/xml_parser.cpp +++ b/src/internal/xml_parser.cpp @@ -1,20 +1,20 @@ /** + return a; * \file * \ingroup Implementation */ #include "xml_parser.hpp" #include -#include -#include #include #include #include #include +#include +#include #include "tslog.hpp" using namespace ldp_xml_parser; -using boost::property_tree::ptree; std::string expandPath(const std::string& parent_dir, const std::string& path) { if (path[0] == '/') { @@ -45,92 +45,212 @@ public: bool ignore_always{false}; }; -int XmlParser::parsePolicy(bool bus, std::string const &fname) { +void parseAssert(bool condition) { + assert(condition); + if (!condition) { + throw std::runtime_error("Parsing error"); + } +} + +void start_element_handler(void *data, const char *el, const char **attr) { + (void)data; + XmlParser& parser = static_parser; + try { + parser.elementStart(el, attr); + } catch (...) { + logError("Exception occurred while processing start element."); + parser.setRetCode(-1); + } +} + +void end_element_handler(void *data, const char *el) { + (void)data; + XmlParser& parser = static_parser; + try { + parser.elementEnd(el); + } catch (...) { + logError("Exception occurred while processing end element."); + parser.setRetCode(-1); + } +} + +void text_handler(void *data, const char *text, int len) { + (void)data; + XmlParser& parser = static_parser; + try { + parser.text(text, len); + } catch (...) { + logError("Exception occurred while processing element text."); + parser.setRetCode(-1); + } +} + +void XmlParser::setRetCode(int ret) { + ret_code = ret; +} + +void XmlParser::text(const char *text, int len) { + current_text.append(text, len); +} + +void XmlParser::elementStart(const char *el, const char **attr) { + current_text = std::string(); + + if (std::strcmp(el, "busconfig") == 0) { + parseAssert(state == IDLE); + state = BUSCONFIG; + } else if (std::strcmp(el, "policy") == 0) { + parseAssert(state == BUSCONFIG); + state = POLICY; + __adapter.parsePolicy(curr_bus, attr); + } else if (std::strcmp(el, "includedir") == 0) { + parseAssert(state == BUSCONFIG); + state = INCLUDEDIR; + } else if (std::strcmp(el, "include") == 0) { + parseAssert(state == BUSCONFIG); + state = INCLUDE; + ignore_missing = false; + ignore_always = false; + for (int i = 0; attr[i]; i += 2) { + if ((std::strcmp(attr[i], "ignore_missing") == 0) && (std::strcmp(attr[i + 1], "yes") == 0)) { + ignore_missing = true; + } else if ((std::strcmp(attr[i], "if_selinux_enabled") == 0) && (std::strcmp(attr[i + 1], "yes") == 0)) { + ignore_always = true; + } + } + } else if (std::strcmp(el, "allow") == 0) { + parseAssert(state == POLICY); + state = RULE; + __adapter.parseRule(el, attr); + } else if (std::strcmp(el, "deny") == 0) { + parseAssert(state == POLICY); + state = RULE; + __adapter.parseRule(el, attr); + } else if (std::strcmp(el, "check") == 0) { + parseAssert(state == POLICY); + state = RULE; + __adapter.parseRule(el, attr); + } +} + +void XmlParser::elementEnd(const char *el) { + if (std::strcmp(el, "busconfig") == 0) { + parseAssert(state == BUSCONFIG); + state = IDLE; + } else if (std::strcmp(el, "includedir") == 0) { + parseAssert(state == INCLUDEDIR); + state = BUSCONFIG; + getIncludedFiles(curr_dir, current_text, included_files); + } else if (std::strcmp(el, "include") == 0) { + parseAssert(state == INCLUDE); + state = BUSCONFIG; + IncludeItem item = parseIncludeItem(); + if (item.ignore_always) { + return; + } + if (access(item.filename.c_str(), F_OK) != 0) { + if (item.ignore_missing) { + return; + } + logError(std::string("Missing required policy file: ").append(item.filename).c_str()); + ret_code = -1; + return; + } + included_files.push_back(item.filename); + } else if (std::strcmp(el, "policy") == 0) { + parseAssert(state == POLICY); + state = BUSCONFIG; + } else if (std::strcmp(el, "allow") == 0) { + parseAssert(state == RULE); + state = POLICY; + } else if (std::strcmp(el, "deny") == 0) { + parseAssert(state == RULE); + state = POLICY; + } else if (std::strcmp(el, "check") == 0) { + parseAssert(state == RULE); + state = POLICY; + } +} + +int XmlParser::parsePolicy(bool bus, const std::string& fname) { if (tslog::enabled()) { std::cout << "XmlParser::parsePolicy called with filename: " << fname << std::endl; } + curr_bus = bus; + ret_code = 0; try { - parsePolicyInternal(bus, fname); - } catch (const boost::property_tree::xml_parser::xml_parser_error& ex) { - logError(ex.what()); - return -1; - } catch (const boost::property_tree::ptree_error& ex) { - logError(ex.what()); - return -1; + parsePolicyInternal(fname); } catch (const std::runtime_error& ex) { - logError(ex.what()); - return -1; + logError(std::string("Exception occured while parsing: ").append(ex.what()).c_str()); + ret_code = -1; } catch (...) { - logError("unknown error"); - return -1; + logError("Exception occurred while parsing."); + ret_code = -1; } - return 0; + return ret_code; } -void XmlParser::parsePolicyInternal(bool bus, std::string const &filename) { - auto included_files = parseXmlFile(bus, filename); - for (const auto& included_file : included_files) { - parsePolicyInternal(bus, included_file); +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); } } -std::vector XmlParser::parseXmlFile(bool bus, const std::string& filename) { - std::vector included_files; +std::unique_ptr file2str(const std::string& filename) { + + FILE *fp = fopen(filename.c_str(), "rb"); + if (fp == nullptr) { + throw std::runtime_error(std::string("Failed to open file: ").append(filename).c_str()); + } + + fseek(fp, 0, SEEK_END); + long fsize = ftell(fp); + fseek(fp, 0, SEEK_SET); //rewind + + auto str = std::unique_ptr(new char[fsize + 1]); // TODO change to make_unique when c++14 becomes available + const int count = 1; + size_t res = fread(str.get(), fsize, count, fp); + parseAssert(res == count); + fclose(fp); + str.get()[fsize] = 0; // TODO change to operator[] when c++17 becomes available + + return str; +} + +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'; } - const std::string curr_dir = getDir(filename); - - boost::property_tree::ptree pt; - read_xml(filename, pt); - if (!pt.empty()) { - const auto& busconfig = pt.get_child("busconfig"); - for (const auto& x : busconfig) { - if (x.first == "policy") { - __adapter.parsePolicy(bus, x.second); - } else if (x.first == "include") { - IncludeItem item = parseIncludeItem(x.second, curr_dir); - if (item.ignore_always) { - continue; - } - if (access(item.filename.c_str(), F_OK) != 0) { - if (item.ignore_missing) { - continue; - } - throw std::runtime_error("Missing required policy file: " + item.filename); - } - included_files.push_back(item.filename); - } else if (x.first == "includedir") { - getIncludedFiles(curr_dir, x.second.data(), included_files); - } - } + curr_dir = getDir(filename); + + auto parser = std::unique_ptr>(XML_ParserCreate(nullptr), + [](XML_Parser p) { XML_ParserFree(p); }); + XML_SetElementHandler(parser.get(), start_element_handler, end_element_handler); + XML_SetCharacterDataHandler(parser.get(), text_handler); + + auto str = file2str(filename); + if (XML_Parse(parser.get(), str.get(), strlen(str.get()), XML_TRUE) == XML_STATUS_ERROR) { + 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"; } - - return included_files; } -XmlParser::IncludeItem XmlParser::parseIncludeItem(const ptree& pt, const std::string& parent_dir) { +XmlParser::IncludeItem XmlParser::parseIncludeItem() { IncludeItem item; - item.filename = expandPath(parent_dir, pt.data()); - const auto xmlattr = pt.get_child_optional(""); - if (xmlattr) { - for (const auto& va : *xmlattr) { - if (va.first == "ignore_missing" && va.second.data() == "yes") { - item.ignore_missing = true; - } else if (va.first == "if_selinux_enabled" && va.second.data() == "yes") { - item.ignore_always = true; - } - } - } + item.filename = expandPath(curr_dir, current_text); + item.ignore_missing = ignore_missing; + item.ignore_always = ignore_always; return item; } @@ -157,3 +277,5 @@ void XmlParser::getIncludedFiles(const std::string& parent_dir, const std::strin std::cout << "could not open directory " << dname << '\n'; } } + +ldp_xml_parser::XmlParser ldp_xml_parser::static_parser; diff --git a/src/internal/xml_parser.hpp b/src/internal/xml_parser.hpp index 9f37674..87789e0 100755 --- a/src/internal/xml_parser.hpp +++ b/src/internal/xml_parser.hpp @@ -32,12 +32,20 @@ namespace ldp_xml_parser { public: /** Parses given config file for declared bus type */ - int parsePolicy(bool bus, std::string const &fname); + int parsePolicy(bool bus, const std::string& fname); void updateGroupPolicy(bool bus, uid_t uid, gid_t gid) { __adapter.updateGroupDb(bus, uid, gid); } + void elementStart(const char *el, const char **attr); + + void elementEnd(const char *el); + + void text(const char *text, int len); + + void setRetCode(int ret); + private: class IncludeItem; @@ -45,17 +53,32 @@ namespace ldp_xml_parser DbAdapter __adapter; /** Parses config file and all files included in it (recursively) */ - void parsePolicyInternal(bool bus, std::string const &filename); + void parsePolicyInternal(const std::string& filename); /** Parses config file and returns all files included in it */ - std::vector parseXmlFile(bool bus, const std::string& filename); + void parseXmlFile(const std::string& filename); /** Parses element */ - IncludeItem parseIncludeItem(const boost::property_tree::ptree& pt, const std::string& parent_dir); + IncludeItem parseIncludeItem(); /** Get all the .conf files within included subdirectory, POSIX style as boost::filesystem is not header-only */ void getIncludedFiles(const std::string& parent_dir, const std::string& incldir, std::vector& files); + + typedef enum { IDLE, BUSCONFIG, INCLUDE, INCLUDEDIR, POLICY, RULE } ParseState; + + ParseState state; + + bool ignore_always; + bool ignore_missing; + bool curr_bus; + std::string current_text; + std::string curr_dir; + int ret_code; + + std::vector included_files; }; + + extern XmlParser static_parser; } //namespace #endif -- 2.7.4