From 7a77d2a41cb651adc3bbbee39dc73acacdfe1d60 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 19 Aug 2019 20:28:34 +0200 Subject: [PATCH] sd-bus: add new call sd_bus_message_sensitive() and SD_BUS_VTABLE_SENSITIVE 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 | 1 + src/libsystemd/sd-bus/bus-message.c | 37 ++++++++++++++++++++++++----- src/libsystemd/sd-bus/bus-message.h | 1 + src/libsystemd/sd-bus/bus-objects.c | 24 +++++++++++++++++++ src/systemd/sd-bus-vtable.h | 1 + src/systemd/sd-bus.h | 1 + 6 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/libsystemd.sym b/src/libsystemd/libsystemd.sym index bed81bf173..a3dd9f68b7 100644 --- a/src/libsystemd/libsystemd.sym +++ b/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; diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index eb029e4453..73127dfe02 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/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; +} diff --git a/src/libsystemd/sd-bus/bus-message.h b/src/libsystemd/sd-bus/bus-message.h index ced0bb3d34..a88a531e15 100644 --- a/src/libsystemd/sd-bus/bus-message.h +++ b/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; diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c index ae643cacc7..26f93d21da 100644 --- a/src/libsystemd/sd-bus/bus-objects.c +++ b/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; diff --git a/src/systemd/sd-bus-vtable.h b/src/systemd/sd-bus-vtable.h index 0f43554d82..95b5914236 100644 --- a/src/systemd/sd-bus-vtable.h +++ b/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 }; diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 84ceb62dc7..3c9792e497 100644 --- a/src/systemd/sd-bus.h +++ b/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 */