From ddba31cf55265f4dbb06fe76ac4a872df4304622 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 13:30:08 +0100 Subject: [PATCH 01/12] device: Removes the Device portal The Device portal is, to the best of our knowledge, not used by any component. It's also weird because it's not a portal expoed to clients which caused confusion. The service it provides is also provided by the permission store, minus the ability to map from arbitrary PID to an app id. This PID to app id mapping isn't something that can be done in general and is most likely broken when the PID is not of the xdg-dbus-proxy. Let's just remove it entirely. --- data/meson.build | 1 - data/org.freedesktop.portal.Device.xml | 68 ----- src/camera.c | 115 +++++++- src/camera.h | 1 + src/device.c | 370 ------------------------- src/device.h | 37 --- src/meson.build | 1 - src/xdg-desktop-portal.c | 9 +- 8 files changed, 112 insertions(+), 490 deletions(-) delete mode 100644 data/org.freedesktop.portal.Device.xml delete mode 100644 src/device.c delete mode 100644 src/device.h diff --git a/data/meson.build b/data/meson.build index eea49adda..4e601077e 100644 --- a/data/meson.build +++ b/data/meson.build @@ -9,7 +9,6 @@ portal_sources = files( 'org.freedesktop.portal.Background.xml', 'org.freedesktop.portal.Camera.xml', 'org.freedesktop.portal.Clipboard.xml', - 'org.freedesktop.portal.Device.xml', 'org.freedesktop.portal.Documents.xml', 'org.freedesktop.portal.DynamicLauncher.xml', 'org.freedesktop.portal.Email.xml', diff --git a/data/org.freedesktop.portal.Device.xml b/data/org.freedesktop.portal.Device.xml deleted file mode 100644 index 2f981cbb1..000000000 --- a/data/org.freedesktop.portal.Device.xml +++ /dev/null @@ -1,68 +0,0 @@ - - - - - - - - - - - - - - - - diff --git a/src/camera.c b/src/camera.c index c358f099d..71ef11312 100644 --- a/src/camera.c +++ b/src/camera.c @@ -23,7 +23,6 @@ #include #include -#include "device.h" #include "request.h" #include "permissions.h" #include "pipewire.h" @@ -31,7 +30,11 @@ #include "xdp-impl-dbus.h" #include "xdp-utils.h" +#define PERMISSION_TABLE "devices" +#define PERMISSION_DEVICE_CAMERA "camera" + static XdpDbusImplLockdown *lockdown; +static XdpDbusImplAccess *access_impl; typedef struct _Camera Camera; typedef struct _CameraClass CameraClass; @@ -66,6 +69,90 @@ static gboolean create_pipewire_remote (Camera *camera, GError **error); +static gboolean +query_permission_sync (Request *request) +{ + Permission permission; + const char *app_id; + gboolean allowed; + + app_id = (const char *)g_object_get_data (G_OBJECT (request), "app-id"); + + permission = get_permission_sync (app_id, PERMISSION_TABLE, PERMISSION_DEVICE_CAMERA); + if (permission == PERMISSION_ASK || permission == PERMISSION_UNSET) + { + GVariantBuilder opt_builder; + g_autofree char *title = NULL; + g_autofree char *body = NULL; + guint32 response = 2; + g_autoptr(GVariant) results = NULL; + g_autoptr(GError) error = NULL; + g_autoptr(GAppInfo) info = NULL; + g_autoptr(XdpDbusImplRequest) impl_request = NULL; + + if (app_id[0] != 0) + { + g_autofree char *desktop_id; + desktop_id = g_strconcat (app_id, ".desktop", NULL); + info = (GAppInfo*)g_desktop_app_info_new (desktop_id); + } + + g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT); + g_variant_builder_add (&opt_builder, "{sv}", "icon", g_variant_new_string ("camera-web-symbolic")); + + if (info) + { + title = g_strdup_printf (_("Allow %s to Use the Camera?"), g_app_info_get_display_name (info)); + body = g_strdup_printf (_("%s wants to access camera devices."), g_app_info_get_display_name (info)); + } + else + { + title = g_strdup (_("Allow app to Use the Camera?")); + body = g_strdup (_("An app wants to access camera devices.")); + } + + impl_request = xdp_dbus_impl_request_proxy_new_sync (g_dbus_proxy_get_connection (G_DBUS_PROXY (access_impl)), + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, + g_dbus_proxy_get_name (G_DBUS_PROXY (access_impl)), + request->id, + NULL, &error); + if (!impl_request) + return FALSE; + + request_set_impl_request (request, impl_request); + + g_debug ("Calling backend for device access to camera"); + + if (!xdp_dbus_impl_access_call_access_dialog_sync (access_impl, + request->id, + app_id, + "", + title, + "", + body, + g_variant_builder_end (&opt_builder), + &response, + &results, + NULL, + &error)) + { + g_warning ("A backend call failed: %s", error->message); + } + + allowed = response == 0; + + if (permission == PERMISSION_UNSET) + { + set_permission_sync (app_id, PERMISSION_TABLE, PERMISSION_DEVICE_CAMERA, + allowed ? PERMISSION_YES : PERMISSION_NO); + } + } + else + allowed = permission == PERMISSION_YES ? TRUE : FALSE; + + return allowed; +} + static void handle_access_camera_in_thread_func (GTask *task, gpointer source_object, @@ -73,12 +160,9 @@ handle_access_camera_in_thread_func (GTask *task, GCancellable *cancellable) { Request *request = (Request *)task_data; - const char *app_id; gboolean allowed; - app_id = (const char *)g_object_get_data (G_OBJECT (request), "app-id"); - - allowed = device_query_permission_sync (app_id, "camera", request); + allowed = query_permission_sync (request); REQUEST_AUTOLOCK (request); @@ -198,7 +282,7 @@ handle_open_pipewire_remote (XdpDbusCamera *object, app_info = xdp_invocation_lookup_app_info_sync (invocation, NULL, &error); app_id = xdp_app_info_get_id (app_info); - permission = device_get_permission_sync (app_id, "camera"); + permission = get_permission_sync (app_id, PERMISSION_TABLE, PERMISSION_DEVICE_CAMERA); if (permission != PERMISSION_YES) { g_dbus_method_invocation_return_error (invocation, @@ -445,11 +529,28 @@ camera_class_init (CameraClass *klass) GDBusInterfaceSkeleton * camera_create (GDBusConnection *connection, - gpointer lockdown_proxy) + const char *access_impl_dbus_name, + gpointer lockdown_proxy) { + g_autoptr(GError) error = NULL; + lockdown = lockdown_proxy; camera = g_object_new (camera_get_type (), NULL); + access_impl = xdp_dbus_impl_access_proxy_new_sync (connection, + G_DBUS_PROXY_FLAGS_NONE, + access_impl_dbus_name, + DESKTOP_PORTAL_OBJECT_PATH, + NULL, + &error); + if (access_impl == NULL) + { + g_warning ("Failed to create access proxy: %s", error->message); + return NULL; + } + + g_dbus_proxy_set_default_timeout (G_DBUS_PROXY (access_impl), G_MAXINT); + return G_DBUS_INTERFACE_SKELETON (camera); } diff --git a/src/camera.h b/src/camera.h index 8163a9973..8690b7b07 100644 --- a/src/camera.h +++ b/src/camera.h @@ -21,4 +21,5 @@ #include GDBusInterfaceSkeleton * camera_create (GDBusConnection *connection, + const char *access_impl_dbus_name, gpointer lockdown_proxy); diff --git a/src/device.c b/src/device.c deleted file mode 100644 index 87a954897..000000000 --- a/src/device.c +++ /dev/null @@ -1,370 +0,0 @@ -/* - * Copyright © 2016 Red Hat, Inc - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see . - * - * Authors: - * Matthias Clasen - */ - -#include "config.h" - -#include -#include -#include - -#include -#include -#include - -#include -#include -#include - -#include "device.h" -#include "request.h" -#include "permissions.h" -#include "xdp-dbus.h" -#include "xdp-impl-dbus.h" -#include "xdp-utils.h" - -#define PERMISSION_TABLE "devices" - -typedef struct _Device Device; -typedef struct _DeviceClass DeviceClass; - -struct _Device -{ - XdpDbusDeviceSkeleton parent_instance; -}; - -struct _DeviceClass -{ - XdpDbusDeviceSkeletonClass parent_class; -}; - -static XdpDbusImplAccess *impl; -static Device *device; -static XdpDbusImplLockdown *lockdown; - -GType device_get_type (void) G_GNUC_CONST; -static void device_iface_init (XdpDbusDeviceIface *iface); - -G_DEFINE_TYPE_WITH_CODE (Device, device, XDP_DBUS_TYPE_DEVICE_SKELETON, - G_IMPLEMENT_INTERFACE (XDP_DBUS_TYPE_DEVICE, - device_iface_init)); - -static const char *known_devices[] = { - "microphone", - "speakers", - "camera", - NULL -}; - -Permission -device_get_permission_sync (const char *app_id, - const char *device) -{ - return get_permission_sync (app_id, PERMISSION_TABLE, device); -} - -gboolean -device_query_permission_sync (const char *app_id, - const char *device, - Request *request) -{ - Permission permission; - gboolean allowed; - - permission = device_get_permission_sync (app_id, device); - if (permission == PERMISSION_ASK || permission == PERMISSION_UNSET) - { - GVariantBuilder opt_builder; - g_autofree char *title = NULL; - g_autofree char *body = NULL; - guint32 response = 2; - g_autoptr(GVariant) results = NULL; - g_autoptr(GError) error = NULL; - g_autoptr(GAppInfo) info = NULL; - g_autoptr(XdpDbusImplRequest) impl_request = NULL; - - if (app_id[0] != 0) - { - g_autofree char *desktop_id; - desktop_id = g_strconcat (app_id, ".desktop", NULL); - info = (GAppInfo*)g_desktop_app_info_new (desktop_id); - } - - g_variant_builder_init (&opt_builder, G_VARIANT_TYPE_VARDICT); - - if (strcmp (device, "microphone") == 0) - { - g_variant_builder_add (&opt_builder, "{sv}", "icon", g_variant_new_string ("audio-input-microphone-symbolic")); - - if (info) - { - title = g_strdup_printf (_("Allow %s to Use the Microphone?"), g_app_info_get_display_name (info)); - body = g_strdup_printf (_("%s wants to access recording devices."), g_app_info_get_display_name (info)); - } - else - { - title = g_strdup (_("Allow app to Use the Microphone?")); - body = g_strdup (_("An app wants to access recording devices.")); - } - } - else if (strcmp (device, "speakers") == 0) - { - g_variant_builder_add (&opt_builder, "{sv}", "icon", g_variant_new_string ("audio-speakers-symbolic")); - - if (info) - { - title = g_strdup_printf (_("Allow %s to Use the Speakers?"), g_app_info_get_display_name (info)); - body = g_strdup_printf (_("%s wants to access audio devices."), g_app_info_get_display_name (info)); - } - else - { - title = g_strdup (_("Allow app to Use the Speakers?")); - body = g_strdup (_("An app wants to access audio devices.")); - } - } - else if (strcmp (device, "camera") == 0) - { - g_variant_builder_add (&opt_builder, "{sv}", "icon", g_variant_new_string ("camera-web-symbolic")); - - if (info) - { - title = g_strdup_printf (_("Allow %s to Use the Camera?"), g_app_info_get_display_name (info)); - body = g_strdup_printf (_("%s wants to access camera devices."), g_app_info_get_display_name (info)); - } - else - { - title = g_strdup (_("Allow app to Use the Camera?")); - body = g_strdup (_("An app wants to access camera devices.")); - } - } - - impl_request = xdp_dbus_impl_request_proxy_new_sync (g_dbus_proxy_get_connection (G_DBUS_PROXY (impl)), - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, - g_dbus_proxy_get_name (G_DBUS_PROXY (impl)), - request->id, - NULL, &error); - if (!impl_request) - return FALSE; - - request_set_impl_request (request, impl_request); - - g_debug ("Calling backend for device access to: %s", device); - - if (!xdp_dbus_impl_access_call_access_dialog_sync (impl, - request->id, - app_id, - "", - title, - "", - body, - g_variant_builder_end (&opt_builder), - &response, - &results, - NULL, - &error)) - { - g_warning ("A backend call failed: %s", error->message); - } - - allowed = response == 0; - - if (permission == PERMISSION_UNSET) - set_permission_sync (app_id, PERMISSION_TABLE, device, allowed ? PERMISSION_YES : PERMISSION_NO); - } - else - allowed = permission == PERMISSION_YES ? TRUE : FALSE; - - return allowed; -} - -static void -handle_access_device_in_thread (GTask *task, - gpointer source_object, - gpointer task_data, - GCancellable *cancellable) -{ - Request *request = (Request *)task_data; - const char *app_id; - const char *device; - gboolean allowed; - - REQUEST_AUTOLOCK (request); - - app_id = (const char *)g_object_get_data (G_OBJECT (request), "app-id"); - device = (const char *)g_object_get_data (G_OBJECT (request), "device"); - - allowed = device_query_permission_sync (app_id, device, request); - - if (request->exported) - { - GVariantBuilder results; - - g_variant_builder_init (&results, G_VARIANT_TYPE_VARDICT); - xdp_dbus_request_emit_response (XDP_DBUS_REQUEST (request), - allowed ? XDG_DESKTOP_PORTAL_RESPONSE_SUCCESS - : XDG_DESKTOP_PORTAL_RESPONSE_CANCELLED, - g_variant_builder_end (&results)); - request_unexport (request); - } -} - -static gboolean -handle_access_device (XdpDbusDevice *object, - GDBusMethodInvocation *invocation, - guint32 pid, - const char * const *devices, - GVariant *arg_options) -{ - Request *request = request_from_invocation (invocation); - g_autoptr(XdpAppInfo) app_info = NULL; - g_autoptr(GError) error = NULL; - g_autoptr(XdpDbusImplRequest) impl_request = NULL; - g_autoptr(GTask) task = NULL; - - if (g_strv_length ((char **)devices) != 1 || !g_strv_contains (known_devices, devices[0])) - { - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_INVALID_ARGUMENT, - "Invalid devices requested"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - - if (g_str_equal (devices[0], "microphone") && - xdp_dbus_impl_lockdown_get_disable_microphone (lockdown)) - { - g_debug ("Microphone access disabled"); - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_NOT_ALLOWED, - "Microphone access disabled"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - if (g_str_equal (devices[0], "camera") && - xdp_dbus_impl_lockdown_get_disable_camera (lockdown)) - { - g_debug ("Camera access disabled"); - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_NOT_ALLOWED, - "Camera access disabled"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - if (g_str_equal (devices[0], "speakers") && - xdp_dbus_impl_lockdown_get_disable_sound_output (lockdown)) - { - g_debug ("Speaker access disabled"); - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_NOT_ALLOWED, - "Speaker access disabled"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - - REQUEST_AUTOLOCK (request); - - if (!xdp_app_info_is_host (request->app_info)) - { - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_NOT_ALLOWED, - "This call is not available inside the sandbox"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - - app_info = xdp_get_app_info_from_pid (pid, &error); - if (app_info == NULL) - { - g_dbus_method_invocation_return_error (invocation, - XDG_DESKTOP_PORTAL_ERROR, - XDG_DESKTOP_PORTAL_ERROR_INVALID_ARGUMENT, - "Invalid pid requested"); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - - g_object_set_data_full (G_OBJECT (request), "app-id", g_strdup (xdp_app_info_get_id (app_info)), g_free); - g_object_set_data_full (G_OBJECT (request), "device", g_strdup (devices[0]), g_free); - - impl_request = xdp_dbus_impl_request_proxy_new_sync (g_dbus_proxy_get_connection (G_DBUS_PROXY (impl)), - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES, - g_dbus_proxy_get_name (G_DBUS_PROXY (impl)), - request->id, - NULL, &error); - if (!impl_request) - { - g_dbus_method_invocation_return_gerror (invocation, error); - return G_DBUS_METHOD_INVOCATION_HANDLED; - } - - request_set_impl_request (request, impl_request); - request_export (request, g_dbus_method_invocation_get_connection (invocation)); - - xdp_dbus_device_complete_access_device (object, invocation, request->id); - - task = g_task_new (object, NULL, NULL, NULL); - g_task_set_task_data (task, g_object_ref (request), g_object_unref); - g_task_run_in_thread (task, handle_access_device_in_thread); - - return G_DBUS_METHOD_INVOCATION_HANDLED; -} - -static void -device_iface_init (XdpDbusDeviceIface *iface) -{ - iface->handle_access_device = handle_access_device; -} - -static void -device_init (Device *device) -{ - xdp_dbus_device_set_version (XDP_DBUS_DEVICE (device), 1); -} - -static void -device_class_init (DeviceClass *klass) -{ -} - -GDBusInterfaceSkeleton * -device_create (GDBusConnection *connection, - const char *dbus_name, - gpointer lockdown_proxy) -{ - g_autoptr(GError) error = NULL; - - lockdown = lockdown_proxy; - - impl = xdp_dbus_impl_access_proxy_new_sync (connection, - G_DBUS_PROXY_FLAGS_NONE, - dbus_name, - DESKTOP_PORTAL_OBJECT_PATH, - NULL, - &error); - if (impl == NULL) - { - g_warning ("Failed to create access proxy: %s", error->message); - return NULL; - } - - g_dbus_proxy_set_default_timeout (G_DBUS_PROXY (impl), G_MAXINT); - - device = g_object_new (device_get_type (), NULL); - - return G_DBUS_INTERFACE_SKELETON (device); -} diff --git a/src/device.h b/src/device.h deleted file mode 100644 index 74d1efb8b..000000000 --- a/src/device.h +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright © 2016 Red Hat, Inc - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 2 of the License, or (at your option) any later version. - * - * This library is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public - * License along with this library. If not, see . - * - * Authors: - * Matthias Clasen - */ - -#pragma once - -#include - -#include "request.h" -#include "permissions.h" - -Permission device_get_permission_sync (const char *app_id, - const char *device); - -gboolean device_query_permission_sync (const char *app_id, - const char *device, - Request *request); - -GDBusInterfaceSkeleton * device_create (GDBusConnection *connection, - const char *dbus_name, - gpointer lockdown); diff --git a/src/meson.build b/src/meson.build index 0bdc94c26..693501ff9 100644 --- a/src/meson.build +++ b/src/meson.build @@ -67,7 +67,6 @@ xdg_desktop_portal_sources = files( 'call.c', 'camera.c', 'clipboard.c', - 'device.c', 'documents.c', 'dynamic-launcher.c', 'email.c', diff --git a/src/xdg-desktop-portal.c b/src/xdg-desktop-portal.c index fdac56ef8..866552755 100644 --- a/src/xdg-desktop-portal.c +++ b/src/xdg-desktop-portal.c @@ -37,7 +37,6 @@ #include "call.h" #include "camera.h" #include "clipboard.h" -#include "device.h" #include "documents.h" #include "dynamic-launcher.h" #include "email.h" @@ -262,10 +261,6 @@ on_bus_acquired (GDBusConnection *connection, { PortalImplementation *tmp; - export_portal_implementation (connection, - device_create (connection, - access_impl->dbus_name, - lockdown)); #ifdef HAVE_GEOCLUE export_portal_implementation (connection, location_create (connection, @@ -274,7 +269,9 @@ on_bus_acquired (GDBusConnection *connection, #endif export_portal_implementation (connection, - camera_create (connection, lockdown)); + camera_create (connection, + access_impl->dbus_name, + lockdown)); tmp = find_portal_implementation ("org.freedesktop.impl.portal.Screenshot"); if (tmp != NULL) From e324f1c234d6e756e2e64e08116d78fb403f4565 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:03:00 +0100 Subject: [PATCH 02/12] utils: Make unused functions private One was a left-over and for the other we just removed the last users. --- src/xdp-utils.c | 5 ++--- src/xdp-utils.h | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 6fad0417e..7bf0064d4 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -489,7 +489,7 @@ xdp_app_info_is_host (XdpAppInfo *app_info) return app_info->kind == XDP_APP_INFO_KIND_HOST; } -gboolean +static gboolean xdp_app_info_supports_opath (XdpAppInfo *app_info) { return @@ -827,8 +827,7 @@ parse_app_info_from_snap (pid_t pid, GError **error) return g_steal_pointer (&app_info); } - -XdpAppInfo * +static XdpAppInfo * xdp_get_app_info_from_pid (pid_t pid, GError **error) { diff --git a/src/xdp-utils.h b/src/xdp-utils.h index c92eea0a8..31055451b 100644 --- a/src/xdp-utils.h +++ b/src/xdp-utils.h @@ -80,7 +80,6 @@ const char *xdp_app_info_get_id (XdpAppInfo *app_info); char * xdp_app_info_get_instance (XdpAppInfo *app_info); gboolean xdp_app_info_is_host (XdpAppInfo *app_info); XdpAppInfoKind xdp_app_info_get_kind (XdpAppInfo *app_info); -gboolean xdp_app_info_supports_opath (XdpAppInfo *app_info); char * xdp_app_info_remap_path (XdpAppInfo *app_info, const char *path); gboolean xdp_app_info_map_pids (XdpAppInfo *app_info, @@ -104,8 +103,6 @@ char * xdp_app_info_get_path_for_fd (XdpAppInfo *app_info, gboolean *writable_out, GError **error); gboolean xdp_app_info_has_network (XdpAppInfo *app_info); -XdpAppInfo *xdp_get_app_info_from_pid (pid_t pid, - GError **error); GAppInfo * xdp_app_info_load_app_info (XdpAppInfo *app_info); char ** xdp_app_info_rewrite_commandline (XdpAppInfo *app_info, const char *const *commandline, From cbaa43308de6966ec231722c4279e99180d31fa4 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:04:44 +0100 Subject: [PATCH 03/12] utils: Add xdp_app_info_is_flatpak This will become useful once we support the dbus Containers1 interface and a new XDP_APP_INFO kind can still refer to a flatpak. --- src/dynamic-launcher.c | 4 ++-- src/xdp-utils.c | 8 ++++++++ src/xdp-utils.h | 1 + 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/dynamic-launcher.c b/src/dynamic-launcher.c index c7c50919b..f591f5b28 100644 --- a/src/dynamic-launcher.c +++ b/src/dynamic-launcher.c @@ -257,7 +257,7 @@ save_icon_and_get_desktop_entry (const char *desktop_file_id, return NULL; /* Don't let the app give itself access to host files */ - if (xdp_app_info_get_kind (xdp_app_info) == XDP_APP_INFO_KIND_FLATPAK && + if (xdp_app_info_is_flatpak (xdp_app_info) && g_strv_contains ((const char * const *)exec_strv, "--file-forwarding")) { g_set_error (error, @@ -284,7 +284,7 @@ save_icon_and_get_desktop_entry (const char *desktop_file_id, if (tryexec_path != NULL) g_key_file_set_value (key_file, G_KEY_FILE_DESKTOP_GROUP, "TryExec", tryexec_path); - if (xdp_app_info_get_kind (xdp_app_info) == XDP_APP_INFO_KIND_FLATPAK) + if (xdp_app_info_is_flatpak (xdp_app_info)) { /* Flatpak checks for this key */ g_key_file_set_value (key_file, G_KEY_FILE_DESKTOP_GROUP, "X-Flatpak", app_id); diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 7bf0064d4..4d24c823a 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -489,6 +489,14 @@ xdp_app_info_is_host (XdpAppInfo *app_info) return app_info->kind == XDP_APP_INFO_KIND_HOST; } +gboolean +xdp_app_info_is_flatpak (XdpAppInfo *app_info) +{ + g_return_val_if_fail (app_info != NULL, FALSE); + + return app_info->kind == XDP_APP_INFO_KIND_FLATPAK; +} + static gboolean xdp_app_info_supports_opath (XdpAppInfo *app_info) { diff --git a/src/xdp-utils.h b/src/xdp-utils.h index 31055451b..2f4f2c8ab 100644 --- a/src/xdp-utils.h +++ b/src/xdp-utils.h @@ -79,6 +79,7 @@ void xdp_app_info_unref (XdpAppInfo *app_info); const char *xdp_app_info_get_id (XdpAppInfo *app_info); char * xdp_app_info_get_instance (XdpAppInfo *app_info); gboolean xdp_app_info_is_host (XdpAppInfo *app_info); +gboolean xdp_app_info_is_flatpak (XdpAppInfo *app_info); XdpAppInfoKind xdp_app_info_get_kind (XdpAppInfo *app_info); char * xdp_app_info_remap_path (XdpAppInfo *app_info, const char *path); From db0b5c55a92e87de3915c2b4e6e546ff674b8152 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:22:45 +0100 Subject: [PATCH 04/12] utils: Move PID lookup into its own function --- src/xdp-utils.c | 82 ++++++++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 31 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 4d24c823a..8964dcf97 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -835,12 +835,60 @@ parse_app_info_from_snap (pid_t pid, GError **error) return g_steal_pointer (&app_info); } +static gboolean +xdp_connection_get_pid (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + guint32 *out_pid, + GError **error) +{ + g_autoptr(GDBusMessage) msg = NULL; + g_autoptr(GDBusMessage) reply = NULL; + g_autoptr(XdpAppInfo) app_info = NULL; + GVariant *body; + + msg = g_dbus_message_new_method_call (DBUS_NAME_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetConnectionUnixProcessID"); + g_dbus_message_set_body (msg, g_variant_new ("(s)", sender)); + + reply = g_dbus_connection_send_message_with_reply_sync (connection, msg, + G_DBUS_SEND_MESSAGE_FLAGS_NONE, + 30000, + NULL, + cancellable, + error); + if (reply == NULL) + return FALSE; + + if (g_dbus_message_get_message_type (reply) == G_DBUS_MESSAGE_TYPE_ERROR) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer pid"); + return FALSE; + } + + body = g_dbus_message_get_body (reply); + g_variant_get (body, "(u)", out_pid); + + return TRUE; +} + static XdpAppInfo * -xdp_get_app_info_from_pid (pid_t pid, - GError **error) +xdp_get_app_info_from_pid (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; g_autoptr(GError) local_error = NULL; + guint32 pid; + + if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, &local_error)) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return NULL; + } app_info = parse_app_info_from_flatpak_info (pid, &local_error); if (app_info == NULL && local_error) @@ -888,41 +936,13 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, GCancellable *cancellable, GError **error) { - g_autoptr(GDBusMessage) msg = NULL; - g_autoptr(GDBusMessage) reply = NULL; g_autoptr(XdpAppInfo) app_info = NULL; - GVariant *body; - guint32 pid = 0; app_info = lookup_cached_app_info_by_sender (sender); if (app_info) return g_steal_pointer (&app_info); - msg = g_dbus_message_new_method_call (DBUS_NAME_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "GetConnectionUnixProcessID"); - g_dbus_message_set_body (msg, g_variant_new ("(s)", sender)); - - reply = g_dbus_connection_send_message_with_reply_sync (connection, msg, - G_DBUS_SEND_MESSAGE_FLAGS_NONE, - 30000, - NULL, - cancellable, - error); - if (reply == NULL) - return NULL; - - if (g_dbus_message_get_message_type (reply) == G_DBUS_MESSAGE_TYPE_ERROR) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer app id"); - return NULL; - } - - body = g_dbus_message_get_body (reply); - g_variant_get (body, "(u)", &pid); - - app_info = xdp_get_app_info_from_pid (pid, error); + app_info = xdp_get_app_info_from_pid (connection, sender, cancellable, error); if (app_info == NULL) return NULL; From ab49c03e7d89afcb170323979c1dea232c744464 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:28:26 +0100 Subject: [PATCH 05/12] utils: Refactor xdp_get_app_info_from_pid --- src/xdp-utils.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 8964dcf97..c94e81f1b 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -881,31 +881,21 @@ xdp_get_app_info_from_pid (GDBusConnection *connection, GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; - g_autoptr(GError) local_error = NULL; guint32 pid; - if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, &local_error)) - { - g_propagate_error (error, g_steal_pointer (&local_error)); - return NULL; - } + if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, error)) + return NULL; - app_info = parse_app_info_from_flatpak_info (pid, &local_error); - if (app_info == NULL && local_error) - { - g_propagate_error (error, g_steal_pointer (&local_error)); - return NULL; - } + app_info = parse_app_info_from_flatpak_info (pid, error); + + if (app_info == NULL && *error) + return NULL; if (app_info == NULL) - { - app_info = parse_app_info_from_snap (pid, &local_error); - if (app_info == NULL && local_error) - { - g_propagate_error (error, g_steal_pointer (&local_error)); - return NULL; - } - } + app_info = parse_app_info_from_snap (pid, error); + + if (app_info == NULL && *error) + return NULL; if (app_info == NULL) app_info = xdp_app_info_new_host (pid); From 0d6d151b1df6e4e80ae43745d7d64c95f2ad6509 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:33:49 +0100 Subject: [PATCH 06/12] utils: Move xdp_get_app_info_from_pid to xdp_connection_lookup_app_info_sync This will make it easier to follow which method of identiying an app will be used under what conditions. --- src/xdp-utils.c | 45 +++++++++++++++------------------------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index c94e81f1b..23d893a30 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -874,35 +874,6 @@ xdp_connection_get_pid (GDBusConnection *connection, return TRUE; } -static XdpAppInfo * -xdp_get_app_info_from_pid (GDBusConnection *connection, - const char *sender, - GCancellable *cancellable, - GError **error) -{ - g_autoptr(XdpAppInfo) app_info = NULL; - guint32 pid; - - if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, error)) - return NULL; - - app_info = parse_app_info_from_flatpak_info (pid, error); - - if (app_info == NULL && *error) - return NULL; - - if (app_info == NULL) - app_info = parse_app_info_from_snap (pid, error); - - if (app_info == NULL && *error) - return NULL; - - if (app_info == NULL) - app_info = xdp_app_info_new_host (pid); - - return g_steal_pointer (&app_info); -} - static XdpAppInfo * lookup_cached_app_info_by_sender (const char *sender) { @@ -927,15 +898,29 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; + guint32 pid; app_info = lookup_cached_app_info_by_sender (sender); if (app_info) return g_steal_pointer (&app_info); - app_info = xdp_get_app_info_from_pid (connection, sender, cancellable, error); + if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, error)) + return NULL; + + app_info = parse_app_info_from_flatpak_info (pid, error); + + if (app_info == NULL && *error) + return NULL; + if (app_info == NULL) + app_info = parse_app_info_from_snap (pid, error); + + if (app_info == NULL && *error) return NULL; + if (app_info == NULL) + app_info = xdp_app_info_new_host (pid); + G_LOCK (app_infos); ensure_app_info_by_unique_name (); g_hash_table_insert (app_info_by_unique_name, g_strdup (sender), From ffbccd8aae1ba6f3fae363bd3b1e8e129b7c61f3 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Thu, 8 Feb 2024 21:28:06 +0100 Subject: [PATCH 07/12] utils: Get rid of tri-state app info creation Instead of returning either NULL with error, NULL without error and non-NULL without error, adhere to GLib convention and return either TRUE with an out param set or FALSE with an error set. --- src/xdp-utils.c | 63 +++++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 23d893a30..bd6afaa51 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -599,9 +599,10 @@ ensure_app_info_by_unique_name (void) (GDestroyNotify)xdp_app_info_unref); } -/* Returns NULL with error set on failure, NULL with no error set if not a flatpak, and app-info otherwise */ -static XdpAppInfo * -parse_app_info_from_flatpak_info (int pid, GError **error) +static gboolean +xdp_app_info_from_flatpak_info (int pid, + XdpAppInfo **out_app_info, + GError **error) { g_autofree char *root_path = NULL; int root_fd = -1; @@ -629,7 +630,10 @@ parse_app_info_from_flatpak_info (int pid, GError **error) */ if (statfs (root_path, &buf) == 0 && buf.f_type == 0x65735546) /* FUSE_SUPER_MAGIC */ - return NULL; + { + *out_app_info = NULL; + return TRUE; + } } /* Otherwise, we should be able to open the root dir. Probably the app died and @@ -637,7 +641,7 @@ parse_app_info_from_flatpak_info (int pid, GError **error) of treating this as privileged. */ g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Unable to open %s", root_path); - return NULL; + return FALSE; } metadata = g_key_file_new (); @@ -648,14 +652,15 @@ parse_app_info_from_flatpak_info (int pid, GError **error) { if (errno == ENOENT) { - /* No file => on the host, return NULL with no error */ - return NULL; + /* No file => on the host, return success */ + *out_app_info = NULL; + return TRUE; } /* Some weird error => failure */ g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Unable to open application info file"); - return NULL; + return FALSE; } if (fstat (info_fd, &stat_buf) != 0 || !S_ISREG (stat_buf.st_mode)) @@ -664,7 +669,7 @@ parse_app_info_from_flatpak_info (int pid, GError **error) close (info_fd); g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Unable to open application info file"); - return NULL; + return FALSE; } mapped = g_mapped_file_new_from_fd (info_fd, FALSE, &local_error); @@ -673,7 +678,7 @@ parse_app_info_from_flatpak_info (int pid, GError **error) close (info_fd); g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't map .flatpak-info file: %s", local_error->message); - return NULL; + return FALSE; } if (!g_key_file_load_from_data (metadata, @@ -684,7 +689,7 @@ parse_app_info_from_flatpak_info (int pid, GError **error) close (info_fd); g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't load .flatpak-info file: %s", local_error->message); - return NULL; + return FALSE; } group = "Application"; @@ -695,7 +700,7 @@ parse_app_info_from_flatpak_info (int pid, GError **error) if (id == NULL || !xdp_is_valid_app_id (id)) { close (info_fd); - return NULL; + return FALSE; } close (info_fd); @@ -704,7 +709,8 @@ parse_app_info_from_flatpak_info (int pid, GError **error) app_info->id = g_steal_pointer (&id); app_info->u.flatpak.keyfile = g_steal_pointer (&metadata); - return g_steal_pointer (&app_info); + *out_app_info = g_steal_pointer (&app_info); + return TRUE; } int @@ -791,9 +797,10 @@ pid_is_snap (pid_t pid, GError **error) return is_snap; } -/* Returns NULL with error set on failure, NULL with no error set if not a snap, and app-info otherwise */ -static XdpAppInfo * -parse_app_info_from_snap (pid_t pid, GError **error) +static gboolean +xdp_app_info_from_snap (int pid, + XdpAppInfo **out_app_info, + GError **error) { g_autoptr(GError) local_error = NULL; g_autofree char *pid_str = NULL; @@ -804,13 +811,17 @@ parse_app_info_from_snap (pid_t pid, GError **error) g_autofree char *snap_name = NULL; /* Check the process's cgroup membership to fail quickly for non-snaps */ - if (!pid_is_snap (pid, error)) return NULL; + if (!pid_is_snap (pid, error)) + { + *out_app_info = NULL; + return TRUE; + } pid_str = g_strdup_printf ("%u", (guint) pid); argv[3] = pid_str; if (!xdp_spawnv (NULL, &output, 0, error, argv)) { - return NULL; + return FALSE; } metadata = g_key_file_new (); @@ -818,21 +829,22 @@ parse_app_info_from_snap (pid_t pid, GError **error) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't read snap info for pid %u: %s", pid, local_error->message); - return NULL; + return FALSE; } snap_name = g_key_file_get_string (metadata, SNAP_METADATA_GROUP_INFO, SNAP_METADATA_KEY_INSTANCE_NAME, error); if (snap_name == NULL) { - return NULL; + return FALSE; } app_info = xdp_app_info_new (XDP_APP_INFO_KIND_SNAP); app_info->id = g_strconcat ("snap.", snap_name, NULL); app_info->u.snap.keyfile = g_steal_pointer (&metadata); - return g_steal_pointer (&app_info); + *out_app_info = g_steal_pointer (&app_info); + return TRUE; } static gboolean @@ -907,15 +919,10 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, error)) return NULL; - app_info = parse_app_info_from_flatpak_info (pid, error); - - if (app_info == NULL && *error) + if (!xdp_app_info_from_flatpak_info (pid, &app_info, error)) return NULL; - if (app_info == NULL) - app_info = parse_app_info_from_snap (pid, error); - - if (app_info == NULL && *error) + if (app_info == NULL && !xdp_app_info_from_snap (pid, &app_info, error)) return NULL; if (app_info == NULL) From 8562874f8adcc81ac7159bb96331af1134a95171 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:32:02 +0100 Subject: [PATCH 08/12] utils: Use GetConnectionCredentials to fetch the PID and pidfd The org.fdo.DBus.GetConnectionCredentials method gives us both a PID (ProcessID) and a pidfd (ProcessFD) in one roundtrip. It fails when the PID can't be retrieved but allows the pidfd to be -1. The pidfd will become useful for host and Containers1 clients later. --- src/xdp-utils.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 2 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index bd6afaa51..420090c61 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -42,6 +42,7 @@ #endif #include +#include #include #include @@ -847,6 +848,135 @@ xdp_app_info_from_snap (int pid, return TRUE; } +static gboolean +xdp_connection_get_pid_legacy (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + int *out_pidfd, + uint32_t *out_pid, + GError **error) +{ + g_autoptr(GVariant) reply = NULL; + + reply = g_dbus_connection_call_sync (connection, + DBUS_NAME_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetConnectionUnixProcessID", + g_variant_new ("(s)", sender), + G_VARIANT_TYPE ("(u)"), + G_DBUS_CALL_FLAGS_NONE, + 30000, + cancellable, + error); + if (!reply) + return FALSE; + + *out_pidfd = -1; + g_variant_get (reply, "(u)", out_pid); + return TRUE; +} + +static gboolean +xdp_connection_get_pidfd (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + int *out_pidfd, + uint32_t *out_pid, + GError **error) +{ + g_autoptr(GVariant) reply = NULL; + g_autoptr(GVariant) dict = NULL; + g_autoptr(GError) local_error = NULL; + g_autoptr(GVariant) process_fd = NULL; + g_autoptr(GVariant) process_id = NULL; + uint32_t pid; + int fd_index; + GUnixFDList *fd_list; + int fds_len = 0; + const int *fds; + int pidfd; + + reply = g_dbus_connection_call_with_unix_fd_list_sync (connection, + DBUS_NAME_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetConnectionCredentials", + g_variant_new ("(s)", sender), + G_VARIANT_TYPE ("(a{sv})"), + G_DBUS_CALL_FLAGS_NONE, + 30000, + NULL, + &fd_list, + cancellable, + &local_error); + + if (!reply) + { + if (g_error_matches (local_error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_INTERFACE)) + { + return xdp_connection_get_pid_legacy (connection, + sender, + cancellable, + out_pidfd, + out_pid, + error); + } + + g_propagate_error (error, local_error); + return FALSE; + } + + g_variant_get (reply, "(@a{sv})", &dict); + + process_id = g_variant_lookup_value (dict, "ProcessID", G_VARIANT_TYPE_UINT32); + if (!process_id) + { + return xdp_connection_get_pid_legacy (connection, + sender, + cancellable, + out_pidfd, + out_pid, + error); + } + + pid = g_variant_get_uint32 (process_id); + + process_fd = g_variant_lookup_value (dict, "ProcessFD", G_VARIANT_TYPE_HANDLE); + if (!process_fd) + { + *out_pidfd = -1; + *out_pid = pid; + return TRUE; + } + + fd_index = g_variant_get_handle (process_fd); + + if (fd_list == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer pidfd"); + return FALSE; + } + + fds = g_unix_fd_list_peek_fds (fd_list, &fds_len); + if (fds_len <= fd_index) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer pidfd"); + return FALSE; + } + + pidfd = dup (fds[fd_index]); + if (pidfd < 0) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't dup pidfd"); + return FALSE; + } + + *out_pidfd = pidfd; + *out_pid = pid; + return TRUE; +} + static gboolean xdp_connection_get_pid (GDBusConnection *connection, const char *sender, @@ -910,13 +1040,14 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; - guint32 pid; + int pidfd = -1; + uint32_t pid; app_info = lookup_cached_app_info_by_sender (sender); if (app_info) return g_steal_pointer (&app_info); - if (!xdp_connection_get_pid (connection, sender, cancellable, &pid, error)) + if (!xdp_connection_get_pidfd (connection, sender, cancellable, &pidfd, &pid, error)) return NULL; if (!xdp_app_info_from_flatpak_info (pid, &app_info, error)) From 2705e6d016c4697ae3eeaac1da4c59f76224d697 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:32:22 +0100 Subject: [PATCH 09/12] utils: Remove unused xdp_connection_get_pid --- src/xdp-utils.c | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 420090c61..3674a852b 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -977,45 +977,6 @@ xdp_connection_get_pidfd (GDBusConnection *connection, return TRUE; } -static gboolean -xdp_connection_get_pid (GDBusConnection *connection, - const char *sender, - GCancellable *cancellable, - guint32 *out_pid, - GError **error) -{ - g_autoptr(GDBusMessage) msg = NULL; - g_autoptr(GDBusMessage) reply = NULL; - g_autoptr(XdpAppInfo) app_info = NULL; - GVariant *body; - - msg = g_dbus_message_new_method_call (DBUS_NAME_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "GetConnectionUnixProcessID"); - g_dbus_message_set_body (msg, g_variant_new ("(s)", sender)); - - reply = g_dbus_connection_send_message_with_reply_sync (connection, msg, - G_DBUS_SEND_MESSAGE_FLAGS_NONE, - 30000, - NULL, - cancellable, - error); - if (reply == NULL) - return FALSE; - - if (g_dbus_message_get_message_type (reply) == G_DBUS_MESSAGE_TYPE_ERROR) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer pid"); - return FALSE; - } - - body = g_dbus_message_get_body (reply); - g_variant_get (body, "(u)", out_pid); - - return TRUE; -} - static XdpAppInfo * lookup_cached_app_info_by_sender (const char *sender) { From 8fe5c36f51baec97d08c3dd3123976e3748d8a95 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:34:48 +0100 Subject: [PATCH 10/12] utils: Move AppInfo cache insertion into its own function --- src/xdp-utils.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 3674a852b..7c060df0c 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -978,7 +978,7 @@ xdp_connection_get_pidfd (GDBusConnection *connection, } static XdpAppInfo * -lookup_cached_app_info_by_sender (const char *sender) +cache_lookup_app_info_by_sender (const char *sender) { XdpAppInfo *app_info = NULL; @@ -994,6 +994,16 @@ lookup_cached_app_info_by_sender (const char *sender) return app_info; } +static void +cache_insert_app_info (const char *sender, XdpAppInfo *app_info) +{ + G_LOCK (app_infos); + ensure_app_info_by_unique_name (); + g_hash_table_insert (app_info_by_unique_name, g_strdup (sender), + xdp_app_info_ref (app_info)); + G_UNLOCK (app_infos); +} + static XdpAppInfo * xdp_connection_lookup_app_info_sync (GDBusConnection *connection, const char *sender, @@ -1004,7 +1014,7 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, int pidfd = -1; uint32_t pid; - app_info = lookup_cached_app_info_by_sender (sender); + app_info = cache_lookup_app_info_by_sender (sender); if (app_info) return g_steal_pointer (&app_info); @@ -1020,11 +1030,7 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, if (app_info == NULL) app_info = xdp_app_info_new_host (pid); - G_LOCK (app_infos); - ensure_app_info_by_unique_name (); - g_hash_table_insert (app_info_by_unique_name, g_strdup (sender), - xdp_app_info_ref (app_info)); - G_UNLOCK (app_infos); + cache_insert_app_info (sender, app_info); return g_steal_pointer (&app_info); } From 2d33fafc5e58bd37362f42fc137049fe655efcf0 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:57:42 +0100 Subject: [PATCH 11/12] utils: Make pid mappings via pidns more generic Every app kind that can somehow get to a pidns of the calling process should be able to do the pid mappings. This will be used in the next commit. --- src/xdp-utils.c | 51 ++++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 7c060df0c..230f18597 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -125,14 +125,15 @@ struct _XdpAppInfo { char *id; XdpAppInfoKind kind; + /* pid namespace mapping */ + GMutex pidns_lock; + ino_t pidns_id; + union { struct { GKeyFile *keyfile; - /* pid namespace mapping */ - GMutex pidns_lock; - ino_t pidns_id; } flatpak; struct { @@ -2237,24 +2238,16 @@ xdp_app_info_get_child_pid (JsonNode *root, } static gboolean -xdp_app_info_ensure_pidns (XdpAppInfo *app_info, - DIR *proc, - GError **error) +xdp_app_info_ensure_pidns_flatpak (XdpAppInfo *app_info, + DIR *proc, + GError **error) { g_autoptr(JsonNode) root = NULL; - g_autoptr(GMutexLocker) guard = NULL; xdp_autofd int fd = -1; pid_t pid; ino_t ns; int r; - g_assert (app_info->kind == XDP_APP_INFO_KIND_FLATPAK); - - guard = g_mutex_locker_new (&(app_info->u.flatpak.pidns_lock)); - - if (app_info->u.flatpak.pidns_id != 0) - return TRUE; - root = xdp_app_info_load_bwrap_info (app_info, error); if (root == NULL) return FALSE; @@ -2266,7 +2259,7 @@ xdp_app_info_ensure_pidns (XdpAppInfo *app_info, if (ns != 0) { g_debug ("Using pid namespace info from bwrap info"); - app_info->u.flatpak.pidns_id = ns; + app_info->pidns_id = ns; return TRUE; } @@ -2288,11 +2281,28 @@ xdp_app_info_ensure_pidns (XdpAppInfo *app_info, return FALSE; } - app_info->u.flatpak.pidns_id = ns; + app_info->pidns_id = ns; return TRUE; } +static gboolean +xdp_app_info_ensure_pidns (XdpAppInfo *app_info, + DIR *proc, + GError **error) +{ + g_autoptr(GMutexLocker) guard = g_mutex_locker_new (&(app_info->pidns_lock)); + + if (app_info->pidns_id != 0) + return TRUE; + + if (app_info->kind == XDP_APP_INFO_KIND_FLATPAK) + return xdp_app_info_ensure_pidns_flatpak (app_info, proc, error); + + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Could not get a pidns"); + return FALSE; +} + /* This is the trunk for xdp_app_info_map_pids()/xdp_app_info_map_tids() */ static gboolean app_info_map_pids (XdpAppInfo *app_info, @@ -2312,13 +2322,6 @@ app_info_map_pids (XdpAppInfo *app_info, if (app_info->kind == XDP_APP_INFO_KIND_HOST) return TRUE; - if (app_info->kind != XDP_APP_INFO_KIND_FLATPAK) - { - g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED, - "Mapping pids is not supported."); - return FALSE; - } - proc = opendir (proc_dir); if (proc == NULL) { @@ -2340,7 +2343,7 @@ app_info_map_pids (XdpAppInfo *app_info, */ uid = getuid (); - ns = app_info->u.flatpak.pidns_id; + ns = app_info->pidns_id; ok = map_pids (proc, ns, pids, n_pids, uid, error); out: From 9beb5f565318d6f2536714c80a9aea5822581550 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 16:09:51 +0100 Subject: [PATCH 12/12] utils: Support pid/tid mapping for host apps Both host apps and snaps connect directly to dbus and thus have a pidfd that points to the calling process. This allows us to do pid mappings independent of the app kind. This will only work when the D-Bus broker supports pidfd (ProcessFD). --- src/xdp-utils.c | 57 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 230f18597..33f35bb8f 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -125,6 +125,9 @@ struct _XdpAppInfo { char *id; XdpAppInfoKind kind; + /* pidfd of the calling process */ + int pidfd; + /* pid namespace mapping */ GMutex pidns_lock; ino_t pidns_id; @@ -148,6 +151,7 @@ xdp_app_info_new (XdpAppInfoKind kind) XdpAppInfo *app_info = g_new0 (XdpAppInfo, 1); app_info->ref_count = 1; app_info->kind = kind; + app_info->pidfd = -1; return app_info; } @@ -233,10 +237,12 @@ set_appid_from_pid (XdpAppInfo *app_info, pid_t pid) } static XdpAppInfo * -xdp_app_info_new_host (pid_t pid) +xdp_app_info_new_host (pid_t pid, + int pidfd) { XdpAppInfo *app_info = xdp_app_info_new (XDP_APP_INFO_KIND_HOST); set_appid_from_pid (app_info, pid); + app_info->pidfd = pidfd; return app_info; } @@ -252,6 +258,7 @@ static void xdp_app_info_free (XdpAppInfo *app_info) { g_free (app_info->id); + xdp_close_fd (&app_info->pidfd); switch (app_info->kind) { @@ -801,6 +808,7 @@ pid_is_snap (pid_t pid, GError **error) static gboolean xdp_app_info_from_snap (int pid, + int pidfd, XdpAppInfo **out_app_info, GError **error) { @@ -843,6 +851,7 @@ xdp_app_info_from_snap (int pid, app_info = xdp_app_info_new (XDP_APP_INFO_KIND_SNAP); app_info->id = g_strconcat ("snap.", snap_name, NULL); + app_info->pidfd = pidfd; app_info->u.snap.keyfile = g_steal_pointer (&metadata); *out_app_info = g_steal_pointer (&app_info); @@ -1025,11 +1034,11 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, if (!xdp_app_info_from_flatpak_info (pid, &app_info, error)) return NULL; - if (app_info == NULL && !xdp_app_info_from_snap (pid, &app_info, error)) + if (app_info == NULL && !xdp_app_info_from_snap (pid, pidfd, &app_info, error)) return NULL; if (app_info == NULL) - app_info = xdp_app_info_new_host (pid); + app_info = xdp_app_info_new_host (pid, pidfd); cache_insert_app_info (sender, app_info); @@ -2276,8 +2285,8 @@ xdp_app_info_ensure_pidns_flatpak (XdpAppInfo *app_info, { int code = g_io_error_from_errno (-r); g_set_error (error, G_IO_ERROR, code, - "Could not query /proc/%u/ns/pid: %s", - (guint) pid, g_strerror (-r)); + "Could not query pidfd for pidns: %s", + g_strerror (-r)); return FALSE; } @@ -2286,6 +2295,28 @@ xdp_app_info_ensure_pidns_flatpak (XdpAppInfo *app_info, return TRUE; } +static gboolean +xdp_app_info_ensure_pidns_pidfd (XdpAppInfo *app_info, + DIR *proc, + GError **error) +{ + ino_t ns; + int r; + + r = lookup_ns_from_pid_fd (app_info->pidfd, &ns); + if (r < 0) + { + int code = g_io_error_from_errno (-r); + g_set_error (error, G_IO_ERROR, code, + "Could not query pidfd for pidns: %s", + g_strerror (-r)); + return FALSE; + } + + app_info->pidns_id = ns; + return TRUE; +} + static gboolean xdp_app_info_ensure_pidns (XdpAppInfo *app_info, DIR *proc, @@ -2296,6 +2327,9 @@ xdp_app_info_ensure_pidns (XdpAppInfo *app_info, if (app_info->pidns_id != 0) return TRUE; + if (app_info->pidfd >= 0) + return xdp_app_info_ensure_pidns_pidfd (app_info, proc, error); + if (app_info->kind == XDP_APP_INFO_KIND_FLATPAK) return xdp_app_info_ensure_pidns_flatpak (app_info, proc, error); @@ -2311,6 +2345,7 @@ app_info_map_pids (XdpAppInfo *app_info, guint n_pids, GError **error) { + g_autoptr(GError) local_error = NULL; gboolean ok; DIR *proc; uid_t uid; @@ -2319,9 +2354,6 @@ app_info_map_pids (XdpAppInfo *app_info, g_return_val_if_fail (app_info != NULL, FALSE); g_return_val_if_fail (pids != NULL, FALSE); - if (app_info->kind == XDP_APP_INFO_KIND_HOST) - return TRUE; - proc = opendir (proc_dir); if (proc == NULL) { @@ -2331,10 +2363,15 @@ app_info_map_pids (XdpAppInfo *app_info, } /* Make sure we know the pid namespace the app is running in */ - ok = xdp_app_info_ensure_pidns (app_info, proc, error); + ok = xdp_app_info_ensure_pidns (app_info, proc, &local_error); if (!ok) { - g_prefix_error (error, "Could not determine pid namespace: "); + /* fallback to not mapping pids if the app is on the host */ + if (app_info->kind == XDP_APP_INFO_KIND_HOST) + return TRUE; + + g_propagate_prefixed_error (error, local_error, + "Could not determine pid namespace: "); goto out; }