]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
sd-bus: add new call sd_bus_message_sensitive() and SD_BUS_VTABLE_SENSITIVE
authorLennart Poettering <lennart@poettering.net>
Mon, 19 Aug 2019 18:28:34 +0000 (20:28 +0200)
committerLennart Poettering <lennart@poettering.net>
Wed, 4 Dec 2019 12:46:23 +0000 (13:46 +0100)
This allows marking messages that contain "sensitive" data with a flag.
If it's set then the messages are erased from memory when the message is
freed.

Similar, a flag may be set on vtable entries: incoming/outgoing message
matching the entry will then automatically be flagged this way.

This is supposed to be an easy method to mark messages containing
potentially sensitive data (such as passwords) for proper destruction.

(Note that this of course is only is as safe as the broker in between is
doing something similar. But let's at least not be the ones at fault
here.)

src/libsystemd/libsystemd.sym
src/libsystemd/sd-bus/bus-message.c
src/libsystemd/sd-bus/bus-message.h
src/libsystemd/sd-bus/bus-objects.c
src/systemd/sd-bus-vtable.h
src/systemd/sd-bus.h

index bed81bf17321cc953421fefc55480b3b36392afd..a3dd9f68b7d38c0a1b6da69c25b8261f88a0d6d9 100644 (file)
@@ -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;
index eb029e44532688e8adf38195b62e2c6146d5fc68..73127dfe0253c85f569cdad59e67ca930a70bae6 100644 (file)
@@ -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;
+}
index ced0bb3d34a91ea1764b70669ba5376648806d03..a88a531e15b82d8b5050f16c7cf61e8cd3b8bbcc 100644 (file)
@@ -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;
index ae643cacc740f5e30e82c4452aa566aeae57a60c..26f93d21dab640af93f650e2219d4d93f4840902 100644 (file)
@@ -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;
index 0f43554d825190d4f53f6b3e0bd88991fbb5b4c0..95b5914236a478b6eeb7028b4c2002dc4c71b65d 100644 (file)
@@ -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
 };
 
index 84ceb62dc79c7ef670ca0173bb8f68a4aecb89b2..3c9792e4975e235fc750690f61997316c6907dce 100644 (file)
@@ -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 */