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@xxxxxxxxxx> >>> >>> 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. I'll test the next round of patches on that system for extra sanity. Mark