Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-07-01 Thread Pavel Machek
Hi!

> > > Yes, it originally was designed that way, but again, the world has
> > > changed so we have to change with it.  That is why USB has for a long
> > > time now, allowed you to not bind drivers to devices that you do not
> > > "trust", and that trust can be determined by userspace.  That all came
> > > about thanks to the work done by the wireless USB spec people and kernel
> > > authors, which showed that maybe you just don't want to trust any device
> > > that comes within range of your system :)
> > 
> > Again, not disagreeing; but note the scale here.
> > 
> > It is mandatory to defend against malicious wireless USB devices.
> 
> Turns out there are no more wireless USB devices in the world, and the
> code for that is gone from Linux :)
> 
> > We probably should work on robustness against malicious USB devices.
> 
> We are, and do have, that support today.
> 
> > Malicious PCI-express devices are lot less of concern.
> 
> Not really, they are a lot of concern to some people.  Valid attacks are
> out there today, see the thunderbolt attacks that numerous people have
> done and published recently and for many years.

In this case PCI-express meant internal cards in PCs. Yes, thunderbolt
would be higher concern than internal card.

> > Defending against malicious CPU/RAM does not make much sense.
> 
> That's what the spectre and rowhammer fixes have been for :)

Yeah, and that's why we have whitelist of working CPUs and only work
on those, riiight? :-). [There's difference between "malicious" and
"buggy".]

Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-07-01 Thread Greg Kroah-Hartman
On Wed, Jul 01, 2020 at 10:47:50AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > We normally trust the hardware NOT to be malicious. (Because if hacker
> > > has physical access to hardware and lot of resources, you lost).
> > 
> > That is what we originally thought, however the world has changed and we
> > need to be better about this, now that it is trivial to create a "bad"
> > device.
> 
> I'm not disagreeing.
> 
> > > This is still true today, but maybe trusting USB devices is bad idea,
> > > so drivers are being cleaned up. PCI drivers will be WORSE in this
> > > regard. And you can't really protect against malicious CPU, and it is
> > > very very hard to protect against malicous RAM (probably not practical
> > > without explicit CPU support).
> > > 
> > > Linux was designed with "don't let hackers near your hardware" threat
> > > model in mind.
> > 
> > Yes, it originally was designed that way, but again, the world has
> > changed so we have to change with it.  That is why USB has for a long
> > time now, allowed you to not bind drivers to devices that you do not
> > "trust", and that trust can be determined by userspace.  That all came
> > about thanks to the work done by the wireless USB spec people and kernel
> > authors, which showed that maybe you just don't want to trust any device
> > that comes within range of your system :)
> 
> Again, not disagreeing; but note the scale here.
> 
> It is mandatory to defend against malicious wireless USB devices.

Turns out there are no more wireless USB devices in the world, and the
code for that is gone from Linux :)

> We probably should work on robustness against malicious USB devices.

We are, and do have, that support today.

> Malicious PCI-express devices are lot less of concern.

Not really, they are a lot of concern to some people.  Valid attacks are
out there today, see the thunderbolt attacks that numerous people have
done and published recently and for many years.

> Defending against malicious CPU/RAM does not make much sense.

That's what the spectre and rowhammer fixes have been for :)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-07-01 Thread Pavel Machek
Hi!

> > We normally trust the hardware NOT to be malicious. (Because if hacker
> > has physical access to hardware and lot of resources, you lost).
> 
> That is what we originally thought, however the world has changed and we
> need to be better about this, now that it is trivial to create a "bad"
> device.

I'm not disagreeing.

> > This is still true today, but maybe trusting USB devices is bad idea,
> > so drivers are being cleaned up. PCI drivers will be WORSE in this
> > regard. And you can't really protect against malicious CPU, and it is
> > very very hard to protect against malicous RAM (probably not practical
> > without explicit CPU support).
> > 
> > Linux was designed with "don't let hackers near your hardware" threat
> > model in mind.
> 
> Yes, it originally was designed that way, but again, the world has
> changed so we have to change with it.  That is why USB has for a long
> time now, allowed you to not bind drivers to devices that you do not
> "trust", and that trust can be determined by userspace.  That all came
> about thanks to the work done by the wireless USB spec people and kernel
> authors, which showed that maybe you just don't want to trust any device
> that comes within range of your system :)

Again, not disagreeing; but note the scale here.

It is mandatory to defend against malicious wireless USB devices.

We probably should work on robustness against malicious USB devices.

Malicious PCI-express devices are lot less of concern.

Defending against malicious CPU/RAM does not make much sense.

Notice that it is quite easy to generate -100V on the USB and kill
your motherboard. Also notice that malicious parts of the hardware
don't need to be electrically connected to the rest of system, and
that they don't even have to contain any electronics. You just have to
be careful. https://en.wikipedia.org/wiki/The_Thing_(listening_device)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-07-01 Thread Greg Kroah-Hartman
On Tue, Jun 30, 2020 at 11:45:59PM +0200, Pavel Machek wrote:
> Hi!
> 
> > Yes such drivers should be fixed, no doubt.  But without lots of
> > fuzzing (we're working on this) and testing we'd like to avoid
> > exposing that attack surface at all.
> > 
> > I think your suggestion to disable driver binding once the initial
> > bus/slot devices have been bound will probably work for this
> > situation.  I just wanted to be clear that without some auditing,
> > fuzzing, and additional testing, we simply have to assume that drivers
> > are *not* secure and avoid using them on untrusted devices until we're
> > fairly confident they can handle them (whether just misbehaving or
> > malicious), in combination with other approaches like IOMMUs of
> > course.  And this isn't because we don't trust driver authors or
> > kernel developers to dtrt, it's just that for many devices (maybe USB
> > is an exception) I think driver authors haven't had to consider this
> > case much, and so I think it's prudent to expect bugs in this area
> > that we need to find & fix.
> 
> We normally trust the hardware NOT to be malicious. (Because if hacker
> has physical access to hardware and lot of resources, you lost).

That is what we originally thought, however the world has changed and we
need to be better about this, now that it is trivial to create a "bad"
device.

> This is still true today, but maybe trusting USB devices is bad idea,
> so drivers are being cleaned up. PCI drivers will be WORSE in this
> regard. And you can't really protect against malicious CPU, and it is
> very very hard to protect against malicous RAM (probably not practical
> without explicit CPU support).
> 
> Linux was designed with "don't let hackers near your hardware" threat
> model in mind.

Yes, it originally was designed that way, but again, the world has
changed so we have to change with it.  That is why USB has for a long
time now, allowed you to not bind drivers to devices that you do not
"trust", and that trust can be determined by userspace.  That all came
about thanks to the work done by the wireless USB spec people and kernel
authors, which showed that maybe you just don't want to trust any device
that comes within range of your system :)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-30 Thread Pavel Machek
Hi!

> Yes such drivers should be fixed, no doubt.  But without lots of
> fuzzing (we're working on this) and testing we'd like to avoid
> exposing that attack surface at all.
> 
> I think your suggestion to disable driver binding once the initial
> bus/slot devices have been bound will probably work for this
> situation.  I just wanted to be clear that without some auditing,
> fuzzing, and additional testing, we simply have to assume that drivers
> are *not* secure and avoid using them on untrusted devices until we're
> fairly confident they can handle them (whether just misbehaving or
> malicious), in combination with other approaches like IOMMUs of
> course.  And this isn't because we don't trust driver authors or
> kernel developers to dtrt, it's just that for many devices (maybe USB
> is an exception) I think driver authors haven't had to consider this
> case much, and so I think it's prudent to expect bugs in this area
> that we need to find & fix.

We normally trust the hardware NOT to be malicious. (Because if hacker
has physical access to hardware and lot of resources, you lost).

This is still true today, but maybe trusting USB devices is bad idea,
so drivers are being cleaned up. PCI drivers will be WORSE in this
regard. And you can't really protect against malicious CPU, and it is
very very hard to protect against malicous RAM (probably not practical
without explicit CPU support).

Linux was designed with "don't let hackers near your hardware" threat
model in mind.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: PGP signature


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-30 Thread Pavel Machek
Hi!

> > > I think that's inherently platform specific to some extent.  We can do
> > > it with our coreboot based firmware, but there's no guarantee other
> > > vendors will adopt the same approach.  But I think at least for the
> > > ChromeOS ecosystem we can come up with something that'll work, and
> > > allow us to dtrt in userspace wrt driver binding.
> > 
> > Agree, we can work with our firmware teams to get that right, and then
> > expose it from kernel to userspace to help it implement the policy we
> > want.
> 
> This is already in the spec for USB, I suggest working to get this added
> to the other bus type specs that you care about as well (UEFI, PCI,
> etc.)

Note that "you can only use authorized SSD and authorized WIFI card in
this notebook" was done before, but is considered antisocial.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: PGP signature


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-15 Thread Rajat Jain
On Wed, Jun 10, 2020 at 12:57 PM Rajat Jain  wrote:
>
> On Tue, Jun 9, 2020 at 6:34 PM Oliver O'Halloran  wrote:
> >
> > On Wed, Jun 10, 2020 at 7:04 AM Bjorn Helgaas  wrote:
> > >
> > > To sketch this out, my understanding of how this would work is:
> > >
> > >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > > today, but doing so would be trivial.  I think I would prefer a
> > > sysfs name like "external" so it's more descriptive and less of a
> > > judgment.
> > >
> > > This comes from either the DT "external-facing" property or the
> > > ACPI "ExternalFacingPort" property.
> >
> > I don't think internal / external is the right distinction to be
> > making. We have a similar trust issue with the BMC in servers even
> > though they're internal devices. They're typically network accessible
> > and infrequently updated so treating them as trustworthy isn't a great
> > idea. We have been slowly de-privileging the BMC over the last few
> > years, but the PCIe interface isn't locked down enough for my liking
> > since the SoCs we use do allow software to set the VDID and perform
> > arbitrary DMAs (thankfully limited to 32bit). If we're going to add in
> > infrastructure for handling possibly untrustworthy PCI devices then
> > I'd like to use that for BMCs too.
> >
> > >   - All devices present at boot are enumerated.  Any statically built
> > > drivers will bind to them before any userspace code runs.
> > >
> > > If you want to keep statically built drivers from binding, you'd
> > > need to invent some mechanism so pci_driver_init() could clear
> > > drivers_autoprobe after registering pci_bus_type.
> > >
> > >   - Early userspace code prevents modular drivers from automatically
> > > binding to PCI devices:
> > >
> > >   echo 0 > /sys/bus/pci/drivers_autoprobe
> > >
> > > This prevents modular drivers from binding to all devices, whether
> > > present at boot or hot-added.
> >
> > I don't see why this is preferable to just disabling autoprobe for
> > untrusted devices. That would dovetail nicely with Rajat's whitelist
> > idea if we want to go down that route and I think we might want to.
> > The BMC usually provides some form of VGA console and we'd like that
> > to continue working out-of-the-box without too much user (or distro)
> > intervention.
>
> I wouldn't mind introducing a kernel parameter to disable auto-probing
> of untrusted devices if there is a wider agreement here.
> The only notch is that in my opinion, if present, that parameter
> should disable auto-probing for "external" devices only (i.e.
> "external-facing" devices should still be auto-probed).

So I looked around at my systems, and I realized that I will have to
go this way (introduce a parameter to disable auto-probing of
untrusted devices), because we do have systems on which we might not
have control over what devices will show up when (external devices'
PCI link may come up before the userspace gets a chance to run). I
shall be sending out a patch soon. I've sent out a couple of other
loosely related patches here:

https://lkml.org/lkml/2020/6/15/1447

Thanks,

Rajat

>
> Thanks,
>
> Rajat
>
> >
> > Oliver


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Rajat Jain
On Wed, Jun 10, 2020 at 4:01 PM Bjorn Helgaas  wrote:
>
> On Tue, Jun 09, 2020 at 05:30:13PM -0700, Rajat Jain wrote:
> > On Tue, Jun 9, 2020 at 5:04 PM Bjorn Helgaas  wrote:
> > > On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> > > > Thanks for sending out the summary, I was about to send it out but got 
> > > > lazy.
> > > > ...
>
> > > > The one thing that still needs more thought is how about the
> > > > "pcieport" driver that enumerates the PCI bridges. I'm unsure if it
> > > > needs to be whitelisted for further enumeration downstream. What do
> > > > you think?
> > >
> > > The pcieport driver is required for AER, PCIe native hotplug, PME,
> > > etc., and it cannot be a module, so the whitelist wouldn't apply to
> > > it.
> >
> > Not that I see the need, but slight clarification needed just to make
> > sure I understand it clearly:
> >
> > Since pcieport driver is statically compiled in, AER, pciehp, PME, DPC
> > etc will always be enabled for devices plugged in during boot. But I
> > can still choose to choose to allow or deny for devices added *after
> > boot* using the whitelist, right?
>
> Yes, I think so.  However, if pcieport doesn't claim hot-added devices
> like a dock, I don't think hotplug of PCI things downstream from the
> dock will work, e.g., if there are Thunderbolt switches in a monitor
> or something.

Yes, understood, thanks.

>
> > Also, denying pcieport driver for hot-added PCIe bridges only disables
> > these pcieport services on those bridges, but device enumeration
> > further downstream those bridges is not an issue?
>
> Right.  Devices without pcieport would not be able to report hotplug
> events, so we wouldn't notice hot-adds or -removes involving those
> devices.

Understood.

Thanks,

Rajat


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Bjorn Helgaas
On Wed, Jun 10, 2020 at 01:17:54PM -0700, Rajat Jain wrote:
> On Tue, Jun 9, 2020 at 5:30 PM Rajat Jain  wrote:
> >
> > On Tue, Jun 9, 2020 at 5:04 PM Bjorn Helgaas  wrote:
> > >
> > > On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> > > > Hi Bjorn,
> > > >
> > > > Thanks for sending out the summary, I was about to send it out but got 
> > > > lazy.
> > > >
> > > > On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas  wrote:
> > > > >
> > > > > On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
> > > > >
> > > > > > Your "problem" I think can be summed up a bit more concise:
> > > > > >   - you don't trust kernel drivers to be "secure" for untrusted
> > > > > > devices
> > > > > >   - you only want to bind kernel drivers to "internal" devices
> > > > > > automatically as you "trust" drivers in that situation.
> > > > > >   - you want to only bind specific kernel drivers that you 
> > > > > > somehow
> > > > > > feel are "secure" to untrusted devices "outside" of a system
> > > > > > when those devices are added to the system.
> > > > > >
> > > > > > Is that correct?
> > > > > >
> > > > > > If so, fine, you can do that today with the bind/unbind ability of
> > > > > > drivers, right?  After boot with your "trusted" drivers bound to
> > > > > > "internal" devices, turn off autobind of drivers to devices and then
> > > > > > manually bind them when you see new devices show up, as those 
> > > > > > "must" be
> > > > > > from external devices (see the bind/unbind files that all drivers 
> > > > > > export
> > > > > > for how to do this, and old lwn.net articles, this feature has been
> > > > > > around for a very long time.)
> > > > > >
> > > > > > I know for USB you can do this, odds are PCI you can turn off
> > > > > > autobinding as well, as I think this is a per-bus flag somewhere.  
> > > > > > If
> > > > > > that's not exported to userspace, should be trivial to do so, 
> > > > > > should be
> > > > > > somewere in the driver model already...
> > > > > >
> > > > > > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files 
> > > > > > in
> > > > > > sysfs for all busses.  Do those not work for you?
> > > > > >
> > > > > > My other points are the fact that you don't want to put policy in 
> > > > > > the
> > > > > > kernel, and I think that you can do everything you want in userspace
> > > > > > today, except maybe the fact that trying to determine what is 
> > > > > > "inside"
> > > > > > and "outside" is not always easy given that most hardware does not
> > > > > > export this information properly, if at all.  Go work with the 
> > > > > > firmware
> > > > > > people on that issue please, that would be most helpful for everyone
> > > > > > involved to get that finally straightened out.
> > > > >
> > > > > To sketch this out, my understanding of how this would work is:
> > > > >
> > > > >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > > > > today, but doing so would be trivial.  I think I would prefer a
> > > > > sysfs name like "external" so it's more descriptive and less of a
> > > > > judgment.
> > > >
> > > > Yes. I think we should probably semantically differentiate between
> > > > "external" and "external facing" devices. Root ports and downstream
> > > > ports can be "external facing" but are actually internal devices.
> > > > Anything below an "external facing" device is "external". So the sysfs
> > > > attribute "external" should be set only for devices that are truly
> > > > external.
> > >
> > > Good point; we (maybe you? :)) should fix that edge case.
> >
> 
> I realized that we may not need to distinguish between internal and
> external devices if we can assume that no internal PCI devices will
> show up after boot. That assumption is 99% true for our use case
> (leaving 1% out because we have some corner cases i.e. PCIe rescans,
> module insertions etc that may probably make some internal devices
> disappear and reappear).  If I find that I can do without the need for
> a UAPI to distinguish internal vs external devices, do you still want
> me to fix this edge case (i.e. "break" the pdev->untrusted flag into
> "external_facing" and "external" devices)?

I'm not sure there's a need to explicitly identify the
"external-facing" devices at all.  AFAIK there's nothing today that
would do anything different with a root port that happens to have an
external connector.

The "untrusted" uses today are things like forcing use of an IOMMU.
Most root ports don't initiate DMA themselves, but even if they did,
there's no reason to treat DMA from the root port differently than DMA
from other internal devices.  What we want is to treat DMA from
devices *connected* to that external connector differently.

So yes, I think we should change the current behavior so we only set
the dev->untrusted flag for devices *downstream* from an
external-facing device, but not the external-facing device 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Bjorn Helgaas
On Tue, Jun 09, 2020 at 05:30:13PM -0700, Rajat Jain wrote:
> On Tue, Jun 9, 2020 at 5:04 PM Bjorn Helgaas  wrote:
> > On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> > > Thanks for sending out the summary, I was about to send it out but got 
> > > lazy.
> > > ...

> > > The one thing that still needs more thought is how about the
> > > "pcieport" driver that enumerates the PCI bridges. I'm unsure if it
> > > needs to be whitelisted for further enumeration downstream. What do
> > > you think?
> >
> > The pcieport driver is required for AER, PCIe native hotplug, PME,
> > etc., and it cannot be a module, so the whitelist wouldn't apply to
> > it.
> 
> Not that I see the need, but slight clarification needed just to make
> sure I understand it clearly:
> 
> Since pcieport driver is statically compiled in, AER, pciehp, PME, DPC
> etc will always be enabled for devices plugged in during boot. But I
> can still choose to choose to allow or deny for devices added *after
> boot* using the whitelist, right?

Yes, I think so.  However, if pcieport doesn't claim hot-added devices
like a dock, I don't think hotplug of PCI things downstream from the
dock will work, e.g., if there are Thunderbolt switches in a monitor
or something.

> Also, denying pcieport driver for hot-added PCIe bridges only disables
> these pcieport services on those bridges, but device enumeration
> further downstream those bridges is not an issue?

Right.  Devices without pcieport would not be able to report hotplug
events, so we wouldn't notice hot-adds or -removes involving those
devices.


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Rajat Jain
On Tue, Jun 9, 2020 at 5:30 PM Rajat Jain  wrote:
>
> On Tue, Jun 9, 2020 at 5:04 PM Bjorn Helgaas  wrote:
> >
> > On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> > > Hi Bjorn,
> > >
> > > Thanks for sending out the summary, I was about to send it out but got 
> > > lazy.
> > >
> > > On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas  wrote:
> > > >
> > > > On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
> > > >
> > > > > Your "problem" I think can be summed up a bit more concise:
> > > > >   - you don't trust kernel drivers to be "secure" for untrusted
> > > > > devices
> > > > >   - you only want to bind kernel drivers to "internal" devices
> > > > > automatically as you "trust" drivers in that situation.
> > > > >   - you want to only bind specific kernel drivers that you somehow
> > > > > feel are "secure" to untrusted devices "outside" of a system
> > > > > when those devices are added to the system.
> > > > >
> > > > > Is that correct?
> > > > >
> > > > > If so, fine, you can do that today with the bind/unbind ability of
> > > > > drivers, right?  After boot with your "trusted" drivers bound to
> > > > > "internal" devices, turn off autobind of drivers to devices and then
> > > > > manually bind them when you see new devices show up, as those "must" 
> > > > > be
> > > > > from external devices (see the bind/unbind files that all drivers 
> > > > > export
> > > > > for how to do this, and old lwn.net articles, this feature has been
> > > > > around for a very long time.)
> > > > >
> > > > > I know for USB you can do this, odds are PCI you can turn off
> > > > > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > > > > that's not exported to userspace, should be trivial to do so, should 
> > > > > be
> > > > > somewere in the driver model already...
> > > > >
> > > > > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > > > > sysfs for all busses.  Do those not work for you?
> > > > >
> > > > > My other points are the fact that you don't want to put policy in the
> > > > > kernel, and I think that you can do everything you want in userspace
> > > > > today, except maybe the fact that trying to determine what is "inside"
> > > > > and "outside" is not always easy given that most hardware does not
> > > > > export this information properly, if at all.  Go work with the 
> > > > > firmware
> > > > > people on that issue please, that would be most helpful for everyone
> > > > > involved to get that finally straightened out.
> > > >
> > > > To sketch this out, my understanding of how this would work is:
> > > >
> > > >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > > > today, but doing so would be trivial.  I think I would prefer a
> > > > sysfs name like "external" so it's more descriptive and less of a
> > > > judgment.
> > >
> > > Yes. I think we should probably semantically differentiate between
> > > "external" and "external facing" devices. Root ports and downstream
> > > ports can be "external facing" but are actually internal devices.
> > > Anything below an "external facing" device is "external". So the sysfs
> > > attribute "external" should be set only for devices that are truly
> > > external.
> >
> > Good point; we (maybe you? :)) should fix that edge case.
>

I realized that we may not need to distinguish between internal and
external devices if we can assume that no internal PCI devices will
show up after boot. That assumption is 99% true for our use case
(leaving 1% out because we have some corner cases i.e. PCIe rescans,
module insertions etc that may probably make some internal devices
disappear and reappear).  If I find that I can do without the need for
a UAPI to distinguish internal vs external devices, do you still want
me to fix this edge case (i.e. "break" the pdev->untrusted flag into
"external_facing" and "external" devices)?

Thanks,

Rajat

> Sure, happy to. I will start a fresh conversation about that (in a
> separate thread).
>
> >
> > > Just a suggestion: Do you think an enum attribute may be better
> > > instead, whose values could be "internal" / "external" /
> > > "external-facing" in case need arises later to distinguish between
> > > them?
> >
> > I don't see the need for an enum yet.  Maybe we should add that
> > if/when we do need it?
>
> Sure, no problems. (I just wanted to slip the thought into the
> conversation as UAPI is hard to change later).
>
> >
> > > >   - Early userspace code prevents modular drivers from automatically
> > > > binding to PCI devices:
> > > >
> > > >   echo 0 > /sys/bus/pci/drivers_autoprobe
> > >
> > > Yes.
> > > I believe this setting will apply it equally to both modular and
> > > statically linked drivers?
> >
> > Yes.  The test is in bus_probe_device(), and it does the same for both
> > modular and statically linked drivers.
> >
> > But for statically linked drivers, it only 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Rajat Jain
On Tue, Jun 9, 2020 at 6:34 PM Oliver O'Halloran  wrote:
>
> On Wed, Jun 10, 2020 at 7:04 AM Bjorn Helgaas  wrote:
> >
> > To sketch this out, my understanding of how this would work is:
> >
> >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > today, but doing so would be trivial.  I think I would prefer a
> > sysfs name like "external" so it's more descriptive and less of a
> > judgment.
> >
> > This comes from either the DT "external-facing" property or the
> > ACPI "ExternalFacingPort" property.
>
> I don't think internal / external is the right distinction to be
> making. We have a similar trust issue with the BMC in servers even
> though they're internal devices. They're typically network accessible
> and infrequently updated so treating them as trustworthy isn't a great
> idea. We have been slowly de-privileging the BMC over the last few
> years, but the PCIe interface isn't locked down enough for my liking
> since the SoCs we use do allow software to set the VDID and perform
> arbitrary DMAs (thankfully limited to 32bit). If we're going to add in
> infrastructure for handling possibly untrustworthy PCI devices then
> I'd like to use that for BMCs too.
>
> >   - All devices present at boot are enumerated.  Any statically built
> > drivers will bind to them before any userspace code runs.
> >
> > If you want to keep statically built drivers from binding, you'd
> > need to invent some mechanism so pci_driver_init() could clear
> > drivers_autoprobe after registering pci_bus_type.
> >
> >   - Early userspace code prevents modular drivers from automatically
> > binding to PCI devices:
> >
> >   echo 0 > /sys/bus/pci/drivers_autoprobe
> >
> > This prevents modular drivers from binding to all devices, whether
> > present at boot or hot-added.
>
> I don't see why this is preferable to just disabling autoprobe for
> untrusted devices. That would dovetail nicely with Rajat's whitelist
> idea if we want to go down that route and I think we might want to.
> The BMC usually provides some form of VGA console and we'd like that
> to continue working out-of-the-box without too much user (or distro)
> intervention.

I wouldn't mind introducing a kernel parameter to disable auto-probing
of untrusted devices if there is a wider agreement here.
The only notch is that in my opinion, if present, that parameter
should disable auto-probing for "external" devices only (i.e.
"external-facing" devices should still be auto-probed).

Thanks,

Rajat

>
> Oliver


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Greg Kroah-Hartman
On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> The one thing that still needs more thought is how about the
> "pcieport" driver that enumerates the PCI bridges. I'm unsure if it
> needs to be whitelisted for further enumeration downstream. What do
> you think?

Why not just do whatever type of "code review" you need to do for that
one core driver to get that off of your "drivers to worry about" list?
:)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-10 Thread Greg Kroah-Hartman
On Tue, Jun 09, 2020 at 04:04:00PM -0500, Bjorn Helgaas wrote:
> On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
> 
> > Your "problem" I think can be summed up a bit more concise:
> > - you don't trust kernel drivers to be "secure" for untrusted
> >   devices
> > - you only want to bind kernel drivers to "internal" devices
> >   automatically as you "trust" drivers in that situation.
> > - you want to only bind specific kernel drivers that you somehow
> >   feel are "secure" to untrusted devices "outside" of a system
> >   when those devices are added to the system.
> > 
> > Is that correct?
> > 
> > If so, fine, you can do that today with the bind/unbind ability of
> > drivers, right?  After boot with your "trusted" drivers bound to
> > "internal" devices, turn off autobind of drivers to devices and then
> > manually bind them when you see new devices show up, as those "must" be
> > from external devices (see the bind/unbind files that all drivers export
> > for how to do this, and old lwn.net articles, this feature has been
> > around for a very long time.)
> > 
> > I know for USB you can do this, odds are PCI you can turn off
> > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > that's not exported to userspace, should be trivial to do so, should be
> > somewere in the driver model already...
> > 
> > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > sysfs for all busses.  Do those not work for you?
> > 
> > My other points are the fact that you don't want to put policy in the
> > kernel, and I think that you can do everything you want in userspace
> > today, except maybe the fact that trying to determine what is "inside"
> > and "outside" is not always easy given that most hardware does not
> > export this information properly, if at all.  Go work with the firmware
> > people on that issue please, that would be most helpful for everyone
> > involved to get that finally straightened out.
> 
> To sketch this out, my understanding of how this would work is:
> 
>   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> today, but doing so would be trivial.  I think I would prefer a
> sysfs name like "external" so it's more descriptive and less of a
> judgment.
> 
> This comes from either the DT "external-facing" property or the
> ACPI "ExternalFacingPort" property.  

Good idea, but as people have pointed out, even these don't always work
so userspace will need to be able to override that somehow :(

>   - All devices present at boot are enumerated.  Any statically built
> drivers will bind to them before any userspace code runs.
> 
> If you want to keep statically built drivers from binding, you'd
> need to invent some mechanism so pci_driver_init() could clear
> drivers_autoprobe after registering pci_bus_type.
> 
>   - Early userspace code prevents modular drivers from automatically
> binding to PCI devices:
> 
>   echo 0 > /sys/bus/pci/drivers_autoprobe
> 
> This prevents modular drivers from binding to all devices, whether
> present at boot or hot-added.
> 
>   - Userspace code uses the sysfs "bind" file to control which drivers
> are loaded and can bind to each device, e.g.,
> 
>   echo :02:00.0 > /sys/bus/pci/drivers/nvme/bind

Seems good to me, and also matches what the current USB tools do for
this type of thing.

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Oliver O'Halloran
On Wed, Jun 10, 2020 at 7:04 AM Bjorn Helgaas  wrote:
>
> To sketch this out, my understanding of how this would work is:
>
>   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> today, but doing so would be trivial.  I think I would prefer a
> sysfs name like "external" so it's more descriptive and less of a
> judgment.
>
> This comes from either the DT "external-facing" property or the
> ACPI "ExternalFacingPort" property.

I don't think internal / external is the right distinction to be
making. We have a similar trust issue with the BMC in servers even
though they're internal devices. They're typically network accessible
and infrequently updated so treating them as trustworthy isn't a great
idea. We have been slowly de-privileging the BMC over the last few
years, but the PCIe interface isn't locked down enough for my liking
since the SoCs we use do allow software to set the VDID and perform
arbitrary DMAs (thankfully limited to 32bit). If we're going to add in
infrastructure for handling possibly untrustworthy PCI devices then
I'd like to use that for BMCs too.

>   - All devices present at boot are enumerated.  Any statically built
> drivers will bind to them before any userspace code runs.
>
> If you want to keep statically built drivers from binding, you'd
> need to invent some mechanism so pci_driver_init() could clear
> drivers_autoprobe after registering pci_bus_type.
>
>   - Early userspace code prevents modular drivers from automatically
> binding to PCI devices:
>
>   echo 0 > /sys/bus/pci/drivers_autoprobe
>
> This prevents modular drivers from binding to all devices, whether
> present at boot or hot-added.

I don't see why this is preferable to just disabling autoprobe for
untrusted devices. That would dovetail nicely with Rajat's whitelist
idea if we want to go down that route and I think we might want to.
The BMC usually provides some form of VGA console and we'd like that
to continue working out-of-the-box without too much user (or distro)
intervention.

Oliver


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Rajat Jain
On Tue, Jun 9, 2020 at 5:04 PM Bjorn Helgaas  wrote:
>
> On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> > Hi Bjorn,
> >
> > Thanks for sending out the summary, I was about to send it out but got lazy.
> >
> > On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas  wrote:
> > >
> > > On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
> > >
> > > > Your "problem" I think can be summed up a bit more concise:
> > > >   - you don't trust kernel drivers to be "secure" for untrusted
> > > > devices
> > > >   - you only want to bind kernel drivers to "internal" devices
> > > > automatically as you "trust" drivers in that situation.
> > > >   - you want to only bind specific kernel drivers that you somehow
> > > > feel are "secure" to untrusted devices "outside" of a system
> > > > when those devices are added to the system.
> > > >
> > > > Is that correct?
> > > >
> > > > If so, fine, you can do that today with the bind/unbind ability of
> > > > drivers, right?  After boot with your "trusted" drivers bound to
> > > > "internal" devices, turn off autobind of drivers to devices and then
> > > > manually bind them when you see new devices show up, as those "must" be
> > > > from external devices (see the bind/unbind files that all drivers export
> > > > for how to do this, and old lwn.net articles, this feature has been
> > > > around for a very long time.)
> > > >
> > > > I know for USB you can do this, odds are PCI you can turn off
> > > > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > > > that's not exported to userspace, should be trivial to do so, should be
> > > > somewere in the driver model already...
> > > >
> > > > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > > > sysfs for all busses.  Do those not work for you?
> > > >
> > > > My other points are the fact that you don't want to put policy in the
> > > > kernel, and I think that you can do everything you want in userspace
> > > > today, except maybe the fact that trying to determine what is "inside"
> > > > and "outside" is not always easy given that most hardware does not
> > > > export this information properly, if at all.  Go work with the firmware
> > > > people on that issue please, that would be most helpful for everyone
> > > > involved to get that finally straightened out.
> > >
> > > To sketch this out, my understanding of how this would work is:
> > >
> > >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > > today, but doing so would be trivial.  I think I would prefer a
> > > sysfs name like "external" so it's more descriptive and less of a
> > > judgment.
> >
> > Yes. I think we should probably semantically differentiate between
> > "external" and "external facing" devices. Root ports and downstream
> > ports can be "external facing" but are actually internal devices.
> > Anything below an "external facing" device is "external". So the sysfs
> > attribute "external" should be set only for devices that are truly
> > external.
>
> Good point; we (maybe you? :)) should fix that edge case.

Sure, happy to. I will start a fresh conversation about that (in a
separate thread).

>
> > Just a suggestion: Do you think an enum attribute may be better
> > instead, whose values could be "internal" / "external" /
> > "external-facing" in case need arises later to distinguish between
> > them?
>
> I don't see the need for an enum yet.  Maybe we should add that
> if/when we do need it?

Sure, no problems. (I just wanted to slip the thought into the
conversation as UAPI is hard to change later).

>
> > >   - Early userspace code prevents modular drivers from automatically
> > > binding to PCI devices:
> > >
> > >   echo 0 > /sys/bus/pci/drivers_autoprobe
> >
> > Yes.
> > I believe this setting will apply it equally to both modular and
> > statically linked drivers?
>
> Yes.  The test is in bus_probe_device(), and it does the same for both
> modular and statically linked drivers.
>
> But for statically linked drivers, it only prevents them from binding
> to *hot-added* devices.  They will claim devices present at boot even
> before userspace code can run.

Yes, understood.

>
> > The one thing that still needs more thought is how about the
> > "pcieport" driver that enumerates the PCI bridges. I'm unsure if it
> > needs to be whitelisted for further enumeration downstream. What do
> > you think?
>
> The pcieport driver is required for AER, PCIe native hotplug, PME,
> etc., and it cannot be a module, so the whitelist wouldn't apply to
> it.

Not that I see the need, but slight clarification needed just to make
sure I understand it clearly:

Since pcieport driver is statically compiled in, AER, pciehp, PME, DPC
etc will always be enabled for devices plugged in during boot. But I
can still choose to choose to allow or deny for devices added *after
boot* using the whitelist, right?

Also, denying 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Bjorn Helgaas
On Tue, Jun 09, 2020 at 04:23:54PM -0700, Rajat Jain wrote:
> Hi Bjorn,
> 
> Thanks for sending out the summary, I was about to send it out but got lazy.
> 
> On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas  wrote:
> >
> > On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
> >
> > > Your "problem" I think can be summed up a bit more concise:
> > >   - you don't trust kernel drivers to be "secure" for untrusted
> > > devices
> > >   - you only want to bind kernel drivers to "internal" devices
> > > automatically as you "trust" drivers in that situation.
> > >   - you want to only bind specific kernel drivers that you somehow
> > > feel are "secure" to untrusted devices "outside" of a system
> > > when those devices are added to the system.
> > >
> > > Is that correct?
> > >
> > > If so, fine, you can do that today with the bind/unbind ability of
> > > drivers, right?  After boot with your "trusted" drivers bound to
> > > "internal" devices, turn off autobind of drivers to devices and then
> > > manually bind them when you see new devices show up, as those "must" be
> > > from external devices (see the bind/unbind files that all drivers export
> > > for how to do this, and old lwn.net articles, this feature has been
> > > around for a very long time.)
> > >
> > > I know for USB you can do this, odds are PCI you can turn off
> > > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > > that's not exported to userspace, should be trivial to do so, should be
> > > somewere in the driver model already...
> > >
> > > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > > sysfs for all busses.  Do those not work for you?
> > >
> > > My other points are the fact that you don't want to put policy in the
> > > kernel, and I think that you can do everything you want in userspace
> > > today, except maybe the fact that trying to determine what is "inside"
> > > and "outside" is not always easy given that most hardware does not
> > > export this information properly, if at all.  Go work with the firmware
> > > people on that issue please, that would be most helpful for everyone
> > > involved to get that finally straightened out.
> >
> > To sketch this out, my understanding of how this would work is:
> >
> >   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> > today, but doing so would be trivial.  I think I would prefer a
> > sysfs name like "external" so it's more descriptive and less of a
> > judgment.
> 
> Yes. I think we should probably semantically differentiate between
> "external" and "external facing" devices. Root ports and downstream
> ports can be "external facing" but are actually internal devices.
> Anything below an "external facing" device is "external". So the sysfs
> attribute "external" should be set only for devices that are truly
> external.

Good point; we (maybe you? :)) should fix that edge case.

> Just a suggestion: Do you think an enum attribute may be better
> instead, whose values could be "internal" / "external" /
> "external-facing" in case need arises later to distinguish between
> them?

I don't see the need for an enum yet.  Maybe we should add that
if/when we do need it?

> >   - Early userspace code prevents modular drivers from automatically
> > binding to PCI devices:
> >
> >   echo 0 > /sys/bus/pci/drivers_autoprobe
> 
> Yes.
> I believe this setting will apply it equally to both modular and
> statically linked drivers?

Yes.  The test is in bus_probe_device(), and it does the same for both
modular and statically linked drivers.

But for statically linked drivers, it only prevents them from binding
to *hot-added* devices.  They will claim devices present at boot even
before userspace code can run.

> The one thing that still needs more thought is how about the
> "pcieport" driver that enumerates the PCI bridges. I'm unsure if it
> needs to be whitelisted for further enumeration downstream. What do
> you think?

The pcieport driver is required for AER, PCIe native hotplug, PME,
etc., and it cannot be a module, so the whitelist wouldn't apply to
it.  I assume you need hotplug support, so you would have pcieport
enabled and built in statically.

If you're using ACPI hotplug, that doesn't require pcieport.


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Rajat Jain
Hi Bjorn,

Thanks for sending out the summary, I was about to send it out but got lazy.

On Tue, Jun 9, 2020 at 2:04 PM Bjorn Helgaas  wrote:
>
> On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:
>
> > Your "problem" I think can be summed up a bit more concise:
> >   - you don't trust kernel drivers to be "secure" for untrusted
> > devices
> >   - you only want to bind kernel drivers to "internal" devices
> > automatically as you "trust" drivers in that situation.
> >   - you want to only bind specific kernel drivers that you somehow
> > feel are "secure" to untrusted devices "outside" of a system
> > when those devices are added to the system.
> >
> > Is that correct?
> >
> > If so, fine, you can do that today with the bind/unbind ability of
> > drivers, right?  After boot with your "trusted" drivers bound to
> > "internal" devices, turn off autobind of drivers to devices and then
> > manually bind them when you see new devices show up, as those "must" be
> > from external devices (see the bind/unbind files that all drivers export
> > for how to do this, and old lwn.net articles, this feature has been
> > around for a very long time.)
> >
> > I know for USB you can do this, odds are PCI you can turn off
> > autobinding as well, as I think this is a per-bus flag somewhere.  If
> > that's not exported to userspace, should be trivial to do so, should be
> > somewere in the driver model already...
> >
> > Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> > sysfs for all busses.  Do those not work for you?
> >
> > My other points are the fact that you don't want to put policy in the
> > kernel, and I think that you can do everything you want in userspace
> > today, except maybe the fact that trying to determine what is "inside"
> > and "outside" is not always easy given that most hardware does not
> > export this information properly, if at all.  Go work with the firmware
> > people on that issue please, that would be most helpful for everyone
> > involved to get that finally straightened out.
>
> To sketch this out, my understanding of how this would work is:
>
>   - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
> today, but doing so would be trivial.  I think I would prefer a
> sysfs name like "external" so it's more descriptive and less of a
> judgment.

Yes. I think we should probably semantically differentiate between
"external" and "external facing" devices. Root ports and downstream
ports can be "external facing" but are actually internal devices.
Anything below an "external facing" device is "external". So the sysfs
attribute "external" should be set only for devices that are truly
external.

So I think we can possibly synthesize "external" sysfs attribute from
"untrusted" bit like this (Sorry code looks more complex than it is):

parent = pdev->bus->self;

if (pdev->untrusted) {
if (parent && parent->untrusted)
pdev->external = true;   /* Device downstream of
un-trusted device = external */
else {
pdev->external = false   /* Root complex or an
internal Downstream port */
} else {
pdev->external = false; /* Trusted device = Internal device */
}

For platforms that don't expose and untrusted" device, everything is
assumed to be an "internal device".

Just a suggestion: Do you think an enum attribute may be better
instead, whose values could be "internal" / "external" /
"external-facing" in case need arises later to distinguish between
them?

>
> This comes from either the DT "external-facing" property or the
> ACPI "ExternalFacingPort" property.
>
>   - All devices present at boot are enumerated.  Any statically built
> drivers will bind to them before any userspace code runs.

Yes. For our (thunderbolt / USB4) use case, we'd still be protected
because we can control the PCIe tunnels to thunderbolt / USB4 devices
and will not enable them until we are ready. So while this approach
may not work for a system that always enables PCIe connections to
external devices at boot, it works for our use case as we are looking
for only thunderbolt / USB4 devices. (Not a problem or concern, just
wanted to be clear).

>
> If you want to keep statically built drivers from binding, you'd
> need to invent some mechanism so pci_driver_init() could clear
> drivers_autoprobe after registering pci_bus_type.

At present I am not planning this.

>
>   - Early userspace code prevents modular drivers from automatically
> binding to PCI devices:
>
>   echo 0 > /sys/bus/pci/drivers_autoprobe

Yes.
I believe this setting will apply it equally to both modular and
statically linked drivers?

>
> This prevents modular drivers from binding to all devices, whether
> present at boot or hot-added.

Yes, at this time, the userspace will need to monitor udev events for
any new PCI devices hot added, and lookup the VID/DID in pci 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Bjorn Helgaas
On Sun, Jun 07, 2020 at 01:36:32PM +0200, Greg Kroah-Hartman wrote:

> Your "problem" I think can be summed up a bit more concise:
>   - you don't trust kernel drivers to be "secure" for untrusted
> devices
>   - you only want to bind kernel drivers to "internal" devices
> automatically as you "trust" drivers in that situation.
>   - you want to only bind specific kernel drivers that you somehow
> feel are "secure" to untrusted devices "outside" of a system
> when those devices are added to the system.
> 
> Is that correct?
> 
> If so, fine, you can do that today with the bind/unbind ability of
> drivers, right?  After boot with your "trusted" drivers bound to
> "internal" devices, turn off autobind of drivers to devices and then
> manually bind them when you see new devices show up, as those "must" be
> from external devices (see the bind/unbind files that all drivers export
> for how to do this, and old lwn.net articles, this feature has been
> around for a very long time.)
> 
> I know for USB you can do this, odds are PCI you can turn off
> autobinding as well, as I think this is a per-bus flag somewhere.  If
> that's not exported to userspace, should be trivial to do so, should be
> somewere in the driver model already...
> 
> Ah, yes, look at the "drivers_autoprobe" and "drivers_probe" files in
> sysfs for all busses.  Do those not work for you?
> 
> My other points are the fact that you don't want to put policy in the
> kernel, and I think that you can do everything you want in userspace
> today, except maybe the fact that trying to determine what is "inside"
> and "outside" is not always easy given that most hardware does not
> export this information properly, if at all.  Go work with the firmware
> people on that issue please, that would be most helpful for everyone
> involved to get that finally straightened out.

To sketch this out, my understanding of how this would work is:

  - Expose the PCI pdev->untrusted bit in sysfs.  We don't expose this
today, but doing so would be trivial.  I think I would prefer a
sysfs name like "external" so it's more descriptive and less of a
judgment.

This comes from either the DT "external-facing" property or the
ACPI "ExternalFacingPort" property.  

  - All devices present at boot are enumerated.  Any statically built
drivers will bind to them before any userspace code runs.

If you want to keep statically built drivers from binding, you'd
need to invent some mechanism so pci_driver_init() could clear
drivers_autoprobe after registering pci_bus_type.

  - Early userspace code prevents modular drivers from automatically
binding to PCI devices:

  echo 0 > /sys/bus/pci/drivers_autoprobe

This prevents modular drivers from binding to all devices, whether
present at boot or hot-added.

  - Userspace code uses the sysfs "bind" file to control which drivers
are loaded and can bind to each device, e.g.,

  echo :02:00.0 > /sys/bus/pci/drivers/nvme/bind

Is that what you're thinking?  Is that enough for the control you
need, Rajat?


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-09 Thread Greg Kroah-Hartman
On Mon, Jun 08, 2020 at 11:41:19AM -0700, Rajat Jain wrote:
> Hi Jesse and Greg,
> 
> On Mon, Jun 8, 2020 at 11:30 AM Jesse Barnes  wrote:
> >
> > > > I think your suggestion to disable driver binding once the initial
> > > > bus/slot devices have been bound will probably work for this
> > > > situation.  I just wanted to be clear that without some auditing,
> > > > fuzzing, and additional testing, we simply have to assume that drivers
> > > > are *not* secure and avoid using them on untrusted devices until we're
> > > > fairly confident they can handle them (whether just misbehaving or
> > > > malicious), in combination with other approaches like IOMMUs of
> > > > course.  And this isn't because we don't trust driver authors or
> > > > kernel developers to dtrt, it's just that for many devices (maybe USB
> > > > is an exception) I think driver authors haven't had to consider this
> > > > case much, and so I think it's prudent to expect bugs in this area
> > > > that we need to find & fix.
> > >
> > > For USB, yes, we have now had to deal with "untrusted devices" lieing
> > > about their ids and sending us horrible data.  That's all due to the
> > > fuzzing tools that have been written over the past few years, and now we
> > > have some of those in the kernel tree itself to help with that testing.
> 
> This is great to hear! I tried to look up but didn't find anything
> else in-kernel, except the kcov support to export coverage info for
> userspace fuzzers. Can you please give us some pointers for in-kernel
> fuzzing tools?

For USB, it's a combination of using syzbot with the "raw gadget" driver
and the loopback gadget/host controller.  See many posts from Andrey
Konovalov  on the linux-...@vger.kernel.org list
for details as to how he does this.

> > > For PCI, heh, good luck, those assumptions about "devices sending valid
> > > data" are everywhere, if our experience with USB is any indication.
> > >
> > > But, to take USB as an example, this is exactly what the USB
> > > "authorized" flag is there for, it's a "trust" setting that userspace
> > > has control over.  This came from the wireless USB spec, where it was
> > > determined that you could not trust devices.  So just use that same
> > > model here, move it to the driver core for all busses to use and you
> > > should be fine.
> > >
> > > If that doesn't meet your needs, please let me know the specifics of
> > > why, with patches :)
> >
> > Yeah will do for sure.  I don't want to carry a big infra for this on our 
> > own!
> >
> > > Now, as to you all getting some sort of "Hardware flag" to determine
> > > "inside" vs. "outside" devices, hah, good luck!  It took us a long time
> > > to get that for USB, and even then, BIOSes lie and get it wrong all the
> > > time.  So you will have to also deal with that in some way, for your
> > > userspace policy.
> >
> > I think that's inherently platform specific to some extent.  We can do
> > it with our coreboot based firmware, but there's no guarantee other
> > vendors will adopt the same approach.  But I think at least for the
> > ChromeOS ecosystem we can come up with something that'll work, and
> > allow us to dtrt in userspace wrt driver binding.
> 
> Agree, we can work with our firmware teams to get that right, and then
> expose it from kernel to userspace to help it implement the policy we
> want.

This is already in the spec for USB, I suggest working to get this added
to the other bus type specs that you care about as well (UEFI, PCI,
etc.)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-08 Thread Greg Kroah-Hartman
On Mon, Jun 08, 2020 at 11:29:58AM -0700, Jesse Barnes wrote:
> > Now, as to you all getting some sort of "Hardware flag" to determine
> > "inside" vs. "outside" devices, hah, good luck!  It took us a long time
> > to get that for USB, and even then, BIOSes lie and get it wrong all the
> > time.  So you will have to also deal with that in some way, for your
> > userspace policy.
> 
> I think that's inherently platform specific to some extent.  We can do
> it with our coreboot based firmware, but there's no guarantee other
> vendors will adopt the same approach.  But I think at least for the
> ChromeOS ecosystem we can come up with something that'll work, and
> allow us to dtrt in userspace wrt driver binding.

Why not work with the UEFI group to add this to their spec so that it
will work for all future firmware releases, not just your
vendor-specific one?  :)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-08 Thread Rajat Jain
Hi Jesse and Greg,

On Mon, Jun 8, 2020 at 11:30 AM Jesse Barnes  wrote:
>
> > > I think your suggestion to disable driver binding once the initial
> > > bus/slot devices have been bound will probably work for this
> > > situation.  I just wanted to be clear that without some auditing,
> > > fuzzing, and additional testing, we simply have to assume that drivers
> > > are *not* secure and avoid using them on untrusted devices until we're
> > > fairly confident they can handle them (whether just misbehaving or
> > > malicious), in combination with other approaches like IOMMUs of
> > > course.  And this isn't because we don't trust driver authors or
> > > kernel developers to dtrt, it's just that for many devices (maybe USB
> > > is an exception) I think driver authors haven't had to consider this
> > > case much, and so I think it's prudent to expect bugs in this area
> > > that we need to find & fix.
> >
> > For USB, yes, we have now had to deal with "untrusted devices" lieing
> > about their ids and sending us horrible data.  That's all due to the
> > fuzzing tools that have been written over the past few years, and now we
> > have some of those in the kernel tree itself to help with that testing.

This is great to hear! I tried to look up but didn't find anything
else in-kernel, except the kcov support to export coverage info for
userspace fuzzers. Can you please give us some pointers for in-kernel
fuzzing tools?

> >
> > For PCI, heh, good luck, those assumptions about "devices sending valid
> > data" are everywhere, if our experience with USB is any indication.
> >
> > But, to take USB as an example, this is exactly what the USB
> > "authorized" flag is there for, it's a "trust" setting that userspace
> > has control over.  This came from the wireless USB spec, where it was
> > determined that you could not trust devices.  So just use that same
> > model here, move it to the driver core for all busses to use and you
> > should be fine.
> >
> > If that doesn't meet your needs, please let me know the specifics of
> > why, with patches :)
>
> Yeah will do for sure.  I don't want to carry a big infra for this on our own!
>
> > Now, as to you all getting some sort of "Hardware flag" to determine
> > "inside" vs. "outside" devices, hah, good luck!  It took us a long time
> > to get that for USB, and even then, BIOSes lie and get it wrong all the
> > time.  So you will have to also deal with that in some way, for your
> > userspace policy.
>
> I think that's inherently platform specific to some extent.  We can do
> it with our coreboot based firmware, but there's no guarantee other
> vendors will adopt the same approach.  But I think at least for the
> ChromeOS ecosystem we can come up with something that'll work, and
> allow us to dtrt in userspace wrt driver binding.

Agree, we can work with our firmware teams to get that right, and then
expose it from kernel to userspace to help it implement the policy we
want.

Thanks & Best Regards,

Rajat

>
> Thanks,
> Jesse


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-08 Thread Jesse Barnes
> > I think your suggestion to disable driver binding once the initial
> > bus/slot devices have been bound will probably work for this
> > situation.  I just wanted to be clear that without some auditing,
> > fuzzing, and additional testing, we simply have to assume that drivers
> > are *not* secure and avoid using them on untrusted devices until we're
> > fairly confident they can handle them (whether just misbehaving or
> > malicious), in combination with other approaches like IOMMUs of
> > course.  And this isn't because we don't trust driver authors or
> > kernel developers to dtrt, it's just that for many devices (maybe USB
> > is an exception) I think driver authors haven't had to consider this
> > case much, and so I think it's prudent to expect bugs in this area
> > that we need to find & fix.
>
> For USB, yes, we have now had to deal with "untrusted devices" lieing
> about their ids and sending us horrible data.  That's all due to the
> fuzzing tools that have been written over the past few years, and now we
> have some of those in the kernel tree itself to help with that testing.
>
> For PCI, heh, good luck, those assumptions about "devices sending valid
> data" are everywhere, if our experience with USB is any indication.
>
> But, to take USB as an example, this is exactly what the USB
> "authorized" flag is there for, it's a "trust" setting that userspace
> has control over.  This came from the wireless USB spec, where it was
> determined that you could not trust devices.  So just use that same
> model here, move it to the driver core for all busses to use and you
> should be fine.
>
> If that doesn't meet your needs, please let me know the specifics of
> why, with patches :)

Yeah will do for sure.  I don't want to carry a big infra for this on our own!

> Now, as to you all getting some sort of "Hardware flag" to determine
> "inside" vs. "outside" devices, hah, good luck!  It took us a long time
> to get that for USB, and even then, BIOSes lie and get it wrong all the
> time.  So you will have to also deal with that in some way, for your
> userspace policy.

I think that's inherently platform specific to some extent.  We can do
it with our coreboot based firmware, but there's no guarantee other
vendors will adopt the same approach.  But I think at least for the
ChromeOS ecosystem we can come up with something that'll work, and
allow us to dtrt in userspace wrt driver binding.

Thanks,
Jesse


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-08 Thread Greg Kroah-Hartman
On Mon, Jun 08, 2020 at 10:03:38AM -0700, Jesse Barnes wrote:
> > > I feel a lot of resistance to the proposal, however, I'm not hearing
> > > any realistic solutions that may help us to move forward. We want to
> > > go with a solution that is acceptable upstream as that is our mission,
> > > and also helps the community, however the behemoth task of "inspect
> > > all drivers and fix them" before launching a product is really an
> > > unfair ask I feel :-(. Can you help us by suggesting a proposal that
> > > does not require us to trust a driver equally for internal / external
> > > devices?
> >
> > I have no idea why you feel you have to "inspect all drivers" other than
> > the fact that for some reason _you_ do not feel they are secure today.
> >
> > What type of "assurance" are you, or anyone else going to be able to
> > provide for any kernel driver that would meet such a "I feel good now"
> > level?  Have you done that work for any specific driver already so that
> > you can show us what you mean by this effort?  Perhaps it's as simple as
> > "oh look, this tool over here runs 'clean' on the source code, all is
> > good!", or not, I really have no idea.
> 
> I think there's a disconnect somewhere in this discussion... maybe
> we're just approaching this with different assumptions?
> 
> I think you recognize the potential for driver vulnerabilities when
> binding to new or potentially hostile devices that may be spoofing
> DID/VID/class/etc than then go on to abuse driver trust or the driver
> using unvalidated inputs from the device to crash or run arbitrary
> code on the target system.
> 
> Yes such drivers should be fixed, no doubt.  But without lots of
> fuzzing (we're working on this) and testing we'd like to avoid
> exposing that attack surface at all.
> 
> I think your suggestion to disable driver binding once the initial
> bus/slot devices have been bound will probably work for this
> situation.  I just wanted to be clear that without some auditing,
> fuzzing, and additional testing, we simply have to assume that drivers
> are *not* secure and avoid using them on untrusted devices until we're
> fairly confident they can handle them (whether just misbehaving or
> malicious), in combination with other approaches like IOMMUs of
> course.  And this isn't because we don't trust driver authors or
> kernel developers to dtrt, it's just that for many devices (maybe USB
> is an exception) I think driver authors haven't had to consider this
> case much, and so I think it's prudent to expect bugs in this area
> that we need to find & fix.

For USB, yes, we have now had to deal with "untrusted devices" lieing
about their ids and sending us horrible data.  That's all due to the
fuzzing tools that have been written over the past few years, and now we
have some of those in the kernel tree itself to help with that testing.

For PCI, heh, good luck, those assumptions about "devices sending valid
data" are everywhere, if our experience with USB is any indication.

But, to take USB as an example, this is exactly what the USB
"authorized" flag is there for, it's a "trust" setting that userspace
has control over.  This came from the wireless USB spec, where it was
determined that you could not trust devices.  So just use that same
model here, move it to the driver core for all busses to use and you
should be fine.

If that doesn't meet your needs, please let me know the specifics of
why, with patches :)

Now, as to you all getting some sort of "Hardware flag" to determine
"inside" vs. "outside" devices, hah, good luck!  It took us a long time
to get that for USB, and even then, BIOSes lie and get it wrong all the
time.  So you will have to also deal with that in some way, for your
userspace policy.

good luck!

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-08 Thread Jesse Barnes
> > I feel a lot of resistance to the proposal, however, I'm not hearing
> > any realistic solutions that may help us to move forward. We want to
> > go with a solution that is acceptable upstream as that is our mission,
> > and also helps the community, however the behemoth task of "inspect
> > all drivers and fix them" before launching a product is really an
> > unfair ask I feel :-(. Can you help us by suggesting a proposal that
> > does not require us to trust a driver equally for internal / external
> > devices?
>
> I have no idea why you feel you have to "inspect all drivers" other than
> the fact that for some reason _you_ do not feel they are secure today.
>
> What type of "assurance" are you, or anyone else going to be able to
> provide for any kernel driver that would meet such a "I feel good now"
> level?  Have you done that work for any specific driver already so that
> you can show us what you mean by this effort?  Perhaps it's as simple as
> "oh look, this tool over here runs 'clean' on the source code, all is
> good!", or not, I really have no idea.

I think there's a disconnect somewhere in this discussion... maybe
we're just approaching this with different assumptions?

I think you recognize the potential for driver vulnerabilities when
binding to new or potentially hostile devices that may be spoofing
DID/VID/class/etc than then go on to abuse driver trust or the driver
using unvalidated inputs from the device to crash or run arbitrary
code on the target system.

Yes such drivers should be fixed, no doubt.  But without lots of
fuzzing (we're working on this) and testing we'd like to avoid
exposing that attack surface at all.

I think your suggestion to disable driver binding once the initial
bus/slot devices have been bound will probably work for this
situation.  I just wanted to be clear that without some auditing,
fuzzing, and additional testing, we simply have to assume that drivers
are *not* secure and avoid using them on untrusted devices until we're
fairly confident they can handle them (whether just misbehaving or
malicious), in combination with other approaches like IOMMUs of
course.  And this isn't because we don't trust driver authors or
kernel developers to dtrt, it's just that for many devices (maybe USB
is an exception) I think driver authors haven't had to consider this
case much, and so I think it's prudent to expect bugs in this area
that we need to find & fix.

Thanks,
Jesse


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-07 Thread Greg Kroah-Hartman
On Fri, Jun 05, 2020 at 06:08:28PM -0700, Rajat Jain wrote:
> Hello Greg,
> 
> Thank you for continuing to work with me through this.
> 
> On Fri, Jun 5, 2020 at 1:02 AM Greg Kroah-Hartman
>  wrote:
> >
> > 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
> > >  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 :)
> 
> Not sure if I understood. Can you please elaborate how that helps
> implement the policy I want?
> 
> >
> > But, think this through, since when do 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-05 Thread Rajat Jain
Hello Greg,

Thank you for continuing to work with me through this.

On Fri, Jun 5, 2020 at 1:02 AM Greg Kroah-Hartman
 wrote:
>
> 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
> >  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 :)

Not sure if I understood. Can you please elaborate how that helps
implement the policy I want?

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

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-05 Thread Greg Kroah-Hartman
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
>  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
> 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-04 Thread Rajat Jain
Hello,

I spent some more thoughts into this...

On Wed, Jun 3, 2020 at 5:16 AM Greg Kroah-Hartman
 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 = 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-03 Thread Greg Kroah-Hartman
On Wed, Jun 03, 2020 at 05:57:02AM -0700, Rajat Jain wrote:
> > 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.
> 
> Understood, will keep that in mind. I may first present a "PCI
> subsystem only" draft just to get a feel for it, since that is more
> familiar to me and also already has some bits and pieces we need for
> this.

Why?  Do it right the first time.  To waste reviewer's time on something
that you know you have to throw away and do it "correctly" again, is not
very nice.  I know I would put you on the bottom of my "patches to
review" list if you did that to me :)

thanks,

greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-03 Thread Rajat Jain
Hi Greg,

Thanks for looking into this thread.

On Wed, Jun 3, 2020 at 5:16 AM Greg Kroah-Hartman
 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.

Agree.

>
> > 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 controlling the
> devices and if a driver should bind to it or not.

Agree. That is precisely what this RFC had in mind: (1) controlling
whether a device is authorized and if so (2) What drivers can bind to
it.

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

Understood, will keep that in mind. I may first present a "PCI
subsystem only" draft just to get a feel for it, since that is more
familiar to me and also already has some bits and pieces we need for
this.

Thanks & Best Regards,

Rajat

>
> thanks,
>
> greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-03 Thread Greg Kroah-Hartman
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.

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


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-03 Thread Rajat Jain
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.
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. 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.

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?

Thanks & Best Regards,

Rajat

> thanks,
>
> greg k-h


Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-03 Thread Greg Kroah-Hartman
On Wed, Jun 03, 2020 at 02:27:33AM +, Rajat Jain wrote:
> On Mon, Jun 1, 2020 at 10:06 PM Greg Kroah-Hartman
>  wrote:
> >
> > On Mon, Jun 01, 2020 at 06:25:42PM -0500, Bjorn Helgaas wrote:
> > > [+cc Greg, linux-kernel for wider exposure]
> >
> > Thanks for the cc:, missed this...
> >
> > >
> > > On Tue, May 26, 2020 at 09:30:08AM -0700, Rajat Jain wrote:
> > > > On Thu, May 14, 2020 at 7:18 PM Rajat Jain  wrote:
> > > > > On Thu, May 14, 2020 at 12:13 PM Raj, Ashok  
> > > > > wrote:
> > > > > > On Wed, May 13, 2020 at 02:26:18PM -0700, Rajat Jain wrote:
> > > > > > > On Wed, May 13, 2020 at 8:19 AM Bjorn Helgaas 
> > > > > > >  wrote:
> > > > > > > > On Fri, May 01, 2020 at 04:07:10PM -0700, Rajat Jain wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > Currently, the PCI subsystem marks the PCI devices as 
> > > > > > > > > "untrusted", if
> > > > > > > > > the firmware asks it to:
> > > > > > > > >
> > > > > > > > > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > > > > > > > > 9cb30a71acd4 ("PCI: OF: Support "external-facing" property")
> > > > > > > > >
> > > > > > > > > An "untrusted" device indicates a (likely external facing) 
> > > > > > > > > device that
> > > > > > > > > may be malicious, and can trigger DMA attacks on the system. 
> > > > > > > > > It may
> > > > > > > > > also try to exploit any vulnerabilities exposed by the 
> > > > > > > > > driver, that
> > > > > > > > > may allow it to read/write unintended addresses in the host 
> > > > > > > > > (e.g. if
> > > > > > > > > DMA buffers for the device, share memory pages with other 
> > > > > > > > > driver data
> > > > > > > > > structures or code etc).
> > > > > > > > >
> > > > > > > > > High Level proposal
> > > > > > > > > ===
> > > > > > > > > Currently, the "untrusted" device property is used as a hint 
> > > > > > > > > to enable
> > > > > > > > > IOMMU restrictions (on Intel), disable ATS (on ARM) etc. We'd 
> > > > > > > > > like to
> > > > > > > > > go a step further, and allow the administrator to build a 
> > > > > > > > > list of
> > > > > > > > > whitelisted drivers for these "untrusted" devices. This 
> > > > > > > > > whitelist of
> > > > > > > > > drivers are the ones that he trusts enough to have little or 
> > > > > > > > > no
> > > > > > > > > vulnerabilities. (He may have built this list of whitelisted 
> > > > > > > > > drivers
> > > > > > > > > by a combination of code analysis of drivers, or by extensive 
> > > > > > > > > testing
> > > > > > > > > using PCIe fuzzing etc). We propose that the administrator be 
> > > > > > > > > allowed
> > > > > > > > > to specify this list of whitelisted drivers to the kernel, 
> > > > > > > > > and the PCI
> > > > > > > > > subsystem to impose this behavior:
> > > > > > > > >
> > > > > > > > > 1) The "untrusted" devices can bind to only "whitelisted 
> > > > > > > > > drivers".
> > > > > > > > > 2) The other devices (i.e. dev->untrusted=0) can bind to any 
> > > > > > > > > driver.
> > > > > > > > >
> > > > > > > > > Of course this behavior is to be imposed only if such a 
> > > > > > > > > whitelist is
> > > > > > > > > provided by the administrator.
> > > >
> > > > I haven't heard much on this proposal after the initial inputs (to
> > > > which I responded). Essentially, I agree that IO-MMU and ACS
> > > > restrictions need to be put in plcase. But I think we need this
> > > > additionally. Does this look acceptable to you? I wanted to start
> > > > spinning out the patches, but wanted to see if there are any pending
> > > > comments or concerns.
> > >
> > > I think it makes sense to code this up and see what it would look
> > > like.  The bare minimum seems like a driver "bind-to-external-devices"
> > > bit that's visible in sysfs plus something in the driver probe path
> > > that checks it.  Neither is inherently PCI-specific, but maybe the
> > > right place will become obvious when implementing it.
> 
> 
> Agree. I'll try to code it up.
> 
> My proposal became PCI specific because
> 
> * The need for my proposal arrived out of the potentially malicious
> *external* devices that can (NOW, with the advent of thunderbolt)
> directly DMA into the CPU memory space. PCI (enabled by Thunderbolt 3
> and USB4) is the only interface that fits the bill for laptops at
> least (There are few more interfaces that allow DMA directly into host
> memory such as LPC etc, but they are all internal so far).

Again, that fits in exactly with what USB does, so you should just steal
that code and move it into the driver core for all busses to use.

> > > I'm also not sure about the administrative details of this.  Certain
> > > versions of the driver may be trusted while others are untrusted.
> > > That would all have to be managed in userspace, so it's not really our
> > > problem, but it sounds like a hassle.  Putting the information in the
> > > driver itself would reduce that.
> 
> I agree to the points you are making. *
> 
> - Who do you think 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-02 Thread Rajat Jain
On Mon, Jun 1, 2020 at 10:06 PM Greg Kroah-Hartman
 wrote:
>
> On Mon, Jun 01, 2020 at 06:25:42PM -0500, Bjorn Helgaas wrote:
> > [+cc Greg, linux-kernel for wider exposure]
>
> Thanks for the cc:, missed this...
>
> >
> > On Tue, May 26, 2020 at 09:30:08AM -0700, Rajat Jain wrote:
> > > On Thu, May 14, 2020 at 7:18 PM Rajat Jain  wrote:
> > > > On Thu, May 14, 2020 at 12:13 PM Raj, Ashok  wrote:
> > > > > On Wed, May 13, 2020 at 02:26:18PM -0700, Rajat Jain wrote:
> > > > > > On Wed, May 13, 2020 at 8:19 AM Bjorn Helgaas  
> > > > > > wrote:
> > > > > > > On Fri, May 01, 2020 at 04:07:10PM -0700, Rajat Jain wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > Currently, the PCI subsystem marks the PCI devices as 
> > > > > > > > "untrusted", if
> > > > > > > > the firmware asks it to:
> > > > > > > >
> > > > > > > > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > > > > > > > 9cb30a71acd4 ("PCI: OF: Support "external-facing" property")
> > > > > > > >
> > > > > > > > An "untrusted" device indicates a (likely external facing) 
> > > > > > > > device that
> > > > > > > > may be malicious, and can trigger DMA attacks on the system. It 
> > > > > > > > may
> > > > > > > > also try to exploit any vulnerabilities exposed by the driver, 
> > > > > > > > that
> > > > > > > > may allow it to read/write unintended addresses in the host 
> > > > > > > > (e.g. if
> > > > > > > > DMA buffers for the device, share memory pages with other 
> > > > > > > > driver data
> > > > > > > > structures or code etc).
> > > > > > > >
> > > > > > > > High Level proposal
> > > > > > > > ===
> > > > > > > > Currently, the "untrusted" device property is used as a hint to 
> > > > > > > > enable
> > > > > > > > IOMMU restrictions (on Intel), disable ATS (on ARM) etc. We'd 
> > > > > > > > like to
> > > > > > > > go a step further, and allow the administrator to build a list 
> > > > > > > > of
> > > > > > > > whitelisted drivers for these "untrusted" devices. This 
> > > > > > > > whitelist of
> > > > > > > > drivers are the ones that he trusts enough to have little or no
> > > > > > > > vulnerabilities. (He may have built this list of whitelisted 
> > > > > > > > drivers
> > > > > > > > by a combination of code analysis of drivers, or by extensive 
> > > > > > > > testing
> > > > > > > > using PCIe fuzzing etc). We propose that the administrator be 
> > > > > > > > allowed
> > > > > > > > to specify this list of whitelisted drivers to the kernel, and 
> > > > > > > > the PCI
> > > > > > > > subsystem to impose this behavior:
> > > > > > > >
> > > > > > > > 1) The "untrusted" devices can bind to only "whitelisted 
> > > > > > > > drivers".
> > > > > > > > 2) The other devices (i.e. dev->untrusted=0) can bind to any 
> > > > > > > > driver.
> > > > > > > >
> > > > > > > > Of course this behavior is to be imposed only if such a 
> > > > > > > > whitelist is
> > > > > > > > provided by the administrator.
> > >
> > > I haven't heard much on this proposal after the initial inputs (to
> > > which I responded). Essentially, I agree that IO-MMU and ACS
> > > restrictions need to be put in plcase. But I think we need this
> > > additionally. Does this look acceptable to you? I wanted to start
> > > spinning out the patches, but wanted to see if there are any pending
> > > comments or concerns.
> >
> > I think it makes sense to code this up and see what it would look
> > like.  The bare minimum seems like a driver "bind-to-external-devices"
> > bit that's visible in sysfs plus something in the driver probe path
> > that checks it.  Neither is inherently PCI-specific, but maybe the
> > right place will become obvious when implementing it.


Agree. I'll try to code it up.

My proposal became PCI specific because

* The need for my proposal arrived out of the potentially malicious
*external* devices that can (NOW, with the advent of thunderbolt)
directly DMA into the CPU memory space. PCI (enabled by Thunderbolt 3
and USB4) is the only interface that fits the bill for laptops at
least (There are few more interfaces that allow DMA directly into host
memory such as LPC etc, but they are all internal so far).

* It hinges on the "untrusted" attribute (I see your concerns on this,
and more on this later) which is part of the "struct pci_dev". If we
can move that flag higher up to "struct device", then we can make this
proposal not PCI specific I think.

> >
> > I'm still not 100% sure the device "external/untrusted" bit is the
> > right thing to check.  If you don't trust a driver enough to expose it
> > to an external device, is it reasonable to trust it for internal
> > devices?  It seems like one could attack the driver of even an
> > internal device like a NIC by controlling the data fed to it.
> >
> > The existing use of "external/untrusted" for IOMMU protection is
> > different.  There we're acknowledging that the *device* itself is
> > unknown and we need to protect ourselves from malicious DMA.
> >
> > Here 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-01 Thread Greg Kroah-Hartman
On Mon, Jun 01, 2020 at 06:25:42PM -0500, Bjorn Helgaas wrote:
> [+cc Greg, linux-kernel for wider exposure]

Thanks for the cc:, missed this...

> 
> On Tue, May 26, 2020 at 09:30:08AM -0700, Rajat Jain wrote:
> > On Thu, May 14, 2020 at 7:18 PM Rajat Jain  wrote:
> > > On Thu, May 14, 2020 at 12:13 PM Raj, Ashok  wrote:
> > > > On Wed, May 13, 2020 at 02:26:18PM -0700, Rajat Jain wrote:
> > > > > On Wed, May 13, 2020 at 8:19 AM Bjorn Helgaas  
> > > > > wrote:
> > > > > > On Fri, May 01, 2020 at 04:07:10PM -0700, Rajat Jain wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Currently, the PCI subsystem marks the PCI devices as 
> > > > > > > "untrusted", if
> > > > > > > the firmware asks it to:
> > > > > > >
> > > > > > > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > > > > > > 9cb30a71acd4 ("PCI: OF: Support "external-facing" property")
> > > > > > >
> > > > > > > An "untrusted" device indicates a (likely external facing) device 
> > > > > > > that
> > > > > > > may be malicious, and can trigger DMA attacks on the system. It 
> > > > > > > may
> > > > > > > also try to exploit any vulnerabilities exposed by the driver, 
> > > > > > > that
> > > > > > > may allow it to read/write unintended addresses in the host (e.g. 
> > > > > > > if
> > > > > > > DMA buffers for the device, share memory pages with other driver 
> > > > > > > data
> > > > > > > structures or code etc).
> > > > > > >
> > > > > > > High Level proposal
> > > > > > > ===
> > > > > > > Currently, the "untrusted" device property is used as a hint to 
> > > > > > > enable
> > > > > > > IOMMU restrictions (on Intel), disable ATS (on ARM) etc. We'd 
> > > > > > > like to
> > > > > > > go a step further, and allow the administrator to build a list of
> > > > > > > whitelisted drivers for these "untrusted" devices. This whitelist 
> > > > > > > of
> > > > > > > drivers are the ones that he trusts enough to have little or no
> > > > > > > vulnerabilities. (He may have built this list of whitelisted 
> > > > > > > drivers
> > > > > > > by a combination of code analysis of drivers, or by extensive 
> > > > > > > testing
> > > > > > > using PCIe fuzzing etc). We propose that the administrator be 
> > > > > > > allowed
> > > > > > > to specify this list of whitelisted drivers to the kernel, and 
> > > > > > > the PCI
> > > > > > > subsystem to impose this behavior:
> > > > > > >
> > > > > > > 1) The "untrusted" devices can bind to only "whitelisted drivers".
> > > > > > > 2) The other devices (i.e. dev->untrusted=0) can bind to any 
> > > > > > > driver.
> > > > > > >
> > > > > > > Of course this behavior is to be imposed only if such a whitelist 
> > > > > > > is
> > > > > > > provided by the administrator.
> > 
> > I haven't heard much on this proposal after the initial inputs (to
> > which I responded). Essentially, I agree that IO-MMU and ACS
> > restrictions need to be put in plcase. But I think we need this
> > additionally. Does this look acceptable to you? I wanted to start
> > spinning out the patches, but wanted to see if there are any pending
> > comments or concerns.
> 
> I think it makes sense to code this up and see what it would look
> like.  The bare minimum seems like a driver "bind-to-external-devices"
> bit that's visible in sysfs plus something in the driver probe path
> that checks it.  Neither is inherently PCI-specific, but maybe the
> right place will become obvious when implementing it.
> 
> I'm still not 100% sure the device "external/untrusted" bit is the
> right thing to check.  If you don't trust a driver enough to expose it
> to an external device, is it reasonable to trust it for internal
> devices?  It seems like one could attack the driver of even an
> internal device like a NIC by controlling the data fed to it.  
> 
> The existing use of "external/untrusted" for IOMMU protection is
> different.  There we're acknowledging that the *device* itself is
> unknown and we need to protect ourselves from malicious DMA.
> 
> Here we're concerned about a *driver* that's completely under our
> control.  If we don't trust the driver, we could (a) fix the problems
> in the driver, (b) change the driver so it handles external/untrusted
> devices differently, or (c) not ship the driver at all.
> 
> I'm also not sure about the administrative details of this.  Certain
> versions of the driver may be trusted while others are untrusted.
> That would all have to be managed in userspace, so it's not really our
> problem, but it sounds like a hassle.  Putting the information in the
> driver itself would reduce that.

What about taking what we have today for USB devices/drivers where we
have different levels of "authorized"?  There's no reason that could not
just move to the driver core and be available for all devices/drivers as
that model has proven to work really well over time.

See the "authorized" sysfs file documentation in
Documentation/ABI/testing/sysfs-bus-usb for some details.  Lots of
userspace 

Re: [RFC] Restrict the untrusted devices, to bind to only a set of "whitelisted" drivers

2020-06-01 Thread Bjorn Helgaas
[+cc Greg, linux-kernel for wider exposure]

On Tue, May 26, 2020 at 09:30:08AM -0700, Rajat Jain wrote:
> On Thu, May 14, 2020 at 7:18 PM Rajat Jain  wrote:
> > On Thu, May 14, 2020 at 12:13 PM Raj, Ashok  wrote:
> > > On Wed, May 13, 2020 at 02:26:18PM -0700, Rajat Jain wrote:
> > > > On Wed, May 13, 2020 at 8:19 AM Bjorn Helgaas  
> > > > wrote:
> > > > > On Fri, May 01, 2020 at 04:07:10PM -0700, Rajat Jain wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Currently, the PCI subsystem marks the PCI devices as "untrusted", 
> > > > > > if
> > > > > > the firmware asks it to:
> > > > > >
> > > > > > 617654aae50e ("PCI / ACPI: Identify untrusted PCI devices")
> > > > > > 9cb30a71acd4 ("PCI: OF: Support "external-facing" property")
> > > > > >
> > > > > > An "untrusted" device indicates a (likely external facing) device 
> > > > > > that
> > > > > > may be malicious, and can trigger DMA attacks on the system. It may
> > > > > > also try to exploit any vulnerabilities exposed by the driver, that
> > > > > > may allow it to read/write unintended addresses in the host (e.g. if
> > > > > > DMA buffers for the device, share memory pages with other driver 
> > > > > > data
> > > > > > structures or code etc).
> > > > > >
> > > > > > High Level proposal
> > > > > > ===
> > > > > > Currently, the "untrusted" device property is used as a hint to 
> > > > > > enable
> > > > > > IOMMU restrictions (on Intel), disable ATS (on ARM) etc. We'd like 
> > > > > > to
> > > > > > go a step further, and allow the administrator to build a list of
> > > > > > whitelisted drivers for these "untrusted" devices. This whitelist of
> > > > > > drivers are the ones that he trusts enough to have little or no
> > > > > > vulnerabilities. (He may have built this list of whitelisted drivers
> > > > > > by a combination of code analysis of drivers, or by extensive 
> > > > > > testing
> > > > > > using PCIe fuzzing etc). We propose that the administrator be 
> > > > > > allowed
> > > > > > to specify this list of whitelisted drivers to the kernel, and the 
> > > > > > PCI
> > > > > > subsystem to impose this behavior:
> > > > > >
> > > > > > 1) The "untrusted" devices can bind to only "whitelisted drivers".
> > > > > > 2) The other devices (i.e. dev->untrusted=0) can bind to any driver.
> > > > > >
> > > > > > Of course this behavior is to be imposed only if such a whitelist is
> > > > > > provided by the administrator.
> 
> I haven't heard much on this proposal after the initial inputs (to
> which I responded). Essentially, I agree that IO-MMU and ACS
> restrictions need to be put in plcase. But I think we need this
> additionally. Does this look acceptable to you? I wanted to start
> spinning out the patches, but wanted to see if there are any pending
> comments or concerns.

I think it makes sense to code this up and see what it would look
like.  The bare minimum seems like a driver "bind-to-external-devices"
bit that's visible in sysfs plus something in the driver probe path
that checks it.  Neither is inherently PCI-specific, but maybe the
right place will become obvious when implementing it.

I'm still not 100% sure the device "external/untrusted" bit is the
right thing to check.  If you don't trust a driver enough to expose it
to an external device, is it reasonable to trust it for internal
devices?  It seems like one could attack the driver of even an
internal device like a NIC by controlling the data fed to it.  

The existing use of "external/untrusted" for IOMMU protection is
different.  There we're acknowledging that the *device* itself is
unknown and we need to protect ourselves from malicious DMA.

Here we're concerned about a *driver* that's completely under our
control.  If we don't trust the driver, we could (a) fix the problems
in the driver, (b) change the driver so it handles external/untrusted
devices differently, or (c) not ship the driver at all.

I'm also not sure about the administrative details of this.  Certain
versions of the driver may be trusted while others are untrusted.
That would all have to be managed in userspace, so it's not really our
problem, but it sounds like a hassle.  Putting the information in the
driver itself would reduce that.

> > > > > > Details
> > > > > > ==
> > > > > >
> > > > > > 1) A kernel argument ("pci.impose_driver_whitelisting") to enable
> > > > > > imposing of whitelisting by PCI subsystem.
> > > > > >
> > > > > > 2) Add a flag ("whitelisted") in struct pci_driver to indicate 
> > > > > > whether
> > > > > > the driver is whitelisted.
> > >
> > > I'm not sure if a driver certifying itself as secure is acceptable.
> > >
> > > Probably the pcie-component-authentication type mechanisms can establish
> > > proper root of trust. Othewise we are just hand waving and any method
> > > has its own gaps. You can probably say use the fuzzer etc, but that more
> > > falls in every adminstrator needs to run and qualify every device. Once 
> > > you
> > > have a firmware update