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 <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101354

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)
 

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 *

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);

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 */

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,

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@ - - -

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);