[systemd-commits] 7 commits - src/bus-proxyd src/libsystemd

Daniel Mack zonque at kemper.freedesktop.org
Tue Nov 11 05:14:52 PST 2014


 src/bus-proxyd/bus-proxyd.c         |  145 ++++++++++++++++++++++--------------
 src/libsystemd/sd-bus/bus-kernel.c  |   27 ++++--
 src/libsystemd/sd-bus/bus-message.c |    5 +
 src/libsystemd/sd-bus/bus-message.h |    2 
 4 files changed, 117 insertions(+), 62 deletions(-)

New commits:
commit 5bb24cccbce846c0d77e71b70a3be7f4b2ba6c0e
Author: Daniel Mack <daniel at zonque.org>
Date:   Thu Oct 23 13:06:38 2014 +0200

    bus-proxyd: make policy checks optional
    
    Retrieve the bus owner creds, and when the uid matches the current user's
    uid and is non-null, don't check the bus policy.

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 3fc341e..c5aeaac 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -475,6 +475,9 @@ static int process_policy(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *polic
         assert(b);
         assert(m);
 
+        if (!policy)
+                return 0;
+
         if (b->is_kernel) {
 
                 /* The message came from the kernel, and is sent to our legacy client. */
@@ -849,7 +852,7 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *polic
                 if (r < 0)
                         return synthetic_reply_method_errno(m, r, NULL);
 
-                if (!policy_check_own(policy, ucred, name))
+                if (policy && !policy_check_own(policy, ucred, name))
                         return synthetic_reply_method_errno(m, -EPERM, NULL);
 
                 if (!service_name_is_valid(name))
@@ -1022,7 +1025,7 @@ static int process_hello(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *policy
                 return -EIO;
         }
 
-        if (!policy_check_hello(policy, ucred)) {
+        if (policy && !policy_check_hello(policy, ucred)) {
                 log_error("Policy denied HELLO");
                 return -EPERM;
         }
@@ -1133,6 +1136,7 @@ static int patch_sender(sd_bus *a, sd_bus_message *m) {
 
 int main(int argc, char *argv[]) {
 
+        _cleanup_bus_creds_unref_ sd_bus_creds *bus_creds = NULL;
         _cleanup_bus_close_unref_ sd_bus *a = NULL, *b = NULL;
         sd_id128_t server_id;
         int r, in_fd, out_fd;
@@ -1251,6 +1255,12 @@ int main(int argc, char *argv[]) {
                 goto finish;
         }
 
+        r = sd_bus_get_owner_creds(a, SD_BUS_CREDS_UID, &bus_creds);
+        if (r < 0) {
+                log_error("Failed to get bus creds: %s", strerror(-r));
+                goto finish;
+        }
+
         r = sd_bus_new(&b);
         if (r < 0) {
                 log_error("Failed to allocate bus: %s", strerror(-r));
@@ -1410,13 +1420,21 @@ int main(int argc, char *argv[]) {
                 }
 
                 if (m) {
+                        Policy *p = NULL;
+                        uid_t uid;
+
+                        assert_se(sd_bus_creds_get_uid(bus_creds, &uid) == 0);
+
+                        if (uid == 0 || uid != ucred.uid)
+                                p = &policy;
+
                         /* We officially got EOF, let's quit */
                         if (sd_bus_message_is_signal(m, "org.freedesktop.DBus.Local", "Disconnected")) {
                                 r = 0;
                                 goto finish;
                         }
 
-                        k = process_hello(a, b, m, &policy, &ucred, &got_hello);
+                        k = process_hello(a, b, m, p, &ucred, &got_hello);
                         if (k < 0) {
                                 r = k;
                                 log_error("Failed to process HELLO: %s", strerror(-r));
@@ -1426,7 +1444,7 @@ int main(int argc, char *argv[]) {
                         if (k > 0)
                                 r = k;
                         else {
-                                k = process_driver(a, b, m, &policy, &ucred);
+                                k = process_driver(a, b, m, p, &ucred);
                                 if (k < 0) {
                                         r = k;
                                         log_error("Failed to process driver calls: %s", strerror(-r));
@@ -1448,6 +1466,9 @@ int main(int argc, char *argv[]) {
                                                 if (k == -ECONNRESET)
                                                         r = 0;
                                                 else {
+
+                                                k = process_policy(a, b, m, p, &ucred);
+                                                if (k < 0) {
                                                         r = k;
                                                         log_error("Failed to send message: %s", strerror(-r));
                                                 }

commit 2a2be74654f0511220cf9a8a72f60ab5705abb87
Author: Daniel Mack <daniel at zonque.org>
Date:   Thu Oct 9 13:26:53 2014 +0200

    bus-proxyd: move name list iteration to policy users
    
    We need to figure out which of the possible names satisfied the policy,
    so we cannot do the iteration in check_policy_item() but have to leave it
    to the users.
    
    Test cases amended accordingly.

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 1106986..3fc341e 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -475,18 +475,26 @@ static int process_policy(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *polic
         assert(b);
         assert(m);
 
-        if (a->is_kernel)
-                return 0;
+        if (b->is_kernel) {
 
-        r = sd_bus_creds_get_well_known_names(&m->creds, &names_strv);
-        if (r < 0)
-                return r;
+                /* The message came from the kernel, and is sent to our legacy client. */
+                r = sd_bus_creds_get_well_known_names(&m->creds, &names_strv);
+                if (r < 0)
+                        return r;
 
-        if (!policy_check_recv(policy, ucred, names_hash, m->header->type, m->path, m->interface, m->member))
-                return -EPERM;
+/*
+                if (!policy_check_recv(policy, ucred, names_hash, m->header->type, m->path, m->interface, m->member))
+                        return -EPERM;
 
-        if (!policy_check_send(policy, ucred, names_strv, m->header->type, m->path, m->interface, m->member))
-                return -EPERM;
+                if (!policy_check_send(policy, ucred, names_strv, m->header->type, m->path, m->interface, m->member))
+                        return -EPERM;
+*/
+        } else {
+
+
+
+
+        }
 
         return 0;
 }

commit 9cd751d2d0310275b2020bbb32c5e3f61a3cd53b
Author: Daniel Mack <daniel at zonque.org>
Date:   Wed Sep 24 17:50:31 2014 +0200

    bus-proxyd: enforce policy for method calls

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 2f26f81..1106986 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -311,48 +311,6 @@ static int synthesize_name_acquired(sd_bus *a, sd_bus *b, sd_bus_message *m) {
         return sd_bus_send(b, n, NULL);
 }
 
-static int process_policy(sd_bus *a, sd_bus *b, sd_bus_message *m) {
-        _cleanup_bus_message_unref_ sd_bus_message *n = NULL;
-        int r;
-
-        assert(a);
-        assert(b);
-        assert(m);
-
-        if (!a->is_kernel)
-                return 0;
-
-        if (!sd_bus_message_is_method_call(m, "org.freedesktop.DBus.Properties", "GetAll"))
-                return 0;
-
-        if (!streq_ptr(m->path, "/org/gnome/DisplayManager/Slave"))
-                return 0;
-
-        r = sd_bus_message_new_method_errorf(m, &n, SD_BUS_ERROR_ACCESS_DENIED, "gdm, you are stupid");
-        if (r < 0)
-                return r;
-
-        r = bus_message_append_sender(n, "org.freedesktop.DBus");
-        if (r < 0) {
-                log_error("Failed to append sender to gdm reply: %s", strerror(-r));
-                return r;
-        }
-
-        r = bus_seal_synthetic_message(b, n);
-        if (r < 0) {
-                log_error("Failed to seal gdm reply: %s", strerror(-r));
-                return r;
-        }
-
-        r = sd_bus_send(b, n, NULL);
-        if (r < 0) {
-                log_error("Failed to send gdm reply: %s", strerror(-r));
-                return r;
-        }
-
-        return 1;
-}
-
 static int synthetic_driver_send(sd_bus *b, sd_bus_message *m) {
         int r;
 
@@ -509,6 +467,30 @@ static int peer_is_privileged(sd_bus *bus, sd_bus_message *m) {
         return false;
 }
 
+static int process_policy(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *policy, const struct ucred *ucred) {
+        int r;
+        char **names_strv;
+
+        assert(a);
+        assert(b);
+        assert(m);
+
+        if (a->is_kernel)
+                return 0;
+
+        r = sd_bus_creds_get_well_known_names(&m->creds, &names_strv);
+        if (r < 0)
+                return r;
+
+        if (!policy_check_recv(policy, ucred, names_hash, m->header->type, m->path, m->interface, m->member))
+                return -EPERM;
+
+        if (!policy_check_send(policy, ucred, names_strv, m->header->type, m->path, m->interface, m->member))
+                return -EPERM;
+
+        return 0;
+}
+
 static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *policy, const struct ucred *ucred) {
         int r;
 
@@ -1436,13 +1418,6 @@ int main(int argc, char *argv[]) {
                         if (k > 0)
                                 r = k;
                         else {
-                                k = process_policy(a, b, m);
-                                if (k < 0) {
-                                        r = k;
-                                        log_error("Failed to process policy: %s", strerror(-r));
-                                        goto finish;
-                                }
-
                                 k = process_driver(a, b, m, &policy, &ucred);
                                 if (k < 0) {
                                         r = k;
@@ -1453,6 +1428,13 @@ int main(int argc, char *argv[]) {
                                 if (k > 0)
                                         r = k;
                                 else {
+                                        k = process_policy(a, b, m, &policy, &ucred);
+                                        if (k < 0) {
+                                                r = k;
+                                                log_error("Failed to process policy: %s", strerror(-r));
+                                                goto finish;
+                                        }
+
                                         k = sd_bus_send(a, m, NULL);
                                         if (k < 0) {
                                                 if (k == -ECONNRESET)

commit f0a4c7391c7c682b658974b82390d332197740e2
Author: Daniel Mack <daniel at zonque.org>
Date:   Wed Sep 24 17:24:20 2014 +0200

    bus-proxyd: enforce policy for name ownership

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index a6554ab..2f26f81 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -509,7 +509,7 @@ static int peer_is_privileged(sd_bus *bus, sd_bus_message *m) {
         return false;
 }
 
-static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
+static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *policy, const struct ucred *ucred) {
         int r;
 
         assert(a);
@@ -859,6 +859,9 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
                 if (r < 0)
                         return synthetic_reply_method_errno(m, r, NULL);
 
+                if (!policy_check_own(policy, ucred, name))
+                        return synthetic_reply_method_errno(m, -EPERM, NULL);
+
                 if (!service_name_is_valid(name))
                         return synthetic_reply_method_errno(m, -EINVAL, NULL);
                 if ((flags & ~(BUS_NAME_ALLOW_REPLACEMENT|BUS_NAME_REPLACE_EXISTING|BUS_NAME_DO_NOT_QUEUE)) != 0)
@@ -1440,7 +1443,7 @@ int main(int argc, char *argv[]) {
                                         goto finish;
                                 }
 
-                                k = process_driver(a, b, m);
+                                k = process_driver(a, b, m, &policy, &ucred);
                                 if (k < 0) {
                                         r = k;
                                         log_error("Failed to process driver calls: %s", strerror(-r));

commit 8573b68fecc65a0cd285e4c5e288831856948e62
Author: Daniel Mack <daniel at zonque.org>
Date:   Wed Sep 24 17:18:35 2014 +0200

    bus-proxyd: enforce policy for Hello messages

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index aaa7924..a6554ab 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -997,7 +997,7 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
         }
 }
 
-static int process_hello(sd_bus *a, sd_bus *b, sd_bus_message *m, bool *got_hello) {
+static int process_hello(sd_bus *a, sd_bus *b, sd_bus_message *m, Policy *policy, const struct ucred *ucred, bool *got_hello) {
         _cleanup_bus_message_unref_ sd_bus_message *n = NULL;
         bool is_hello;
         int r;
@@ -1029,6 +1029,11 @@ static int process_hello(sd_bus *a, sd_bus *b, sd_bus_message *m, bool *got_hell
                 return -EIO;
         }
 
+        if (!policy_check_hello(policy, ucred)) {
+                log_error("Policy denied HELLO");
+                return -EPERM;
+        }
+
         *got_hello = true;
 
         if (!a->is_kernel)
@@ -1418,7 +1423,7 @@ int main(int argc, char *argv[]) {
                                 goto finish;
                         }
 
-                        k = process_hello(a, b, m, &got_hello);
+                        k = process_hello(a, b, m, &policy, &ucred, &got_hello);
                         if (k < 0) {
                                 r = k;
                                 log_error("Failed to process HELLO: %s", strerror(-r));

commit ac4eaf6dd4e314515f3595c2838b2da3231fa357
Author: Daniel Mack <daniel at zonque.org>
Date:   Wed Sep 24 17:10:31 2014 +0200

    bus-proxyd: keep track of names acquired by legacy client
    
    Store names successfully acquired by the legacy client into a hashmap.
    We need to take these names into account when checking for send policies.

diff --git a/src/bus-proxyd/bus-proxyd.c b/src/bus-proxyd/bus-proxyd.c
index 88346ed..aaa7924 100644
--- a/src/bus-proxyd/bus-proxyd.c
+++ b/src/bus-proxyd/bus-proxyd.c
@@ -51,6 +51,8 @@ static char *arg_command_line_buffer = NULL;
 static bool arg_drop_privileges = false;
 static char **arg_configuration = NULL;
 
+static Hashmap *names_hash = NULL;
+
 static int help(void) {
 
         printf("%s [OPTIONS...]\n\n"
@@ -837,6 +839,8 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
                         return synthetic_reply_method_errno(m, r, NULL);
                 }
 
+                hashmap_remove(names_hash, name);
+
                 return synthetic_reply_method_return(m, "u", BUS_NAME_RELEASED);
 
         } else if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "ReloadConfig")) {
@@ -849,6 +853,7 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
         } else if (sd_bus_message_is_method_call(m, "org.freedesktop.DBus", "RequestName")) {
                 const char *name;
                 uint32_t flags, param;
+                bool in_queue;
 
                 r = sd_bus_message_read(m, "su", &name, &flags);
                 if (r < 0)
@@ -876,7 +881,13 @@ static int process_driver(sd_bus *a, sd_bus *b, sd_bus_message *m) {
                         return synthetic_reply_method_errno(m, r, NULL);
                 }
 
-                if (r == 0)
+                in_queue = (r == 0);
+
+                r = hashmap_put(names_hash, name, NULL);
+                if (r < 0)
+                        return synthetic_reply_method_errno(m, r, NULL);
+
+                if (in_queue)
                         return synthetic_reply_method_return(m, "u", BUS_NAME_IN_QUEUE);
 
                 return synthetic_reply_method_return(m, "u", BUS_NAME_PRIMARY_OWNER);
@@ -1186,6 +1197,12 @@ int main(int argc, char *argv[]) {
                         goto finish;
         }
 
+        names_hash = hashmap_new(&string_hash_ops);
+        if (!names_hash) {
+                log_oom();
+                goto finish;
+        }
+
         r = sd_bus_new(&a);
         if (r < 0) {
                 log_error("Failed to allocate bus: %s", strerror(-r));
@@ -1514,6 +1531,7 @@ finish:
 
         policy_free(&policy);
         strv_free(arg_configuration);
+        hashmap_free(names_hash);
         free(arg_address);
 
         return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;

commit 022fb8558e797483709ab3e9fe846f04f7026dac
Author: Daniel Mack <daniel at zonque.org>
Date:   Wed Oct 22 14:41:53 2014 +0200

    sd-bus: add sd_bus_message.verify_destination_id and .destination_ptr
    
    kdbus learned to accept both a numerical destination ID as well as a
    well-known-name. In that case, kdbus makes sure that the numerical ID is in
    fact the owner of the provided name and fails otherwise.
    
    This allows for race-free assertion of a bus name owner while sending a
    message, which is a requirement for bus-proxyd.
    
    Add two new fields to sd_bus_message, and set the numerical ID to
    verify_destination_id if bus_message_setup_kmsg() is called for a
    message with a well-known name.
    
    Also, set the destination's name in the kdbus item to .destination_ptr
    if it is non-NULL.
    
    Normal users should not touch these fields, and they're not publicy
    accessible.

diff --git a/src/libsystemd/sd-bus/bus-kernel.c b/src/libsystemd/sd-bus/bus-kernel.c
index 153abee..5e7bc12 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -204,6 +204,7 @@ static int bus_message_setup_bloom(sd_bus_message *m, struct kdbus_bloom_filter
 static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
         struct bus_body_part *part;
         struct kdbus_item *d;
+        const char *destination;
         bool well_known;
         uint64_t unique;
         size_t sz, dl;
@@ -219,8 +220,10 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
         if (m->kdbus)
                 return 0;
 
-        if (m->destination) {
-                r = bus_kernel_parse_unique_name(m->destination, &unique);
+        destination = m->destination ?: m->destination_ptr;
+
+        if (destination) {
+                r = bus_kernel_parse_unique_name(destination, &unique);
                 if (r < 0)
                         return r;
 
@@ -244,7 +247,7 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
 
         /* Add in well-known destination header */
         if (well_known) {
-                dl = strlen(m->destination);
+                dl = strlen(destination);
                 sz += ALIGN8(offsetof(struct kdbus_item, str) + dl + 1);
         }
 
@@ -264,9 +267,17 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
         m->kdbus->flags =
                 ((m->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED) ? 0 : KDBUS_MSG_FLAGS_EXPECT_REPLY) |
                 ((m->header->flags & BUS_MESSAGE_NO_AUTO_START) ? KDBUS_MSG_FLAGS_NO_AUTO_START : 0);
-        m->kdbus->dst_id =
-                well_known ? 0 :
-                m->destination ? unique : KDBUS_DST_ID_BROADCAST;
+
+        if (well_known) {
+                /* verify_destination_id will usually be 0, which makes the kernel driver only look
+                 * at the provided well-known name. Otherwise, the kernel will make sure the provided
+                 * destination id matches the owner of the provided weel-known-name, and fail if they
+                 * differ. Currently, this is only needed for bus-proxyd. */
+                m->kdbus->dst_id = m->verify_destination_id;
+        } else {
+                m->kdbus->dst_id = destination ? unique : KDBUS_DST_ID_BROADCAST;
+        }
+
         m->kdbus->payload_type = KDBUS_PAYLOAD_DBUS;
         m->kdbus->cookie = (uint64_t) m->header->serial;
         m->kdbus->priority = m->priority;
@@ -284,7 +295,7 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
         d = m->kdbus->items;
 
         if (well_known)
-                append_destination(&d, m->destination, dl);
+                append_destination(&d, destination, dl);
 
         append_payload_vec(&d, m->header, BUS_MESSAGE_BODY_BEGIN(m));
 
@@ -299,7 +310,7 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) {
                         continue;
                 }
 
-                if (part->memfd >= 0 && part->sealed && m->destination) {
+                if (part->memfd >= 0 && part->sealed && destination) {
                         /* Try to send a memfd, if the part is
                          * sealed and this is not a broadcast. Since we can only  */
 
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index be36d9f..1a8c445 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -148,6 +148,11 @@ static void message_free(sd_bus_message *m) {
         if (m->iovec != m->iovec_fixed)
                 free(m->iovec);
 
+        if (m->destination_ptr) {
+                free(m->destination_ptr);
+                m->destination_ptr = NULL;
+        }
+
         message_reset_containers(m);
         free(m->root_container.signature);
         free(m->root_container.offsets);
diff --git a/src/libsystemd/sd-bus/bus-message.h b/src/libsystemd/sd-bus/bus-message.h
index df79294..8aa71fa 100644
--- a/src/libsystemd/sd-bus/bus-message.h
+++ b/src/libsystemd/sd-bus/bus-message.h
@@ -100,6 +100,7 @@ struct sd_bus_message {
         usec_t realtime;
         uint64_t seqnum;
         int64_t priority;
+        uint64_t verify_destination_id;
 
         bool sealed:1;
         bool dont_send:1;
@@ -143,6 +144,7 @@ struct sd_bus_message {
 
         char sender_buffer[3 + DECIMAL_STR_MAX(uint64_t) + 1];
         char destination_buffer[3 + DECIMAL_STR_MAX(uint64_t) + 1];
+        char *destination_ptr;
 
         size_t header_offsets[_BUS_MESSAGE_HEADER_MAX];
         unsigned n_header_offsets;



More information about the systemd-commits mailing list