Commit cc9bc56a authored by Simon McVittie's avatar Simon McVittie

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=92074Reviewed-by: Philip Withnall's avatarPhilip Withnall <philip.withnall@collabora.co.uk>
Reviewed-by: 's avatarLars Uebernickel <lars@uebernic.de>
(cherry picked from commit f3be583b)
Signed-off-by: Simon McVittie's avatarSimon McVittie <smcv@collabora.com>
parent 4a2e7024
......@@ -1967,6 +1967,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
......@@ -2015,11 +2016,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);
......@@ -2033,8 +2037,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),
message, error);
retval = bus_dispatch_matches (activation_transaction, NULL,
systemd, message, error);
}
else
{
......
......@@ -2208,6 +2208,7 @@ bus_transaction_get_context (BusTransaction *transaction)
dbus_bool_t
bus_transaction_capture (BusTransaction *transaction,
DBusConnection *sender,
DBusConnection *addressed_recipient,
DBusMessage *message)
{
BusConnections *connections;
......@@ -2227,8 +2228,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,
&recipients))
if (!bus_matchmaker_get_recipients (mm, connections, sender,
addressed_recipient, message, &recipients))
goto out;
for (link = _dbus_list_get_first_link (&recipients);
......@@ -2250,6 +2251,7 @@ out:
dbus_bool_t
bus_transaction_capture_error_reply (BusTransaction *transaction,
DBusConnection *addressed_recipient,
const DBusError *error,
DBusMessage *in_reply_to)
{
......@@ -2276,7 +2278,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);
......@@ -2318,7 +2320,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
......@@ -2330,7 +2332,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 "
......
......@@ -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,
......
......@@ -72,8 +72,8 @@ send_one_message (DBusConnection *connection,
message,
&stack_error))
{
if (!bus_transaction_capture_error_reply (transaction, &stack_error,
message))
if (!bus_transaction_capture_error_reply (transaction, sender,
&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,
message))
if (!bus_transaction_capture_error_reply (transaction, sender,
&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))
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 (service_name &&
strcmp (service_name, DBUS_SERVICE_DBUS) == 0) /* to bus driver */
{
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;
}
}
......
......@@ -257,7 +257,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);
......@@ -1026,7 +1026,7 @@ bus_driver_send_or_activate (BusTransaction *transaction,
activation = bus_context_get_activation (context);
if (!bus_transaction_capture (transaction, NULL, message))
if (!bus_transaction_capture (transaction, NULL, NULL, message))
{
BUS_SET_OOM (error);
_dbus_verbose ("No memory for bus_transaction_capture()");
......
......@@ -1888,8 +1888,17 @@ match_rule_matches (BusMatchRule *rule,
if (addressed_recipient == NULL)
{
if (strcmp (rule->destination,
DBUS_SERVICE_DBUS) != 0)
/* If the message is going to be delivered to the dbus-daemon
* 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
......
......@@ -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)
match_rules = f->config->match_rules;
if (config == NULL)
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,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment