Re: [PATCH v1 18/21] ieee1275: set use_static_keys flag

[Stefan Berger]

On 12/18/24 9:56 AM, Sudhakar Kuppusamy wrote:
> if secure boot enabled with PKS, it set the use_static_keys flag

I was not sure at this point what the patch actually does so I 
reformulated it a bit. I would start the patch description with the 
reason why you are introducing the use_static_key:

Introduce the use_static_keys flag to indicate that static keys are to 
be used rather than keys from the PKS storage's DB variable. This 
variable is set when Secure Boot is enabled with PKS but the DB variable 
is not present in the PKS storage. The appendedsig module would use this 
variable to extract the default DB keys from the ELF note and store the 
keys found there in the trustedlist... or something like this.


> when DB variable is not present in PKS storage and the appendedsig (module)
> would use it later to extract the default DB key's from ELF Note and
> store it in trustedlist.
>
> Signed-off-by: Sudhakar Kuppusamy <sudhakar@linux.ibm.com>
> ---
>   grub-core/kern/ieee1275/platform_keystore.c | 15 ++++++++++++++-
>   include/grub/platform_keystore.h            | 12 +++++++-----
>   2 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/platform_keystore.c 
> b/grub-core/kern/ieee1275/platform_keystore.c
> index 1c564d5da..ddc11afd9 100644
> --- a/grub-core/kern/ieee1275/platform_keystore.c
> +++ b/grub-core/kern/ieee1275/platform_keystore.c
> @@ -34,7 +34,11 @@
>   /* Platform Keystore */
>   static grub_size_t pks_max_object_size;
>   grub_uint8_t grub_use_platform_keystore = 0;
> -grub_pks_t grub_platform_keystore = { .db = NULL, .dbx = NULL, .db_entries = 
> 0, .dbx_entries = 0 };
> +grub_pks_t grub_platform_keystore = { .db = NULL,
> +                                      .dbx = NULL,
> +                                      .db_entries = 0,
> +                                      .dbx_entries = 0,
> +                                      .use_static_keys = 0 };

Use false

>   
>   /* converts the esl data into the ESL */
>   static grub_esl_t *
> @@ -316,6 +320,15 @@ grub_platform_keystore_init (void)
>         /* DB */
>         rc = grub_read_secure_boot_variables (0, DB, &grub_platform_keystore.db,
>                                               
> &grub_platform_keystore.db_entries);
> +      if (rc == PKS_OBJECT_NOT_FOUND)
> +        {
> +          rc = GRUB_ERR_NONE;
> +          /*
> +           * DB variable won't be available by default in PKS.

The DB variable.

> +           * So, it will loads the Default Keys from ELF Note */

s/loads/load
-> If it is not available load the default keys from the ELF node (?)


> +          grub_platform_keystore.use_static_keys = 1;
> +        }
> +
>         if (rc == GRUB_ERR_NONE)
>           {
>             /* DBX */
> diff --git a/include/grub/platform_keystore.h b/include/grub/platform_keystore.h
> index 7a7378926..b333db7fd 100644
> --- a/include/grub/platform_keystore.h
> +++ b/include/grub/platform_keystore.h
> @@ -49,6 +49,7 @@
>   #define GRUB_UUID_SIZE 16
>   #define GRUB_MAX_HASH_SIZE 64
>   
> +typedef grub_uint8_t grub_bool_t;

This would be a candidate for types.h. term/tparam.c already defines it 
as char. bool also already exists and is also used in some places.

>   typedef struct grub_uuid grub_uuid_t;
>   typedef struct grub_esd grub_esd_t;
>   typedef struct grub_esl grub_esl_t;
> @@ -207,10 +208,11 @@ struct grub_pks_sd
>   /* The structure of a PKS.*/
>   struct grub_pks
>   {
> -  grub_pks_sd_t *db;        /* signature database */
> -  grub_pks_sd_t *dbx;       /* forbidden signature database */
> -  grub_size_t db_entries;   /* size of signature database */
> -  grub_size_t dbx_entries;  /* size of forbidden signature database */
> +  grub_pks_sd_t *db;          /* signature database */
> +  grub_pks_sd_t *dbx;         /* forbidden signature database */
> +  grub_size_t db_entries;     /* size of signature database */
> +  grub_size_t dbx_entries;    /* size of forbidden signature database */
> +  grub_bool_t use_static_keys;/* flag to indicate use of static keys */
>   } GRUB_PACKED;
>   
>   #ifdef __powerpc__
> @@ -225,7 +227,7 @@ extern grub_pks_t EXPORT_VAR(grub_platform_keystore);
>   #else
>   
>   #define grub_use_platform_keystore	0
> -grub_pks_t grub_platform_keystore = {NULL, NULL, 0, 0};
> +grub_pks_t grub_platform_keystore = {NULL, NULL, 0, 0, 0};

Use false

>   void grub_release_platform_keystore (void);
>   
>   #endif