From db885e0542700fa5aa98f890a78699ee6e182bb7 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Wed, 26 Jan 2022 16:11:46 +0000
Subject: [PATCH] run: Allow remapping Xauthority entries for remote or
 forwarded X11

As with non-path-based AF_UNIX sockets, both of these are going to
require --share=network to be enabled, so print a warning if it isn't.
We don't automatically enable --share=network, because that elevates
the privileges of apps that would otherwise have entered a new network
namespace, but regular users of remote X11 can choose to enable it with
`flatpak run --share=network` or `flatpak override --share=network`.

Resolves: https://github.com/flatpak/flatpak/issues/397
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 common/flatpak-run.c | 52 +++++++++++++++++++++++++++-----------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/common/flatpak-run.c b/common/flatpak-run.c
index 2678bc4c..c8465fcf 100644
--- a/common/flatpak-run.c
+++ b/common/flatpak-run.c
@@ -116,15 +116,24 @@ auth_streq (const char *str,
 
 static gboolean
 xauth_entry_should_propagate (const Xauth *xa,
-                              const char  *hostname,
+                              int          family,
+                              const char  *remote_hostname,
+                              const char  *local_hostname,
                               const char  *number)
 {
-  /* ensure entry isn't for remote access */
-  if (xa->family != FamilyLocal && xa->family != FamilyWild)
+  /* ensure entry isn't for a different type of access */
+  if (family != FamilyWild && xa->family != family && xa->family != FamilyWild)
+    return FALSE;
+
+  /* ensure entry isn't for remote access, except that if remote_hostname
+   * is specified, then remote access to that hostname is OK */
+  if (xa->family != FamilyWild && xa->family != FamilyLocal &&
+      (remote_hostname == NULL ||
+       !auth_streq (remote_hostname, xa->address, xa->address_length)))
     return FALSE;
 
   /* ensure entry is for this machine */
-  if (xa->family == FamilyLocal && !auth_streq (hostname, xa->address, xa->address_length))
+  if (xa->family == FamilyLocal && !auth_streq (local_hostname, xa->address, xa->address_length))
     {
       /* OpenSUSE inherits the hostname value from DHCP without updating
        * its X11 authentication cookie. The old hostname value can still
@@ -152,7 +161,9 @@ xauth_entry_should_propagate (const Xauth *xa,
 }
 
 static void
-write_xauth (const char *number,
+write_xauth (int family,
+             const char *remote_host,
+             const char *number,
              const char *replace_number,
              FILE       *output)
 {
@@ -177,7 +188,8 @@ write_xauth (const char *number,
       xa = XauReadAuth (f);
       if (xa == NULL)
         break;
-      if (xauth_entry_should_propagate (xa, unames.nodename, number))
+      if (xauth_entry_should_propagate (xa, family, remote_host,
+                                        unames.nodename, number))
         {
           local_xa = *xa;
           if (local_xa.number != NULL && replace_number != NULL)
@@ -330,16 +342,8 @@ flatpak_run_add_x11_args (FlatpakBwrap         *bwrap,
 
       g_assert (original_display_nr != NULL);
 
-      if (family != FamilyLocal)
-        {
-          g_warning ("Non-local X11 address DISPLAY=%s", display);
-          flatpak_bwrap_unset_env (bwrap, "DISPLAY");
-          return;
-        }
-
-      g_assert (x11_socket != NULL);
-
-      if (g_file_test (x11_socket, G_FILE_TEST_EXISTS))
+      if (x11_socket != NULL
+          && g_file_test (x11_socket, G_FILE_TEST_EXISTS))
         {
           flatpak_bwrap_add_args (bwrap,
                                   "--ro-bind", x11_socket, "/tmp/.X11-unix/X99",
@@ -353,9 +357,16 @@ flatpak_run_add_x11_args (FlatpakBwrap         *bwrap,
            * doesn't exist, then the only way this is going to work
            * is if the app can connect to abstract socket
            * @/tmp/.X11-unix/X42 or to TCP port localhost:6042,
-           * either of which requires a shared network namespace. */
-          g_warning ("X11 socket %s does not exist in filesystem.",
-                     x11_socket);
+           * either of which requires a shared network namespace.
+           *
+           * Alternatively, if DISPLAY is othermachine:23, then we
+           * definitely need access to TCP port othermachine:6023. */
+          if (x11_socket != NULL)
+            g_warning ("X11 socket %s does not exist in filesystem.",
+                       x11_socket);
+          else
+            g_warning ("Remote X11 display detected.");
+
           g_warning ("X11 access will require --share=network permission.");
         }
       else if (x11_socket != NULL)
@@ -383,7 +394,8 @@ flatpak_run_add_x11_args (FlatpakBwrap         *bwrap,
                 {
                   static const char dest[] = "/run/flatpak/Xauthority";
 
-                  write_xauth (original_display_nr, replace_display_nr, output);
+                  write_xauth (family, remote_host, original_display_nr,
+                               replace_display_nr, output);
                   flatpak_bwrap_add_args_data_fd (bwrap, "--ro-bind-data", tmp_fd, dest);
 
                   flatpak_bwrap_set_env (bwrap, "XAUTHORITY", dest, TRUE);