| Message ID | 20220619052022.2694-4-masahisa.kojima@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | eficonfig: add UEFI Secure Boot key maintenance interface | expand |
On 6/19/22 07:20, Masahisa Kojima wrote: > This commit add the menu-driven interface to delete the > signature database entry. > EFI Signature Lists can contain the multiple signature > entries, this menu can delete the indivisual entry. > > If the PK is enrolled and UEFI Secure Boot is in User Mode, Why don't you mention Deployed Mode? > user can not delete the existing signature lists since the > signature lists must be signed by KEK or PK but signing > information is not stored in the signature database. Updating PK with an empty value must be possible if if the new value is signed with the old PK. > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > --- > cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 217 insertions(+), 1 deletion(-) > > diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c > index 02ab8f8218..142bb4cef5 100644 > --- a/cmd/eficonfig_sbkey.c > +++ b/cmd/eficonfig_sbkey.c > @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { > /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ > }; > Please, add documentation to all functions. > +static int eficonfig_console_yes_no(void) > +{ > + int esc = 0; > + enum bootmenu_key key = KEY_NONE; > + > + while (1) { > + bootmenu_loop(NULL, &key, &esc); > + > + switch (key) { > + case KEY_SELECT: > + return 1; > + case KEY_QUIT: > + return 0; > + default: > + break; > + } > + } > + > + /* never happens */ > + debug("eficonfig: this should not happen"); > + return 0; If this code is unreachable, remove it. > +} > + > static void eficonfig_console_wait_enter(void) > { > int esc = 0; > @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) > > /* never happens */ > debug("eficonfig: this should not happen"); > - return; Please remove unreachable code. > +} > + > +static bool is_setupmode(void) > +{ > + efi_status_t ret; > + u8 setup_mode; > + efi_uintn_t size; > + > + size = sizeof(setup_mode); > + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, > + NULL, &size, &setup_mode, NULL); > + > + return setup_mode == 1; > } > > static bool is_secureboot_enabled(void) > @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) > return EFI_SUCCESS; > } > > +static efi_status_t eficonfig_process_sigdata_delete(void *data) > +{ > + int yes_no; > + struct eficonfig_sig_data *sg = data; > + > + display_sigdata_header(sg, "Delete"); > + display_sigdata_info(sg); > + > + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); > + yes_no = eficonfig_console_yes_no(); > + if (!yes_no) > + return EFI_NOT_READY; > + > + return EFI_SUCCESS; > +} > + > +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, > + struct eficonfig_sig_data *target, > + struct list_head *siglist_list) > +{ > + u8 *dest, *start; > + struct list_head *pos, *n; > + u32 remain; > + u32 size = *db_size; > + u8 *end = (u8 *)db + size; > + struct eficonfig_sig_data *sg; > + > + list_for_each_safe(pos, n, siglist_list) { > + sg = list_entry(pos, struct eficonfig_sig_data, list); > + if (sg->esl == target->esl && sg->esd == target->esd) { > + remain = sg->esl->signature_list_size - > + (sizeof(struct efi_signature_list) - > + sg->esl->signature_header_size) - > + sg->esl->signature_size; > + if (remain > 0) { > + /* only delete the single signature data */ > + sg->esl->signature_list_size -= sg->esl->signature_size; > + size -= sg->esl->signature_size; > + dest = (u8 *)sg->esd; > + start = (u8 *)sg->esd + sg->esl->signature_size; > + } else { > + /* delete entire signature list */ > + dest = (u8 *)sg->esl; > + start = (u8 *)sg->esl + sg->esl->signature_list_size; > + size -= sg->esl->signature_list_size; > + } > + memmove(dest, start, (end - start)); > + } > + } > + > + *db_size = size; > +} > + > +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) > +{ > + efi_status_t ret; > + struct efi_time time; > + efi_uintn_t total_size; > + struct efi_variable_authentication_2 *auth; > + > + *new_db = NULL; > + > + /* > + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS > + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it > + * without certificate data in it. > + */ > + total_size = sizeof(struct efi_variable_authentication_2) + *size; > + > + auth = calloc(1, total_size); > + if (!auth) > + return EFI_OUT_OF_RESOURCES; > + > + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); > + if (ret != EFI_SUCCESS) { > + free(auth); > + return EFI_OUT_OF_RESOURCES; > + } > + time.pad1 = 0; > + time.nanosecond = 0; > + time.timezone = 0; > + time.daylight = 0; > + time.pad2 = 0; > + memcpy(&auth->time_stamp, &time, sizeof(time)); > + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); > + auth->auth_info.hdr.wRevision = 0x0200; > + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; > + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); > + if (db) > + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); > + > + *new_db = auth; > + *size = total_size; > + > + return EFI_SUCCESS; > +} > + > static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, > void *db, efi_uintn_t db_size, > eficonfig_entry_func func, > @@ -378,6 +510,68 @@ out: > return ret; > } > > +static efi_status_t process_delete_key(void *varname) > +{ > + u32 attr, i, count = 0; > + efi_status_t ret; > + struct eficonfig_item *menu_item = NULL, *iter; > + void *db = NULL, *new_db = NULL; > + efi_uintn_t db_size; > + struct list_head siglist_list; > + struct eficonfig_sig_data *selected; > + > + 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."); Please, use the terms of the UEFI specification. %s/signature database/signature store/ > + return EFI_NOT_FOUND; > + } > + > + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, > + eficonfig_process_sigdata_delete, &selected, > + &siglist_list, &count); > + if (ret != EFI_SUCCESS) > + goto out; > + > + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); > + > + if (ret == EFI_SUCCESS) { > + delete_selected_signature_data(db, &db_size, selected, &siglist_list); > + > + ret = create_time_based_payload(db, &new_db, &db_size); > + if (ret != EFI_SUCCESS) > + goto out; > + > + attr = EFI_VARIABLE_NON_VOLATILE | > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS | > + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), > + attr, db_size, new_db, false); > + if (ret != EFI_SUCCESS) { > + eficonfig_print_msg("ERROR! Fail to delete signature database"); %s/Fail/Failed/ Please, use the terms of the UEFI specification. %s/signature database/signature store/ > + goto out; > + } > + } > + > +out: > + if (menu_item) { > + iter = menu_item; > + for (i = 0; i < count - 1; iter++, i++) { > + free(iter->title); > + free(iter->data); > + } > + } > + > + free(menu_item); > + free(db); > + free(new_db); > + > + /* to stay the parent menu */ > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > + > + return ret; > +} > + > static efi_status_t eficonfig_process_show_signature_db(void *data) > { > efi_status_t ret; > @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) > return ret; > } > > +static efi_status_t eficonfig_process_delete_key(void *data) > +{ > + efi_status_t ret; > + > + if (!is_setupmode()) { > + eficonfig_print_msg("Not in the SetupMode, can not delete."); Both in Setup Mode and in Audit Mode you should be able to edit keys. > + return EFI_SUCCESS; > + } > + > + while (1) { > + ret = process_delete_key(data); > + if (ret != EFI_SUCCESS) > + break; > + } > + > + /* to stay the parent menu */ > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > + > + return ret; > +} > + > static struct eficonfig_item key_config_pk_menu_items[] = { > {"Enroll New Key", eficonfig_process_enroll_key}, > {"Show Signature Database", eficonfig_process_show_signature_db}, %s/Signature Database/signature store/ > @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { > static struct eficonfig_item key_config_menu_items[] = { > {"Enroll New Key", eficonfig_process_enroll_key}, > {"Show Signature Database", eficonfig_process_show_signature_db}, %s/Signature Database/signature store/ Best regards Heinrich > + {"Delete Key", eficonfig_process_delete_key}, > {"Quit", eficonfig_process_quit}, > }; >
On Sun, Jul 10, 2022 at 12:10:13PM +0200, Heinrich Schuchardt wrote: > On 6/19/22 07:20, Masahisa Kojima wrote: > > This commit add the menu-driven interface to delete the > > signature database entry. > > EFI Signature Lists can contain the multiple signature > > entries, this menu can delete the indivisual entry. > > > > If the PK is enrolled and UEFI Secure Boot is in User Mode, > > Why don't you mention Deployed Mode? > > > user can not delete the existing signature lists since the > > signature lists must be signed by KEK or PK but signing > > information is not stored in the signature database. > > Updating PK with an empty value must be possible if if the new value is > signed with the old PK. > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 217 insertions(+), 1 deletion(-) > > > > diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c > > index 02ab8f8218..142bb4cef5 100644 > > --- a/cmd/eficonfig_sbkey.c > > +++ b/cmd/eficonfig_sbkey.c > > @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { > > /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ > > }; > > > > Please, add documentation to all functions. > > > +static int eficonfig_console_yes_no(void) > > +{ > > + int esc = 0; > > + enum bootmenu_key key = KEY_NONE; > > + > > + while (1) { > > + bootmenu_loop(NULL, &key, &esc); > > + > > + switch (key) { > > + case KEY_SELECT: > > + return 1; > > + case KEY_QUIT: > > + return 0; > > + default: > > + break; > > + } > > + } > > + > > + /* never happens */ > > + debug("eficonfig: this should not happen"); > > + return 0; > > If this code is unreachable, remove it. > > > +} > > + > > static void eficonfig_console_wait_enter(void) > > { > > int esc = 0; > > @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) > > > > /* never happens */ > > debug("eficonfig: this should not happen"); > > - return; > > Please remove unreachable code. > > > +} > > + > > +static bool is_setupmode(void) > > +{ > > + efi_status_t ret; > > + u8 setup_mode; > > + efi_uintn_t size; > > + > > + size = sizeof(setup_mode); > > + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, > > + NULL, &size, &setup_mode, NULL); > > + > > + return setup_mode == 1; > > } > > > > static bool is_secureboot_enabled(void) > > @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) > > return EFI_SUCCESS; > > } > > > > +static efi_status_t eficonfig_process_sigdata_delete(void *data) > > +{ > > + int yes_no; > > + struct eficonfig_sig_data *sg = data; > > + > > + display_sigdata_header(sg, "Delete"); > > + display_sigdata_info(sg); > > + > > + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); > > + yes_no = eficonfig_console_yes_no(); > > + if (!yes_no) > > + return EFI_NOT_READY; > > + > > + return EFI_SUCCESS; > > +} > > + > > +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, > > + struct eficonfig_sig_data *target, > > + struct list_head *siglist_list) > > +{ > > + u8 *dest, *start; > > + struct list_head *pos, *n; > > + u32 remain; > > + u32 size = *db_size; > > + u8 *end = (u8 *)db + size; > > + struct eficonfig_sig_data *sg; > > + > > + list_for_each_safe(pos, n, siglist_list) { > > + sg = list_entry(pos, struct eficonfig_sig_data, list); > > + if (sg->esl == target->esl && sg->esd == target->esd) { > > + remain = sg->esl->signature_list_size - > > + (sizeof(struct efi_signature_list) - > > + sg->esl->signature_header_size) - > > + sg->esl->signature_size; > > + if (remain > 0) { > > + /* only delete the single signature data */ > > + sg->esl->signature_list_size -= sg->esl->signature_size; > > + size -= sg->esl->signature_size; > > + dest = (u8 *)sg->esd; > > + start = (u8 *)sg->esd + sg->esl->signature_size; > > + } else { > > + /* delete entire signature list */ > > + dest = (u8 *)sg->esl; > > + start = (u8 *)sg->esl + sg->esl->signature_list_size; > > + size -= sg->esl->signature_list_size; > > + } > > + memmove(dest, start, (end - start)); > > + } > > + } > > + > > + *db_size = size; > > +} > > + > > +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) > > +{ > > + efi_status_t ret; > > + struct efi_time time; > > + efi_uintn_t total_size; > > + struct efi_variable_authentication_2 *auth; > > + > > + *new_db = NULL; > > + > > + /* > > + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS > > + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it > > + * without certificate data in it. > > + */ > > + total_size = sizeof(struct efi_variable_authentication_2) + *size; > > + > > + auth = calloc(1, total_size); > > + if (!auth) > > + return EFI_OUT_OF_RESOURCES; > > + > > + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); > > + if (ret != EFI_SUCCESS) { > > + free(auth); > > + return EFI_OUT_OF_RESOURCES; > > + } > > + time.pad1 = 0; > > + time.nanosecond = 0; > > + time.timezone = 0; > > + time.daylight = 0; > > + time.pad2 = 0; > > + memcpy(&auth->time_stamp, &time, sizeof(time)); > > + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); > > + auth->auth_info.hdr.wRevision = 0x0200; > > + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; > > + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); > > + if (db) > > + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); > > + > > + *new_db = auth; > > + *size = total_size; > > + > > + return EFI_SUCCESS; > > +} > > + > > static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, > > void *db, efi_uintn_t db_size, > > eficonfig_entry_func func, > > @@ -378,6 +510,68 @@ out: > > return ret; > > } > > > > +static efi_status_t process_delete_key(void *varname) > > +{ > > + u32 attr, i, count = 0; > > + efi_status_t ret; > > + struct eficonfig_item *menu_item = NULL, *iter; > > + void *db = NULL, *new_db = NULL; > > + efi_uintn_t db_size; > > + struct list_head siglist_list; > > + struct eficonfig_sig_data *selected; > > + > > + 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."); > > Please, use the terms of the UEFI specification. > %s/signature database/signature store/ Signature database is also a common term throughout the specification. See "section 32.4.1 Signature Database", for example. -Takahiro Akashi > > + return EFI_NOT_FOUND; > > + } > > + > > + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, > > + eficonfig_process_sigdata_delete, &selected, > > + &siglist_list, &count); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); > > + > > + if (ret == EFI_SUCCESS) { > > + delete_selected_signature_data(db, &db_size, selected, &siglist_list); > > + > > + ret = create_time_based_payload(db, &new_db, &db_size); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + attr = EFI_VARIABLE_NON_VOLATILE | > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS | > > + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > > + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), > > + attr, db_size, new_db, false); > > + if (ret != EFI_SUCCESS) { > > + eficonfig_print_msg("ERROR! Fail to delete signature database"); > > %s/Fail/Failed/ > > Please, use the terms of the UEFI specification. > %s/signature database/signature store/ > > > + goto out; > > + } > > + } > > + > > +out: > > + if (menu_item) { > > + iter = menu_item; > > + for (i = 0; i < count - 1; iter++, i++) { > > + free(iter->title); > > + free(iter->data); > > + } > > + } > > + > > + free(menu_item); > > + free(db); > > + free(new_db); > > + > > + /* to stay the parent menu */ > > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > > + > > + return ret; > > +} > > + > > static efi_status_t eficonfig_process_show_signature_db(void *data) > > { > > efi_status_t ret; > > @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) > > return ret; > > } > > > > +static efi_status_t eficonfig_process_delete_key(void *data) > > +{ > > + efi_status_t ret; > > + > > + if (!is_setupmode()) { > > + eficonfig_print_msg("Not in the SetupMode, can not delete."); > > Both in Setup Mode and in Audit Mode you should be able to edit keys. > > > + return EFI_SUCCESS; > > + } > > + > > + while (1) { > > + ret = process_delete_key(data); > > + if (ret != EFI_SUCCESS) > > + break; > > + } > > + > > + /* to stay the parent menu */ > > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > > + > > + return ret; > > +} > > + > > static struct eficonfig_item key_config_pk_menu_items[] = { > > {"Enroll New Key", eficonfig_process_enroll_key}, > > {"Show Signature Database", eficonfig_process_show_signature_db}, > > %s/Signature Database/signature store/ > > > @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { > > static struct eficonfig_item key_config_menu_items[] = { > > {"Enroll New Key", eficonfig_process_enroll_key}, > > {"Show Signature Database", eficonfig_process_show_signature_db}, > > %s/Signature Database/signature store/ > > Best regards > > Heinrich > > > + {"Delete Key", eficonfig_process_delete_key}, > > {"Quit", eficonfig_process_quit}, > > }; > > >
Hi Heinrich, Takahiro, On Sun, 10 Jul 2022 at 19:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 6/19/22 07:20, Masahisa Kojima wrote: > > This commit add the menu-driven interface to delete the > > signature database entry. > > EFI Signature Lists can contain the multiple signature > > entries, this menu can delete the indivisual entry. > > > > If the PK is enrolled and UEFI Secure Boot is in User Mode, > > Why don't you mention Deployed Mode? Yes, I will mention DeployedMode. > > > user can not delete the existing signature lists since the > > signature lists must be signed by KEK or PK but signing > > information is not stored in the signature database. > > Updating PK with an empty value must be possible if if the new value is > signed with the old PK. Yes, I will add this description. > > > > > Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > > --- > > cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 217 insertions(+), 1 deletion(-) > > > > diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c > > index 02ab8f8218..142bb4cef5 100644 > > --- a/cmd/eficonfig_sbkey.c > > +++ b/cmd/eficonfig_sbkey.c > > @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { > > /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ > > }; > > > > Please, add documentation to all functions. OK. > > > +static int eficonfig_console_yes_no(void) > > +{ > > + int esc = 0; > > + enum bootmenu_key key = KEY_NONE; > > + > > + while (1) { > > + bootmenu_loop(NULL, &key, &esc); > > + > > + switch (key) { > > + case KEY_SELECT: > > + return 1; > > + case KEY_QUIT: > > + return 0; > > + default: > > + break; > > + } > > + } > > + > > + /* never happens */ > > + debug("eficonfig: this should not happen"); > > + return 0; > > If this code is unreachable, remove it. OK. > > > +} > > + > > static void eficonfig_console_wait_enter(void) > > { > > int esc = 0; > > @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) > > > > /* never happens */ > > debug("eficonfig: this should not happen"); > > - return; > > Please remove unreachable code. OK. > > > +} > > + > > +static bool is_setupmode(void) > > +{ > > + efi_status_t ret; > > + u8 setup_mode; > > + efi_uintn_t size; > > + > > + size = sizeof(setup_mode); > > + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, > > + NULL, &size, &setup_mode, NULL); > > + > > + return setup_mode == 1; > > } > > > > static bool is_secureboot_enabled(void) > > @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) > > return EFI_SUCCESS; > > } > > > > +static efi_status_t eficonfig_process_sigdata_delete(void *data) > > +{ > > + int yes_no; > > + struct eficonfig_sig_data *sg = data; > > + > > + display_sigdata_header(sg, "Delete"); > > + display_sigdata_info(sg); > > + > > + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); > > + yes_no = eficonfig_console_yes_no(); > > + if (!yes_no) > > + return EFI_NOT_READY; > > + > > + return EFI_SUCCESS; > > +} > > + > > +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, > > + struct eficonfig_sig_data *target, > > + struct list_head *siglist_list) > > +{ > > + u8 *dest, *start; > > + struct list_head *pos, *n; > > + u32 remain; > > + u32 size = *db_size; > > + u8 *end = (u8 *)db + size; > > + struct eficonfig_sig_data *sg; > > + > > + list_for_each_safe(pos, n, siglist_list) { > > + sg = list_entry(pos, struct eficonfig_sig_data, list); > > + if (sg->esl == target->esl && sg->esd == target->esd) { > > + remain = sg->esl->signature_list_size - > > + (sizeof(struct efi_signature_list) - > > + sg->esl->signature_header_size) - > > + sg->esl->signature_size; > > + if (remain > 0) { > > + /* only delete the single signature data */ > > + sg->esl->signature_list_size -= sg->esl->signature_size; > > + size -= sg->esl->signature_size; > > + dest = (u8 *)sg->esd; > > + start = (u8 *)sg->esd + sg->esl->signature_size; > > + } else { > > + /* delete entire signature list */ > > + dest = (u8 *)sg->esl; > > + start = (u8 *)sg->esl + sg->esl->signature_list_size; > > + size -= sg->esl->signature_list_size; > > + } > > + memmove(dest, start, (end - start)); > > + } > > + } > > + > > + *db_size = size; > > +} > > + > > +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) > > +{ > > + efi_status_t ret; > > + struct efi_time time; > > + efi_uintn_t total_size; > > + struct efi_variable_authentication_2 *auth; > > + > > + *new_db = NULL; > > + > > + /* > > + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS > > + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it > > + * without certificate data in it. > > + */ > > + total_size = sizeof(struct efi_variable_authentication_2) + *size; > > + > > + auth = calloc(1, total_size); > > + if (!auth) > > + return EFI_OUT_OF_RESOURCES; > > + > > + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); > > + if (ret != EFI_SUCCESS) { > > + free(auth); > > + return EFI_OUT_OF_RESOURCES; > > + } > > + time.pad1 = 0; > > + time.nanosecond = 0; > > + time.timezone = 0; > > + time.daylight = 0; > > + time.pad2 = 0; > > + memcpy(&auth->time_stamp, &time, sizeof(time)); > > + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); > > + auth->auth_info.hdr.wRevision = 0x0200; > > + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; > > + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); > > + if (db) > > + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); > > + > > + *new_db = auth; > > + *size = total_size; > > + > > + return EFI_SUCCESS; > > +} > > + > > static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, > > void *db, efi_uintn_t db_size, > > eficonfig_entry_func func, > > @@ -378,6 +510,68 @@ out: > > return ret; > > } > > > > +static efi_status_t process_delete_key(void *varname) > > +{ > > + u32 attr, i, count = 0; > > + efi_status_t ret; > > + struct eficonfig_item *menu_item = NULL, *iter; > > + void *db = NULL, *new_db = NULL; > > + efi_uintn_t db_size; > > + struct list_head siglist_list; > > + struct eficonfig_sig_data *selected; > > + > > + 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."); > > Please, use the terms of the UEFI specification. > %s/signature database/signature store/ As Takahiro already mentioned, UEFI specifications use the following term. - signature database - signature store - signature list The UEFI specification has section "32.4.1 Signature Database" and I think "signature database" is most common in the spec. > > > + return EFI_NOT_FOUND; > > + } > > + > > + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, > > + eficonfig_process_sigdata_delete, &selected, > > + &siglist_list, &count); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); > > + > > + if (ret == EFI_SUCCESS) { > > + delete_selected_signature_data(db, &db_size, selected, &siglist_list); > > + > > + ret = create_time_based_payload(db, &new_db, &db_size); > > + if (ret != EFI_SUCCESS) > > + goto out; > > + > > + attr = EFI_VARIABLE_NON_VOLATILE | > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS | > > + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > > + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), > > + attr, db_size, new_db, false); > > + if (ret != EFI_SUCCESS) { > > + eficonfig_print_msg("ERROR! Fail to delete signature database"); > > %s/Fail/Failed/ OK. > > Please, use the terms of the UEFI specification. > %s/signature database/signature store/ Same as above. > > > + goto out; > > + } > > + } > > + > > +out: > > + if (menu_item) { > > + iter = menu_item; > > + for (i = 0; i < count - 1; iter++, i++) { > > + free(iter->title); > > + free(iter->data); > > + } > > + } > > + > > + free(menu_item); > > + free(db); > > + free(new_db); > > + > > + /* to stay the parent menu */ > > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > > + > > + return ret; > > +} > > + > > static efi_status_t eficonfig_process_show_signature_db(void *data) > > { > > efi_status_t ret; > > @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) > > return ret; > > } > > > > +static efi_status_t eficonfig_process_delete_key(void *data) > > +{ > > + efi_status_t ret; > > + > > + if (!is_setupmode()) { > > + eficonfig_print_msg("Not in the SetupMode, can not delete."); > > Both in Setup Mode and in Audit Mode you should be able to edit keys. Yes, I will update the code and message. > > > + return EFI_SUCCESS; > > + } > > + > > + while (1) { > > + ret = process_delete_key(data); > > + if (ret != EFI_SUCCESS) > > + break; > > + } > > + > > + /* to stay the parent menu */ > > + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > > + > > + return ret; > > +} > > + > > static struct eficonfig_item key_config_pk_menu_items[] = { > > {"Enroll New Key", eficonfig_process_enroll_key}, > > {"Show Signature Database", eficonfig_process_show_signature_db}, > > %s/Signature Database/signature store/ Same as above. > > > @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { > > static struct eficonfig_item key_config_menu_items[] = { > > {"Enroll New Key", eficonfig_process_enroll_key}, > > {"Show Signature Database", eficonfig_process_show_signature_db}, > > %s/Signature Database/signature store/ Same as above. Thank you for your review. Regards, Masahias Kojima > > Best regards > > Heinrich > > > + {"Delete Key", eficonfig_process_delete_key}, > > {"Quit", eficonfig_process_quit}, > > }; > > >
On 7/12/22 09:13, Masahisa Kojima wrote: > Hi Heinrich, Takahiro, > > On Sun, 10 Jul 2022 at 19:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 6/19/22 07:20, Masahisa Kojima wrote: >>> This commit add the menu-driven interface to delete the >>> signature database entry. >>> EFI Signature Lists can contain the multiple signature >>> entries, this menu can delete the indivisual entry. >>> >>> If the PK is enrolled and UEFI Secure Boot is in User Mode, >> >> Why don't you mention Deployed Mode? > > Yes, I will mention DeployedMode. > >> >>> user can not delete the existing signature lists since the >>> signature lists must be signed by KEK or PK but signing >>> information is not stored in the signature database. >> >> Updating PK with an empty value must be possible if if the new value is >> signed with the old PK. > > Yes, I will add this description. > >> >>> >>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> >>> --- >>> cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 217 insertions(+), 1 deletion(-) >>> >>> diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c >>> index 02ab8f8218..142bb4cef5 100644 >>> --- a/cmd/eficonfig_sbkey.c >>> +++ b/cmd/eficonfig_sbkey.c >>> @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { >>> /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ >>> }; >>> >> >> Please, add documentation to all functions. > > OK. > >> >>> +static int eficonfig_console_yes_no(void) >>> +{ >>> + int esc = 0; >>> + enum bootmenu_key key = KEY_NONE; >>> + >>> + while (1) { >>> + bootmenu_loop(NULL, &key, &esc); >>> + >>> + switch (key) { >>> + case KEY_SELECT: >>> + return 1; >>> + case KEY_QUIT: >>> + return 0; >>> + default: >>> + break; >>> + } >>> + } >>> + >>> + /* never happens */ >>> + debug("eficonfig: this should not happen"); >>> + return 0; >> >> If this code is unreachable, remove it. > > OK. > >> >>> +} >>> + >>> static void eficonfig_console_wait_enter(void) >>> { >>> int esc = 0; >>> @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) >>> >>> /* never happens */ >>> debug("eficonfig: this should not happen"); >>> - return; >> >> Please remove unreachable code. > > OK. > >> >>> +} >>> + >>> +static bool is_setupmode(void) >>> +{ >>> + efi_status_t ret; >>> + u8 setup_mode; >>> + efi_uintn_t size; >>> + >>> + size = sizeof(setup_mode); >>> + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, >>> + NULL, &size, &setup_mode, NULL); >>> + >>> + return setup_mode == 1; >>> } >>> >>> static bool is_secureboot_enabled(void) >>> @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) >>> return EFI_SUCCESS; >>> } >>> >>> +static efi_status_t eficonfig_process_sigdata_delete(void *data) >>> +{ >>> + int yes_no; >>> + struct eficonfig_sig_data *sg = data; >>> + >>> + display_sigdata_header(sg, "Delete"); >>> + display_sigdata_info(sg); >>> + >>> + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); >>> + yes_no = eficonfig_console_yes_no(); >>> + if (!yes_no) >>> + return EFI_NOT_READY; >>> + >>> + return EFI_SUCCESS; >>> +} >>> + >>> +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, >>> + struct eficonfig_sig_data *target, >>> + struct list_head *siglist_list) >>> +{ >>> + u8 *dest, *start; >>> + struct list_head *pos, *n; >>> + u32 remain; >>> + u32 size = *db_size; >>> + u8 *end = (u8 *)db + size; >>> + struct eficonfig_sig_data *sg; >>> + >>> + list_for_each_safe(pos, n, siglist_list) { >>> + sg = list_entry(pos, struct eficonfig_sig_data, list); >>> + if (sg->esl == target->esl && sg->esd == target->esd) { >>> + remain = sg->esl->signature_list_size - >>> + (sizeof(struct efi_signature_list) - >>> + sg->esl->signature_header_size) - >>> + sg->esl->signature_size; >>> + if (remain > 0) { >>> + /* only delete the single signature data */ >>> + sg->esl->signature_list_size -= sg->esl->signature_size; >>> + size -= sg->esl->signature_size; >>> + dest = (u8 *)sg->esd; >>> + start = (u8 *)sg->esd + sg->esl->signature_size; >>> + } else { >>> + /* delete entire signature list */ >>> + dest = (u8 *)sg->esl; >>> + start = (u8 *)sg->esl + sg->esl->signature_list_size; >>> + size -= sg->esl->signature_list_size; >>> + } >>> + memmove(dest, start, (end - start)); >>> + } >>> + } >>> + >>> + *db_size = size; >>> +} >>> + >>> +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) >>> +{ >>> + efi_status_t ret; >>> + struct efi_time time; >>> + efi_uintn_t total_size; >>> + struct efi_variable_authentication_2 *auth; >>> + >>> + *new_db = NULL; >>> + >>> + /* >>> + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS >>> + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it >>> + * without certificate data in it. >>> + */ >>> + total_size = sizeof(struct efi_variable_authentication_2) + *size; >>> + >>> + auth = calloc(1, total_size); >>> + if (!auth) >>> + return EFI_OUT_OF_RESOURCES; >>> + >>> + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); >>> + if (ret != EFI_SUCCESS) { >>> + free(auth); >>> + return EFI_OUT_OF_RESOURCES; >>> + } >>> + time.pad1 = 0; >>> + time.nanosecond = 0; >>> + time.timezone = 0; >>> + time.daylight = 0; >>> + time.pad2 = 0; >>> + memcpy(&auth->time_stamp, &time, sizeof(time)); >>> + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); >>> + auth->auth_info.hdr.wRevision = 0x0200; >>> + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; >>> + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); >>> + if (db) >>> + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); >>> + >>> + *new_db = auth; >>> + *size = total_size; >>> + >>> + return EFI_SUCCESS; >>> +} >>> + >>> static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, >>> void *db, efi_uintn_t db_size, >>> eficonfig_entry_func func, >>> @@ -378,6 +510,68 @@ out: >>> return ret; >>> } >>> >>> +static efi_status_t process_delete_key(void *varname) >>> +{ >>> + u32 attr, i, count = 0; >>> + efi_status_t ret; >>> + struct eficonfig_item *menu_item = NULL, *iter; >>> + void *db = NULL, *new_db = NULL; >>> + efi_uintn_t db_size; >>> + struct list_head siglist_list; >>> + struct eficonfig_sig_data *selected; >>> + >>> + 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."); >> >> Please, use the terms of the UEFI specification. >> %s/signature database/signature store/ > > As Takahiro already mentioned, UEFI specifications use the following term. > - signature database Yes. The term signature database is used in references to db, dbx, dbr, dbt. > - signature store Signature store is only used in the description of dbDefault, dbrDefault, dbtDefault, dbxDefault. > - signature list Signature list refers to one of the esl files concatenated in a signature store. Best regards Heinrich > > The UEFI specification has section "32.4.1 Signature Database" and > I think "signature database" is most common in the spec. > >> >>> + return EFI_NOT_FOUND; >>> + } >>> + >>> + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, >>> + eficonfig_process_sigdata_delete, &selected, >>> + &siglist_list, &count); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); >>> + >>> + if (ret == EFI_SUCCESS) { >>> + delete_selected_signature_data(db, &db_size, selected, &siglist_list); >>> + >>> + ret = create_time_based_payload(db, &new_db, &db_size); >>> + if (ret != EFI_SUCCESS) >>> + goto out; >>> + >>> + attr = EFI_VARIABLE_NON_VOLATILE | >>> + EFI_VARIABLE_BOOTSERVICE_ACCESS | >>> + EFI_VARIABLE_RUNTIME_ACCESS | >>> + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; >>> + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), >>> + attr, db_size, new_db, false); >>> + if (ret != EFI_SUCCESS) { >>> + eficonfig_print_msg("ERROR! Fail to delete signature database"); >> >> %s/Fail/Failed/ > > OK. > >> >> Please, use the terms of the UEFI specification. >> %s/signature database/signature store/ > > Same as above. > >> >>> + goto out; >>> + } >>> + } >>> + >>> +out: >>> + if (menu_item) { >>> + iter = menu_item; >>> + for (i = 0; i < count - 1; iter++, i++) { >>> + free(iter->title); >>> + free(iter->data); >>> + } >>> + } >>> + >>> + free(menu_item); >>> + free(db); >>> + free(new_db); >>> + >>> + /* to stay the parent menu */ >>> + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; >>> + >>> + return ret; >>> +} >>> + >>> static efi_status_t eficonfig_process_show_signature_db(void *data) >>> { >>> efi_status_t ret; >>> @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) >>> return ret; >>> } >>> >>> +static efi_status_t eficonfig_process_delete_key(void *data) >>> +{ >>> + efi_status_t ret; >>> + >>> + if (!is_setupmode()) { >>> + eficonfig_print_msg("Not in the SetupMode, can not delete."); >> >> Both in Setup Mode and in Audit Mode you should be able to edit keys. > > Yes, I will update the code and message. > >> >>> + return EFI_SUCCESS; >>> + } >>> + >>> + while (1) { >>> + ret = process_delete_key(data); >>> + if (ret != EFI_SUCCESS) >>> + break; >>> + } >>> + >>> + /* to stay the parent menu */ >>> + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; >>> + >>> + return ret; >>> +} >>> + >>> static struct eficonfig_item key_config_pk_menu_items[] = { >>> {"Enroll New Key", eficonfig_process_enroll_key}, >>> {"Show Signature Database", eficonfig_process_show_signature_db}, >> >> %s/Signature Database/signature store/ > > Same as above. > >> >>> @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { >>> static struct eficonfig_item key_config_menu_items[] = { >>> {"Enroll New Key", eficonfig_process_enroll_key}, >>> {"Show Signature Database", eficonfig_process_show_signature_db}, >> >> %s/Signature Database/signature store/ > > Same as above. > > Thank you for your review. > > Regards, > Masahias Kojima > >> >> Best regards >> >> Heinrich >> >>> + {"Delete Key", eficonfig_process_delete_key}, >>> {"Quit", eficonfig_process_quit}, >>> }; >>> >>
On Tue, 12 Jul 2022 at 17:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 7/12/22 09:13, Masahisa Kojima wrote: > > Hi Heinrich, Takahiro, > > > > On Sun, 10 Jul 2022 at 19:10, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 6/19/22 07:20, Masahisa Kojima wrote: > >>> This commit add the menu-driven interface to delete the > >>> signature database entry. > >>> EFI Signature Lists can contain the multiple signature > >>> entries, this menu can delete the indivisual entry. > >>> > >>> If the PK is enrolled and UEFI Secure Boot is in User Mode, > >> > >> Why don't you mention Deployed Mode? > > > > Yes, I will mention DeployedMode. > > > >> > >>> user can not delete the existing signature lists since the > >>> signature lists must be signed by KEK or PK but signing > >>> information is not stored in the signature database. > >> > >> Updating PK with an empty value must be possible if if the new value is > >> signed with the old PK. > > > > Yes, I will add this description. > > > >> > >>> > >>> Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> > >>> --- > >>> cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- > >>> 1 file changed, 217 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c > >>> index 02ab8f8218..142bb4cef5 100644 > >>> --- a/cmd/eficonfig_sbkey.c > >>> +++ b/cmd/eficonfig_sbkey.c > >>> @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { > >>> /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ > >>> }; > >>> > >> > >> Please, add documentation to all functions. > > > > OK. > > > >> > >>> +static int eficonfig_console_yes_no(void) > >>> +{ > >>> + int esc = 0; > >>> + enum bootmenu_key key = KEY_NONE; > >>> + > >>> + while (1) { > >>> + bootmenu_loop(NULL, &key, &esc); > >>> + > >>> + switch (key) { > >>> + case KEY_SELECT: > >>> + return 1; > >>> + case KEY_QUIT: > >>> + return 0; > >>> + default: > >>> + break; > >>> + } > >>> + } > >>> + > >>> + /* never happens */ > >>> + debug("eficonfig: this should not happen"); > >>> + return 0; > >> > >> If this code is unreachable, remove it. > > > > OK. > > > >> > >>> +} > >>> + > >>> static void eficonfig_console_wait_enter(void) > >>> { > >>> int esc = 0; > >>> @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) > >>> > >>> /* never happens */ > >>> debug("eficonfig: this should not happen"); > >>> - return; > >> > >> Please remove unreachable code. > > > > OK. > > > >> > >>> +} > >>> + > >>> +static bool is_setupmode(void) > >>> +{ > >>> + efi_status_t ret; > >>> + u8 setup_mode; > >>> + efi_uintn_t size; > >>> + > >>> + size = sizeof(setup_mode); > >>> + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, > >>> + NULL, &size, &setup_mode, NULL); > >>> + > >>> + return setup_mode == 1; > >>> } > >>> > >>> static bool is_secureboot_enabled(void) > >>> @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) > >>> return EFI_SUCCESS; > >>> } > >>> > >>> +static efi_status_t eficonfig_process_sigdata_delete(void *data) > >>> +{ > >>> + int yes_no; > >>> + struct eficonfig_sig_data *sg = data; > >>> + > >>> + display_sigdata_header(sg, "Delete"); > >>> + display_sigdata_info(sg); > >>> + > >>> + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); > >>> + yes_no = eficonfig_console_yes_no(); > >>> + if (!yes_no) > >>> + return EFI_NOT_READY; > >>> + > >>> + return EFI_SUCCESS; > >>> +} > >>> + > >>> +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, > >>> + struct eficonfig_sig_data *target, > >>> + struct list_head *siglist_list) > >>> +{ > >>> + u8 *dest, *start; > >>> + struct list_head *pos, *n; > >>> + u32 remain; > >>> + u32 size = *db_size; > >>> + u8 *end = (u8 *)db + size; > >>> + struct eficonfig_sig_data *sg; > >>> + > >>> + list_for_each_safe(pos, n, siglist_list) { > >>> + sg = list_entry(pos, struct eficonfig_sig_data, list); > >>> + if (sg->esl == target->esl && sg->esd == target->esd) { > >>> + remain = sg->esl->signature_list_size - > >>> + (sizeof(struct efi_signature_list) - > >>> + sg->esl->signature_header_size) - > >>> + sg->esl->signature_size; > >>> + if (remain > 0) { > >>> + /* only delete the single signature data */ > >>> + sg->esl->signature_list_size -= sg->esl->signature_size; > >>> + size -= sg->esl->signature_size; > >>> + dest = (u8 *)sg->esd; > >>> + start = (u8 *)sg->esd + sg->esl->signature_size; > >>> + } else { > >>> + /* delete entire signature list */ > >>> + dest = (u8 *)sg->esl; > >>> + start = (u8 *)sg->esl + sg->esl->signature_list_size; > >>> + size -= sg->esl->signature_list_size; > >>> + } > >>> + memmove(dest, start, (end - start)); > >>> + } > >>> + } > >>> + > >>> + *db_size = size; > >>> +} > >>> + > >>> +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) > >>> +{ > >>> + efi_status_t ret; > >>> + struct efi_time time; > >>> + efi_uintn_t total_size; > >>> + struct efi_variable_authentication_2 *auth; > >>> + > >>> + *new_db = NULL; > >>> + > >>> + /* > >>> + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS > >>> + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it > >>> + * without certificate data in it. > >>> + */ > >>> + total_size = sizeof(struct efi_variable_authentication_2) + *size; > >>> + > >>> + auth = calloc(1, total_size); > >>> + if (!auth) > >>> + return EFI_OUT_OF_RESOURCES; > >>> + > >>> + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); > >>> + if (ret != EFI_SUCCESS) { > >>> + free(auth); > >>> + return EFI_OUT_OF_RESOURCES; > >>> + } > >>> + time.pad1 = 0; > >>> + time.nanosecond = 0; > >>> + time.timezone = 0; > >>> + time.daylight = 0; > >>> + time.pad2 = 0; > >>> + memcpy(&auth->time_stamp, &time, sizeof(time)); > >>> + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); > >>> + auth->auth_info.hdr.wRevision = 0x0200; > >>> + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; > >>> + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); > >>> + if (db) > >>> + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); > >>> + > >>> + *new_db = auth; > >>> + *size = total_size; > >>> + > >>> + return EFI_SUCCESS; > >>> +} > >>> + > >>> static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, > >>> void *db, efi_uintn_t db_size, > >>> eficonfig_entry_func func, > >>> @@ -378,6 +510,68 @@ out: > >>> return ret; > >>> } > >>> > >>> +static efi_status_t process_delete_key(void *varname) > >>> +{ > >>> + u32 attr, i, count = 0; > >>> + efi_status_t ret; > >>> + struct eficonfig_item *menu_item = NULL, *iter; > >>> + void *db = NULL, *new_db = NULL; > >>> + efi_uintn_t db_size; > >>> + struct list_head siglist_list; > >>> + struct eficonfig_sig_data *selected; > >>> + > >>> + 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."); > >> > >> Please, use the terms of the UEFI specification. > >> %s/signature database/signature store/ > > > > As Takahiro already mentioned, UEFI specifications use the following term. > > - signature database > > Yes. > > The term signature database is used in references to db, dbx, dbr, dbt. > > > - signature store > > Signature store is only used in the description of dbDefault, > dbrDefault, dbtDefault, dbxDefault. > > > - signature list > > Signature list refers to one of the esl files concatenated in a > signature store. Thank you for adding the information. The KEK variable description in UEFI specification is: "The Key Exchange Key Signature Database." So I would like to use "signature database" in this patch series. Thanks, Masahisa Kojima > > Best regards > > Heinrich > > > > > The UEFI specification has section "32.4.1 Signature Database" and > > I think "signature database" is most common in the spec. > > > >> > >>> + return EFI_NOT_FOUND; > >>> + } > >>> + > >>> + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, > >>> + eficonfig_process_sigdata_delete, &selected, > >>> + &siglist_list, &count); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); > >>> + > >>> + if (ret == EFI_SUCCESS) { > >>> + delete_selected_signature_data(db, &db_size, selected, &siglist_list); > >>> + > >>> + ret = create_time_based_payload(db, &new_db, &db_size); > >>> + if (ret != EFI_SUCCESS) > >>> + goto out; > >>> + > >>> + attr = EFI_VARIABLE_NON_VOLATILE | > >>> + EFI_VARIABLE_BOOTSERVICE_ACCESS | > >>> + EFI_VARIABLE_RUNTIME_ACCESS | > >>> + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > >>> + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), > >>> + attr, db_size, new_db, false); > >>> + if (ret != EFI_SUCCESS) { > >>> + eficonfig_print_msg("ERROR! Fail to delete signature database"); > >> > >> %s/Fail/Failed/ > > > > OK. > > > >> > >> Please, use the terms of the UEFI specification. > >> %s/signature database/signature store/ > > > > Same as above. > > > >> > >>> + goto out; > >>> + } > >>> + } > >>> + > >>> +out: > >>> + if (menu_item) { > >>> + iter = menu_item; > >>> + for (i = 0; i < count - 1; iter++, i++) { > >>> + free(iter->title); > >>> + free(iter->data); > >>> + } > >>> + } > >>> + > >>> + free(menu_item); > >>> + free(db); > >>> + free(new_db); > >>> + > >>> + /* to stay the parent menu */ > >>> + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > >>> + > >>> + return ret; > >>> +} > >>> + > >>> static efi_status_t eficonfig_process_show_signature_db(void *data) > >>> { > >>> efi_status_t ret; > >>> @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) > >>> return ret; > >>> } > >>> > >>> +static efi_status_t eficonfig_process_delete_key(void *data) > >>> +{ > >>> + efi_status_t ret; > >>> + > >>> + if (!is_setupmode()) { > >>> + eficonfig_print_msg("Not in the SetupMode, can not delete."); > >> > >> Both in Setup Mode and in Audit Mode you should be able to edit keys. > > > > Yes, I will update the code and message. > > > >> > >>> + return EFI_SUCCESS; > >>> + } > >>> + > >>> + while (1) { > >>> + ret = process_delete_key(data); > >>> + if (ret != EFI_SUCCESS) > >>> + break; > >>> + } > >>> + > >>> + /* to stay the parent menu */ > >>> + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; > >>> + > >>> + return ret; > >>> +} > >>> + > >>> static struct eficonfig_item key_config_pk_menu_items[] = { > >>> {"Enroll New Key", eficonfig_process_enroll_key}, > >>> {"Show Signature Database", eficonfig_process_show_signature_db}, > >> > >> %s/Signature Database/signature store/ > > > > Same as above. > > > >> > >>> @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { > >>> static struct eficonfig_item key_config_menu_items[] = { > >>> {"Enroll New Key", eficonfig_process_enroll_key}, > >>> {"Show Signature Database", eficonfig_process_show_signature_db}, > >> > >> %s/Signature Database/signature store/ > > > > Same as above. > > > > Thank you for your review. > > > > Regards, > > Masahias Kojima > > > >> > >> Best regards > >> > >> Heinrich > >> > >>> + {"Delete Key", eficonfig_process_delete_key}, > >>> {"Quit", eficonfig_process_quit}, > >>> }; > >>> > >> >
diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c index 02ab8f8218..142bb4cef5 100644 --- a/cmd/eficonfig_sbkey.c +++ b/cmd/eficonfig_sbkey.c @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { /* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ }; +static int eficonfig_console_yes_no(void) +{ + int esc = 0; + enum bootmenu_key key = KEY_NONE; + + while (1) { + bootmenu_loop(NULL, &key, &esc); + + switch (key) { + case KEY_SELECT: + return 1; + case KEY_QUIT: + return 0; + default: + break; + } + } + + /* never happens */ + debug("eficonfig: this should not happen"); + return 0; +} + static void eficonfig_console_wait_enter(void) { int esc = 0; @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) /* never happens */ debug("eficonfig: this should not happen"); - return; +} + +static bool is_setupmode(void) +{ + efi_status_t ret; + u8 setup_mode; + efi_uintn_t size; + + size = sizeof(setup_mode); + ret = efi_get_variable_int(u"SetupMode", &efi_global_variable_guid, + NULL, &size, &setup_mode, NULL); + + return setup_mode == 1; } static bool is_secureboot_enabled(void) @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) return EFI_SUCCESS; } +static efi_status_t eficonfig_process_sigdata_delete(void *data) +{ + int yes_no; + struct eficonfig_sig_data *sg = data; + + display_sigdata_header(sg, "Delete"); + display_sigdata_info(sg); + + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); + yes_no = eficonfig_console_yes_no(); + if (!yes_no) + return EFI_NOT_READY; + + return EFI_SUCCESS; +} + +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, + struct eficonfig_sig_data *target, + struct list_head *siglist_list) +{ + u8 *dest, *start; + struct list_head *pos, *n; + u32 remain; + u32 size = *db_size; + u8 *end = (u8 *)db + size; + struct eficonfig_sig_data *sg; + + list_for_each_safe(pos, n, siglist_list) { + sg = list_entry(pos, struct eficonfig_sig_data, list); + if (sg->esl == target->esl && sg->esd == target->esd) { + remain = sg->esl->signature_list_size - + (sizeof(struct efi_signature_list) - + sg->esl->signature_header_size) - + sg->esl->signature_size; + if (remain > 0) { + /* only delete the single signature data */ + sg->esl->signature_list_size -= sg->esl->signature_size; + size -= sg->esl->signature_size; + dest = (u8 *)sg->esd; + start = (u8 *)sg->esd + sg->esl->signature_size; + } else { + /* delete entire signature list */ + dest = (u8 *)sg->esl; + start = (u8 *)sg->esl + sg->esl->signature_list_size; + size -= sg->esl->signature_list_size; + } + memmove(dest, start, (end - start)); + } + } + + *db_size = size; +} + +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) +{ + efi_status_t ret; + struct efi_time time; + efi_uintn_t total_size; + struct efi_variable_authentication_2 *auth; + + *new_db = NULL; + + /* + * SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS + * attribute requires EFI_VARIABLE_AUTHENTICATED_2 descriptor, prepare it + * without certificate data in it. + */ + total_size = sizeof(struct efi_variable_authentication_2) + *size; + + auth = calloc(1, total_size); + if (!auth) + return EFI_OUT_OF_RESOURCES; + + ret = EFI_CALL((*efi_runtime_services.get_time)(&time, NULL)); + if (ret != EFI_SUCCESS) { + free(auth); + return EFI_OUT_OF_RESOURCES; + } + time.pad1 = 0; + time.nanosecond = 0; + time.timezone = 0; + time.daylight = 0; + time.pad2 = 0; + memcpy(&auth->time_stamp, &time, sizeof(time)); + auth->auth_info.hdr.dwLength = sizeof(struct win_certificate_uefi_guid); + auth->auth_info.hdr.wRevision = 0x0200; + auth->auth_info.hdr.wCertificateType = WIN_CERT_TYPE_EFI_GUID; + guidcpy(&auth->auth_info.cert_type, &efi_guid_cert_type_pkcs7); + if (db) + memcpy((u8 *)auth + sizeof(struct efi_variable_authentication_2), db, *size); + + *new_db = auth; + *size = total_size; + + return EFI_SUCCESS; +} + static efi_status_t prepare_signature_db_list(struct eficonfig_item **output, void *varname, void *db, efi_uintn_t db_size, eficonfig_entry_func func, @@ -378,6 +510,68 @@ out: return ret; } +static efi_status_t process_delete_key(void *varname) +{ + u32 attr, i, count = 0; + efi_status_t ret; + struct eficonfig_item *menu_item = NULL, *iter; + void *db = NULL, *new_db = NULL; + efi_uintn_t db_size; + struct list_head siglist_list; + struct eficonfig_sig_data *selected; + + 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; + } + + ret = prepare_signature_db_list(&menu_item, varname, db, db_size, + eficonfig_process_sigdata_delete, &selected, + &siglist_list, &count); + if (ret != EFI_SUCCESS) + goto out; + + ret = eficonfig_process_common(menu_item, count, " ** Delete Key **"); + + if (ret == EFI_SUCCESS) { + delete_selected_signature_data(db, &db_size, selected, &siglist_list); + + ret = create_time_based_payload(db, &new_db, &db_size); + if (ret != EFI_SUCCESS) + goto out; + + attr = EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; + ret = efi_set_variable_int((u16 *)varname, efi_auth_var_get_guid((u16 *)varname), + attr, db_size, new_db, false); + if (ret != EFI_SUCCESS) { + eficonfig_print_msg("ERROR! Fail to delete signature database"); + goto out; + } + } + +out: + if (menu_item) { + iter = menu_item; + for (i = 0; i < count - 1; iter++, i++) { + free(iter->title); + free(iter->data); + } + } + + free(menu_item); + free(db); + free(new_db); + + /* to stay the parent menu */ + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; + + return ret; +} + static efi_status_t eficonfig_process_show_signature_db(void *data) { efi_status_t ret; @@ -394,6 +588,27 @@ static efi_status_t eficonfig_process_show_signature_db(void *data) return ret; } +static efi_status_t eficonfig_process_delete_key(void *data) +{ + efi_status_t ret; + + if (!is_setupmode()) { + eficonfig_print_msg("Not in the SetupMode, can not delete."); + return EFI_SUCCESS; + } + + while (1) { + ret = process_delete_key(data); + if (ret != EFI_SUCCESS) + break; + } + + /* to stay the parent menu */ + ret = (ret == EFI_ABORTED) ? EFI_NOT_READY : ret; + + return ret; +} + static struct eficonfig_item key_config_pk_menu_items[] = { {"Enroll New Key", eficonfig_process_enroll_key}, {"Show Signature Database", eficonfig_process_show_signature_db}, @@ -403,6 +618,7 @@ static struct eficonfig_item key_config_pk_menu_items[] = { static struct eficonfig_item key_config_menu_items[] = { {"Enroll New Key", eficonfig_process_enroll_key}, {"Show Signature Database", eficonfig_process_show_signature_db}, + {"Delete Key", eficonfig_process_delete_key}, {"Quit", eficonfig_process_quit}, };
This commit add the menu-driven interface to delete the signature database entry. EFI Signature Lists can contain the multiple signature entries, this menu can delete the indivisual entry. If the PK is enrolled and UEFI Secure Boot is in User Mode, user can not delete the existing signature lists since the signature lists must be signed by KEK or PK but signing information is not stored in the signature database. Signed-off-by: Masahisa Kojima <masahisa.kojima@linaro.org> --- cmd/eficonfig_sbkey.c | 218 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 217 insertions(+), 1 deletion(-)