From 078ef7b85ad77ba999588f72b31a50ced5907692 Mon Sep 17 00:00:00 2001 From: Daniel Mack 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 -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 @@ + +