refactored parsing policy

Changed unnecessarily recursive code into code that reflects the actual
configuration syntax, for better readability and maintainability.
Removed unused __tag_state and __attr variables.
Fixed potential future bug: ptree api doesn't guarantee that <xmlattr> will be
the first child, so we shouldn't assume it will be first.
Fixed bug, when policy at_console was parsed as the last seen policy type
(or as unitialized type, if it was the first policy read).
Added test case to assure that at_console policies will always be ignored.

Change-Id: I7e28262ee4837547e10ca1b33f7d5a90166f4667

diff --git a/src/internal/naive_policy_db.cpp b/src/internal/naive_policy_db.cpp
index be88fa0..2437105 100755 (executable)
--- a/src/internal/naive_policy_db.cpp
+++ b/src/internal/naive_policy_db.cpp
@@ -180,6 +180,9 @@ void NaivePolicyDb::addItem(NaivePolicyDb::PolicyTypeSetSR& set,
        case PolicyType::GROUP:
                set.group[policy_type_value.group].addItem(item);
                break;
+       case PolicyType::NONE:
+               assert(false);
+               break;
        }
 }
 

diff --git a/src/internal/policy.cpp b/src/internal/policy.cpp
index f3937df..f40407d 100755 (executable)
--- a/src/internal/policy.cpp
+++ b/src/internal/policy.cpp
@@ -10,9 +10,11 @@
 #include <cstdlib>
 #include <sys/types.h>
 #include <grp.h>
+#include <map>
 #include <pwd.h>
 
 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"};
@@ -46,9 +48,6 @@ static inline const char* __decision_to_str(Decision dec) {
        return message_decision[static_cast<std::size_t>(dec)];
 }
 
-DbAdapter::DbAdapter() : __attr(false), __tag_state(NONE) {
-}
-
 static uid_t convertToUid(const char* user) {
        long val = -1;
        errno = 0;
@@ -85,125 +84,123 @@ static gid_t convertToGid(const char* group) {
        return gg->gr_gid;
 }
 
-static bool field_has(const boost::property_tree::ptree::value_type& v, const std::string& substr) {
+static bool field_has(const ptree::value_type& v, const std::string& substr) {
        return (v.first.find(substr) != std::string::npos);
 }
 
-void DbAdapter::updateDecision(const boost::property_tree::ptree::value_type& v,
-                                                          PolicyType& policy_type,
-                                                          PolicyTypeValue& policy_type_value,
-                                                          state& t,
-                                                          bool& attr) {
-       const char* value = NULL;
-       if (v.first == "allow" && t == POLICY) {
-               __builder.reset();
-               __builder.addDecision(Decision::ALLOW);
-               t = ALLOW_DENY_CHECK;
-               attr = false;
-       } else if (v.first == "deny" && t == POLICY) {
-               __builder.reset();
-               __builder.addDecision(Decision::DENY);
-               t = ALLOW_DENY_CHECK;
-               attr = false;
-       } else if (v.first == "check" && t == POLICY) {
-               __builder.reset();
-               __builder.addDecision(Decision::CHECK);
-               t = ALLOW_DENY_CHECK;
-           attr = false;
-       } else if (v.first == "<xmlattr>") {
-               attr = true;
-       } else if (attr && t == POLICY) {
-               if (v.second.data() != "*")
-                       value = v.second.data().c_str();
-
-               if (value) {
-                       if (v.first == "context") {
-                               if (std::strcmp(value, "mandatory") == 0 ) {
-                                       policy_type = PolicyType::CONTEXT;
-                                       policy_type_value.context = ContextType::MANDATORY;
-                               } else if (std::strcmp(value, "default") == 0) {
-                                       policy_type = PolicyType::CONTEXT;
-                                       policy_type_value.context = ContextType::DEFAULT;
-                               }
-                       } else if (v.first == "user") {
-                               policy_type = PolicyType::USER;
-                               policy_type_value.user = convertToUid(value);
-                       } else if (v.first == "group") {
-                               policy_type = PolicyType::GROUP;
-                               policy_type_value.group = convertToGid(value);
-                       }
-               }
-       } else if (attr && t == ALLOW_DENY_CHECK) {
-               if (v.second.data() != "*")
-                       value = v.second.data().c_str();
-
-               if (field_has(v, "send_")) {
-                       __builder.addDirection(MessageDirection::SEND);
-               } else if (field_has(v, "receive_")) {
-                       __builder.addDirection(MessageDirection::RECEIVE);
-               } else if (v.first == "own") {
-                       __builder.addOwner(value);
-                       __builder.setPrefix(false);
-               } else if (v.first == "own_prefix") {
-                       __builder.addOwner(value);
-                       __builder.setPrefix(true);
-               } else if (v.first == "privilege") {
-                       __builder.addPrivilege(value);
+void DbAdapter::updateDb(bool bus, const ptree& pt, std::vector<std::string>& incl_dirs) {
+       const auto& busconfig = pt.get_child("busconfig");
+       for (const auto& x : busconfig) {
+               if (x.first == "policy") {
+                       parsePolicy(bus, x.second);
+               } else if (x.first == "includedir") {
+                       incl_dirs.push_back(x.second.data());
                }
-
-               if (field_has(v, "_destination"))
-                       __builder.addName(value);
-               else if (field_has(v, "_sender"))
-                       __builder.addName(value);
-               else if (field_has(v, "_path"))
-                       __builder.addPath(value);
-               else if (field_has(v, "_interface"))
-                       __builder.addInterface(value);
-               else if (field_has(v, "_member"))
-                       __builder.addMember(value);
-               else if (field_has(v, "_type"))
-                       __builder.addMessageType(__str_to_message_type(value));
-       } else {
-               attr = false;
-               t = NONE;
        }
 }
 
-void DbAdapter::xmlTraversal(bool bus,
-                                                        const boost::property_tree::ptree& pt,
-                                                        DbAdapter::state tag,
-                                                        PolicyType& policy_type,
-                                                        PolicyTypeValue& policy_type_value,
-                                                        bool attr,
-                                                        int level) {
-       static const int Q_XML_MAX_LEVEL = 10;
-       if (level < Q_XML_MAX_LEVEL) {
-               for(const auto& v : pt) {
-                       if (v.first == "<xmlcomment>") { continue; }
-                       state t = tag;
-                       updateDecision(v, policy_type, policy_type_value, t, attr);
-                       xmlTraversal(bus, v.second, t, policy_type, policy_type_value, attr, level + 1);
+static const std::map<std::string, Decision> str2decision{
+       {"allow", Decision::ALLOW},
+       {"deny", Decision::DENY},
+       {"check", Decision::CHECK}};
+
+void DbAdapter::parsePolicy(bool bus, const ptree& pt)
+{
+       PolicyType policy_type = PolicyType::NONE;
+       PolicyTypeValue policy_type_value;
+
+       // we must read policy attrs before parsing rules, as ptree API
+       // doesn't guarantee that <xmlattr> will occur before other
+       // child elements
+       const auto xmlattr = pt.get_child_optional("<xmlattr>");
+       if (xmlattr) {
+               for (const auto& va : *xmlattr) {
+                       parsePolicyAttribute(va, policy_type, policy_type_value);
                }
-               if (!pt.empty() && level > 1)
+       }
+       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);
+               }
        }
 }
 
-void DbAdapter::updateDb(bool bus, boost::property_tree::ptree& xmlTree, std::vector<std::string>& incl_dirs) {
-       const auto& children = xmlTree.get_child("busconfig");
-       PolicyType policy_type;
-       PolicyTypeValue policy_type_value;
-       for (const auto& x : children) {
-               if (x.first == "policy") {
-                       __tag_state = POLICY;
-                       __attr = false;
-                       xmlTraversal(bus, x.second, POLICY, policy_type, policy_type_value);
-               } else if (x.first == "includedir") {
-                       incl_dirs.push_back(x.second.data());
+void DbAdapter::parsePolicyAttribute(const ptree::value_type& v, 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(value, "mandatory") == 0 ) {
+                       policy_type = PolicyType::CONTEXT;
+                       policy_type_value.context = ContextType::MANDATORY;
+               } else if (std::strcmp(value, "default") == 0) {
+                       policy_type = PolicyType::CONTEXT;
+                       policy_type_value.context = ContextType::DEFAULT;
+               }
+       } else if (v.first == "user") {
+               policy_type = PolicyType::USER;
+               policy_type_value.user = convertToUid(value);
+       } else if (v.first == "group") {
+               policy_type = PolicyType::GROUP;
+               policy_type_value.group = convertToGid(value);
+       }
+}
+
+void DbAdapter::parseRule(const ptree::value_type& v)
+{
+       __builder.reset();
+       __builder.addDecision(str2decision.at(v.first));
+
+       const auto xmlattr = v.second.get_child_optional("<xmlattr>");
+       if (xmlattr) {
+               for (const auto& va : *xmlattr) {
+                       parseRuleAttribute(va);
                }
        }
 }
 
+void DbAdapter::parseRuleAttribute(const ptree::value_type& v)
+{
+       const char* value = nullptr;
+       if (v.second.data() != "*")
+               value = v.second.data().c_str();
+
+       if (field_has(v, "send_")) {
+               __builder.addDirection(MessageDirection::SEND);
+       } else if (field_has(v, "receive_")) {
+               __builder.addDirection(MessageDirection::RECEIVE);
+       } else if (v.first == "own") {
+               __builder.addOwner(value);
+               __builder.setPrefix(false);
+       } else if (v.first == "own_prefix") {
+               __builder.addOwner(value);
+               __builder.setPrefix(true);
+       } else if (v.first == "privilege") {
+               __builder.addPrivilege(value);
+       }
+
+       if (field_has(v, "_destination"))
+               __builder.addName(value);
+       else if (field_has(v, "_sender"))
+               __builder.addName(value);
+       else if (field_has(v, "_path"))
+               __builder.addPath(value);
+       else if (field_has(v, "_interface"))
+               __builder.addInterface(value);
+       else if (field_has(v, "_member"))
+               __builder.addMember(value);
+       else if (field_has(v, "_type"))
+               __builder.addMessageType(__str_to_message_type(value));
+}
+
 void DbAdapter::updateGroupDb(bool bus)
 {
        policy_checker().db(bus).updateSupGroup();

diff --git a/src/internal/policy.hpp b/src/internal/policy.hpp
index 2fb720e..428bb44 100755 (executable)
--- a/src/internal/policy.hpp
+++ b/src/internal/policy.hpp
@@ -60,6 +60,7 @@ namespace ldp_xml_parser
 
        /** Policy type, either context, user or group */
        enum class PolicyType : uint8_t {
+               NONE,
                CONTEXT,
                USER,
                GROUP
@@ -217,29 +218,14 @@ namespace ldp_xml_parser
        /** Adapter which allows to access policy db. Contains references to system and session db. */
        class DbAdapter {
        private:
-               enum state {
-                       NONE,
-                       POLICY,
-                       ALLOW_DENY_CHECK
-               };
-               bool __attr;
-               state __tag_state;
                ItemBuilder __builder;
-               void updateDecision(const boost::property_tree::ptree::value_type& v,
-                                                       PolicyType& policy_type,
-                                                       PolicyTypeValue& policy_type_value,
-                                                       state& tag,
-                                                       bool& attr);
-               void xmlTraversal(bool bus,
-                                                 const boost::property_tree::ptree& pt,
-                                                 state tag,
-                                                 PolicyType& policy_type,
-                                                 PolicyTypeValue& policy_type_value,
-                                                 bool attr = false,
-                                                 int level = 0);
+               void parsePolicy(bool bus, const boost::property_tree::ptree& pt);
+               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);
+
        public:
-               DbAdapter();
-               void updateDb(bool bus, boost::property_tree::ptree& xmlTree, std::vector<std::string>& incl_dirs);
+               void updateDb(bool bus, const boost::property_tree::ptree& xmlTree, std::vector<std::string>& incl_dirs);
                void updateGroupDb(bool bus);
        };
 }

diff --git a/tests/default_deny/system.d/ownerships.test.conf b/tests/default_deny/system.d/ownerships.test.conf
index 8ebd47f..c3527a5 100644 (file)
--- a/tests/default_deny/system.d/ownerships.test.conf
+++ b/tests/default_deny/system.d/ownerships.test.conf
@@ -76,5 +76,10 @@
     <deny own_prefix="org.tizen.pok2.a"/>
     <check own_prefix="org.tizen.pok2.a.b" privilege="privilege2"/>
   </policy>
+
+  <policy at_console="true">
+    <!-- just check that it will be never loaded -->
+    <deny own_prefix="org.tizen"/>
+  </policy>
 </busconfig>
 <!-- vim: set ft=xml: -->