[RFC PATCH 3/3] eficonfig: add "Delete Key" menu entry
Masahisa Kojima
masahisa.kojima at linaro.org
Tue Jul 12 13:15:08 CEST 2022
On Tue, 12 Jul 2022 at 17:02, Heinrich Schuchardt <xypron.glpk at 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 at 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 at 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},
> >>> };
> >>>
> >>
>
More information about the U-Boot
mailing list