From 8549bc8a5ddf1f4b8752501d38400ead0e394d62 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 13:30:08 +0100 Subject: [PATCH 01/15] device: Removes the Device portal A shell of its former self remains which informs any potential user about the removal. 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. --- data/org.freedesktop.portal.Device.xml | 4 + src/camera.c | 115 +++++++++- src/camera.h | 1 + src/device.c | 280 +------------------------ src/device.h | 12 +- src/xdg-desktop-portal.c | 8 +- 6 files changed, 124 insertions(+), 296 deletions(-) diff --git a/data/org.freedesktop.portal.Device.xml b/data/org.freedesktop.portal.Device.xml index 2f981cbb1..43630309c 100644 --- a/data/org.freedesktop.portal.Device.xml +++ b/data/org.freedesktop.portal.Device.xml @@ -23,6 +23,10 @@ org.freedesktop.portal.Device: @short_description: Portal for device access + WARNING: This interface is deprecated and will be removed in a + future version. Please contact upstream, if you are using this + interface. + This interface lets services ask if an application should get access to devices such as microphones, speakers or cameras. Not a portal in the strict sense, since the API is not directly 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 index 87a954897..aac046198 100644 --- a/src/device.c +++ b/src/device.c @@ -39,8 +39,6 @@ #include "xdp-impl-dbus.h" #include "xdp-utils.h" -#define PERMISSION_TABLE "devices" - typedef struct _Device Device; typedef struct _DeviceClass DeviceClass; @@ -54,9 +52,7 @@ 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); @@ -65,165 +61,6 @@ 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, @@ -231,96 +68,11 @@ handle_access_device (XdpDbusDevice *object, 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); - + g_dbus_method_invocation_return_error (invocation, + XDG_DESKTOP_PORTAL_ERROR, + XDG_DESKTOP_PORTAL_ERROR_FAILED, + "This interface is deprecated and will be removed in a future version. " + "Please contact upstream, if you are using this interface."); return G_DBUS_METHOD_INVOCATION_HANDLED; } @@ -342,28 +94,8 @@ device_class_init (DeviceClass *klass) } GDBusInterfaceSkeleton * -device_create (GDBusConnection *connection, - const char *dbus_name, - gpointer lockdown_proxy) +device_create (GDBusConnection *connection) { - 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 index 74d1efb8b..6d3594f6e 100644 --- a/src/device.h +++ b/src/device.h @@ -23,15 +23,5 @@ #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); +GDBusInterfaceSkeleton * device_create (GDBusConnection *connection); diff --git a/src/xdg-desktop-portal.c b/src/xdg-desktop-portal.c index fdac56ef8..e4c1d33bf 100644 --- a/src/xdg-desktop-portal.c +++ b/src/xdg-desktop-portal.c @@ -263,9 +263,7 @@ on_bus_acquired (GDBusConnection *connection, PortalImplementation *tmp; export_portal_implementation (connection, - device_create (connection, - access_impl->dbus_name, - lockdown)); + device_create (connection)); #ifdef HAVE_GEOCLUE export_portal_implementation (connection, location_create (connection, @@ -274,7 +272,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 b18b893a4f1dc0f45635bf79b71ec6b1df2fc7d5 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:03:00 +0100 Subject: [PATCH 02/15] 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 eb59444a4..726649722 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -481,7 +481,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 @@ -819,8 +819,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 bd3310992481d2bc8c97a6402bd8bbd9308479b3 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:04:44 +0100 Subject: [PATCH 03/15] 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 726649722..17b0ea650 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -481,6 +481,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 b9ad7558504f5a59ee74919af8510ccb89ea27a4 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:22:45 +0100 Subject: [PATCH 04/15] 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 17b0ea650..f649bb22d 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -827,12 +827,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) @@ -880,41 +928,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 fa954ce17846c7adc42449279b886cb8390a6323 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:28:26 +0100 Subject: [PATCH 05/15] 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 f649bb22d..5d9b1c71d 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -873,31 +873,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 0793641de67b51d46e6904ac97dadfe96a853ef5 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 14:33:49 +0100 Subject: [PATCH 06/15] 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 5d9b1c71d..99961604c 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -866,35 +866,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) { @@ -919,15 +890,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 6bb06d78515c30d340276699d17c51b33256dba6 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Thu, 8 Feb 2024 21:28:06 +0100 Subject: [PATCH 07/15] 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 99961604c..beaef74b4 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -591,9 +591,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; @@ -621,7 +622,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 @@ -629,7 +633,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 (); @@ -640,14 +644,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)) @@ -656,7 +661,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); @@ -665,7 +670,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, @@ -676,7 +681,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"; @@ -687,7 +692,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); @@ -696,7 +701,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 @@ -783,9 +789,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; @@ -796,13 +803,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 (); @@ -810,21 +821,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 @@ -899,15 +911,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 4e5eb41833b6fec506e83054d3517aa31dcf2c3b Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:32:02 +0100 Subject: [PATCH 08/15] 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 | 86 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index beaef74b4..e347ddee8 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -42,6 +42,7 @@ #endif #include +#include #include #include @@ -839,6 +840,88 @@ xdp_app_info_from_snap (int pid, return TRUE; } +static gboolean +xdp_connection_get_pidfd (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + int *out_pidfd, + guint32 *out_pid, + GError **error) +{ + g_autoptr(GVariant) parameters = NULL; + g_autoptr(GVariant) reply = NULL; + g_autoptr(GVariant) process_fd = NULL; + g_autoptr(GVariant) process_id = NULL; + guint32 pid; + int fd_index; + GUnixFDList *fd_list; + gint fds_len = 0; + const gint *fds; + gint pidfd; + + parameters = g_variant_new ("(s)", sender); + + reply = g_dbus_connection_call_with_unix_fd_list_sync (connection, + DBUS_NAME_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetConnectionCredentials", + parameters, + G_VARIANT_TYPE ("(a{sv})"), + G_DBUS_CALL_FLAGS_NONE, + 30000, + NULL, + &fd_list, + cancellable, + error); + + if (!reply) + return FALSE; + + process_id = g_variant_lookup_value (reply, "ProcessID", G_VARIANT_TYPE_UINT32); + if (!process_id) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Can't find peer pid"); + return FALSE; + } + + pid = g_variant_get_uint32 (process_id); + + process_fd = g_variant_lookup_value (reply, "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, @@ -902,13 +985,14 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; + int pidfd = -1; guint32 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 df88e798d41b183ae3cda8c9efdee30cd732a4c7 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:32:22 +0100 Subject: [PATCH 09/15] 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 e347ddee8..fe818ba8d 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -922,45 +922,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 1537fd57f3eb9164effb793d8e17967755c6f080 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:34:48 +0100 Subject: [PATCH 10/15] 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 fe818ba8d..12d5bc642 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -923,7 +923,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; @@ -939,6 +939,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, @@ -949,7 +959,7 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, int pidfd = -1; guint32 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); @@ -965,11 +975,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 4d1243eebd27c2a4c4b99be214f7cbf0654c105c Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:19:31 +0100 Subject: [PATCH 11/15] utils: Fetch Containers1 metadata Returns a GVariant containign all the metadata that a container engine like flatpak has set on the socket for this app instance. This contains the container type, app id, instance id and additional arbitrary metadata such as the desktop file name. If the D-Bus broker doesn't support the Containers1 interface or the app using the portal is not running in a container, we fall back to the other ways of identifying apps. This specifically means it's safe to run a D-Bus broker which supports Containers1 while flatpak doesn't. Eventually we want to remove the flatpak and snap specific paths at which point it's a requirement to have a Containers1 capable flatpak. --- src/xdp-utils.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 12d5bc642..f000ae225 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -50,6 +50,7 @@ #define DBUS_NAME_DBUS "org.freedesktop.DBus" #define DBUS_INTERFACE_DBUS DBUS_NAME_DBUS +#define DBUS_INTERFACE_DBUS_CONTAINERS1 DBUS_NAME_DBUS ".Containers1" #define DBUS_PATH_DBUS "/org/freedesktop/DBus" G_LOCK_DEFINE (app_infos); @@ -840,6 +841,59 @@ xdp_app_info_from_snap (int pid, return TRUE; } +static gboolean +xdp_app_info_from_containers1 (GVariant *containers1_data, + int pidfd, + XdpAppInfo **out_app_info, + GError **error) +{ + *out_app_info = NULL; + return TRUE; +} + +static gboolean +xdp_connection_get_containers1 (GDBusConnection *connection, + const char *sender, + GCancellable *cancellable, + GVariant **out_variant, + GError **error) +{ + g_autoptr(GVariant) parameters = NULL; + g_autoptr(GVariant) reply = NULL; + g_autoptr(GError) local_error = NULL; + + parameters = g_variant_new ("(s)", sender); + + reply = g_dbus_connection_call_sync (connection, + DBUS_NAME_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS_CONTAINERS1, + "GetConnectionInfo", + parameters, + G_VARIANT_TYPE ("(oa{sv}sssa{sv})"), + G_DBUS_CALL_FLAGS_NONE, + 30000, + cancellable, + &local_error); + + if (reply == NULL) + { + if (g_error_matches (local_error, G_DBUS_ERROR, G_DBUS_ERROR_UNKNOWN_INTERFACE) || + g_strcmp0 (g_dbus_error_get_remote_error (local_error), + "org.freedesktop.DBus.Error.NotContainer") == 0) + { + *out_variant = NULL; + return TRUE; + } + + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + + *out_variant = g_steal_pointer (&reply); + return TRUE; +} + static gboolean xdp_connection_get_pidfd (GDBusConnection *connection, const char *sender, @@ -956,6 +1010,7 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, GError **error) { g_autoptr(XdpAppInfo) app_info = NULL; + g_autoptr(GVariant) containers1_data = NULL; int pidfd = -1; guint32 pid; @@ -963,10 +1018,17 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, if (app_info) return g_steal_pointer (&app_info); + if (!xdp_connection_get_containers1 (connection, sender, cancellable, + &containers1_data, error)) + return NULL; + if (!xdp_connection_get_pidfd (connection, sender, cancellable, &pidfd, &pid, error)) return NULL; - if (!xdp_app_info_from_flatpak_info (pid, &app_info, error)) + if (!xdp_app_info_from_containers1 (containers1_data, pidfd, &app_info, error)) + return NULL; + + if (app_info == NULL && !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)) From 7315913de7aa742ad9f466d003ecee0a5aece584 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 15:57:42 +0100 Subject: [PATCH 12/15] utils: Make pid mappings via pidns more generic Both host apps and Containers1 apps can do pid mappings via pidfd. This is some prep work for that. --- src/xdp-utils.c | 50 +++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index f000ae225..78dd78cf6 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -126,14 +126,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 { @@ -2239,24 +2240,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; @@ -2268,7 +2261,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; } @@ -2290,11 +2283,27 @@ 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); + + 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, @@ -2314,13 +2323,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) { @@ -2342,7 +2344,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 ca85d7e75b736f6cec1f974f50fd5475ab4212a7 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 16:09:51 +0100 Subject: [PATCH 13/15] utils: Support pid/tid mapping for host apps Do some best effort pid remapping for host apps. This will only work when the D-Bus broker supports pidfd (ProcessFD). --- src/xdp-utils.c | 49 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 6 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 78dd78cf6..28912c388 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -132,6 +132,10 @@ struct _XdpAppInfo { union { + struct + { + int pidfd; + } host; struct { GKeyFile *keyfile; @@ -234,10 +238,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->u.host.pidfd = pidfd; return app_info; } @@ -248,6 +254,10 @@ xdp_app_info_free (XdpAppInfo *app_info) switch (app_info->kind) { + case XDP_APP_INFO_KIND_HOST: + xdp_close_fd (&app_info->u.host.pidfd); + break; + case XDP_APP_INFO_KIND_FLATPAK: g_clear_pointer (&app_info->u.flatpak.keyfile, g_key_file_free); break; @@ -256,7 +266,6 @@ xdp_app_info_free (XdpAppInfo *app_info) g_clear_pointer (&app_info->u.snap.keyfile, g_key_file_free); break; - case XDP_APP_INFO_KIND_HOST: default: break; } @@ -1036,7 +1045,7 @@ xdp_connection_lookup_app_info_sync (GDBusConnection *connection, 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); @@ -2278,8 +2287,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; } @@ -2288,6 +2297,31 @@ xdp_app_info_ensure_pidns_flatpak (XdpAppInfo *app_info, return TRUE; } +static gboolean +xdp_app_info_ensure_pidns_host (XdpAppInfo *app_info, + DIR *proc, + GError **error) +{ + ino_t ns; + int r; + + if (app_info->u.host.pidfd < 0) + return FALSE; + + r = lookup_ns_from_pid_fd (app_info->u.host.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, @@ -2301,6 +2335,9 @@ xdp_app_info_ensure_pidns (XdpAppInfo *app_info, if (app_info->kind == XDP_APP_INFO_KIND_FLATPAK) return xdp_app_info_ensure_pidns_flatpak (app_info, proc, error); + if (app_info->kind == XDP_APP_INFO_KIND_HOST) + return xdp_app_info_ensure_pidns_host (app_info, proc, error); + return FALSE; } @@ -2320,7 +2357,7 @@ 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) + if (app_info->kind == XDP_APP_INFO_KIND_HOST && app_info->u.host.pidfd < 0) return TRUE; proc = opendir (proc_dir); From ba4ab57c0bbdd450b19dd34bc8b1f4656c9d8260 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 16:25:50 +0100 Subject: [PATCH 14/15] utils: Introduce XDP_APP_INFO_KIND_CONTAINERS1 This new app kind is authenticated by D-Bus and x-d-p gets all the information about the app from D-Bus which itself get the information from the sandbox engine which set up the sandbox of the app. Currently only flatpak and dbus (dbus-daemon) supports this mechanism but it can be supported by any other sandbox engine, such as snap and firejail. This also means that an app with XDP_APP_INFO_KIND_CONTAINERS1 can still be a flatpak or a snap app. The goal is to make as many paths in x-d-p agnostic to the actual sandbox engine and handle all XDP_APP_INFO_KIND_CONTAINERS1 apps the same. Eventually we can then remove any other XDP_APP_INFO_KIND variants. This commit sets up a XdpAppInfo object with all the metadata x-d-p will need. The follow up commit implements various XdpAppInfo functionalities. --- src/xdp-utils.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++- src/xdp-utils.h | 7 +++--- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 28912c388..84e16cab7 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -144,6 +144,14 @@ struct _XdpAppInfo { { GKeyFile *keyfile; } snap; + struct + { + char *container_type; + char *instance_id; + int pidfd; + char *desktop_file; + gboolean has_network; + } containers1; } u; }; @@ -266,6 +274,13 @@ xdp_app_info_free (XdpAppInfo *app_info) g_clear_pointer (&app_info->u.snap.keyfile, g_key_file_free); break; + case XDP_APP_INFO_KIND_CONTAINERS1: + g_clear_pointer (&app_info->u.containers1.container_type, g_free); + g_clear_pointer (&app_info->u.containers1.instance_id, g_free); + g_clear_pointer (&app_info->u.containers1.desktop_file, g_free); + xdp_close_fd (&app_info->u.containers1.pidfd); + break; + default: break; } @@ -857,7 +872,48 @@ xdp_app_info_from_containers1 (GVariant *containers1_data, XdpAppInfo **out_app_info, GError **error) { - *out_app_info = NULL; + XdpAppInfo *app_info = NULL; + const char *container_type; + const char *app_id; + const char *instance_id; + GVariant *metadata; + const char *desktop_file; + gboolean network_access; + + if (!containers1_data || pidfd < 0) + { + *out_app_info = NULL; + return TRUE; + } + + g_variant_get (containers1_data, "(o@a{sv}sss@a{sv})", + NULL, + NULL, + &container_type, + &app_id, + &instance_id, + &metadata); + + if (!app_id || !instance_id || !container_type) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Bad Containers1 metadata"); + return FALSE; + } + + app_info = xdp_app_info_new (XDP_APP_INFO_KIND_CONTAINERS1); + app_info->id = g_strdup (app_id); + app_info->u.containers1.container_type = g_strdup (container_type); + app_info->u.containers1.instance_id = g_strdup (instance_id); + app_info->u.containers1.pidfd = pidfd; + + if (g_variant_lookup (metadata, "DesktopFile", "&s", &desktop_file)) + app_info->u.containers1.desktop_file = g_strdup (desktop_file); + + app_info->u.containers1.has_network = TRUE; + if (g_variant_lookup (metadata, "NetworkAccess", "b", &network_access)) + app_info->u.containers1.has_network = network_access; + + *out_app_info = app_info; return TRUE; } diff --git a/src/xdp-utils.h b/src/xdp-utils.h index 2f4f2c8ab..8bfb77ce8 100644 --- a/src/xdp-utils.h +++ b/src/xdp-utils.h @@ -48,9 +48,10 @@ typedef enum { - XDP_APP_INFO_KIND_HOST = 0, - XDP_APP_INFO_KIND_FLATPAK = 1, - XDP_APP_INFO_KIND_SNAP = 2, + XDP_APP_INFO_KIND_HOST = 0, + XDP_APP_INFO_KIND_FLATPAK = 1, + XDP_APP_INFO_KIND_SNAP = 2, + XDP_APP_INFO_KIND_CONTAINERS1 = 3, } XdpAppInfoKind; gint xdp_mkstempat (int dir_fd, From b342409d63c6412b0582433e6811e1ee6a800f3b Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Tue, 23 Jan 2024 16:31:43 +0100 Subject: [PATCH 15/15] utils: Implement XdpAppInfo utils for KIND_CONTAINERS1 Uses some of the metadata from Containers1 directly and falls back to the flatpak path in some cases. Some of the fallbacks can be removed once the D-Bus broker supports ACL and we can get rid of xdg-dbus-proxy. The path remapping and tryexec functionalities are not implemented right now. --- src/xdp-utils.c | 78 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 84e16cab7..02b267e9a 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -342,6 +342,10 @@ xdp_app_info_load_app_info (XdpAppInfo *app_info) NULL); break; + case XDP_APP_INFO_KIND_CONTAINERS1: + desktop_id = g_strdup (app_info->u.containers1.desktop_file); + break; + case XDP_APP_INFO_KIND_HOST: default: desktop_id = NULL; @@ -398,7 +402,7 @@ xdp_app_info_rewrite_commandline (XdpAppInfo *app_info, g_ptr_array_add (args, NULL); return (char **)g_ptr_array_free (g_steal_pointer (&args), FALSE); } - else if (app_info->kind == XDP_APP_INFO_KIND_FLATPAK) + else if (xdp_app_info_is_flatpak (app_info)) { args = g_ptr_array_new_with_free_func (g_free); @@ -489,15 +493,28 @@ xdp_app_info_get_tryexec_path (XdpAppInfo *app_info) char * xdp_app_info_get_instance (XdpAppInfo *app_info) { + char *instance = NULL; + g_return_val_if_fail (app_info != NULL, NULL); - if (app_info->kind != XDP_APP_INFO_KIND_FLATPAK) - return NULL; + switch (app_info->kind) + { + case XDP_APP_INFO_KIND_FLATPAK: + instance = g_key_file_get_string (app_info->u.flatpak.keyfile, + FLATPAK_METADATA_GROUP_INSTANCE, + FLATPAK_METADATA_KEY_INSTANCE_ID, + NULL); + break; + + case XDP_APP_INFO_KIND_CONTAINERS1: + instance = g_strdup (app_info->u.containers1.instance_id); + break; + + default: + break; + } - return g_key_file_get_string (app_info->u.flatpak.keyfile, - FLATPAK_METADATA_GROUP_INSTANCE, - FLATPAK_METADATA_KEY_INSTANCE_ID, - NULL); + return instance; } gboolean @@ -513,7 +530,9 @@ 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; + return app_info->kind == XDP_APP_INFO_KIND_FLATPAK || + (app_info->kind == XDP_APP_INFO_KIND_CONTAINERS1 && + g_strcmp0 (app_info->u.containers1.container_type, "org.flatpak") == 0); } static gboolean @@ -600,6 +619,10 @@ xdp_app_info_has_network (XdpAppInfo *app_info) SNAP_METADATA_KEY_NETWORK, NULL); break; + case XDP_APP_INFO_KIND_CONTAINERS1: + has_network = app_info->u.containers1.has_network; + break; + case XDP_APP_INFO_KIND_HOST: default: has_network = TRUE; @@ -2378,6 +2401,42 @@ xdp_app_info_ensure_pidns_host (XdpAppInfo *app_info, return TRUE; } +static gboolean +xdp_app_info_ensure_pidns_containers1 (XdpAppInfo *app_info, + DIR *proc, + GError **error) +{ + ino_t ns; + int r; + + if (xdp_app_info_is_flatpak (app_info)) + { + /* Containers1 is supposed to be generic but currently flatpak still + * sets up the xdg-dbus-proxy which the pidfd is pointing at. When dbus + * learns about ACL, it can replace the proxy and the pidfd starts + * pointing to the right process. + * Until that happens, we can safely fall back to the flatpak-specific + * path. It uses the flatpak instance id to look up the PID and + * Containers1 knows the instance id. + */ + return xdp_app_info_ensure_pidns_flatpak (app_info, proc, error); + } + + r = lookup_ns_from_pid_fd (app_info->u.containers1.pidfd, &ns); + if (r < 0) + { + int code = g_io_error_from_errno (-r); + g_set_error (error, G_IO_ERROR, code, + "Could not lookup PID namespace from pidfd: %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, @@ -2394,6 +2453,9 @@ xdp_app_info_ensure_pidns (XdpAppInfo *app_info, if (app_info->kind == XDP_APP_INFO_KIND_HOST) return xdp_app_info_ensure_pidns_host (app_info, proc, error); + if (app_info->kind == XDP_APP_INFO_KIND_CONTAINERS1) + return xdp_app_info_ensure_pidns_containers1 (app_info, proc, error); + return FALSE; }