refactoring: eliminate __internal_can_* functions

This removes __internal_can_* functions as they do not really
do anything useful anymore...

And, as always, it allows further simplifications.

Change-Id: Id30db12afbf8d60dce87c23355c41cff0d907d05

diff --git a/src/internal/internal.cpp b/src/internal/internal.cpp
index 9800e86..6078b05 100644 (file)
--- a/src/internal/internal.cpp
+++ b/src/internal/internal.cpp
@@ -86,49 +86,3 @@ void __internal_exit()
        if (tslog::enabled())
                pthread_mutex_unlock(&g_mutex);
 }
-
-int __internal_can_open(BusType bus_type,
-                                               uid_t bus_owner,
-                                               uid_t user,
-                                               gid_t group,
-                                               const char* const label)
-{
-       return static_cast<int>(policy_checker(bus_type).check(bus_owner, user, group, label));
-}
-
-int __internal_can_send(BusType bus_type,
-                                               const uid_t user,
-                                               const gid_t group,
-                                               const char* const label,
-                                               const KdbusBusNames &destination_names,
-                                               const char* const path,
-                                               const char* const interface,
-                                               const char* const member,
-                                               int type)
-{
-       MatchItemSend matcher(interface, member, path, static_cast<MessageType>(type), destination_names);
-       return static_cast<int>(policy_checker(bus_type).check(user, group, label, matcher));
-}
-
-int __internal_can_recv(BusType bus_type,
-                                               const uid_t user,
-                                               const gid_t group,
-                                               const char* const label,
-                                               const KdbusBusNames &sender_names,
-                                               const char* const path,
-                                               const char* const interface,
-                                               const char* const member,
-                                               int type)
-{
-       MatchItemReceive matcher(interface, member, path, static_cast<MessageType>(type), sender_names);
-       return static_cast<int>(policy_checker(bus_type).check(user, group, label, matcher));
-}
-
-int __internal_can_own(BusType bus_type,
-                                          const uid_t user,
-                                          const gid_t group,
-                                          const char* const label,
-                                          const char* const service)
-{
-       return static_cast<int>(policy_checker(bus_type).check(user, group, label, MatchItemOwn(service)));
-}

diff --git a/src/internal/internal.h b/src/internal/internal.h
index 75246c6..910a2b1 100644 (file)
--- a/src/internal/internal.h
+++ b/src/internal/internal.h
@@ -81,72 +81,4 @@ void __internal_enter(void);
 /** Disables logger mutex */
 void __internal_exit(void);
 
-/** checks if user can open dbus bus */
-int __internal_can_open(BusType bus_type,
-                                               uid_t bus_owner,
-                                               uid_t user,
-                                               gid_t group,
-                                               const char* const label);
-
-/** Checks if user can send message to given location.
- * \param[in] bus_type Bus type (system/session)
- * \param[in] user User id
- * \param[in] group User group id
- * \param[in] label Sender label
- * \param[in] destination Message destination
- * \param[in] destination_names Array containing names owned by the destination
- * \param[in] destination_names_size Size of destination_names array
- * \param[in] path Path
- * \param[in] interface Interface name
- * \param[in] member Member name
- * \param[in] type Message type
- * \return 1 on allow, 0 on deny, negative error code otherwise
- */
-int __internal_can_send(BusType bus_type,
-                                               const uid_t  user,
-                                               const gid_t  group,
-                                               const char* const label,
-                                               const KdbusBusNames &destination_names,
-                                               const char* const path,
-                                               const char* const interface,
-                                               const char* const member,
-                                               int type);
-
-/** Check if user can receive messages.
- * \param[in] bus_type Bus type (system/session)
- * \param[in] user User id
- * \param[in] group User group id
- * \param[in] label User label
- * \param[in] sender Sender of received message
- * \param[in] sender_names Array containing names owned by the sender
- * \param[in] sender_names_size Size of sender_names array
- * \param[in] path Path
- * \param[in] interface Interface name
- * \param[in] member Member name
- * \param[in] type Message type
- * \return 1 on allow, 0 on deny, negative error code otherwise
- */
-int __internal_can_recv(BusType bus_type,
-                                               uid_t user,
-                                               gid_t group,
-                                               const char* const label,
-                                               const KdbusBusNames &sender_names,
-                                               const char* const path,
-                                               const char* const interface,
-                                               const char* const member,
-                                               int type);
-
-/** Checks if given user can own interface with given label
- * \param[in] bus_type Bus type (system/session)
- * \param[in] user User id
- * \param[in] group User group id
- * \param[in] label Label given to Cynara
- * \param[in] service Name to own
- * \return 1 on allow, 0 on deny, negative error code otherwise
- */
-int __internal_can_own(BusType bus_type,
-                                          uid_t user,
-                                          gid_t group,
-                                          const char* const label,
-                                          const char* const service);
 #endif

diff --git a/src/libdbuspolicy1.cpp b/src/libdbuspolicy1.cpp
index 46d1648..7eadb73 100644 (file)
--- a/src/libdbuspolicy1.cpp
+++ b/src/libdbuspolicy1.cpp
@@ -15,10 +15,11 @@
 */
 
 
-#include "kdbus.h"
 #include "internal/internal.h"
-#include "libdbuspolicy1-private.h"
 #include "internal/transaction_guard.hpp"
+#include "internal/naive_policy_checker.hpp"
+#include "kdbus.h"
+#include "libdbuspolicy1-private.h"
 
 #include <assert.h>
 #include <boost/utility/string_ref.hpp>
@@ -39,6 +40,11 @@
 #define KDBUS_PATH_PREFIX "/sys/fs/kdbus/"
 #define KDBUS_SYSTEM_BUS_PATH KDBUS_PATH_PREFIX"0-system/bus"
 
+using ldp_xml_parser::DecisionResult;
+using ldp_xml_parser::MatchItemSend;
+using ldp_xml_parser::MatchItemReceive;
+using ldp_xml_parser::MatchItemOwn;
+
 struct kconn {
        KdbusConnection conn;
        BusType bus_type;
@@ -207,12 +213,12 @@ static kconn *init_shared_fd(BusType bus_type, int fd)
        return result;
 }
 
-static bool can_open(BusType bus_type, uid_t bus_owner) noexcept
+static DecisionResult can_open(BusType bus_type, uid_t bus_owner) noexcept
 {
        std::call_once(init_once_done, dbuspolicy_init_once_locked);
        std::call_once(init_once_db[bus_type], init_common_locked, bus_type);
 
-       return __internal_can_open(bus_type, bus_owner, g_udesc.uid, g_udesc.gid, g_udesc.label) > 0;
+       return policy_checker(bus_type).check(bus_owner, g_udesc.uid, g_udesc.gid, g_udesc.label);
 }
 
 DBUSPOLICY1_EXPORT void* dbuspolicy1_init_shared(const char *bus_path, int fd)
@@ -230,7 +236,7 @@ DBUSPOLICY1_EXPORT void* dbuspolicy1_init_shared(const char *bus_path, int fd)
        }
 
        struct kconn *result = nullptr;
-       if (can_open(bus_type, bus_owner)) {
+       if (can_open(bus_type, bus_owner) == DecisionResult::ALLOW) {
                if (-1 == fd) {
                        result = get_global_conn(bus_type, resolved_path);
                } else {
@@ -278,6 +284,14 @@ static int getNames(const char *name, KdbusConnectionInfo &info,
        return 0;
 }
 
+static inline int decisionToRetCode(DecisionResult decision) {
+       return static_cast<int>(decision);
+}
+
+static inline ldp_xml_parser::MessageType toMessageType(int type) {
+       return static_cast<ldp_xml_parser::MessageType>(type);
+}
+
 DBUSPOLICY1_EXPORT int dbuspolicy1_check_out(void* configuration,
                                                                                         const char *destination,
                                                                                         const char *sender,
@@ -306,12 +320,13 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_out(void* configuration,
        if (r < 0)
                return r;
 
-       r = __internal_can_send(kconn->bus_type,
-                       g_udesc.uid, g_udesc.gid, g_udesc.label,
-                       destinationInfo.names(), path, interface, member, message_type);
+       auto m_type = toMessageType(message_type);
 
-       if (r <= 0)
-               return r;
+       MatchItemSend sendItem(interface, member, path, m_type, destinationInfo.names());
+       auto decision = policy_checker(kconn->bus_type).check(g_udesc.uid, g_udesc.gid, g_udesc.label, sendItem);
+
+       if (DecisionResult::ALLOW != decision)
+               return decisionToRetCode(decision);
 
        KdbusConnectionInfo senderInfo(kconn->conn);
        /* check can recv */
@@ -320,9 +335,11 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_out(void* configuration,
        if (r < 0)
                return r;
 
-       return __internal_can_recv(kconn->bus_type,
-                       destinationInfo.uid(), destinationInfo.gid(), destinationInfo.label(),
-                       senderInfo.names(), path, interface, member, message_type);
+       MatchItemReceive receiveItem(interface, member, path, m_type, senderInfo.names());
+       return decisionToRetCode(policy_checker(kconn->bus_type).check(destinationInfo.uid(),
+                                                                                                                                  destinationInfo.gid(),
+                                                                                                                                  destinationInfo.label(),
+                                                                                                                                  receiveItem));
 }
 
 DBUSPOLICY1_EXPORT int dbuspolicy1_check_in(void* configuration,
@@ -349,27 +366,25 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_check_in(void* configuration,
        if (r < 0)
                return r;
 
-       r = __internal_can_send(kconn->bus_type,
-                       sender_uid, sender_gid, sender_label,
-                       info.names(), path, interface, member, message_type);
+       auto m_type = toMessageType(message_type);
 
-       if (r <= 0)
-               return r;
+       MatchItemSend sendItem(interface, member, path, m_type, info.names());
+       auto decision = policy_checker(kconn->bus_type).check(sender_uid, sender_gid, sender_label, sendItem);
+
+       if (DecisionResult::ALLOW != decision)
+               return decisionToRetCode(decision);
 
        if (!sender)
                sender = ":";
 
        /* libdbus, gdbus pass multiple sender as parameter : eg. "name_A name_B name_C". */
        KdbusBusNames names;
-       r = __internal_can_recv(kconn->bus_type,
-                       g_udesc.uid, g_udesc.gid, g_udesc.label,
-                       names.addSpaceSeparatedNames(sender),
-                       path, interface, member, message_type);
-
-       if (r <= 0)
-               return r;
+       MatchItemReceive receiveItem(interface, member, path, m_type, names.addSpaceSeparatedNames(sender));
 
-       return r;
+       return decisionToRetCode(policy_checker(kconn->bus_type).check(g_udesc.uid,
+                                                                                                                                  g_udesc.gid,
+                                                                                                                                  g_udesc.label,
+                                                                                                                                  receiveItem));
 }
 
 DBUSPOLICY1_EXPORT int dbuspolicy1_can_own(void* configuration, const char* const service)
@@ -379,5 +394,8 @@ DBUSPOLICY1_EXPORT int dbuspolicy1_can_own(void* configuration, const char* cons
        __internal_enter();
        auto internal_enter_guard = transaction_guard::makeGuard([] () { __internal_exit(); });
 
-       return __internal_can_own(kconn->bus_type, g_udesc.uid, g_udesc.gid, g_udesc.label, service);
+       return decisionToRetCode(policy_checker(kconn->bus_type).check(g_udesc.uid,
+                                                                                                                                  g_udesc.gid,
+                                                                                                                                  g_udesc.label,
+                                                                                                                                  MatchItemOwn(service)));
 }

diff --git a/src/test-libdbuspolicy1-access-deny.cpp b/src/test-libdbuspolicy1-access-deny.cpp
index c2e69da..72682fb 100644 (file)
--- a/src/test-libdbuspolicy1-access-deny.cpp
+++ b/src/test-libdbuspolicy1-access-deny.cpp
@@ -1,8 +1,11 @@
 #include "internal/internal.h"
+#include "internal/naive_policy_checker.hpp"
 #include <dbuspolicy1/libdbuspolicy1.h>
 #include <string>
 #include <vector>
 
+using ldp_xml_parser::DecisionResult;
+
 struct AccessTest {
        bool expected_result;
        uid_t user;
@@ -73,7 +76,7 @@ void print_test(const struct AccessTest* t, bool result) {
 void run_tests_for_bus(BusType bus_type, const std::vector<AccessTest>& test_setup, int& i, bool& passed) {
        for (const auto& test : test_setup) {
                __internal_init_sup_group(bus_type, test.user, test.group);
-               bool res = __internal_can_open(bus_type, bus_owner, test.user, test.group, test.label);
+               bool res = policy_checker(bus_type).check(bus_owner, test.user, test.group, test.label) == DecisionResult::ALLOW;
                if (res != test.expected_result) {
                        printf("[ERROR][%d] access test failed: %d %d ", i, (int)test.expected_result, (int)res);
                        print_test(&test, res);

diff --git a/src/test-libdbuspolicy1-method.cpp b/src/test-libdbuspolicy1-method.cpp
index edd579d..ad04e94 100644 (file)
--- a/src/test-libdbuspolicy1-method.cpp
+++ b/src/test-libdbuspolicy1-method.cpp
@@ -4,6 +4,7 @@
 #include <sys/types.h>
 #include <dbuspolicy1/libdbuspolicy1.h>
 #include "internal/internal.h"
+#include "internal/naive_policy_checker.hpp"
 #include "internal/policy.hpp"
 
 using namespace ldp_xml_parser;
@@ -63,26 +64,39 @@ void methodTest_print(struct MethodTest* t, bool result) {
 bool method_test() {
        unsigned  i = 0;
        bool flag = true;
-       bool ret = true;
        __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf");
        for (i = 0; i < sizeof(method_tests)/sizeof(struct MethodTest); i++) {
                KdbusBusNames bus_names;
                bus_names.addSpaceSeparatedNames(method_tests[i].name);
+
+               ldp_xml_parser::DecisionResult decision;
                if (method_tests[i].recv_send == MessageDirection::SEND)
                {
-                       ret = __internal_can_send(SYSTEM_BUS,
-                                       method_tests[i].user, method_tests[i].group, method_tests[i].label,
-                                       bus_names,
-                                       method_tests[i].path, method_tests[i].interface, method_tests[i].member,
-                                       static_cast<int>(method_tests[i].type));
-               } else if (method_tests[i].recv_send == MessageDirection::RECEIVE) {
-                       ret = __internal_can_recv(SYSTEM_BUS,
-                                       method_tests[i].user, method_tests[i].group, method_tests[i].label,
-                                       bus_names,
-                                       method_tests[i].path, method_tests[i].interface, method_tests[i].member,
-                                       static_cast<int>(method_tests[i].type));
+                       MatchItemSend itemSend(method_tests[i].interface,
+                                                                  method_tests[i].member,
+                                                                  method_tests[i].path,
+                                                                  method_tests[i].type,
+                                                                  bus_names);
+                       decision = policy_checker_system().check(method_tests[i].user,
+                                                                                                        method_tests[i].group,
+                                                                                                        method_tests[i].label,
+                                                                                                        itemSend);
+               } else {
+                       assert(method_tests[i].recv_send == MessageDirection::RECEIVE);
+                       MatchItemReceive itemReceive(method_tests[i].interface,
+                                                                                method_tests[i].member,
+                                                                                method_tests[i].path,
+                                                                                method_tests[i].type,
+                                                                                bus_names);
+
+                       decision = policy_checker_system().check(method_tests[i].user,
+                                                                                                        method_tests[i].group,
+                                                                                                        method_tests[i].label,
+                                                                                                        itemReceive);
                }
-               if ( (int)((method_tests[i].expected_result)) != ret) {
+               bool ret = decision == DecisionResult::ALLOW;
+
+               if ( method_tests[i].expected_result != ret) {
                        printf("[ERROR][%d] method test failed: %d %d ", i, (int)((method_tests[i].expected_result)), ret);
                        methodTest_print(&method_tests[i], ret);
                        printf("\n");

diff --git a/src/test-libdbuspolicy1-ownership-deny.cpp b/src/test-libdbuspolicy1-ownership-deny.cpp
index 31ac025..da1df16 100644 (file)
--- a/src/test-libdbuspolicy1-ownership-deny.cpp
+++ b/src/test-libdbuspolicy1-ownership-deny.cpp
@@ -1,6 +1,8 @@
 #include "internal/internal.h"
-#include <dbuspolicy1/libdbuspolicy1.h>
-#include <string>
+#include "internal/naive_policy_checker.hpp"
+
+using ldp_xml_parser::MatchItemOwn;
+using ldp_xml_parser::DecisionResult;
 
 struct OwnershipTest {
        bool expected_result;
@@ -129,11 +131,14 @@ void ownershipTest_print(struct OwnershipTest* t, bool result) {
 bool ownership_test() {
        unsigned  i = 0;
        bool flag = true;
-       bool ret = true;
        __internal_init(SYSTEM_BUS, "tests/default_deny/system.conf");
        for (i = 0; i < sizeof(ownership_tests)/sizeof(struct OwnershipTest); i++) {
-               ret = __internal_can_own(SYSTEM_BUS, ownership_tests[i].user,  ownership_tests[i].group,  ownership_tests[i].label,  ownership_tests[i].service);
-               if ( (int)((ownership_tests[i].expected_result)) != ret) {
+               auto decision = policy_checker_system().check(ownership_tests[i].user,
+                                                                                                         ownership_tests[i].group,
+                                                                                                         ownership_tests[i].label,
+                                                                                                         MatchItemOwn(ownership_tests[i].service));
+               bool ret = DecisionResult::ALLOW == decision;
+               if ( ownership_tests[i].expected_result != ret) {
                        printf("[ERROR][%d] ownership test failed: %d %d ", i, (int)((ownership_tests[i].expected_result)), ret);
                        ownershipTest_print(&ownership_tests[i], ret);
                        printf("\n");

diff --git a/src/test-libdbuspolicy1-ownership.cpp b/src/test-libdbuspolicy1-ownership.cpp
index cd8c1b7..8199f94 100644 (file)
--- a/src/test-libdbuspolicy1-ownership.cpp
+++ b/src/test-libdbuspolicy1-ownership.cpp
@@ -1,8 +1,8 @@
 #include "internal/internal.h"
-#include <dbuspolicy1/libdbuspolicy1.h>
-#include <iostream>
-#include <string>
-#include <sys/types.h>
+#include "internal/naive_policy_checker.hpp"
+
+using ldp_xml_parser::MatchItemOwn;
+using ldp_xml_parser::DecisionResult;
 
 struct OwnershipTest {
        bool expected_result;
@@ -53,11 +53,14 @@ void ownershipTest_print(struct OwnershipTest* t, bool result) {
 bool ownership_test() {
        unsigned  i = 0;
        bool flag = true;
-       bool ret = true;
        __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf");
        for (i = 0; i < sizeof(ownership_tests)/sizeof(struct OwnershipTest); i++) {
-               ret = __internal_can_own(SYSTEM_BUS, ownership_tests[i].user,  ownership_tests[i].group,  ownership_tests[i].label,  ownership_tests[i].service);
-               if ( (int)((ownership_tests[i].expected_result)) != ret) {
+               auto decision = policy_checker_system().check(ownership_tests[i].user,
+                                                                                                         ownership_tests[i].group,
+                                                                                                         ownership_tests[i].label,
+                                                                                                         MatchItemOwn(ownership_tests[i].service));
+               bool ret = DecisionResult::ALLOW == decision;
+               if (ownership_tests[i].expected_result != ret) {
                        printf("[ERROR][%d] ownership test failed: %d %d ", i, (int)((ownership_tests[i].expected_result)), ret);
                        ownershipTest_print(&ownership_tests[i], ret);
                        printf("\n");

diff --git a/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp
index 61d5a1b..7d1e7ea 100644 (file)
--- a/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp
+++ b/src/test-libdbuspolicy1-send_destination_prefix-deny.cpp
@@ -1,9 +1,13 @@
 #include "internal/internal.h"
+#include "internal/naive_policy_checker.hpp"
 #include "internal/policy.hpp"
 #include "libdbuspolicy1-private.h"
 #include <dbuspolicy1/libdbuspolicy1.h>
 #include <string>
 
+using ldp_xml_parser::MatchItemSend;
+using ldp_xml_parser::DecisionResult;
+
 struct Test {
        bool expected_result;
        uid_t user;
@@ -135,17 +139,22 @@ bool test()
 {
        unsigned  i = 0;
        bool flag = true;
-       bool ret = true;
 
        __internal_init(SYSTEM_BUS, "tests/default_deny/system.conf");
 
        for (i = 0; i < sizeof(tests)/sizeof(struct Test); i++) {
                KdbusBusNames names;
-               ret = __internal_can_send(SYSTEM_BUS, tests[i].user,
-                               tests[i].group, tests[i].label,
-                               names.addSpaceSeparatedNames(tests[i].destination),
-                               tests[i].path, tests[i].interface, tests[i].member,
-                               (int)tests[i].type);
+               MatchItemSend itemSend(tests[i].interface,
+                                                          tests[i].member,
+                                                          tests[i].path,
+                                                          tests[i].type,
+                                                          names.addSpaceSeparatedNames(tests[i].destination));
+
+               auto decision = policy_checker_system().check(tests[i].user,
+                                                                                                         tests[i].group,
+                                                                                                         tests[i].label,
+                                                                                                         itemSend);
+               bool ret = DecisionResult::ALLOW == decision;
 
                if (tests[i].expected_result != ret) {
                        printf("[ERROR][%d] test failed: %d %d ", i, (int)((tests[i].expected_result)), ret);

diff --git a/src/test-libdbuspolicy1-signal.cpp b/src/test-libdbuspolicy1-signal.cpp
index f9c57b2..18f0b3e 100644 (file)
--- a/src/test-libdbuspolicy1-signal.cpp
+++ b/src/test-libdbuspolicy1-signal.cpp
@@ -1,9 +1,10 @@
 #include "internal/internal.h"
-#include <dbuspolicy1/libdbuspolicy1.h>
-#include <iostream>
-#include <string>
+#include "internal/naive_policy_checker.hpp"
 #include <sys/types.h>
 
+using ldp_xml_parser::MatchItemSend;
+using ldp_xml_parser::DecisionResult;
+
 struct SignalTest {
        bool expected_result;
        uid_t user;
@@ -29,15 +30,22 @@ void signalTest_print(struct SignalTest* t, bool result) {
 bool signal_test() {
        unsigned  i = 0;
        bool flag = true;
-       bool ret = true;
        __internal_init(SYSTEM_BUS, "tests/default_allow/system.conf");
        for (i = 0; i < sizeof(signal_tests)/sizeof(struct SignalTest); i++) {
                KdbusBusNames names;
-               ret = __internal_can_send(SYSTEM_BUS,
-                               signal_tests[i].user, signal_tests[i].group, signal_tests[i].label,
-                               names.addSpaceSeparatedNames(signal_tests[i].dest),
-                               NULL, signal_tests[i].interface, NULL, DBUSPOLICY_MESSAGE_TYPE_SIGNAL);
-               if ( (int)((signal_tests[i].expected_result)) != ret) {
+               MatchItemSend itemSend(signal_tests[i].interface,
+                                                          nullptr,
+                                                          nullptr,
+                                                          ldp_xml_parser::MessageType::SIGNAL,
+                                                          names.addSpaceSeparatedNames(signal_tests[i].dest));
+
+               auto decision = policy_checker_system().check(signal_tests[i].user,
+                                                                                                         signal_tests[i].group,
+                                                                                                         signal_tests[i].label,
+                                                                                                         itemSend);
+
+               bool ret = DecisionResult::ALLOW == decision;
+               if (signal_tests[i].expected_result != ret) {
                        printf("[ERROR][%d] signal test failed: %d %d ", i, (int)((signal_tests[i].expected_result)), ret);
                        signalTest_print(&signal_tests[i], ret);
                        printf("\n");