Re: [RFC v2] USB: core: add a way to revoke access to open USB devices

[Alan Stern <stern@rowland.harvard.edu>]

On Thu, Aug 04, 2022 at 06:03:04PM +0200, Bastien Nocera wrote:
> Hey,
> 
> This is a follow-up to "[RFC v1] USB: core: add USBDEVFS_REVOKE ioctl" from April.
> 
> The end-goal is being able to cut-off access to USB devices for
> applications/programs that open raw USB devices using, in most cases,
> libusb.
> 
> I've implemented this using BPF, so we don't need to add new ioctls.

I'm not sure that's a good reason for using BPF.

> The presence or absence of that feature can be evaluated at runtime,
> and can be used to implement revoke support on session switching,
> in logind for example:
> https://github.com/hadess/usb-revoke-userspace/

This isn't clear.  Are you talking about having the kernel automatically 
do this when certain conditions are met?  Or do you mean that logind 
could invoke the BPF program at the appropriate times?

Left unsaid here is that logind generally won't have a file descriptor 
for any of the open USB devices that it wants to revoke access to.  
That's a good reason to use BPF instead of ioctl.

If you're going to revoke access to devices upon session switching, 
shouldn't the mechanism be more general?  I mean, shouldn't it apply to 
all sorts of devices, not just those that happen to be USB?

Also, if you're going to use session switching as your criterion for 
revoking access to USB devices then what will you do to restore access 
when the session switches back?

> I have some notes and questions on the API as it is exposed.
> 
> - I didn't see a point in having multiple kernel functions as entry
>   points as I was going to use a single BPF program as an entry point,
>   which can check the arguments. Can I rely on the BPF program checking
>   those arguments, or should I re-check them again in the kernel
>   implementation?

What is there to check?

> - Are my UID checks correct if I expect revoking to be called outside
>   namespaces, on the effective UID of the user. Is there a way to assert
>   that?

We're probably not the right people to ask.  You could try Eric 
Biederman.

> - Is there a good "errno" error for ENOSUCHUSER for using in the
>   usb_revoke() loop if we couldn't find any USB device matching the
>   requested user?

Why do you want that to be an error?  If you tell the kernel "Remove all 
access to USB devices for user X", why should it be an error if X doesn't 
have any open file descriptors for USB devices?

For that matter, are you certain that basing this on the UID is the right 
way to go?  What if there are two different login sessions both using the 
same UID?

Alan Stern

> 
> Cheers
> 
> Bastien Nocera (2):
>   USB: core: add a way to revoke access to open USB devices
>   usb: Implement usb_revoke() BPF function
> 
>  drivers/usb/core/devio.c | 105 ++++++++++++++++++++++++++++++++++++---
>  drivers/usb/core/usb.c   |  51 +++++++++++++++++++
>  drivers/usb/core/usb.h   |   8 +++
>  3 files changed, 158 insertions(+), 6 deletions(-)
> 
> -- 
> 2.36.1
>