From: Simon McVittie Date: Mon, 2 Feb 2015 19:45:17 +0000 (+0000) Subject: Capture all messages received or sent, and send them to monitors X-Git-Tag: dbus-1.9.10~15 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9fce7380331d24e8dd5fb9203eb8275ebb49e1d8;p=thirdparty%2Fdbus.git 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 --- diff --git a/bus/activation.c b/bus/activation.c index 138d69e1c..fb25d68e6 100644 --- a/bus/activation.c +++ b/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, diff --git a/bus/connection.c b/bus/connection.c index 1d9bfb2b4..410aaaca2 100644 --- a/bus/connection.c +++ b/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; diff --git a/bus/connection.h b/bus/connection.h index f8d616517..280fbf150 100644 --- a/bus/connection.h +++ b/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); diff --git a/bus/dispatch.c b/bus/dispatch.c index 97fe37129..630f814c5 100644 --- a/bus/dispatch.c +++ b/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); diff --git a/bus/driver.c b/bus/driver.c index ac29b9f13..793dbfb00 100644 --- a/bus/driver.c +++ b/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 diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml index 1d23fa048..2f9fef8b0 100644 --- a/doc/dbus-specification.xml +++ b/doc/dbus-specification.xml @@ -4642,6 +4642,13 @@ adding the same rule with the eavesdrop match omitted. + + + Eavesdropping interacts poorly with buses with non-trivial + access control restrictions. The + method provides + an alternative way to monitor buses. + @@ -6229,6 +6236,96 @@ + + <literal>org.freedesktop.DBus.Monitoring.BecomeMonitor</literal> + + As a method: + + BecomeMonitor (in ARRAY of STRING rule, in UINT32 flags) + + Message arguments: + + + + + Argument + Type + Description + + + + + 0 + ARRAY of STRING + Match rules to add to the connection + + + 1 + UINT32 + Not used, must be 0 + + + + + + + + Converts the connection into a monitor + connection 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 + + 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. + + . + + + + 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. + + + + 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 eavesdrop='true' member. + + + + 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. + + + + The second argument might be used for flags to influence the + behaviour of the monitor connection in future D-Bus versions. + + + + 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. + + +