Patches to add ability to run Xvnc a non-root user

c52747b9 · Tyson Whitehead · e2c9dfad · c52747b9 · c52747b9 · c52747b9
Commit c52747b9 authored Jun 11, 2020 by Tyson Whitehead
--- a/0001-Add-ability-to-drop-priviledges-switch-user-when-for.patch
+++ b/0001-Add-ability-to-drop-priviledges-switch-user-when-for.patch
+From 57a6b3d08e6cd365cf0013f48d5c9d1bf4a90cd4 Mon Sep 17 00:00:00 2001
+From: Tyson Whitehead <twhitehead@gmail.com>
+Date: Wed, 27 May 2020 18:45:00 -0400
+Subject: [PATCH 1/3] Add ability to drop priviledges (switch user) when
+ forking Xvnc
+
+---
+ common/configuration.c |  1 +
+ data/lightdm.conf      |  1 +
+ src/process.c          | 49 ++++++++++++++++++++++++++++++++++++++++++
+ src/process.h          |  3 +++
+ src/seat-xvnc.c        | 10 +++++++++
+ src/x-server-local.c   | 28 ++++++++++++++++++++----
+ src/x-server-local.h   |  3 +++
+ 7 files changed, 91 insertions(+), 4 deletions(-)
+
+diff --git a/common/configuration.c b/common/configuration.c
+index a1b30dad..51c8ec0b 100644
+--- a/common/configuration.c
+++ b/common/configuration.c
+@@ -399,6 +399,7 @@ config_init (Configuration *config)
+ 
+     g_hash_table_insert (config->priv->vnc_keys, "enabled", GINT_TO_POINTER (KEY_SUPPORTED));
+     g_hash_table_insert (config->priv->vnc_keys, "command", GINT_TO_POINTER (KEY_SUPPORTED));
+    g_hash_table_insert (config->priv->vnc_keys, "user", GINT_TO_POINTER (KEY_SUPPORTED));
+     g_hash_table_insert (config->priv->vnc_keys, "port", GINT_TO_POINTER (KEY_SUPPORTED));
+     g_hash_table_insert (config->priv->vnc_keys, "listen-address", GINT_TO_POINTER (KEY_SUPPORTED));
+     g_hash_table_insert (config->priv->vnc_keys, "width", GINT_TO_POINTER (KEY_SUPPORTED));
+diff --git a/data/lightdm.conf b/data/lightdm.conf
+index 2082c1ac..d2244168 100644
+--- a/data/lightdm.conf
+++ b/data/lightdm.conf
+@@ -162,6 +162,7 @@
+ [VNCServer]
+ #enabled=false
+ #command=Xvnc
+#user=
+ #port=5900
+ #listen-address=
+ #width=1024
+diff --git a/src/process.c b/src/process.c
+index 549ccd94..bf6765da 100644
+--- a/src/process.c
+++ b/src/process.c
+@@ -9,6 +9,7 @@
+  * license.
+  */
+ 
+#define _GNU_SOURCE      /* Required to set saved under uid and gid */
+ #include <stdlib.h>
+ #include <string.h>
+ #include <unistd.h>
+@@ -21,6 +22,7 @@
+ 
+ #include "log-file.h"
+ #include "process.h"
+#include "accounts.h"
+ 
+ enum {
+     GOT_DATA,
+@@ -44,6 +46,9 @@ struct ProcessPrivate
+     /* Command to run */
+     gchar *command;
+ 
+    /* Optional user to drop privileges (switch) to */
+    User *user;
+
+     /* TRUE to clear the environment in this process */
+     gboolean clear_environment;
+ 
+@@ -151,6 +156,16 @@ process_get_command (Process *process)
+     return process->priv->command;
+ }
+ 
+void
+process_set_user (Process *process, User *user)
+{
+    g_return_if_fail (process != NULL);
+    g_return_if_fail (user != NULL);
+
+    g_clear_object (&process->priv->user);
+    process->priv->user = g_object_ref (user);
+}
+
+ static void
+ process_watch_cb (GPid pid, gint status, gpointer data)
+ {
+@@ -235,6 +250,39 @@ process_start (Process *process, gboolean block)
+         /* Reset SIGPIPE handler so the child has default behaviour (we disabled it at LightDM start) */
+         signal (SIGPIPE, SIG_DFL);
+ 
+        /* Drop privileges (checks duplicated from OpenSSH code) */
+        if (process->priv->user != NULL && getuid () == 0) {
+            gid_t gid_new = user_get_gid (process->priv->user);
+            gid_t uid_new = user_get_uid (process->priv->user);
+            gid_t gid_old = getgid ();
+            uid_t uid_old = getuid ();
+
+            /* Switch gid and uid (gid has to be first) */
+            if (setresgid (gid_new, gid_new, gid_new) == -1)
+                g_error ("Switching user: setresgid %u failed: %s", (u_int)gid_new, strerror (errno));
+
+            if (setresuid(uid_new, uid_new, uid_new) == -1)
+                g_error ("Switching user: setresuid %u failed: %s", (u_int)uid_new, strerror (errno));
+
+            /* Verify unable to restore gid */
+            if (gid_old != gid_new && uid_new != 0)
+                if (setgid (gid_old) != -1 || setegid (gid_old) != -1)
+                    g_error ("Switched user: able to restore old [e]gid");
+
+            /* Verify gid drop was successful  */
+            if (getgid () != gid_new || getegid () != gid_new)
+                g_error ("Switched user: incorrect gid:%u egid:%u (should be %u)", (u_int)getgid (), (u_int)getegid (), (u_int)gid_new);
+
+            /* Verify unable to restore uid */
+            if (uid_old != uid_new)
+                if (setuid (uid_old) != -1 || seteuid (uid_old) != -1)
+                    g_error ("Switched user: able to restore old [e]uid");
+
+            /* Verify uid drop was successful  */
+            if (getuid () != uid_new || geteuid () != uid_new)
+                g_error ("Switched user: incorrect uid:%u euid:%u (should be %u)", (u_int)getuid (), (u_int)geteuid (), (u_int)uid_new);
+        }
+
+         execvp (argv[0], argv);
+         _exit (EXIT_FAILURE);
+     }
+@@ -353,6 +401,7 @@ process_finalize (GObject *object)
+ 
+     g_clear_pointer (&self->priv->log_file, g_free);
+     g_clear_pointer (&self->priv->command, g_free);
+    g_clear_object (&self->priv->user);
+     g_hash_table_unref (self->priv->env);
+     if (self->priv->quit_timeout)
+         g_source_remove (self->priv->quit_timeout);
+diff --git a/src/process.h b/src/process.h
+index 395762fe..d857d915 100644
+--- a/src/process.h
+++ b/src/process.h
+@@ -15,6 +15,7 @@
+ #include <glib-object.h>
+ 
+ #include "log-file.h"
+#include "accounts.h"
+ 
+ G_BEGIN_DECLS
+ 
+@@ -67,6 +68,8 @@ void process_set_command (Process *process, const gchar *command);
+ 
+ const gchar *process_get_command (Process *process);
+ 
+void process_set_user (Process *process, User *user);
+
+ gboolean process_start (Process *process, gboolean block);
+ 
+ gboolean process_get_is_running (Process *process);
+diff --git a/src/seat-xvnc.c b/src/seat-xvnc.c
+index 232bcc72..d16d5287 100644
+--- a/src/seat-xvnc.c
+++ b/src/seat-xvnc.c
+@@ -14,6 +14,7 @@
+ #include "seat-xvnc.h"
+ #include "x-server-xvnc.h"
+ #include "configuration.h"
+#include "accounts.h"
+ 
+ G_DEFINE_TYPE (SeatXVNC, seat_xvnc, SEAT_TYPE)
+ 
+@@ -62,6 +63,15 @@ seat_xvnc_create_display_server (Seat *seat, Session *session)
+     if (command)
+         x_server_local_set_command (X_SERVER_LOCAL (x_server), command);
+ 
+    const gchar *username = config_get_string (config_get_instance (), "VNCServer", "user");
+    if (username) {
+        g_autoptr(User) user = accounts_get_user_by_name (username);
+        if (user)
+            x_server_local_set_user (X_SERVER_LOCAL (x_server), user);
+        else
+            l_warning(seat, "Unable to lookup records for user %s (will default to running user)", username);
+    }
+
+     if (config_has_key (config_get_instance (), "VNCServer", "width") &&
+         config_has_key (config_get_instance (), "VNCServer", "height"))
+     {
+diff --git a/src/x-server-local.c b/src/x-server-local.c
+index 80e0ab0d..b42df206 100644
+--- a/src/x-server-local.c
+++ b/src/x-server-local.c
+@@ -19,6 +19,7 @@
+ 
+ #include "x-server-local.h"
+ #include "configuration.h"
+#include "accounts.h"
+ #include "process.h"
+ #include "vt.h"
+ 
+@@ -30,6 +31,9 @@ struct XServerLocalPrivate
+     /* Command to run the X server */
+     gchar *command;
+ 
+    /* Optional user to drop privileges (switch) to */
+    User *user;
+
+     /* Display number to use */
+     guint display_number;
+ 
+@@ -192,6 +196,15 @@ x_server_local_set_command (XServerLocal *server, const gchar *command)
+     server->priv->command = g_strdup (command);
+ }
+ 
+void
+x_server_local_set_user(XServerLocal *server, User *user)
+{
+    g_return_if_fail (server != NULL);
+    g_return_if_fail (user != NULL);
+    g_clear_object (&server->priv->user);
+    server->priv->user = g_object_ref(user);
+}
+
+ void
+ x_server_local_set_vt (XServerLocal *server, gint vt)
+ {
+@@ -393,13 +406,14 @@ write_authority_file (XServerLocal *server)
+     /* Get file to write to if have authority */
+     if (!server->priv->authority_file)
+     {
+-        g_autofree gchar *run_dir = NULL;
+-        g_autofree gchar *dir = NULL;
+        g_autofree gchar *run_dir = config_get_string (config_get_instance (), "LightDM", "run-directory");
+        g_autofree gchar *dir = g_build_filename (run_dir, server->priv->user ? user_get_name (server->priv->user) : "root", NULL);
+ 
+-        run_dir = config_get_string (config_get_instance (), "LightDM", "run-directory");
+-        dir = g_build_filename (run_dir, "root", NULL);
+         if (g_mkdir_with_parents (dir, S_IRWXU) < 0)
+             l_warning (server, "Failed to make authority directory %s: %s", dir, strerror (errno));
+        if (server->priv->user != NULL && getuid () == 0)
+            if (chown (dir, user_get_uid (server->priv->user), user_get_gid (server->priv->user)) < 0)
+                l_warning (server, "Failed to set ownership of x-server authority dir: %s", strerror (errno));
+ 
+         server->priv->authority_file = g_build_filename (dir, x_server_get_address (X_SERVER (server)), NULL);
+     }
+@@ -410,6 +424,9 @@ write_authority_file (XServerLocal *server)
+     x_authority_write (authority, XAUTH_WRITE_MODE_REPLACE, server->priv->authority_file, &error);
+     if (error)
+         l_warning (server, "Failed to write authority: %s", error->message);
+    if (server->priv->user != NULL && getuid () == 0)
+      if (chown (server->priv->authority_file, user_get_uid (server->priv->user), user_get_gid (server->priv->user)) < 0)
+            l_warning (server, "Failed to set ownership of authority: %s", strerror (errno));
+ }
+ 
+ static gboolean
+@@ -489,6 +506,8 @@ x_server_local_start (DisplayServer *display_server)
+         X_SERVER_LOCAL_GET_CLASS (server)->add_args (server, command);
+ 
+     process_set_command (server->priv->x_server_process, command->str);
+    if (server->priv->user)
+        process_set_user (server->priv->x_server_process, server->priv->user);
+ 
+     l_debug (display_server, "Launching X Server");
+ 
+@@ -550,6 +569,7 @@ x_server_local_finalize (GObject *object)
+         g_signal_handlers_disconnect_matched (self->priv->x_server_process, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self);
+     g_clear_object (&self->priv->x_server_process);
+     g_clear_pointer (&self->priv->command, g_free);
+    g_clear_object (&self->priv->user);
+     g_clear_pointer (&self->priv->config_file, g_free);
+     g_clear_pointer (&self->priv->layout, g_free);
+     g_clear_pointer (&self->priv->xdg_seat, g_free);
+diff --git a/src/x-server-local.h b/src/x-server-local.h
+index c1714585..b68225bb 100644
+--- a/src/x-server-local.h
+++ b/src/x-server-local.h
+@@ -14,6 +14,7 @@
+ 
+ #include "x-server.h"
+ #include "process.h"
+#include "accounts.h"
+ 
+ G_BEGIN_DECLS
+ 
+@@ -56,6 +57,8 @@ XServerLocal *x_server_local_new (void);
+ 
+ void x_server_local_set_command (XServerLocal *server, const gchar *command);
+ 
+void x_server_local_set_user(XServerLocal *server, User *user);
+
+ void x_server_local_set_vt (XServerLocal *server, gint vt);
+ 
+ void x_server_local_set_config (XServerLocal *server, const gchar *path);
+-- 
+2.25.4
+
--- a/0003-Detect-local-X-start-by-socket-creation-if-switching.patch
+++ b/0003-Detect-local-X-start-by-socket-creation-if-switching.patch
+From 0990b9cf0def423515b96ec23c90062458dc9cb7 Mon Sep 17 00:00:00 2001
+From: Tyson Whitehead <twhitehead@gmail.com>
+Date: Wed, 10 Jun 2020 17:25:05 -0400
+Subject: [PATCH 3/3] Detect local X start by socket creation if switching
+ users
+
+* A depriviledged X cannot send back a SIGUSR1 when it is ready
+---
+ src/x-server-local.c | 47 +++++++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 46 insertions(+), 1 deletion(-)
+
+diff --git a/src/x-server-local.c b/src/x-server-local.c
+index 761e1324..cee8a12e 100644
+--- a/src/x-server-local.c
+++ b/src/x-server-local.c
+@@ -64,6 +64,9 @@ struct XServerLocalPrivate
+     /* TRUE when received ready signal */
+     gboolean got_signal;
+ 
+    /* Poll source ID (fallback for signal if uids differ) */
+    guint poll_for_socket_source;
+
+     /* VT to run on */
+     gint vt;
+     gboolean have_vt_ref;
+@@ -370,6 +373,41 @@ got_signal_cb (Process *process, int signum, XServerLocal *server)
+     }
+ }
+ 
+static gboolean
+poll_for_socket_cb (XServerLocal *server) {
+
+  /* Check is X11 socket file exists as an alternative startup test to SIGUSR1 */
+  GStatBuf statbuf;
+  g_autofree gchar *socketpath = g_strdup_printf ("/tmp/.X11-unix/X%d", server->priv->display_number);
+
+  if ( g_stat (socketpath, &statbuf) == 0 ) {
+    uid_t uid = server->priv->user ? user_get_uid(server->priv->user) : 0;
+
+    /* It has to be a valid socket file */
+    if (!(statbuf.st_mode & S_IFSOCK))
+        l_debug (server, "X11 socket file is not a socket: %s", socketpath);
+
+    /* It has to be owned by the correct user */
+    else if (statbuf.st_uid != uid)
+        l_debug (server, "X11 socket file is not owned by uid %d: %s", uid, socketpath);
+
+    /* Consider SIGUSR1 to have been recieved */
+    else {
+        server->priv->got_signal = TRUE;
+        l_debug (server, "Detected valid X11 socket for X server :%d", server->priv->display_number);
+
+        // FIXME: Check return value
+        DISPLAY_SERVER_CLASS (x_server_local_parent_class)->start (DISPLAY_SERVER (server));
+    }
+
+    /* Cancel the timer as the file showed up */
+    return FALSE;
+  }
+
+  /* Wait another second and check again */
+  return TRUE;
+}
+
+ static void
+ stopped_cb (Process *process, XServerLocal *server)
+ {
+@@ -442,7 +480,10 @@ x_server_local_start (DisplayServer *display_server)
+     ProcessRunFunc run_cb = X_SERVER_LOCAL_GET_CLASS (server)->get_run_function (server);
+     server->priv->x_server_process = process_new (run_cb, server);
+     process_set_clear_environment (server->priv->x_server_process, TRUE);
+-    g_signal_connect (server->priv->x_server_process, PROCESS_SIGNAL_GOT_SIGNAL, G_CALLBACK (got_signal_cb), server);
+    if (server->priv->user == NULL || user_get_uid (server->priv->user) == getuid ())
+        g_signal_connect (server->priv->x_server_process, PROCESS_SIGNAL_GOT_SIGNAL, G_CALLBACK (got_signal_cb), server);
+    else if (!server->priv->poll_for_socket_source)
+        server->priv->poll_for_socket_source = g_timeout_add_seconds(1, (GSourceFunc)poll_for_socket_cb, server);
+     g_signal_connect (server->priv->x_server_process, PROCESS_SIGNAL_STOPPED, G_CALLBACK (stopped_cb), server);
+ 
+     /* Setup logging */
+@@ -566,6 +607,10 @@ x_server_local_finalize (GObject *object)
+ 
+     if (self->priv->x_server_process)
+         g_signal_handlers_disconnect_matched (self->priv->x_server_process, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self);
+    if (self->priv->poll_for_socket_source) {
+        g_source_remove (self->priv->poll_for_socket_source);
+        self->priv->poll_for_socket_source = 0;
+    }
+     g_clear_object (&self->priv->x_server_process);
+     g_clear_pointer (&self->priv->command, g_free);
+     g_clear_object (&self->priv->user);
+-- 
+2.25.4
+
--- a/lightdm.spec
+++ b/lightdm.spec
@@ -34,6 +34,8 @@ Patch0:		%{giturl}/pull/3.patch#/%{name}-1.25.1-fix_transition_plymouth.patch
 Patch1:		%{giturl}/pull/4.patch#/%{name}-1.25.1-fix_Autotools.patch
 Patch2:		%{giturl}/pull/5.patch#/%{name}-1.25.1-disable_dmrc.patch
 Patch3:		%{giturl}/pull/6.patch#/%{name}-1.25.1-remove_incorrect_use_of_const.patch
+Patch4:		0001-Add-ability-to-drop-priviledges-switch-user-when-for.patch
+Patch5:		0003-Detect-local-X-start-by-socket-creation-if-switching.patch

 BuildRequires:	gettext
 BuildRequires:	gnome-common