> On 13 Aug 2025, at 8:13 PM, Daniel Kiper <[email protected]> wrote:
>
> On Tue, Jul 29, 2025 at 08:21:51PM +0530, Sudhakar Kuppusamy wrote:
>> If Secure Boot is enabled with dynamic key management mode and the
>> use_static_keys flag is set, then read the static keys as a db default
>> keys from the ELF Note and add stored in the db list.
>
> s/stored in the db list/them into the db/
Sure, will do.
>
>> Signed-off-by: Sudhakar Kuppusamy <[email protected]>
>> Reviewed-by: Stefan Berger <[email protected]>
>> Reviewed-by: Avnish Chouhan <[email protected]>
>> ---
>> grub-core/commands/appendedsig/appendedsig.c | 57 ++++++++++++++------
>> 1 file changed, 42 insertions(+), 15 deletions(-)
>>
>> diff --git a/grub-core/commands/appendedsig/appendedsig.c
>> b/grub-core/commands/appendedsig/appendedsig.c
>> index f696ef476..fa908b963 100644
>> --- a/grub-core/commands/appendedsig/appendedsig.c
>> +++ b/grub-core/commands/appendedsig/appendedsig.c
>> @@ -1051,7 +1051,7 @@ create_dbx_list (void)
>> * parse it, and add it to the db list.
>> */
>> static grub_err_t
>> -build_static_db_list (const struct grub_module_header *header)
>> +build_static_db_list (const struct grub_module_header *header, const bool
>> is_pks)
>> {
>> grub_err_t err;
>> struct grub_file pseudo_file;
>> @@ -1070,6 +1070,12 @@ build_static_db_list (const struct grub_module_header
>> *header)
>> if (err != GRUB_ERR_NONE)
>> return err;
>>
>> + if (is_pks == true)
>> + {
>> + if (is_dbx_cert_hash (cert_data, cert_data_size) == true)
>> + return GRUB_ERR_ACCESS_DENIED;
>> + }
>> +
>> err = add_certificate (cert_data, cert_data_size, &db, true);
>> grub_free (cert_data);
>>
>> @@ -1122,6 +1128,25 @@ free_dbx_list (void)
>> grub_memset (&dbx, 0, sizeof (dbx));
>> }
>>
>> +static grub_err_t
>> +load_static_keys (const struct grub_module_header *header, const bool
>> is_pks)
>> +{
>> + int rc = GRUB_ERR_NONE;
>> +
>> + FOR_MODULES (header)
>> + {
>> + /* Not an ELF module, skip. */
>
> Comment says different thing than the "if" below does...
Sure. Will correct it.
>
>> + if (header->type != OBJ_TYPE_X509_PUBKEY)
>> + continue;
>> +
>> + rc = build_static_db_list (header, is_pks);
>> + if (rc != GRUB_ERR_NONE)
>> + return rc;
>> + }
>> +
>> + return rc;
>> +}
>> +
>> GRUB_MOD_INIT (appendedsig)
>> {
>> int rc;
>> @@ -1147,21 +1172,15 @@ GRUB_MOD_INIT (appendedsig)
>> */
>> if (grub_pks_use_keystore == false && check_sigs == true)
>> {
>> - FOR_MODULES (header)
>> + rc = load_static_keys (header, false);
>> + if (rc != GRUB_ERR_NONE)
>> {
>> - /* Not an ELF module, skip. */
>> - if (header->type != OBJ_TYPE_X509_PUBKEY)
>> - continue;
>> - rc = build_static_db_list (header);
>> - if (rc != GRUB_ERR_NONE)
>> - {
>> - free_db_list ();
>> - grub_error (rc, "static db list creation failed");
>> - }
>> - else
>> - grub_dprintf ("appendedsig", "the db list now has %"
>> PRIuGRUB_SIZE " static keys\n",
>> - db.cert_entries);
>> + free_db_list ();
>
> Again, you should not free partial db lists. I would just print
> a warning in case of error…
Sure, will do it.
Thanks,
Sudhakar
>
> Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
