Merge pull request #14209 from poettering/sd-bus-sensitive

sd-bus bits from homed PR

src/libsystemd/libsystemd.sym

@@ -685,6 +685,7 @@ global:
 
 LIBSYSTEMD_245 {
 global:
+        sd_bus_message_sensitive;
         sd_event_add_child_pidfd;
         sd_event_source_get_child_pidfd;
         sd_event_source_get_child_pidfd_own;

src/libsystemd/sd-bus/bus-message.c

@@ -45,12 +45,24 @@ static void message_free_part(sd_bus_message *m, struct bus_body_part *part) {
         assert(m);
         assert(part);
 
-        if (part->memfd >= 0)
+        if (part->memfd >= 0) {
+                /* erase if requested, but ony if the memfd is not sealed yet, i.e. is writable */
+                if (m->sensitive && !m->sealed)
+                        explicit_bzero_safe(part->data, part->size);
+
                 close_and_munmap(part->memfd, part->mmap_begin, part->mapped);
-        else if (part->munmap_this)
+        } else if (part->munmap_this)
+                /* We don't erase sensitive data here, since the data is memory mapped from someone else, and
+                 * we just don't know if it's OK to write to it */
                 munmap(part->mmap_begin, part->mapped);
-        else if (part->free_this)
-                free(part->data);
+        else {
+                /* Erase this if that is requested. Since this is regular memory we know we can write it. */
+                if (m->sensitive)
+                        explicit_bzero_safe(part->data, part->size);
+
+                if (part->free_this)
+                        free(part->data);
+        }
 
         if (part != &m->body)
                 free(part);
@@ -113,11 +125,11 @@ static void message_reset_containers(sd_bus_message *m) {
 static sd_bus_message* message_free(sd_bus_message *m) {
         assert(m);
 
+        message_reset_parts(m);
+
         if (m->free_header)
                 free(m->header);
 
-        message_reset_parts(m);
-
         /* Note that we don't unref m->bus here. That's already done by sd_bus_message_unref() as each user
          * reference to the bus message also is considered a reference to the bus connection itself. */
 
@@ -727,6 +739,12 @@ static int message_new_reply(
         t->dont_send = !!(call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED);
         t->enforced_reply_signature = call->enforced_reply_signature;
 
+        /* let's copy the sensitive flag over. Let's do that as a safety precaution to keep a transaction
+         * wholly sensitive if already the incoming message was sensitive. This is particularly useful when a
+         * vtable record sets the SD_BUS_VTABLE_SENSITIVE flag on a method call, since this means it applies
+         * to both the message call and the reply. */
+        t->sensitive = call->sensitive;
+
         *m = TAKE_PTR(t);
         return 0;
 }
@@ -5919,3 +5937,10 @@ _public_ int sd_bus_message_set_priority(sd_bus_message *m, int64_t priority) {
         m->priority = priority;
         return 0;
 }
+
+_public_ int sd_bus_message_sensitive(sd_bus_message *m) {
+        assert_return(m, -EINVAL);
+
+        m->sensitive = true;
+        return 0;
+}

src/libsystemd/sd-bus/bus-message.h

@@ -85,6 +85,7 @@ struct sd_bus_message {
         bool free_header:1;
         bool free_fds:1;
         bool poisoned:1;
+        bool sensitive:1;
 
         /* The first and last bytes of the message */
         struct bus_header *header;

src/libsystemd/sd-bus/bus-objects.c

@@ -353,6 +353,12 @@ static int method_callbacks_run(
         if (require_fallback && !c->parent->is_fallback)
                 return 0;
 
+        if (FLAGS_SET(c->vtable->flags, SD_BUS_VTABLE_SENSITIVE)) {
+                r = sd_bus_message_sensitive(m);
+                if (r < 0)
+                        return r;
+        }
+
         r = check_access(bus, m, c, &error);
         if (r < 0)
                 return bus_maybe_reply_error(m, r, &error);
@@ -577,6 +583,12 @@ static int property_get_set_callbacks_run(
         if (require_fallback && !c->parent->is_fallback)
                 return 0;
 
+        if (FLAGS_SET(c->vtable->flags, SD_BUS_VTABLE_SENSITIVE)) {
+                r = sd_bus_message_sensitive(m);
+                if (r < 0)
+                        return r;
+        }
+
         r = vtable_property_get_userdata(bus, m->path, c, &u, &error);
         if (r <= 0)
                 return bus_maybe_reply_error(m, r, &error);
@@ -591,6 +603,12 @@ static int property_get_set_callbacks_run(
         if (r < 0)
                 return r;
 
+        if (FLAGS_SET(c->vtable->flags, SD_BUS_VTABLE_SENSITIVE)) {
+                r = sd_bus_message_sensitive(reply);
+                if (r < 0)
+                        return r;
+        }
+
         if (is_get) {
                 /* Note that we do not protect against reexecution
                  * here (using the last_iteration check, see below),
@@ -692,6 +710,12 @@ static int vtable_append_one_property(
         assert(c);
         assert(v);
 
+        if (FLAGS_SET(c->vtable->flags, SD_BUS_VTABLE_SENSITIVE)) {
+                r = sd_bus_message_sensitive(reply);
+                if (r < 0)
+                        return r;
+        }
+
         r = sd_bus_message_open_container(reply, 'e', "sv");
         if (r < 0)
                 return r;
@@ -750,9 +774,18 @@ static int vtable_append_all_properties(
                 if (v->flags & SD_BUS_VTABLE_HIDDEN)
                         continue;
 
+                /* Let's not include properties marked as "explicit" in any message that contians a generic
+                 * dump of properties, but only in those generated as a response to an explicit request. */
                 if (v->flags & SD_BUS_VTABLE_PROPERTY_EXPLICIT)
                         continue;
 
+                /* Let's not include properties marked only for invalidation on change (i.e. in contrast to
+                 * those whose new values are included in PropertiesChanges message) in any signals. This is
+                 * useful to ensure they aren't included in InterfacesAdded messages. */
+                if (reply->header->type != SD_BUS_MESSAGE_METHOD_RETURN &&
+                    FLAGS_SET(v->flags, SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION))
+                        continue;
+
                 r = vtable_append_one_property(bus, reply, path, c, v, userdata, error);
                 if (r < 0)
                         return r;

src/systemd/sd-bus-vtable.h

@@ -43,6 +43,7 @@ enum {
         SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE        = 1ULL << 5,
         SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION  = 1ULL << 6,
         SD_BUS_VTABLE_PROPERTY_EXPLICIT            = 1ULL << 7,
+        SD_BUS_VTABLE_SENSITIVE                    = 1ULL << 8, /* covers both directions: method call + reply */
         _SD_BUS_VTABLE_CAPABILITY_MASK             = 0xFFFFULL << 40
 };
 

src/systemd/sd-bus.h

@@ -328,6 +328,7 @@ int sd_bus_message_peek_type(sd_bus_message *m, char *type, const char **content
 int sd_bus_message_verify_type(sd_bus_message *m, char type, const char *contents);
 int sd_bus_message_at_end(sd_bus_message *m, int complete);
 int sd_bus_message_rewind(sd_bus_message *m, int complete);
+int sd_bus_message_sensitive(sd_bus_message *m);
 
 /* Bus management */