From 1a6aa9a493ce249c3981366d7ae704768d09fa5f Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 15 Dec 2020 13:00:28 +0000 Subject: [PATCH] gdbus: Add flags to require authentication as the same user This eliminates a common use case for the `GDBusAuthObserver::authorize-authenticated-peer` signal, which is often implemented incorrectly by people. Suggested by Simon McVittie. Signed-off-by: Philip Withnall Fixes: #1804 --- gio/gdbusauth.c | 24 ++++++++++++++++++++---- gio/gdbusauth.h | 1 + gio/gdbusauthobserver.c | 4 +++- gio/gdbusconnection.c | 17 +++++++++++------ gio/gdbusserver.c | 9 +++++++-- gio/gioenums.h | 10 ++++++++-- 6 files changed, 50 insertions(+), 15 deletions(-) diff --git a/gio/gdbusauth.c b/gio/gdbusauth.c index 8a076ce20..c430f0cf0 100644 --- a/gio/gdbusauth.c +++ b/gio/gdbusauth.c @@ -924,6 +924,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth, GDBusAuthObserver *observer, const gchar *guid, gboolean allow_anonymous, + gboolean require_same_user, GDBusCapabilityFlags offered_capabilities, GDBusCapabilityFlags *out_negotiated_capabilities, GCredentials **out_received_credentials, @@ -941,6 +942,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth, gchar *s; GDBusCapabilityFlags negotiated_capabilities; GCredentials *credentials; + GCredentials *own_credentials = NULL; debug_print ("SERVER: initiating"); @@ -1039,6 +1041,8 @@ _g_dbus_auth_run_server (GDBusAuth *auth, debug_print ("SERVER: didn't receive any credentials"); } + own_credentials = g_credentials_new (); + state = SERVER_STATE_WAITING_FOR_AUTH; while (TRUE) { @@ -1155,10 +1159,21 @@ _g_dbus_auth_run_server (GDBusAuth *auth, switch (_g_dbus_auth_mechanism_server_get_state (mech)) { case G_DBUS_AUTH_MECHANISM_STATE_ACCEPTED: - if (observer != NULL && - !g_dbus_auth_observer_authorize_authenticated_peer (observer, - auth->priv->stream, - credentials)) + if (require_same_user && + (credentials == NULL || + !g_credentials_is_same_user (credentials, own_credentials, NULL))) + { + /* disconnect */ + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_FAILED, + _("User IDs must be the same for peer and server")); + goto out; + } + else if (observer != NULL && + !g_dbus_auth_observer_authorize_authenticated_peer (observer, + auth->priv->stream, + credentials)) { /* disconnect */ g_set_error_literal (error, @@ -1351,6 +1366,7 @@ _g_dbus_auth_run_server (GDBusAuth *auth, g_clear_object (&mech); g_clear_object (&dis); g_clear_object (&dos); + g_clear_object (&own_credentials); /* ensure return value is FALSE if error is set */ if (error != NULL && *error != NULL) diff --git a/gio/gdbusauth.h b/gio/gdbusauth.h index 2b4652bea..70b6a6039 100644 --- a/gio/gdbusauth.h +++ b/gio/gdbusauth.h @@ -67,6 +67,7 @@ gboolean _g_dbus_auth_run_server (GDBusAuth *auth, GDBusAuthObserver *observer, const gchar *guid, gboolean allow_anonymous, + gboolean require_same_user, GDBusCapabilityFlags offered_capabilities, GDBusCapabilityFlags *out_negotiated_capabilities, GCredentials **out_received_credentials, diff --git a/gio/gdbusauthobserver.c b/gio/gdbusauthobserver.c index f46c7d67d..7bafa94ec 100644 --- a/gio/gdbusauthobserver.c +++ b/gio/gdbusauthobserver.c @@ -70,7 +70,9 @@ * connections from any successfully authenticated user (but not from * anonymous connections using the `ANONYMOUS` mechanism). If you only * want to allow D-Bus connections from processes owned by the same uid - * as the server, you would use a signal handler like the following: + * as the server, since GLib 2.68, you should use the + * %G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER flag. It’s equivalent + * to the following signal handler: * * |[ * static gboolean diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index bad1ed4fe..a37611275 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -125,7 +125,8 @@ 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) + G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING | \ + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER) /** * SECTION:gdbusconnection @@ -2518,7 +2519,8 @@ initable_init (GInitable *initable, g_assert (connection->stream == NULL); if ((connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER) || - (connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS)) + (connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS) || + (connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER)) { g_set_error_literal (&connection->initialization_error, G_IO_ERROR, @@ -2553,6 +2555,7 @@ initable_init (GInitable *initable, connection->authentication_observer, connection->guid, (connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS), + (connection->flags & G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER), get_offered_capabilities_max (connection), &connection->capabilities, &connection->credentials, @@ -2838,8 +2841,9 @@ g_dbus_connection_new_sync (GIOStream *stream, * This constructor can only be used to initiate client-side * connections - use g_dbus_connection_new() if you need to act as the * server. In particular, @flags cannot contain the - * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER or - * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS flags. + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER, + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS or + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER flags. * * When the operation is finished, @callback will be invoked. You can * then call g_dbus_connection_new_for_address_finish() to get the result of @@ -2929,8 +2933,9 @@ g_dbus_connection_new_for_address_finish (GAsyncResult *res, * This constructor can only be used to initiate client-side * connections - use g_dbus_connection_new_sync() if you need to act * as the server. In particular, @flags cannot contain the - * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER or - * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS flags. + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER, + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS or + * %G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER flags. * * This is a synchronous failable constructor. See * g_dbus_connection_new_for_address() for the asynchronous version. diff --git a/gio/gdbusserver.c b/gio/gdbusserver.c index b3a5e9f63..3cf98587b 100644 --- a/gio/gdbusserver.c +++ b/gio/gdbusserver.c @@ -59,7 +59,8 @@ #define G_DBUS_SERVER_FLAGS_ALL \ (G_DBUS_SERVER_FLAGS_RUN_IN_THREAD | \ - G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS) + G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS | \ + G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER) /** * SECTION:gdbusserver @@ -81,7 +82,9 @@ * Note that a minimal #GDBusServer will accept connections from any * peer. In many use-cases it will be necessary to add a #GDBusAuthObserver * that only accepts connections that have successfully authenticated - * as the same user that is running the #GDBusServer. + * as the same user that is running the #GDBusServer. Since GLib 2.68 this can + * be achieved more simply by passing the + * %G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER flag to the server. */ /** @@ -1037,6 +1040,8 @@ on_run (GSocketService *service, G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING; if (server->flags & G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS) connection_flags |= G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS; + if (server->flags & G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER) + connection_flags |= G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER; connection = g_dbus_connection_new_sync (G_IO_STREAM (socket_connection), server->guid, diff --git a/gio/gioenums.h b/gio/gioenums.h index 2692b746d..5e83e9693 100644 --- a/gio/gioenums.h +++ b/gio/gioenums.h @@ -1206,6 +1206,8 @@ typedef enum * message bus. This means that the Hello() method will be invoked as part of the connection setup. * @G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING: If set, processing of D-Bus messages is * delayed until g_dbus_connection_start_message_processing() is called. + * @G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER: When authenticating + * as a server, require the UID of the peer to be the same as the UID of the server. (Since: 2.68) * * Flags used when creating a new #GDBusConnection. * @@ -1217,7 +1219,8 @@ typedef enum { G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_SERVER = (1<<1), G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS = (1<<2), G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION = (1<<3), - G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING = (1<<4) + G_DBUS_CONNECTION_FLAGS_DELAY_MESSAGE_PROCESSING = (1<<4), + G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER = (1<<5) } GDBusConnectionFlags; /** @@ -1368,6 +1371,8 @@ typedef enum * details). * @G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS: Allow the anonymous * authentication method. + * @G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER: Require the UID of the + * peer to be the same as the UID of the server when authenticating. (Since: 2.68) * * Flags used when creating a #GDBusServer. * @@ -1377,7 +1382,8 @@ typedef enum { G_DBUS_SERVER_FLAGS_NONE = 0, G_DBUS_SERVER_FLAGS_RUN_IN_THREAD = (1<<0), - G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS = (1<<1) + G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS = (1<<1), + G_DBUS_SERVER_FLAGS_AUTHENTICATION_REQUIRE_SAME_USER = (1<<2) } GDBusServerFlags; /**