From 078ef7b85ad77ba999588f72b31a50ced5907692 Mon Sep 17 00:00:00 2001

From: Daniel Mack <daniel@zonque.org>

Date: Wed, 24 Sep 2014 17:02:08 +0200

Subject: [PATCH] bus-policy: split API for bus-proxyd

Instead of operating on an sd_bus_message object, expose an API that has 4

functions:

  policy_check_own()

  policy_check_hello()

  policy_check_recv()

  policy_check_send()

This also allows dropping extra code to parse message contents - the bus

proxy already has dedicated code paths for that, and we can hook into

those later.

Tests amended accordingly.

---

 src/bus-proxyd/bus-policy.c      | 200 ++++++++++++++++++++++++---------------

 src/bus-proxyd/bus-policy.h      |  17 +++-

 src/bus-proxyd/test-bus-policy.c |  92 ++++++------------

 test/bus-policy/methods.conf     |   2 +

 4 files changed, 171 insertions(+), 140 deletions(-)

diff --git a/src/bus-proxyd/bus-policy.c b/src/bus-proxyd/bus-policy.c

index 151d679f6b..165e763f57 100644

--- a/src/bus-proxyd/bus-policy.c

+++ b/src/bus-proxyd/bus-policy.c

@@ -592,93 +592,73 @@ static int file_load(Policy *p, const char *path) {

         }

 }

-static bool is_matching_name_request(sd_bus_message *m, const char *name, bool prefix) {

-

-        char *n = NULL;

-        int r;

-

-        if (!sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "RequestName"))

-                return false;

-

-        r = sd_bus_message_read(m, "s", &n);

-        if (r < 0)

-                return false;

-

-        r = sd_bus_message_rewind(m, true);

-        if (r < 0)

-                return false;

-

-        if (prefix)

-                return startswith(name, n);

-        else

-                return streq_ptr(name, n);

-}

-

-static bool is_matching_call(PolicyItem *i, sd_bus_message *m, const char *name) {

-

-        if (i->message_type && (i->message_type != m->header->type))

-                return false;

-

-        if (i->path && (!m->path || !streq(i->path, m->path)))

-                return false;

-

-        if (i->member && (!m->member || !streq(i->member, m->member)))

-                return false;

-

-        if (i->interface && (!m->interface || !streq(i->interface, m->interface)))

-                return false;

-

-        if (i->name && (!name || !streq(i->name, name)))

-                return false;

-

-        return true;

-}

-

 enum {

         ALLOW,

         DUNNO,

         DENY,

 };

+struct policy_check_filter {

+        int class;

+        const struct ucred *ucred;

+        int message_type;

+        const char *interface;

+        const char *path;

+        const char *member;

+        char **names_strv;

+        Hashmap *names_hash;

+};

+

 static int is_permissive(PolicyItem *i) {

         return (i->type == POLICY_ITEM_ALLOW) ? ALLOW : DENY;

 }

-static int check_policy_item(PolicyItem *i, sd_bus_message *m, const struct ucred *ucred) {

+static int check_policy_item(PolicyItem *i, const struct policy_check_filter *filter) {

         switch (i->class) {

         case POLICY_ITEM_SEND:

-                if ((m->bus->is_kernel  && is_matching_call(i, m, m->destination)) ||

-                    (!m->bus->is_kernel && is_matching_call(i, m, m->sender)))

-                        return is_permissive(i);

-                break;

-

         case POLICY_ITEM_RECV:

-                if ((m->bus->is_kernel  && is_matching_call(i, m, m->sender)) ||

-                    (!m->bus->is_kernel && is_matching_call(i, m, m->destination)))

-                        return is_permissive(i);

-                break;

+

+                if (i->name) {

+                        if (filter->names_hash && !hashmap_contains(filter->names_hash, i->name))

+                                break;

+

+                        if (filter->names_strv && !strv_contains(filter->names_strv, i->name))

+                                break;

+                }

+

+                if (i->message_type && (i->message_type != filter->message_type))

+                        break;

+

+                if (i->path && !streq_ptr(i->path, filter->path))

+                        break;

+

+                if (i->member && !streq_ptr(i->member, filter->member))

+                        break;

+

+                if (i->interface && !streq_ptr(i->interface, filter->interface))

+                        break;

+

+                return is_permissive(i);

         case POLICY_ITEM_OWN:

-                if (is_matching_name_request(m, i->name, false))

+                if (streq(i->name, filter->member))

                         return is_permissive(i);

                 break;

         case POLICY_ITEM_OWN_PREFIX:

-                if (is_matching_name_request(m, i->name, true))

+                if (startswith(i->name, filter->member))

                         return is_permissive(i);

                 break;

         case POLICY_ITEM_USER:

-                if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "Hello") &&

-                    (streq_ptr(i->name, "*") || (i->uid_valid && i->uid == ucred->uid)))

+                if ((streq_ptr(i->name, "*") || (i->uid_valid && i->uid == filter->ucred->uid)))

                         return is_permissive(i);

                 break;

         case POLICY_ITEM_GROUP:

-                if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "Hello") &&

-                    (streq_ptr(i->name, "*") || (i->gid_valid && i->gid == ucred->gid)))

+                if ((streq_ptr(i->name, "*") || (i->gid_valid && i->gid == filter->ucred->gid)))

                         return is_permissive(i);

                 break;

@@ -690,7 +670,7 @@ static int check_policy_item(PolicyItem *i, sd_bus_message *m, const struct ucre

         return DUNNO;

 }

-static int check_policy_items(PolicyItem *items, sd_bus_message *m, const struct ucred *ucred) {

+static int check_policy_items(PolicyItem *items, const struct policy_check_filter *filter) {

         PolicyItem *i;

         int r, ret = DUNNO;

@@ -698,7 +678,10 @@ static int check_policy_items(PolicyItem *items, sd_bus_message *m, const struct

         /* Check all policies in a set - a broader one might be followed by a more specific one,

          * and the order of rules in policy definitions matters */

         LIST_FOREACH(items, i, items) {

-                r = check_policy_item(i, m, ucred);

+                if (i->class != filter->class)

+                        continue;

+

+                r = check_policy_item(i, filter);

                 if (r != DUNNO)

                         ret = r;

         }

@@ -706,7 +689,7 @@ static int check_policy_items(PolicyItem *items, sd_bus_message *m, const struct

         return ret;

 }

-bool policy_check(Policy *p, sd_bus_message *m, const struct ucred *ucred) {

+static int policy_check(Policy *p, const struct policy_check_filter *filter) {

         PolicyItem *items;

         int r;

@@ -720,31 +703,100 @@ bool policy_check(Policy *p, sd_bus_message *m, const struct ucred *ucred) {

          * 4. If the message isn't caught be the defaults either, reject it.

          */

-        r = check_policy_items(p->mandatory_items, m, ucred);

+        r = check_policy_items(p->mandatory_items, filter);

         if (r != DUNNO)

-                return r == ALLOW;

+                return r;

-        if (ucred->pid > 0) {

-                items = hashmap_get(p->user_items, UINT32_TO_PTR(ucred->uid));

+        if (filter->ucred) {

+                items = hashmap_get(p->user_items, UINT32_TO_PTR(filter->ucred->uid));

                 if (items) {

-                        r = check_policy_items(items, m, ucred);

+                        r = check_policy_items(items, filter);

                         if (r != DUNNO)

-                                return r == ALLOW;

+                                return r;

                 }

-                items = hashmap_get(p->group_items, UINT32_TO_PTR(ucred->gid));

+                items = hashmap_get(p->group_items, UINT32_TO_PTR(filter->ucred->gid));

                 if (items) {

-                        r = check_policy_items(items, m, ucred);

+                        r = check_policy_items(items, filter);

                         if (r != DUNNO)

-                                return r == ALLOW;

+                                return r;

                 }

         }

-        r = check_policy_items(p->default_items, m, ucred);

-        if (r != DUNNO)

-                return r == ALLOW;

+        return check_policy_items(p->default_items, filter);

+}

+

+bool policy_check_own(Policy *p, const struct ucred *ucred, const char *name) {

+

+        struct policy_check_filter filter = {

+                .class  = POLICY_ITEM_OWN,

+                .ucred  = ucred,

+                .member = name,

+        };

+

+        return policy_check(p, &filter) == ALLOW;

+}

+

+bool policy_check_hello(Policy *p, const struct ucred *ucred) {

+

+        struct policy_check_filter filter = {

+                .class  = POLICY_ITEM_USER,

+                .ucred  = ucred,

+        };

+        int user, group;

+

+        user = policy_check(p, &filter);

+        if (user == DENY)

+                return false;

+

+        filter.class = POLICY_ITEM_GROUP;

+        group = policy_check(p, &filter);

+        if (user == DUNNO && group == DUNNO)

+                return false;

+

+        return !(user == DENY || group == DENY);

+}

+

+bool policy_check_recv(Policy *p,

+                       const struct ucred *ucred,

+                       Hashmap *names,

+                       int message_type,

+                       const char *path,

+                       const char *interface,

+                       const char *member) {

+

+        struct policy_check_filter filter = {

+                .class        = POLICY_ITEM_RECV,

+                .ucred        = ucred,

+                .names_hash   = names,

+                .message_type = message_type,

+                .interface    = interface,

+                .path         = path,

+                .member       = member,

+        };

+

+        return policy_check(p, &filter) == ALLOW;

+}

-        return false;

+bool policy_check_send(Policy *p,

+                       const struct ucred *ucred,

+                       char **names,

+                       int message_type,

+                       const char *path,

+                       const char *interface,

+                       const char *member) {

+

+        struct policy_check_filter filter = {

+                .class        = POLICY_ITEM_SEND,

+                .ucred        = ucred,

+                .names_strv   = names,

+                .message_type = message_type,

+                .interface    = interface,

+                .path         = path,

+                .member       = member,

+        };

+

+        return policy_check(p, &filter) == ALLOW;

 }

 int policy_load(Policy *p, char **files) {

diff --git a/src/bus-proxyd/bus-policy.h b/src/bus-proxyd/bus-policy.h

index 2222716e7a..5b4d9d0c10 100644

--- a/src/bus-proxyd/bus-policy.h

+++ b/src/bus-proxyd/bus-policy.h

@@ -76,7 +76,22 @@ typedef struct Policy {

 int policy_load(Policy *p, char **files);

 void policy_free(Policy *p);

-bool policy_check(Policy *p, sd_bus_message *m, const struct ucred *c);

+bool policy_check_own(Policy *p, const struct ucred *ucred, const char *name);

+bool policy_check_hello(Policy *p, const struct ucred *ucred);

+bool policy_check_recv(Policy *p,

+                       const struct ucred *ucred,

+                       Hashmap *names,

+                       int message_type,

+                       const char *path,

+                       const char *interface,

+                       const char *member);

+bool policy_check_send(Policy *p,

+                       const struct ucred *ucred,

+                       char **names,

+                       int message_type,

+                       const char *path,

+                       const char *interface,

+                       const char *member);

 void policy_dump(Policy *p);

diff --git a/src/bus-proxyd/test-bus-policy.c b/src/bus-proxyd/test-bus-policy.c

index ed17bfe96e..37e66274f0 100644

--- a/src/bus-proxyd/test-bus-policy.c

+++ b/src/bus-proxyd/test-bus-policy.c

@@ -44,122 +44,84 @@

 #include <bus-proxyd/bus-policy.h>

-static int make_name_request(sd_bus *bus,

-                             const char *name,

-                             sd_bus_message **ret) {

-

-        int r;

-        sd_bus_message *m = NULL;

-

-        r = sd_bus_message_new_method_call(bus, &m, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "RequestName");

-        if (r < 0)

-                return r;

-

-        r = sd_bus_message_append_basic(m, 's', name);

-        if (r < 0)

-                return r;

-

-        m->sealed = 1;

-        sd_bus_message_rewind(m, true);

-

-        *ret = m;

-        return 0;

-}

-

 int main(int argc, char *argv[]) {

         Policy p = {};

-        sd_bus_message *m;

         struct ucred ucred = {};

-        _cleanup_bus_close_unref_ sd_bus *bus = NULL;;

-

-        assert_se(sd_bus_default_system(&bus) >= 0);

-

-        /* Fake pid for policy checks */

-        ucred.pid = 1;

+        char **names_strv;

+        Hashmap *names_hash;

         /* Ownership tests */

         assert_se(policy_load(&p, STRV_MAKE("test/bus-policy/ownerships.conf")) == 0);

-        assert_se(make_name_request(bus, "org.test.test1", &m) == 0);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == true);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test1") == true);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == true);

-        assert_se(sd_bus_message_unref(m) == 0);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test1") == true);

-        assert_se(make_name_request(bus, "org.test.test2", &m) == 0);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == true);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test2") == true);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == false);

-        assert_se(sd_bus_message_unref(m) == 0);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test2") == false);

-        assert_se(make_name_request(bus, "org.test.test3", &m) == 0);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == false);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test3") == false);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == false);

-        assert_se(sd_bus_message_unref(m) == 0);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test3") == false);

-        assert_se(make_name_request(bus, "org.test.test4", &m) == 0);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == false);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test4") == false);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == true);

-        assert_se(sd_bus_message_unref(m) == 0);

+        assert_se(policy_check_own(&p, &ucred, "org.test.test4") == true);

         policy_free(&p);

-        /* Signal test */

+        /* Signaltest */

         assert_se(policy_load(&p, STRV_MAKE("test/bus-policy/signals.conf")) == 0);

+        names_strv = STRV_MAKE("bli.bla.blubb");

-        assert_se(sd_bus_message_new_signal(bus, &m, "/an/object/path", "bli.bla.blubb", "Name") == 0);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == true);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_SIGNAL, NULL, "/an/object/path", NULL) == true);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == false);

-        assert_se(sd_bus_message_unref(m) == 0);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_SIGNAL, NULL, "/an/object/path", NULL) == false);

         policy_free(&p);

         /* Method calls */

         assert_se(policy_load(&p, STRV_MAKE("test/bus-policy/methods.conf")) == 0);

+        names_strv = STRV_MAKE("org.test.test1");

+        policy_dump(&p);

         ucred.uid = 0;

-        assert_se(sd_bus_message_new_method_call(bus, &m, "org.foo.bar", "/an/object/path", "bli.bla.blubb", "Member") == 0);

-        assert_se(policy_check(&p, m, &ucred) == false);

-

-        assert_se(sd_bus_message_new_method_call(bus, &m, "org.test.test1", "/an/object/path", "bli.bla.blubb", "Member") == 0);

-        assert_se(policy_check(&p, m, &ucred) == false);

-        bus->is_kernel = 1;

-        assert_se(sd_bus_message_new_method_call(bus, &m, "org.test.test1", "/an/object/path", "org.test.int1", "Member") == 0);

-        assert_se(policy_check(&p, m, &ucred) == true);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_METHOD_CALL, "/an/object/path", "bli.bla.blubb", "Member") == false);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_METHOD_CALL, "/an/object/path", "bli.bla.blubb", "Member") == false);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_METHOD_CALL, "/an/object/path", "org.test.int1", "Member") == true);

+        assert_se(policy_check_send(&p, &ucred, names_strv, SD_BUS_MESSAGE_METHOD_CALL, "/an/object/path", "org.test.int2", "Member") == true);

-        assert_se(sd_bus_message_new_method_call(bus, &m, "org.test.test1", "/an/object/path", "org.test.int2", "Member") == 0);

-        assert_se(policy_check(&p, m, &ucred) == true);

+        names_hash = hashmap_new(&string_hash_ops);

+        assert(names_hash != NULL);

+        assert_se(hashmap_put(names_hash, "org.test.test3", NULL) >= 0);

+        assert_se(policy_check_recv(&p, &ucred, names_hash, SD_BUS_MESSAGE_METHOD_CALL, "/an/object/path", "org.test.int3", "Member111") == true);

         policy_free(&p);

         /* User and groups */

         assert_se(policy_load(&p, STRV_MAKE("test/bus-policy/hello.conf")) == 0);

-        assert_se(sd_bus_message_new_method_call(bus, &m, "org.freedesktop.DBus", "/org/freedesktop/DBus", "org.freedesktop.DBus", "Hello") == 0);

         policy_dump(&p);

         ucred.uid = 0;

-        assert_se(policy_check(&p, m, &ucred) == true);

+        assert_se(policy_check_hello(&p, &ucred) == true);

         ucred.uid = 1;

-        assert_se(policy_check(&p, m, &ucred) == false);

+        assert_se(policy_check_hello(&p, &ucred) == false);

         ucred.uid = 0;

         ucred.gid = 1;

-        assert_se(policy_check(&p, m, &ucred) == false);

+        assert_se(policy_check_hello(&p, &ucred) == false);

         policy_free(&p);

-

         return EXIT_SUCCESS;

 }

diff --git a/test/bus-policy/methods.conf b/test/bus-policy/methods.conf

index d6c28c71bc..4d4675ea10 100644

--- a/test/bus-policy/methods.conf

+++ b/test/bus-policy/methods.conf

@@ -10,6 +10,8 @@

                 <deny send_destination="org.test.test1"/>

                 <allow send_destination="org.test.test1" send_interface="org.test.int1"/>

                 <allow send_destination="org.test.test1" send_interface="org.test.int2"/>

+

+                <allow receive_sender="org.test.test3" receive_interface="org.test.int3" receiver_member="Member111"/>

         </policy>

 </busconfig>