From a7da87669601e45f535c33f7193e580e4f8e1d8a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 23 Jan 2015 19:14:39 +0000 Subject: [PATCH 2/6] 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. --- bus/activation.c | 11 +++ bus/connection.c | 175 ++++++++++++++++++++++++++++++++++++++++++--- bus/connection.h | 12 ++-- bus/dispatch.c | 45 ++++++++---- bus/driver.c | 99 +++++++++++++++++++++++-- doc/dbus-specification.xml | 68 ++++++++++++++++++ 6 files changed, 376 insertions(+), 34 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 9610c04..f0460ab 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -2007,6 +2007,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 e592808..9b61954 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -64,6 +64,7 @@ struct BusConnections int stamp; /**< Incrementing number */ BusExpireList *pending_replies; /**< List of pending replies */ DBusList *monitors; /**< List of all monitoring connections, a subset of completed */ + BusMatchmaker *monitor_matchmaker; #ifdef DBUS_ENABLE_STATS int total_match_rules; @@ -289,6 +290,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; } @@ -550,7 +556,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); @@ -1269,6 +1278,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))) @@ -2117,11 +2130,86 @@ 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; + + _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); + ret = bus_transaction_capture (transaction, NULL, reply); + 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 @@ -2146,14 +2234,31 @@ 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"); + } + dbus_error_free (&error); + return FALSE; + } return bus_transaction_send (transaction, connection, message); } @@ -2517,10 +2622,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; @@ -2538,9 +2686,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; @@ -2554,6 +2710,7 @@ bus_connection_be_monitor (DBusConnection *connection, 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 f8d6165..280fbf1 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 cb9d2a3..7bfb705 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -270,6 +270,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; @@ -295,13 +299,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 && @@ -381,14 +402,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 ec31c92..b6b55fb 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -214,6 +214,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); @@ -1791,27 +1794,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 1d23fa0..84854de 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,67 @@ + + <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. + + + + 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. + + + + The second argument might be used for flags to influence the + behaviour of the monitor connection in future D-Bus versions. + + + -- 2.1.4