Re: [PATCH v2 16/22] grub-install: support embedding x509 certificates

[Stefan Berger <stefanb@linux.ibm.com>]

On 6/30/21 4:40 AM, Daniel Axtens wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> To support verification of appended signatures, we need a way to
> embed the necessary public keys. Existing appended signature schemes
> in the Linux kernel use X.509 certificates, so allow certificates to
> be embedded in the grub core image in the same way as PGP keys.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Signed-off-by: Daniel Axtens <dja@axtens.net>


Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>


> ---
>   grub-core/commands/pgp.c    |  2 +-
>   include/grub/kernel.h       |  3 ++-
>   include/grub/util/install.h |  7 +++++--
>   util/grub-install-common.c  | 22 +++++++++++++++++++-
>   util/grub-mkimage.c         | 15 ++++++++++++--
>   util/mkimage.c              | 41 ++++++++++++++++++++++++++++++++++---
>   6 files changed, 80 insertions(+), 10 deletions(-)
>
> diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
> index 355a43844acc..b81ac0ae46ce 100644
> --- a/grub-core/commands/pgp.c
> +++ b/grub-core/commands/pgp.c
> @@ -944,7 +944,7 @@ GRUB_MOD_INIT(pgp)
>       grub_memset (&pseudo_file, 0, sizeof (pseudo_file));
>   
>       /* Not an ELF module, skip.  */
> -    if (header->type != OBJ_TYPE_PUBKEY)
> +    if (header->type != OBJ_TYPE_GPG_PUBKEY)
>         continue;
>   
>       pseudo_file.fs = &pseudo_fs;
> diff --git a/include/grub/kernel.h b/include/grub/kernel.h
> index abbca5ea3359..d3aafc8848d2 100644
> --- a/include/grub/kernel.h
> +++ b/include/grub/kernel.h
> @@ -28,7 +28,8 @@ enum
>     OBJ_TYPE_MEMDISK,
>     OBJ_TYPE_CONFIG,
>     OBJ_TYPE_PREFIX,
> -  OBJ_TYPE_PUBKEY,
> +  OBJ_TYPE_GPG_PUBKEY,
> +  OBJ_TYPE_X509_PUBKEY,
>     OBJ_TYPE_DTB,
>     OBJ_TYPE_DISABLE_SHIM_LOCK
>   };
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index cf4531e02b66..51f3b13ac130 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -67,6 +67,8 @@
>         N_("SBAT metadata"), 0 },						\
>     { "disable-shim-lock", GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK, 0, 0,	\
>         N_("disable shim_lock verifier"), 0 },				\
> +  { "x509key",   'x', N_("FILE"), 0,					\
> +      N_("embed FILE as an x509 certificate for signature checking"), 0}, \
>     { "appended-signature-size", GRUB_INSTALL_OPTIONS_APPENDED_SIGNATURE_SIZE,\
>       "SIZE", 0, N_("Add a note segment reserving SIZE bytes for an appended signature"), \
>       1},                                                                 \
> @@ -188,8 +190,9 @@ void
>   grub_install_generate_image (const char *dir, const char *prefix,
>   			     FILE *out,
>   			     const char *outname, char *mods[],
> -			     char *memdisk_path, char **pubkey_paths,
> -			     size_t npubkeys,
> +			     char *memdisk_path,
> +			     char **pubkey_paths, size_t npubkeys,
> +			     char **x509key_paths, size_t nx509keys,
>   			     char *config_path,
>   			     const struct grub_install_image_target_desc *image_target,
>   			     int note, size_t appsig_size,
> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> index 1216a203c292..7bfa9752a031 100644
> --- a/util/grub-install-common.c
> +++ b/util/grub-install-common.c
> @@ -460,6 +460,8 @@ static char **pubkeys;
>   static size_t npubkeys;
>   static char *sbat;
>   static int disable_shim_lock;
> +static char **x509keys;
> +static size_t nx509keys;
>   static grub_compression_t compression;
>   static size_t appsig_size;
>   
> @@ -501,6 +503,12 @@ grub_install_parse (int key, char *arg)
>       case GRUB_INSTALL_OPTIONS_DISABLE_SHIM_LOCK:
>         disable_shim_lock = 1;
>         return 1;
> +    case 'x':
> +      x509keys = xrealloc (x509keys,
> +			  sizeof (x509keys[0])
> +			  * (nx509keys + 1));
> +      x509keys[nx509keys++] = xstrdup (arg);
> +      return 1;
>   
>       case GRUB_INSTALL_OPTIONS_VERBOSITY:
>         verbosity++;
> @@ -627,6 +635,9 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
>     for (pk = pubkeys; pk < pubkeys + npubkeys; pk++)
>       slen += 20 + grub_strlen (*pk);
>   
> +  for (pk = x509keys; pk < x509keys + nx509keys; pk++)
> +    slen += 10 + grub_strlen (*pk);
> +
>     for (md = modules.entries; *md; md++)
>       {
>         slen += 10 + grub_strlen (*md);
> @@ -655,6 +666,14 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
>         *p++ = ' ';
>       }
>   
> +  for (pk = x509keys; pk < x509keys + nx509keys; pk++)
> +    {
> +      p = grub_stpcpy (p, "--x509 '");
> +      p = grub_stpcpy (p, *pk);
> +      *p++ = '\'';
> +      *p++ = ' ';
> +    }
> +
>     for (md = modules.entries; *md; md++)
>       {
>         *p++ = '\'';
> @@ -683,7 +702,8 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
>   
>     grub_install_generate_image (dir, prefix, fp, outname,
>   			       modules.entries, memdisk_path,
> -			       pubkeys, npubkeys, config_path, tgt,
> +			       pubkeys, npubkeys, x509keys, nx509keys,
> +			       config_path, tgt,
>   			       note, appsig_size, compression, dtb, sbat,
>   			       disable_shim_lock);
>     while (dc--)
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index d01eaeb8443a..7d61ef3ea046 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -75,7 +75,8 @@ static struct argp_option options[] = {
>      /* TRANSLATORS: "embed" is a verb (command description).  "*/
>     {"config",   'c', N_("FILE"), 0, N_("embed FILE as an early config"), 0},
>      /* TRANSLATORS: "embed" is a verb (command description).  "*/
> -  {"pubkey",   'k', N_("FILE"), 0, N_("embed FILE as public key for signature checking"), 0},
> +  {"pubkey",   'k', N_("FILE"), 0, N_("embed FILE as public key for PGP signature checking"), 0},
> +  {"x509",     'x', N_("FILE"), 0, N_("embed FILE as an x509 certificate for appended signature checking"), 0},
>     /* TRANSLATORS: NOTE is a name of segment.  */
>     {"note",   'n', 0, 0, N_("add NOTE segment for CHRP IEEE1275"), 0},
>     {"output",  'o', N_("FILE"), 0, N_("output a generated image to FILE [default=stdout]"), 0},
> @@ -124,6 +125,8 @@ struct arguments
>     char *dtb;
>     char **pubkeys;
>     size_t npubkeys;
> +  char **x509keys;
> +  size_t nx509keys;
>     char *font;
>     char *config;
>     char *sbat;
> @@ -206,6 +209,13 @@ argp_parser (int key, char *arg, struct argp_state *state)
>         arguments->pubkeys[arguments->npubkeys++] = xstrdup (arg);
>         break;
>   
> +    case 'x':
> +      arguments->x509keys = xrealloc (arguments->x509keys,
> +				      sizeof (arguments->x509keys[0])
> +				      * (arguments->nx509keys + 1));
> +      arguments->x509keys[arguments->nx509keys++] = xstrdup (arg);
> +      break;
> +
>       case 'c':
>         if (arguments->config)
>   	free (arguments->config);
> @@ -332,7 +342,8 @@ main (int argc, char *argv[])
>     grub_install_generate_image (arguments.dir, arguments.prefix, fp,
>   			       arguments.output, arguments.modules,
>   			       arguments.memdisk, arguments.pubkeys,
> -			       arguments.npubkeys, arguments.config,
> +			       arguments.npubkeys, arguments.x509keys,
> +			       arguments.nx509keys, arguments.config,
>   			       arguments.image_target, arguments.note,
>   			       arguments.appsig_size,
>   			       arguments.comp, arguments.dtb,
> diff --git a/util/mkimage.c b/util/mkimage.c
> index d2cb33883557..5a8021a213cf 100644
> --- a/util/mkimage.c
> +++ b/util/mkimage.c
> @@ -866,8 +866,10 @@ init_pe_section(const struct grub_install_image_target_desc *image_target,
>   void
>   grub_install_generate_image (const char *dir, const char *prefix,
>   			     FILE *out, const char *outname, char *mods[],
> -			     char *memdisk_path, char **pubkey_paths,
> -			     size_t npubkeys, char *config_path,
> +			     char *memdisk_path,
> +			     char **pubkey_paths, size_t npubkeys,
> +			     char **x509key_paths, size_t nx509keys,
> +			     char *config_path,
>   			     const struct grub_install_image_target_desc *image_target,
>   			     int note, size_t appsig_size, grub_compression_t comp,
>   			     const char *dtb_path, const char *sbat_path,
> @@ -913,6 +915,19 @@ grub_install_generate_image (const char *dir, const char *prefix,
>         }
>     }
>   
> +  {
> +    size_t i;
> +    for (i = 0; i < nx509keys; i++)
> +      {
> +	size_t curs;
> +	curs = ALIGN_ADDR (grub_util_get_image_size (x509key_paths[i]));
> +	grub_util_info ("the size of x509 public key %u is 0x%"
> +			GRUB_HOST_PRIxLONG_LONG,
> +			(unsigned) i, (unsigned long long) curs);
> +	total_module_size += curs + sizeof (struct grub_module_header);
> +      }
> +  }
> +
>     if (memdisk_path)
>       {
>         memdisk_size = ALIGN_UP(grub_util_get_image_size (memdisk_path), 512);
> @@ -1034,7 +1049,7 @@ grub_install_generate_image (const char *dir, const char *prefix,
>   	curs = grub_util_get_image_size (pubkey_paths[i]);
>   
>   	header = (struct grub_module_header *) (kernel_img + offset);
> -	header->type = grub_host_to_target32 (OBJ_TYPE_PUBKEY);
> +	header->type = grub_host_to_target32 (OBJ_TYPE_GPG_PUBKEY);
>   	header->size = grub_host_to_target32 (curs + sizeof (*header));
>   	offset += sizeof (*header);
>   
> @@ -1043,6 +1058,26 @@ grub_install_generate_image (const char *dir, const char *prefix,
>         }
>     }
>   
> +  {
> +    size_t i;
> +    for (i = 0; i < nx509keys; i++)
> +      {
> +	size_t curs;
> +	struct grub_module_header *header;
> +
> +	curs = grub_util_get_image_size (x509key_paths[i]);
> +
> +	header = (struct grub_module_header *) (kernel_img + offset);
> +	header->type = grub_host_to_target32 (OBJ_TYPE_X509_PUBKEY);
> +	header->size = grub_host_to_target32 (curs + sizeof (*header));
> +	offset += sizeof (*header);
> +
> +	grub_util_load_image (x509key_paths[i], kernel_img + offset);
> +	offset += ALIGN_ADDR (curs);
> +      }
> +  }
> +
> +
>     if (memdisk_path)
>       {
>         struct grub_module_header *header;