From a7babbf10f162c938e464247a964811ed7d03fef 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. The upstream dbus distribution no longer contains integration glue for non-systemd boot systems, but downstreams that maintain a non-systemd boot system and are interested in the Containers interface should create /run/dbus/containers during boot. Signed-off-by: Simon McVittie Reviewed-by: Philip Withnall Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101354 --- bus/Makefile.am | 1 + bus/bus.c | 35 +++-- bus/bus.h | 3 + bus/containers.c | 294 +++++++++++++++++++++++++++++++++++- bus/containers.h | 1 + bus/tmpfiles.d/dbus.conf.in | 4 + dbus/dbus-internals.h | 1 + 7 files changed, 324 insertions(+), 15 deletions(-) diff --git a/bus/Makefile.am b/bus/Makefile.am index 337514124..d74080493 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 8b08b8ea7..c4b71600b 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -102,7 +102,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; @@ -219,6 +220,25 @@ setup_server (BusContext *context, DBusServer *server, 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; @@ -234,16 +254,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, @@ -1112,6 +1122,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 edc957734..27d9502d0 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -147,6 +147,9 @@ 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); #ifdef DBUS_ENABLE_EMBEDDED_TESTS void bus_context_quiet_log_begin (BusContext *context); diff --git a/bus/containers.c b/bus/containers.c index 1c9229520..a59226b73 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; }; @@ -72,14 +74,53 @@ bus_containers_new (void) /* We allocate the hash table lazily, expecting that the common case will * be a connection where this feature is never used */ BusContainers *self = dbus_new0 (BusContainers, 1); + DBusString invalid = _DBUS_STRING_INIT_INVALID; if (self == NULL) - return NULL; + goto oom; self->refcount = 1; self->instances_by_path = NULL; self->next_container_id = DBUS_UINT64_CONSTANT (0); + self->address_template = invalid; + + /* 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,21 @@ 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); + _dbus_assert (self->refcount < _DBUS_INT_MAX); + + self->refcount++; + return self; +} + static void bus_container_instance_unref (BusContainerInstance *self) { @@ -112,6 +164,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 +192,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 +240,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 +276,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 = _DBUS_STRING_INIT_INVALID; + 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); + + runtime_dir = _dbus_getenv ("XDG_RUNTIME_DIR"); + + if (runtime_dir != NULL) + { + if (!_dbus_string_init (&dir)) + { + BUS_SET_OOM (error); + goto out; + } + + /* 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 +407,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 +513,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 +597,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 +639,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 bda0a8791..9a7ac0ba3 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 754f0220b..6ebecb6d2 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 b210d6157..57a67d080 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -376,6 +376,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.47.3