diff --git a/common/flatpak-context.c b/common/flatpak-context.c index 75e12850e..60f42d2ce 100644 --- a/common/flatpak-context.c +++ b/common/flatpak-context.c @@ -216,28 +216,6 @@ 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 correctly layer removal of conditional is to completely - remove everything 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, @@ -564,17 +542,6 @@ 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) @@ -649,56 +616,6 @@ 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, @@ -750,6 +667,13 @@ flatpak_permissions_merge (GHashTable *permissions, const char *name; FlatpakPermission *other_permission; GHashTableIter iter; + FlatpakPermission *x11; + + /* If we reset the x11 conditionals, the fallback-x11 permission also must go + * because it is a conditional. */ + x11 = g_hash_table_lookup (other, "x11"); + if (x11 && x11->reset) + g_hash_table_remove (permissions, "fallback-x11"); g_hash_table_iter_init (&iter, other); while (g_hash_table_iter_next (&iter, @@ -1023,9 +947,10 @@ static void flatpak_permissions_test_backwards_compat (void) static void flatpak_permissions_test_fallback_x11 (void) { g_autoptr(GHashTable) perms = NULL; + g_autoptr(GHashTable) res_perms = NULL; { - FlatpakPermission *x11; + FlatpakPermission *fallback_x11; FlatpakPermission *wayland; g_autoptr(GError) error = NULL; gboolean ok; @@ -1041,77 +966,31 @@ static void flatpak_permissions_test_fallback_x11 (void) 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); + fallback_x11 = g_hash_table_lookup (perms, "fallback-x11"); + g_assert_nonnull (fallback_x11); + g_assert_true (fallback_x11->allowed); 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); } + /* If we only add a conditional, we don't reset fallback-x11 */ { g_autoptr(GHashTable) perms2 = NULL; FlatpakPermission *x11; + FlatpakPermission *fallback_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; + res_perms = flatpak_permissions_dup (perms); perms2 = flatpak_permissions_new (); ok = flatpak_permissions_from_strv (perms2, (const char * []) { - "fallback-x11", "if:x11:foo", NULL, }, @@ -1119,7 +998,6 @@ static void flatpak_permissions_test_fallback_x11 (void) 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); @@ -1127,47 +1005,124 @@ static void flatpak_permissions_test_fallback_x11 (void) 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"); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "foo"); - perms3 = flatpak_permissions_new (); - ok = flatpak_permissions_from_strv (perms3, + flatpak_permissions_merge (res_perms, perms2); + + g_assert_cmpint (g_hash_table_size (res_perms), ==, 3); + + x11 = g_hash_table_lookup (res_perms, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_false (x11->reset); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "foo"); + + fallback_x11 = g_hash_table_lookup (res_perms, "fallback-x11"); + g_assert_nonnull (fallback_x11); + g_assert_true (fallback_x11->allowed); + + wayland = g_hash_table_lookup (res_perms, "wayland"); + g_assert_nonnull (wayland); + g_assert_true (wayland->allowed); + } + + /* If we set x11, we reset conditionals *and* fallback-x11 */ + { + g_autoptr(GHashTable) perms2 = NULL; + FlatpakPermission *x11; + FlatpakPermission *wayland; + g_autoptr(GError) error = NULL; + gboolean ok; + + res_perms = flatpak_permissions_dup (perms); + + perms2 = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms2, (const char * []) { - "if:x11:!has-wayland", - "!fallback-x11", - "if:x11:bar", + "x11", 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_nonnull (perms2); g_assert_cmpint (g_hash_table_size (perms2), ==, 1); + x11 = g_hash_table_lookup (perms2, "x11"); + g_assert_nonnull (x11); + g_assert_true (x11->allowed); + g_assert_true (x11->reset); + + flatpak_permissions_merge (res_perms, perms2); + + g_assert_cmpint (g_hash_table_size (res_perms), ==, 2); + + x11 = g_hash_table_lookup (res_perms, "x11"); + g_assert_nonnull (x11); + g_assert_true (x11->allowed); + + wayland = g_hash_table_lookup (res_perms, "wayland"); + g_assert_nonnull (wayland); + g_assert_true (wayland->allowed); + } + + /* Only add a conditional and nosocket fallback-x11, gives nosocket fallback-x11 */ + { + g_autoptr(GHashTable) perms2 = NULL; + FlatpakPermission *x11; + FlatpakPermission *fallback_x11; + FlatpakPermission *wayland; + g_autoptr(GError) error = NULL; + gboolean ok; + + res_perms = flatpak_permissions_dup (perms); + + perms2 = flatpak_permissions_new (); + ok = flatpak_permissions_from_strv (perms2, + (const char * []) { + "if:x11:foo", + "!fallback-x11", + NULL, + }, + &error); + g_assert_true (ok); + g_assert_no_error (error); + g_assert_nonnull (perms2); + + g_assert_cmpint (g_hash_table_size (perms2), ==, 2); + 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, ==, 1); - g_assert_cmpstr (x11->conditionals->pdata[0], ==, "bar"); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "foo"); + + fallback_x11 = g_hash_table_lookup (perms2, "fallback-x11"); + g_assert_nonnull (fallback_x11); + g_assert_false (fallback_x11->allowed); + + flatpak_permissions_merge (res_perms, perms2); + + g_assert_cmpint (g_hash_table_size (res_perms), ==, 3); + + x11 = g_hash_table_lookup (res_perms, "x11"); + g_assert_nonnull (x11); + g_assert_false (x11->allowed); + g_assert_false (x11->reset); + g_assert_cmpint (x11->conditionals->len, ==, 1); + g_assert_cmpstr (x11->conditionals->pdata[0], ==, "foo"); + + fallback_x11 = g_hash_table_lookup (res_perms, "fallback-x11"); + g_assert_nonnull (fallback_x11); + g_assert_false (fallback_x11->allowed); + + wayland = g_hash_table_lookup (res_perms, "wayland"); + g_assert_nonnull (wayland); + g_assert_true (wayland->allowed); } } @@ -2138,14 +2093,6 @@ 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); @@ -2165,13 +2112,6 @@ 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); @@ -2826,7 +2766,6 @@ 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)) @@ -3105,8 +3044,7 @@ flatpak_context_save_metadata (FlatpakContext *context, int i; shared = flatpak_permissions_to_strv (context->shares_permissions, flatten); - g_autoptr(GHashTable) socket_permissions = flatpak_decanonicalize_x11_permissions (context->socket_permissions); - sockets = flatpak_permissions_to_strv (socket_permissions, flatten); + sockets = flatpak_permissions_to_strv (context->socket_permissions, flatten); devices = flatpak_permissions_to_strv (context->device_permissions, flatten); features = flatpak_permissions_to_strv (context->features_permissions, flatten); @@ -4500,7 +4438,27 @@ FlatpakContextSockets flatpak_context_compute_allowed_sockets (FlatpakContext *context, FlatpakContextConditionEvaluator evaluator) { - return flatpak_permissions_compute_allowed (context->socket_permissions, + g_autoptr(GHashTable) permissions = + g_hash_table_new_similar (context->socket_permissions); + GHashTableIter iter; + gpointer key, value; + FlatpakPermission *fallback_x11; + + g_hash_table_iter_init (&iter, context->socket_permissions); + while (g_hash_table_iter_next (&iter, &key, &value)) + g_hash_table_insert (permissions, g_strdup (key), flatpak_permission_dup (value)); + + fallback_x11 = g_hash_table_lookup (context->socket_permissions, "fallback-x11"); + if (fallback_x11 && fallback_x11->allowed) + { + FlatpakPermission *x11 = flatpak_permissions_ensure (permissions, "x11"); + + x11->allowed = FALSE; + flatpak_permission_set_allowed_if (x11, "!has-wayland"); + } + g_hash_table_remove (permissions, "fallback-x11"); + + return flatpak_permissions_compute_allowed (permissions, flatpak_context_sockets, evaluator); } diff --git a/tests/test-exports.c b/tests/test-exports.c index 303db7dbc..2146488be 100644 --- a/tests/test-exports.c +++ b/tests/test-exports.c @@ -371,6 +371,7 @@ 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);