From fd265663f2050e2b65d231c4f42fa6b9480c0eb7 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 8 Mar 2024 20:10:29 +0000 Subject: [PATCH] tests: Add test coverage for signals that match the message bus's name This is a special case of unique names, even though it's syntactically a well-known name. Signed-off-by: Simon McVittie --- gio/tests/gdbus-subscribe.c | 161 ++++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 7 deletions(-) diff --git a/gio/tests/gdbus-subscribe.c b/gio/tests/gdbus-subscribe.c index 350ec9f52..af100de7d 100644 --- a/gio/tests/gdbus-subscribe.c +++ b/gio/tests/gdbus-subscribe.c @@ -13,6 +13,7 @@ #define DBUS_SERVICE_DBUS "org.freedesktop.DBus" #define DBUS_PATH_DBUS "/org/freedesktop/DBus" #define DBUS_INTERFACE_DBUS DBUS_SERVICE_DBUS +#define NAME_OWNER_CHANGED "NameOwnerChanged" /* A signal that each connection emits to indicate that it has finished * emitting other signals */ @@ -101,6 +102,7 @@ typedef struct const char *iface; const char *member; const char *arg0; + const char *args; guint received_by_conn; guint received_by_proxy; } TestEmitSignal; @@ -120,6 +122,8 @@ typedef struct { const char *name; TestConn owner; + guint received_by_conn; + guint received_by_proxy; } TestOwnName; typedef enum @@ -461,6 +465,63 @@ static const TestPlan plan_limit_by_well_known_name = }, }; +static const TestPlan plan_limit_to_message_bus = +{ + .description = "A subscription to the message bus only accepts messages " + "from the message bus", + .steps = { + { + /* Subscriber wants to receive signals from the message bus itself */ + .action = TEST_ACTION_SUBSCRIBE, + .u.subscribe = { + .string_sender = DBUS_SERVICE_DBUS, + .path = DBUS_PATH_DBUS, + .iface = DBUS_INTERFACE_DBUS, + }, + }, + { + /* Attacker wants to trick subscriber into thinking that the message + * bus sent a signal */ + .action = TEST_ACTION_EMIT_SIGNAL, + .u.signal = { + .sender = TEST_CONN_ATTACKER, + .path = DBUS_PATH_DBUS, + .iface = DBUS_INTERFACE_DBUS, + .member = NAME_OWNER_CHANGED, + .arg0 = "would I lie to you?", + .received_by_conn = 0, + .received_by_proxy = 0 + }, + }, + { + /* Attacker tries harder, by sending a signal unicast directly to + * the subscriber, and using more realistic arguments */ + .action = TEST_ACTION_EMIT_SIGNAL, + .u.signal = { + .unicast_to = TEST_CONN_SUBSCRIBER, + .sender = TEST_CONN_ATTACKER, + .path = DBUS_PATH_DBUS, + .iface = DBUS_INTERFACE_DBUS, + .member = NAME_OWNER_CHANGED, + .args = "('com.example.Name', '', ':1.12')", + .received_by_conn = 0, + .received_by_proxy = 0 + }, + }, + { + /* When the message bus sends a signal (in this case triggered by + * owning a name), it should still get through */ + .action = TEST_ACTION_OWN_NAME, + .u.own_name = { + .name = OWNED_LATER_NAME, + .owner = TEST_CONN_SERVICE, + .received_by_conn = 1, + .received_by_proxy = 1 + }, + }, + }, +}; + typedef struct { const TestPlan *plan; @@ -591,7 +652,18 @@ fixture_received_signal (Fixture *f, } } - g_assert_cmpint (received->sender, !=, TEST_CONN_NONE); + if (g_str_equal (sender_name, DBUS_SERVICE_DBUS)) + { + g_test_message ("Signal received from message bus %s", + sender_name); + } + else + { + g_test_message ("Signal received from %s %s", + test_conn_descriptions[received->sender], + sender_name); + g_assert_cmpint (received->sender, !=, TEST_CONN_NONE); + } g_test_message ("Signal received from %s %s via %s", test_conn_descriptions[received->sender], @@ -607,13 +679,56 @@ fixture_received_signal (Fixture *f, g_test_message ("\tString argument 0: %s", received->arg0); g_test_message ("\tSent in step: %u", received->step); } - else + else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(uu)"))) { - g_assert_cmpstr (g_variant_get_type_string (parameters), ==, "(uu)"); g_variant_get (parameters, "(uu)", NULL, &received->step); g_test_message ("\tArgument 0: (not a string)"); g_test_message ("\tSent in step: %u", received->step); } + else if (g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(sss)"))) + { + const char *name; + const char *old_owner; + const char *new_owner; + + /* The only signal of this signature that we legitimately receive + * during this test is NameOwnerChanged, so just assert that it + * is from the message bus and can be matched to a plausible step. + * (This is less thorough than the above, and will not work if we + * add a test scenario where a name's ownership is repeatedly + * changed while watching NameOwnerChanged - so don't do that.) */ + g_assert_cmpstr (sender_name, ==, DBUS_SERVICE_DBUS); + g_assert_cmpstr (path, ==, DBUS_PATH_DBUS); + g_assert_cmpstr (iface, ==, DBUS_INTERFACE_DBUS); + g_assert_cmpstr (member, ==, NAME_OWNER_CHANGED); + + g_variant_get (parameters, "(&s&s&s)", &name, &old_owner, &new_owner); + + for (i = 0; i < G_N_ELEMENTS (f->plan->steps); i++) + { + const TestStep *step = &f->plan->steps[i]; + + if (step->action == TEST_ACTION_OWN_NAME) + { + const TestOwnName *own_name = &step->u.own_name; + + if (g_str_equal (name, own_name->name) + && g_str_equal (new_owner, f->unique_names[own_name->owner]) + && own_name->received_by_conn > 0) + { + received->step = i; + break; + } + } + + if (i >= G_N_ELEMENTS (f->plan->steps)) + g_error ("Could not match message to a test step"); + } + } + else + { + g_error ("Unexpected message received"); + } g_ptr_array_add (f->received, g_steal_pointer (&received)); } @@ -782,10 +897,15 @@ fixture_emit_signal (Fixture *f, * Otherwise put something that will not match any arg0. * Either way, put the sequence number in argument 1 so we can * correlate sent messages with received messages later. */ - if (signal->arg0 != NULL) + if (signal->args != NULL) + { + /* floating */ + body = g_variant_new_parsed (signal->args); + g_assert_nonnull (body); + } + else if (signal->arg0 != NULL) { g_test_message ("\tString argument 0: %s", signal->arg0); - /* floating */ body = g_variant_new ("(su)", signal->arg0, (guint32) step_number); } else @@ -933,8 +1053,6 @@ fixture_run_plan (Fixture *f, g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_conn)); g_assert_cmpuint (received->step, <, G_N_ELEMENTS (f->received_by_proxy)); - g_assert_cmpint (plan->steps[received->step].action, - ==, TEST_ACTION_EMIT_SIGNAL); if (received->received_by_proxy != NULL) f->received_by_proxy[received->step] += 1; @@ -974,6 +1092,34 @@ fixture_run_plan (Fixture *f, g_assert_cmpuint (f->received_by_proxy[i], ==, 0); } } + else if (step->action == TEST_ACTION_OWN_NAME) + { + const TestOwnName *own_name = &plan->steps[i].u.own_name; + + if (mode != SUBSCRIPTION_MODE_PROXY) + { + g_test_message ("NameOwnerChanged from step %u was received %u " + "times by GDBusConnection, expected %u", + i, f->received_by_conn[i], own_name->received_by_conn); + g_assert_cmpuint (f->received_by_conn[i], ==, own_name->received_by_conn); + } + else + { + g_assert_cmpuint (f->received_by_conn[i], ==, 0); + } + + if (mode != SUBSCRIPTION_MODE_CONN) + { + g_test_message ("NameOwnerChanged from step %u was received %u " + "times by GDBusProxy, expected %u", + i, f->received_by_proxy[i], own_name->received_by_proxy); + g_assert_cmpuint (f->received_by_proxy[i], ==, own_name->received_by_proxy); + } + else + { + g_assert_cmpuint (f->received_by_proxy[i], ==, 0); + } + } } } @@ -1100,6 +1246,7 @@ main (int argc, ADD_SUBSCRIBE_TEST (limit_by_unique_name); ADD_SUBSCRIBE_TEST (nonexistent_unique_name); ADD_SUBSCRIBE_TEST (limit_by_well_known_name); + ADD_SUBSCRIBE_TEST (limit_to_message_bus); return g_test_run(); }