Patchwork usb-redir: Add the posibility to filter out certain devices from redirecion

login
register
mail settings
Submitter Hans de Goede
Date Jan. 12, 2012, 4:31 p.m.
Message ID <1326385868-26538-1-git-send-email-hdegoede@redhat.com>
Download mbox | patch
Permalink /patch/135646/
State New
Headers show

Comments

Hans de Goede - Jan. 12, 2012, 4:31 p.m.
This patch adds the posibility to filter out certain devices from redirecion.
To use this pass the filter property to -device usb-redir.  The filter
property takes a string consisting of filter rules, the format for a rule is:
<class>:<vendor>:<product>:<version>:<allow>

-1 can be used to allow any value for a field.

Muliple rules can be concatonated using | as a separator. Note that if
a device matches none of the passed in rules, redirecting it will not be
allowed!

Example:
-device usb-redir,filter='-1:0x0781:0x5567:-1:0|0x08:-1:-1:-1:1'

This example will deny the Sandisk Cruzer Blade being redirected, as it
has a usb id of 0781:5567, it will allow any other usb mass storage devices,
and it will deny any other devices (the default for devices not matching any
of the rules.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 configure   |    2 +-
 usb-redir.c |  114 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 105 insertions(+), 11 deletions(-)
Jan Kiszka - Jan. 12, 2012, 4:50 p.m.
On 2012-01-12 17:31, Hans de Goede wrote:
> This patch adds the posibility to filter out certain devices from redirecion.
> To use this pass the filter property to -device usb-redir.  The filter
> property takes a string consisting of filter rules, the format for a rule is:
> <class>:<vendor>:<product>:<version>:<allow>
> 
> -1 can be used to allow any value for a field.
> 
> Muliple rules can be concatonated using | as a separator. Note that if
> a device matches none of the passed in rules, redirecting it will not be
> allowed!
> 

Hmm, '|' makes quoting mandatory. And the :<allow> is also not that
beautiful IMHO.

What about

[+]|-<class>:<vendor>:<product>:<version>[/[+]|-<class>:...]

? Optional '+' means allow, '-' disallow. An empty field could also
serve as wildcard (instead of explicit -1 which would collide with
disallow).

Jan
Gerd Hoffmann - Jan. 13, 2012, 7:43 a.m.
On 01/12/12 17:31, Hans de Goede wrote:
> This patch adds the posibility to filter out certain devices from redirecion.
> To use this pass the filter property to -device usb-redir.  The filter
> property takes a string consisting of filter rules, the format for a rule is:
> <class>:<vendor>:<product>:<version>:<allow>

Hmm, what is the point of having this filter in qemu?

I'd expect the other end of the network connection which talks to the
actual usb device (i.e. the tcp standalone server or spice client) doing
the filtering ...

cheers,
  Gerd
Gerd Hoffmann - Jan. 23, 2012, 2:51 p.m.
On 01/13/12 08:43, Gerd Hoffmann wrote:
> On 01/12/12 17:31, Hans de Goede wrote:
>> This patch adds the posibility to filter out certain devices from redirecion.
>> To use this pass the filter property to -device usb-redir.  The filter
>> property takes a string consisting of filter rules, the format for a rule is:
>> <class>:<vendor>:<product>:<version>:<allow>
> 
> Hmm, what is the point of having this filter in qemu?
> 
> I'd expect the other end of the network connection which talks to the
> actual usb device (i.e. the tcp standalone server or spice client) doing
> the filtering ...

Also fails checkpatch:

WARNING: line over 80 characters
#75: FILE: usb-redir.c:898:
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter", "a
usb device filter string");

total: 0 errors, 1 warnings, 208 lines checked

cheers,
  Gerd
Hans de Goede - Jan. 25, 2012, 8:50 a.m.
Hi,

On 01/23/2012 03:51 PM, Gerd Hoffmann wrote:
> On 01/13/12 08:43, Gerd Hoffmann wrote:
>> On 01/12/12 17:31, Hans de Goede wrote:
>>> This patch adds the posibility to filter out certain devices from redirecion.
>>> To use this pass the filter property to -device usb-redir.  The filter
>>> property takes a string consisting of filter rules, the format for a rule is:
>>> <class>:<vendor>:<product>:<version>:<allow>
>>
>> Hmm, what is the point of having this filter in qemu?
>>
>> I'd expect the other end of the network connection which talks to the
>> actual usb device (i.e. the tcp standalone server or spice client) doing
>> the filtering ...

The admin of the vm may want to allow only certain devices to be redirected
to the vm. And while he has administrative control over the vm, he does not
necessarily have administrative control over the client.

>
> Also fails checkpatch:
>
> WARNING: line over 80 characters
> #75: FILE: usb-redir.c:898:
> +            qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter", "a
> usb device filter string");

Oops, I'll send a new version.

Regards,

Hans

Patch

diff --git a/configure b/configure
index 7ecf44e..c7e37df 100755
--- a/configure
+++ b/configure
@@ -2541,7 +2541,7 @@  fi
 
 # check for usbredirparser for usb network redirection support
 if test "$usb_redir" != "no" ; then
-    if $pkg_config libusbredirparser >/dev/null 2>&1 ; then
+    if $pkg_config --atleast-version=0.3.3 libusbredirparser >/dev/null 2>&1 ; then
         usb_redir="yes"
         usb_redir_cflags=$($pkg_config --cflags libusbredirparser 2>/dev/null)
         usb_redir_libs=$($pkg_config --libs libusbredirparser 2>/dev/null)
diff --git a/usb-redir.c b/usb-redir.c
index 6e92f14..41c0745 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -34,6 +34,7 @@ 
 #include <sys/ioctl.h>
 #include <signal.h>
 #include <usbredirparser.h>
+#include <usbredirfilter.h>
 
 #include "hw/usb.h"
 
@@ -72,6 +73,7 @@  struct USBRedirDevice {
     /* Properties */
     CharDriverState *cs;
     uint8_t debug;
+    char *filter_str;
     /* Data passed from chardev the fd_read cb to the usbredirparser read cb */
     const uint8_t *read_buf;
     int read_buf_size;
@@ -84,6 +86,11 @@  struct USBRedirDevice {
     struct endp_data endpoint[MAX_ENDPOINTS];
     uint32_t packet_id;
     QTAILQ_HEAD(, AsyncURB) asyncq;
+    /* Data for device filtering */
+    struct usb_redir_device_connect_header device_info;
+    struct usb_redir_interface_info_header interface_info;
+    struct usbredirfilter_rule *filter_rules;
+    int filter_rules_count;
 };
 
 struct AsyncURB {
@@ -790,6 +797,7 @@  static int usbredir_handle_control(USBDevice *udev, USBPacket *p,
 static void usbredir_open_close_bh(void *opaque)
 {
     USBRedirDevice *dev = opaque;
+    uint32_t caps[USB_REDIR_CAPS_SIZE] = { 0, };
 
     usbredir_device_disconnect(dev);
 
@@ -820,7 +828,9 @@  static void usbredir_open_close_bh(void *opaque)
         dev->parser->interrupt_packet_func = usbredir_interrupt_packet;
         dev->read_buf = NULL;
         dev->read_buf_size = 0;
-        usbredirparser_init(dev->parser, VERSION, NULL, 0, 0);
+
+        usbredirparser_caps_set_cap(caps, usb_redir_cap_connect_device_version);
+        usbredirparser_init(dev->parser, VERSION, caps, USB_REDIR_CAPS_SIZE, 0);
         usbredirparser_do_write(dev->parser);
     }
 }
@@ -908,6 +918,16 @@  static int usbredir_initfn(USBDevice *udev)
         return -1;
     }
 
+    if (dev->filter_str) {
+        i = usbredirfilter_string_to_rules(dev->filter_str, ":", "|",
+                                           &dev->filter_rules,
+                                           &dev->filter_rules_count);
+        if (i) {
+            qerror_report(QERR_INVALID_PARAMETER_VALUE, "filter", "a usb device filter string");
+            return -1;
+        }
+    }
+
     dev->open_close_bh = qemu_bh_new(usbredir_open_close_bh, dev);
     dev->attach_timer = qemu_new_timer_ms(vm_clock, usbredir_do_attach, dev);
 
@@ -956,6 +976,44 @@  static void usbredir_handle_destroy(USBDevice *udev)
     if (dev->parser) {
         usbredirparser_destroy(dev->parser);
     }
+
+    free(dev->filter_rules);
+}
+
+static int usbredir_check_filter(USBRedirDevice *dev)
+{
+    if (dev->interface_info.interface_count == 0) {
+        ERROR("No interface info for device\n");
+        return -1;
+    }
+
+    if (dev->filter_rules) {
+        if (!usbredirparser_peer_has_cap(dev->parser,
+                                    usb_redir_cap_connect_device_version)) {
+            ERROR("Device filter specified and peer does not have the "
+                  "connect_device_version capability\n");
+            return -1;
+        }
+
+        if (usbredirfilter_check(
+                dev->filter_rules,
+                dev->filter_rules_count,
+                dev->device_info.device_class,
+                dev->device_info.device_subclass,
+                dev->device_info.device_protocol,
+                dev->interface_info.interface_class,
+                dev->interface_info.interface_subclass,
+                dev->interface_info.interface_protocol,
+                dev->interface_info.interface_count,
+                dev->device_info.vendor_id,
+                dev->device_info.product_id,
+                dev->device_info.device_version_bcd,
+                0) != 0) {
+            return -1;
+        }
+    }
+
+    return 0;
 }
 
 /*
@@ -984,6 +1042,7 @@  static void usbredir_device_connect(void *priv,
     struct usb_redir_device_connect_header *device_connect)
 {
     USBRedirDevice *dev = priv;
+    const char *speed;
 
     if (qemu_timer_pending(dev->attach_timer) || dev->dev.attached) {
         ERROR("Received device connect while already connected\n");
@@ -992,26 +1051,48 @@  static void usbredir_device_connect(void *priv,
 
     switch (device_connect->speed) {
     case usb_redir_speed_low:
-        DPRINTF("attaching low speed device\n");
+        speed = "low speed";
         dev->dev.speed = USB_SPEED_LOW;
         break;
     case usb_redir_speed_full:
-        DPRINTF("attaching full speed device\n");
+        speed = "full speed";
         dev->dev.speed = USB_SPEED_FULL;
         break;
     case usb_redir_speed_high:
-        DPRINTF("attaching high speed device\n");
+        speed = "high speed";
         dev->dev.speed = USB_SPEED_HIGH;
         break;
     case usb_redir_speed_super:
-        DPRINTF("attaching super speed device\n");
+        speed = "super speed";
         dev->dev.speed = USB_SPEED_SUPER;
         break;
     default:
-        DPRINTF("attaching unknown speed device, assuming full speed\n");
+        speed = "unknown speed";
         dev->dev.speed = USB_SPEED_FULL;
     }
+
+    if (usbredirparser_peer_has_cap(dev->parser,
+                                    usb_redir_cap_connect_device_version)) {
+        INFO("attaching %s device %04x:%04x version %d.%d class %02x\n",
+             speed, device_connect->vendor_id, device_connect->product_id,
+             device_connect->device_version_bcd >> 8,
+             device_connect->device_version_bcd & 0xff,
+             device_connect->device_class);
+    } else {
+        INFO("attaching %s device %04x:%04x class %02x\n", speed,
+             device_connect->vendor_id, device_connect->product_id,
+             device_connect->device_class);
+    }
+
     dev->dev.speedmask = (1 << dev->dev.speed);
+    dev->device_info = *device_connect;
+
+    if (usbredir_check_filter(dev)) {
+        WARNING("Device %04x:%04x rejected by device filter, not attaching\n",
+                device_connect->vendor_id, device_connect->product_id);
+        return;
+    }
+
     qemu_mod_timer(dev->attach_timer, dev->next_attach_time);
 }
 
@@ -1038,15 +1119,27 @@  static void usbredir_device_disconnect(void *priv)
     for (i = 0; i < MAX_ENDPOINTS; i++) {
         QTAILQ_INIT(&dev->endpoint[i].bufpq);
     }
+    dev->interface_info.interface_count = 0;
 }
 
 static void usbredir_interface_info(void *priv,
     struct usb_redir_interface_info_header *interface_info)
 {
-    /* The intention is to allow specifying acceptable interface classes
-       for redirection on the cmdline and in the future verify this here,
-       and disconnect (or never connect) the device if a not accepted
-       interface class is detected */
+    USBRedirDevice *dev = priv;
+
+    dev->interface_info = *interface_info;
+
+    /*
+     * If we receive interface info after the device has already been
+     * connected (ie on a set_config), re-check the filter.
+     */
+    if (qemu_timer_pending(dev->attach_timer) || dev->dev.attached) {
+        if (usbredir_check_filter(dev)) {
+            ERROR("Device no longer matches filter after interface info "
+                  "change, disconnecting!\n");
+            usbredir_device_disconnect(dev);
+        }
+    }
 }
 
 static void usbredir_ep_info(void *priv,
@@ -1356,6 +1449,7 @@  static struct USBDeviceInfo usbredir_dev_info = {
     .qdev.props     = (Property[]) {
         DEFINE_PROP_CHR("chardev", USBRedirDevice, cs),
         DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, 0),
+        DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str),
         DEFINE_PROP_END_OF_LIST(),
     },
 };