[RFC PATCH 3/3] eficonfig: add "Delete Key" menu entry

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Jul 12 10:02:50 CEST 2022


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.

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