[Spice-devel] [PATCH 2/5] xspice: chown both files used by vdagent for suid Xorg case

[Alon Levy]

When running a suid Xorg mkfifo and bind produce files owned by root.
Change the ownership so that the user launched vdagent & vdagentd can
write to them. This also makes it easier to cleanup the files from the
Xspice process that is not running as root (and is launching both
vdagent & vdagentd, hence they are not running as root either).

The patch adds two new parameters, uid and gid of the agent used files,
namely the virtio unix domain socket and the uinput fifo.
---
 scripts/Xspice         |  5 ++++-
 src/Makefile.am        |  2 ++
 src/qxl.h              |  2 ++
 src/qxl_driver.c       |  4 ++++
 src/spiceqxl_uinput.c  |  2 ++
 src/spiceqxl_util.c    | 22 ++++++++++++++++++++++
 src/spiceqxl_util.h    |  8 ++++++++
 src/spiceqxl_vdagent.c |  2 ++
 8 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 src/spiceqxl_util.c
 create mode 100644 src/spiceqxl_util.h

diff --git a/scripts/Xspice b/scripts/Xspice
index 17439a1..34a5fcc 100755
--- a/scripts/Xspice
+++ b/scripts/Xspice
@@ -91,6 +91,8 @@ parser.add_argument('--vdagent-uinput-path', default='/tmp/xspice-uinput', help=
 parser.add_argument('--vdagentd-exec', default='spice-vdagentd')
 parser.add_argument('--vdagent-exec', default='spice-vdagent')
 parser.add_argument('--vdagent-no-launch', default=True, action='store_false', dest='vdagent_launch')
+parser.add_argument('--vdagent-uid', default=str(os.getuid()))
+parser.add_argument('--vdagent-gid', default=str(os.getgid()))
 parser.add_argument('--audio-fifo-dir', default='')
 
 #TODO
@@ -231,7 +233,8 @@ var_args = ['port', 'tls_port', 'disable_ticketing',
     'tls_ciphers', 'dh_file', 'password', 'image_compression',
     'jpeg_wan_compression', 'zlib_glz_wan_compression',
     'streaming_video', 'deferred_fps', 'exit_on_disconnect',
-    'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path']
+    'vdagent_enabled', 'vdagent_virtio_path', 'vdagent_uinput_path',
+    'vdagent_uid', 'vdagent_gid']
 
 for arg in var_args:
     if getattr(args, arg):
diff --git a/src/Makefile.am b/src/Makefile.am
index 4c4ae40..bf50ae1 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -79,6 +79,8 @@ spiceqxl_drv_la_SOURCES =				\
 	qxl.h					\
 	qxl_option_helpers.c			\
 	qxl_option_helpers.h			\
+	spiceqxl_util.h					\
+	spiceqxl_util.c					\
 	spiceqxl_spice_server.c			\
 	spiceqxl_spice_server.h			\
 	spiceqxl_io_port.c			\
diff --git a/src/qxl.h b/src/qxl.h
index c699c58..a44875b 100644
--- a/src/qxl.h
+++ b/src/qxl.h
@@ -147,6 +147,8 @@ enum {
     OPTION_SPICE_VDAGENT_ENABLED,
     OPTION_SPICE_VDAGENT_VIRTIO_PATH,
     OPTION_SPICE_VDAGENT_UINPUT_PATH,
+    OPTION_SPICE_VDAGENT_UID,
+    OPTION_SPICE_VDAGENT_GID,
 #endif
     OPTION_COUNT,
 };
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 91ba6c2..29b1551 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -142,6 +142,10 @@ const OptionInfoRec DefaultOptions[] =
       "SpiceVdagentVirtioPath",   OPTV_STRING,    {.str = spice_vdagent_virtio_path_default}, FALSE},
     { OPTION_SPICE_VDAGENT_UINPUT_PATH,
       "SpiceVdagentUinputPath",   OPTV_STRING,    {.str = spice_vdagent_uinput_path_default}, FALSE},
+    { OPTION_SPICE_VDAGENT_UID,
+      "SpiceVdagentUid",          OPTV_INTEGER,    {0}, FALSE},
+    { OPTION_SPICE_VDAGENT_GID,
+      "SpiceVdagentGid",          OPTV_INTEGER,    {0}, FALSE},
 #endif
     
     { -1, NULL, OPTV_NONE, {0}, FALSE }
diff --git a/src/spiceqxl_uinput.c b/src/spiceqxl_uinput.c
index 443f931..1e61907 100644
--- a/src/spiceqxl_uinput.c
+++ b/src/spiceqxl_uinput.c
@@ -13,6 +13,7 @@
 #include <linux/uinput.h>
 
 #include "qxl_option_helpers.h"
+#include "spiceqxl_util.h"
 #include "spiceqxl_inputs.h"
 
 #include "spiceqxl_uinput.h"
@@ -113,6 +114,7 @@ void spiceqxl_uinput_init(qxl_screen_t *qxl)
                 uinput_filename, strerror(errno));
         return;
     }
+    spiceqxl_chown_agent_file(qxl, uinput_filename);
     uinput_fd = open(uinput_filename, O_RDONLY | O_NONBLOCK, 0666);
     if (uinput_fd == -1) {
         fprintf(stderr, "spice: failed creating uinput file %s: %s\n",
diff --git a/src/spiceqxl_util.c b/src/spiceqxl_util.c
new file mode 100644
index 0000000..49cd5ab
--- /dev/null
+++ b/src/spiceqxl_util.c
@@ -0,0 +1,22 @@
+#include "config.h"
+
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+
+#include "qxl_option_helpers.h"
+#include "spiceqxl_util.h"
+
+void spiceqxl_chown_agent_file(qxl_screen_t *qxl, const char *filename)
+{
+    int uid, gid;
+
+    uid = get_int_option(qxl->options, OPTION_SPICE_VDAGENT_UID, "XSPICE_VDAGENT_UID");
+    gid = get_int_option(qxl->options, OPTION_SPICE_VDAGENT_GID, "XSPICE_VDAGENT_GID");
+    if (uid && gid) {
+        if (chown(filename, uid, gid) != 0) {
+            fprintf(stderr, "spice: failed to chain ownership of '%s' to %d/%d: %s\n",
+                    filename, uid, gid, strerror(errno));
+        }
+    }
+}
diff --git a/src/spiceqxl_util.h b/src/spiceqxl_util.h
new file mode 100644
index 0000000..4726bc1
--- /dev/null
+++ b/src/spiceqxl_util.h
@@ -0,0 +1,8 @@
+#ifndef SPICEQXL_UTIL_H
+#define SPICEQXL_UTIL_H
+
+#include "qxl.h"
+
+void spiceqxl_chown_agent_file(qxl_screen_t *qxl, const char *filename);
+
+#endif
diff --git a/src/spiceqxl_vdagent.c b/src/spiceqxl_vdagent.c
index fdeddd3..ba04cb8 100644
--- a/src/spiceqxl_vdagent.c
+++ b/src/spiceqxl_vdagent.c
@@ -10,6 +10,7 @@
 
 #include "qxl_option_helpers.h"
 
+#include "spiceqxl_util.h"
 #include "spiceqxl_uinput.h"
 #include "spiceqxl_vdagent.h"
 
@@ -156,6 +157,7 @@ void spiceqxl_vdagent_init(qxl_screen_t *qxl)
                 vdagent_virtio_filename, strerror(errno));
         return;
     }
+    spiceqxl_chown_agent_file(qxl, vdagent_virtio_filename);
     c = listen(virtio_fd, 1);
     if (c != 0) {
         fprintf(stderr, "error listening to unix domain socket: %s\n", strerror(errno));
-- 
1.8.3.1