[Spice-devel] [PATCH v4] usbredir: fix redirection of user-accesible device nodes.

[Hans de Goede]

Hi,

On 20-07-15 11:51, Christophe Fergeau wrote:
> Hey,
>
> Looks good te me now.
> Hans, would you mind taking a quick look at that
> patch in case you have objections on the change (if
> spice_usb_acl_helper_open_acl_finish() fails, try to directly open the
> device node anyway as it may be user-accessible).

I do not think that this is the right thing todo, this means e.g.
that if policykit / the admin explicitly denies redirection, but
the usb device node happens to be opened up (which happens with
e.g. scanners), then we will still redirect, this seems wrong to me.

Instead Michal should fixup his policykit so that the helper works
for him.

Regards,

Hans


>
> Thanks,
>
> Christophe
>
>
> On Wed, Jul 15, 2015 at 10:07:37AM +0100, Michal Suchanek wrote:
>> When calling ACL helper fails also try to open the device node directly.
>>
>> Otherwise user-accessible device nodes are rejected when policykit
>> support is compiled in and policy is not set up when in fact the device
>> could be accessed.
>>
>> Signed-off-by: Michal Suchanek <hramrach at gmail.com>
>>
>> --
>>
>> v2:
>>
>>   - clear errors properly
>>
>> v4:
>>
>>   - handle connection cancellation
>> ---
>>   src/channel-usbredir.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
>> index 292b82f..c6f9e5e 100644
>> --- a/src/channel-usbredir.c
>> +++ b/src/channel-usbredir.c
>> @@ -276,13 +276,14 @@ static void spice_usbredir_channel_open_acl_cb(
>>       SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(user_data);
>>       SpiceUsbredirChannelPrivate *priv = channel->priv;
>>       GError *err = NULL;
>> +    GError *acl_err = NULL;
>>
>>       g_return_if_fail(acl_helper == priv->acl_helper);
>>       g_return_if_fail(priv->state == STATE_WAITING_FOR_ACL_HELPER ||
>>                        priv->state == STATE_DISCONNECTING);
>>
>> -    spice_usb_acl_helper_open_acl_finish(acl_helper, acl_res, &err);
>> -    if (!err && priv->state == STATE_DISCONNECTING) {
>> +    spice_usb_acl_helper_open_acl_finish(acl_helper, acl_res, &acl_err);
>> +    if (priv->state == STATE_DISCONNECTING) {
>>           err = g_error_new_literal(G_IO_ERROR, G_IO_ERROR_CANCELLED,
>>                                     "USB redirection channel connect cancelled");
>>       }
>> @@ -290,13 +291,21 @@ static void spice_usbredir_channel_open_acl_cb(
>>           spice_usbredir_channel_open_device(channel, &err);
>>       }
>>       if (err) {
>> -        g_simple_async_result_take_error(priv->result, err);
>> +        if (acl_err) {
>> +            g_simple_async_result_take_error(priv->result, acl_err);
>> +            acl_err = NULL;
>> +        } else {
>> +            g_simple_async_result_take_error(priv->result, err);
>> +            err = NULL;
>> +        }
>>           libusb_unref_device(priv->device);
>>           priv->device = NULL;
>>           g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
>>           priv->spice_device = NULL;
>>           priv->state  = STATE_DISCONNECTED;
>>       }
>> +    g_clear_error(&acl_err);
>> +    g_clear_error(&err);
>>
>>       spice_usb_acl_helper_close_acl(priv->acl_helper);
>>       g_clear_object(&priv->acl_helper);
>> --
>> 2.1.4
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel