Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Rajat Jain <rajatja@google.com>
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: Fri, 5 Jun 2020 10:02:29 +0200
Message-ID: <20200605080229.GC2209311@kroah.com> (raw)
In-Reply-To: <CACK8Z6EOGduHX1m7eyhFgsGV7CYiVN0en4U0cM4BEWJwk2bmoA@mail.gmail.com>

On Thu, Jun 04, 2020 at 12:38:18PM -0700, Rajat Jain wrote:
> 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.

Not true, you can do that today with the explicit binding/unbinding of
devices to drivers in userspace.  Been there for many decades :)

But, think this through, since when do you have _multiple_ drivers that
have support to control the same type of device?  We almost never allow
that in the kernel today as that way lies madness (no heiarchy of
drivers to bind to what devices and so on.)

We always strive to keep a one-to-one mapping of "this device is only
allowed to be controlled by this one driver" today, why would you want
to change that basic premise now?

> 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.

Since when do different drivers control the same type of USB device?  :)

> 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.

Note the wireless USB code should now be gone from the tree.  If you see
any remants of it floating around, let me know and I will remove them, I
think there might be a few bits left that I missed.

> 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.

Nope, don't go down this path.  We tried to do this for USB where the
BIOS tells us that a device is "internal" vs. "external" but in reality,
BIOSes get this wrong and it's not always all that useful.

And why would you somehow "trust" a device that is in your system more
than one is not?  The same driver binds to it no matter what (as I state
above), so you should be able to trust it the same.

> 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).

Again, nope, no difference, see above.

> 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.

Ick, no, again, work on a per-device authorized setting.  That way it
works the same all across the system.  Don't get stuck in a "external
vs. internal" discussion as this will get messy really quickly (think
about "internal" devices with "external" links to them like PCI
"drawers" of devices that we currently support on large systems.  Or
things like thunderbolt hubs with "internal" devices like I have on my
desktop right now.

In summary, if a driver is "trusted enough" to control an internal
device, it should be "trusted enough" to control an external device.  If
not, then fix that driver so that you do "trust" it.

thanks,

greg k-h

  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-01 23:07 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
2020-06-05  8:02                         ` Greg Kroah-Hartman [this message]
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=20200605080229.GC2209311@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --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=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=rajatja@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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git