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

@@ -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,

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);
 

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);

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);