From 18192c485b11ee4864bf9b413ced19268f6e7801 Mon Sep 17 00:00:00 2001 From: Julian Sparber Date: Tue, 2 Jul 2024 11:45:32 +0200 Subject: [PATCH] xdp-utils: Use XdpSealedFd instead of temporary file for icon validation This will allow us in future to write GBytesIcons once to a shareable read-only fd. So that we don't need to load the same date in multiple places. Also in a future commit the notification portal will start to accept sealable memfds (stored in XdpSealedFd) that will be passed onto the backend. In general, there are multiple places where we can consume an fd instead of using serialized data (including sending over DBus). --- src/dynamic-launcher.c | 35 ++++++++++++++++---- src/notification.c | 35 ++++++++++++-------- src/xdp-utils.c | 72 +++--------------------------------------- src/xdp-utils.h | 8 ++--- 4 files changed, 58 insertions(+), 92 deletions(-) diff --git a/src/dynamic-launcher.c b/src/dynamic-launcher.c index 4371be40a..cb127637e 100644 --- a/src/dynamic-launcher.c +++ b/src/dynamic-launcher.c @@ -137,6 +137,33 @@ validate_desktop_file_id (const char *app_id, return TRUE; } +static gboolean +validate_serialized_icon (GVariant *arg_icon_v, + char **icon_format, + char **icon_size) +{ + GBytes *bytes; + g_autoptr(GIcon) icon = NULL; + g_autoptr(GVariant) icon_v = NULL; + g_autoptr(XdpSealedFd) sealed_icon = NULL; + + if (!arg_icon_v) + return FALSE; + + if (!(icon_v = g_variant_get_variant (arg_icon_v))) + return FALSE; + + icon = g_icon_deserialize (icon_v); + + if (!G_IS_BYTES_ICON (icon)) + return FALSE; + + bytes = g_bytes_icon_get_bytes (G_BYTES_ICON (icon)); + sealed_icon = xdp_sealed_fd_new_from_bytes (bytes, NULL); + + return sealed_icon && xdp_validate_icon (sealed_icon, icon_format, icon_size); +} + static gboolean write_icon_to_disk (GVariant *icon_v, const char *icon_subdir, @@ -617,9 +644,7 @@ handle_prepare_install (XdpDbusDynamicLauncher *object, } /* Do some validation on the icon before passing it along */ - icon_v = g_variant_get_variant (arg_icon_v); - if (!icon_v || !xdp_validate_serialized_icon (icon_v, TRUE /* bytes_only */, - &icon_format, &icon_size)) + if (!validate_serialized_icon (arg_icon_v, &icon_format, &icon_size)) { g_dbus_method_invocation_return_error (invocation, XDG_DESKTOP_PORTAL_ERROR, @@ -688,9 +713,7 @@ handle_request_install_token (XdpDbusDynamicLauncher *object, if (response == 0) { /* Do some validation on the icon before saving it */ - icon_v = g_variant_get_variant (arg_icon_v); - if (!icon_v || !xdp_validate_serialized_icon (icon_v, TRUE /* bytes_only */, - &icon_format, &icon_size)) + if (!validate_serialized_icon (arg_icon_v, &icon_format, &icon_size)) { g_dbus_method_invocation_return_error (invocation, XDG_DESKTOP_PORTAL_ERROR, diff --git a/src/notification.c b/src/notification.c index 871806fde..5eb7ffaf1 100644 --- a/src/notification.c +++ b/src/notification.c @@ -398,7 +398,7 @@ parse_serialized_icon (GVariantBuilder *builder, g_set_error_literal (error, XDG_DESKTOP_PORTAL_ERROR, XDG_DESKTOP_PORTAL_ERROR_NOT_ALLOWED, - "invalid icon: only themed icons can be a string"); + "only themed icons can be a string"); return FALSE; } } @@ -412,28 +412,32 @@ parse_serialized_icon (GVariantBuilder *builder, if (strcmp (key, "themed") == 0) { if (!check_value_type (key, value, G_VARIANT_TYPE_STRING_ARRAY, error)) - { - g_prefix_error (error, "invalid icon: "); - return FALSE; - } + return FALSE; + + g_variant_builder_add (builder, "{sv}", "icon", icon); } else if (strcmp (key, "bytes") == 0) { + g_autoptr(GBytes) icon_bytes = NULL; + g_autoptr(GError) local_error = NULL; + g_autoptr(XdpSealedFd) sealed_icon = NULL; + if (!check_value_type (key, value, G_VARIANT_TYPE_BYTESTRING, error)) - { - g_prefix_error (error, "invalid icon: "); - return FALSE; - } + return FALSE; + + icon_bytes = g_variant_get_data_as_bytes (value); + sealed_icon = xdp_sealed_fd_new_from_bytes (icon_bytes, &local_error); + + if (!sealed_icon) + g_warning ("Failed to read icon: %s", local_error->message); + else if (xdp_validate_icon (sealed_icon, NULL, NULL)) + g_variant_builder_add (builder, "{sv}", "icon", icon); } else { g_debug ("Unsupported icon %s filtered from notification", key); - return TRUE; } - if (xdp_validate_serialized_icon (icon, FALSE, NULL, NULL)) - g_variant_builder_add (builder, "{sv}", "icon", icon); - return TRUE; } @@ -464,7 +468,10 @@ parse_notification (GVariantBuilder *builder, else if (strcmp (key, "icon") == 0) { if (!parse_serialized_icon (builder, value, error)) - return FALSE; + { + g_prefix_error (error, "invalid icon: "); + return FALSE; + } } else if (strcmp (key, "priority") == 0) { diff --git a/src/xdp-utils.c b/src/xdp-utils.c index 7148118d7..6c5744b5c 100644 --- a/src/xdp-utils.c +++ b/src/xdp-utils.c @@ -549,60 +549,22 @@ xdp_has_path_prefix (const char *str, } } -void -cleanup_temp_file (void *p) -{ - void **pp = (void **)p; - - if (*pp) - remove (*pp); - g_free (*pp); -} - #define VALIDATOR_INPUT_FD 3 #define ICON_VALIDATOR_GROUP "Icon Validator" gboolean -xdp_validate_serialized_icon (GVariant *v, - gboolean bytes_only, - char **out_format, - char **out_size) +xdp_validate_icon (XdpSealedFd *icon, + char **out_format, + char **out_size) { - g_autoptr(GIcon) icon = NULL; - GBytes *bytes; - __attribute__((cleanup(cleanup_temp_file))) char *name = NULL; - xdp_autofd int fd = -1; - g_autoptr(GOutputStream) stream = NULL; g_autofree char *format = NULL; g_autoptr(GError) error = NULL; const char *icon_validator = LIBEXECDIR "/xdg-desktop-portal-validate-icon"; const char *args[5]; int size; - gconstpointer bytes_data; - gsize bytes_len; g_autofree char *output = NULL; g_autoptr(GKeyFile) key_file = NULL; - icon = g_icon_deserialize (v); - if (!icon) - { - g_warning ("Icon deserialization failed"); - return FALSE; - } - - if (!bytes_only && G_IS_THEMED_ICON (icon)) - { - g_autofree char *a = g_strjoinv (" ", (char **)g_themed_icon_get_names (G_THEMED_ICON (icon))); - g_debug ("Icon validation: themed icon (%s) is ok", a); - return TRUE; - } - - if (!G_IS_BYTES_ICON (icon)) - { - g_warning ("Unexpected icon type: %s", G_OBJECT_TYPE_NAME (icon)); - return FALSE; - } - if (g_getenv ("XDP_VALIDATE_ICON")) icon_validator = g_getenv ("XDP_VALIDATE_ICON"); @@ -612,39 +574,13 @@ xdp_validate_serialized_icon (GVariant *v, return FALSE; } - bytes = g_bytes_icon_get_bytes (G_BYTES_ICON (icon)); - fd = g_file_open_tmp ("iconXXXXXX", &name, &error); - if (fd == -1) - { - g_warning ("Icon validation: %s", error->message); - return FALSE; - } - - stream = g_unix_output_stream_new (fd, FALSE); - - /* Use write_all() instead of write_bytes() so we don't have to worry about - * partial writes (https://gitlab.gnome.org/GNOME/glib/-/issues/570). - */ - bytes_data = g_bytes_get_data (bytes, &bytes_len); - if (!g_output_stream_write_all (stream, bytes_data, bytes_len, NULL, NULL, &error)) - { - g_warning ("Icon validation: %s", error->message); - return FALSE; - } - - if (!g_output_stream_close (stream, NULL, &error)) - { - g_warning ("Icon validation: %s", error->message); - return FALSE; - } - args[0] = icon_validator; args[1] = "--sandbox"; args[2] = "--fd"; args[3] = G_STRINGIFY (VALIDATOR_INPUT_FD); args[4] = NULL; - output = xdp_spawn_full (args, xdp_steal_fd (&fd), VALIDATOR_INPUT_FD, &error); + output = xdp_spawn_full (args, xdp_sealed_fd_dup_fd (icon), VALIDATOR_INPUT_FD, &error); if (!output) { g_warning ("Icon validation: Rejecting icon because validator failed: %s", error->message); diff --git a/src/xdp-utils.h b/src/xdp-utils.h index 4b8304a85..b58ab762d 100644 --- a/src/xdp-utils.h +++ b/src/xdp-utils.h @@ -30,6 +30,7 @@ #include #include "glib-backports.h" +#include "xdp-sealed-fd.h" #define DESKTOP_PORTAL_OBJECT_PATH "/org/freedesktop/portal/desktop" @@ -42,10 +43,9 @@ gboolean xdp_is_valid_app_id (const char *string); char *xdp_get_app_id_from_desktop_id (const char *desktop_id); -gboolean xdp_validate_serialized_icon (GVariant *v, - gboolean bytes_only, - char **out_format, - char **out_size); +gboolean xdp_validate_icon (XdpSealedFd *icon, + char **out_format, + char **out_size); typedef void (*XdpPeerDiedCallback) (const char *name);