Re: [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers

["Emilio López" <emilio.lopez@collabora.co.uk>]

Hi,

I'm reviving this thread as there's no consensus so far, and I'd like to 
see this supported in some way in mainline.

El 30/11/15 a las 15:48, Krzysztof Opasiak escribió:
>
>
> On 11/30/2015 06:20 PM, Greg KH wrote:
>> On Mon, Nov 30, 2015 at 06:12:22PM +0100, Krzysztof Opasiak wrote:
>>>
>>>
>>> On 11/30/2015 05:16 PM, Alan Stern wrote:
>>>> On Fri, 27 Nov 2015, Krzysztof Opasiak wrote:
>>>>
>>>>>>> I run through your code and as far as I understand above is not
>>>>>>> exactly
>>>>>>> true. Your patch allows only to prevent userspace from accessing
>>>>>>> interfaces
>>>>>>> which has kernel drivers, there is no way to stop an application
>>>>>>> from taking
>>>>>>> control over all free interfaces.
>>>>>>>
>>>>>>> Let's say that your device has 3 interfaces. First of them has a
>>>>>>> kernel
>>>>>>> driver but second and third doesn't. You have 2 apps. One should
>>>>>>> communicate
>>>>>>> using second interface and another one third. But first app is
>>>>>>> malicious and
>>>>>>> it claims all free interfaces of received device (your patch
>>>>>>> doesn't prevent
>>>>>>> this). And when second app starts it is unable to do anything
>>>>>>> with the
>>>>>>> device because all interfaces are taken. How would you like to
>>>>>>> handle this?
>>>>>>
>>>>>> You can't, and why would you ever want to, as you can't tell what
>>>>>> an app
>>>>>> "should" or "should not" do.  If you really care about this, then
>>>>>> use a
>>>>>> LSM policy to prevent this.
>>>>>
>>>>> Well, an app can declare what it does and what it needs in it's
>>>>> manifest
>>>>> file (or some equivalent of this) and the platform should ensure that
>>>>> app can do only what it has declared.
>>>>>
>>>>> I would really like to use LSM policy in here but currently it is
>>>>> impossible as one device node represents whole device. Permissions
>>>>> (even
>>>>> those from LSM) are being checked only on open() not on each
>>>>> ioctl() so
>>>>> as far as I know there is nothing which prevents any owner of
>>>>> opened fd
>>>>> to claim all available (not taken by someone else) interfaces and LSM
>>>>> policy is unable to filter those calls (unless we add some LSM hooks
>>>>> over there).
>>>>
>>>> How about this approach?  Once a process has dropped its usbfs
>>>> privileges, it's not allowed to claim any interfaces (either explicitly
>>>> or implicitly).  Instead, it or some manager program must claim the
>>>> appropriate interfaces before dropping privileges.
>>>>
>>>
>>> I agree that restricting interface claiming only to privileged
>>> process is a
>>> good idea. Unfortunately this generates a problem when program needs
>>> more
>>> than one interface (like in cdc - data + control for example). We
>>> need to
>>> declare both of them in first call to "usb-manager" or reopen the dev
>>> node
>>> at second call and claim all interfaces claimed using this fd till
>>> now and
>>> claim one more and then drop privileges and send a new fd.

I talked with Reilly some time back and they proposed using an extra 
parameter in the ioctl. This parameter would be a mask that specifies 
which interfaces the unprivileged process is allowed to claim (but 
doesn't necessarily have to do so). Providing a mask of ~0 would retain 
the current patch behaviour, and allow existing programs using 
out-of-tree implementations to continue working with little changes.

Now, if this were to be used on a system that knew better, via an app 
manifest or similar, the manager process would generate a suitable mask 
based on the interfaces required by the unprivileged process and use it 
as a parameter when dropping privileges. This way, the unprivileged 
process would be unable to claim interfaces it is not supposed to claim, 
while allowing it the liberty to do as it wishes with the interfaces it 
is allowed to use.

Krzysztof et al, would you find such an interface suitable? If you think 
it could be a way forward I'll gladly write the code and send a new patch.

Cheers,
Emilio