On Mon, Jul 14, 2025 at 11:05:11PM +0530, Sudhakar Kuppusamy wrote:
> If Secure Boot is enabled with PKS and the use_static_keys flag is set,
> then read the static keys as a DB default keys from the ELF note and
By convention UEFI uses lowercase "db". However, as I can see you mix
cases or even prefer DB. I suggest to stick to UEFI convention and use
db, dbx, ... Please fix this in all patches.
> add stored in the db list.
>
> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
> Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
> Reviewed-by: Avnish Chouhan <avn...@linux.ibm.com>
> ---
> grub-core/commands/appendedsig/appendedsig.c | 59 ++++++++++++++------
> 1 file changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/grub-core/commands/appendedsig/appendedsig.c
> b/grub-core/commands/appendedsig/appendedsig.c
> index fb2119844..1356babaf 100644
> --- a/grub-core/commands/appendedsig/appendedsig.c
> +++ b/grub-core/commands/appendedsig/appendedsig.c
> @@ -1006,7 +1006,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;
> @@ -1025,6 +1025,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);
>
> @@ -1077,6 +1083,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. */
> + 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;
> @@ -1095,26 +1120,28 @@ GRUB_MOD_INIT (appendedsig)
>
> if (!grub_pks_use_keystore && check_sigs == CHECK_SIGS_FORCED)
Again, this shows that almost certainly grub_pks_use_keystore is a flag.
So, it should be defined as bool...
Additionally, after closer looking at the code it seems to me check_sigs
could be defined as bool too. Only CHECK_SIGS_NO and CHECK_SIGS_FORCED
seems to have meaningful usage and CHECK_SIGS_ENFORCE is used only
in definition and one barely meaningful place.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
