[systemd-commits] 3 commits - TODO man/sd_bus_negotiate_fds.xml src/libsystemd src/systemd

Lennart Poettering lennart at kemper.freedesktop.org
Tue Nov 25 17:21:23 PST 2014


 TODO                                    |    1 
 man/sd_bus_negotiate_fds.xml            |   45 +++++++++++++++----------
 src/libsystemd/sd-bus/bus-control.c     |   57 ++++++++++++++++----------------
 src/libsystemd/sd-bus/bus-kernel.c      |   37 +++++++++++++++-----
 src/libsystemd/sd-bus/bus-kernel.h      |    6 ++-
 src/libsystemd/sd-bus/busctl.c          |    2 -
 src/libsystemd/sd-bus/sd-bus.c          |   38 +++++++++++++++++----
 src/libsystemd/sd-bus/test-bus-kernel.c |    9 +++--
 src/systemd/sd-bus.h                    |    2 -
 9 files changed, 128 insertions(+), 69 deletions(-)

New commits:
commit 1dfac061ced2be5eba2b060045154428d13d8c27
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Nov 26 02:21:15 2014 +0100

    update TODO

diff --git a/TODO b/TODO
index 87fe30a..ab77ebe 100644
--- a/TODO
+++ b/TODO
@@ -315,7 +315,6 @@ Features:
   - see if we can introduce a new sd_bus_get_owner_machine_id() call to retrieve the machine ID of the machine of the bus itself
   - when kdbus does not take our message without memfds, try again with memfds
   - systemd-bus-proxyd needs to enforce good old XML policy
-  - allow updating attach flags during runtime
   - introduce sd_bus_emit_object_added()/sd_bus_emit_object_removed() that automatically includes the build-in interfaces in the list
   - port to sd-resolve for connecting to TCP dbus servers
   - see if we can drop more message validation on the sending side

commit b5dae4c7f77f7c87b91e0afb60a31c690dda4a1f
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Nov 26 02:20:28 2014 +0100

    sd-bus: add suppot for renegotiating message credential attach flags

diff --git a/man/sd_bus_negotiate_fds.xml b/man/sd_bus_negotiate_fds.xml
index 3d7a20b..4fbffdf 100644
--- a/man/sd_bus_negotiate_fds.xml
+++ b/man/sd_bus_negotiate_fds.xml
@@ -70,6 +70,7 @@ along with systemd; If not, see <http://www.gnu.org/licenses/>.
         <funcdef>int <function>sd_bus_negotiate_creds</function></funcdef>
         <paramdef>sd_bus *<parameter>bus</parameter></paramdef>
         <paramdef>int <parameter>b</parameter></paramdef>
+        <paramdef>uint64_t <parameter>flags</parameter></paramdef>
       </funcprototype>
     </funcsynopsis>
   </refsynopsisdiv>
@@ -107,25 +108,34 @@ along with systemd; If not, see <http://www.gnu.org/licenses/>.
     <citerefentry><refentrytitle>sd_bus_message_get_seqno</refentrytitle><manvolnum>3</manvolnum></citerefentry>
     fail with <constant>-ENODATA</constant> on incoming messages. Note
     that not all transports support timestamping of messages. On local
-    transports, the timestamping is applied by the kernel and cannot be
-    manipulated by userspace.</para>
+    transports, the timestamping is applied by the kernel and cannot
+    be manipulated by userspace. By default, message timestamping is
+    not negotiated for all connections.</para>
 
     <para><function>sd_bus_negotiate_creds()</function> controls
     whether implicit sender credentials shall be attached
-    automatically to all incoming messages. Takes a bus object and a
-    bit mask value, which controls which credential parameters are
-    attached. If this is not used,
-    <citerefentry><refentrytitle>sd_bus_message_get_creds</refentrytitle><manvolnum>3</manvolnum></citerefentry>
-    fails with <constant>-ENODATA</constant> on incoming
-    messages. Note that not all transports support attaching sender
-    credentials to messages, or do not support all types of sender
-    credential parameters. On local transports, the sender credentials
-    are attached by the kernel and cannot be manipulated by
-    userspace. By default, no sender credentials are attached.</para>
-
-    <para>These functions may be called only before the connection has
-    been started with
-    <citerefentry><refentrytitle>sd_bus_start</refentrytitle><manvolnum>3</manvolnum></citerefentry>.</para>
+    automatically to all incoming messages. Takes a bus object, a
+    boolean indicating wether to enable or disable the credential
+    parts encoded in the bit mask value argument. Note that not all
+    transports support attaching sender credentials to messages, or do
+    not support all types of sender credential parameters, or might
+    suppress them under certain circumstances for individual
+    messages. On local transports, the sender credentials are attached
+    by the kernel and cannot be manipulated by userspace. By default,
+    no sender credentials are attached.</para>
+
+    <para>The <function>sd_bus_negotiate_fds()</function> function may
+    be called only before the connection has been started with
+    <citerefentry><refentrytitle>sd_bus_start</refentrytitle><manvolnum>3</manvolnum></citerefentry>. Both
+    <function>sd_bus_negotiate_timestamp()</function> and
+    <function>sd_bus_negotiate_creds()</function> also may be called
+    after a connection has been set up. Note that when operating on a
+    connection that is shared between multiple components of the same
+    program (for example via
+    <citerefentry><refentrytitle>sd_bus_default</refentrytitle><manvolnum>3</manvolnum></citerefentry>)
+    it is highly recommended to only enable additional per message
+    metadata fields, but never disable them again, in order not to
+    disable functionality needed by other components.</para>
   </refsect1>
 
   <refsect1>
@@ -169,7 +179,8 @@ along with systemd; If not, see <http://www.gnu.org/licenses/>.
       <citerefentry><refentrytitle>sd_bus_start</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>sd_bus_message_can_send</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
       <citerefentry><refentrytitle>sd_bus_message_get_monotonic_usec</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
-      <citerefentry><refentrytitle>sd_bus_message_get_creds</refentrytitle><manvolnum>3</manvolnum></citerefentry>
+      <citerefentry><refentrytitle>sd_bus_message_get_creds</refentrytitle><manvolnum>3</manvolnum></citerefentry>,
+      <citerefentry><refentrytitle>systemd.busname</refentrytitle><manvolnum>5</manvolnum></citerefentry>
     </para>
   </refsect1>
 
diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c
index 0ebaf85..9cd5cd5 100644
--- a/src/libsystemd/sd-bus/bus-control.c
+++ b/src/libsystemd/sd-bus/bus-control.c
@@ -62,7 +62,7 @@ static int bus_request_name_kernel(sd_bus *bus, const char *name, uint64_t flags
         size = offsetof(struct kdbus_cmd_name, items) + KDBUS_ITEM_SIZE(l);
         n = alloca0_align(size, 8);
         n->size = size;
-        kdbus_translate_request_name_flags(flags, (uint64_t *) &n->flags);
+        n->flags = request_name_flags_to_kdbus(flags);
 
         n->items[0].size = KDBUS_ITEM_HEADER_SIZE + l;
         n->items[0].type = KDBUS_ITEM_NAME;
@@ -643,7 +643,7 @@ static int bus_get_name_creds_kdbus(
         }
 
         cmd->size = size;
-        kdbus_translate_attach_flags(mask, (uint64_t*) &cmd->flags);
+        cmd->flags = attach_flags_to_kdbus(mask);
 
         /* If augmentation is on, and the bus doesn't didn't allow us
          * to get the bits we want, then ask for the PID/TID so that we
@@ -927,7 +927,7 @@ _public_ int sd_bus_get_owner_creds(sd_bus *bus, uint64_t mask, sd_bus_creds **r
                 struct kdbus_info *creator_info;
 
                 cmd.size = sizeof(cmd);
-                kdbus_translate_attach_flags(mask, (uint64_t*) &cmd.flags);
+                cmd.flags = attach_flags_to_kdbus(mask);
 
                 /* If augmentation is on, and the bus doesn't didn't allow us
                  * to get the bits we want, then ask for the PID/TID so that we
diff --git a/src/libsystemd/sd-bus/bus-kernel.c b/src/libsystemd/sd-bus/bus-kernel.c
index e03e447..d0cb7ee 100644
--- a/src/libsystemd/sd-bus/bus-kernel.c
+++ b/src/libsystemd/sd-bus/bus-kernel.c
@@ -1313,11 +1313,9 @@ void bus_kernel_flush_memfd(sd_bus *b) {
                 close_and_munmap(b->memfd_cache[i].fd, b->memfd_cache[i].address, b->memfd_cache[i].mapped);
 }
 
-int kdbus_translate_request_name_flags(uint64_t flags, uint64_t *kdbus_flags) {
+uint64_t request_name_flags_to_kdbus(uint64_t flags) {
         uint64_t f = 0;
 
-        assert(kdbus_flags);
-
         if (flags & SD_BUS_NAME_ALLOW_REPLACEMENT)
                 f |= KDBUS_NAME_ALLOW_REPLACEMENT;
 
@@ -1327,15 +1325,12 @@ int kdbus_translate_request_name_flags(uint64_t flags, uint64_t *kdbus_flags) {
         if (flags & SD_BUS_NAME_QUEUE)
                 f |= KDBUS_NAME_QUEUE;
 
-        *kdbus_flags = f;
-        return 0;
+        return f;
 }
 
-int kdbus_translate_attach_flags(uint64_t mask, uint64_t *kdbus_mask) {
+uint64_t attach_flags_to_kdbus(uint64_t mask) {
         uint64_t m = 0;
 
-        assert(kdbus_mask);
-
         if (mask & (SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_SUID|SD_BUS_CREDS_FSUID|
                     SD_BUS_CREDS_GID|SD_BUS_CREDS_EGID|SD_BUS_CREDS_SGID|SD_BUS_CREDS_FSGID))
                 m |= KDBUS_ATTACH_CREDS;
@@ -1376,8 +1371,7 @@ int kdbus_translate_attach_flags(uint64_t mask, uint64_t *kdbus_mask) {
         if (mask & SD_BUS_CREDS_SUPPLEMENTARY_GIDS)
                 m |= KDBUS_ATTACH_AUXGROUPS;
 
-        *kdbus_mask = m;
-        return 0;
+        return m;
 }
 
 int bus_kernel_create_bus(const char *name, bool world, char **s) {
@@ -1713,3 +1707,26 @@ int bus_kernel_drop_one(int fd) {
 
         return 0;
 }
+
+int bus_kernel_realize_attach_flags(sd_bus *bus) {
+        struct kdbus_cmd_update *update;
+        struct kdbus_item *n;
+
+        assert(bus);
+        assert(bus->is_kernel);
+
+        update = alloca0_align(ALIGN8(offsetof(struct kdbus_cmd_update, items) +
+                                      offsetof(struct kdbus_item, data64) + sizeof(uint64_t)), 8);
+
+        n = update->items;
+        n->type = KDBUS_ITEM_ATTACH_FLAGS_RECV;
+        n->size = offsetof(struct kdbus_item, data64) + sizeof(uint64_t);
+        n->data64[0] = bus->attach_flags;
+
+        update->size = offsetof(struct kdbus_cmd_update, items) + n->size;
+
+        if (ioctl(bus->input_fd, KDBUS_CMD_CONN_UPDATE, update) < 0)
+                return -errno;
+
+        return 0;
+}
diff --git a/src/libsystemd/sd-bus/bus-kernel.h b/src/libsystemd/sd-bus/bus-kernel.h
index c1ee285..8994b35 100644
--- a/src/libsystemd/sd-bus/bus-kernel.h
+++ b/src/libsystemd/sd-bus/bus-kernel.h
@@ -81,9 +81,11 @@ void bus_kernel_flush_memfd(sd_bus *bus);
 
 int bus_kernel_parse_unique_name(const char *s, uint64_t *id);
 
-int kdbus_translate_request_name_flags(uint64_t sd_bus_flags, uint64_t *kdbus_flags);
-int kdbus_translate_attach_flags(uint64_t sd_bus_flags, uint64_t *kdbus_flags);
+uint64_t request_name_flags_to_kdbus(uint64_t sd_bus_flags);
+uint64_t attach_flags_to_kdbus(uint64_t sd_bus_flags);
 
 int bus_kernel_try_close(sd_bus *bus);
 
 int bus_kernel_drop_one(int fd);
+
+int bus_kernel_realize_attach_flags(sd_bus *bus);
diff --git a/src/libsystemd/sd-bus/busctl.c b/src/libsystemd/sd-bus/busctl.c
index 50291ba..e432db4 100644
--- a/src/libsystemd/sd-bus/busctl.c
+++ b/src/libsystemd/sd-bus/busctl.c
@@ -1975,7 +1975,7 @@ int main(int argc, char *argv[]) {
                         goto finish;
                 }
 
-                r = sd_bus_negotiate_creds(bus, _SD_BUS_CREDS_ALL);
+                r = sd_bus_negotiate_creds(bus, true, _SD_BUS_CREDS_ALL);
                 if (r < 0) {
                         log_error("Failed to enable credentials: %s", strerror(-r));
                         goto finish;
diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c
index 5aa8eac..6b01c0c 100644
--- a/src/libsystemd/sd-bus/sd-bus.c
+++ b/src/libsystemd/sd-bus/sd-bus.c
@@ -274,24 +274,50 @@ _public_ int sd_bus_negotiate_fds(sd_bus *bus, int b) {
 }
 
 _public_ int sd_bus_negotiate_timestamp(sd_bus *bus, int b) {
+        uint64_t new_flags;
         assert_return(bus, -EINVAL);
-        assert_return(bus->state == BUS_UNSET, -EPERM);
+        assert_return(!IN_SET(bus->state, BUS_CLOSING, BUS_CLOSED), -EPERM);
         assert_return(!bus_pid_changed(bus), -ECHILD);
 
-        SET_FLAG(bus->attach_flags, KDBUS_ATTACH_TIMESTAMP, b);
+        new_flags = bus->attach_flags;
+        SET_FLAG(new_flags, KDBUS_ATTACH_TIMESTAMP, b);
+
+        if (bus->attach_flags == new_flags)
+                return 0;
+
+        bus->attach_flags = new_flags;
+        if (bus->state != BUS_UNSET && bus->is_kernel)
+                bus_kernel_realize_attach_flags(bus);
+
         return 0;
 }
 
-_public_ int sd_bus_negotiate_creds(sd_bus *bus, uint64_t mask) {
+_public_ int sd_bus_negotiate_creds(sd_bus *bus, int b, uint64_t mask) {
+        uint64_t new_flags;
+
         assert_return(bus, -EINVAL);
         assert_return(mask <= _SD_BUS_CREDS_ALL, -EINVAL);
-        assert_return(bus->state == BUS_UNSET, -EPERM);
+        assert_return(!IN_SET(bus->state, BUS_CLOSING, BUS_CLOSED), -EPERM);
         assert_return(!bus_pid_changed(bus), -ECHILD);
 
+        if (b)
+                bus->creds_mask |= mask;
+        else
+                bus->creds_mask &= ~mask;
+
         /* The well knowns we need unconditionally, so that matches can work */
-        bus->creds_mask = mask | SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME;
+        bus->creds_mask |= SD_BUS_CREDS_WELL_KNOWN_NAMES|SD_BUS_CREDS_UNIQUE_NAME;
+
+        /* Make sure we don't lose the timestamp flag */
+        new_flags = (bus->attach_flags & KDBUS_ATTACH_TIMESTAMP) | attach_flags_to_kdbus(bus->creds_mask);
+        if (bus->attach_flags == new_flags)
+                return 0;
+
+        bus->attach_flags = new_flags;
+        if (bus->state != BUS_UNSET && bus->is_kernel)
+                bus_kernel_realize_attach_flags(bus);
 
-        return kdbus_translate_attach_flags(bus->creds_mask, &bus->attach_flags);
+        return 0;
 }
 
 _public_ int sd_bus_set_server(sd_bus *bus, int b, sd_id128_t server_id) {
diff --git a/src/libsystemd/sd-bus/test-bus-kernel.c b/src/libsystemd/sd-bus/test-bus-kernel.c
index 9cc0f01..0e6c2ac 100644
--- a/src/libsystemd/sd-bus/test-bus-kernel.c
+++ b/src/libsystemd/sd-bus/test-bus-kernel.c
@@ -70,10 +70,10 @@ int main(int argc, char *argv[]) {
         assert_se(r >= 0);
 
         assert_se(sd_bus_negotiate_timestamp(a, 1) >= 0);
-        assert_se(sd_bus_negotiate_creds(a, _SD_BUS_CREDS_ALL) >= 0);
+        assert_se(sd_bus_negotiate_creds(a, true, _SD_BUS_CREDS_ALL) >= 0);
 
-        assert_se(sd_bus_negotiate_timestamp(b, 1) >= 0);
-        assert_se(sd_bus_negotiate_creds(b, _SD_BUS_CREDS_ALL) >= 0);
+        assert_se(sd_bus_negotiate_timestamp(b, 0) >= 0);
+        assert_se(sd_bus_negotiate_creds(b, true, 0) >= 0);
 
         r = sd_bus_start(a);
         assert_se(r >= 0);
@@ -81,6 +81,9 @@ int main(int argc, char *argv[]) {
         r = sd_bus_start(b);
         assert_se(r >= 0);
 
+        assert_se(sd_bus_negotiate_timestamp(b, 1) >= 0);
+        assert_se(sd_bus_negotiate_creds(b, true, _SD_BUS_CREDS_ALL) >= 0);
+
         r = sd_bus_get_unique_name(a, &ua);
         assert_se(r >= 0);
         printf("unique a: %s\n", ua);
diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h
index cb53a32..28c7ee4 100644
--- a/src/systemd/sd-bus.h
+++ b/src/systemd/sd-bus.h
@@ -127,7 +127,7 @@ int sd_bus_set_description(sd_bus *bus, const char *description);
 int sd_bus_set_monitor(sd_bus *bus, int b);
 int sd_bus_negotiate_fds(sd_bus *bus, int b);
 int sd_bus_negotiate_timestamp(sd_bus *bus, int b);
-int sd_bus_negotiate_creds(sd_bus *bus, uint64_t creds_mask);
+int sd_bus_negotiate_creds(sd_bus *bus, int b, uint64_t creds_mask);
 int sd_bus_start(sd_bus *ret);
 
 int sd_bus_try_close(sd_bus *bus);

commit f3c0588651927ebac691130aa861b878fa22e527
Author: Lennart Poettering <lennart at poettering.net>
Date:   Wed Nov 26 01:45:14 2014 +0100

    sd-bus: use free_and_strdup() where appropriate
    
    This simplifies things a bit and makes sure we free any previously set
    creds component before writing in a new one.

diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c
index 23fb0e7..0ebaf85 100644
--- a/src/libsystemd/sd-bus/bus-control.c
+++ b/src/libsystemd/sd-bus/bus-control.c
@@ -392,10 +392,11 @@ _public_ int sd_bus_list_names(sd_bus *bus, char ***acquired, char ***activatabl
                 return bus_list_names_dbus1(bus, acquired, activatable);
 }
 
-static int bus_populate_creds_from_items(sd_bus *bus,
-                                         struct kdbus_info *info,
-                                         uint64_t mask,
-                                         sd_bus_creds *c) {
+static int bus_populate_creds_from_items(
+                sd_bus *bus,
+                struct kdbus_info *info,
+                uint64_t mask,
+                sd_bus_creds *c) {
 
         struct kdbus_item *item;
         uint64_t m;
@@ -470,9 +471,9 @@ static int bus_populate_creds_from_items(sd_bus *bus,
 
                 case KDBUS_ITEM_PID_COMM:
                         if (mask & SD_BUS_CREDS_COMM) {
-                                c->comm = strdup(item->str);
-                                if (!c->comm)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->comm, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= SD_BUS_CREDS_COMM;
                         }
@@ -480,9 +481,9 @@ static int bus_populate_creds_from_items(sd_bus *bus,
 
                 case KDBUS_ITEM_TID_COMM:
                         if (mask & SD_BUS_CREDS_TID_COMM) {
-                                c->tid_comm = strdup(item->str);
-                                if (!c->tid_comm)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->tid_comm, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= SD_BUS_CREDS_TID_COMM;
                         }
@@ -490,9 +491,9 @@ static int bus_populate_creds_from_items(sd_bus *bus,
 
                 case KDBUS_ITEM_EXE:
                         if (mask & SD_BUS_CREDS_EXE) {
-                                c->exe = strdup(item->str);
-                                if (!c->exe)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->exe, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= SD_BUS_CREDS_EXE;
                         }
@@ -515,17 +516,17 @@ static int bus_populate_creds_from_items(sd_bus *bus,
                              SD_BUS_CREDS_SESSION | SD_BUS_CREDS_OWNER_UID) & mask;
 
                         if (m) {
-                                c->cgroup = strdup(item->str);
-                                if (!c->cgroup)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->cgroup, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 r = bus_get_root_path(bus);
                                 if (r < 0)
                                         return r;
 
-                                c->cgroup_root = strdup(bus->cgroup_root);
-                                if (!c->cgroup_root)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->cgroup_root, bus->cgroup_root);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= m;
                         }
@@ -547,9 +548,9 @@ static int bus_populate_creds_from_items(sd_bus *bus,
 
                 case KDBUS_ITEM_SECLABEL:
                         if (mask & SD_BUS_CREDS_SELINUX_CONTEXT) {
-                                c->label = strdup(item->str);
-                                if (!c->label)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->label, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= SD_BUS_CREDS_SELINUX_CONTEXT;
                         }
@@ -579,9 +580,9 @@ static int bus_populate_creds_from_items(sd_bus *bus,
 
                 case KDBUS_ITEM_CONN_DESCRIPTION:
                         if (mask & SD_BUS_CREDS_DESCRIPTION) {
-                                c->description = strdup(item->str);
-                                if (!c->description)
-                                        return -ENOMEM;
+                                r = free_and_strdup(&c->description, item->str);
+                                if (r < 0)
+                                        return r;
 
                                 c->mask |= SD_BUS_CREDS_DESCRIPTION;
                         }



More information about the systemd-commits mailing list