[Spice-devel] [PATCH spice-gtk 1/3] channel-usbredir: Call the acl helper without first trying to open the device

[Hans de Goede]

Normally opening the USB device without first calling the helper will fail,
except when the process using spice-gtk is running as root.

So the current code which first tries to open the USB device before calling
the helper is optimizing for an exception rather then for the default code
path. More over it also causes libusb to print the following errors to stderr:
libusb:error [op_open] libusb couldn't open USB device /dev/bus/usb/002/017: Permission denied.
libusb:error [op_open] libusb requires write access to USB device nodes.

So this patch changes things to first call the helper and only then try
to open the device node.

This patch also modifies the helper to not call policykit when called by
a root process, since the set_facl which it will do, if policykit says it
is ok, is a no-op for root anyways. Instead it directly returns a success
status without doing anything when called by a root process.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 gtk/channel-usbredir.c                 |   30 +++++++++++++++---------------
 gtk/spice-client-glib-usb-acl-helper.c |   25 +++++++++++++++++--------
 2 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
index 00647b6..fce4783 100644
--- a/gtk/channel-usbredir.c
+++ b/gtk/channel-usbredir.c
@@ -245,7 +245,6 @@ void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
 {
     SpiceUsbredirChannelPrivate *priv = channel->priv;
     GSimpleAsyncResult *result;
-    GError *err = NULL;
 
     g_return_if_fail(SPICE_IS_USBREDIR_CHANNEL(channel));
     g_return_if_fail(context != NULL);
@@ -265,26 +264,27 @@ void spice_usbredir_channel_connect_async(SpiceUsbredirChannel *channel,
 
     priv->context = g_object_ref(context);
     priv->device  = g_object_ref(device);
-    if (!spice_usbredir_channel_open_device(channel, &err)) {
 #if USE_POLKIT
-        priv->result = result;
-        priv->state = STATE_WAITING_FOR_ACL_HELPER;
-        priv->acl_helper = spice_usb_acl_helper_new();
-        g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
-                     "inhibit-keyboard-grab", TRUE, NULL);
-        spice_usb_acl_helper_open_acl(priv->acl_helper,
-                                      g_usb_device_get_bus(device),
-                                      g_usb_device_get_address(device),
-                                      cancellable,
-                                      spice_usbredir_channel_open_acl_cb,
-                                      channel);
-        return;
+    priv->result = result;
+    priv->state = STATE_WAITING_FOR_ACL_HELPER;
+    priv->acl_helper = spice_usb_acl_helper_new();
+    g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
+                 "inhibit-keyboard-grab", TRUE, NULL);
+    spice_usb_acl_helper_open_acl(priv->acl_helper,
+                                  g_usb_device_get_bus(device),
+                                  g_usb_device_get_address(device),
+                                  cancellable,
+                                  spice_usbredir_channel_open_acl_cb,
+                                  channel);
+    return;
 #else
+    GError *err = NULL;
+    if (!spice_usbredir_channel_open_device(channel, &err)) {
         g_simple_async_result_take_error(result, err);
         g_clear_object(&priv->context);
         g_clear_object(&priv->device);
-#endif
     }
+#endif
 
 done:
     g_simple_async_result_complete_in_idle(result);
diff --git a/gtk/spice-client-glib-usb-acl-helper.c b/gtk/spice-client-glib-usb-acl-helper.c
index 2e31bd3..698820b 100644
--- a/gtk/spice-client-glib-usb-acl-helper.c
+++ b/gtk/spice-client-glib-usb-acl-helper.c
@@ -229,14 +229,23 @@ static void stdin_read_complete(GObject *src, GAsyncResult *res, gpointer data)
             break;
         }
 
-        state = STATE_WAITING_FOR_POL_KIT;
-
-        polkit_cancellable = g_cancellable_new();
-        polkit_authority_check_authorization(
-            authority, subject, "org.spice-space.lowlevelusbaccess", NULL,
-            POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
-            polkit_cancellable, (GAsyncReadyCallback)check_authorization_cb,
-            loop);
+        /*
+         * The set_facl() call is a no-op for root, so no need to ask PolKit
+         * and then if ok call set_facl(), when called by a root process.
+         */
+        if (getuid() != 0) {
+            polkit_cancellable = g_cancellable_new();
+            polkit_authority_check_authorization(
+                authority, subject, "org.spice-space.lowlevelusbaccess", NULL,
+                POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
+                polkit_cancellable,
+                (GAsyncReadyCallback)check_authorization_cb, NULL);
+            state = STATE_WAITING_FOR_POL_KIT;
+        } else {
+            fprintf(stdout, "SUCCESS\n");
+            fflush(stdout);
+            state = STATE_WAITING_FOR_STDIN_EOF;
+        }
 
         g_data_input_stream_read_line_async(stdin_stream, G_PRIORITY_DEFAULT,
                                             NULL, stdin_read_complete, NULL);
-- 
1.7.7.4