Re: [External] Re: [PATCH v2 1/2] Documentation: syfs-class-firmware-attributes: Lenovo Certificate support

From: Hans de Goede <hdegoede@redhat.com>
To: Mark Pearson <markpearson@lenovo.com>
Cc: markgross@kernel.org, platform-driver-x86@vger.kernel.org
Subject: Re: [External] Re: [PATCH v2 1/2] Documentation: syfs-class-firmware-attributes: Lenovo Certificate support
Date: Thu, 17 Mar 2022 19:16:42 +0100	[thread overview]
Message-ID: <78fd3e69-7c81-fe85-15ee-9e95a0ef2460@redhat.com> (raw)
In-Reply-To: <d938f450-2760-e7b3-b0e3-6006550269b5@lenovo.com>

Hi,

On 3/17/22 19:08, Mark Pearson wrote:
> 
> 
> 
> On 2022-03-17 13:23, Hans de Goede wrote:
>> Hi,
>>
>> On 3/17/22 18:08, Mark Pearson wrote:
>>>
>>> Hi Hans,
>>>
>>> Thanks for the review
>>>
>>> On 2022-03-17 06:58, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 3/15/22 20:56, Mark Pearson wrote:
>>>>> Certificate based authentication is available as an alternative to
>>>>> password based authentication.
>>>>>
>>>>> The WMI commands are cryptographically signed using a separate
>>>>> signing server and will be verified by the BIOS before being
>>>>> accepted.
>>>>>
>>>>> This commit details the fields that are needed to support that
>>>>> implementation. At present the changes are intended for Lenovo
>>>>> platforms, but have been designed to keep them as flexible as possible
>>>>> for future implementations from other vendors.
>>>>>
>>>>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
>>>>
>>>> This looks good, but looking at this a second time I still
>>>> have one open question:
>>>>
>>>> What is the difference between removing a certificate and
>>>> switching back to password auth?
>>> The main difference is clear goes to no-authentication, and switching
>>> obviously switches to password
>>>
>>>>
>>>> Looking at the WMI calls there are 4 different calls:
>>>>
>>>> LENOVO_SET_BIOS_CERT_GUID
>>>> LENOVO_UPDATE_BIOS_CERT_GUID
>>>> LENOVO_CLEAR_BIOS_CERT_GUI
>>>> LENOVO_CERT_TO_PASSWORD_GUID
>>>>
>>>> Going by these names I guess there can be only 1 certificate
>>>> otherwise I would expect:
>>>>
>>>> 1. add/remove naming
>>>> 2. update to take an id of which certificate to replace
>>>>
>>> Correct - there is only one certificate
>>>
>>>> So I guess that LENOVO_CLEAR_BIOS_CERT_GUI disables all
>>>> authentication. IOW, installing a cert replaces/clears
>>>> the supervisor password and the difference between
>>>> clearing the certificate and cert-to-password is that
>>>> after clearing it we end up with no supervisor password
>>>> set, where as cert-to-password sets the passed in password
>>>> as the new supervisor password?
>>> Correct
>>>
>>>>
>>>> Or does clearing the certificate fall back to the old
>>>> supervisor password if one was set?  (that might lead to
>>>> some interesting issues if users clear the certificate
>>>> many years after the password was last used ...)
>>> clearing reverts to no password
>>>
>>>>
>>>> Given where we are in the cycle I was planning on adding
>>>> this to my review-hans branch so that it could maybe still
>>>> get into 5.18, but given the above questions as well
>>>> the remark about the test X1 BIOS you are using I've
>>>> a feeling it would be better to give this some more time
>>>> to bake and target 5.19 instead. Do you agree ?
>>>
>>> I'd love to have it in 5.18 as I expect his feature to be available in
>>> our 2022 platforms and they're all going to start landing in the next
>>> couple of months. If that's unrealistic I can live with it so I'll defer
>>> to your preference
>>
>> The 5.18 merge window starts coming Monday, if you can get me
>> a v3 with the last few minor items addressed sometime tomorrow,
>> then I can throw it into my for-next branch and if it does not
>> cause any issues there then it can make 5.18.
>>
>> But if anything non trivial pops up while this is baking in -next
>> I'll probably drop it from -next and then this becomes 5.19 material.
>>
>> Regards,
>>
>> Hans
> 
> OK - sounds good :)
> As a note - the feature is in the release BIOS, I just checked on my X1
> Yoga 7 updated to the latest.

Ah, good to know that the BIOS side of this is released now,
that removes one worry about this.

> I'll test the next round of patches on
> that system for extra sanity.

Sounds good.

Regards,

Hans