Message-ID: <DM6PR19MB2636626A94385EDC7C0CACF9FA3E0@DM6PR19MB2636.namprd19.prod.outlook.com>
Date:   Thu, 17 Sep 2020 16:18:21 +0000
From:   "Limonciello, Mario" <Mario.Limonciello@...l.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        Divya Bharathi <divya27392@...il.com>,
        "dvhart@...radead.org" <dvhart@...radead.org>
CC:     LKML <linux-kernel@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "Bharathi, Divya" <Divya.Bharathi@...l.com>,
        "Ksr, Prasanth" <Prasanth.Ksr@...l.com>,
        Richard Hughes <rhughes@...hat.com>,
        Jared Dominguez <jaredz@...hat.com>
Subject: RE: [PATCH] Introduce support for Systems Management Driver over WMI
 for Dell Systems

> > Those are very different flows to get to and change the same "types" of
> data.  By Dell's interface
> > being Dell specific we can guarantee that the documented flow works how it
> should.
> 
> Documenting the flow could be part of the documentation for the
> different passwd types. 

In the concept of a "generic" API I don't think the word "password" is futureproof
and it would need to be avoided.  I think a better term would be "authentication".

> For how things currently work the User and
> Admin password attributed would have a type of "password", the hash
> example you gave will have a different type for its password attribute,
> e.g. "hotp" and not only the type could be different but also
> the sysfs-attributes, e.g. the "Admin" password-dir which has a "type"
> sysfs-atrribute which returns "htop" may not have a current_password
> attribute at all, instead it may would have a hash attribute, making
> it (more) clear that this one works differently.

In our definition `current_password` is intentionally not readable by userspace.
One process could be writing it (think obtaining it from an escrow service) and
another interacting with attributes, and their threat models might not match.

Furthermore - what's to say multiple authentication schemes might not be
simultaneously supported and this needs to be expressed?  This can be a difference
between OEM implementations.

> 
> This would mean that existing userspace software can not work with
> systems using the new "hotp" password atrributes, but that is
> unavoidable.

> 
> I think that the current generic password flow will work well
> for other vendors too, they may need to not cache it in the
> kernel (instead sending it directly to the firmware once), but the basic
> concept of having to write the plain-text bios Admin password before
> being able to change protected settings seems like it is something which
> matches how most current BIOS-es work.  And needing a way to re-lock the
> settings also sounds like something which will be pretty common for most
> implementations.
> 

OK so here is another place that I think vendors might have a different
implementation.  When you have a BIOS admin password set, Dell requires that
password to interact with any of these attributes.  Another vendor might
only require it only for certain attributes they deemed protected.

So again, Dell's flow might not scale to everyone else.

I do acknowledge this might be mitigatable by adding a sysfs file to every
attribute for Dell's implementation "is_authentication_required" that is always
1 when admin password required and in another implementation an OEM might choose
to set that on a case by case basis.

> >> We could even do something like we have for .desktop files
> >> fields, where we add something to the sysfs ABI documentation
> >> that vendors may add vendor specific types prefixed with X-<vendorname>.
> >>
> >> So all in all I believe that we can make using the proposed sysfs ABI
> >> a generic one work, and that this will be worth it to avoid having the
> >> issue of eventually having both a couple of vendor specific APIs +
> >> one grant unified generic API replacing those vendor-APIs
> >> (where we can never drop the vendor specific APIs because of
> >> backward compat. guarantees).
> >
> > I'm personally leaning on the right place to have a vendor agnostic view is
> "outside"
> > of the kernel in an userland library.  All the vendor drivers that change
> settings can
> > behave very similarly for the most part, but differences between vendor
> implementations
> > can be better expressed there.
> 
> We have tried that before in several cases (that I'm aware of) and this never
> works out. Esp. not when the basic kernel interface is reasonable sane,
> then a lot of people / projects avoid the lib and just poke the kernel API
> directly. We have seen this e.g. with hwmon-class devices and with v4l devices
> and with backlight-class devices. Since I've seen this happen 3 times already
> I'm not a big believer in adding a userspace library to hide the vendor
> differences.
> 
> Regards,
> 
> Hans

Another area that comes to mind is Dell's value_modifier and modifier rules.  This
dependency logic is handled and expressed by the firmware.  You'll notice the Dell
driver only displays the information that came out of the firmware in sysfs and doesn't
do any processing in driver.

So by using Dell's format, another vendor's driver will need to follow Dell's
formatting and rule validation/generation which their firmware might not support and
they will be forced to implement Dell's schema in their kernel driver.

Lastly I want to caution that individual firmware items with the same name might have
a different meaning across vendors.  Here is my hypothetical example:

Dell has an attribute called "Camera"  With V3 it populates under:
/sys/devices/platform/dell-wmi-sysman/attributes/Camera

The description sysfs for it reads as "Enable Camera" and it's possible values are
"Disabled;Enabled;".  For Dell this is pretty obviously it turns on and off the camera
functionality.

For another vendor they might actually not offer to enable/disable the camera but instead
To enable the control of an electromagnetic camera shutter from such an attribute. 
Their attribute could still be called "Camera" but the description might read as
"Enable camera shutter control".  For them it would still read as "Disabled;Enabled;"
for possible values but have a completely different meaning!

There is no standard for this, and again userspace will need to basically look at
the directory and structure to figure out what the meaning actually is.
 

Powered by blists - more mailing lists