gdbus: Track name owners for signal subscriptions

We will use this in a subsequent commit to prevent signals from an
impostor from being delivered to a subscriber.

To avoid message reordering leading to misleading situations, this does
not use the existing mechanism for watching bus name ownership, which
delivers the ownership changes to other main-contexts. Instead, it all
happens on the single thread used by the GDBusWorker, so the order in
which messages are received is the order in which they are processed.

Signed-off-by: Simon McVittie <smcv@collabora.com>

gio/gdbusconnection.c

@@ -324,6 +324,31 @@ signal_subscriber_unref (SignalSubscriber *subscriber)
 }
 
 typedef struct
+{
+  /*
+   * 1 reference while waiting for GetNameOwner() to finish
+   * 1 reference for each SignalData that points to this one as its
+   *   shared_name_watcher
+   */
+  grefcount ref_count;
+
+  gchar *owner;
+  guint32 get_name_owner_serial;
+} WatchedName;
+
+static WatchedName *
+watched_name_new (void)
+{
+  WatchedName *watched_name = g_new0 (WatchedName, 1);
+
+  g_ref_count_init (&watched_name->ref_count);
+  watched_name->owner = NULL;
+  return g_steal_pointer (&watched_name);
+}
+
+typedef struct SignalData SignalData;
+
+struct SignalData
 {
   gchar *rule;
   gchar *sender;
@@ -333,13 +358,36 @@ typedef struct
   gchar *arg0;
   GDBusSignalFlags flags;
   GPtrArray *subscribers;  /* (owned) (element-type SignalSubscriber) */
-  gboolean sender_is_its_own_owner;
+
-} SignalData;
+  /*
+   * If the sender is a well-known name, this is an unowned SignalData
+   * representing the NameOwnerChanged signal that tracks its owner.
+   * NULL if sender is NULL.
+   * NULL if sender is its own owner (a unique name or DBUS_SERVICE_DBUS).
+   *
+   * Invariants: if not NULL, then
+   * shared_name_watcher->sender == DBUS_SERVICE_DBUS
+   * shared_name_watcher->interface_name == DBUS_INTERFACE_DBUS
+   * shared_name_watcher->member == "NameOwnerChanged"
+   * shared_name_watcher->object_path == DBUS_PATH_DBUS
+   * shared_name_watcher->arg0 == sender
+   * shared_name_watcher->flags == NONE
+   * shared_name_watcher->watched_name == NULL
+   */
+  SignalData *shared_name_watcher;
+
+  /*
+   * Non-NULL if this SignalData is another SignalData's shared_name_watcher.
+   * One reference for each SignalData that has this one as its
+   * shared_name_watcher.
+   * Otherwise NULL.
+   */
+  WatchedName *watched_name;
+};
 
 static SignalData *
 signal_data_new_take (gchar *rule,
                       gchar *sender,
-                      gboolean sender_is_its_own_owner,
                       gchar *interface_name,
                       gchar *member,
                       gchar *object_path,
@@ -350,7 +398,6 @@ signal_data_new_take (gchar *rule,
 
   signal_data->rule = rule;
   signal_data->sender = sender;
-  signal_data->sender_is_its_own_owner = sender_is_its_own_owner;
   signal_data->interface_name = interface_name;
   signal_data->member = member;
   signal_data->object_path = object_path;
@@ -363,6 +410,17 @@ signal_data_new_take (gchar *rule,
 static void
 signal_data_free (SignalData *signal_data)
 {
+  /* The SignalData should not be freed while it still has subscribers */
+  g_assert (signal_data->subscribers->len == 0);
+
+  /* The SignalData should not be freed while it is watching for
+   * NameOwnerChanged on behalf of another SignalData */
+  g_assert (signal_data->watched_name == NULL);
+
+  /* The SignalData should be detached from its name watcher, if any,
+   * before it is freed */
+  g_assert (signal_data->shared_name_watcher == NULL);
+
   g_free (signal_data->rule);
   g_free (signal_data->sender);
   g_free (signal_data->interface_name);
@@ -370,6 +428,7 @@ signal_data_free (SignalData *signal_data)
   g_free (signal_data->object_path);
   g_free (signal_data->arg0);
   g_ptr_array_unref (signal_data->subscribers);
+
   g_free (signal_data);
 }
 
@@ -493,6 +552,7 @@ struct _GDBusConnection
 
   /* Map used for managing method replies, protected by @lock */
   GHashTable *map_method_serial_to_task;  /* guint32 -> owned GTask* */
+  GHashTable *map_method_serial_to_name_watcher;  /* guint32 -> unowned SignalData* */
 
   /* Maps used for managing signal subscription, protected by @lock */
   GHashTable *map_rule_to_signal_data;                      /* match rule (gchar*)    -> SignalData */
@@ -741,6 +801,7 @@ g_dbus_connection_finalize (GObject *object)
     g_error_free (connection->initialization_error);
 
   g_hash_table_unref (connection->map_method_serial_to_task);
+  g_hash_table_unref (connection->map_method_serial_to_name_watcher);
 
   g_hash_table_unref (connection->map_rule_to_signal_data);
   g_hash_table_unref (connection->map_id_to_signal_data);
@@ -1127,6 +1188,7 @@ g_dbus_connection_init (GDBusConnection *connection)
   g_mutex_init (&connection->init_lock);
 
   connection->map_method_serial_to_task = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
+  connection->map_method_serial_to_name_watcher = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, NULL);
 
   connection->map_rule_to_signal_data = g_hash_table_new (g_str_hash,
                                                           g_str_equal);
@@ -2254,6 +2316,191 @@ g_dbus_connection_send_message_with_reply_sync (GDBusConnection        *connecti
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/*
+ * Called in any thread.
+ * Must hold the connection lock when calling this, unless
+ * connection->finalizing is TRUE.
+ */
+static void
+name_watcher_unref_watched_name (GDBusConnection *connection,
+                                 SignalData *name_watcher)
+{
+  WatchedName *watched_name = name_watcher->watched_name;
+
+  g_assert (watched_name != NULL);
+
+  if (!g_ref_count_dec (&watched_name->ref_count))
+    return;
+
+  /* Removing watched_name from the name_watcher may result in
+   * name_watcher being freed, so we must make sure name_watcher is no
+   * longer in map_method_serial_to_name_watcher.
+   *
+   * If we stop watching the name while our GetNameOwner call was still
+   * in-flight, then when the reply eventually arrives, we will not find
+   * its serial number in the map and harmlessly ignore it as a result. */
+  if (watched_name->get_name_owner_serial != 0)
+    g_hash_table_remove (connection->map_method_serial_to_name_watcher,
+                         GUINT_TO_POINTER (watched_name->get_name_owner_serial));
+
+  name_watcher->watched_name = NULL;
+  g_free (watched_name->owner);
+  g_free (watched_name);
+}
+
+/* called in GDBusWorker thread with lock held */
+static void
+name_watcher_set_name_owner_unlocked (SignalData *name_watcher,
+                                      const char *new_owner)
+{
+  if (new_owner != NULL && new_owner[0] == '\0')
+    new_owner = NULL;
+
+  g_assert (name_watcher->watched_name != NULL);
+  g_set_str (&name_watcher->watched_name->owner, new_owner);
+}
+
+/* called in GDBusWorker thread with lock held */
+static void
+name_watcher_deliver_name_owner_changed_unlocked (SignalData *name_watcher,
+                                                  GDBusMessage *message)
+{
+  GVariant *body;
+
+  body = g_dbus_message_get_body (message);
+
+  if (G_LIKELY (body != NULL && g_variant_is_of_type (body, G_VARIANT_TYPE ("(sss)"))))
+    {
+      const char *name;
+      const char *new_owner;
+
+      g_variant_get (body, "(&s&s&s)", &name, NULL, &new_owner);
+
+      /* Our caller already checked this */
+      g_assert (g_strcmp0 (name_watcher->arg0, name) == 0);
+
+      if (G_LIKELY (new_owner[0] == '\0' || g_dbus_is_unique_name (new_owner)))
+        name_watcher_set_name_owner_unlocked (name_watcher, new_owner);
+      else
+        g_warning ("Received NameOwnerChanged signal with invalid owner \"%s\" for \"%s\"",
+                   new_owner, name);
+    }
+  else
+    {
+      g_warning ("Received NameOwnerChanged signal with unexpected "
+                 "signature %s",
+                 body == NULL ? "()" : g_variant_get_type_string (body));
+
+    }
+}
+
+/* called in GDBusWorker thread with lock held */
+static void
+name_watcher_deliver_get_name_owner_reply_unlocked (SignalData *name_watcher,
+                                                    GDBusConnection *connection,
+                                                    GDBusMessage *message)
+{
+  GDBusMessageType type;
+  GVariant *body;
+  WatchedName *watched_name;
+
+  watched_name = name_watcher->watched_name;
+  g_assert (watched_name != NULL);
+  g_assert (watched_name->get_name_owner_serial != 0);
+
+  type = g_dbus_message_get_message_type (message);
+  body = g_dbus_message_get_body (message);
+
+  if (type == G_DBUS_MESSAGE_TYPE_ERROR)
+    {
+      if (g_strcmp0 (g_dbus_message_get_error_name (message),
+                     "org.freedesktop.DBus.Error.NameHasNoOwner"))
+        name_watcher_set_name_owner_unlocked (name_watcher, NULL);
+      /* else it's something like NoReply or AccessDenied, which tells
+       * us nothing - leave the owner set to whatever we most recently
+       * learned from NameOwnerChanged, or NULL */
+    }
+  else if (type != G_DBUS_MESSAGE_TYPE_METHOD_RETURN)
+    {
+      g_warning ("Received GetNameOwner reply with unexpected type %d",
+                 type);
+    }
+  else if (G_LIKELY (body != NULL && g_variant_is_of_type (body, G_VARIANT_TYPE ("(s)"))))
+    {
+      const char *new_owner;
+
+      g_variant_get (body, "(&s)", &new_owner);
+
+      if (G_LIKELY (g_dbus_is_unique_name (new_owner)))
+        name_watcher_set_name_owner_unlocked (name_watcher, new_owner);
+      else
+        g_warning ("Received GetNameOwner reply with invalid owner \"%s\" for \"%s\"",
+                   new_owner, name_watcher->arg0);
+    }
+  else
+    {
+      g_warning ("Received GetNameOwner reply with unexpected signature %s",
+                 body == NULL ? "()" : g_variant_get_type_string (body));
+    }
+
+  g_hash_table_remove (connection->map_method_serial_to_name_watcher,
+                       GUINT_TO_POINTER (watched_name->get_name_owner_serial));
+  watched_name->get_name_owner_serial = 0;
+}
+
+/* Called in a user thread, lock is held */
+static void
+name_watcher_call_get_name_owner_unlocked (GDBusConnection *connection,
+                                           SignalData *name_watcher)
+{
+  GDBusMessage *message;
+  GError *local_error = NULL;
+  WatchedName *watched_name;
+
+  g_assert (g_strcmp0 (name_watcher->sender, DBUS_SERVICE_DBUS) == 0);
+  g_assert (g_strcmp0 (name_watcher->interface_name, DBUS_INTERFACE_DBUS) == 0);
+  g_assert (g_strcmp0 (name_watcher->member, "NameOwnerChanged") == 0);
+  g_assert (g_strcmp0 (name_watcher->object_path, DBUS_PATH_DBUS) == 0);
+  /* arg0 of the NameOwnerChanged message is the well-known name whose owner
+   * we are interested in */
+  g_assert (g_dbus_is_name (name_watcher->arg0));
+  g_assert (name_watcher->flags == G_DBUS_SIGNAL_FLAGS_NONE);
+
+  watched_name = name_watcher->watched_name;
+  g_assert (watched_name != NULL);
+  g_assert (watched_name->owner == NULL);
+  g_assert (watched_name->get_name_owner_serial == 0);
+  g_assert (name_watcher->shared_name_watcher == NULL);
+
+  message = g_dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+                                            DBUS_PATH_DBUS,
+                                            DBUS_INTERFACE_DBUS,
+                                            "GetNameOwner");
+  g_dbus_message_set_body (message, g_variant_new ("(s)", name_watcher->arg0));
+
+  if (g_dbus_connection_send_message_unlocked (connection, message,
+                                               G_DBUS_SEND_MESSAGE_FLAGS_NONE,
+                                               &watched_name->get_name_owner_serial,
+                                               &local_error))
+    {
+      g_assert (watched_name->get_name_owner_serial != 0);
+      g_hash_table_insert (connection->map_method_serial_to_name_watcher,
+                           GUINT_TO_POINTER (watched_name->get_name_owner_serial),
+                           name_watcher);
+    }
+  else
+    {
+      g_critical ("Error while sending GetNameOwner() message: %s",
+                  local_error->message);
+      g_clear_error (&local_error);
+      g_assert (watched_name->get_name_owner_serial == 0);
+    }
+
+  g_object_unref (message);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
 typedef struct
 {
   guint                       id;
@@ -2377,6 +2624,7 @@ on_worker_message_received (GDBusWorker  *worker,
         {
           guint32 reply_serial;
           GTask *task;
+          SignalData *name_watcher;
 
           reply_serial = g_dbus_message_get_reply_serial (message);
           CONNECTION_LOCK (connection);
@@ -2392,6 +2640,19 @@ on_worker_message_received (GDBusWorker  *worker,
             {
               //g_debug ("message reply/error for serial %d but no SendMessageData found for %p", reply_serial, connection);
             }
+
+          name_watcher = g_hash_table_lookup (connection->map_method_serial_to_name_watcher,
+                                              GUINT_TO_POINTER (reply_serial));
+
+          if (name_watcher != NULL)
+            {
+              g_assert (name_watcher->watched_name != NULL);
+              g_assert (name_watcher->watched_name->get_name_owner_serial == reply_serial);
+              name_watcher_deliver_get_name_owner_reply_unlocked (name_watcher,
+                                                                  connection,
+                                                                  message);
+            }
+
           CONNECTION_UNLOCK (connection);
         }
       else if (message_type == G_DBUS_MESSAGE_TYPE_SIGNAL)
@@ -3580,6 +3841,7 @@ g_dbus_connection_signal_subscribe (GDBusConnection     *connection,
 {
   gchar *rule;
   SignalData *signal_data;
+  SignalData *name_watcher = NULL;
   SignalSubscriber *subscriber;
   gboolean sender_is_its_own_owner;
   const gchar *sender_unique_name;
@@ -3645,13 +3907,59 @@ g_dbus_connection_signal_subscribe (GDBusConnection     *connection,
 
   signal_data = signal_data_new_take (g_steal_pointer (&rule),
                                       g_strdup (sender),
-                                      sender_is_its_own_owner,
                                       g_strdup (interface_name),
                                       g_strdup (member),
                                       g_strdup (object_path),
                                       g_strdup (arg0),
                                       flags);
   g_ptr_array_add (signal_data->subscribers, subscriber);
+
+  /* If subscribing to a signal from a specific sender with a well-known
+   * name, we must first subscribe to NameOwnerChanged signals for that
+   * well-known name, so that we can match the current owner of the name
+   * against the sender of each signal. */
+  if (sender != NULL && !sender_is_its_own_owner)
+    {
+      gchar *name_owner_rule = NULL;
+
+      /* We already checked that sender != NULL implies MESSAGE_BUS_CONNECTION */
+      g_assert (connection->flags & G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION);
+
+      name_owner_rule = args_to_rule (DBUS_SERVICE_DBUS,
+                                      DBUS_INTERFACE_DBUS,
+                                      "NameOwnerChanged",
+                                      DBUS_PATH_DBUS,
+                                      sender,
+                                      G_DBUS_SIGNAL_FLAGS_NONE);
+      name_watcher = g_hash_table_lookup (connection->map_rule_to_signal_data, name_owner_rule);
+
+      if (name_watcher == NULL)
+        {
+          name_watcher = signal_data_new_take (g_steal_pointer (&name_owner_rule),
+                                               g_strdup (DBUS_SERVICE_DBUS),
+                                               g_strdup (DBUS_INTERFACE_DBUS),
+                                               g_strdup ("NameOwnerChanged"),
+                                               g_strdup (DBUS_PATH_DBUS),
+                                               g_strdup (sender),
+                                               G_DBUS_SIGNAL_FLAGS_NONE);
+          add_signal_data (connection, name_watcher, DBUS_SERVICE_DBUS);
+        }
+
+      if (name_watcher->watched_name == NULL)
+        {
+          name_watcher->watched_name = watched_name_new ();
+          name_watcher_call_get_name_owner_unlocked (connection, name_watcher);
+        }
+      else
+        {
+          g_ref_count_inc (&name_watcher->watched_name->ref_count);
+        }
+
+      signal_data->shared_name_watcher = name_watcher;
+
+      g_clear_pointer (&name_owner_rule, g_free);
+    }
+
   add_signal_data (connection, signal_data, sender_unique_name);
 
  out:
@@ -3679,10 +3987,18 @@ remove_signal_data_if_unused (GDBusConnection *connection,
   const gchar *sender_unique_name;
   GPtrArray *signal_data_array;
 
+  /* Cannot remove while there are still subscribers */
   if (signal_data->subscribers->len != 0)
     return;
 
-  if (signal_data->sender_is_its_own_owner)
+  /* Cannot remove while another SignalData is still using this one
+   * as its shared_name_watcher, which holds watched_name->ref_count > 0 */
+  if (signal_data->watched_name != NULL)
+    return;
+
+  /* Point of no return: we have committed to removing it */
+
+  if (signal_data->sender != NULL && signal_data->shared_name_watcher == NULL)
     sender_unique_name = signal_data->sender;
   else
     sender_unique_name = "";
@@ -3715,6 +4031,15 @@ remove_signal_data_if_unused (GDBusConnection *connection,
       remove_match_rule (connection, signal_data->rule);
     }
 
+  if (signal_data->shared_name_watcher != NULL)
+    {
+      SignalData *name_watcher = g_steal_pointer (&signal_data->shared_name_watcher);
+
+      name_watcher_unref_watched_name (connection, name_watcher);
+      /* May free signal_data */
+      remove_signal_data_if_unused (connection, name_watcher);
+    }
+
   signal_data_free (signal_data);
 }
 
@@ -3991,6 +4316,17 @@ schedule_callbacks (GDBusConnection *connection,
             continue;
         }
 
+      if (signal_data->watched_name != NULL)
+        {
+          /* Invariant: SignalData should only have a watched_name if it
+           * represents the NameOwnerChanged signal */
+          g_assert (g_strcmp0 (sender, DBUS_SERVICE_DBUS) == 0);
+          g_assert (g_strcmp0 (interface, DBUS_INTERFACE_DBUS) == 0);
+          g_assert (g_strcmp0 (path, DBUS_PATH_DBUS) == 0);
+          g_assert (g_strcmp0 (member, "NameOwnerChanged") == 0);
+          name_watcher_deliver_name_owner_changed_unlocked (signal_data, message);
+        }
+
       for (m = 0; m < signal_data->subscribers->len; m++)
         {
           SignalSubscriber *subscriber = signal_data->subscribers->pdata[m];
@@ -4062,7 +4398,7 @@ distribute_signals (GDBusConnection *connection,
         schedule_callbacks (connection, signal_data_array, message, sender);
     }
 
-  /* collect subscribers not matching on sender */
+  /* collect subscribers not matching on sender, or matching a well-known name */
   signal_data_array = g_hash_table_lookup (connection->map_sender_unique_name_to_signal_data_array, "");
   if (signal_data_array != NULL)
     schedule_callbacks (connection, signal_data_array, message, sender);