Re: [PATCH v4 13/14] platform/x86: dell-smbios-wmi: introduce userspace interface

[Greg KH]

On Thu, Oct 05, 2017 at 05:56:19PM +0200, Pali Rohár wrote:
> On Thursday 05 October 2017 17:44:45 Greg KH wrote:
> > On Thu, Oct 05, 2017 at 02:22:37PM +0000, Mario.Limonciello@xxxxxxxx wrote:
> > > > -----Original Message-----
> > > > From: Alan Cox [mailto:gnomes@xxxxxxxxxxxxxxxxxxx]
> > > > Sent: Thursday, October 5, 2017 8:59 AM
> > > > To: Limonciello, Mario <Mario_Limonciello@xxxxxxxx>
> > > > Cc: dvhart@xxxxxxxxxxxxx; Andy Shevchenko <andy.shevchenko@xxxxxxxxx>;
> > > > LKML <linux-kernel@xxxxxxxxxxxxxxx>; platform-driver-x86@xxxxxxxxxxxxxxx;
> > > > Andy Lutomirski <luto@xxxxxxxxxx>; quasisec@xxxxxxxxxx;
> > > > pali.rohar@xxxxxxxxx; rjw@xxxxxxxxxxxxx; mjg59@xxxxxxxxxx; hch@xxxxxx; Greg
> > > > KH <greg@xxxxxxxxx>
> > > > Subject: Re: [PATCH v4 13/14] platform/x86: dell-smbios-wmi: introduce
> > > > userspace interface
> > > > 
> > > > On Wed,  4 Oct 2017 17:48:39 -0500
> > > > Mario Limonciello <mario.limonciello@xxxxxxxx> wrote:
> > > > 
> > > > > This userspace character device will be used to perform SMBIOS calls
> > > > > from any applications.
> > > > >
> > > > > It provides an ioctl that will allow passing the 32k WMI calling
> > > > > interface buffer between userspace and kernel space.
> > > > 
> > > > What is your security model for firing 32K of random crap at the BIOS ?
> > > 
> > > Adding new class and select methods requires a review with the security
> > > team.  They will do STRIDE analysis and threat modeling.
> > > 
> > > > Do you fuzz test the BIOS interface ?
> > > 
> > > Yes there has been internal fuzz testing classes and selects used in the 
> > > ACPI interface in the past.  I can't comment on how regularly that is done.
> > > I do think it's interesting is to use the interface in Linux for further fuzz
> > > testing though.
> > 
> > That should be simple, start firing random data at this memory location
> > and see what happens.  Can you brick the box?  Change the
> > manufactured-date?  Change the serial number?  Normally these types of
> > BIOS interfaces allow all sorts of "fun" things like this, which is why
> > we have the kernel "own" the interface, to protect yourself from
> > breaking the box.
> 
> There are also Dell calls to "permanently disable TPM" and similar which
> are irreversible. So this can be really a problem.
> 
> > > > How do we know that between now and the end of the universe every call is
> > > > safe to execute as any random user without upsetting other users on the
> > > > same PC ?
> > > 
> > > Any random user shouldn't be executing the ioctl.
> > > Only root should be executing any of these calls.
> > 
> > "only root" isn't the best protection method, you should know better :)
> > 
> > You are going to have to do some kind of parsing/whitelisting here,
> > trust us...
> 
> That is also why I in past suggested to create fully transparent
> whitelisting filter for all WMI calls. And properly implement particular
> filter in kernel...
> 
> But that is of course hard as you need to know all details of internal
> structures and data which may be sent via userspace. To prevent e.g.
> action "permanently disable TPM" in kernel.

It's not "hard" as userspace would have to know these same things if it
were to just have an clean "pipe" to the BIOS as it has to create and
parse the commands to get the BIOS to do anything.

So I agree with you, whitelisting seems to be the only sane solution,
unless people don't like their TPMs to be erased by root exploits?  :)

thanks,

greg k-h