Capture all messages received or sent, and send them to monitors

Unlike eavesdropping, the point of capture is when the message is
received, except for messages originating inside the dbus-daemon.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=46787
Reviewed-by: Philip Withnall <philip.withnall@collabora.co.uk>

bus/activation.c

@@ -2004,6 +2004,17 @@ bus_activation_activate_service (BusActivation  *activation,
           _dbus_string_init_const (&service_string, "org.freedesktop.systemd1");
           service = bus_registry_lookup (registry, &service_string);
 
+          /* 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))
+            {
+              dbus_message_unref (message);
+              BUS_SET_OOM (error);
+              return FALSE;
+            }
+
           if (service != NULL)
             {
               bus_context_log (activation->context,

bus/connection.c

@@ -67,6 +67,7 @@ struct BusConnections
   /** List of all monitoring connections, a subset of completed.
    * Each member is a #DBusConnection. */
   DBusList *monitors;
+  BusMatchmaker *monitor_matchmaker;
 
 #ifdef DBUS_ENABLE_STATS
   int total_match_rules;
@@ -292,6 +293,11 @@ bus_connection_disconnected (DBusConnection *connection)
 
   if (d->link_in_monitors != NULL)
     {
+      BusMatchmaker *mm = d->connections->monitor_matchmaker;
+
+      if (mm != NULL)
+        bus_matchmaker_disconnected (mm, connection);
+
       _dbus_list_remove_link (&d->connections->monitors, d->link_in_monitors);
       d->link_in_monitors = NULL;
     }
@@ -553,7 +559,10 @@ bus_connections_unref (BusConnections *connections)
       _dbus_timeout_unref (connections->expire_timeout);
       
       _dbus_hash_table_unref (connections->completed_by_user);
-      
+
+      if (connections->monitor_matchmaker != NULL)
+        bus_matchmaker_unref (connections->monitor_matchmaker);
+
       dbus_free (connections);
 
       dbus_connection_free_data_slot (&connection_data_slot);
@@ -1272,6 +1281,10 @@ bus_connection_send_oom_error (DBusConnection *connection,
   _dbus_assert (d != NULL);  
   _dbus_assert (d->oom_message != NULL);
 
+  bus_context_log (d->connections->context, DBUS_SYSTEM_LOG_WARNING,
+                   "dbus-daemon transaction failed (OOM), sending error to "
+                   "sender %s", bus_connection_get_loginfo (connection));
+
   /* should always succeed since we set it to a placeholder earlier */
   if (!dbus_message_set_reply_serial (d->oom_message,
                                       dbus_message_get_serial (in_reply_to)))
@@ -2120,11 +2133,95 @@ bus_transaction_get_context (BusTransaction  *transaction)
   return transaction->context;
 }
 
+/**
+ * Reserve enough memory to capture the given message if the
+ * transaction goes through.
+ */
+dbus_bool_t
+bus_transaction_capture (BusTransaction *transaction,
+                         DBusConnection *sender,
+                         DBusMessage    *message)
+{
+  BusConnections *connections;
+  BusMatchmaker *mm;
+  DBusList *link;
+  DBusList *recipients = NULL;
+  dbus_bool_t ret = FALSE;
+
+  connections = bus_context_get_connections (transaction->context);
+
+  /* shortcut: don't compose the message unless someone wants it */
+  if (connections->monitors == NULL)
+    return TRUE;
+
+  mm = connections->monitor_matchmaker;
+  /* This is non-null if there has ever been a monitor - we don't GC it.
+   * 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,
+        &recipients))
+    goto out;
+
+  for (link = _dbus_list_get_first_link (&recipients);
+      link != NULL;
+      link = _dbus_list_get_next_link (&recipients, link))
+    {
+      DBusConnection *recipient = link->data;
+
+      if (!bus_transaction_send (transaction, recipient, message))
+        goto out;
+    }
+
+  ret = TRUE;
+
+out:
+  _dbus_list_clear (&recipients);
+  return ret;
+}
+
+static dbus_bool_t
+bus_transaction_capture_error_reply (BusTransaction  *transaction,
+                                     const DBusError *error,
+                                     DBusMessage     *in_reply_to)
+{
+  BusConnections *connections;
+  DBusMessage *reply;
+  dbus_bool_t ret = FALSE;
+
+  _dbus_assert (error != NULL);
+  _DBUS_ASSERT_ERROR_IS_SET (error);
+
+  connections = bus_context_get_connections (transaction->context);
+
+  /* shortcut: don't compose the message unless someone wants it */
+  if (connections->monitors == NULL)
+    return TRUE;
+
+  reply = dbus_message_new_error (in_reply_to,
+                                  error->name,
+                                  error->message);
+
+  if (reply == NULL)
+    return FALSE;
+
+  if (!dbus_message_set_sender (reply, DBUS_SERVICE_DBUS))
+    goto out;
+
+  ret = bus_transaction_capture (transaction, NULL, reply);
+
+out:
+  dbus_message_unref (reply);
+  return ret;
+}
+
 dbus_bool_t
 bus_transaction_send_from_driver (BusTransaction *transaction,
                                   DBusConnection *connection,
                                   DBusMessage    *message)
 {
+  DBusError error = DBUS_ERROR_INIT;
+
   /* We have to set the sender to the driver, and have
    * to check security policy since it was not done in
    * dispatch.c
@@ -2149,14 +2246,34 @@ bus_transaction_send_from_driver (BusTransaction *transaction,
   
   /* bus driver never wants a reply */
   dbus_message_set_no_reply (message, TRUE);
-  
-  /* If security policy doesn't allow the message, we silently
-   * eat it; the driver doesn't care about getting a reply.
+
+  /* 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))
+    return FALSE;
+
+  /* If security policy doesn't allow the message, we would silently
+   * eat it; the driver doesn't care about getting a reply. However,
+   * if we're actively capturing messages, it's nice to log that we
+   * tried to send it and did not allow ourselves to do so.
    */
   if (!bus_context_check_security_policy (bus_transaction_get_context (transaction),
                                           transaction,
-                                          NULL, connection, connection, message, NULL))
-    return TRUE;
+                                          NULL, connection, connection, message, &error))
+    {
+      if (!bus_transaction_capture_error_reply (transaction, &error, message))
+        {
+          bus_context_log (transaction->context, DBUS_SYSTEM_LOG_WARNING,
+                           "message from dbus-daemon rejected but not enough "
+                           "memory to capture it");
+        }
+
+      /* This is not fatal to the transaction so silently eat the disallowed
+       * message (see reasoning above) */
+      dbus_error_free (&error);
+      return TRUE;
+    }
 
   return bus_transaction_send (transaction, connection, message);
 }
@@ -2520,10 +2637,53 @@ bus_connection_is_monitor (DBusConnection *connection)
   return d != NULL && d->link_in_monitors != NULL;
 }
 
+static dbus_bool_t
+bcd_add_monitor_rules (BusConnectionData  *d,
+                       DBusConnection     *connection,
+                       DBusList          **rules)
+{
+  BusMatchmaker *mm = d->connections->monitor_matchmaker;
+  DBusList *iter;
+
+  if (mm == NULL)
+    {
+      mm = bus_matchmaker_new ();
+
+      if (mm == NULL)
+        return FALSE;
+
+      d->connections->monitor_matchmaker = mm;
+    }
+
+  for (iter = _dbus_list_get_first_link (rules);
+      iter != NULL;
+      iter = _dbus_list_get_next_link (rules, iter))
+    {
+      if (!bus_matchmaker_add_rule (mm, iter->data))
+        {
+          bus_matchmaker_disconnected (mm, connection);
+          return FALSE;
+        }
+    }
+
+  return TRUE;
+}
+
+static void
+bcd_drop_monitor_rules (BusConnectionData *d,
+                        DBusConnection *connection)
+{
+  BusMatchmaker *mm = d->connections->monitor_matchmaker;
+
+  if (mm != NULL)
+    bus_matchmaker_disconnected (mm, connection);
+}
+
 dbus_bool_t
-bus_connection_be_monitor (DBusConnection *connection,
-                           BusTransaction *transaction,
-                           DBusError      *error)
+bus_connection_be_monitor (DBusConnection  *connection,
+                           BusTransaction  *transaction,
+                           DBusList       **rules,
+                           DBusError       *error)
 {
   BusConnectionData *d;
   DBusList *link;
@@ -2541,9 +2701,17 @@ bus_connection_be_monitor (DBusConnection *connection,
       return FALSE;
     }
 
+  if (!bcd_add_monitor_rules (d, connection, rules))
+    {
+      _dbus_list_free_link (link);
+      BUS_SET_OOM (error);
+      return FALSE;
+    }
+
   /* release all its names */
   if (!_dbus_list_copy (&d->services_owned, &tmp))
     {
+      bcd_drop_monitor_rules (d, connection);
       _dbus_list_free_link (link);
       BUS_SET_OOM (error);
       return FALSE;
@@ -2560,6 +2728,7 @@ bus_connection_be_monitor (DBusConnection *connection,
        * the transaction is cancelled due to OOM. */
       if (!bus_service_remove_owner (service, connection, transaction, error))
         {
+          bcd_drop_monitor_rules (d, connection);
           _dbus_list_free_link (link);
           _dbus_list_clear (&tmp);
           return FALSE;

bus/connection.h

@@ -116,10 +116,11 @@ dbus_bool_t      bus_connection_get_unix_groups  (DBusConnection       *connecti
                                                   DBusError            *error);
 BusClientPolicy* bus_connection_get_policy  (DBusConnection       *connection);
 
-dbus_bool_t bus_connection_is_monitor (DBusConnection *connection);
-dbus_bool_t bus_connection_be_monitor (DBusConnection *connection,
-                                       BusTransaction *transaction,
-                                       DBusError      *error);
+dbus_bool_t bus_connection_is_monitor (DBusConnection  *connection);
+dbus_bool_t bus_connection_be_monitor (DBusConnection  *connection,
+                                       BusTransaction  *transaction,
+                                       DBusList       **rules,
+                                       DBusError       *error);
 
 /* transaction API so we can send or not send a block of messages as a whole */
 
@@ -130,6 +131,9 @@ BusContext*     bus_transaction_get_context      (BusTransaction               *
 dbus_bool_t     bus_transaction_send             (BusTransaction               *transaction,
                                                   DBusConnection               *connection,
                                                   DBusMessage                  *message);
+dbus_bool_t     bus_transaction_capture          (BusTransaction               *transaction,
+                                                  DBusConnection               *connection,
+                                                  DBusMessage                  *message);
 dbus_bool_t     bus_transaction_send_from_driver (BusTransaction               *transaction,
                                                   DBusConnection               *connection,
                                                   DBusMessage                  *message);

bus/dispatch.c

@@ -291,6 +291,10 @@ bus_dispatch (DBusConnection *connection,
         {
           /* DBusConnection also handles some of these automatically, we leave
            * it to do so.
+           *
+           * FIXME: this means monitors won't get the opportunity to see
+           * non-signals with NULL destination, or their replies (which in
+           * practice are UnknownMethod errors)
            */
           result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
           goto out;
@@ -316,13 +320,30 @@ bus_dispatch (DBusConnection *connection,
           BUS_SET_OOM (&error);
           goto out;
         }
+    }
+  else
+    {
+      /* For monitors' benefit: we don't want the sender to be able to
+       * trick the monitor by supplying a forged sender, and we also
+       * don't want the message to have no sender at all. */
+      if (!dbus_message_set_sender (message, ":not.active.yet"))
+        {
+          BUS_SET_OOM (&error);
+          goto out;
+        }
+    }
 
-      /* We need to refetch the service name here, because
-       * dbus_message_set_sender can cause the header to be
-       * reallocated, and thus the service_name pointer will become
-       * invalid.
-       */
-      service_name = dbus_message_get_destination (message);
+  /* We need to refetch the service name here, because
+   * dbus_message_set_sender can cause the header to be
+   * reallocated, and thus the service_name pointer will become
+   * invalid.
+   */
+  service_name = dbus_message_get_destination (message);
+
+  if (!bus_transaction_capture (transaction, connection, message))
+    {
+      BUS_SET_OOM (&error);
+      goto out;
     }
 
   if (service_name &&
@@ -402,14 +423,10 @@ bus_dispatch (DBusConnection *connection,
  out:
   if (dbus_error_is_set (&error))
     {
-      if (!dbus_connection_get_is_connected (connection))
-        {
-          /* If we disconnected it, we won't bother to send it any error
-           * messages.
-           */
-          _dbus_verbose ("Not sending error to connection we disconnected\n");
-        }
-      else if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
+      /* Even if we disconnected it, pretend to send it any pending error
+       * messages so that monitors can observe them.
+       */
+      if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
         {
           bus_connection_send_oom_error (connection, message);
 

bus/driver.c

@@ -217,6 +217,9 @@ 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))
+    goto oom;
+
   retval = bus_dispatch_matches (transaction, NULL, NULL, message, error);
   dbus_message_unref (message);
 
@@ -1794,27 +1797,109 @@ bus_driver_handle_become_monitor (DBusConnection *connection,
                                   DBusMessage    *message,
                                   DBusError      *error)
 {
+  char **match_rules = NULL;
+  BusMatchRule *rule;
+  DBusList *rules = NULL;
+  DBusList *iter;
+  DBusString str;
+  int i;
+  int n_match_rules;
+  dbus_uint32_t flags;
+  dbus_bool_t ret = FALSE;
+
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
 
   if (!bus_driver_check_message_is_for_us (message, error))
-    return FALSE;
+    goto out;
 
   if (!bus_driver_check_caller_is_privileged (connection, transaction,
                                               message, error))
-    return FALSE;
+    goto out;
+
+  if (!dbus_message_get_args (message, error,
+        DBUS_TYPE_ARRAY, DBUS_TYPE_STRING, &match_rules, &n_match_rules,
+        DBUS_TYPE_UINT32, &flags,
+        DBUS_TYPE_INVALID))
+    goto out;
+
+  if (flags != 0)
+    {
+      dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
+          "BecomeMonitor does not support any flags yet");
+      goto out;
+    }
+
+  /* Special case: a zero-length array becomes [""] */
+  if (n_match_rules == 0)
+    {
+      match_rules = dbus_malloc (2 * sizeof (char *));
+
+      if (match_rules == NULL)
+        {
+          BUS_SET_OOM (error);
+          goto out;
+        }
+
+      match_rules[0] = _dbus_strdup ("");
+
+      if (match_rules[0] == NULL)
+        {
+          BUS_SET_OOM (error);
+          goto out;
+        }
+
+      match_rules[1] = NULL;
+      n_match_rules = 1;
+    }
+
+  for (i = 0; i < n_match_rules; i++)
+    {
+      _dbus_string_init_const (&str, match_rules[i]);
+      rule = bus_match_rule_parse (connection, &str, error);
+
+      if (rule == NULL)
+        {
+          BUS_SET_OOM (error);
+          goto out;
+        }
+
+      /* monitors always eavesdrop */
+      bus_match_rule_set_client_is_eavesdropping (rule, TRUE);
+
+      if (!_dbus_list_append (&rules, rule))
+        {
+          BUS_SET_OOM (error);
+          bus_match_rule_unref (rule);
+          goto out;
+        }
+    }
 
   /* Send the ack before we remove the rule, since the ack is undone
    * on transaction cancel, but becoming a monitor isn't.
    */
   if (!send_ack_reply (connection, transaction, message, error))
-    return FALSE;
+    goto out;
 
-  /* FIXME: use the array of filters from the message */
+  if (!bus_connection_be_monitor (connection, transaction, &rules, error))
+    goto out;
 
-  if (!bus_connection_be_monitor (connection, transaction, error))
-    return FALSE;
+  ret = TRUE;
 
-  return TRUE;
+out:
+  if (ret)
+    _DBUS_ASSERT_ERROR_IS_CLEAR (error);
+  else
+    _DBUS_ASSERT_ERROR_IS_SET (error);
+
+  for (iter = _dbus_list_get_first_link (&rules);
+      iter != NULL;
+      iter = _dbus_list_get_next_link (&rules, iter))
+    bus_match_rule_unref (iter->data);
+
+  _dbus_list_clear (&rules);
+
+  dbus_free_string_array (match_rules);
+  return ret;
 }
 
 typedef struct

doc/dbus-specification.xml

@@ -4642,6 +4642,13 @@
           adding the same rule with the <literal>eavesdrop</literal> match
           omitted.
         </para>
+
+        <para>
+          Eavesdropping interacts poorly with buses with non-trivial
+          access control restrictions. The
+          <xref linkend="bus-messages-become-monitor"/> method provides
+          an alternative way to monitor buses.
+        </para>
       </sect3>
 
       <sect3 id="message-bus-routing-match-rules">
@@ -6229,6 +6236,96 @@
         </para>
       </sect3>
 
+      <sect3 id="bus-messages-become-monitor">
+        <title><literal>org.freedesktop.DBus.Monitoring.BecomeMonitor</literal></title>
+        <para>
+          As a method:
+          <programlisting>
+            BecomeMonitor (in ARRAY of STRING rule, in UINT32 flags)
+          </programlisting>
+          Message arguments:
+          <informaltable>
+            <tgroup cols="3">
+              <thead>
+                <row>
+                  <entry>Argument</entry>
+                  <entry>Type</entry>
+                  <entry>Description</entry>
+                </row>
+              </thead>
+              <tbody>
+                <row>
+                  <entry>0</entry>
+                  <entry>ARRAY of STRING</entry>
+                  <entry>Match rules to add to the connection</entry>
+                </row>
+                <row>
+                  <entry>1</entry>
+                  <entry>UINT32</entry>
+                  <entry>Not used, must be 0</entry>
+                </row>
+              </tbody>
+            </tgroup>
+          </informaltable>
+        </para>
+
+        <para>
+          Converts the connection into a <emphasis>monitor
+            connection</emphasis> which can be used as a debugging/monitoring
+          tool. Only a user who is privileged on this
+          bus (by some implementation-specific definition) may create
+          monitor connections<footnote>
+            <para>
+              In the reference implementation,
+              the default configuration is that each user (identified by
+              numeric user ID) may monitor their own session bus,
+              and the root user (user ID zero) may monitor the
+              system bus.
+            </para>
+          </footnote>.
+       </para>
+
+       <para>
+         Monitor connections lose all their bus names, including the unique
+         connection name, and all their match rules. Sending messages on a
+         monitor connection is not allowed: applications should use a private
+         connection for monitoring.
+       </para>
+
+       <para>
+         Monitor connections may receive all messages, even messages that
+         should only have gone to some other connection ("eavesdropping").
+         The first argument is a list of match rules, which replace any
+         match rules that were previously active for this connection.
+         These match rules are always treated as if they contained the
+         special <literal>eavesdrop='true'</literal> member.
+       </para>
+
+       <para>
+         As a special case, an empty list of match rules (which would
+         otherwise match nothing, making the monitor useless) is treated
+         as a shorthand for matching all messages.
+       </para>
+
+       <para>
+         The second argument might be used for flags to influence the
+         behaviour of the monitor connection in future D-Bus versions.
+       </para>
+
+       <para>
+         Message bus implementations should attempt to minimize the
+         side-effects of monitoring — in particular, unlike ordinary
+         eavesdropping, monitoring the system bus does not require the
+         access control rules to be relaxed, which would change the set
+         of messages that can be delivered to their (non-monitor)
+         destinations. However, it is unavoidable that monitoring
+         will increase the message bus's resource consumption. In
+         edge cases where there was barely enough time or memory without
+         monitoring, this might result in message deliveries failing
+         when they would otherwise have succeeded.
+       </para>
+      </sect3>
+
     </sect2>
 
   </sect1>