From ba25c8a77099dc47f2f775d93a2414742c8d88b1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 8 Feb 2021 11:57:45 +0000 Subject: [PATCH] gdbus: Reject attempts to set future connection or server flags The GDBusConnectionFlags and GDBusServerFlags can affect how we carry out authentication and authorization, either making it more or less restrictive, so it's desirable to "fail closed" if a program is compiled against a new version of GLib but run against an old version. Signed-off-by: Simon McVittie --- gio/gdbusconnection.c | 11 +++ gio/gdbusserver.c | 5 ++ gio/tests/gdbus-peer.c | 187 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 203 insertions(+) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 520a9eca2..bad1ed4fe 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -120,6 +120,13 @@ #include "glibintl.h" +#define G_DBUS_CONNECTION_FLAGS_ALL \ + (G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT | \ + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER | \ + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS | \ + G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION | \ + G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING) + /** * SECTION:gdbusconnection * @short_description: D-Bus Connections @@ -2711,6 +2718,7 @@ g_dbus_connection_new (GIOStream *stream, _g_dbus_initialize (); g_return_if_fail (G_IS_IO_STREAM (stream)); + g_return_if_fail ((flags & ~G_DBUS_CONNECTION_FLAGS_ALL) == 0); g_async_initable_new_async (G_TYPE_DBUS_CONNECTION, G_PRIORITY_DEFAULT, @@ -2799,6 +2807,7 @@ g_dbus_connection_new_sync (GIOStream *stream, { _g_dbus_initialize (); g_return_val_if_fail (G_IS_IO_STREAM (stream), NULL); + g_return_val_if_fail ((flags & ~G_DBUS_CONNECTION_FLAGS_ALL) == 0, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); return g_initable_new (G_TYPE_DBUS_CONNECTION, cancellable, @@ -2856,6 +2865,7 @@ g_dbus_connection_new_for_address (const gchar *address, _g_dbus_initialize (); g_return_if_fail (address != NULL); + g_return_if_fail ((flags & ~G_DBUS_CONNECTION_FLAGS_ALL) == 0); g_async_initable_new_async (G_TYPE_DBUS_CONNECTION, G_PRIORITY_DEFAULT, @@ -2943,6 +2953,7 @@ g_dbus_connection_new_for_address_sync (const gchar *address, _g_dbus_initialize (); g_return_val_if_fail (address != NULL, NULL); + g_return_val_if_fail ((flags & ~G_DBUS_CONNECTION_FLAGS_ALL) == 0, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); return g_initable_new (G_TYPE_DBUS_CONNECTION, cancellable, diff --git a/gio/gdbusserver.c b/gio/gdbusserver.c index 9009734d5..b3a5e9f63 100644 --- a/gio/gdbusserver.c +++ b/gio/gdbusserver.c @@ -57,6 +57,10 @@ #include "glibintl.h" +#define G_DBUS_SERVER_FLAGS_ALL \ + (G_DBUS_SERVER_FLAGS_RUN_IN_THREAD | \ + G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS) + /** * SECTION:gdbusserver * @short_description: Helper for accepting connections @@ -512,6 +516,7 @@ g_dbus_server_new_sync (const gchar *address, g_return_val_if_fail (address != NULL, NULL); g_return_val_if_fail (g_dbus_is_guid (guid), NULL); + g_return_val_if_fail ((flags & ~G_DBUS_SERVER_FLAGS_ALL) == 0, NULL); g_return_val_if_fail (error == NULL || *error == NULL, NULL); server = g_initable_new (G_TYPE_DBUS_SERVER, diff --git a/gio/tests/gdbus-peer.c b/gio/tests/gdbus-peer.c index 23a22981a..34fab5713 100644 --- a/gio/tests/gdbus-peer.c +++ b/gio/tests/gdbus-peer.c @@ -1139,6 +1139,183 @@ test_peer (void) /* ---------------------------------------------------------------------------------------------------- */ +#define VALID_GUID "0123456789abcdef0123456789abcdef" + +static void +test_peer_invalid_server (void) +{ + GDBusServer *server; + + if (!g_test_undefined ()) + { + g_test_skip ("Not exercising programming errors"); + return; + } + + if (g_test_subprocess ()) + { + /* This assumes we are not going to run out of GDBusServerFlags + * any time soon */ + server = g_dbus_server_new_sync ("tcp:", (GDBusServerFlags) (1 << 30), + VALID_GUID, + NULL, NULL, NULL); + g_assert_null (server); + } + else + { + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*G_DBUS_SERVER_FLAGS_ALL*"); + } +} + +static void +test_peer_invalid_conn_stream_sync (void) +{ + GSocket *sock; + GSocketConnection *socket_conn; + GIOStream *iostream; + GDBusConnection *conn; + + if (!g_test_undefined ()) + { + g_test_skip ("Not exercising programming errors"); + return; + } + + sock = g_socket_new (G_SOCKET_FAMILY_IPV4, + G_SOCKET_TYPE_STREAM, + G_SOCKET_PROTOCOL_TCP, + NULL); + + if (sock == NULL) + { + g_test_skip ("TCP not available?"); + return; + } + + socket_conn = g_socket_connection_factory_create_connection (sock); + g_assert_nonnull (socket_conn); + iostream = G_IO_STREAM (socket_conn); + g_assert_nonnull (iostream); + + if (g_test_subprocess ()) + { + /* This assumes we are not going to run out of GDBusConnectionFlags + * any time soon */ + conn = g_dbus_connection_new_sync (iostream, VALID_GUID, + (GDBusConnectionFlags) (1 << 30), + NULL, NULL, NULL); + g_assert_null (conn); + } + else + { + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*G_DBUS_CONNECTION_FLAGS_ALL*"); + } + + g_clear_object (&sock); + g_clear_object (&socket_conn); +} + +static void +test_peer_invalid_conn_stream_async (void) +{ + GSocket *sock; + GSocketConnection *socket_conn; + GIOStream *iostream; + + if (!g_test_undefined ()) + { + g_test_skip ("Not exercising programming errors"); + return; + } + + sock = g_socket_new (G_SOCKET_FAMILY_IPV4, + G_SOCKET_TYPE_STREAM, + G_SOCKET_PROTOCOL_TCP, + NULL); + + if (sock == NULL) + { + g_test_skip ("TCP not available?"); + return; + } + + socket_conn = g_socket_connection_factory_create_connection (sock); + g_assert_nonnull (socket_conn); + iostream = G_IO_STREAM (socket_conn); + g_assert_nonnull (iostream); + + if (g_test_subprocess ()) + { + g_dbus_connection_new (iostream, VALID_GUID, + (GDBusConnectionFlags) (1 << 30), + NULL, NULL, NULL, NULL); + } + else + { + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*G_DBUS_CONNECTION_FLAGS_ALL*"); + } + + g_clear_object (&sock); + g_clear_object (&socket_conn); +} + +static void +test_peer_invalid_conn_addr_sync (void) +{ + GDBusConnection *conn; + + if (!g_test_undefined ()) + { + g_test_skip ("Not exercising programming errors"); + return; + } + + if (g_test_subprocess ()) + { + conn = g_dbus_connection_new_for_address_sync ("tcp:", + (GDBusConnectionFlags) (1 << 30), + NULL, NULL, NULL); + g_assert_null (conn); + } + else + { + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*G_DBUS_CONNECTION_FLAGS_ALL*"); + } +} + +static void +test_peer_invalid_conn_addr_async (void) +{ + if (!g_test_undefined ()) + { + g_test_skip ("Not exercising programming errors"); + return; + } + + if (g_test_subprocess ()) + { + g_dbus_connection_new_for_address ("tcp:", + (GDBusConnectionFlags) (1 << 30), + NULL, NULL, NULL, NULL); + } + else + { + g_test_trap_subprocess (NULL, 0, 0); + g_test_trap_assert_failed (); + g_test_trap_assert_stderr ("*CRITICAL*G_DBUS_CONNECTION_FLAGS_ALL*"); + } +} + +/* ---------------------------------------------------------------------------------------------------- */ + static void test_peer_signals (void) { @@ -2010,6 +2187,16 @@ main (int argc, test_interface_introspection_data = introspection_data->interfaces[0]; g_test_add_func ("/gdbus/peer-to-peer", test_peer); + g_test_add_func ("/gdbus/peer-to-peer/invalid/server", + test_peer_invalid_server); + g_test_add_func ("/gdbus/peer-to-peer/invalid/conn/stream/async", + test_peer_invalid_conn_stream_async); + g_test_add_func ("/gdbus/peer-to-peer/invalid/conn/stream/sync", + test_peer_invalid_conn_stream_sync); + g_test_add_func ("/gdbus/peer-to-peer/invalid/conn/addr/async", + test_peer_invalid_conn_addr_async); + g_test_add_func ("/gdbus/peer-to-peer/invalid/conn/addr/sync", + test_peer_invalid_conn_addr_sync); g_test_add_func ("/gdbus/peer-to-peer/signals", test_peer_signals); g_test_add_func ("/gdbus/delayed-message-processing", delayed_message_processing); g_test_add_func ("/gdbus/nonce-tcp", test_nonce_tcp);