monitor: use the addressed_recipient to select matches

This means we respect the destination keyword in arguments to
BecomeMonitor.

In bus_dispatch(), this means that we need to defer capturing until
we have decided whether there is an addressed recipient; so instead
of capturing once, we capture at each leaf of the decision tree.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92074
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: Lars Uebernickel <lars@uebernic.de>

bus/activation.c

@@ -1964,6 +1964,7 @@ bus_activation_activate_service (BusActivation  *activation,
           DBusString service_string;
           BusService *service;
           BusRegistry *registry;
+          DBusConnection *systemd = NULL;
 
           /* OK, we have a systemd service configured for this entry,
              hence let's enqueue an activation request message. This
@@ -2012,11 +2013,14 @@ bus_activation_activate_service (BusActivation  *activation,
           _dbus_string_init_const (&service_string, "org.freedesktop.systemd1");
           service = bus_registry_lookup (registry, &service_string);
 
+          if (service)
+            systemd = bus_service_get_primary_owners_connection (service);
+
           /* Following the general principle of "log early and often",
            * we capture that we *want* to send the activation message, even if
            * systemd is not actually there to receive it yet */
           if (!bus_transaction_capture (activation_transaction,
-                NULL, message))
+                                        NULL, systemd, message))
             {
               dbus_message_unref (message);
               BUS_SET_OOM (error);
@@ -2030,8 +2034,8 @@ bus_activation_activate_service (BusActivation  *activation,
                                service_name,
                                entry->systemd_service);
               /* Wonderful, systemd is connected, let's just send the msg */
-              retval = bus_dispatch_matches (activation_transaction, NULL, bus_service_get_primary_owners_connection (service),
+              retval = bus_dispatch_matches (activation_transaction, NULL,
-                                             message, error);
+                                             systemd, message, error);
             }
           else
             {

bus/connection.c

@@ -2182,6 +2182,7 @@ bus_transaction_get_context (BusTransaction  *transaction)
 dbus_bool_t
 bus_transaction_capture (BusTransaction *transaction,
                          DBusConnection *sender,
+                         DBusConnection *addressed_recipient,
                          DBusMessage    *message)
 {
   BusConnections *connections;
@@ -2201,8 +2202,8 @@ bus_transaction_capture (BusTransaction *transaction,
    * There's little point, since there is up to 1 per process. */
   _dbus_assert (mm != NULL);
 
-  if (!bus_matchmaker_get_recipients (mm, connections, sender, NULL, message,
+  if (!bus_matchmaker_get_recipients (mm, connections, sender,
-        &recipients))
+        addressed_recipient, message, &recipients))
     goto out;
 
   for (link = _dbus_list_get_first_link (&recipients);
@@ -2224,6 +2225,7 @@ out:
 
 dbus_bool_t
 bus_transaction_capture_error_reply (BusTransaction  *transaction,
+                                     DBusConnection  *addressed_recipient,
                                      const DBusError *error,
                                      DBusMessage     *in_reply_to)
 {
@@ -2250,7 +2252,7 @@ bus_transaction_capture_error_reply (BusTransaction  *transaction,
   if (!dbus_message_set_sender (reply, DBUS_SERVICE_DBUS))
     goto out;
 
-  ret = bus_transaction_capture (transaction, NULL, reply);
+  ret = bus_transaction_capture (transaction, NULL, addressed_recipient, reply);
 
 out:
   dbus_message_unref (reply);
@@ -2292,7 +2294,7 @@ bus_transaction_send_from_driver (BusTransaction *transaction,
   /* Capture it for monitors, even if the real recipient's receive policy
    * does not allow it to receive this message from us (which would be odd).
    */
-  if (!bus_transaction_capture (transaction, NULL, message))
+  if (!bus_transaction_capture (transaction, NULL, connection, message))
     return FALSE;
 
   /* If security policy doesn't allow the message, we would silently
@@ -2304,7 +2306,8 @@ bus_transaction_send_from_driver (BusTransaction *transaction,
                                           transaction,
                                           NULL, connection, connection, message, &error))
     {
-      if (!bus_transaction_capture_error_reply (transaction, &error, message))
+      if (!bus_transaction_capture_error_reply (transaction, connection,
+                                                &error, message))
         {
           bus_context_log (transaction->context, DBUS_SYSTEM_LOG_WARNING,
                            "message from dbus-daemon rejected but not enough "

bus/connection.h

@@ -134,8 +134,10 @@ dbus_bool_t     bus_transaction_send             (BusTransaction               *
                                                   DBusMessage                  *message);
 dbus_bool_t     bus_transaction_capture          (BusTransaction               *transaction,
                                                   DBusConnection               *connection,
+                                                  DBusConnection               *addressed_recipient,
                                                   DBusMessage                  *message);
 dbus_bool_t     bus_transaction_capture_error_reply (BusTransaction            *transaction,
+                                                  DBusConnection               *addressed_recipient,
                                                   const DBusError              *error,
                                                   DBusMessage                  *in_reply_to);
 dbus_bool_t     bus_transaction_send_from_driver (BusTransaction               *transaction,

bus/dispatch.c

@@ -72,8 +72,8 @@ send_one_message (DBusConnection *connection,
                                           message,
                                           &stack_error))
     {
-      if (!bus_transaction_capture_error_reply (transaction, &stack_error,
+      if (!bus_transaction_capture_error_reply (transaction, sender,
-                                                message))
+                                                &stack_error, message))
         {
           bus_context_log (context, DBUS_SYSTEM_LOG_WARNING,
                            "broadcast rejected, but not enough "
@@ -93,8 +93,8 @@ send_one_message (DBusConnection *connection,
                       bus_connection_get_name (connection),
                       bus_connection_get_loginfo (connection));
 
-      if (!bus_transaction_capture_error_reply (transaction, &stack_error,
+      if (!bus_transaction_capture_error_reply (transaction, sender,
-                                                message))
+                                                &stack_error, message))
         {
           bus_context_log (context, DBUS_SYSTEM_LOG_WARNING,
                            "broadcast with Unix fd not delivered, but not "
@@ -370,15 +370,15 @@ bus_dispatch (DBusConnection *connection,
    */
   service_name = dbus_message_get_destination (message);
 
-  if (!bus_transaction_capture (transaction, connection, message))
-    {
-      BUS_SET_OOM (&error);
-      goto out;
-    }
-
   if (service_name &&
       strcmp (service_name, DBUS_SERVICE_DBUS) == 0) /* to bus driver */
     {
+      if (!bus_transaction_capture (transaction, connection, NULL, message))
+        {
+          BUS_SET_OOM (&error);
+          goto out;
+        }
+
       if (!bus_context_check_security_policy (context, transaction,
                                               connection, NULL, NULL, message, &error))
         {
@@ -392,6 +392,12 @@ bus_dispatch (DBusConnection *connection,
     }
   else if (!bus_connection_is_active (connection)) /* clients must talk to bus driver first */
     {
+      if (!bus_transaction_capture (transaction, connection, NULL, message))
+        {
+          BUS_SET_OOM (&error);
+          goto out;
+        }
+
       _dbus_verbose ("Received message from non-registered client. Disconnecting.\n");
       dbus_connection_close (connection);
       goto out;
@@ -412,6 +418,14 @@ bus_dispatch (DBusConnection *connection,
       if (service == NULL && dbus_message_get_auto_start (message))
         {
           BusActivation *activation;
+
+          if (!bus_transaction_capture (transaction, connection, NULL,
+                                        message))
+            {
+              BUS_SET_OOM (&error);
+              goto out;
+            }
+
           /* We can't do the security policy check here, since the addressed
            * recipient service doesn't exist yet. We do it before sending the
            * message after the service has been created.
@@ -430,6 +444,13 @@ bus_dispatch (DBusConnection *connection,
         }
       else if (service == NULL)
         {
+          if (!bus_transaction_capture (transaction, connection,
+                                        NULL, message))
+            {
+              BUS_SET_OOM (&error);
+              goto out;
+            }
+
           dbus_set_error (&error,
                           DBUS_ERROR_NAME_HAS_NO_OWNER,
                           "Name \"%s\" does not exist",
@@ -440,6 +461,21 @@ bus_dispatch (DBusConnection *connection,
         {
           addressed_recipient = bus_service_get_primary_owners_connection (service);
           _dbus_assert (addressed_recipient != NULL);
+
+          if (!bus_transaction_capture (transaction, connection,
+                                        addressed_recipient, message))
+            {
+              BUS_SET_OOM (&error);
+              goto out;
+            }
+        }
+    }
+  else /* service_name == NULL */
+    {
+      if (!bus_transaction_capture (transaction, connection, NULL, message))
+        {
+          BUS_SET_OOM (&error);
+          goto out;
         }
     }
 

bus/driver.c

@@ -243,7 +243,7 @@ bus_driver_send_service_owner_changed (const char     *service_name,
 
   _dbus_assert (dbus_message_has_signature (message, "sss"));
 
-  if (!bus_transaction_capture (transaction, NULL, message))
+  if (!bus_transaction_capture (transaction, NULL, NULL, message))
     goto oom;
 
   retval = bus_dispatch_matches (transaction, NULL, NULL, message, error);

bus/signals.c

@@ -1887,9 +1887,18 @@ match_rule_matches (BusMatchRule    *rule,
         return FALSE;
 
       if (addressed_recipient == NULL)
-        {          
+        {
-          if (strcmp (rule->destination,
+          /* If the message is going to be delivered to the dbus-daemon
-                      DBUS_SERVICE_DBUS) != 0)
+           * itself, its destination will be "org.freedesktop.DBus",
+           * which we again match against the rule (see bus_dispatch()
+           * in bus/dispatch.c, which checks for o.fd.DBus first).
+           *
+           * If we are monitoring and we don't know who is going to receive
+           * the message (for instance because they haven't been activated yet),
+           * assume they will own the requested destination name and no other,
+           * and match the rule's destination against that.
+           */
+          if (strcmp (rule->destination, destination) != 0)
             return FALSE;
         }
       else

test/monitor.c

@@ -92,6 +92,11 @@ static const char * const selective_match_rules[] = {
     FALSE
 };
 
+static const char * const well_known_destination_match_rules[] = {
+    "destination='com.example.Recipient'",
+    NULL
+};
+
 static Config forbidding_config = {
     "valid-config-files/forbidding.conf",
     NULL,
@@ -110,6 +115,12 @@ static Config selective_config = {
     FALSE
 };
 
+static Config well_known_destination_config = {
+    NULL,
+    well_known_destination_match_rules,
+    FALSE
+};
+
 static Config no_rules_config = {
     NULL,
     no_match_rules,
@@ -414,6 +425,19 @@ activated_filter (DBusConnection *connection,
   return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 }
 
+static void
+take_well_known_name (Fixture *f,
+    DBusConnection *connection,
+    const char *name)
+{
+  int ret;
+
+  ret = dbus_bus_request_name (connection, name,
+      DBUS_NAME_FLAG_DO_NOT_QUEUE, &f->e);
+  test_assert_no_error (&f->e);
+  g_assert_cmpint (ret, ==, DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
+}
+
 static void
 setup (Fixture *f,
     gconstpointer context)
@@ -446,7 +470,8 @@ setup (Fixture *f,
 }
 
 static void
-become_monitor (Fixture *f)
+become_monitor (Fixture *f,
+    const Config *config)
 {
   DBusMessage *m;
   DBusPendingCall *pc;
@@ -458,8 +483,11 @@ become_monitor (Fixture *f)
 
   dbus_connection_set_route_peer_messages (f->monitor, TRUE);
 
-  if (f->config != NULL && f->config->match_rules != NULL)
+  if (config == NULL)
-    match_rules = f->config->match_rules;
+    config = f->config;
+
+  if (config != NULL && config->match_rules != NULL)
+    match_rules = config->match_rules;
   else
     match_rules = wildcard_match_rules;
 
@@ -746,7 +774,7 @@ test_become_monitor (Fixture *f,
         }
     }
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   while (!lost_unique || !lost_a || !lost_b || !lost_c)
     {
@@ -848,7 +876,7 @@ test_broadcast (Fixture *f,
   dbus_bus_add_match (f->recipient, "type='signal'", &f->e);
   test_assert_no_error (&f->e);
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_signal ("/foo", "com.example.bar", "BroadcastSignal1");
   dbus_connection_send (f->sender, m, NULL);
@@ -896,7 +924,7 @@ test_forbidden_broadcast (Fixture *f,
   dbus_bus_add_match (f->recipient, "type='signal'", &f->e);
   test_assert_no_error (&f->e);
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_signal ("/foo", "com.example.CannotSend",
       "BroadcastSignal1");
@@ -959,7 +987,7 @@ test_unicast_signal (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_signal ("/foo", "com.example.bar", "UnicastSignal1");
   if (!dbus_message_set_destination (m, f->recipient_name))
@@ -1010,7 +1038,7 @@ test_forbidden (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_signal ("/foo", "com.example.CannotSend",
       "UnicastSignal1");
@@ -1079,7 +1107,7 @@ test_method_call (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   /* regression test for
    * https://bugs.freedesktop.org/show_bug.cgi?id=90952 */
@@ -1134,7 +1162,7 @@ test_forbidden_method_call (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_method_call (f->recipient_name, "/foo",
       "com.example.CannotSend", "Call1");
@@ -1189,7 +1217,7 @@ test_dbus_daemon (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   res = dbus_bus_request_name (f->sender, "com.example.Sender",
       DBUS_NAME_FLAG_DO_NOT_QUEUE, &f->e);
@@ -1266,7 +1294,7 @@ test_selective (Fixture *f,
       "eavesdrop='true',interface='com.example.Tedious'", &f->e);
   test_assert_no_error (&f->e);
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   m = dbus_message_new_signal ("/foo", "com.example.Interesting",
       "UnicastSignal1");
@@ -1309,6 +1337,129 @@ test_selective (Fixture *f,
   g_assert (m == NULL);
 }
 
+static void
+test_well_known_destination (Fixture *f,
+    gconstpointer context)
+{
+  DBusMessage *m;
+
+  if (f->address == NULL)
+    return;
+
+  take_well_known_name (f, f->recipient, "com.example.Recipient");
+  /* we don't expect_take_well_known_name here because the
+   * monitor isn't up yet */
+
+  become_monitor (f, NULL);
+
+  /* The sender sends a message to itself. It will not be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Unobserved");
+  if (!dbus_message_set_destination (m, f->sender_name))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  /* The sender sends a message to the recipient by well-known name.
+   * It will be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Observed1");
+  if (!dbus_message_set_destination (m, "com.example.Recipient"))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  /* The sender sends a message to the recipient by unique name.
+   * It will still be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Observed2");
+  if (!dbus_message_set_destination (m, f->recipient_name))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  while (g_queue_get_length (&f->monitored) < 2)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  m = g_queue_pop_head (&f->monitored);
+  assert_signal (m, f->sender_name, "/foo", "com.example.bar",
+      "Observed1", "", "com.example.Recipient");
+  dbus_message_unref (m);
+
+  m = g_queue_pop_head (&f->monitored);
+  assert_signal (m, f->sender_name, "/foo", "com.example.bar",
+      "Observed2", "", f->recipient_name);
+  dbus_message_unref (m);
+
+  m = g_queue_pop_head (&f->monitored);
+  g_assert (m == NULL);
+}
+
+static void
+test_unique_destination (Fixture *f,
+    gconstpointer context)
+{
+  DBusMessage *m;
+  Config config = {
+    NULL,
+    NULL, /* match rules */
+    FALSE
+  };
+  const gchar *match_rules[2] = { NULL, NULL };
+  gchar *rule;
+
+  if (f->address == NULL)
+    return;
+
+  take_well_known_name (f, f->recipient, "com.example.Recipient");
+  /* we don't expect_take_well_known_name here because the
+   * monitor isn't up yet */
+
+  rule = g_strdup_printf ("destination='%s'", f->recipient_name);
+  /* free it later */
+  g_test_queue_free (rule);
+  match_rules[0] = rule;
+  config.match_rules = match_rules;
+
+  become_monitor (f, &config);
+
+  /* The sender sends a message to itself. It will not be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Unobserved");
+  if (!dbus_message_set_destination (m, f->sender_name))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  /* The sender sends a message to the recipient by well-known name.
+   * It will be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Observed1");
+  if (!dbus_message_set_destination (m, "com.example.Recipient"))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  /* The sender sends a message to the recipient by unique name.
+   * It will still be observed. */
+  m = dbus_message_new_signal ("/foo", "com.example.bar", "Observed2");
+  if (!dbus_message_set_destination (m, f->recipient_name))
+    g_error ("OOM");
+  dbus_connection_send (f->sender, m, NULL);
+  dbus_message_unref (m);
+
+  while (g_queue_get_length (&f->monitored) < 2)
+    test_main_context_iterate (f->ctx, TRUE);
+
+  m = g_queue_pop_head (&f->monitored);
+  assert_signal (m, f->sender_name, "/foo", "com.example.bar",
+      "Observed1", "", "com.example.Recipient");
+  dbus_message_unref (m);
+
+  m = g_queue_pop_head (&f->monitored);
+  assert_signal (m, f->sender_name, "/foo", "com.example.bar",
+      "Observed2", "", f->recipient_name);
+  dbus_message_unref (m);
+
+  m = g_queue_pop_head (&f->monitored);
+  g_assert (m == NULL);
+}
+
 #ifdef DBUS_UNIX
 /* currently only used for the systemd activation test */
 static void
@@ -1337,20 +1488,6 @@ expect_new_connection (Fixture *f)
   dbus_message_unref (m);
 }
 
-/* currently only used for the systemd activation test */
-static void
-take_well_known_name (Fixture *f,
-    DBusConnection *connection,
-    const char *name)
-{
-  int ret;
-
-  ret = dbus_bus_request_name (connection, name,
-      DBUS_NAME_FLAG_DO_NOT_QUEUE, &f->e);
-  test_assert_no_error (&f->e);
-  g_assert_cmpint (ret, ==, DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
-}
-
 /* currently only used for the systemd activation test */
 static void
 expect_take_well_known_name (Fixture *f,
@@ -1392,7 +1529,7 @@ test_activation (Fixture *f,
   if (f->address == NULL)
     return;
 
-  become_monitor (f);
+  become_monitor (f, NULL);
 
   /* The sender sends a message to an activatable service. */
   m = dbus_message_new_signal ("/foo", "com.example.bar", "UnicastSignal1");
@@ -1666,6 +1803,12 @@ main (int argc,
       setup, test_dbus_daemon, teardown);
   g_test_add ("/monitor/selective", Fixture, &selective_config,
       setup, test_selective, teardown);
+  g_test_add ("/monitor/well-known-destination",
+      Fixture, &well_known_destination_config,
+      setup, test_well_known_destination, teardown);
+  g_test_add ("/monitor/unique-destination",
+      Fixture, NULL,
+      setup, test_unique_destination, teardown);
   g_test_add ("/monitor/wildcard", Fixture, &wildcard_config,
       setup, test_unicast_signal, teardown);
   g_test_add ("/monitor/no-rule", Fixture, &no_rules_config,