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 > >> >> Regardless of this is 5.18 or 5.19 material can you? : >> >> a) confirm that I've understood how the clearing vs cert-to-password >> works correctly ? > Confirmed :) > >> b) confirm that despite you using a test BIOS the WMI API for this is >> final and that it will *not* change before there are production >> BIOS-es with this ? > I think I'm safe confirming this (I always get slightly nervous as it's > outside my control, and weird things happen...) We have an internal spec for > this feature and this implementation is meeting that spec. I'd be very > surprised if there are extra changes. > >> c) submit a version to clarify the clearing vs cert-to-password thing, e.g.: >> >> @@ -276,6 +276,8 @@ Description: >> >> You cannot enable certificate authentication if a supervisor password >> has not been set. >> + Clearing the certificate results in no bios-admin authentication >> + method being configured allowing anyone to make changes. >> After any of these operations the system must reboot for the changes to >> take effect. >> > Ack will do > > Mark >
