All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Opasiak <k.opasiak-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
To: Alan Stern <stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org>
Cc: "Greg KH"
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	"Emilio López"
	<emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>,
	kborer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	reillyg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	jorgelo-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org,
	dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org
Subject: Re: [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers
Date: Mon, 30 Nov 2015 18:12:22 +0100	[thread overview]
Message-ID: <565C8376.6070505@samsung.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1511301113120.1938-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>



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.

Maybe better option would be to add optional argument to claim interface 
ioctl() and allow to claim interface for other fd than the current one? 
So "usb-manager" could have fd with full control and claim interfaces 
for apps which have fds with restricted privileges.

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Opasiak <k.opasiak@samsung.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Greg KH" <gregkh@linuxfoundation.org>,
	"Emilio López" <emilio.lopez@collabora.co.uk>,
	kborer@gmail.com, reillyg@chromium.org, keescook@chromium.org,
	linux-api@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, jorgelo@chromium.org,
	dan.carpenter@oracle.com
Subject: Re: [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers
Date: Mon, 30 Nov 2015 18:12:22 +0100	[thread overview]
Message-ID: <565C8376.6070505@samsung.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1511301113120.1938-100000@iolanthe.rowland.org>



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.

Maybe better option would be to add optional argument to claim interface 
ioctl() and allow to claim interface for other fd than the current one? 
So "usb-manager" could have fd with full control and claim interfaces 
for apps which have fds with restricted privileges.

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics

  parent reply	other threads:[~2015-11-30 17:12 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-25 15:45 [PATCH v1 0/1] ioctl to disallow detaching kernel USB drivers Emilio López
2015-11-25 15:45 ` Emilio López
2015-11-25 15:45 ` [PATCH v1] usb: devio: Add " Emilio López
2015-11-26  8:59   ` Peter Chen
2015-11-26  8:59     ` Peter Chen
2015-11-26  9:20     ` Dan Carpenter
     [not found] ` <1448466334-21346-1-git-send-email-emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2015-11-26  9:19   ` [PATCH v1 0/1] " Krzysztof Opasiak
2015-11-26  9:19     ` Krzysztof Opasiak
2015-11-26 17:29     ` Greg KH
     [not found]       ` <20151126172914.GA8671-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2015-11-27  8:44         ` Krzysztof Opasiak
2015-11-27  8:44           ` Krzysztof Opasiak
     [not found]           ` <565817FD.3090409-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2015-11-28  2:39             ` Greg KH
2015-11-28  2:39               ` Greg KH
     [not found]               ` <20151128023925.GA5177-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2015-11-30  9:08                 ` Oliver Neukum
2015-11-30  9:08                   ` Oliver Neukum
2015-11-30 16:16           ` Alan Stern
     [not found]             ` <Pine.LNX.4.44L0.1511301113120.1938-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2015-11-30 17:12               ` Krzysztof Opasiak [this message]
2015-11-30 17:12                 ` Krzysztof Opasiak
     [not found]                 ` <565C8376.6070505-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2015-11-30 17:20                   ` Greg KH
2015-11-30 17:20                     ` Greg KH
     [not found]                     ` <20151130172028.GA1088-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2015-11-30 18:48                       ` Krzysztof Opasiak
2015-11-30 18:48                         ` Krzysztof Opasiak
     [not found]                         ` <565C9A18.6000006-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-01-19 16:39                           ` Emilio López
2016-01-19 16:39                             ` Emilio López
     [not found]                             ` <569E66DF.3050004-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2016-01-19 18:07                               ` Greg KH
2016-01-19 18:07                                 ` Greg KH
     [not found]                                 ` <20160119180752.GA10487-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2016-01-21 23:54                                   ` [PATCH v2] usb: devio: Add " Emilio López
2016-01-21 23:54                                     ` Emilio López
     [not found]                                     ` <1453420476-26125-1-git-send-email-emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2016-01-22  9:41                                       ` Bjørn Mork
2016-01-22  9:41                                         ` Bjørn Mork
     [not found]                                         ` <8760ymdk94.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-01-25  1:40                                           ` Emilio López
2016-01-25  1:40                                             ` Emilio López
2016-01-25  8:39                                             ` Bjørn Mork
     [not found]                                               ` <87powqrr1s.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-01-25 15:21                                                 ` Alan Stern
2016-01-25 15:21                                                   ` Alan Stern
     [not found]                                                   ` <Pine.LNX.4.44L0.1601251016490.1849-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2016-01-25 15:32                                                     ` Bjørn Mork
2016-01-25 15:32                                                       ` Bjørn Mork
     [not found]                                                       ` <8737tln08x.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-01-25 15:46                                                         ` Alan Stern
2016-01-25 15:46                                                           ` Alan Stern
2016-01-22 16:10                                       ` Alan Stern
2016-01-22 16:10                                         ` Alan Stern
2016-01-25  2:01                                         ` Emilio López
2016-02-04  3:20                                       ` [PATCH v3] " Emilio López
2016-02-04  3:20                                         ` Emilio López
2016-02-04 16:27                                         ` Alan Stern
2016-02-04 16:27                                           ` Alan Stern
     [not found]                                           ` <Pine.LNX.4.44L0.1602041122340.1515-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2016-02-08  1:56                                             ` Emilio López
2016-02-08  1:56                                               ` Emilio López
     [not found]                                         ` <1454556057-18956-1-git-send-email-emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2016-02-04  3:46                                           ` Greg KH
2016-02-04  3:46                                             ` Greg KH
2016-02-15  1:41                                           ` [PATCH v4] " Emilio López
2016-02-15  1:41                                             ` Emilio López
     [not found]                                             ` <1455500516-21590-1-git-send-email-emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
2016-02-18 18:44                                               ` Alan Stern
2016-02-18 18:44                                                 ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=565C8376.6070505@samsung.com \
    --to=k.opasiak-sze3o3uu22jbdgjk7y7tuq@public.gmane.org \
    --cc=dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org \
    --cc=emilio.lopez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org \
    --cc=gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org \
    --cc=jorgelo-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=kborer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=reillyg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org \
    --cc=stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.