Re: [PATCH v8 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
Hi Ilias,
On Fri, 11 Nov 2022 at 21:33, Ilias Apalodimas
wrote:
>
> Hello Kojima-san!
>
> [...]
>
> > + file_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
> > + if (!file_info.current_path) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out;
> > + }
> > +
> > + ret = eficonfig_process_select_file(_info);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = efi_open_volume_int(file_info.current_volume, );
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = efi_file_open_int(root, , file_info.current_path,
> > EFI_FILE_MODE_READ, 0);
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + ret = efi_file_size(f, );
> > + if (ret != EFI_SUCCESS)
> > + goto out;
> > +
> > + buf = calloc(1, size);
> > + if (!buf) {
> > + ret = EFI_OUT_OF_RESOURCES;
> > + goto out;
> > + }
> > + if (size == 0) {
> > + eficonfig_print_msg("ERROR! File is empty.");
> > + goto out;
> > + }
>
> This needs to move before the allocation and if (!size)
OK.
>
> > +
> > + ret = efi_file_read_int(f, , buf);
> > + if (ret != EFI_SUCCESS) {
> > + eficonfig_print_msg("ERROR! Failed to read file.");
> > + goto out;
> > + }
> > + if (!file_have_auth_header(buf, size)) {
> > + eficonfig_print_msg("ERROR! Invalid file format. Only .auth
> > variables is allowed.");
> > + ret = EFI_INVALID_PARAMETER;
> > + goto out;
> > + }
> > +
> > + attr = EFI_VARIABLE_NON_VOLATILE |
> > +EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > +EFI_VARIABLE_RUNTIME_ACCESS |
> > +EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> > +
> > + /* PK can enroll only one certificate */
> > + if (u16_strcmp(data, u"PK")) {
> > + efi_uintn_t db_size = 0;
> > +
> > + /* check the variable exists. If exists, add APPEND_WRITE
> > attribute */
> > + ret = efi_get_variable_int(data, efi_auth_var_get_guid(data),
> > NULL,
> > +_size, NULL, NULL);
> > + if (ret == EFI_BUFFER_TOO_SMALL)
> > + attr |= EFI_VARIABLE_APPEND_WRITE;
> > + }
> > +
> > + ret = efi_set_variable_int((u16 *)data, efi_auth_var_get_guid((u16
> > *)data),
> > +attr, size, buf, false);
> > + if (ret != EFI_SUCCESS)
> > + eficonfig_print_msg("ERROR! Failed to update signature
> > database");
> > +
> > +out:
> > + free(file_info.current_path);
> > + free(buf);
> > + if (f)
> > + efi_file_close_int(f);
> > +
> > + /* return to the parent menu */
> > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
>
> I assume we are changing the return message here because the upper layers
> can't handle EFI_ABORTED?
This return value change is tricky.
In eficonfig menu handling, EFI_ABORTED means "exit from the current
menu", it usually happens
by selecting "Quit" entry or pressing ESC key.
eficonfig menu implements a multi-layer menu, if the child menu
returns with EFI_ABORTED,
the parent menu also exit with EFI_ABORTED. This is why EFI_ABORTED is
changed to EFI_NOT_READY.
EFI_NOT_READY means that we stay in the current menu.
>
> > +
> > + return ret;
> > +}
> > +
> > +static struct eficonfig_item key_config_menu_items[] = {
> > + {"Enroll New Key", eficonfig_process_enroll_key},
> > + {"Quit", eficonfig_process_quit},
> > +};
> > +
>
> with the size changes
> Reviewed-by: Ilias Apalodimas
Thanks,
Masahisa Kojima
>
Re: [PATCH v8 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
Hello Kojima-san!
[...]
> + file_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE);
> + if (!file_info.current_path) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> +
> + ret = eficonfig_process_select_file(_info);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = efi_open_volume_int(file_info.current_volume, );
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = efi_file_open_int(root, , file_info.current_path,
> EFI_FILE_MODE_READ, 0);
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + ret = efi_file_size(f, );
> + if (ret != EFI_SUCCESS)
> + goto out;
> +
> + buf = calloc(1, size);
> + if (!buf) {
> + ret = EFI_OUT_OF_RESOURCES;
> + goto out;
> + }
> + if (size == 0) {
> + eficonfig_print_msg("ERROR! File is empty.");
> + goto out;
> + }
This needs to move before the allocation and if (!size)
> +
> + ret = efi_file_read_int(f, , buf);
> + if (ret != EFI_SUCCESS) {
> + eficonfig_print_msg("ERROR! Failed to read file.");
> + goto out;
> + }
> + if (!file_have_auth_header(buf, size)) {
> + eficonfig_print_msg("ERROR! Invalid file format. Only .auth
> variables is allowed.");
> + ret = EFI_INVALID_PARAMETER;
> + goto out;
> + }
> +
> + attr = EFI_VARIABLE_NON_VOLATILE |
> +EFI_VARIABLE_BOOTSERVICE_ACCESS |
> +EFI_VARIABLE_RUNTIME_ACCESS |
> +EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> +
> + /* PK can enroll only one certificate */
> + if (u16_strcmp(data, u"PK")) {
> + efi_uintn_t db_size = 0;
> +
> + /* check the variable exists. If exists, add APPEND_WRITE
> attribute */
> + ret = efi_get_variable_int(data, efi_auth_var_get_guid(data),
> NULL,
> +_size, NULL, NULL);
> + if (ret == EFI_BUFFER_TOO_SMALL)
> + attr |= EFI_VARIABLE_APPEND_WRITE;
> + }
> +
> + ret = efi_set_variable_int((u16 *)data, efi_auth_var_get_guid((u16
> *)data),
> +attr, size, buf, false);
> + if (ret != EFI_SUCCESS)
> + eficonfig_print_msg("ERROR! Failed to update signature
> database");
> +
> +out:
> + free(file_info.current_path);
> + free(buf);
> + if (f)
> + efi_file_close_int(f);
> +
> + /* return to the parent menu */
> + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
I assume we are changing the return message here because the upper layers
can't handle EFI_ABORTED?
> +
> + return ret;
> +}
> +
> +static struct eficonfig_item key_config_menu_items[] = {
> + {"Enroll New Key", eficonfig_process_enroll_key},
> + {"Quit", eficonfig_process_quit},
> +};
> +
with the size changes
Reviewed-by: Ilias Apalodimas
[PATCH v8 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
This commit adds the menu-driven UEFI Secure Boot Key
enrollment interface. User can enroll PK, KEK, db
and dbx by selecting file.
Only the signed EFI Signature List(s) with an authenticated
header, typically '.auth' file, is accepted.
To clear the PK, KEK, db and dbx, user needs to enroll the null key
signed by PK or KEK.
Signed-off-by: Masahisa Kojima
---
Changes in v8:
- fix missing efi_file_close_int() call
Changes in v7:
- only accept .auth file.
- remove creating time based authenticated variable
- update commit message
- use efi_file_size()
Changes in v6:
- use efi_secure_boot_enabled()
- replace with WIN_CERT_REVISION_2_0 from pe.h
- call efi_build_signature_store() to check the valid EFI Signature List
- update comment
Changes in v4:
- add CONFIG_EFI_MM_COMM_TEE dependency
- fix error handling
Changes in v3:
- fix error handling
Changes in v2:
- allow to enroll .esl file
- fix typos
- add function comments
cmd/Makefile | 5 +
cmd/eficonfig.c | 3 +
cmd/eficonfig_sbkey.c | 244 ++
include/efi_config.h | 5 +
4 files changed, 257 insertions(+)
create mode 100644 cmd/eficonfig_sbkey.c
diff --git a/cmd/Makefile b/cmd/Makefile
index 2444d116c0..0b6a96c1d9 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -66,6 +66,11 @@ obj-$(CONFIG_CMD_EEPROM) += eeprom.o
obj-$(CONFIG_EFI) += efi.o
obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
+ifdef CONFIG_CMD_EFICONFIG
+ifdef CONFIG_EFI_MM_COMM_TEE
+obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
+endif
+endif
obj-$(CONFIG_CMD_ELF) += elf.o
obj-$(CONFIG_CMD_EROFS) += erofs.o
obj-$(CONFIG_HUSH_PARSER) += exit.o
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 12babb76c2..d79194794b 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2435,6 +2435,9 @@ static const struct eficonfig_item
maintenance_menu_items[] = {
{"Edit Boot Option", eficonfig_process_edit_boot_option},
{"Change Boot Order", eficonfig_process_change_boot_order},
{"Delete Boot Option", eficonfig_process_delete_boot_option},
+#if (CONFIG_IS_ENABLED(EFI_SECURE_BOOT) && CONFIG_IS_ENABLED(EFI_MM_COMM_TEE))
+ {"Secure Boot Configuration", eficonfig_process_secure_boot_config},
+#endif
{"Quit", eficonfig_process_quit},
};
diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
new file mode 100644
index 00..de1d1e8426
--- /dev/null
+++ b/cmd/eficonfig_sbkey.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Menu-driven UEFI Secure Boot Key Maintenance
+ *
+ * Copyright (c) 2022 Masahisa Kojima, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum efi_sbkey_signature_type {
+ SIG_TYPE_X509 = 0,
+ SIG_TYPE_HASH,
+ SIG_TYPE_CRL,
+ SIG_TYPE_RSA2048,
+};
+
+struct eficonfig_sigtype_to_str {
+ efi_guid_t sig_type;
+ char *str;
+ enum efi_sbkey_signature_type type;
+};
+
+static const struct eficonfig_sigtype_to_str sigtype_to_str[] = {
+ {EFI_CERT_X509_GUID,"X509", SIG_TYPE_X509},
+ {EFI_CERT_SHA256_GUID, "SHA256", SIG_TYPE_HASH},
+ {EFI_CERT_X509_SHA256_GUID, "X509_SHA256 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA384_GUID, "X509_SHA384 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA512_GUID, "X509_SHA512 CRL", SIG_TYPE_CRL},
+ /* U-Boot does not support the following signature types */
+/* {EFI_CERT_RSA2048_GUID, "RSA2048",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_RSA2048_SHA256_GUID, "RSA2048_SHA256",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_SHA1_GUID,"SHA1", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_RSA2048_SHA_GUID, "RSA2048_SHA",
SIG_TYPE_RSA2048 }, */
+/* {EFI_CERT_SHA224_GUID, "SHA224", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_SHA384_GUID, "SHA384", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH},
*/
+};
+
+/**
+ * file_have_auth_header() - check file has EFI_VARIABLE_AUTHENTICATION_2
header
+ * @buf: pointer to file
+ * @size: file size
+ * Return: true if file has auth header, false otherwise
+ */
+static bool file_have_auth_header(void *buf, efi_uintn_t size)
+{
+ struct efi_variable_authentication_2 *auth = buf;
+
+ if (auth->auth_info.hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID)
+ return false;
+
+ if (guidcmp(>auth_info.cert_type, _guid_cert_type_pkcs7))
+ return false;
+
+ return true;
+}
+
+/**
+ * eficonfig_process_enroll_key() - enroll key into signature database
+ *
+ * @data: pointer to the data for each entry
+ * Return: status code
+ */
+static efi_status_t eficonfig_process_enroll_key(void *data)
+{
+ u32
