[PATCH v10 0/7] eficonfig: add UEFI Secure Boot key maintenance interface

[PATCH v10 0/7] eficonfig: add UEFI Secure Boot key maintenance interface

[Masahisa Kojima]

This series adds the UEFI Secure Boot key maintenance interface
to the eficonfig command.
User can enroll PK, KEK, db and dbx.

Source code can be cloned with:
$ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git -b kojima/eficonfig_sbkey_v10

[major changes]
-rebase to v2023.01-rc1

Masahisa Kojima (7):
  eficonfig: refactor file selection handling
  eficonfig: expose append entry function
  eficonfig: refactor change boot order implementation
  eficonfig: expose eficonfig_create_device_path()
  eficonfig: use protocol interface for file selection
  eficonfig: add UEFI Secure Boot Key enrollment interface
  eficonfig: add "Show Signature Database" menu entry

 cmd/Makefile                                  |   5 +
 cmd/eficonfig.c                               | 234 ++++----
 cmd/eficonfig_sbkey.c                         | 498 ++++++++++++++++++
 include/efi_config.h                          |  13 +-
 .../py/tests/test_eficonfig/test_eficonfig.py |   1 +
 5 files changed, 632 insertions(+), 119 deletions(-)
 create mode 100644 cmd/eficonfig_sbkey.c

-- 
2.17.1

[PATCH v10 1/7] eficonfig: refactor file selection handling

[Masahisa Kojima]

eficonfig_select_file_handler() is commonly used to select the
file. eficonfig_display_select_file_option() adds an additional
menu to clear the selected file.
eficonfig_display_select_file_option() is not always necessary
for the file selection process, so it must be outside of
eficonfig_select_file_handler().

This commit also renames the following functions to avoid confusion.
 eficonfig_select_file_handler() -> eficonfig_process_select_file()
 eficonfig_select_file() -> eficonfig_show_file_selection()
 eficonfig_display_select_file_option() -> eficonfig_process_show_file_option()

Finally, test_eficonfig.py need to be updated to get aligned with
the above modification.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
No update since v7

Changes in v7:
- rename functio name to avoid confusion
- remove unused function
- update commit message

newly created in v2

 cmd/eficonfig.c                               | 37 ++++++-------------
 include/efi_config.h                          |  2 +-
 .../py/tests/test_eficonfig/test_eficonfig.py |  1 +
 3 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 2595dd9563..571e2b9ac0 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -756,14 +756,14 @@ out:
 }
 
 /**
- * eficonfig_select_file() - construct the file selection menu
+ * eficonfig_show_file_selection() - construct the file selection menu
  *
  * @file_info:	pointer to the file selection structure
  * @root:	pointer to the file handle
  * Return:	status code
  */
-static efi_status_t eficonfig_select_file(struct eficonfig_select_file_info *file_info,
-					  struct efi_file_handle *root)
+static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_info *file_info,
+						  struct efi_file_handle *root)
 {
 	u32 count = 0, i;
 	efi_uintn_t len;
@@ -938,17 +938,6 @@ static efi_status_t eficonfig_boot_edit_save(void *data)
 	return EFI_SUCCESS;
 }
 
-/**
- * eficonfig_process_select_file() - callback function for "Select File" entry
- *
- * @data:	pointer to the data
- * Return:	status code
- */
-efi_status_t eficonfig_process_select_file(void *data)
-{
-	return EFI_SUCCESS;
-}
-
 /**
  * eficonfig_process_clear_file_selection() - callback function for "Clear" entry
  *
@@ -973,19 +962,19 @@ static struct eficonfig_item select_file_menu_items[] = {
 	{"Quit", eficonfig_process_quit},
 };
 
-
 /**
- * eficonfig_display_select_file_option() - display select file option
+ * eficonfig_process_show_file_option() - display select file option
  *
  * @file_info:	pointer to the file information structure
  * Return:	status code
  */
-efi_status_t eficonfig_display_select_file_option(struct eficonfig_select_file_info *file_info)
+efi_status_t eficonfig_process_show_file_option(void *data)
 {
 	efi_status_t ret;
 	struct efimenu *efi_menu;
 
-	select_file_menu_items[1].data = file_info;
+	select_file_menu_items[0].data = data;
+	select_file_menu_items[1].data = data;
 	efi_menu = eficonfig_create_fixed_menu(select_file_menu_items,
 					       ARRAY_SIZE(select_file_menu_items));
 	if (!efi_menu)
@@ -1001,12 +990,12 @@ efi_status_t eficonfig_display_select_file_option(struct eficonfig_select_file_i
 }
 
 /**
- * eficonfig_select_file_handler() - handle user file selection
+ * eficonfig_process_select_file() - handle user file selection
  *
  * @data:	pointer to the data
  * Return:	status code
  */
-efi_status_t eficonfig_select_file_handler(void *data)
+efi_status_t eficonfig_process_select_file(void *data)
 {
 	size_t len;
 	efi_status_t ret;
@@ -1016,10 +1005,6 @@ efi_status_t eficonfig_select_file_handler(void *data)
 	struct eficonfig_select_file_info *tmp = NULL;
 	struct eficonfig_select_file_info *file_info = data;
 
-	ret = eficonfig_display_select_file_option(file_info);
-	if (ret != EFI_SUCCESS)
-		return ret;
-
 	tmp = calloc(1, sizeof(struct eficonfig_select_file_info));
 	if (!tmp)
 		return EFI_OUT_OF_RESOURCES;
@@ -1046,7 +1031,7 @@ efi_status_t eficonfig_select_file_handler(void *data)
 		if (ret != EFI_SUCCESS)
 			goto out;
 
-		ret = eficonfig_select_file(tmp, root);
+		ret = eficonfig_show_file_selection(tmp, root);
 		if (ret == EFI_ABORTED)
 			continue;
 		if (ret != EFI_SUCCESS)
@@ -1284,7 +1269,7 @@ static efi_status_t prepare_file_selection_entry(struct efimenu *efi_menu, char
 	utf8_utf16_strcpy(&p, devname);
 	u16_strlcat(file_name, file_info->current_path, len);
 	ret = create_boot_option_entry(efi_menu, title, file_name,
-				       eficonfig_select_file_handler, file_info);
+				       eficonfig_process_show_file_option, file_info);
 out:
 	free(devname);
 	free(file_name);
diff --git a/include/efi_config.h b/include/efi_config.h
index 098cac2115..cc6aa51393 100644
--- a/include/efi_config.h
+++ b/include/efi_config.h
@@ -89,7 +89,7 @@ void eficonfig_print_msg(char *msg);
 void eficonfig_destroy(struct efimenu *efi_menu);
 efi_status_t eficonfig_process_quit(void *data);
 efi_status_t eficonfig_process_common(struct efimenu *efi_menu, char *menu_header);
-efi_status_t eficonfig_select_file_handler(void *data);
+efi_status_t eficonfig_process_select_file(void *data);
 efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
 					     efi_uintn_t buf_size, u32 *index);
 efi_status_t eficonfig_append_bootorder(u16 index);
diff --git a/test/py/tests/test_eficonfig/test_eficonfig.py b/test/py/tests/test_eficonfig/test_eficonfig.py
index 99606d9c4b..102bfd7541 100644
--- a/test/py/tests/test_eficonfig/test_eficonfig.py
+++ b/test/py/tests/test_eficonfig/test_eficonfig.py
@@ -349,6 +349,7 @@ def test_efi_eficonfig(u_boot_console, efi_eficonfig_data):
         press_up_down_enter_and_wait(0, 1, True, 'Quit')
         press_up_down_enter_and_wait(0, 0, True, 'No block device found!')
         press_escape_key(False)
+        press_escape_key(False)
         check_current_is_maintenance_menu()
         # Return to U-Boot console
         press_escape_key(True)
-- 
2.17.1

[PATCH v10 2/7] eficonfig: expose append entry function

[Masahisa Kojima]

Following commits are adding support for UEFI variable management
via the eficonfig menu. Those functions needs to use
append_entry() and append_quit_entry() to construct the
menu, so move them out of their static declarations.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
no update since v7

changes in v7:
- update commit message

newly created in v2

 cmd/eficonfig.c      | 32 +++++++++++++++++---------------
 include/efi_config.h |  5 +++++
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 571e2b9ac0..b392de7954 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -263,7 +263,7 @@ efi_status_t eficonfig_process_quit(void *data)
 }
 
 /**
- * append_entry() - append menu item
+ * eficonfig_append_menu_entry() - append menu item
  *
  * @efi_menu:	pointer to the efimenu structure
  * @title:	pointer to the entry title
@@ -271,8 +271,9 @@ efi_status_t eficonfig_process_quit(void *data)
  * @data:	pointer to the data to be passed to each entry callback
  * Return:	status code
  */
-static efi_status_t append_entry(struct efimenu *efi_menu,
-				 char *title, eficonfig_entry_func func, void *data)
+efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
+					 char *title, eficonfig_entry_func func,
+					 void *data)
 {
 	struct eficonfig_entry *entry;
 
@@ -295,12 +296,12 @@ static efi_status_t append_entry(struct efimenu *efi_menu,
 }
 
 /**
- * append_quit_entry() - append quit entry
+ * eficonfig_append_quit_entry() - append quit entry
  *
  * @efi_menu:	pointer to the efimenu structure
  * Return:	status code
  */
-static efi_status_t append_quit_entry(struct efimenu *efi_menu)
+efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu)
 {
 	char *title;
 	efi_status_t ret;
@@ -309,7 +310,7 @@ static efi_status_t append_quit_entry(struct efimenu *efi_menu)
 	if (!title)
 		return EFI_OUT_OF_RESOURCES;
 
-	ret = append_entry(efi_menu, title, eficonfig_process_quit, NULL);
+	ret = eficonfig_append_menu_entry(efi_menu, title, eficonfig_process_quit, NULL);
 	if (ret != EFI_SUCCESS)
 		free(title);
 
@@ -341,7 +342,7 @@ void *eficonfig_create_fixed_menu(const struct eficonfig_item *items, int count)
 		if (!title)
 			goto out;
 
-		ret = append_entry(efi_menu, title, iter->func, iter->data);
+		ret = eficonfig_append_menu_entry(efi_menu, title, iter->func, iter->data);
 		if (ret != EFI_SUCCESS) {
 			free(title);
 			goto out;
@@ -634,14 +635,15 @@ static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *f
 		info->v = v;
 		info->dp = device_path;
 		info->file_info = file_info;
-		ret = append_entry(efi_menu, devname, eficonfig_volume_selected, info);
+		ret = eficonfig_append_menu_entry(efi_menu, devname, eficonfig_volume_selected,
+						  info);
 		if (ret != EFI_SUCCESS) {
 			free(info);
 			goto out;
 		}
 	}
 
-	ret = append_quit_entry(efi_menu);
+	ret = eficonfig_append_quit_entry(efi_menu);
 	if (ret != EFI_SUCCESS)
 		goto out;
 
@@ -745,8 +747,8 @@ eficonfig_create_file_entry(struct efimenu *efi_menu, u32 count,
 	      (int (*)(const void *, const void *))sort_file);
 
 	for (i = 0; i < entry_num; i++) {
-		ret = append_entry(efi_menu, tmp_infos[i]->file_name,
-				   eficonfig_file_selected, tmp_infos[i]);
+		ret = eficonfig_append_menu_entry(efi_menu, tmp_infos[i]->file_name,
+						  eficonfig_file_selected, tmp_infos[i]);
 		if (ret != EFI_SUCCESS)
 			goto out;
 	}
@@ -815,7 +817,7 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
 		if (ret != EFI_SUCCESS)
 			goto err;
 
-		ret = append_quit_entry(efi_menu);
+		ret = eficonfig_append_quit_entry(efi_menu);
 		if (ret != EFI_SUCCESS)
 			goto err;
 
@@ -1206,7 +1208,7 @@ static efi_status_t create_boot_option_entry(struct efimenu *efi_menu, char *tit
 		utf16_utf8_strcpy(&p, val);
 	}
 
-	return append_entry(efi_menu, buf, func, data);
+	return eficonfig_append_menu_entry(efi_menu, buf, func, data);
 }
 
 /**
@@ -1665,7 +1667,7 @@ static efi_status_t eficonfig_add_boot_selection_entry(struct efimenu *efi_menu,
 	utf16_utf8_strcpy(&p, lo.label);
 	info->boot_index = boot_index;
 	info->selected = selected;
-	ret = append_entry(efi_menu, buf, eficonfig_process_boot_selected, info);
+	ret = eficonfig_append_menu_entry(efi_menu, buf, eficonfig_process_boot_selected, info);
 	if (ret != EFI_SUCCESS) {
 		free(load_option);
 		free(info);
@@ -1724,7 +1726,7 @@ static efi_status_t eficonfig_show_boot_selection(unsigned int *selected)
 			break;
 	}
 
-	ret = append_quit_entry(efi_menu);
+	ret = eficonfig_append_quit_entry(efi_menu);
 	if (ret != EFI_SUCCESS)
 		goto out;
 
diff --git a/include/efi_config.h b/include/efi_config.h
index cc6aa51393..064f2efe3f 100644
--- a/include/efi_config.h
+++ b/include/efi_config.h
@@ -95,4 +95,9 @@ efi_status_t eficonfig_get_unused_bootoption(u16 *buf,
 efi_status_t eficonfig_append_bootorder(u16 index);
 efi_status_t eficonfig_generate_media_device_boot_option(void);
 
+efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
+					 char *title, eficonfig_entry_func func,
+					 void *data);
+efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
+
 #endif
-- 
2.17.1

[PATCH v10 3/7] eficonfig: refactor change boot order implementation

[Masahisa Kojima]

All the eficonfig menus other than "Change Boot Order"
use 'eficonfig_entry' structure for each menu entry.
This commit refactors change boot order implementation
to use 'eficonfig_entry' structure same as other menus
to have consistent menu handling.

This commit also simplifies the data->active handling when
KEY_SPACE is pressed, and sizeof() parameter.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
No update since v7

Changes in v7:
- simplify the data->active handling
- update sizeof() parameter
- update commit message

Changes in v5:
- remove direct access mode

 cmd/eficonfig.c | 129 +++++++++++++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 62 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index b392de7954..12babb76c2 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -93,20 +93,14 @@ struct eficonfig_boot_selection_data {
 };
 
 /**
- * struct eficonfig_boot_order - structure to be used to update BootOrder variable
+ * struct eficonfig_boot_order_data - structure to be used to update BootOrder variable
  *
- * @num:		index in the menu entry
- * @description:	pointer to the description string
  * @boot_index:		boot option index
  * @active:		flag to include the boot option into BootOrder variable
- * @list:		list structure
  */
-struct eficonfig_boot_order {
-	u32 num;
-	u16 *description;
+struct eficonfig_boot_order_data {
 	u32 boot_index;
 	bool active;
-	struct list_head list;
 };
 
 /**
@@ -1802,7 +1796,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
 {
 	bool reverse;
 	struct list_head *pos, *n;
-	struct eficonfig_boot_order *entry;
+	struct eficonfig_entry *entry;
 
 	printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION
 	       "\n  ** Change Boot Order **\n"
@@ -1818,7 +1812,7 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
 
 	/* draw boot option list */
 	list_for_each_safe(pos, n, &efi_menu->list) {
-		entry = list_entry(pos, struct eficonfig_boot_order, list);
+		entry = list_entry(pos, struct eficonfig_entry, list);
 		reverse = (entry->num == efi_menu->active);
 
 		printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
@@ -1827,13 +1821,13 @@ static void eficonfig_display_change_boot_order(struct efimenu *efi_menu)
 			puts(ANSI_COLOR_REVERSE);
 
 		if (entry->num < efi_menu->count - 2) {
-			if (entry->active)
+			if (((struct eficonfig_boot_order_data *)entry->data)->active)
 				printf("[*]  ");
 			else
 				printf("[ ]  ");
 		}
 
-		printf("%ls", entry->description);
+		printf("%s", entry->title);
 
 		if (reverse)
 			puts(ANSI_COLOR_RESET);
@@ -1850,9 +1844,8 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 {
 	int esc = 0;
 	struct list_head *pos, *n;
-	struct eficonfig_boot_order *tmp;
 	enum bootmenu_key key = KEY_NONE;
-	struct eficonfig_boot_order *entry;
+	struct eficonfig_entry *entry, *tmp;
 
 	while (1) {
 		bootmenu_loop(NULL, &key, &esc);
@@ -1861,11 +1854,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 		case KEY_PLUS:
 			if (efi_menu->active > 0) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
-					entry = list_entry(pos, struct eficonfig_boot_order, list);
+					entry = list_entry(pos, struct eficonfig_entry, list);
 					if (entry->num == efi_menu->active)
 						break;
 				}
-				tmp = list_entry(pos->prev, struct eficonfig_boot_order, list);
+				tmp = list_entry(pos->prev, struct eficonfig_entry, list);
 				entry->num--;
 				tmp->num++;
 				list_del(&tmp->list);
@@ -1879,11 +1872,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 		case KEY_MINUS:
 			if (efi_menu->active < efi_menu->count - 3) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
-					entry = list_entry(pos, struct eficonfig_boot_order, list);
+					entry = list_entry(pos, struct eficonfig_entry, list);
 					if (entry->num == efi_menu->active)
 						break;
 				}
-				tmp = list_entry(pos->next, struct eficonfig_boot_order, list);
+				tmp = list_entry(pos->next, struct eficonfig_entry, list);
 				entry->num++;
 				tmp->num--;
 				list_del(&entry->list);
@@ -1909,9 +1902,11 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 		case KEY_SPACE:
 			if (efi_menu->active < efi_menu->count - 2) {
 				list_for_each_safe(pos, n, &efi_menu->list) {
-					entry = list_entry(pos, struct eficonfig_boot_order, list);
+					entry = list_entry(pos, struct eficonfig_entry, list);
 					if (entry->num == efi_menu->active) {
-						entry->active = entry->active ? false : true;
+						struct eficonfig_boot_order_data *data = entry->data;
+
+						data->active = !data->active;
 						return EFI_NOT_READY;
 					}
 				}
@@ -1937,12 +1932,13 @@ static efi_status_t eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
 static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_menu,
 							  u32 boot_index, bool active)
 {
+	char *title, *p;
 	efi_status_t ret;
 	efi_uintn_t size;
 	void *load_option;
 	struct efi_load_option lo;
 	u16 varname[] = u"Boot####";
-	struct eficonfig_boot_order *entry;
+	struct eficonfig_boot_order_data *data;
 
 	efi_create_indexed_name(varname, sizeof(varname), "Boot", boot_index);
 	load_option = efi_get_var(varname, &efi_global_variable_guid, &size);
@@ -1950,31 +1946,38 @@ static efi_status_t eficonfig_add_change_boot_order_entry(struct efimenu *efi_me
 		return EFI_SUCCESS;
 
 	ret = efi_deserialize_load_option(&lo, load_option, &size);
-	if (ret != EFI_SUCCESS) {
-		free(load_option);
-		return ret;
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	data = calloc(1, sizeof(*data));
+	if (!data) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
 	}
 
-	entry = calloc(1, sizeof(struct eficonfig_boot_order));
-	if (!entry) {
-		free(load_option);
-		return EFI_OUT_OF_RESOURCES;
+	title = calloc(1, utf16_utf8_strlen(lo.label) + 1);
+	if (!title) {
+		free(data);
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
 	}
+	p = title;
+	utf16_utf8_strcpy(&p, lo.label);
 
-	entry->description = u16_strdup(lo.label);
-	if (!entry->description) {
-		free(load_option);
-		free(entry);
-		return EFI_OUT_OF_RESOURCES;
+	data->boot_index = boot_index;
+	data->active = active;
+
+	ret = eficonfig_append_menu_entry(efi_menu, title, NULL, data);
+	if (ret != EFI_SUCCESS) {
+		free(data);
+		free(title);
+		goto out;
 	}
-	entry->num = efi_menu->count++;
-	entry->boot_index = boot_index;
-	entry->active = active;
-	list_add_tail(&entry->list, &efi_menu->list);
 
+out:
 	free(load_option);
 
-	return EFI_SUCCESS;
+	return ret;
 }
 
 /**
@@ -1989,8 +1992,8 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
 							     u16 *bootorder, efi_uintn_t num)
 {
 	u32 i;
+	char *title;
 	efi_status_t ret;
-	struct eficonfig_boot_order *entry;
 
 	/* list the load option in the order of BootOrder variable */
 	for (i = 0; i < num; i++) {
@@ -2017,27 +2020,25 @@ static efi_status_t eficonfig_create_change_boot_order_entry(struct efimenu *efi
 	}
 
 	/* add "Save" and "Quit" entries */
-	entry = calloc(1, sizeof(struct eficonfig_boot_order));
-	if (!entry)
+	title = strdup("Save");
+	if (!title) {
+		ret = EFI_OUT_OF_RESOURCES;
 		goto out;
+	}
 
-	entry->num = efi_menu->count++;
-	entry->description = u16_strdup(u"Save");
-	list_add_tail(&entry->list, &efi_menu->list);
-
-	entry = calloc(1, sizeof(struct eficonfig_boot_order));
-	if (!entry)
+	ret = eficonfig_append_menu_entry(efi_menu, title, NULL, NULL);
+	if (ret != EFI_SUCCESS)
 		goto out;
 
-	entry->num = efi_menu->count++;
-	entry->description = u16_strdup(u"Quit");
-	list_add_tail(&entry->list, &efi_menu->list);
+	ret = eficonfig_append_quit_entry(efi_menu);
+	if (ret != EFI_SUCCESS)
+		goto out;
 
 	efi_menu->active = 0;
 
 	return EFI_SUCCESS;
 out:
-	return EFI_OUT_OF_RESOURCES;
+	return ret;
 }
 
 /**
@@ -2053,7 +2054,7 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
 	efi_status_t ret;
 	efi_uintn_t num, size;
 	struct list_head *pos, *n;
-	struct eficonfig_boot_order *entry;
+	struct eficonfig_entry *entry;
 	struct efimenu *efi_menu;
 
 	efi_menu = calloc(1, sizeof(struct efimenu));
@@ -2084,9 +2085,16 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
 			/* create new BootOrder  */
 			count = 0;
 			list_for_each_safe(pos, n, &efi_menu->list) {
-				entry = list_entry(pos, struct eficonfig_boot_order, list);
-				if (entry->active)
-					new_bootorder[count++] = entry->boot_index;
+				struct eficonfig_boot_order_data *data;
+
+				entry = list_entry(pos, struct eficonfig_entry, list);
+				/* exit the loop when iteration reaches "Save" */
+				if (!strncmp(entry->title, "Save", strlen("Save")))
+					break;
+
+				data = entry->data;
+				if (data->active)
+					new_bootorder[count++] = data->boot_index;
 			}
 
 			size = count * sizeof(u16);
@@ -2105,15 +2113,12 @@ static efi_status_t eficonfig_process_change_boot_order(void *data)
 		}
 	}
 out:
+	free(bootorder);
 	list_for_each_safe(pos, n, &efi_menu->list) {
-		entry = list_entry(pos, struct eficonfig_boot_order, list);
-		list_del(&entry->list);
-		free(entry->description);
-		free(entry);
+		entry = list_entry(pos, struct eficonfig_entry, list);
+		free(entry->data);
 	}
-
-	free(bootorder);
-	free(efi_menu);
+	eficonfig_destroy(efi_menu);
 
 	/* to stay the parent menu */
 	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
-- 
2.17.1

[PATCH v10 4/7] eficonfig: expose eficonfig_create_device_path()

[Masahisa Kojima]

Following commits are adding support for UEFI variable management
via the eficonfig menu. Those functions needs to use
eficonfig_create_device_path() to construct the full device path
from device path of the volume and file path, so move it
out of their static declarations.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v10

 cmd/eficonfig.c      | 20 +++++++++++---------
 include/efi_config.h |  2 ++
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 12babb76c2..58a6856666 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data)
 }
 
 /**
- * create_selected_device_path() - create device path
+ * eficonfig_create_device_path() - create device path
  *
- * @file_info:	pointer to the selected file information
+ * @dp_volume:	pointer to the volume
+ * @current_path: pointer to the file path u16 string
  * Return:
  * device path or NULL. Caller must free the returned value
  */
-static
-struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info)
+struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
+						     u16 *current_path)
 {
 	char *p;
 	void *buf;
@@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
 	struct efi_device_path_file_path *fp;
 
 	fp_size = sizeof(struct efi_device_path) +
-		  ((u16_strlen(file_info->current_path) + 1) * sizeof(u16));
+		  ((u16_strlen(current_path) + 1) * sizeof(u16));
 	buf = calloc(1, fp_size + sizeof(END));
 	if (!buf)
 		return NULL;
@@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
 	fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
 	fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
 	fp->dp.length = (u16)fp_size;
-	u16_strcpy(fp->str, file_info->current_path);
+	u16_strcpy(fp->str, current_path);
 
 	p = buf;
 	p += fp_size;
 	*((struct efi_device_path *)p) = END;
 
-	dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf);
+	dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf);
 	free(buf);
 
 	return dp;
@@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
 	}
 
 	if (bo->initrd_info.dp_volume) {
-		dp = create_selected_device_path(&bo->initrd_info);
+		dp = eficonfig_create_device_path(bo->initrd_info.dp_volume,
+						 bo->initrd_info.current_path);
 		if (!dp) {
 			ret = EFI_OUT_OF_RESOURCES;
 			goto out;
@@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
 		efi_free_pool(dp);
 	}
 
-	dp = create_selected_device_path(&bo->file_info);
+	dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path);
 	if (!dp) {
 		ret = EFI_OUT_OF_RESOURCES;
 		goto out;
diff --git a/include/efi_config.h b/include/efi_config.h
index 064f2efe3f..934de41e85 100644
--- a/include/efi_config.h
+++ b/include/efi_config.h
@@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
 					 char *title, eficonfig_entry_func func,
 					 void *data);
 efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
+struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
+						     u16 *current_path);
 
 #endif
-- 
2.17.1

[PATCH v10 5/7] eficonfig: use protocol interface for file selection

[Masahisa Kojima]

EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is not always provided
by U-Boot. Use protocol interface functions instead of
U-Boot internal functions.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
---
Newly created in v10

 cmd/eficonfig.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 58a6856666..6779b1792e 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -696,14 +696,14 @@ eficonfig_create_file_entry(struct efimenu *efi_menu, u32 count,
 	u32 i, entry_num = 0;
 	struct eficonfig_file_entry_data *info;
 
-	efi_file_setpos_int(f, 0);
+	EFI_CALL(f->setpos(f, 0));
 	/* Read directory and construct menu structure */
 	for (i = 0; i < count; i++) {
 		if (entry_num >= EFICONFIG_ENTRY_NUM_MAX - 1)
 			break;
 
 		len = sizeof(struct efi_file_info) + EFICONFIG_FILE_PATH_BUF_SIZE;
-		ret = efi_file_read_int(f, &len, buf);
+		ret = EFI_CALL(f->read(f, &len, buf));
 		if (ret != EFI_SUCCESS || len == 0)
 			break;
 
@@ -782,7 +782,8 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
 		}
 		INIT_LIST_HEAD(&efi_menu->list);
 
-		ret = efi_file_open_int(root, &f, file_info->current_path, EFI_FILE_MODE_READ, 0);
+		ret = EFI_CALL(root->open(root, &f, file_info->current_path,
+					  EFI_FILE_MODE_READ, 0));
 		if (ret != EFI_SUCCESS) {
 			eficonfig_print_msg("Reading volume failed!");
 			free(efi_menu);
@@ -793,7 +794,7 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
 		/* Count the number of directory entries */
 		for (;;) {
 			len = sizeof(struct efi_file_info) + EFICONFIG_FILE_PATH_BUF_SIZE;
-			ret = efi_file_read_int(f, &len, buf);
+			ret = EFI_CALL(f->read(f, &len, buf));
 			if (ret != EFI_SUCCESS || len == 0)
 				break;
 
@@ -818,7 +819,7 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
 
 		ret = eficonfig_process_common(efi_menu, "  ** Select File **");
 err:
-		efi_file_close_int(f);
+		EFI_CALL(f->close(f));
 		eficonfig_destroy(efi_menu);
 
 		if (tmp_infos) {
@@ -1024,7 +1025,7 @@ efi_status_t eficonfig_process_select_file(void *data)
 		if (!tmp->current_volume)
 			return EFI_INVALID_PARAMETER;
 
-		ret = efi_open_volume_int(tmp->current_volume, &root);
+		ret = EFI_CALL(tmp->current_volume->open_volume(tmp->current_volume, &root));
 		if (ret != EFI_SUCCESS)
 			goto out;
 
-- 
2.17.1

[PATCH v10 6/7] eficonfig: add UEFI Secure Boot Key enrollment interface

[Masahisa Kojima]

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 <masahisa.kojima@linaro.org>
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes in v10:
- use protocol interface
- use efi_file_from_path()
- use malloc instead of calloc

Changes in v9:
- move file size check, set ret = EFI_INVALID_PARAMETER

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 | 263 ++++++++++++++++++++++++++++++++++++++++++
 include/efi_config.h  |   4 +
 4 files changed, 275 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 6779b1792e..44eb82c3c1 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2438,6 +2438,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 0000000000..5130848dd9
--- /dev/null
+++ b/cmd/eficonfig_sbkey.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Menu-driven UEFI Secure Boot Key Maintenance
+ *
+ *  Copyright (c) 2022 Masahisa Kojima, Linaro Limited
+ */
+
+#include <ansi.h>
+#include <common.h>
+#include <charset.h>
+#include <hexdump.h>
+#include <log.h>
+#include <malloc.h>
+#include <menu.h>
+#include <efi_loader.h>
+#include <efi_config.h>
+#include <efi_variable.h>
+#include <crypto/pkcs7_parser.h>
+
+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->auth_info.cert_type, &efi_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 attr;
+	char *buf = NULL;
+	efi_uintn_t size;
+	efi_status_t ret;
+	struct efi_file_handle *f = NULL;
+	struct efi_device_path *full_dp = NULL;
+	struct eficonfig_select_file_info file_info;
+
+	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(&file_info);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	full_dp = eficonfig_create_device_path(file_info.dp_volume, file_info.current_path);
+	if (!full_dp) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+	f = efi_file_from_path(full_dp);
+	if (!f) {
+		ret = EFI_NOT_FOUND;
+		goto out;
+	}
+
+	size = 0;
+	ret = EFI_CALL(f->getinfo(f, &efi_file_info_guid, &size, NULL));
+	if (ret != EFI_BUFFER_TOO_SMALL)
+		goto out;
+
+	buf = malloc(size);
+	if (!buf) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+	ret = EFI_CALL(f->getinfo(f, &efi_file_info_guid, &size, buf));
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	size = ((struct efi_file_info *)buf)->file_size;
+	free(buf);
+
+	if (!size) {
+		eficonfig_print_msg("ERROR! File is empty.");
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	buf = malloc(size);
+	if (!buf) {
+		ret = EFI_OUT_OF_RESOURCES;
+		goto out;
+	}
+
+	ret = EFI_CALL(f->read(f, &size, 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,
+					   &db_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);
+	efi_free_pool(full_dp);
+	if (f)
+		EFI_CALL(f->close(f));
+
+	/* return to the parent menu */
+	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
+
+	return ret;
+}
+
+static struct eficonfig_item key_config_menu_items[] = {
+	{"Enroll New Key", eficonfig_process_enroll_key},
+	{"Quit", eficonfig_process_quit},
+};
+
+/**
+ * eficonfig_process_set_secure_boot_key() - display the key configuration menu
+ *
+ * @data:	pointer to the data for each entry
+ * Return:	status code
+ */
+static efi_status_t eficonfig_process_set_secure_boot_key(void *data)
+{
+	u32 i;
+	efi_status_t ret;
+	char header_str[32];
+	struct efimenu *efi_menu;
+
+	for (i = 0; i < ARRAY_SIZE(key_config_menu_items); i++)
+		key_config_menu_items[i].data = data;
+
+	snprintf(header_str, sizeof(header_str), "  ** Configure %ls **", (u16 *)data);
+
+	while (1) {
+		efi_menu = eficonfig_create_fixed_menu(key_config_menu_items,
+						       ARRAY_SIZE(key_config_menu_items));
+
+		ret = eficonfig_process_common(efi_menu, header_str);
+		eficonfig_destroy(efi_menu);
+
+		if (ret == EFI_ABORTED)
+			break;
+	}
+
+	/* return to the parent menu */
+	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
+
+	return ret;
+}
+
+static const struct eficonfig_item secure_boot_menu_items[] = {
+	{"PK", eficonfig_process_set_secure_boot_key, u"PK"},
+	{"KEK", eficonfig_process_set_secure_boot_key, u"KEK"},
+	{"db", eficonfig_process_set_secure_boot_key, u"db"},
+	{"dbx", eficonfig_process_set_secure_boot_key, u"dbx"},
+	{"Quit", eficonfig_process_quit},
+};
+
+/**
+ * eficonfig_process_secure_boot_config() - display the key list menu
+ *
+ * @data:	pointer to the data for each entry
+ * Return:	status code
+ */
+efi_status_t eficonfig_process_secure_boot_config(void *data)
+{
+	efi_status_t ret;
+	struct efimenu *efi_menu;
+
+	while (1) {
+		char header_str[64];
+
+		snprintf(header_str, sizeof(header_str),
+			 "  ** UEFI Secure Boot Key Configuration (SecureBoot : %s) **",
+			 (efi_secure_boot_enabled() ? "ON" : "OFF"));
+
+		efi_menu = eficonfig_create_fixed_menu(secure_boot_menu_items,
+						       ARRAY_SIZE(secure_boot_menu_items));
+		if (!efi_menu) {
+			ret = EFI_OUT_OF_RESOURCES;
+			break;
+		}
+
+		ret = eficonfig_process_common(efi_menu, header_str);
+		eficonfig_destroy(efi_menu);
+
+		if (ret == EFI_ABORTED)
+			break;
+	}
+
+	/* return to the parent menu */
+	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
+
+	return ret;
+}
diff --git a/include/efi_config.h b/include/efi_config.h
index 934de41e85..fd69926343 100644
--- a/include/efi_config.h
+++ b/include/efi_config.h
@@ -101,5 +101,9 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
 efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
 struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
 						     u16 *current_path);
+void *eficonfig_create_fixed_menu(const struct eficonfig_item *items, int count);
+#ifdef CONFIG_EFI_SECURE_BOOT
+efi_status_t eficonfig_process_secure_boot_config(void *data);
+#endif
 
 #endif
-- 
2.17.1

[PATCH v10 7/7] eficonfig: add "Show Signature Database" menu entry

[Masahisa Kojima]

This commit adds the menu-driven interface to show the
signature list content.

Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Changes in v10:
- change buffer size to appropriate value(37)
- use strdup()

No update since v7

Changes in v7:
- remove delete signature list feature
  user can clear the signature database with signed null key
- rename function name to avoid confusion
- update commit message

Changes in v6:
- update comment

Changes in v2:
- integrate show and delete signature database menu
- add confirmation message before delete
- add function comment

 cmd/eficonfig_sbkey.c | 235 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 235 insertions(+)

diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
index 5130848dd9..6e0bebf1d4 100644
--- a/cmd/eficonfig_sbkey.c
+++ b/cmd/eficonfig_sbkey.c
@@ -17,6 +17,13 @@
 #include <efi_variable.h>
 #include <crypto/pkcs7_parser.h>
 
+struct eficonfig_sig_data {
+	struct efi_signature_list *esl;
+	struct efi_signature_data *esd;
+	struct list_head list;
+	u16 *varname;
+};
+
 enum efi_sbkey_signature_type {
 	SIG_TYPE_X509 = 0,
 	SIG_TYPE_HASH,
@@ -176,8 +183,236 @@ out:
 	return ret;
 }
 
+/**
+ * eficonfig_process_show_siglist() - show signature list content
+ *
+ * @data:	pointer to the data for each entry
+ * Return:	status code
+ */
+static efi_status_t eficonfig_process_show_siglist(void *data)
+{
+	u32 i;
+	struct eficonfig_sig_data *sg = data;
+
+	puts(ANSI_CURSOR_HIDE);
+	puts(ANSI_CLEAR_CONSOLE);
+	printf(ANSI_CURSOR_POSITION, 1, 1);
+
+	printf("\n  ** Show Signature Database (%ls) **\n\n"
+	       "    Owner GUID:\n"
+	       "      %pUL\n",
+	       sg->varname, sg->esd->signature_owner.b);
+
+	for (i = 0; i < ARRAY_SIZE(sigtype_to_str); i++) {
+		if (!guidcmp(&sg->esl->signature_type, &sigtype_to_str[i].sig_type)) {
+			printf("    Signature Type:\n"
+			       "      %s\n", sigtype_to_str[i].str);
+
+			switch (sigtype_to_str[i].type) {
+			case SIG_TYPE_X509:
+			{
+				struct x509_certificate *cert_tmp;
+
+				cert_tmp = x509_cert_parse(sg->esd->signature_data,
+							   sg->esl->signature_size);
+				printf("    Subject:\n"
+				       "      %s\n"
+				       "    Issuer:\n"
+				       "      %s\n",
+				       cert_tmp->subject, cert_tmp->issuer);
+				break;
+			}
+			case SIG_TYPE_CRL:
+			{
+				u32 hash_size = sg->esl->signature_size - sizeof(efi_guid_t) -
+						sizeof(struct efi_time);
+				struct efi_time *time =
+					(struct efi_time *)((u8 *)sg->esd->signature_data +
+					hash_size);
+
+				printf("    ToBeSignedHash:\n");
+				print_hex_dump("      ", DUMP_PREFIX_NONE, 16, 1,
+					       sg->esd->signature_data, hash_size, false);
+				printf("    TimeOfRevocation:\n"
+				       "      %d-%d-%d %02d:%02d:%02d\n",
+				       time->year, time->month, time->day,
+				       time->hour, time->minute, time->second);
+				break;
+			}
+			case SIG_TYPE_HASH:
+			{
+				u32 hash_size = sg->esl->signature_size - sizeof(efi_guid_t);
+
+				printf("    Hash:\n");
+				print_hex_dump("      ", DUMP_PREFIX_NONE, 16, 1,
+					       sg->esd->signature_data, hash_size, false);
+				break;
+			}
+			default:
+				eficonfig_print_msg("ERROR! Unsupported format.");
+				return EFI_INVALID_PARAMETER;
+			}
+		}
+	}
+
+	while (tstc())
+		getchar();
+
+	printf("\n\n  Press any key to continue");
+	getchar();
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * prepare_signature_list_menu() - create the signature list menu entry
+ *
+ * @efimenu:	pointer to the efimenu structure
+ * @varname:	pointer to the variable name
+ * @db:		pointer to the variable raw data
+ * @db_size:	variable data size
+ * @func:	callback of each entry
+ * Return:	status code
+ */
+static efi_status_t prepare_signature_list_menu(struct efimenu *efi_menu, void *varname,
+						void *db, efi_uintn_t db_size,
+						eficonfig_entry_func func)
+{
+	u32 num = 0;
+	efi_uintn_t size;
+	struct eficonfig_sig_data *sg;
+	struct efi_signature_list *esl;
+	struct efi_signature_data *esd;
+	efi_status_t ret = EFI_SUCCESS;
+
+	INIT_LIST_HEAD(&efi_menu->list);
+
+	esl = db;
+	size = db_size;
+	while (size > 0) {
+		u32 remain;
+
+		esd = (struct efi_signature_data *)((u8 *)esl +
+						    (sizeof(struct efi_signature_list) +
+						    esl->signature_header_size));
+		remain = esl->signature_list_size - sizeof(struct efi_signature_list) -
+			 esl->signature_header_size;
+		for (; remain > 0; remain -= esl->signature_size) {
+			char buf[37];
+			char *title;
+
+			if (num >= EFICONFIG_ENTRY_NUM_MAX - 1) {
+				ret = EFI_OUT_OF_RESOURCES;
+				goto out;
+			}
+
+			sg = calloc(1, sizeof(struct eficonfig_sig_data));
+			if (!sg) {
+				ret = EFI_OUT_OF_RESOURCES;
+				goto err;
+			}
+
+			snprintf(buf, sizeof(buf), "%pUL", &esd->signature_owner);
+			title = strdup(buf);
+			if (!title) {
+				free(sg);
+				ret = EFI_OUT_OF_RESOURCES;
+				goto err;
+			}
+
+			sg->esl = esl;
+			sg->esd = esd;
+			sg->varname = varname;
+			ret = eficonfig_append_menu_entry(efi_menu, title, func, sg);
+			if (ret != EFI_SUCCESS) {
+				free(sg);
+				free(title);
+				goto err;
+			}
+			esd = (struct efi_signature_data *)((u8 *)esd + esl->signature_size);
+			num++;
+		}
+
+		size -= esl->signature_list_size;
+		esl = (struct efi_signature_list *)((u8 *)esl + esl->signature_list_size);
+	}
+out:
+	ret = eficonfig_append_quit_entry(efi_menu);
+err:
+	return ret;
+}
+
+/**
+ * enumerate_and_show_signature_database() - enumerate and show the signature database
+ *
+ * @data:	pointer to the data for each entry
+ * Return:	status code
+ */
+static efi_status_t enumerate_and_show_signature_database(void *varname)
+{
+	void *db;
+	char buf[50];
+	efi_status_t ret;
+	efi_uintn_t db_size;
+	struct efimenu *efi_menu;
+	struct list_head *pos, *n;
+	struct eficonfig_entry *entry;
+
+	db = efi_get_var(varname, efi_auth_var_get_guid(varname), &db_size);
+	if (!db) {
+		eficonfig_print_msg("There is no entry in the signature database.");
+		return EFI_NOT_FOUND;
+	}
+
+	efi_menu = calloc(1, sizeof(struct efimenu));
+	if (!efi_menu) {
+		free(db);
+		return EFI_OUT_OF_RESOURCES;
+	}
+
+	ret = prepare_signature_list_menu(efi_menu, varname, db, db_size,
+					  eficonfig_process_show_siglist);
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	snprintf(buf, sizeof(buf), "  ** Show Signature Database (%ls) **", (u16 *)varname);
+	ret = eficonfig_process_common(efi_menu, buf);
+out:
+	list_for_each_safe(pos, n, &efi_menu->list) {
+		entry = list_entry(pos, struct eficonfig_entry, list);
+		free(entry->data);
+	}
+	eficonfig_destroy(efi_menu);
+	free(db);
+
+	return ret;
+}
+
+/**
+ * eficonfig_process_show_signature_database() - process show signature database
+ *
+ * @data:	pointer to the data for each entry
+ * Return:	status code
+ */
+static efi_status_t eficonfig_process_show_signature_database(void *data)
+{
+	efi_status_t ret;
+
+	while (1) {
+		ret = enumerate_and_show_signature_database(data);
+		if (ret != EFI_SUCCESS && ret != EFI_NOT_READY)
+			break;
+	}
+
+	/* return to the parent menu */
+	ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret;
+
+	return ret;
+}
+
 static struct eficonfig_item key_config_menu_items[] = {
 	{"Enroll New Key", eficonfig_process_enroll_key},
+	{"Show Signature Database", eficonfig_process_show_signature_database},
 	{"Quit", eficonfig_process_quit},
 };
 
-- 
2.17.1

Re: [PATCH v10 4/7] eficonfig: expose eficonfig_create_device_path()

[Heinrich Schuchardt]

On 11/20/22 01:21, Masahisa Kojima wrote:
> Following commits are adding support for UEFI variable management
> via the eficonfig menu. Those functions needs to use
> eficonfig_create_device_path() to construct the full device path
> from device path of the volume and file path, so move it
> out of their static declarations.
>
> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> ---
> Newly created in v10
>
>   cmd/eficonfig.c      | 20 +++++++++++---------
>   include/efi_config.h |  2 ++
>   2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 12babb76c2..58a6856666 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data)
>   }
>
>   /**
> - * create_selected_device_path() - create device path
> + * eficonfig_create_device_path() - create device path
>    *
> - * @file_info:	pointer to the selected file information
> + * @dp_volume:	pointer to the volume
> + * @current_path: pointer to the file path u16 string
>    * Return:
>    * device path or NULL. Caller must free the returned value
>    */
> -static
> -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info)
> +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> +						     u16 *current_path)
>   {
>   	char *p;
>   	void *buf;
> @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
>   	struct efi_device_path_file_path *fp;

This function should be moved to lib/efi_loader/efi_device_path.c so
that we can use it to simplify efi_dp_from_file().

>
>   	fp_size = sizeof(struct efi_device_path) +
> -		  ((u16_strlen(file_info->current_path) + 1) * sizeof(u16));
> +		  ((u16_strlen(current_path) + 1) * sizeof(u16));

You could reduce the code size a little with:

fp_size = sizeof(struct efi_device_path) +
u16_strsize(file_info->current_path);

Best regards

Heinrich

>   	buf = calloc(1, fp_size + sizeof(END));
>   	if (!buf)
>   		return NULL;
> @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
>   	fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
>   	fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
>   	fp->dp.length = (u16)fp_size;
> -	u16_strcpy(fp->str, file_info->current_path);
> +	u16_strcpy(fp->str, current_path);
>
>   	p = buf;
>   	p += fp_size;
>   	*((struct efi_device_path *)p) = END;
>
> -	dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf);
> +	dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf);
>   	free(buf);
>
>   	return dp;
> @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   	}
>
>   	if (bo->initrd_info.dp_volume) {
> -		dp = create_selected_device_path(&bo->initrd_info);
> +		dp = eficonfig_create_device_path(bo->initrd_info.dp_volume,
> +						 bo->initrd_info.current_path);
>   		if (!dp) {
>   			ret = EFI_OUT_OF_RESOURCES;
>   			goto out;
> @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
>   		efi_free_pool(dp);
>   	}
>
> -	dp = create_selected_device_path(&bo->file_info);
> +	dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path);
>   	if (!dp) {
>   		ret = EFI_OUT_OF_RESOURCES;
>   		goto out;
> diff --git a/include/efi_config.h b/include/efi_config.h
> index 064f2efe3f..934de41e85 100644
> --- a/include/efi_config.h
> +++ b/include/efi_config.h
> @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
>   					 char *title, eficonfig_entry_func func,
>   					 void *data);
>   efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
> +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> +						     u16 *current_path);
>
>   #endif

Re: [PATCH v10 4/7] eficonfig: expose eficonfig_create_device_path()

[Masahisa Kojima]

Hi Heinrich,

On Sun, 20 Nov 2022 at 20:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/20/22 01:21, Masahisa Kojima wrote:
> > Following commits are adding support for UEFI variable management
> > via the eficonfig menu. Those functions needs to use
> > eficonfig_create_device_path() to construct the full device path
> > from device path of the volume and file path, so move it
> > out of their static declarations.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> > ---
> > Newly created in v10
> >
> >   cmd/eficonfig.c      | 20 +++++++++++---------
> >   include/efi_config.h |  2 ++
> >   2 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > index 12babb76c2..58a6856666 100644
> > --- a/cmd/eficonfig.c
> > +++ b/cmd/eficonfig.c
> > @@ -436,14 +436,15 @@ static efi_status_t eficonfig_volume_selected(void *data)
> >   }
> >
> >   /**
> > - * create_selected_device_path() - create device path
> > + * eficonfig_create_device_path() - create device path
> >    *
> > - * @file_info:       pointer to the selected file information
> > + * @dp_volume:       pointer to the volume
> > + * @current_path: pointer to the file path u16 string
> >    * Return:
> >    * device path or NULL. Caller must free the returned value
> >    */
> > -static
> > -struct efi_device_path *create_selected_device_path(struct eficonfig_select_file_info *file_info)
> > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> > +                                                  u16 *current_path)
> >   {
> >       char *p;
> >       void *buf;
> > @@ -452,7 +453,7 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
> >       struct efi_device_path_file_path *fp;
>
> This function should be moved to lib/efi_loader/efi_device_path.c so
> that we can use it to simplify efi_dp_from_file().

eficonfig_create_device_path() takes u16 file path string, but
efi_dp_from_file() takes
file path string, so additional charset conversion is required to call
this library function
from efi_dp_from_file() and I wonder it simplifies the code.

>
> >
> >       fp_size = sizeof(struct efi_device_path) +
> > -               ((u16_strlen(file_info->current_path) + 1) * sizeof(u16));
> > +               ((u16_strlen(current_path) + 1) * sizeof(u16));
>
> You could reduce the code size a little with:
>
> fp_size = sizeof(struct efi_device_path) +
> u16_strsize(file_info->current_path);

I'm aware that you have already merged this series.
I will send a follow up patch to fix this u16_strsize().

Thanks,
Masahisa Kojima


>
> Best regards
>
> Heinrich
>
> >       buf = calloc(1, fp_size + sizeof(END));
> >       if (!buf)
> >               return NULL;
> > @@ -461,13 +462,13 @@ struct efi_device_path *create_selected_device_path(struct eficonfig_select_file
> >       fp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >       fp->dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
> >       fp->dp.length = (u16)fp_size;
> > -     u16_strcpy(fp->str, file_info->current_path);
> > +     u16_strcpy(fp->str, current_path);
> >
> >       p = buf;
> >       p += fp_size;
> >       *((struct efi_device_path *)p) = END;
> >
> > -     dp = efi_dp_append(file_info->dp_volume, (struct efi_device_path *)buf);
> > +     dp = efi_dp_append(dp_volume, (struct efi_device_path *)buf);
> >       free(buf);
> >
> >       return dp;
> > @@ -1472,7 +1473,8 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
> >       }
> >
> >       if (bo->initrd_info.dp_volume) {
> > -             dp = create_selected_device_path(&bo->initrd_info);
> > +             dp = eficonfig_create_device_path(bo->initrd_info.dp_volume,
> > +                                              bo->initrd_info.current_path);
> >               if (!dp) {
> >                       ret = EFI_OUT_OF_RESOURCES;
> >                       goto out;
> > @@ -1481,7 +1483,7 @@ static efi_status_t eficonfig_edit_boot_option(u16 *varname, struct eficonfig_bo
> >               efi_free_pool(dp);
> >       }
> >
> > -     dp = create_selected_device_path(&bo->file_info);
> > +     dp = eficonfig_create_device_path(bo->file_info.dp_volume, bo->file_info.current_path);
> >       if (!dp) {
> >               ret = EFI_OUT_OF_RESOURCES;
> >               goto out;
> > diff --git a/include/efi_config.h b/include/efi_config.h
> > index 064f2efe3f..934de41e85 100644
> > --- a/include/efi_config.h
> > +++ b/include/efi_config.h
> > @@ -99,5 +99,7 @@ efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
> >                                        char *title, eficonfig_entry_func func,
> >                                        void *data);
> >   efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu);
> > +struct efi_device_path *eficonfig_create_device_path(struct efi_device_path *dp_volume,
> > +                                                  u16 *current_path);
> >
> >   #endif
>