[systemd-commits] src/bus-proxyd
David Herrmann
dvdhrm at kemper.freedesktop.org
Sat Jan 17 12:20:01 PST 2015
src/bus-proxyd/bus-xml-policy.c | 127 +++++++++++++++++++++++++++--------
src/bus-proxyd/bus-xml-policy.h | 26 ++++++-
src/bus-proxyd/proxy.c | 96 ++++++--------------------
src/bus-proxyd/test-bus-xml-policy.c | 34 ++++-----
4 files changed, 162 insertions(+), 121 deletions(-)
New commits:
commit 7447362c530e3f7128f16a35d1e43da4251144cc
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Sat Jan 17 21:18:52 2015 +0100
bus-proxy: don't print error-messages if we check multiple dests
If we test the policy against multiple destination names, we really should
not print warnings if one of the names results in DENY. Instead, pass the
whole array of names to the policy and let it deal with it.
diff --git a/src/bus-proxyd/bus-xml-policy.c b/src/bus-proxyd/bus-xml-policy.c
index 0c60b6b..f7f3388 100644
--- a/src/bus-proxyd/bus-xml-policy.c
+++ b/src/bus-proxyd/bus-xml-policy.c
@@ -22,6 +22,7 @@
#include "xml.h"
#include "fileio.h"
#include "strv.h"
+#include "set.h"
#include "conf-files.h"
#include "bus-internal.h"
#include "bus-message.h"
@@ -865,15 +866,14 @@ bool policy_check_hello(Policy *p, uid_t uid, gid_t gid) {
return verdict == ALLOW;
}
-bool policy_check_recv(Policy *p,
- uid_t uid,
- gid_t gid,
- int message_type,
- const char *name,
- const char *path,
- const char *interface,
- const char *member,
- bool dbus_to_kernel) {
+bool policy_check_one_recv(Policy *p,
+ uid_t uid,
+ gid_t gid,
+ int message_type,
+ const char *name,
+ const char *path,
+ const char *interface,
+ const char *member) {
struct policy_check_filter filter = {
.class = POLICY_ITEM_RECV,
@@ -886,30 +886,64 @@ bool policy_check_recv(Policy *p,
.member = member,
};
- int verdict;
-
assert(p);
- verdict = policy_check(p, &filter);
-
- log_full(LOG_AUTH | (verdict != ALLOW ? LOG_WARNING : LOG_DEBUG),
- "Receive permission check %s for uid=" UID_FMT " gid=" GID_FMT" message=%s name=%s path=%s interface=%s member=%s: %s",
- dbus_to_kernel ? "dbus-1 to kernel" : "kernel to dbus-1", uid, gid, bus_message_type_to_string(message_type), strna(name),
- strna(path), strna(interface), strna(member), strna(verdict_to_string(verdict)));
-
- return verdict == ALLOW;
+ return policy_check(p, &filter) == ALLOW;
}
-bool policy_check_send(Policy *p,
+bool policy_check_recv(Policy *p,
uid_t uid,
gid_t gid,
int message_type,
- const char *name,
+ Set *names,
+ char **namesv,
const char *path,
const char *interface,
const char *member,
bool dbus_to_kernel) {
+ char *n, **nv, *last = NULL;
+ bool allow = false;
+ Iterator i;
+
+ assert(p);
+
+ if (set_isempty(names) && strv_isempty(namesv)) {
+ allow = policy_check_one_recv(p, uid, gid, message_type, NULL, path, interface, member);
+ } else {
+ SET_FOREACH(n, names, i) {
+ last = n;
+ allow = policy_check_one_recv(p, uid, gid, message_type, n, path, interface, member);
+ if (allow)
+ break;
+ }
+ if (!allow) {
+ STRV_FOREACH(nv, namesv) {
+ last = *nv;
+ allow = policy_check_one_recv(p, uid, gid, message_type, *nv, path, interface, member);
+ if (allow)
+ break;
+ }
+ }
+ }
+
+ log_full(LOG_AUTH | (!allow ? LOG_WARNING : LOG_DEBUG),
+ "Receive permission check %s for uid=" UID_FMT " gid=" GID_FMT" message=%s name=%s path=%s interface=%s member=%s: %s",
+ dbus_to_kernel ? "dbus-1 to kernel" : "kernel to dbus-1", uid, gid, bus_message_type_to_string(message_type), strna(last),
+ strna(path), strna(interface), strna(member), allow ? "ALLOW" : "DENY");
+
+ return allow;
+}
+
+bool policy_check_one_send(Policy *p,
+ uid_t uid,
+ gid_t gid,
+ int message_type,
+ const char *name,
+ const char *path,
+ const char *interface,
+ const char *member) {
+
struct policy_check_filter filter = {
.class = POLICY_ITEM_SEND,
.uid = uid,
@@ -921,18 +955,57 @@ bool policy_check_send(Policy *p,
.member = member,
};
- int verdict;
+ assert(p);
+
+ return policy_check(p, &filter) == ALLOW;
+}
+
+bool policy_check_send(Policy *p,
+ uid_t uid,
+ gid_t gid,
+ int message_type,
+ Set *names,
+ char **namesv,
+ const char *path,
+ const char *interface,
+ const char *member,
+ bool dbus_to_kernel,
+ char **out_used_name) {
+
+ char *n, **nv, *last = NULL;
+ bool allow = false;
+ Iterator i;
assert(p);
- verdict = policy_check(p, &filter);
+ if (set_isempty(names) && strv_isempty(namesv)) {
+ allow = policy_check_one_send(p, uid, gid, message_type, NULL, path, interface, member);
+ } else {
+ SET_FOREACH(n, names, i) {
+ last = n;
+ allow = policy_check_one_send(p, uid, gid, message_type, n, path, interface, member);
+ if (allow)
+ break;
+ }
+ if (!allow) {
+ STRV_FOREACH(nv, namesv) {
+ last = *nv;
+ allow = policy_check_one_send(p, uid, gid, message_type, *nv, path, interface, member);
+ if (allow)
+ break;
+ }
+ }
+ }
- log_full(LOG_AUTH | (verdict != ALLOW ? LOG_WARNING : LOG_DEBUG),
+ if (out_used_name)
+ *out_used_name = last;
+
+ log_full(LOG_AUTH | (!allow ? LOG_WARNING : LOG_DEBUG),
"Send permission check %s for uid=" UID_FMT " gid=" GID_FMT" message=%s name=%s path=%s interface=%s member=%s: %s",
- dbus_to_kernel ? "dbus-1 to kernel" : "kernel to dbus-1", uid, gid, bus_message_type_to_string(message_type), strna(name),
- strna(path), strna(interface), strna(member), strna(verdict_to_string(verdict)));
+ dbus_to_kernel ? "dbus-1 to kernel" : "kernel to dbus-1", uid, gid, bus_message_type_to_string(message_type), strna(last),
+ strna(path), strna(interface), strna(member), allow ? "ALLOW" : "DENY");
- return verdict == ALLOW;
+ return allow;
}
int policy_load(Policy *p, char **files) {
diff --git a/src/bus-proxyd/bus-xml-policy.h b/src/bus-proxyd/bus-xml-policy.h
index 9716772..f2ec1bb 100644
--- a/src/bus-proxyd/bus-xml-policy.h
+++ b/src/bus-proxyd/bus-xml-policy.h
@@ -26,6 +26,7 @@
#include "list.h"
#include "hashmap.h"
+#include "set.h"
typedef enum PolicyItemType {
_POLICY_ITEM_TYPE_UNSET = 0,
@@ -91,24 +92,43 @@ void policy_free(Policy *p);
bool policy_check_own(Policy *p, uid_t uid, gid_t gid, const char *name);
bool policy_check_hello(Policy *p, uid_t uid, gid_t gid);
+bool policy_check_one_recv(Policy *p,
+ uid_t uid,
+ gid_t gid,
+ int message_type,
+ const char *name,
+ const char *path,
+ const char *interface,
+ const char *member);
bool policy_check_recv(Policy *p,
uid_t uid,
gid_t gid,
int message_type,
- const char *name,
+ Set *names,
+ char **namesv,
const char *path,
const char *interface,
const char *member,
bool dbus_to_kernel);
+bool policy_check_one_send(Policy *p,
+ uid_t uid,
+ gid_t gid,
+ int message_type,
+ const char *name,
+ const char *path,
+ const char *interface,
+ const char *member);
bool policy_check_send(Policy *p,
uid_t uid,
gid_t gid,
int message_type,
- const char *name,
+ Set *names,
+ char **namesv,
const char *path,
const char *interface,
const char *member,
- bool dbus_to_kernel);
+ bool dbus_to_kernel,
+ char **out_used_name);
void policy_dump(Policy *p);
diff --git a/src/bus-proxyd/proxy.c b/src/bus-proxyd/proxy.c
index ab16447..f7daec6 100644
--- a/src/bus-proxyd/proxy.c
+++ b/src/bus-proxyd/proxy.c
@@ -425,7 +425,6 @@ static int process_policy_unlocked(sd_bus *from, sd_bus *to, sd_bus_message *m,
uid_t sender_uid = UID_INVALID;
gid_t sender_gid = GID_INVALID;
char **sender_names = NULL;
- bool granted = false;
/* Driver messages are always OK */
if (streq_ptr(m->sender, "org.freedesktop.DBus"))
@@ -456,34 +455,9 @@ static int process_policy_unlocked(sd_bus *from, sd_bus *to, sd_bus_message *m,
}
/* First check whether the sender can send the message to our name */
- if (set_isempty(owned_names)) {
- if (policy_check_send(policy, sender_uid, sender_gid, m->header->type, NULL, m->path, m->interface, m->member, false))
- granted = true;
- } else {
- Iterator i;
- char *n;
-
- SET_FOREACH(n, owned_names, i)
- if (policy_check_send(policy, sender_uid, sender_gid, m->header->type, n, m->path, m->interface, m->member, false)) {
- granted = true;
- break;
- }
- }
-
- if (granted) {
- /* Then check whether us (the recipient) can receive from the sender's name */
- if (strv_isempty(sender_names)) {
- if (policy_check_recv(policy, our_ucred->uid, our_ucred->gid, m->header->type, NULL, m->path, m->interface, m->member, false))
- return 0;
- } else {
- char **n;
-
- STRV_FOREACH(n, sender_names) {
- if (policy_check_recv(policy, our_ucred->uid, our_ucred->gid, m->header->type, *n, m->path, m->interface, m->member, false))
- return 0;
- }
- }
- }
+ if (policy_check_send(policy, sender_uid, sender_gid, m->header->type, owned_names, NULL, m->path, m->interface, m->member, false, NULL) &&
+ policy_check_recv(policy, our_ucred->uid, our_ucred->gid, m->header->type, NULL, sender_names, m->path, m->interface, m->member, false))
+ return 0;
/* Return an error back to the caller */
if (m->header->type == SD_BUS_MESSAGE_METHOD_CALL)
@@ -499,7 +473,7 @@ static int process_policy_unlocked(sd_bus *from, sd_bus *to, sd_bus_message *m,
gid_t destination_gid = GID_INVALID;
const char *destination_unique = NULL;
char **destination_names = NULL;
- bool granted = false;
+ char *n;
/* Driver messages are always OK */
if (streq_ptr(m->destination, "org.freedesktop.DBus"))
@@ -525,42 +499,24 @@ static int process_policy_unlocked(sd_bus *from, sd_bus *to, sd_bus_message *m,
}
/* First check if we (the sender) can send to this name */
- if (strv_isempty(destination_names)) {
- if (policy_check_send(policy, our_ucred->uid, our_ucred->gid, m->header->type, NULL, m->path, m->interface, m->member, true))
- granted = true;
- } else {
- char **n;
-
- STRV_FOREACH(n, destination_names) {
- if (policy_check_send(policy, our_ucred->uid, our_ucred->gid, m->header->type, *n, m->path, m->interface, m->member, true)) {
-
- /* If we made a receiver decision,
- then remember which name's policy
- we used, and to which unique ID it
- mapped when we made the
- decision. Then, let's pass this to
- the kernel when sending the
- message, so that it refuses the
- operation should the name and
- unique ID not map to each other
- anymore. */
-
- r = free_and_strdup(&m->destination_ptr, *n);
- if (r < 0)
- return r;
-
- r = bus_kernel_parse_unique_name(destination_unique, &m->verify_destination_id);
- if (r < 0)
- break;
-
- granted = true;
- break;
- }
+ if (policy_check_send(policy, our_ucred->uid, our_ucred->gid, m->header->type, NULL, destination_names, m->path, m->interface, m->member, true, &n)) {
+ if (n) {
+ /* If we made a receiver decision, then remember which
+ * name's policy we used, and to which unique ID it
+ * mapped when we made the decision. Then, let's pass
+ * this to the kernel when sending the message, so that
+ * it refuses the operation should the name and unique
+ * ID not map to each other anymore. */
+
+ r = free_and_strdup(&m->destination_ptr, n);
+ if (r < 0)
+ return r;
+
+ r = bus_kernel_parse_unique_name(destination_unique, &m->verify_destination_id);
+ if (r < 0)
+ return r;
}
- }
- /* Then check if the recipient can receive from our name */
- if (granted) {
if (sd_bus_message_is_signal(m, NULL, NULL)) {
/* If we forward a signal from dbus-1 to kdbus,
* we have no idea who the recipient is.
@@ -571,16 +527,8 @@ static int process_policy_unlocked(sd_bus *from, sd_bus *to, sd_bus_message *m,
* receiver policies to the message. Therefore,
* skip policy checks in this case. */
return 0;
- } else if (set_isempty(owned_names)) {
- if (policy_check_recv(policy, destination_uid, destination_gid, m->header->type, NULL, m->path, m->interface, m->member, true))
- return 0;
- } else {
- Iterator i;
- char *n;
-
- SET_FOREACH(n, owned_names, i)
- if (policy_check_recv(policy, destination_uid, destination_gid, m->header->type, n, m->path, m->interface, m->member, true))
- return 0;
+ } else if (policy_check_recv(policy, destination_uid, destination_gid, m->header->type, owned_names, NULL, m->path, m->interface, m->member, true)) {
+ return 0;
}
}
diff --git a/src/bus-proxyd/test-bus-xml-policy.c b/src/bus-proxyd/test-bus-xml-policy.c
index cd1b4b2..4b07747 100644
--- a/src/bus-proxyd/test-bus-xml-policy.c
+++ b/src/bus-proxyd/test-bus-xml-policy.c
@@ -105,8 +105,8 @@ int main(int argc, char *argv[]) {
/* Signaltest */
assert_se(test_policy_load(&p, "signals.conf") == 0);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_SIGNAL, "bli.bla.blubb", NULL, "/an/object/path", NULL, false) == true);
- assert_se(policy_check_send(&p, 1, 0, SD_BUS_MESSAGE_SIGNAL, "bli.bla.blubb", NULL, "/an/object/path", NULL, false) == false);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_SIGNAL, "bli.bla.blubb", NULL, "/an/object/path", NULL) == true);
+ assert_se(policy_check_one_send(&p, 1, 0, SD_BUS_MESSAGE_SIGNAL, "bli.bla.blubb", NULL, "/an/object/path", NULL) == false);
policy_free(&p);
@@ -114,12 +114,12 @@ int main(int argc, char *argv[]) {
assert_se(test_policy_load(&p, "methods.conf") == 0);
policy_dump(&p);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "bli.bla.blubb", "Member", false) == false);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "bli.bla.blubb", "Member", false) == false);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int1", "Member", false) == true);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member", false) == true);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "bli.bla.blubb", "Member") == false);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "bli.bla.blubb", "Member") == false);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int1", "Member") == true);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member") == true);
- assert_se(policy_check_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test3", "/an/object/path", "org.test.int3", "Member111", false) == true);
+ assert_se(policy_check_one_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test3", "/an/object/path", "org.test.int3", "Member111") == true);
policy_free(&p);
@@ -162,19 +162,19 @@ int main(int argc, char *argv[]) {
assert_se(policy_check_own(&p, 0, 0, "org.foo.FooService") == true);
assert_se(policy_check_own(&p, 0, 0, "org.foo.FooService2") == false);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member", false) == false);
- assert_se(policy_check_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == true);
- assert_se(policy_check_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == true);
- assert_se(policy_check_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface2", "Member", false) == false);
- assert_se(policy_check_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService2", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == false);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member") == false);
+ assert_se(policy_check_one_send(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == true);
+ assert_se(policy_check_one_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == true);
+ assert_se(policy_check_one_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface2", "Member") == false);
+ assert_se(policy_check_one_recv(&p, 0, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService2", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == false);
assert_se(policy_check_own(&p, 100, 0, "org.foo.FooService") == false);
assert_se(policy_check_own(&p, 100, 0, "org.foo.FooService2") == false);
- assert_se(policy_check_send(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member", false) == false);
- assert_se(policy_check_send(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == false);
- assert_se(policy_check_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == true);
- assert_se(policy_check_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface2", "Member", false) == false);
- assert_se(policy_check_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService2", "/an/object/path", "org.foo.FooBroadcastInterface", "Member", false) == false);
+ assert_se(policy_check_one_send(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.test.int2", "Member") == false);
+ assert_se(policy_check_one_send(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.test.test1", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == false);
+ assert_se(policy_check_one_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == true);
+ assert_se(policy_check_one_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService", "/an/object/path", "org.foo.FooBroadcastInterface2", "Member") == false);
+ assert_se(policy_check_one_recv(&p, 100, 0, SD_BUS_MESSAGE_METHOD_CALL, "org.foo.FooService2", "/an/object/path", "org.foo.FooBroadcastInterface", "Member") == false);
policy_free(&p);
More information about the systemd-commits
mailing list