From e2a58f775a698bc40d758122a19effd6ba6cea6e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 21 Jul 2017 20:09:15 +0100 Subject: [PATCH] bus/containers: Create a DBusServer and add it to the main loop This means we can accept connections on the new socket. For now, we don't process them and they get closed. For the system bus (or root's session bus, where the difference is harmless but makes automated testing easier), rely on system-wide infrastructure to create /run/dbus/containers. At the moment this is only done for systemd systems via tmpfiles.d; I'd accept a tested patch for our Red Hat, Slackware and Cygwin init scripts to do the same, but I suspect they might actually be unused and unmaintained (see Bug #101706). Signed-off-by: Simon McVittie --- Make more use of dbus_clear_whatever https://bugs.freedesktop.org/show_bug.cgi?id=101354 --- bus/Makefile.am | 1 + bus/bus.c | 35 ++++-- bus/bus.h | 3 + bus/containers.c | 293 +++++++++++++++++++++++++++++++++++++++++++- bus/containers.h | 1 + bus/tmpfiles.d/dbus.conf.in | 4 + dbus/dbus-internals.h | 1 + 7 files changed, 323 insertions(+), 15 deletions(-) diff --git a/bus/Makefile.am b/bus/Makefile.am index c7dd24e7..4780c8b4 100644 --- a/bus/Makefile.am +++ b/bus/Makefile.am @@ -29,6 +29,7 @@ AM_CPPFLAGS = \ $(EXPAT_CFLAGS) \ $(APPARMOR_CFLAGS) \ -DDBUS_SYSTEM_CONFIG_FILE=\""$(dbusdatadir)/system.conf"\" \ + -DDBUS_RUNSTATEDIR=\""$(runstatedir)"\" \ -DDBUS_COMPILATION \ $(NULL) diff --git a/bus/bus.c b/bus/bus.c index 5d64c02b..23c22ce6 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -99,7 +99,8 @@ server_get_context (DBusServer *server) bd = BUS_SERVER_DATA (server); - /* every DBusServer in the dbus-daemon has gone through setup_server() */ + /* every DBusServer in the dbus-daemon's main loop has gone through + * bus_context_setup_server() */ _dbus_assert (bd != NULL); context = bd->context; @@ -218,6 +219,25 @@ setup_server (BusContext *context, char **auth_mechanisms, DBusError *error) { + if (!bus_context_setup_server (context, server, error)) + return FALSE; + + if (!dbus_server_set_auth_mechanisms (server, (const char**) auth_mechanisms)) + { + BUS_SET_OOM (error); + return FALSE; + } + + dbus_server_set_new_connection_function (server, new_connection_callback, + context, NULL); + return TRUE; +} + +dbus_bool_t +bus_context_setup_server (BusContext *context, + DBusServer *server, + DBusError *error) +{ BusServerData *bd; bd = dbus_new0 (BusServerData, 1); @@ -232,16 +252,6 @@ setup_server (BusContext *context, bd->context = context; - if (!dbus_server_set_auth_mechanisms (server, (const char**) auth_mechanisms)) - { - BUS_SET_OOM (error); - return FALSE; - } - - dbus_server_set_new_connection_function (server, - new_connection_callback, - context, NULL); - if (!dbus_server_set_watch_functions (server, add_server_watch, remove_server_watch, @@ -1110,6 +1120,9 @@ bus_context_shutdown (BusContext *context) link = _dbus_list_get_next_link (&context->servers, link); } + + if (context->containers != NULL) + bus_containers_stop_listening (context->containers); } BusContext * diff --git a/bus/bus.h b/bus/bus.h index 5a531ed0..c382575c 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -147,5 +147,8 @@ dbus_bool_t bus_context_check_security_policy (BusContext BusActivationEntry *activation_entry, DBusError *error); void bus_context_check_all_watches (BusContext *context); +dbus_bool_t bus_context_setup_server (BusContext *context, + DBusServer *server, + DBusError *error); #endif /* BUS_BUS_H */ diff --git a/bus/containers.c b/bus/containers.c index fac5c8e0..69dfbf0c 100644 --- a/bus/containers.c +++ b/bus/containers.c @@ -51,6 +51,7 @@ typedef struct DBusVariant *metadata; BusContext *context; BusContainers *containers; + DBusServer *server; } BusContainerInstance; /* @@ -63,6 +64,7 @@ struct BusContainers /* path borrowed from BusContainerInstance => unowned BusContainerInstance * The BusContainerInstance removes itself from here on destruction. */ DBusHashTable *instances_by_path; + DBusString address_template; dbus_uint64_t next_container_id; }; @@ -74,12 +76,51 @@ bus_containers_new (void) BusContainers *self = dbus_new0 (BusContainers, 1); if (self == NULL) - return NULL; + goto oom; self->refcount = 1; self->instances_by_path = NULL; self->next_container_id = DBUS_UINT64_CONSTANT (0); + /* So we can safely call _dbus_free on it, even before _dbus_string_init() */ + _dbus_string_init_const (&self->address_template, ""); + + /* Make it mutable */ + if (!_dbus_string_init (&self->address_template)) + goto oom; + + if (_dbus_getuid () == 0) + { + DBusString dir; + + /* System bus (we haven't dropped privileges at this point), or + * root's session bus. Use random socket paths resembling + * /run/dbus/containers/dbus-abcdef, which is next to /run/dbus/pid + * (if not using the Red Hat init scripts, which use a different + * pid file for historical reasons). + * + * We rely on the tmpfiles.d snippet or an OS-specific init script to + * have created this directory with the appropriate owner; if it hasn't, + * creating container sockets will just fail. */ + _dbus_string_init_const (&dir, DBUS_RUNSTATEDIR "/dbus/containers"); + + /* We specifically use paths, because an abstract socket that you can't + * bind-mount is not particularly useful. */ + if (!_dbus_string_append (&self->address_template, "unix:dir=") || + !_dbus_address_append_escaped (&self->address_template, &dir)) + goto oom; + } + else + { + /* Otherwise defer creating the directory for sockets until we need it, + * so that we can have better error behaviour */ + } + return self; + +oom: + bus_clear_containers (&self); + + return NULL; } BusContainers * @@ -101,10 +142,20 @@ bus_containers_unref (BusContainers *self) if (--self->refcount == 0) { _dbus_clear_hash_table (&self->instances_by_path); + _dbus_string_free (&self->address_template); dbus_free (self); } } +static BusContainerInstance * +bus_container_instance_ref (BusContainerInstance *self) +{ + _dbus_assert (self->refcount > 0); + + self->refcount++; + return self; +} + static void bus_container_instance_unref (BusContainerInstance *self) { @@ -112,6 +163,11 @@ bus_container_instance_unref (BusContainerInstance *self) if (--self->refcount == 0) { + /* As long as the server is listening, the BusContainerInstance can't + * be freed, because the DBusServer holds a reference to the + * BusContainerInstance */ + _dbus_assert (self->server == NULL); + /* It's OK to do this even if we were never added to instances_by_path, * because the paths are globally unique. */ if (self->path != NULL && self->containers->instances_by_path != NULL) @@ -135,6 +191,22 @@ bus_clear_container_instance (BusContainerInstance **instance_p) bus_container_instance_unref); } +static void +bus_container_instance_stop_listening (BusContainerInstance *self) +{ + /* In case the DBusServer holds the last reference to self */ + bus_container_instance_ref (self); + + if (self->server != NULL) + { + dbus_server_set_new_connection_function (self->server, NULL, NULL, NULL); + dbus_server_disconnect (self->server); + dbus_clear_server (&self->server); + } + + bus_container_instance_unref (self); +} + static BusContainerInstance * bus_container_instance_new (BusContext *context, BusContainers *containers, @@ -167,6 +239,7 @@ bus_container_instance_new (BusContext *context, self->metadata = NULL; self->context = bus_context_ref (context); self->containers = bus_containers_ref (containers); + self->server = NULL; if (containers->next_container_id >= DBUS_UINT64_CONSTANT (0xFFFFFFFFFFFFFFFF)) @@ -202,6 +275,129 @@ fail: return NULL; } +/* We only accept EXTERNAL authentication, because Unix platforms that are + * sufficiently capable to have app-containers ought to have it. */ +static const char * const auth_mechanisms[] = +{ + "EXTERNAL", + NULL +}; + +static void +new_connection_cb (DBusServer *server, + DBusConnection *new_connection, + void *data) +{ + /* TODO: handle new connection */ +} + +static const char * +bus_containers_ensure_address_template (BusContainers *self, + DBusError *error) +{ + DBusString dir; + const char *ret = NULL; + const char *runtime_dir; + + /* Early-return if we already did this */ + if (_dbus_string_get_length (&self->address_template) > 0) + return _dbus_string_get_const_data (&self->address_template); + + if (!_dbus_string_init (&dir)) + { + BUS_SET_OOM (error); + return FALSE; + } + + runtime_dir = _dbus_getenv ("XDG_RUNTIME_DIR"); + + if (runtime_dir != NULL) + { + /* We listen on a random socket path resembling + * /run/user/1000/dbus-1/containers/dbus-abcdef, chosen to share + * the dbus-1 directory with the dbus-1/services used for transient + * session services. */ + if (!_dbus_string_append (&dir, runtime_dir) || + !_dbus_string_append (&dir, "/dbus-1")) + { + BUS_SET_OOM (error); + goto out; + } + + if (!_dbus_ensure_directory (&dir, error)) + goto out; + + if (!_dbus_string_append (&dir, "/containers")) + { + BUS_SET_OOM (error); + goto out; + } + + if (!_dbus_ensure_directory (&dir, error)) + goto out; + } + else + { + /* No XDG_RUNTIME_DIR, so don't do anything special or clever: just + * use a random socket like /tmp/dbus-abcdef. */ + const char *tmpdir; + + tmpdir = _dbus_get_tmpdir (); + _dbus_string_init_const (&dir, tmpdir); + } + + /* We specifically use paths, even on Linux (unix:dir= not unix:tmpdir=), + * because an abstract socket that you can't bind-mount is not useful + * when you want something you can bind-mount into a container. */ + if (!_dbus_string_append (&self->address_template, "unix:dir=") || + !_dbus_address_append_escaped (&self->address_template, &dir)) + { + _dbus_string_set_length (&self->address_template, 0); + BUS_SET_OOM (error); + goto out; + } + + ret = _dbus_string_get_const_data (&self->address_template); + +out: + _dbus_string_free (&dir); + return ret; +} + +static dbus_bool_t +bus_container_instance_listen (BusContainerInstance *self, + DBusError *error) +{ + BusContainers *containers = bus_context_get_containers (self->context); + const char *address; + + address = bus_containers_ensure_address_template (containers, error); + + if (address == NULL) + return FALSE; + + self->server = dbus_server_listen (address, error); + + if (self->server == NULL) + return FALSE; + + if (!bus_context_setup_server (self->context, self->server, error)) + return FALSE; + + if (!dbus_server_set_auth_mechanisms (self->server, + (const char **) auth_mechanisms)) + { + BUS_SET_OOM (error); + return FALSE; + } + + /* Cannot fail because the memory it uses was already allocated */ + dbus_server_set_new_connection_function (self->server, new_connection_cb, + bus_container_instance_ref (self), + (DBusFreeFunction) bus_container_instance_unref); + return TRUE; +} + dbus_bool_t bus_containers_handle_add_server (DBusConnection *connection, BusTransaction *transaction, @@ -210,11 +406,18 @@ bus_containers_handle_add_server (DBusConnection *connection, { DBusMessageIter iter; DBusMessageIter dict_iter; + DBusMessageIter writer; + DBusMessageIter array_writer; const char *type; const char *name; + const char *path; BusContainerInstance *instance = NULL; BusContext *context; BusContainers *containers; + char *address = NULL; + DBusAddressEntry **entries = NULL; + int n_entries; + DBusMessage *reply = NULL; context = bus_transaction_get_context (transaction); containers = bus_context_get_containers (context); @@ -309,16 +512,74 @@ bus_containers_handle_add_server (DBusConnection *connection, instance->path, instance)) goto oom; - /* TODO: Actually implement the method */ - dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented"); - goto fail; + /* This part is separated out because we eventually want to be able to + * accept a fd-passed server socket in the named parameters, instead of + * creating our own server, and defer listening on it until later */ + if (!bus_container_instance_listen (instance, error)) + goto fail; + + address = dbus_server_get_address (instance->server); + + if (!dbus_parse_address (address, &entries, &n_entries, error)) + _dbus_assert_not_reached ("listening on unix:dir= should yield a valid address"); + + _dbus_assert (n_entries == 1); + _dbus_assert (strcmp (dbus_address_entry_get_method (entries[0]), "unix") == 0); + + path = dbus_address_entry_get_value (entries[0], "path"); + _dbus_assert (path != NULL); + + reply = dbus_message_new_method_return (message); + + if (!dbus_message_append_args (reply, + DBUS_TYPE_OBJECT_PATH, &instance->path, + DBUS_TYPE_INVALID)) + goto oom; + + dbus_message_iter_init_append (reply, &writer); + + if (!dbus_message_iter_open_container (&writer, DBUS_TYPE_ARRAY, + DBUS_TYPE_BYTE_AS_STRING, + &array_writer)) + goto oom; + + if (!dbus_message_iter_append_fixed_array (&array_writer, DBUS_TYPE_BYTE, + &path, strlen (path) + 1)) + { + dbus_message_iter_abandon_container (&writer, &array_writer); + goto oom; + } + + if (!dbus_message_iter_close_container (&writer, &array_writer)) + goto oom; + + if (!dbus_message_append_args (reply, + DBUS_TYPE_STRING, &address, + DBUS_TYPE_INVALID)) + goto oom; + + _dbus_assert (dbus_message_has_signature (reply, "oays")); + + if (! bus_transaction_send_from_driver (transaction, connection, reply)) + goto oom; + + dbus_message_unref (reply); + bus_container_instance_unref (instance); + dbus_address_entries_free (entries); + dbus_free (address); + return TRUE; oom: BUS_SET_OOM (error); /* fall through */ fail: + if (instance != NULL) + bus_container_instance_stop_listening (instance); + dbus_clear_message (&reply); + dbus_clear_address_entries (&entries); bus_clear_container_instance (&instance); + dbus_free (address); return FALSE; } @@ -335,6 +596,24 @@ bus_containers_supported_arguments_getter (BusContext *context, dbus_message_iter_close_container (var_iter, &arr_iter); } +void +bus_containers_stop_listening (BusContainers *self) +{ + if (self->instances_by_path != NULL) + { + DBusHashIter iter; + + _dbus_hash_iter_init (self->instances_by_path, &iter); + + while (_dbus_hash_iter_next (&iter)) + { + BusContainerInstance *instance = _dbus_hash_iter_get_value (&iter); + + bus_container_instance_stop_listening (instance); + } + } +} + #else BusContainers * @@ -359,4 +638,10 @@ bus_containers_unref (BusContainers *self) _dbus_assert (self == (BusContainers *) 1); } +void +bus_containers_stop_listening (BusContainers *self) +{ + _dbus_assert (self == (BusContainers *) 1); +} + #endif /* DBUS_ENABLE_CONTAINERS */ diff --git a/bus/containers.h b/bus/containers.h index bda0a879..9a7ac0ba 100644 --- a/bus/containers.h +++ b/bus/containers.h @@ -30,6 +30,7 @@ BusContainers *bus_containers_new (void); BusContainers *bus_containers_ref (BusContainers *self); void bus_containers_unref (BusContainers *self); +void bus_containers_stop_listening (BusContainers *self); dbus_bool_t bus_containers_handle_add_server (DBusConnection *connection, BusTransaction *transaction, diff --git a/bus/tmpfiles.d/dbus.conf.in b/bus/tmpfiles.d/dbus.conf.in index 754f0220..6ebecb6d 100644 --- a/bus/tmpfiles.d/dbus.conf.in +++ b/bus/tmpfiles.d/dbus.conf.in @@ -3,3 +3,7 @@ # Make ${localstatedir}/lib/dbus/machine-id a symlink to /etc/machine-id # if it does not already exist L @EXPANDED_LOCALSTATEDIR@/lib/dbus/machine-id - - - - /etc/machine-id + +# Create ${runstatedir}/dbus/containers owned by the system bus user. +# org.freedesktop.DBus.Containers1 uses this to create sockets. +d @EXPANDED_RUNSTATEDIR@/dbus/containers 0755 @DBUS_USER@ - - - diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index e7649e1d..2817e4e5 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -375,6 +375,7 @@ void _dbus_unlock (DBusGlobalLock lock); DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_threads_init_debug (void); +DBUS_PRIVATE_EXPORT dbus_bool_t _dbus_address_append_escaped (DBusString *escaped, const DBusString *unescaped); -- 2.13.3