From c38e481fb42eafa8f3ecb1ac7f7ca14785968ec2 Mon Sep 17 00:00:00 2001 From: Sebastian Wick Date: Fri, 22 Aug 2025 17:28:44 +0200 Subject: [PATCH] context: Handle x11-fallback by converting to a conditional This internally converts `x11-fallback` to `if:x11:!has-wayland` at the earliest place possible, and converts back when serializing to a file. --- common/flatpak-context-private.h | 2 +- common/flatpak-context.c | 262 ++++++++++++++++++++++++++++++- tests/test-context.c | 74 +++++++++ tests/test-exports.c | 5 +- 4 files changed, 337 insertions(+), 6 deletions(-) diff --git a/common/flatpak-context-private.h b/common/flatpak-context-private.h index d172e201..79643b7b 100644 --- a/common/flatpak-context-private.h +++ b/common/flatpak-context-private.h @@ -55,7 +55,7 @@ typedef enum { FLATPAK_CONTEXT_SOCKET_PULSEAUDIO = 1 << 2, FLATPAK_CONTEXT_SOCKET_SESSION_BUS = 1 << 3, FLATPAK_CONTEXT_SOCKET_SYSTEM_BUS = 1 << 4, - FLATPAK_CONTEXT_SOCKET_FALLBACK_X11 = 1 << 5, /* For backwards compat, also set SOCKET_X11 */ + FLATPAK_CONTEXT_SOCKET_FALLBACK_X11 = 1 << 5, /* For backwards compat, not used internally */ FLATPAK_CONTEXT_SOCKET_SSH_AUTH = 1 << 6, FLATPAK_CONTEXT_SOCKET_PCSC = 1 << 7, FLATPAK_CONTEXT_SOCKET_CUPS = 1 << 8, diff --git a/common/flatpak-context.c b/common/flatpak-context.c index a0557ab5..eb1f98f2 100644 --- a/common/flatpak-context.c +++ b/common/flatpak-context.c @@ -208,6 +208,28 @@ flatpak_permission_set_allowed_if (FlatpakPermission *permission, g_ptr_array_sort (permission->conditionals, flatpak_strcmp0_ptr); } +static void +flatpak_permission_remove_conditional (FlatpakPermission *permission, + const char *condition) +{ + guint index; + + /* If we are already unconditionally allowed, we don't have conditions */ + if (permission->allowed) + return; + + /* The only way to correcly layer removal of conditional is to completely + remove eveything from the lower layer */ + permission->reset = TRUE; + + if (!g_ptr_array_find_with_equal_func (permission->conditionals, + condition, + g_str_equal, &index)) + return; + + g_ptr_array_remove_index (permission->conditionals, index); +} + static void flatpak_permission_serialize (FlatpakPermission *permission, const char *name, @@ -533,6 +555,17 @@ flatpak_permissions_set_allowed_if (GHashTable *permissions, condition); } + +static void +flatpak_permissions_remove_conditional (GHashTable *permissions, + const char *name, + const char *condition) +{ + flatpak_permission_remove_conditional (flatpak_permissions_ensure (permissions, + name), + condition); +} + static gboolean flatpak_permissions_allows_unconditionally (GHashTable *permissions, const char *name) @@ -606,6 +639,56 @@ flatpak_permissions_compute_allowed (GHashTable *permissi return bitmask; } +static void +flatpak_canonicalize_x11_permissions (GHashTable *permissions) +{ + /* The on-disk format for sockets supports the old fallback-x11 + * permission, but in-memory we remove that converting it to a modern. + * conditional check if-wayland. + */ + + FlatpakPermission *fallback_x11 = g_hash_table_lookup (permissions, "fallback-x11"); + if (fallback_x11) + { + /* Remove full-access plain x11, which used to be added when + fallback-x11 was added. */ + FlatpakPermission *x11 = flatpak_permissions_ensure (permissions, "x11"); + x11->allowed = FALSE; + x11->reset = FALSE; + + if (fallback_x11->allowed) + flatpak_permission_set_allowed_if (x11, "!has-wayland"); + else + flatpak_permission_remove_conditional (x11, "!has-wayland"); + + /* Remove fallback-x11 (which is deprecated) */ + g_hash_table_remove (permissions, "fallback-x11"); + } +} + +static GHashTable * +flatpak_decanonicalize_x11_permissions (GHashTable *permissions) +{ + /* Convert from internal format to on-disk backwards compatible format. + * Note: This only handles the specific case where there is only + * the fallback-x11 conditional. More complex cases are handled + * with the full conditional syntax. + */ + + FlatpakPermission *x11 = g_hash_table_lookup (permissions, "x11"); + if (x11 != NULL && !x11->allowed && x11->conditionals->len == 1 && + strcmp (x11->conditionals->pdata[0], "!has-wayland") == 0) + { + GHashTable *copy = flatpak_permissions_dup (permissions); + flatpak_permissions_set_allowed (copy, "fallback-x11"); + g_hash_table_remove (copy, "x11"); + + return copy; + } + + return g_hash_table_ref (permissions); +} + static gboolean flatpak_permissions_from_strv (GHashTable *permissions, const char **strv, @@ -927,10 +1010,163 @@ static void flatpak_permissions_test_backwards_compat (void) } } +static void flatpak_permissions_test_fallback_x11 (void) +{ + g_autoptr(GHashTable) perms = NULL; + + { + FlatpakPermission *x11; + FlatpakPermission *wayland; + g_autoptr(GError) error = NULL; + gboolean ok; + + perms = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms, + (const char * []) { + "fallback-x11", + "wayland", + NULL, + }, + &error); + g_assert_true (ok); + g_assert_no_error (error); + g_assert_nonnull (perms); + flatpak_canonicalize_x11_permissions (perms); + + g_assert_cmpint (g_hash_table_size (perms), ==, 2); + + x11 = g_hash_table_lookup (perms, "x11"); + g_assert_nonnull (x11); + wayland = g_hash_table_lookup (perms, "wayland"); + g_assert_nonnull (wayland); + + g_assert_false (x11->allowed); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "!has-wayland"); + g_assert_true (wayland->allowed); + } + + { + g_autoptr(GHashTable) perms2 = NULL; + FlatpakPermission *x11; + FlatpakPermission *wayland; + g_autoptr(GError) error = NULL; + gboolean ok; + + perms2 = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms2, + (const char * []) { + "if:x11:!has-wayland", + NULL, + }, + &error); + g_assert_true (ok); + g_assert_no_error (error); + g_assert_nonnull (perms2); + flatpak_canonicalize_x11_permissions (perms2); + + g_assert_cmpint (g_hash_table_size (perms2), ==, 1); + + x11 = g_hash_table_lookup (perms, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "!has-wayland"); + + /* lower: fallback-x11 + * upper: if:x11:!has-wayland + * -> if:x11:!has-wayland */ + flatpak_permissions_merge (perms, perms2); + + g_assert_cmpint (g_hash_table_size (perms), ==, 2); + + x11 = g_hash_table_lookup (perms, "x11"); + g_assert_nonnull (x11); + wayland = g_hash_table_lookup (perms, "wayland"); + g_assert_nonnull (wayland); + + g_assert_false (x11->allowed); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "!has-wayland"); + g_assert_true (wayland->allowed); + } + + { + g_autoptr(GHashTable) perms2 = NULL; + g_autoptr(GHashTable) perms3 = NULL; + FlatpakPermission *x11; + g_autoptr(GError) error = NULL; + gboolean ok; + + perms2 = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms2, + (const char * []) { + "fallback-x11", + "if:x11:foo", + NULL, + }, + &error); + g_assert_true (ok); + g_assert_no_error (error); + g_assert_nonnull (perms2); + flatpak_canonicalize_x11_permissions (perms2); + + g_assert_cmpint (g_hash_table_size (perms2), ==, 1); + + x11 = g_hash_table_lookup (perms2, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_false (x11->reset); + g_assert_cmpint (x11->conditionals->len, ==, 2); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "!has-wayland"); + g_assert_cmpstr (x11->conditionals->pdata[1], ==, "foo"); + + perms3 = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms3, + (const char * []) { + "if:x11:!has-wayland", + "!fallback-x11", + "if:x11:bar", + NULL, + }, + &error); + g_assert_true (ok); + g_assert_no_error (error); + g_assert_nonnull (perms3); + flatpak_canonicalize_x11_permissions (perms3); + + g_assert_cmpint (g_hash_table_size (perms3), ==, 1); + + x11 = g_hash_table_lookup (perms3, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_true (x11->reset); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "bar"); + + /* lower: fallback-x11;if:x11:foo + * upper: if:x11:!has-wayland;!fallback-x11;if:x11:bar + * -> if:x11:bar + * The !fallback-x11 removes the if:x11:!has-wayland conditional which + * turns into !x11. */ + flatpak_permissions_merge (perms2, perms3); + + g_assert_cmpint (g_hash_table_size (perms2), ==, 1); + + x11 = g_hash_table_lookup (perms2, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "bar"); + } +} + FLATPAK_INTERNAL_TEST("/context/permissions/basic", flatpak_permissions_test_basic); FLATPAK_INTERNAL_TEST("/context/permissions/backwards-compat", flatpak_permissions_test_backwards_compat); +FLATPAK_INTERNAL_TEST("/context/permissions/fallback-x11", + flatpak_permissions_test_fallback_x11); #endif /* INCLUDE_INTERNAL_TESTS */ @@ -1970,6 +2206,14 @@ option_socket_cb (const gchar *option_name, if (socket == 0) return FALSE; + if (socket == FLATPAK_CONTEXT_SOCKET_FALLBACK_X11) + { + flatpak_permissions_set_allowed_if (context->socket_permissions, + "x11", + "!has-wayland"); + return TRUE; + } + flatpak_permissions_set_allowed (context->socket_permissions, value); @@ -1989,6 +2233,13 @@ option_nosocket_cb (const gchar *option_name, if (socket == 0) return FALSE; + if (socket == FLATPAK_CONTEXT_SOCKET_FALLBACK_X11) + { + flatpak_permissions_remove_conditional (context->socket_permissions, + "x11", "!has-wayland"); + return TRUE; + } + flatpak_permissions_set_not_allowed (context->socket_permissions, value); @@ -2034,6 +2285,13 @@ option_socket_if_cb (const gchar *option_name, if (socket == 0) return FALSE; + if (socket == FLATPAK_CONTEXT_SOCKET_FALLBACK_X11) + { + g_set_error (error, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, + _("fallback-x11 can not be conditional")); + return FALSE; + } + flatpak_permissions_set_allowed_if (context->socket_permissions, name, condition); return TRUE; @@ -2685,6 +2943,7 @@ flatpak_context_load_metadata (FlatpakContext *context, if (!flatpak_permissions_from_strv (context->socket_permissions, (const char **)sockets, error)) return FALSE; + flatpak_canonicalize_x11_permissions (context->socket_permissions); } if (g_key_file_has_key (metakey, FLATPAK_METADATA_GROUP_CONTEXT, FLATPAK_METADATA_KEY_DEVICES, NULL)) @@ -2983,7 +3242,8 @@ flatpak_context_save_metadata (FlatpakContext *context, } shared = flatpak_context_shared_to_string (shares_mask, shares_valid); - sockets = flatpak_permissions_to_strv (context->socket_permissions, flatten); + g_autoptr(GHashTable) socket_permissions = flatpak_decanonicalize_x11_permissions (context->socket_permissions); + sockets = flatpak_permissions_to_strv (socket_permissions, flatten); devices = flatpak_permissions_to_strv (context->device_permissions, flatten); features = flatpak_context_features_to_string (features_mask, features_valid); diff --git a/tests/test-context.c b/tests/test-context.c index 7e1e4472..f8013332 100644 --- a/tests/test-context.c +++ b/tests/test-context.c @@ -948,6 +948,79 @@ test_devices (void) g_assert_cmpint (devices, ==, FLATPAK_CONTEXT_DEVICE_INPUT); } +static gboolean +test_sockets_evaluate_conditions_false (FlatpakContextConditions condition) +{ + return FALSE; +} + +static gboolean +test_sockets_evaluate_conditions_has_wayland (FlatpakContextConditions condition) +{ + switch (condition) + { + case FLATPAK_CONTEXT_CONDITION_HAS_WAYLAND: + return TRUE; + default: + return FALSE; + } +} + +static void +test_sockets (void) +{ + FlatpakContextSockets sockets; + + /* test fallback-x11 special handling */ + g_autoptr(GKeyFile) keyfile1 = g_key_file_new (); + g_key_file_set_value (keyfile1, + FLATPAK_METADATA_GROUP_CONTEXT, + FLATPAK_METADATA_KEY_SOCKETS, + "fallback-x11;wayland"); + g_autoptr(FlatpakContext) context1 = flatpak_context_new (); + flatpak_context_load_metadata (context1, keyfile1, NULL); + + + g_autoptr(GKeyFile) metakey = g_key_file_new (); + g_autofree char *sockets_str = NULL; + flatpak_context_save_metadata (context1, FALSE, metakey); + sockets_str = g_key_file_get_value (metakey, + FLATPAK_METADATA_GROUP_CONTEXT, + FLATPAK_METADATA_KEY_SOCKETS, + NULL); + g_assert_cmpstr (sockets_str, ==, "fallback-x11;wayland;"); + + sockets = flatpak_context_compute_allowed_sockets ( + context1, + test_sockets_evaluate_conditions_false); + g_assert_cmpint (sockets, ==, FLATPAK_CONTEXT_SOCKET_X11 | + FLATPAK_CONTEXT_SOCKET_WAYLAND); + + sockets = flatpak_context_compute_allowed_sockets ( + context1, + test_sockets_evaluate_conditions_has_wayland); + g_assert_cmpint (sockets, ==, FLATPAK_CONTEXT_SOCKET_WAYLAND); + + g_autoptr(GKeyFile) keyfile2 = g_key_file_new (); + g_key_file_set_value (keyfile2, + FLATPAK_METADATA_GROUP_CONTEXT, + FLATPAK_METADATA_KEY_SOCKETS, + "!x11"); + g_autoptr(FlatpakContext) context2 = flatpak_context_new (); + flatpak_context_load_metadata (context2, keyfile2, NULL); + flatpak_context_merge (context1, context2); + + sockets = flatpak_context_compute_allowed_sockets ( + context1, + test_sockets_evaluate_conditions_false); + g_assert_cmpint (sockets, ==, FLATPAK_CONTEXT_SOCKET_WAYLAND); + + sockets = flatpak_context_compute_allowed_sockets ( + context1, + test_sockets_evaluate_conditions_has_wayland); + g_assert_cmpint (sockets, ==, FLATPAK_CONTEXT_SOCKET_WAYLAND); +} + int main (int argc, char *argv[]) { @@ -959,6 +1032,7 @@ main (int argc, char *argv[]) g_test_add_func ("/context/validate-path-args", test_validate_path_args); g_test_add_func ("/context/validate-path-meta", test_validate_path_meta); g_test_add_func ("/context/devices", test_devices); + g_test_add_func ("/context/sockets", test_sockets); g_test_add_func ("/context/usb-list", test_usb_list); g_test_add_func ("/context/usb-rules/all", test_usb_rules_all); diff --git a/tests/test-exports.c b/tests/test-exports.c index f0dc46bb..a93bfe05 100644 --- a/tests/test-exports.c +++ b/tests/test-exports.c @@ -278,13 +278,11 @@ test_full_context (void) FLATPAK_CONTEXT_DEVICE_SHM)); FlatpakContextSockets sockets = flatpak_context_compute_allowed_sockets (context, NULL); g_assert_cmpuint (sockets, ==, - (FLATPAK_CONTEXT_SOCKET_X11 | - FLATPAK_CONTEXT_SOCKET_WAYLAND | + (FLATPAK_CONTEXT_SOCKET_WAYLAND | FLATPAK_CONTEXT_SOCKET_INHERIT_WAYLAND_SOCKET | FLATPAK_CONTEXT_SOCKET_PULSEAUDIO | FLATPAK_CONTEXT_SOCKET_SESSION_BUS | FLATPAK_CONTEXT_SOCKET_SYSTEM_BUS | - FLATPAK_CONTEXT_SOCKET_FALLBACK_X11 | FLATPAK_CONTEXT_SOCKET_SSH_AUTH | FLATPAK_CONTEXT_SOCKET_PCSC | FLATPAK_CONTEXT_SOCKET_CUPS)); @@ -378,7 +376,6 @@ test_full_context (void) g_assert_cmpstr (strv[i++], ==, "ssh-auth"); g_assert_cmpstr (strv[i++], ==, "system-bus"); g_assert_cmpstr (strv[i++], ==, "wayland"); - g_assert_cmpstr (strv[i++], ==, "x11"); g_assert_cmpstr (strv[i], ==, NULL); g_assert_cmpuint (i, ==, n); g_clear_pointer (&strv, g_strfreev);