| Message ID | 20241021193837.7641-4-mpearson-lenovo@squebb.ca (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/4] platform/x86: think-lmi: improve check if BIOS account security enabled | expand |
On Mon, 21 Oct 2024, Mark Pearson wrote: > Lenovo are adding support for both Admin and System certificates to > the certificate based authentication feature > > This commit adds the support for this. > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > --- > .../testing/sysfs-class-firmware-attributes | 1 + > drivers/platform/x86/think-lmi.c | 141 ++++++++++++++---- > drivers/platform/x86/think-lmi.h | 4 + > 3 files changed, 116 insertions(+), 30 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-class-firmware-attributes b/Documentation/ABI/testing/sysfs-class-firmware-attributes > index 1a8b59f5d6e3..2713efa509b4 100644 > --- a/Documentation/ABI/testing/sysfs-class-firmware-attributes > +++ b/Documentation/ABI/testing/sysfs-class-firmware-attributes > @@ -303,6 +303,7 @@ Description: > being configured allowing anyone to make changes. > After any of these operations the system must reboot for the changes to > take effect. > + Admin and System certificates are supported from 2025 systems onward. > > certificate_thumbprint: > Read only attribute used to display the MD5, SHA1 and SHA256 thumbprints > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 751e351dfc42..fca190232c24 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -169,11 +169,12 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support"); > */ > #define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4" > > -#define TLMI_POP_PWD BIT(0) /* Supervisor */ > -#define TLMI_PAP_PWD BIT(1) /* Power-on */ > -#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ > -#define TLMI_SMP_PWD BIT(6) /* System Management */ > -#define TLMI_CERT BIT(7) /* Certificate Based */ > +#define TLMI_POP_PWD BIT(0) /* Supervisor */ > +#define TLMI_PAP_PWD BIT(1) /* Power-on */ > +#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ > +#define TLMI_SMP_PWD BIT(6) /* System Management */ > +#define TLMI_CERT_SVC BIT(7) /* Admin Certificate Based */ > +#define TLMI_CERT_SMC BIT(8) /* System Certificate Based */ > > static const struct tlmi_err_codes tlmi_errs[] = { > {"Success", 0}, > @@ -678,18 +679,35 @@ static ssize_t cert_thumbprint(char *buf, const char *arg, int count) > return count; > } > > +#define NUM_THUMBTYPES 3 > +static char *thumbtypes[NUM_THUMBTYPES] = {"Md5", "Sha1", "Sha256"}; > + > static ssize_t certificate_thumbprint_show(struct kobject *kobj, struct kobj_attribute *attr, > char *buf) > { > struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); > - int count = 0; > + char *wmistr; > + int count = 0, i; Reverse xmas-tree order. > > if (!tlmi_priv.certificate_support || !setting->cert_installed) > return -EOPNOTSUPP; > > - count += cert_thumbprint(buf, "Md5", count); > - count += cert_thumbprint(buf, "Sha1", count); > - count += cert_thumbprint(buf, "Sha256", count); > + for (i = 0; i < NUM_THUMBTYPES; i++) { Use ARRAY_SIZE() + add include for it. These days, the usual custom is to use unsigned int for loop variables that are never meant to be negative. > + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { > + /* Format: 'SVC | SMC, Thumbtype' */ > + wmistr = kasprintf(GFP_KERNEL, "%s,%s", > + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", > + thumbtypes[i]); > + } else { > + /* Format: 'Thumbtype' */ > + wmistr = kasprintf(GFP_KERNEL, "%s", thumbtypes[i]); > + } > + if (!wmistr) > + return -ENOMEM; > + count += cert_thumbprint(buf, wmistr, count); > + kfree(wmistr); > + } > + > return count; > } > > @@ -720,8 +738,15 @@ static ssize_t cert_to_password_store(struct kobject *kobj, > if (!passwd) > return -ENOMEM; > > - /* Format: 'Password,Signature' */ > - auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); > + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { > + /* Format: 'SVC | SMC, password, signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", > + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", > + passwd, setting->signature); > + } else { > + /* Format: 'Password,Signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); > + } > if (!auth_str) { > kfree_sensitive(passwd); > return -ENOMEM; > @@ -735,12 +760,19 @@ static ssize_t cert_to_password_store(struct kobject *kobj, > > static struct kobj_attribute auth_cert_to_password = __ATTR_WO(cert_to_password); > > +enum cert_install_mode { > + TLMI_CERT_INSTALL, > + TLMI_CERT_UPDATE, > +}; > + > static ssize_t certificate_store(struct kobject *kobj, > struct kobj_attribute *attr, > const char *buf, size_t count) > { > struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); > + enum cert_install_mode install_mode = TLMI_CERT_INSTALL; > char *auth_str, *new_cert; > + char *signature; > char *guid; > int ret; > > @@ -756,10 +788,18 @@ static ssize_t certificate_store(struct kobject *kobj, > if (!setting->signature || !setting->signature[0]) > return -EACCES; > > - /* Format: 'serial#, signature' */ > - auth_str = kasprintf(GFP_KERNEL, "%s,%s", > - dmi_get_system_info(DMI_PRODUCT_SERIAL), > - setting->signature); > + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { > + /* Format: 'SVC | SMC, serial#, signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", > + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", > + dmi_get_system_info(DMI_PRODUCT_SERIAL), > + setting->signature); > + } else { > + /* Format: 'serial#, signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s", > + dmi_get_system_info(DMI_PRODUCT_SERIAL), > + setting->signature); > + } > if (!auth_str) > return -ENOMEM; > > @@ -776,24 +816,59 @@ static ssize_t certificate_store(struct kobject *kobj, > > if (setting->cert_installed) { > /* Certificate is installed so this is an update */ > - if (!setting->signature || !setting->signature[0]) { > + install_mode = TLMI_CERT_UPDATE; > + /* If admin account enabled - need to use it's signature */ > + if (tlmi_priv.pwd_admin->pwd_enabled) > + signature = tlmi_priv.pwd_admin->signature; > + else > + signature = setting->signature; > + } else { /* Cert install */ > + /* Check if SMC and SVC already installed */ > + if ((setting == tlmi_priv.pwd_system) && tlmi_priv.pwd_admin->cert_installed) { > + /* This gets treated as a cert update */ > + install_mode = TLMI_CERT_UPDATE; > + signature = tlmi_priv.pwd_admin->signature; > + } else { /* Regular cert install */ > + install_mode = TLMI_CERT_INSTALL; > + signature = setting->signature; > + } > + } > + > + if (install_mode == TLMI_CERT_UPDATE) { > + /* This is a certificate update */ > + if (!signature || !signature[0]) { > kfree(new_cert); > return -EACCES; > } > guid = LENOVO_UPDATE_BIOS_CERT_GUID; > - /* Format: 'Certificate,Signature' */ > - auth_str = kasprintf(GFP_KERNEL, "%s,%s", > - new_cert, setting->signature); > + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { > + /* Format: 'SVC | SMC, certificate, signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", > + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", > + new_cert, signature); > + } else { > + /* Format: 'Certificate,Signature' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s", > + new_cert, signature); > + } > } else { > /* This is a fresh install */ > - if (!setting->pwd_enabled || !setting->password[0]) { > + /* To set admin cert, a password must be enabled */ > + if ((setting == tlmi_priv.pwd_admin) && > + (!setting->pwd_enabled || !setting->password[0])) { > kfree(new_cert); > return -EACCES; > } > guid = LENOVO_SET_BIOS_CERT_GUID; > - /* Format: 'Certificate,Admin-password' */ > - auth_str = kasprintf(GFP_KERNEL, "%s,%s", > - new_cert, setting->password); > + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { > + /* Format: 'SVC | SMC, Certificate, password' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", > + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", > + new_cert, setting->password); > + } else { > + /* Format: 'Certificate, password' */ > + auth_str = kasprintf(GFP_KERNEL, "%s,%s", new_cert, setting->password); > + } Have you considered creating a helper for this if/else construct to create the string? (you've basically repeated it multiple times by now with limited change to only parameters AFAICT). > } > kfree(new_cert); > if (!auth_str) > @@ -873,14 +948,19 @@ static umode_t auth_attr_is_visible(struct kobject *kobj, > return 0; > } > > - /* We only display certificates on Admin account, if supported */ > + /* We only display certificates, if supported */ > if (attr == &auth_certificate.attr || > attr == &auth_signature.attr || > attr == &auth_save_signature.attr || > attr == &auth_cert_thumb.attr || > attr == &auth_cert_to_password.attr) { > - if ((setting == tlmi_priv.pwd_admin) && tlmi_priv.certificate_support) > + if (tlmi_priv.certificate_support) { > + if (setting == tlmi_priv.pwd_admin) These two if()s combined look the same as the old ode, why are you redoing it? > + return attr->mode; > + if ((tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) && > + (setting == tlmi_priv.pwd_system)) > return attr->mode; > + } > return 0; > } > > @@ -1700,12 +1780,13 @@ static int tlmi_analyze(void) > } > } > > - if (tlmi_priv.certificate_support && > - (tlmi_priv.pwdcfg.core.password_state & TLMI_CERT)) > - tlmi_priv.pwd_admin->cert_installed = true; > - > + if (tlmi_priv.certificate_support) { > + tlmi_priv.pwd_admin->cert_installed = > + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SVC; > + tlmi_priv.pwd_system->cert_installed = > + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SMC; > + } > return 0; > - Stray change.
Hi Ilpo, Thanks for the review. On Tue, Oct 22, 2024, at 4:14 AM, Ilpo Järvinen wrote: > On Mon, 21 Oct 2024, Mark Pearson wrote: > >> Lenovo are adding support for both Admin and System certificates to >> the certificate based authentication feature >> >> This commit adds the support for this. >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> --- >> .../testing/sysfs-class-firmware-attributes | 1 + >> drivers/platform/x86/think-lmi.c | 141 ++++++++++++++---- >> drivers/platform/x86/think-lmi.h | 4 + >> 3 files changed, 116 insertions(+), 30 deletions(-) >> >> diff --git a/Documentation/ABI/testing/sysfs-class-firmware-attributes b/Documentation/ABI/testing/sysfs-class-firmware-attributes >> index 1a8b59f5d6e3..2713efa509b4 100644 >> --- a/Documentation/ABI/testing/sysfs-class-firmware-attributes >> +++ b/Documentation/ABI/testing/sysfs-class-firmware-attributes >> @@ -303,6 +303,7 @@ Description: >> being configured allowing anyone to make changes. >> After any of these operations the system must reboot for the changes to >> take effect. >> + Admin and System certificates are supported from 2025 systems onward. >> >> certificate_thumbprint: >> Read only attribute used to display the MD5, SHA1 and SHA256 thumbprints >> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c >> index 751e351dfc42..fca190232c24 100644 >> --- a/drivers/platform/x86/think-lmi.c >> +++ b/drivers/platform/x86/think-lmi.c >> @@ -169,11 +169,12 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support"); >> */ >> #define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4" >> >> -#define TLMI_POP_PWD BIT(0) /* Supervisor */ >> -#define TLMI_PAP_PWD BIT(1) /* Power-on */ >> -#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ >> -#define TLMI_SMP_PWD BIT(6) /* System Management */ >> -#define TLMI_CERT BIT(7) /* Certificate Based */ >> +#define TLMI_POP_PWD BIT(0) /* Supervisor */ >> +#define TLMI_PAP_PWD BIT(1) /* Power-on */ >> +#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ >> +#define TLMI_SMP_PWD BIT(6) /* System Management */ >> +#define TLMI_CERT_SVC BIT(7) /* Admin Certificate Based */ >> +#define TLMI_CERT_SMC BIT(8) /* System Certificate Based */ >> >> static const struct tlmi_err_codes tlmi_errs[] = { >> {"Success", 0}, >> @@ -678,18 +679,35 @@ static ssize_t cert_thumbprint(char *buf, const char *arg, int count) >> return count; >> } >> >> +#define NUM_THUMBTYPES 3 >> +static char *thumbtypes[NUM_THUMBTYPES] = {"Md5", "Sha1", "Sha256"}; >> + >> static ssize_t certificate_thumbprint_show(struct kobject *kobj, struct kobj_attribute *attr, >> char *buf) >> { >> struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); >> - int count = 0; >> + char *wmistr; >> + int count = 0, i; > > Reverse xmas-tree order. Ack > >> >> if (!tlmi_priv.certificate_support || !setting->cert_installed) >> return -EOPNOTSUPP; >> >> - count += cert_thumbprint(buf, "Md5", count); >> - count += cert_thumbprint(buf, "Sha1", count); >> - count += cert_thumbprint(buf, "Sha256", count); >> + for (i = 0; i < NUM_THUMBTYPES; i++) { > > Use ARRAY_SIZE() + add include for it. > > These days, the usual custom is to use unsigned int for loop variables > that are never meant to be negative. > Will update. >> + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { >> + /* Format: 'SVC | SMC, Thumbtype' */ >> + wmistr = kasprintf(GFP_KERNEL, "%s,%s", >> + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", >> + thumbtypes[i]); >> + } else { >> + /* Format: 'Thumbtype' */ >> + wmistr = kasprintf(GFP_KERNEL, "%s", thumbtypes[i]); >> + } >> + if (!wmistr) >> + return -ENOMEM; >> + count += cert_thumbprint(buf, wmistr, count); >> + kfree(wmistr); >> + } >> + >> return count; >> } >> >> @@ -720,8 +738,15 @@ static ssize_t cert_to_password_store(struct kobject *kobj, >> if (!passwd) >> return -ENOMEM; >> >> - /* Format: 'Password,Signature' */ >> - auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); >> + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { >> + /* Format: 'SVC | SMC, password, signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", >> + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", >> + passwd, setting->signature); >> + } else { >> + /* Format: 'Password,Signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); >> + } >> if (!auth_str) { >> kfree_sensitive(passwd); >> return -ENOMEM; >> @@ -735,12 +760,19 @@ static ssize_t cert_to_password_store(struct kobject *kobj, >> >> static struct kobj_attribute auth_cert_to_password = __ATTR_WO(cert_to_password); >> >> +enum cert_install_mode { >> + TLMI_CERT_INSTALL, >> + TLMI_CERT_UPDATE, >> +}; >> + >> static ssize_t certificate_store(struct kobject *kobj, >> struct kobj_attribute *attr, >> const char *buf, size_t count) >> { >> struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); >> + enum cert_install_mode install_mode = TLMI_CERT_INSTALL; >> char *auth_str, *new_cert; >> + char *signature; >> char *guid; >> int ret; >> >> @@ -756,10 +788,18 @@ static ssize_t certificate_store(struct kobject *kobj, >> if (!setting->signature || !setting->signature[0]) >> return -EACCES; >> >> - /* Format: 'serial#, signature' */ >> - auth_str = kasprintf(GFP_KERNEL, "%s,%s", >> - dmi_get_system_info(DMI_PRODUCT_SERIAL), >> - setting->signature); >> + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { >> + /* Format: 'SVC | SMC, serial#, signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", >> + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", >> + dmi_get_system_info(DMI_PRODUCT_SERIAL), >> + setting->signature); >> + } else { >> + /* Format: 'serial#, signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s", >> + dmi_get_system_info(DMI_PRODUCT_SERIAL), >> + setting->signature); >> + } >> if (!auth_str) >> return -ENOMEM; >> >> @@ -776,24 +816,59 @@ static ssize_t certificate_store(struct kobject *kobj, >> >> if (setting->cert_installed) { >> /* Certificate is installed so this is an update */ >> - if (!setting->signature || !setting->signature[0]) { >> + install_mode = TLMI_CERT_UPDATE; >> + /* If admin account enabled - need to use it's signature */ >> + if (tlmi_priv.pwd_admin->pwd_enabled) >> + signature = tlmi_priv.pwd_admin->signature; >> + else >> + signature = setting->signature; >> + } else { /* Cert install */ >> + /* Check if SMC and SVC already installed */ >> + if ((setting == tlmi_priv.pwd_system) && tlmi_priv.pwd_admin->cert_installed) { >> + /* This gets treated as a cert update */ >> + install_mode = TLMI_CERT_UPDATE; >> + signature = tlmi_priv.pwd_admin->signature; >> + } else { /* Regular cert install */ >> + install_mode = TLMI_CERT_INSTALL; >> + signature = setting->signature; >> + } >> + } >> + >> + if (install_mode == TLMI_CERT_UPDATE) { >> + /* This is a certificate update */ >> + if (!signature || !signature[0]) { >> kfree(new_cert); >> return -EACCES; >> } >> guid = LENOVO_UPDATE_BIOS_CERT_GUID; >> - /* Format: 'Certificate,Signature' */ >> - auth_str = kasprintf(GFP_KERNEL, "%s,%s", >> - new_cert, setting->signature); >> + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { >> + /* Format: 'SVC | SMC, certificate, signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", >> + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", >> + new_cert, signature); >> + } else { >> + /* Format: 'Certificate,Signature' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s", >> + new_cert, signature); >> + } >> } else { >> /* This is a fresh install */ >> - if (!setting->pwd_enabled || !setting->password[0]) { >> + /* To set admin cert, a password must be enabled */ >> + if ((setting == tlmi_priv.pwd_admin) && >> + (!setting->pwd_enabled || !setting->password[0])) { >> kfree(new_cert); >> return -EACCES; >> } >> guid = LENOVO_SET_BIOS_CERT_GUID; >> - /* Format: 'Certificate,Admin-password' */ >> - auth_str = kasprintf(GFP_KERNEL, "%s,%s", >> - new_cert, setting->password); >> + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { >> + /* Format: 'SVC | SMC, Certificate, password' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", >> + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", >> + new_cert, setting->password); >> + } else { >> + /* Format: 'Certificate, password' */ >> + auth_str = kasprintf(GFP_KERNEL, "%s,%s", new_cert, setting->password); >> + } > > Have you considered creating a helper for this if/else construct to create > the string? (you've basically repeated it multiple times by now with > limited change to only parameters AFAICT). > I hadn't considered it, but yeah, it probably makes sense. Not completely sure how I would do it to be honest - if you've got any suggestions let me know, but I will look into it and see if I can improve it. >> } >> kfree(new_cert); >> if (!auth_str) >> @@ -873,14 +948,19 @@ static umode_t auth_attr_is_visible(struct kobject *kobj, >> return 0; >> } >> >> - /* We only display certificates on Admin account, if supported */ >> + /* We only display certificates, if supported */ >> if (attr == &auth_certificate.attr || >> attr == &auth_signature.attr || >> attr == &auth_save_signature.attr || >> attr == &auth_cert_thumb.attr || >> attr == &auth_cert_to_password.attr) { >> - if ((setting == tlmi_priv.pwd_admin) && tlmi_priv.certificate_support) >> + if (tlmi_priv.certificate_support) { >> + if (setting == tlmi_priv.pwd_admin) > > These two if()s combined look the same as the old ode, why are you redoing > it? My indenting here is somehow messed up...not sure how that happened (will fix). The block below should be indented too...and then I think it makes more sense. Let me know if you disagree. > >> + return attr->mode; >> + if ((tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) && >> + (setting == tlmi_priv.pwd_system)) >> return attr->mode; >> + } >> return 0; >> } >> >> @@ -1700,12 +1780,13 @@ static int tlmi_analyze(void) >> } >> } >> >> - if (tlmi_priv.certificate_support && >> - (tlmi_priv.pwdcfg.core.password_state & TLMI_CERT)) >> - tlmi_priv.pwd_admin->cert_installed = true; >> - >> + if (tlmi_priv.certificate_support) { >> + tlmi_priv.pwd_admin->cert_installed = >> + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SVC; >> + tlmi_priv.pwd_system->cert_installed = >> + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SMC; >> + } >> return 0; >> - > > Stray change. Oops. Will fix. > > -- > i. > >> fail_clear_attr: >> for (i = 0; i < TLMI_SETTINGS_COUNT; ++i) { >> if (tlmi_priv.setting[i]) { >> diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h >> index 4728f40143a3..f267d8b46957 100644 >> --- a/drivers/platform/x86/think-lmi.h >> +++ b/drivers/platform/x86/think-lmi.h >> @@ -41,6 +41,10 @@ enum save_mode { >> }; >> >> /* password configuration details */ >> +#define TLMI_PWDCFG_MODE_LEGACY 0 >> +#define TLMI_PWDCFG_MODE_PASSWORD 1 >> +#define TLMI_PWDCFG_MODE_MULTICERT 3 >> + >> struct tlmi_pwdcfg_core { >> uint32_t password_mode; >> uint32_t password_state; >> Mark
diff --git a/Documentation/ABI/testing/sysfs-class-firmware-attributes b/Documentation/ABI/testing/sysfs-class-firmware-attributes index 1a8b59f5d6e3..2713efa509b4 100644 --- a/Documentation/ABI/testing/sysfs-class-firmware-attributes +++ b/Documentation/ABI/testing/sysfs-class-firmware-attributes @@ -303,6 +303,7 @@ Description: being configured allowing anyone to make changes. After any of these operations the system must reboot for the changes to take effect. + Admin and System certificates are supported from 2025 systems onward. certificate_thumbprint: Read only attribute used to display the MD5, SHA1 and SHA256 thumbprints diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 751e351dfc42..fca190232c24 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -169,11 +169,12 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support"); */ #define LENOVO_CERT_THUMBPRINT_GUID "C59119ED-1C0D-4806-A8E9-59AA318176C4" -#define TLMI_POP_PWD BIT(0) /* Supervisor */ -#define TLMI_PAP_PWD BIT(1) /* Power-on */ -#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ -#define TLMI_SMP_PWD BIT(6) /* System Management */ -#define TLMI_CERT BIT(7) /* Certificate Based */ +#define TLMI_POP_PWD BIT(0) /* Supervisor */ +#define TLMI_PAP_PWD BIT(1) /* Power-on */ +#define TLMI_HDD_PWD BIT(2) /* HDD/NVME */ +#define TLMI_SMP_PWD BIT(6) /* System Management */ +#define TLMI_CERT_SVC BIT(7) /* Admin Certificate Based */ +#define TLMI_CERT_SMC BIT(8) /* System Certificate Based */ static const struct tlmi_err_codes tlmi_errs[] = { {"Success", 0}, @@ -678,18 +679,35 @@ static ssize_t cert_thumbprint(char *buf, const char *arg, int count) return count; } +#define NUM_THUMBTYPES 3 +static char *thumbtypes[NUM_THUMBTYPES] = {"Md5", "Sha1", "Sha256"}; + static ssize_t certificate_thumbprint_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); - int count = 0; + char *wmistr; + int count = 0, i; if (!tlmi_priv.certificate_support || !setting->cert_installed) return -EOPNOTSUPP; - count += cert_thumbprint(buf, "Md5", count); - count += cert_thumbprint(buf, "Sha1", count); - count += cert_thumbprint(buf, "Sha256", count); + for (i = 0; i < NUM_THUMBTYPES; i++) { + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { + /* Format: 'SVC | SMC, Thumbtype' */ + wmistr = kasprintf(GFP_KERNEL, "%s,%s", + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", + thumbtypes[i]); + } else { + /* Format: 'Thumbtype' */ + wmistr = kasprintf(GFP_KERNEL, "%s", thumbtypes[i]); + } + if (!wmistr) + return -ENOMEM; + count += cert_thumbprint(buf, wmistr, count); + kfree(wmistr); + } + return count; } @@ -720,8 +738,15 @@ static ssize_t cert_to_password_store(struct kobject *kobj, if (!passwd) return -ENOMEM; - /* Format: 'Password,Signature' */ - auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { + /* Format: 'SVC | SMC, password, signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", + passwd, setting->signature); + } else { + /* Format: 'Password,Signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s", passwd, setting->signature); + } if (!auth_str) { kfree_sensitive(passwd); return -ENOMEM; @@ -735,12 +760,19 @@ static ssize_t cert_to_password_store(struct kobject *kobj, static struct kobj_attribute auth_cert_to_password = __ATTR_WO(cert_to_password); +enum cert_install_mode { + TLMI_CERT_INSTALL, + TLMI_CERT_UPDATE, +}; + static ssize_t certificate_store(struct kobject *kobj, struct kobj_attribute *attr, const char *buf, size_t count) { struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj); + enum cert_install_mode install_mode = TLMI_CERT_INSTALL; char *auth_str, *new_cert; + char *signature; char *guid; int ret; @@ -756,10 +788,18 @@ static ssize_t certificate_store(struct kobject *kobj, if (!setting->signature || !setting->signature[0]) return -EACCES; - /* Format: 'serial#, signature' */ - auth_str = kasprintf(GFP_KERNEL, "%s,%s", - dmi_get_system_info(DMI_PRODUCT_SERIAL), - setting->signature); + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { + /* Format: 'SVC | SMC, serial#, signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", + dmi_get_system_info(DMI_PRODUCT_SERIAL), + setting->signature); + } else { + /* Format: 'serial#, signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s", + dmi_get_system_info(DMI_PRODUCT_SERIAL), + setting->signature); + } if (!auth_str) return -ENOMEM; @@ -776,24 +816,59 @@ static ssize_t certificate_store(struct kobject *kobj, if (setting->cert_installed) { /* Certificate is installed so this is an update */ - if (!setting->signature || !setting->signature[0]) { + install_mode = TLMI_CERT_UPDATE; + /* If admin account enabled - need to use it's signature */ + if (tlmi_priv.pwd_admin->pwd_enabled) + signature = tlmi_priv.pwd_admin->signature; + else + signature = setting->signature; + } else { /* Cert install */ + /* Check if SMC and SVC already installed */ + if ((setting == tlmi_priv.pwd_system) && tlmi_priv.pwd_admin->cert_installed) { + /* This gets treated as a cert update */ + install_mode = TLMI_CERT_UPDATE; + signature = tlmi_priv.pwd_admin->signature; + } else { /* Regular cert install */ + install_mode = TLMI_CERT_INSTALL; + signature = setting->signature; + } + } + + if (install_mode == TLMI_CERT_UPDATE) { + /* This is a certificate update */ + if (!signature || !signature[0]) { kfree(new_cert); return -EACCES; } guid = LENOVO_UPDATE_BIOS_CERT_GUID; - /* Format: 'Certificate,Signature' */ - auth_str = kasprintf(GFP_KERNEL, "%s,%s", - new_cert, setting->signature); + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { + /* Format: 'SVC | SMC, certificate, signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", + new_cert, signature); + } else { + /* Format: 'Certificate,Signature' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s", + new_cert, signature); + } } else { /* This is a fresh install */ - if (!setting->pwd_enabled || !setting->password[0]) { + /* To set admin cert, a password must be enabled */ + if ((setting == tlmi_priv.pwd_admin) && + (!setting->pwd_enabled || !setting->password[0])) { kfree(new_cert); return -EACCES; } guid = LENOVO_SET_BIOS_CERT_GUID; - /* Format: 'Certificate,Admin-password' */ - auth_str = kasprintf(GFP_KERNEL, "%s,%s", - new_cert, setting->password); + if (tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) { + /* Format: 'SVC | SMC, Certificate, password' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s", + setting == tlmi_priv.pwd_admin ? "SVC" : "SMC", + new_cert, setting->password); + } else { + /* Format: 'Certificate, password' */ + auth_str = kasprintf(GFP_KERNEL, "%s,%s", new_cert, setting->password); + } } kfree(new_cert); if (!auth_str) @@ -873,14 +948,19 @@ static umode_t auth_attr_is_visible(struct kobject *kobj, return 0; } - /* We only display certificates on Admin account, if supported */ + /* We only display certificates, if supported */ if (attr == &auth_certificate.attr || attr == &auth_signature.attr || attr == &auth_save_signature.attr || attr == &auth_cert_thumb.attr || attr == &auth_cert_to_password.attr) { - if ((setting == tlmi_priv.pwd_admin) && tlmi_priv.certificate_support) + if (tlmi_priv.certificate_support) { + if (setting == tlmi_priv.pwd_admin) + return attr->mode; + if ((tlmi_priv.pwdcfg.core.password_mode >= TLMI_PWDCFG_MODE_MULTICERT) && + (setting == tlmi_priv.pwd_system)) return attr->mode; + } return 0; } @@ -1700,12 +1780,13 @@ static int tlmi_analyze(void) } } - if (tlmi_priv.certificate_support && - (tlmi_priv.pwdcfg.core.password_state & TLMI_CERT)) - tlmi_priv.pwd_admin->cert_installed = true; - + if (tlmi_priv.certificate_support) { + tlmi_priv.pwd_admin->cert_installed = + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SVC; + tlmi_priv.pwd_system->cert_installed = + tlmi_priv.pwdcfg.core.password_state & TLMI_CERT_SMC; + } return 0; - fail_clear_attr: for (i = 0; i < TLMI_SETTINGS_COUNT; ++i) { if (tlmi_priv.setting[i]) { diff --git a/drivers/platform/x86/think-lmi.h b/drivers/platform/x86/think-lmi.h index 4728f40143a3..f267d8b46957 100644 --- a/drivers/platform/x86/think-lmi.h +++ b/drivers/platform/x86/think-lmi.h @@ -41,6 +41,10 @@ enum save_mode { }; /* password configuration details */ +#define TLMI_PWDCFG_MODE_LEGACY 0 +#define TLMI_PWDCFG_MODE_PASSWORD 1 +#define TLMI_PWDCFG_MODE_MULTICERT 3 + struct tlmi_pwdcfg_core { uint32_t password_mode; uint32_t password_state;
Lenovo are adding support for both Admin and System certificates to the certificate based authentication feature This commit adds the support for this. Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> --- .../testing/sysfs-class-firmware-attributes | 1 + drivers/platform/x86/think-lmi.c | 141 ++++++++++++++---- drivers/platform/x86/think-lmi.h | 4 + 3 files changed, 116 insertions(+), 30 deletions(-)