All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rajat Jain <rajatja@google.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Rajat Jain <rajatxjain@gmail.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	"Raj, Ashok" <ashok.raj@intel.com>,
	"Krishnakumar,
	Lalithambika" <lalithambika.krishnakumar@intel.com>,
	Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Mika Westerberg <mika.westerberg@linux.intel.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Prashant Malani <pmalani@google.com>,
	Benson Leung <bleung@google.com>, Todd Broch <tbroch@google.com>,
	Alex Levin <levinale@google.com>,
	Mattias Nissler <mnissler@google.com>,
	Zubin Mithra <zsm@google.com>,
	Bernie Keany <bernie.keany@intel.com>,
	Aaron Durbin <adurbin@google.com>,
	Diego Rivas <diegorivas@google.com>,
	Duncan Laurie <dlaurie@google.com>,
	Furquan Shaikh <furquan@google.com>,
	Jesse Barnes <jsbarnes@google.com>,
	Christian Kellner <christian@kellner.me>,
	Alex Williamson <alex.williamson@redhat.com>,
	Joerg Roedel <joro@8bytes.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers
Date: Thu, 4 Jun 2020 12:38:18 -0700	[thread overview]
Message-ID: <CACK8Z6EOGduHX1m7eyhFgsGV7CYiVN0en4U0cM4BEWJwk2bmoA@mail.gmail.com> (raw)
In-Reply-To: <20200603121613.GA1488883@kroah.com>

Hello,

I spent some more thoughts into this...

On Wed, Jun 3, 2020 at 5:16 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 03, 2020 at 04:51:18AM -0700, Rajat Jain wrote:
> > Hello,
> >
> > >
> > > > Thanks for the pointer! I'm still looking at the details yet, but a
> > > > quick look (usb_dev_authorized()) seems to suggest that this API is
> > > > "device based". The multiple levels of "authorized" seem to take shape
> > > > from either how it is wired or from userspace choice. Once authorized,
> > > > USB device or interface is authorized to be used by *anyone* (can be
> > > > attached to any drivers). Do I understand it right that it does not
> > > > differentiate between drivers?
> > >
> > > Yes, and that is what you should do, don't fixate on drivers.  Users
> > > know how to control and manage devices.  Us kernel developers are
> > > responsible for writing solid drivers and getting them merged into the
> > > kernel tree and maintaining them over time.  Drivers in the kernel
> > > should always be trusted, ...
> >
> > 1) Yes, I agree that this would be ideal, and this should be our
> > mission. I should clarify that I may have used the wrong term
> > "Trusted/Certified drivers". I didn't really mean that the drivers may
> > be malicious by intent. What I really meant is that a driver may have
> > an attack surface, which is a vulnerability that may be exploited.
>
> Any code has such a thing, proving otherwise is a tough problem :)
>
> > Realistically speaking, finding vulnerabilities in drivers, creating
> > attacks to exploit them, and fixing them is a never ending cat and
> > mouse game. At Least "identifying the vulnerabilities" part is better
> > performed by security folks rather than driver writers.
>
> Are you sure about that?  It's hard to prove a negative :)
>
> > Earlier in the
> > thread I had mentioned certain studies/projects that identified and
> > exploited such vulnerabilities in the drivers. I should have used the
> > term "Vetted Drivers" maybe to convey the intent better - drivers that
> > have been vetted by a security focussed team (admin). What I'm
> > advocating here is an administrator's right to control the drivers
> > that he wants to allow for external ports on his systems.
>
> That's an odd thing, but sure, if you want to write up such a policy for
> your systems, great.  But that policy does not belong in the kernel, it
> belongs in userspace.
>
> > 2) In addition to the problem of driver negligences / vulnerabilities
> > to be exploited, we ran into another problem with the "whitelist
> > devices only" approach. We did start with the "device based" approach
> > only initially - but quickly realized that anything we use to
> > whitelist an external device can only be based on the info provided by
> > *that device* itself. So until we have devices that exchange
> > certificates with kernel [1], it is easy for a malicious device to
> > spoof a whitelisted device (by presenting the same VID:DID or any
> > other data that is used by us to whitelist it).
> >
> > [1] https://www.intel.com/content/www/us/en/io/pci-express/pcie-device-security-enhancements-spec.html
> >
> > I hope that helps somewhat clarify how / why we reached here?
>
> Kind of, I still think all you need to do is worry about controling the
> devices and if a driver should bind to it or not.  Again, much like USB
> has been doing for a very long time now.  The idea of "spoofing" ids
> also is not new, and has been around for a very long time as well, and
> again, the controls that the USB core gives you allows you to make any
> type of policy decision you want to, in userspace.

Er, *currently* it doesn't allow the userspace to make the particular
policy I want to, right? Specifically, today an administrator can not
control which USB *drivers* he wants to allow on an *external* USB
port. He can only control which USB devices he wants to authorize, but
once authorized, they are free to bind to any of the USB drivers. So
if I want to allow the administrator to implement a policy that allows
him to control the drivers for external ports, we'll need to enhance
the current code (whether we want to do it specific to a bus, or more
generically in the driver core). Are we on the same page?

To implement the policy that I want to in the driver core, what is
missing today in driver core is a distinction between "internal" and
"external" devices. Some buses have this knowledge locally today (PCI
has "untrusted" flag which can be used, USB uses hcd->wireless and
hub->port->connect_type) but it is not shared with the core.

So just to make sure if I'm thinking in the right direction, this is
what I'm thinking:

1) The device core needs a notion of internal vs external devices (a
flag) - a knowledge that needs to be filled in by the bus as it
discovers the device.

2) The driver core needs to allow an admin to provide a whitelist of
drivers for external devices. (Via Command line or a driver flag.
Default = everything is whitelisted).

3) While matching a driver to a device, the driver core needs to
impose the whitelist if the device is external, and if the
administrator has provided a whitelist.

Any bus that wants to use this can use it if it wants to, for external devices.

Thoughts?

Thanks & Best Regards,

Rajat

>
> So please, in summary:
>         - don't think this is some new type of thing, it's an old issue
>           transferred to yet-another-hardware-bus.  Not to say this is
>           not important, just please look at the work that others have
>           done in the past to help mitigate and solve this (reading the
>           Wireless USB spec should help you out here too, as they
>           detailed all of this.)
>         - do copy what USB did, by moving that logic into the driver
>           core so that all busses who want to take advantage of this
>           type of functionality, easily can do so.
>
> thanks,
>
> greg k-h

  parent reply	other threads:[~2020-06-04 19:39 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 23:07 [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers Rajat Jain
2020-05-04 11:47 ` Jean-Philippe Brucker
2020-05-04 11:59   ` Jean-Philippe Brucker
2020-05-04 19:17     ` Rajat Jain
2020-05-05 12:33 ` Mika Westerberg
2020-05-06 18:51   ` Rajat Jain
2020-05-11 20:31 ` Rajat Jain
2020-05-13 15:19 ` Bjorn Helgaas
2020-05-13 21:26   ` Rajat Jain
2020-05-14 13:42     ` Mika Westerberg
2020-05-14 19:12     ` Raj, Ashok
2020-05-15  2:18       ` Rajat Jain
2020-05-26 16:30         ` Rajat Jain
2020-06-01 23:25           ` Bjorn Helgaas
2020-06-02  5:06             ` Greg Kroah-Hartman
2020-06-03  2:27               ` Rajat Jain
2020-06-03  6:07                 ` Greg Kroah-Hartman
2020-06-03 11:51                   ` Rajat Jain
2020-06-03 12:16                     ` Greg Kroah-Hartman
2020-06-03 12:57                       ` Rajat Jain
2020-06-03 13:29                         ` Greg Kroah-Hartman
2020-06-04 19:38                       ` Rajat Jain [this message]
2020-06-05  8:02                         ` Greg Kroah-Hartman
2020-06-06  1:08                           ` Rajat Jain
2020-06-07 11:36                             ` Greg Kroah-Hartman
2020-06-08 17:03                               ` Jesse Barnes
2020-06-08 17:50                                 ` Greg Kroah-Hartman
2020-06-08 18:29                                   ` Jesse Barnes
2020-06-08 18:41                                     ` Rajat Jain
2020-06-09  9:54                                       ` Greg Kroah-Hartman
2020-06-30 21:46                                         ` Pavel Machek
2020-06-09  5:57                                     ` Greg Kroah-Hartman
2020-06-30 21:45                                 ` Pavel Machek
2020-07-01  6:54                                   ` Greg Kroah-Hartman
2020-07-01  8:47                                     ` Pavel Machek
2020-07-01 10:57                                       ` Greg Kroah-Hartman
2020-07-01 11:08                                         ` Pavel Machek
2020-06-09 21:04                               ` Bjorn Helgaas
2020-06-09 23:23                                 ` Rajat Jain
2020-06-10  0:04                                   ` Bjorn Helgaas
2020-06-10  0:30                                     ` Rajat Jain
2020-06-10 20:17                                       ` Rajat Jain
2020-06-10 23:09                                         ` Bjorn Helgaas
2020-06-10 23:01                                       ` Bjorn Helgaas
2020-06-10 23:46                                         ` Rajat Jain
2020-06-10  7:13                                   ` Greg Kroah-Hartman
2020-06-10  1:34                                 ` Oliver O'Halloran
2020-06-10 19:57                                   ` Rajat Jain
2020-06-16  1:24                                     ` Rajat Jain
2020-06-10  7:12                                 ` Greg Kroah-Hartman
2020-05-15 12:44     ` Joerg Roedel

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=CACK8Z6EOGduHX1m7eyhFgsGV7CYiVN0en4U0cM4BEWJwk2bmoA@mail.gmail.com \
    --to=rajatja@google.com \
    --cc=adurbin@google.com \
    --cc=alex.williamson@redhat.com \
    --cc=ashok.raj@intel.com \
    --cc=bernie.keany@intel.com \
    --cc=bhelgaas@google.com \
    --cc=bleung@google.com \
    --cc=christian@kellner.me \
    --cc=diegorivas@google.com \
    --cc=dlaurie@google.com \
    --cc=furquan@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=helgaas@kernel.org \
    --cc=jean-philippe@linaro.org \
    --cc=joro@8bytes.org \
    --cc=jsbarnes@google.com \
    --cc=lalithambika.krishnakumar@intel.com \
    --cc=levinale@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=mnissler@google.com \
    --cc=pmalani@google.com \
    --cc=rajatxjain@gmail.com \
    --cc=tbroch@google.com \
    --cc=zsm@google.com \
    /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.