[RFC PATCH 0/4] Wrap font files into PE files

[RFC PATCH 0/4] Wrap font files into PE files

[Zhang Boyang]

Hi,

This patchset introduces a utility: grub-wrap. It can wrap a non-PE file
as a PE file, which can be subsequently digitally signed (and verified
by shim). The wrapped file will be unwrapped at runtime automatically
and transparently by the unwrap filter.

With grub-wrap, a PF2 font file can be wrapped and digitally signed. The
name of PE section must be .GRUBpf2, to clearly identifies such PE file
is a wrapper of a PF2 font file.

Example usage:

# generate a MOK key for signing
pesign --certficate 'MyKey' --export-cert MyKey.crt

# import the new MOK key
mokutil --import MyKey.crt 
reboot

# wrap unicode.pf2 as unicode.pf2.dll
grub-wrap --input unicode.pf2 --output unicode.pf2.dll --name .GRUBpf2 -O x86_64-efi

# sign the wrapper with MOK key
pesign --in unicode.pf2.dll --out /boot/grub/fonts/unicode.pf2 --force --certficate 'MyKey' --sign

There is one problem: those wrappers mustn't be accepted by other PE
loaders. To distinguish these wrappers from normal EFI applications, I
decided to make these wrappers looks like plain Windows resource-only
DLLs (no entry point, and subsystem tag is set to Windows GUI instead of
EFI Application). Currently, shim, tianocore, and my patched GRUB will
reject these wrappers while trying to use them as executables. However,
I'm not sure whether in-market UEFI firmwares will reject them. So I
think the best workaround is to forbid keys available in firmware from
signing these wrappers, and only use keys in shim to sign wrappers (this
seems like exactly what we are doing for GRUB itself).

Best Regards,
Zhang Boyang

[RFC PATCH 1/4] util/grub-wrap: New tool to wrap a file as a PE image

[Zhang Boyang]

This new tool can wrap a non-PE file as a PE image, which can be
subsequently digitally signed (and verified by shim).

It's important that this type of PE images must be rejected when trying
to execute them. This is done by making them look like plain Windows
DLLs instead of UEFI images. Major differences between these wrappers
and normal UEFI images are:

 * DLL flag is set (nobody cares)
 * Entry point is set to 0 (shim rejects this)
 * Subsystem is set to Windows GUI instead of EFI Application (tianocore
   rejects this)

Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
---
 .gitignore                  |   2 +
 Makefile.util.def           |  20 +++
 docs/man/grub-wrap.h2m      |   4 +
 include/grub/efi/pe32.h     |   4 +
 include/grub/util/install.h |   3 +-
 util/grub-install-common.c  |   2 +-
 util/grub-mkimage.c         |   3 +-
 util/grub-wrap.c            | 160 ++++++++++++++++++++++++
 util/mkimage.c              | 239 ++++++++++++++++++++++--------------
 9 files changed, 341 insertions(+), 96 deletions(-)
 create mode 100644 docs/man/grub-wrap.h2m
 create mode 100644 util/grub-wrap.c

diff --git a/.gitignore b/.gitignore
index f6a1bd051..e2802578e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -179,6 +179,8 @@ widthspec.bin
 /grub-sparc64-setup.exe
 /grub-syslinux2cfg
 /grub-syslinux2cfg.exe
+/grub-wrap
+/grub-wrap.exe
 /grub_cmd_date
 /grub_cmd_echo
 /grub_cmd_regexp
diff --git a/Makefile.util.def b/Makefile.util.def
index d919c562c..c02632fbd 100644
--- a/Makefile.util.def
+++ b/Makefile.util.def
@@ -1385,3 +1385,23 @@ program = {
   ldadd = grub-core/lib/gnulib/libgnu.a;
   ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM)';
 };
+
+program = {
+  name = grub-wrap;
+  mansection = 1;
+
+  common = util/grub-wrap.c;
+  common = util/mkimage.c;
+  common = util/grub-mkimage32.c;
+  common = util/grub-mkimage64.c;
+  common = util/resolve.c;
+  common = grub-core/kern/emu/argp_common.c;
+  common = grub-core/osdep/init.c;
+
+  ldadd = '$(LIBLZMA)';
+  ldadd = libgrubmods.a;
+  ldadd = libgrubgcry.a;
+  ldadd = libgrubkern.a;
+  ldadd = grub-core/lib/gnulib/libgnu.a;
+  ldadd = '$(LIBINTL) $(LIBDEVMAPPER) $(LIBZFS) $(LIBNVPAIR) $(LIBGEOM)';
+};
diff --git a/docs/man/grub-wrap.h2m b/docs/man/grub-wrap.h2m
new file mode 100644
index 000000000..f9d44fcd7
--- /dev/null
+++ b/docs/man/grub-wrap.h2m
@@ -0,0 +1,4 @@
+[NAME]
+grub-wrap \- Wrap a file as a PE image
+[DESCRIPTION]
+grub-wrap wraps a file as a PE image.
diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
index 98c4ff177..7609b5198 100644
--- a/include/grub/efi/pe32.h
+++ b/include/grub/efi/pe32.h
@@ -227,8 +227,12 @@ struct grub_pe64_optional_header
 #define GRUB_PE32_PE32_MAGIC	0x10b
 #define GRUB_PE32_PE64_MAGIC	0x20b
 
+#define GRUB_PE32_SUBSYSTEM_WINDOWS_GUI		2
 #define GRUB_PE32_SUBSYSTEM_EFI_APPLICATION	10
 
+#define GRUB_PE32_DLLCHARACTERISTICS_DYNAMIC_BASE	0x0040
+#define GRUB_PE32_DLLCHARACTERISTICS_NX_COMPAT		0x0100
+
 #define GRUB_PE32_NUM_DATA_DIRECTORIES	16
 
 struct grub_pe32_section_table
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index 7d7445af9..eac34abaf 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -190,7 +190,8 @@ grub_install_generate_image (const char *dir, const char *prefix,
 			     const struct grub_install_image_target_desc *image_target,
 			     int note,
 			     grub_compression_t comp, const char *dtb_file,
-			     const char *sbat_path, const int disable_shim_lock);
+			     const char *sbat_path, const int disable_shim_lock,
+			     const char *wrap_path, const char *wrap_section);
 
 const struct grub_install_image_target_desc *
 grub_install_get_image_target (const char *arg);
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index e45fb2903..c64a061b7 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -681,7 +681,7 @@ grub_install_make_image_wrap_file (const char *dir, const char *prefix,
 			       modules.entries, memdisk_path,
 			       pubkeys, npubkeys, config_path, tgt,
 			       note, compression, dtb, sbat,
-			       disable_shim_lock);
+			       disable_shim_lock, NULL, NULL);
   while (dc--)
     grub_install_pop_module ();
 }
diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
index c0d559937..51aebb40e 100644
--- a/util/grub-mkimage.c
+++ b/util/grub-mkimage.c
@@ -325,7 +325,8 @@ main (int argc, char *argv[])
 			       arguments.npubkeys, arguments.config,
 			       arguments.image_target, arguments.note,
 			       arguments.comp, arguments.dtb,
-			       arguments.sbat, arguments.disable_shim_lock);
+			       arguments.sbat, arguments.disable_shim_lock,
+			       NULL, NULL);
 
   if (grub_util_file_sync (fp) < 0)
     grub_util_error (_("cannot sync `%s': %s"), arguments.output ? : "stdout",
diff --git a/util/grub-wrap.c b/util/grub-wrap.c
new file mode 100644
index 000000000..d54190b2d
--- /dev/null
+++ b/util/grub-wrap.c
@@ -0,0 +1,160 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022 Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include <grub/util/misc.h>
+#include <grub/i18n.h>
+#include <grub/term.h>
+#include <grub/util/install.h>
+
+#define _GNU_SOURCE	1
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+
+#pragma GCC diagnostic ignored "-Wmissing-prototypes"
+#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#include <argp.h>
+#pragma GCC diagnostic error "-Wmissing-prototypes"
+#pragma GCC diagnostic error "-Wmissing-declarations"
+
+#include "progname.h"
+
+struct arguments
+{
+  char *input;
+  char *name;
+  char *sbat;
+  char *output;
+  const struct grub_install_image_target_desc *image_target;
+  int verbosity;
+};
+
+static struct argp_option options[] = {
+  {"input",  'i', N_("FILE"), 0, N_("set input filename."), 0},
+  {"name",  'n', N_("NAME"), 0, N_("set section name."), 0},
+  {"sbat",  's', N_("FILE"), 0, N_("SBAT metadata"), 0},
+  {"output",  'o', N_("FILE"), 0, N_("set output filename."), 0},
+  {"format",  'O', N_("FORMAT"), 0, N_("generate an image in FORMAT"), 0},
+  {"verbose",     'v', 0,      0, N_("print verbose messages."), 0},
+  { 0, 0, 0, 0, 0, 0 }
+};
+
+static error_t
+argp_parser (int key, char *arg, struct argp_state *state)
+{
+  /* Get the input argument from argp_parse, which we
+     know is a pointer to our arguments structure. */
+  struct arguments *arguments = state->input;
+
+  switch (key)
+    {
+    case 'i':
+      free (arguments->input);
+      arguments->input = xstrdup (arg);
+      break;
+
+    case 'n':
+      free (arguments->name);
+      arguments->name = xstrdup (arg);
+      break;
+
+    case 's':
+      free (arguments->sbat);
+      arguments->sbat = xstrdup (arg);
+      break;
+
+    case 'o':
+      free (arguments->output);
+      arguments->output = xstrdup (arg);
+      break;
+
+    case 'O':
+      {
+	arguments->image_target = grub_install_get_image_target (arg);
+	if (!arguments->image_target)
+	  {
+	    printf (_("unknown target format %s\n"), arg);
+	    argp_usage (state);
+	    exit (1);
+	  }
+	break;
+      }
+
+    case 'v':
+      arguments->verbosity++;
+      break;
+
+    default:
+      return ARGP_ERR_UNKNOWN;
+    }
+
+  return 0;
+}
+
+static struct argp argp = {
+  options, argp_parser, N_("[OPTIONS]"),
+  N_("Wrap a file as a PE image."),
+  NULL, NULL, NULL
+};
+
+int
+main (int argc, char *argv[])
+{
+  FILE *out;
+
+  struct arguments arguments;
+
+  grub_util_host_init (&argc, &argv);
+
+  /* Check for options.  */
+  memset (&arguments, 0, sizeof (struct arguments));
+  if (argp_parse (&argp, argc, argv, 0, 0, &arguments) != 0)
+    {
+      fprintf (stderr, "%s", _("Error in parsing command line arguments\n"));
+      exit(1);
+    }
+
+  if (!arguments.input)
+    grub_util_error ("%s", _("input file must be specified"));
+
+  if (!arguments.name)
+    grub_util_error ("%s", _("section name must be specified"));
+
+  if (!arguments.output)
+    grub_util_error ("%s", _("output file must be specified"));
+
+  if (!arguments.image_target)
+    grub_util_error ("%s", _("target format must be specified"));
+
+  out = grub_util_fopen (arguments.output, "wb");
+  if (!out)
+    grub_util_error (_("cannot open `%s': %s"), arguments.output, strerror (errno));
+
+  grub_install_generate_image (NULL, NULL, out, NULL, NULL, NULL, NULL, 0,
+			       NULL, arguments.image_target,
+			       0, GRUB_COMPRESSION_NONE, NULL,
+			       arguments.sbat, 0,
+			       arguments.input, arguments.name);
+
+  return 0;
+}
diff --git a/util/mkimage.c b/util/mkimage.c
index 43078c71c..349f055bb 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -61,18 +61,20 @@
 /* use 2015-01-01T00:00:00+0000 as a stock timestamp */
 #define STABLE_EMBEDDING_TIMESTAMP 1420070400
 
+#define EFI_MAX_SECTIONS 5
+
 #define EFI32_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE		\
 				    + GRUB_PE32_SIGNATURE_SIZE		\
 				    + sizeof (struct grub_pe32_coff_header) \
 				    + sizeof (struct grub_pe32_optional_header) \
-				    + 4 * sizeof (struct grub_pe32_section_table), \
+				    + EFI_MAX_SECTIONS * sizeof (struct grub_pe32_section_table), \
 				    GRUB_PE32_FILE_ALIGNMENT)
 
 #define EFI64_HEADER_SIZE ALIGN_UP (GRUB_PE32_MSDOS_STUB_SIZE		\
 				    + GRUB_PE32_SIGNATURE_SIZE		\
 				    + sizeof (struct grub_pe32_coff_header) \
 				    + sizeof (struct grub_pe64_optional_header) \
-				    + 4 * sizeof (struct grub_pe32_section_table), \
+				    + EFI_MAX_SECTIONS * sizeof (struct grub_pe32_section_table), \
 				    GRUB_PE32_FILE_ALIGNMENT)
 
 static const struct grub_install_image_target_desc image_targets[] =
@@ -819,9 +821,9 @@ grub_install_get_image_targets_string (void)
 /*
  * The image_target parameter is used by the grub_host_to_target32() macro.
  */
-static struct grub_pe32_section_table *
+static void
 init_pe_section(const struct grub_install_image_target_desc *image_target,
-		struct grub_pe32_section_table *section,
+		struct grub_pe32_section_table *section, int *n_sections,
 		const char * const name,
 		grub_uint32_t *vma, grub_uint32_t vsz, grub_uint32_t valign,
 		grub_uint32_t *rda, grub_uint32_t rsz,
@@ -829,6 +831,13 @@ init_pe_section(const struct grub_install_image_target_desc *image_target,
 {
   size_t len = strlen (name);
 
+  if (vsz == 0)
+    return;
+
+  if (*n_sections >= EFI_MAX_SECTIONS)
+    grub_util_error (_("too many sections"));
+  section = &section[(*n_sections)++];
+
   if (len > sizeof (section->name))
     grub_util_error (_("section name %s length is bigger than %lu"),
 		     name, (unsigned long) sizeof (section->name));
@@ -844,8 +853,6 @@ init_pe_section(const struct grub_install_image_target_desc *image_target,
   (*rda) = ALIGN_UP (*rda + rsz, GRUB_PE32_FILE_ALIGNMENT);
 
   section->characteristics = grub_host_to_target32 (characteristics);
-
-  return section + 1;
 }
 
 /*
@@ -870,17 +877,22 @@ grub_install_generate_image (const char *dir, const char *prefix,
 			     size_t npubkeys, char *config_path,
 			     const struct grub_install_image_target_desc *image_target,
 			     int note, grub_compression_t comp, const char *dtb_path,
-			     const char *sbat_path, int disable_shim_lock)
+			     const char *sbat_path, int disable_shim_lock,
+			     const char *wrap_path, const char *wrap_section)
 {
-  char *kernel_img, *core_img;
+  char *kernel_img = NULL, *core_img;
   size_t total_module_size, core_size;
   size_t memdisk_size = 0, config_size = 0;
   size_t prefix_size = 0, dtb_size = 0, sbat_size = 0;
-  char *kernel_path;
-  size_t offset;
-  struct grub_util_path_list *path_list, *p;
+  char *kernel_path = NULL;
+  size_t offset = 0;
+  struct grub_util_path_list *path_list = NULL, *p;
   size_t decompress_size = 0;
   struct grub_mkimage_layout layout;
+  memset (&layout, 0, sizeof (layout));
+
+  if (wrap_path != NULL && image_target->id != IMAGE_EFI)
+    grub_util_error (_("wrapping is not supported by this target"));
 
   if (comp == GRUB_COMPRESSION_AUTO)
     comp = image_target->default_compression;
@@ -890,9 +902,12 @@ grub_install_generate_image (const char *dir, const char *prefix,
       || image_target->id == IMAGE_I386_PC_ELTORITO)
     comp = GRUB_COMPRESSION_LZMA;
 
-  path_list = grub_util_resolve_dependencies (dir, "moddep.lst", mods);
+  if (dir)
+    {
+      path_list = grub_util_resolve_dependencies (dir, "moddep.lst", mods);
 
-  kernel_path = grub_util_get_path (dir, "kernel.img");
+      kernel_path = grub_util_get_path (dir, "kernel.img");
+    }
 
   if (image_target->voidp_sizeof == 8)
     total_module_size = sizeof (struct grub_module_info64);
@@ -953,12 +968,16 @@ grub_install_generate_image (const char *dir, const char *prefix,
   grub_util_info ("the total module size is 0x%" GRUB_HOST_PRIxLONG_LONG,
 		  (unsigned long long) total_module_size);
 
-  if (image_target->voidp_sizeof == 4)
-    kernel_img = grub_mkimage_load_image32 (kernel_path, total_module_size,
-					    &layout, image_target);
-  else
-    kernel_img = grub_mkimage_load_image64 (kernel_path, total_module_size,
-					    &layout, image_target);
+  if (kernel_path)
+    {
+      if (image_target->voidp_sizeof == 4)
+	kernel_img = grub_mkimage_load_image32 (kernel_path, total_module_size,
+						&layout, image_target);
+      else
+	kernel_img = grub_mkimage_load_image64 (kernel_path, total_module_size,
+						&layout, image_target);
+    }
+
   if ((image_target->id == IMAGE_XEN || image_target->id == IMAGE_XEN_PVH) &&
       layout.align < 4096)
     layout.align = 4096;
@@ -974,37 +993,40 @@ grub_install_generate_image (const char *dir, const char *prefix,
       memset (kernel_img, 0, total_module_size);
     }
 
-  if (image_target->voidp_sizeof == 8)
+  if (kernel_img)
     {
-      /* Fill in the grub_module_info structure.  */
-      struct grub_module_info64 *modinfo;
-      if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
-	modinfo = (struct grub_module_info64 *) kernel_img;
-      else
-	modinfo = (struct grub_module_info64 *) (kernel_img + layout.kernel_size);
-      modinfo->magic = grub_host_to_target32 (GRUB_MODULE_MAGIC);
-      modinfo->offset = grub_host_to_target_addr (sizeof (struct grub_module_info64));
-      modinfo->size = grub_host_to_target_addr (total_module_size);
-      if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
-	offset = sizeof (struct grub_module_info64);
-      else
-	offset = layout.kernel_size + sizeof (struct grub_module_info64);
-    }
-  else
-    {
-      /* Fill in the grub_module_info structure.  */
-      struct grub_module_info32 *modinfo;
-      if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
-	modinfo = (struct grub_module_info32 *) kernel_img;
-      else
-	modinfo = (struct grub_module_info32 *) (kernel_img + layout.kernel_size);
-      modinfo->magic = grub_host_to_target32 (GRUB_MODULE_MAGIC);
-      modinfo->offset = grub_host_to_target_addr (sizeof (struct grub_module_info32));
-      modinfo->size = grub_host_to_target_addr (total_module_size);
-      if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
-	offset = sizeof (struct grub_module_info32);
+      if (image_target->voidp_sizeof == 8)
+	{
+	  /* Fill in the grub_module_info structure.  */
+	  struct grub_module_info64 *modinfo;
+	  if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
+	    modinfo = (struct grub_module_info64 *) kernel_img;
+	  else
+	    modinfo = (struct grub_module_info64 *) (kernel_img + layout.kernel_size);
+	  modinfo->magic = grub_host_to_target32 (GRUB_MODULE_MAGIC);
+	  modinfo->offset = grub_host_to_target_addr (sizeof (struct grub_module_info64));
+	  modinfo->size = grub_host_to_target_addr (total_module_size);
+	  if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
+	    offset = sizeof (struct grub_module_info64);
+	  else
+	    offset = layout.kernel_size + sizeof (struct grub_module_info64);
+	}
       else
-	offset = layout.kernel_size + sizeof (struct grub_module_info32);
+	{
+	  /* Fill in the grub_module_info structure.  */
+	  struct grub_module_info32 *modinfo;
+	  if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
+	    modinfo = (struct grub_module_info32 *) kernel_img;
+	  else
+	    modinfo = (struct grub_module_info32 *) (kernel_img + layout.kernel_size);
+	  modinfo->magic = grub_host_to_target32 (GRUB_MODULE_MAGIC);
+	  modinfo->offset = grub_host_to_target_addr (sizeof (struct grub_module_info32));
+	  modinfo->size = grub_host_to_target_addr (total_module_size);
+	  if (image_target->flags & PLATFORM_FLAGS_MODULES_BEFORE_KERNEL)
+	    offset = sizeof (struct grub_module_info32);
+	  else
+	    offset = layout.kernel_size + sizeof (struct grub_module_info32);
+	}
     }
 
   for (p = path_list; p; p = p->next)
@@ -1104,12 +1126,21 @@ grub_install_generate_image (const char *dir, const char *prefix,
       offset += prefix_size;
     }
 
-  grub_util_info ("kernel_img=%p, kernel_size=0x%" GRUB_HOST_PRIxLONG_LONG,
-		  kernel_img,
-		  (unsigned long long) layout.kernel_size);
-  compress_kernel (image_target, kernel_img, layout.kernel_size + total_module_size,
-		   &core_img, &core_size, comp);
-  free (kernel_img);
+  if (kernel_img)
+    {
+      grub_util_info ("kernel_img=%p, kernel_size=0x%" GRUB_HOST_PRIxLONG_LONG,
+		      kernel_img,
+		      (unsigned long long) layout.kernel_size);
+      compress_kernel (image_target, kernel_img, layout.kernel_size + total_module_size,
+		       &core_img, &core_size, comp);
+      free (kernel_img);
+    }
+
+  if (wrap_path)
+    {
+      core_size = grub_util_get_image_size (wrap_path);
+      core_img = grub_util_read_image (wrap_path);
+    }
 
   grub_util_info ("the core size is 0x%" GRUB_HOST_PRIxLONG_LONG,
 		  (unsigned long long) core_size);
@@ -1308,7 +1339,7 @@ grub_install_generate_image (const char *dir, const char *prefix,
       {
 	char *pe_img, *pe_sbat, *header;
 	struct grub_pe32_section_table *section;
-	size_t n_sections = 4;
+	int n_sections = 0;
 	size_t scn_size;
 	grub_uint32_t vma, raw_data;
 	size_t pe_size, header_size;
@@ -1345,17 +1376,15 @@ grub_install_generate_image (const char *dir, const char *prefix,
 	c = (struct grub_pe32_coff_header *) (header + GRUB_PE32_MSDOS_STUB_SIZE
 					      + GRUB_PE32_SIGNATURE_SIZE);
 	c->machine = grub_host_to_target16 (image_target->pe_target);
-
-	if (sbat_path != NULL)
-	  n_sections++;
-
-	c->num_sections = grub_host_to_target16 (n_sections);
 	c->time = grub_host_to_target32 (STABLE_EMBEDDING_TIMESTAMP);
 	c->characteristics = grub_host_to_target16 (GRUB_PE32_EXECUTABLE_IMAGE
 						    | GRUB_PE32_LINE_NUMS_STRIPPED
 						    | ((image_target->voidp_sizeof == 4)
 						       ? GRUB_PE32_32BIT_MACHINE
 						       : 0)
+						    | ((wrap_path == NULL)
+						       ? 0
+						       : GRUB_PE32_DLL)
 						    | GRUB_PE32_LOCAL_SYMS_STRIPPED
 						    | GRUB_PE32_DEBUG_STRIPPED);
 
@@ -1393,7 +1422,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
 	PE_OHDR (o32, o64, image_size) = grub_host_to_target32 (pe_size);
 	PE_OHDR (o32, o64, section_alignment) = grub_host_to_target32 (image_target->section_align);
 	PE_OHDR (o32, o64, file_alignment) = grub_host_to_target32 (GRUB_PE32_FILE_ALIGNMENT);
-	PE_OHDR (o32, o64, subsystem) = grub_host_to_target16 (GRUB_PE32_SUBSYSTEM_EFI_APPLICATION);
+	PE_OHDR (o32, o64, subsystem) = grub_host_to_target16 ((wrap_path == NULL)
+							       ? GRUB_PE32_SUBSYSTEM_EFI_APPLICATION
+							       : GRUB_PE32_SUBSYSTEM_WINDOWS_GUI);
+	PE_OHDR (o32, o64, dll_characteristics) = grub_host_to_target16 ((wrap_path == NULL)
+									 ? 0
+									 : GRUB_PE32_DLLCHARACTERISTICS_DYNAMIC_BASE
+									   | GRUB_PE32_DLLCHARACTERISTICS_NX_COMPAT);
 
 	/* Do these really matter? */
 	PE_OHDR (o32, o64, stack_reserve_size) = grub_host_to_target32 (0x10000);
@@ -1409,13 +1444,13 @@ grub_install_generate_image (const char *dir, const char *prefix,
 #if __GNUC__ >= 12
 #pragma GCC diagnostic pop
 #endif
-	section = init_pe_section (image_target, section, ".text",
-				   &vma, layout.exec_size,
-				   image_target->section_align,
-				   &raw_data, layout.exec_size,
-				   GRUB_PE32_SCN_CNT_CODE |
-				   GRUB_PE32_SCN_MEM_EXECUTE |
-				   GRUB_PE32_SCN_MEM_READ);
+	init_pe_section (image_target, section, &n_sections, ".text",
+			 &vma, layout.exec_size,
+			 image_target->section_align,
+			 &raw_data, layout.exec_size,
+			 GRUB_PE32_SCN_CNT_CODE |
+			 GRUB_PE32_SCN_MEM_EXECUTE |
+			 GRUB_PE32_SCN_MEM_READ);
 
 	scn_size = ALIGN_UP (layout.kernel_size - layout.exec_size, GRUB_PE32_FILE_ALIGNMENT);
 #if __GNUC__ >= 12
@@ -1423,39 +1458,55 @@ grub_install_generate_image (const char *dir, const char *prefix,
 #pragma GCC diagnostic ignored "-Wdangling-pointer"
 #endif
 	/* ALIGN_UP (sbat_size, GRUB_PE32_FILE_ALIGNMENT) is done earlier. */
-	PE_OHDR (o32, o64, data_size) = grub_host_to_target32 (scn_size + sbat_size +
-							       ALIGN_UP (total_module_size,
-									 GRUB_PE32_FILE_ALIGNMENT));
+	PE_OHDR (o32, o64, data_size) = grub_host_to_target32 ((wrap_path == NULL)
+							       ? scn_size + sbat_size +
+								 ALIGN_UP (total_module_size,
+									   GRUB_PE32_FILE_ALIGNMENT)
+							       : sbat_size +
+								 ALIGN_UP (core_size,
+									   GRUB_PE32_FILE_ALIGNMENT));
 #if __GNUC__ >= 12
 #pragma GCC diagnostic pop
 #endif
 
-	section = init_pe_section (image_target, section, ".data",
-				   &vma, scn_size, image_target->section_align,
-				   &raw_data, scn_size,
-				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
-				   GRUB_PE32_SCN_MEM_READ |
-				   GRUB_PE32_SCN_MEM_WRITE);
-
-	scn_size = pe_size - layout.reloc_size - sbat_size - raw_data;
-	section = init_pe_section (image_target, section, "mods",
-				   &vma, scn_size, image_target->section_align,
-				   &raw_data, scn_size,
-				   GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
-				   GRUB_PE32_SCN_MEM_READ |
-				   GRUB_PE32_SCN_MEM_WRITE);
+	init_pe_section (image_target, section, &n_sections, ".data",
+			 &vma, scn_size, image_target->section_align,
+			 &raw_data, scn_size,
+			 GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
+			 GRUB_PE32_SCN_MEM_READ |
+			 GRUB_PE32_SCN_MEM_WRITE);
+
+        if (wrap_path == NULL)
+	  {
+	    scn_size = pe_size - layout.reloc_size - sbat_size - raw_data;
+	    init_pe_section (image_target, section, &n_sections, "mods",
+			     &vma, scn_size, image_target->section_align,
+			     &raw_data, scn_size,
+			     GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
+			     GRUB_PE32_SCN_MEM_READ |
+			     GRUB_PE32_SCN_MEM_WRITE);
+	  }
+	else
+	  {
+	    init_pe_section (image_target, section, &n_sections, wrap_section,
+			     &vma, core_size,
+			     image_target->section_align,
+			     &raw_data, ALIGN_UP(core_size, GRUB_PE32_FILE_ALIGNMENT),
+			     GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
+			     GRUB_PE32_SCN_MEM_READ);
+	  }
 
 	if (sbat_path != NULL)
 	  {
 	    pe_sbat = pe_img + raw_data;
 	    grub_util_load_image (sbat_path, pe_sbat);
 
-	    section = init_pe_section (image_target, section, ".sbat",
-				       &vma, sbat_size,
-				       image_target->section_align,
-				       &raw_data, sbat_size,
-				       GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
-				       GRUB_PE32_SCN_MEM_READ);
+	    init_pe_section (image_target, section, &n_sections, ".sbat",
+			     &vma, sbat_size,
+			     image_target->section_align,
+			     &raw_data, sbat_size,
+			     GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
+			     GRUB_PE32_SCN_MEM_READ);
 	  }
 
 	scn_size = layout.reloc_size;
@@ -1463,19 +1514,21 @@ grub_install_generate_image (const char *dir, const char *prefix,
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wdangling-pointer"
 #endif
-	PE_OHDR (o32, o64, base_relocation_table.rva) = grub_host_to_target32 (vma);
+	PE_OHDR (o32, o64, base_relocation_table.rva) = grub_host_to_target32 ((scn_size > 0) ? vma : 0);
 	PE_OHDR (o32, o64, base_relocation_table.size) = grub_host_to_target32 (scn_size);
 #if __GNUC__ >= 12
 #pragma GCC diagnostic pop
 #endif
 	memcpy (pe_img + raw_data, layout.reloc_section, scn_size);
-	init_pe_section (image_target, section, ".reloc",
+	init_pe_section (image_target, section, &n_sections, ".reloc",
 			 &vma, scn_size, image_target->section_align,
 			 &raw_data, scn_size,
 			 GRUB_PE32_SCN_CNT_INITIALIZED_DATA |
 			 GRUB_PE32_SCN_MEM_DISCARDABLE |
 			 GRUB_PE32_SCN_MEM_READ);
 
+	c->num_sections = grub_host_to_target16 (n_sections);
+
 	free (core_img);
 	core_img = pe_img;
 	core_size = pe_size;
-- 
2.30.2

[RFC PATCH 2/4] kern/unwrap: File filter to unwrap files wrapped by grub-wrap

[Zhang Boyang]

This file filter can unwrap a file wrapped by grub-wrap. This process is
done automatically and transparently, thus no additional code is
required at user's side.

First, it checks for file type flags and ignore irrelevant files. Then,
it find target PE section and unwraps the file. Subsequent read
operations will operate on unwrapped data. If error occurred during
unwrapping, e.g. file is not wrapped, it passthroughs original file.

Currently, this filter only operate on font files (GRUB_FILE_TYPE_FONT),
and they must be wrapped with section name set to ".GRUBpf2".

Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
---
 grub-core/Makefile.core.def |   1 +
 grub-core/kern/main.c       |   3 +
 grub-core/kern/unwrap.c     | 188 ++++++++++++++++++++++++++++++++++++
 include/grub/efi/pe32.h     |  75 ++++++++++++++
 include/grub/file.h         |   1 +
 include/grub/unwrap.h       |  25 +++++
 6 files changed, 293 insertions(+)
 create mode 100644 grub-core/kern/unwrap.c
 create mode 100644 include/grub/unwrap.h

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 95942fc8c..fb6a25ed1 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -148,6 +148,7 @@ kernel = {
   common = kern/rescue_reader.c;
   common = kern/term.c;
   common = kern/verifiers.c;
+  common = kern/unwrap.c;
 
   noemu = kern/compiler-rt.c;
   noemu = kern/mm.c;
diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 731c07c29..fa08c0086 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -30,6 +30,7 @@
 #include <grub/reader.h>
 #include <grub/parser.h>
 #include <grub/verify.h>
+#include <grub/unwrap.h>
 
 #ifdef GRUB_MACHINE_PCBIOS
 #include <grub/machine/memory.h>
@@ -281,6 +282,8 @@ grub_main (void)
   /* Init verifiers API. */
   grub_verifiers_init ();
 
+  grub_unwrap_init ();
+
   grub_load_config ();
 
   grub_boot_time ("Before loading embedded modules.");
diff --git a/grub-core/kern/unwrap.c b/grub-core/kern/unwrap.c
new file mode 100644
index 000000000..dc32ccdad
--- /dev/null
+++ b/grub-core/kern/unwrap.c
@@ -0,0 +1,188 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ *  PE unwrapper.
+ */
+
+#include <grub/file.h>
+#include <grub/unwrap.h>
+#include <grub/safemath.h>
+#include <grub/efi/pe32.h>
+
+struct grub_unwrapped
+{
+  grub_file_t file;
+  void *buf;
+  void *real_data;
+  grub_size_t real_size;
+};
+typedef struct grub_unwrapped *grub_unwrapped_t;
+
+static void
+unwrapped_free (grub_unwrapped_t unwrapped)
+{
+  if (unwrapped)
+    {
+      grub_free (unwrapped->buf);
+      grub_free (unwrapped);
+    }
+}
+
+static grub_ssize_t
+unwrapped_read (struct grub_file *file, char *buf, grub_size_t len)
+{
+  grub_unwrapped_t unwrapped = file->data;
+
+  grub_memcpy (buf, (char *) unwrapped->real_data + file->offset, len);
+  return len;
+}
+
+static grub_err_t
+unwrapped_close (struct grub_file *file)
+{
+  grub_unwrapped_t unwrapped = file->data;
+
+  grub_file_close (unwrapped->file);
+  unwrapped_free (unwrapped);
+  file->data = 0;
+
+  /* Device and name are freed by parent. */
+  file->device = 0;
+  file->name = 0;
+
+  return grub_errno;
+}
+
+struct grub_fs unwrapped_fs =
+{
+  .name = "unwrapped_read",
+  .fs_read = unwrapped_read,
+  .fs_close = unwrapped_close
+};
+
+static void
+try_unwrap (grub_unwrapped_t unwrapped, const char (*name)[8])
+{
+  void *buffer = unwrapped->real_data;
+  grub_size_t size = unwrapped->real_size;
+  int i;
+  grub_uint32_t pe_image_header, section_table;
+  grub_uint16_t num_sections;
+  struct grub_pe32_section_table *sections;
+
+  pe_image_header = grub_pe32_get_pe_image_header (buffer, size);
+  if (pe_image_header == 0)
+    goto fail;
+  grub_dprintf ("unwrap", "pe image header at 0x%" PRIxGRUB_UINT32_T ".\n", pe_image_header);
+
+  section_table = grub_pe32_get_section_table (buffer, size, pe_image_header, &num_sections);
+  if (section_table == 0)
+    goto fail;
+  grub_dprintf ("unwrap", "section table at 0x%" PRIxGRUB_UINT32_T ".\n", section_table);
+
+  sections = (void *) ((char *) buffer + section_table);
+  for (i = 0; i < (int) num_sections; i++)
+    {
+      struct grub_pe32_section_table *section = &sections[i];
+      if (grub_memcmp (section->name, *name, 8) == 0)
+	{
+	  grub_uint32_t offset, length, end;
+	  grub_memcpy (&offset, &section->raw_data_offset, sizeof (offset));
+	  grub_memcpy (&length, &section->virtual_size, sizeof (length));
+	  if (grub_add (offset, length, &end) || end > size)
+	    goto fail;
+
+	  grub_dprintf ("unwrap", "unwrap ok, offset 0x%" PRIxGRUB_UINT32_T ", length 0x%" PRIxGRUB_UINT32_T ".\n", offset, length);
+	  unwrapped->real_data = (char *) buffer + offset;
+	  unwrapped->real_size = length;
+	  return;
+	}
+    }
+  grub_dprintf ("unwrap", "section %.8s not found.\n", *name);
+
+fail:
+  grub_dprintf ("unwrap", "unwrap failed, passthrough original file.\n");
+}
+
+static grub_file_t
+grub_unwrap_open (grub_file_t io, enum grub_file_type type)
+{
+  grub_unwrapped_t unwrapped = NULL;
+  grub_file_t ret = NULL;
+  char name[8] = { 0 };
+
+  switch (type & GRUB_FILE_TYPE_MASK)
+    {
+    /* Only process file types we known. */
+    case GRUB_FILE_TYPE_FONT:
+      grub_strncpy(name, ".GRUBpf2", 8);
+      break;
+
+    /* Don't touch other files. */
+    default:
+      return io;
+    }
+
+  ret = grub_malloc (sizeof (*ret));
+  if (ret == NULL)
+    goto fail;
+  *ret = *io;
+
+  ret->fs = &unwrapped_fs;
+  ret->not_easily_seekable = 0;
+  if (ret->size >> (sizeof (grub_size_t) * GRUB_CHAR_BIT - 1))
+    {
+      grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+		  N_("big file unwrapping isn't implemented yet"));
+      goto fail;
+    }
+
+  unwrapped = grub_malloc (sizeof (*unwrapped));
+  if (unwrapped == NULL)
+    goto fail;
+
+  unwrapped->buf = grub_malloc (ret->size);
+  if (unwrapped->buf == NULL)
+    goto fail;
+  if (grub_file_read (io, unwrapped->buf, ret->size) != (grub_ssize_t) ret->size)
+    {
+      if (!grub_errno)
+	grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file %s"),
+		    io->name);
+      goto fail;
+    }
+  unwrapped->real_data = unwrapped->buf;
+  unwrapped->real_size = ret->size;
+
+  try_unwrap (unwrapped, &name);
+
+  unwrapped->file = io;
+  ret->size = unwrapped->real_size;
+  ret->data = unwrapped;
+  return ret;
+
+fail:
+  unwrapped_free (unwrapped);
+  grub_free (ret);
+  return NULL;
+}
+
+void
+grub_unwrap_init (void)
+{
+  grub_file_filter_register (GRUB_FILE_FILTER_UNWRAP, grub_unwrap_open);
+}
diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
index 7609b5198..a52de0ed2 100644
--- a/include/grub/efi/pe32.h
+++ b/include/grub/efi/pe32.h
@@ -20,6 +20,8 @@
 #define GRUB_EFI_PE32_HEADER	1
 
 #include <grub/types.h>
+#include <grub/mm.h>
+#include <grub/safemath.h>
 #include <grub/efi/memory.h>
 
 /* The MSDOS compatibility stub. This was copied from the output of
@@ -280,9 +282,11 @@ struct grub_pe_image_header
 #if GRUB_TARGET_SIZEOF_VOID_P == 8
   /* The Optional header.  */
   struct grub_pe64_optional_header optional_header;
+#define GRUB_PE32_OHDR_MAGIC GRUB_PE32_PE64_MAGIC
 #else
   /* The Optional header.  */
   struct grub_pe32_optional_header optional_header;
+#define GRUB_PE32_OHDR_MAGIC GRUB_PE32_PE32_MAGIC
 #endif
 };
 
@@ -343,4 +347,75 @@ struct grub_pe32_reloc
 #define GRUB_PE32_REL_I386_DIR32	0x6
 #define GRUB_PE32_REL_I386_REL32	0x14
 
+/*
+ * Get offset of PE image header of given in-memory file, as grub_uint32_t.
+ * Returns 0 if magic numbers are not found or header location is illegal.
+ */
+static inline grub_uint32_t
+grub_pe32_get_pe_image_header (const void *buffer, grub_size_t size)
+{
+  grub_uint16_t magic;
+  grub_uint32_t offset, end;
+  const struct grub_msdos_image_header *msdos_hdr;
+  const struct grub_pe_image_header *pe_hdr;
+
+  if (size < sizeof (*msdos_hdr))
+    return 0;
+  msdos_hdr = buffer;
+
+  magic = grub_cpu_to_le16 (GRUB_PE32_MAGIC);
+  if (grub_memcmp (&msdos_hdr->msdos_magic, &magic, sizeof (magic)) != 0)
+    return 0;
+
+  grub_memcpy (&offset, &msdos_hdr->pe_image_header_offset, sizeof (offset));
+  offset = grub_le_to_cpu32 (offset);
+  if (grub_add (offset, sizeof (*pe_hdr), &end) || end > size)
+    return 0;
+  pe_hdr = (const void *) ((const char *) buffer + offset);
+
+  if (grub_memcmp (&pe_hdr->signature, "PE\0\0", GRUB_PE32_SIGNATURE_SIZE) != 0)
+    return 0;
+
+  magic = grub_cpu_to_le16 (GRUB_PE32_OHDR_MAGIC);
+  if (grub_memcmp (&pe_hdr->optional_header.magic, &magic, sizeof (magic)) != 0)
+    return 0;
+
+  return offset;
+}
+
+/*
+ * Get offset of PE section table of given in-memory file, as grub_uint32_t.
+ * `pe_image_header` should be the value returned by grub_pe32_get_pe_image_header().
+ * Number of sections is stored at `*num_sections`, as grub_uint16_t.
+ * Returns 0 if section table's location is illegal.
+ */
+static inline grub_uint32_t
+grub_pe32_get_section_table (const void *buffer, grub_size_t size,
+			     grub_uint32_t pe_image_header,
+			     grub_uint16_t *num_sections)
+{
+  grub_uint32_t offset, length, end;
+  const struct grub_pe_image_header *pe_hdr;
+  grub_uint16_t ohdr_size, section_count;
+
+  /* grub_pe32_get_pe_image_header() has already checked boundaries. */
+  pe_hdr = (const void *) ((const char *) buffer + pe_image_header);
+  offset = pe_image_header + (grub_uint32_t) ((const char *) &pe_hdr->optional_header - (const char *) pe_hdr);
+
+  grub_memcpy (&ohdr_size, &pe_hdr->coff_header.optional_header_size, sizeof (ohdr_size));
+  ohdr_size = grub_le_to_cpu16 (ohdr_size);
+  if (grub_add (offset, ohdr_size, &offset))
+    return 0;
+
+  grub_memcpy (&section_count, &pe_hdr->coff_header.num_sections, sizeof (section_count));
+  section_count = grub_le_to_cpu16 (section_count);
+  if (grub_mul (section_count, sizeof (struct grub_pe32_section_table), &length))
+    return 0;
+  if (grub_add (offset, length, &end) || end > size)
+    return 0;
+
+  *num_sections = section_count;
+  return offset;
+}
+
 #endif /* ! GRUB_EFI_PE32_HEADER */
diff --git a/include/grub/file.h b/include/grub/file.h
index a5bf3a792..5fae2a8fc 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -183,6 +183,7 @@ extern grub_disk_read_hook_t EXPORT_VAR(grub_file_progress_hook);
 typedef enum grub_file_filter_id
   {
     GRUB_FILE_FILTER_VERIFY,
+    GRUB_FILE_FILTER_UNWRAP,
     GRUB_FILE_FILTER_GZIO,
     GRUB_FILE_FILTER_XZIO,
     GRUB_FILE_FILTER_LZOPIO,
diff --git a/include/grub/unwrap.h b/include/grub/unwrap.h
new file mode 100644
index 000000000..c5155fc6d
--- /dev/null
+++ b/include/grub/unwrap.h
@@ -0,0 +1,25 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2022  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_UNWRAP_HEADER
+#define GRUB_UNWRAP_HEADER 1
+
+extern void
+grub_unwrap_init (void);
+
+#endif /* ! GRUB_UNWRAP_HEADER */
-- 
2.30.2

[RFC PATCH 3/4] kern/efi/sb: Set requirements for PE images

[Zhang Boyang]

There is a problem in shim lock verifier after PE wrappers created by
grub-wrap are introduced. The shim lock verifier always says a file is
ok as long as shim says that file is ok, so there is a possibility that
wrappers may be used as kernel images, leading to crashes or
vulnerabilities.

This patch fixes the problem by introducing PE requirements checks to
shim lock verifier. For kernel images, it now only says ok to files
marked as EFI Application (wrappers are marked as Windows GUI).

Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
---
 grub-core/kern/efi/sb.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index db42c2539..4fb751d8a 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -113,12 +113,41 @@ grub_efi_get_secureboot (void)
   return secureboot;
 }
 
+struct pe_requirements {
+  grub_uint16_t subsystem;
+};
+
+static bool verify_pe_requirements (void *buf, grub_size_t size,
+				   struct pe_requirements *constraints)
+{
+  grub_uint32_t pe_image_header;
+  struct grub_pe_image_header *pe_hdr;
+  grub_uint16_t subsystem;
+
+  pe_image_header = grub_pe32_get_pe_image_header (buf, size);
+  if (pe_image_header == 0)
+    return false;
+  pe_hdr = (void *) ((char *) buf + pe_image_header);
+
+  grub_memcpy (&subsystem, &pe_hdr->optional_header.subsystem, sizeof (subsystem));
+  subsystem = grub_le_to_cpu16 (subsystem);
+  if (subsystem != constraints->subsystem)
+    return false;
+
+  return true;
+}
+
+static struct pe_requirements kernel_pe_requirements = {
+  .subsystem = GRUB_PE32_SUBSYSTEM_EFI_APPLICATION,
+};
+
 static grub_err_t
 shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)),
 			 enum grub_file_type type,
-			 void **context __attribute__ ((unused)),
+			 void **context,
 			 enum grub_verify_flags *flags)
 {
+  *context = NULL;
   *flags = GRUB_VERIFY_FLAGS_NONE;
 
   switch (type & GRUB_FILE_TYPE_MASK)
@@ -130,6 +159,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)),
     case GRUB_FILE_TYPE_XNU_KERNEL:
     case GRUB_FILE_TYPE_PLAN9_KERNEL:
     case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE:
+      *context = &kernel_pe_requirements;
       *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
       return GRUB_ERR_NONE;
 
@@ -171,7 +201,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)),
 }
 
 static grub_err_t
-shim_lock_verifier_write (void *context __attribute__ ((unused)), void *buf, grub_size_t size)
+shim_lock_verifier_write (void *context, void *buf, grub_size_t size)
 {
   grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol (&shim_lock_guid, 0);
 
@@ -181,6 +211,9 @@ shim_lock_verifier_write (void *context __attribute__ ((unused)), void *buf, gru
   if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
     return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
 
+  if (!verify_pe_requirements (buf, size, context))
+    return grub_error (GRUB_ERR_BAD_FILE_TYPE, N_("bad file type"));
+
   return GRUB_ERR_NONE;
 }
 
-- 
2.30.2

[RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Zhang Boyang]

Since font files can be wrapped as PE images by grub-wrap, use shim to
verify font files if Secure Boot is enabled. To prevent other PE files
(e.g. kernel images) used as wrappers, it only allows files marked as
Windows GUI used as wrappers.

Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
---
 grub-core/kern/efi/sb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index 4fb751d8a..a86763232 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -141,6 +141,10 @@ static struct pe_requirements kernel_pe_requirements = {
   .subsystem = GRUB_PE32_SUBSYSTEM_EFI_APPLICATION,
 };
 
+static struct pe_requirements wrapper_pe_requirements = {
+  .subsystem = GRUB_PE32_SUBSYSTEM_WINDOWS_GUI,
+};
+
 static grub_err_t
 shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)),
 			 enum grub_file_type type,
@@ -163,6 +167,11 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)),
       *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
       return GRUB_ERR_NONE;
 
+    case GRUB_FILE_TYPE_FONT:
+      *context = &wrapper_pe_requirements;
+      *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
+      return GRUB_ERR_NONE;
+
     /* Files that do not affect secureboot state. */
     case GRUB_FILE_TYPE_NONE:
     case GRUB_FILE_TYPE_LOOPBACK:
-- 
2.30.2

Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

Zhang Boyang <zhangboyang.id@gmail.com> writes:

> Since font files can be wrapped as PE images by grub-wrap, use shim to
> verify font files if Secure Boot is enabled. To prevent other PE files
> (e.g. kernel images) used as wrappers, it only allows files marked as
> Windows GUI used as wrappers.

Thanks for writing this; it's helpful to have something concrete to look
at.

This approach is very font-focused, and while I understand that given
the discussion, I do still wonder if it wouldn't be better to make fonts
an instance of modules.  If fonts become instances of modules, and
modules are wrapped into PE files, that not only seems cleaner but also
gives us signed module support without baking those into the image.

What do you think?

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Steve McIntyre]

On Tue, Dec 06, 2022 at 11:09:57AM -0500, Robbie Harwood wrote:
>Zhang Boyang <zhangboyang.id@gmail.com> writes:
>
>> Since font files can be wrapped as PE images by grub-wrap, use shim to
>> verify font files if Secure Boot is enabled. To prevent other PE files
>> (e.g. kernel images) used as wrappers, it only allows files marked as
>> Windows GUI used as wrappers.
>
>Thanks for writing this; it's helpful to have something concrete to look
>at.

Definitely!

>This approach is very font-focused, and while I understand that given
>the discussion, I do still wonder if it wouldn't be better to make fonts
>an instance of modules.  If fonts become instances of modules, and
>modules are wrapped into PE files, that not only seems cleaner but also
>gives us signed module support without baking those into the image.
>
>What do you think?

Nod, that probably makes more sense if we want to go this way. I'm not
sure we do personally, but...

-- 
Steve McIntyre, Cambridge, UK.                                steve@einval.com
Dance like no one's watching. Encrypt like everyone is.
 - @torproject

Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Michael Chang]

On Tue, Dec 06, 2022 at 11:09:57AM -0500, Robbie Harwood wrote:
> Zhang Boyang <zhangboyang.id@gmail.com> writes:
> 
> > Since font files can be wrapped as PE images by grub-wrap, use shim to
> > verify font files if Secure Boot is enabled. To prevent other PE files
> > (e.g. kernel images) used as wrappers, it only allows files marked as
> > Windows GUI used as wrappers.
> 
> Thanks for writing this; it's helpful to have something concrete to look
> at.
> 
> This approach is very font-focused, and while I understand that given
> the discussion, I do still wonder if it wouldn't be better to make fonts
> an instance of modules.  If fonts become instances of modules, and
> modules are wrapped into PE files, that not only seems cleaner but also
> gives us signed module support without baking those into the image.

Why not just making the PE wrap applicable to all file types, be it font
files, grub modules or even (static) initrd. Providing a solution to
sign arbitrary data or binary with this PE envelope sounds to me a very
attractive feature and worthwhile the extra miles. :)

Thanks,
Michael

> 
> What do you think?
> 
> Be well,
> --Robbie

Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Dimitri John Ledkov]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

Yes yes yes yes. Signed dtb in grub at last.

On Wed, 7 Dec 2022, 03:16 Michael Chang via Grub-devel, <grub-devel@gnu.org>
wrote:

> On Tue, Dec 06, 2022 at 11:09:57AM -0500, Robbie Harwood wrote:
> > Zhang Boyang <zhangboyang.id@gmail.com> writes:
> >
> > > Since font files can be wrapped as PE images by grub-wrap, use shim to
> > > verify font files if Secure Boot is enabled. To prevent other PE files
> > > (e.g. kernel images) used as wrappers, it only allows files marked as
> > > Windows GUI used as wrappers.
> >
> > Thanks for writing this; it's helpful to have something concrete to look
> > at.
> >
> > This approach is very font-focused, and while I understand that given
> > the discussion, I do still wonder if it wouldn't be better to make fonts
> > an instance of modules.  If fonts become instances of modules, and
> > modules are wrapped into PE files, that not only seems cleaner but also
> > gives us signed module support without baking those into the image.
>
> Why not just making the PE wrap applicable to all file types, be it font
> files, grub modules or even (static) initrd. Providing a solution to
> sign arbitrary data or binary with this PE envelope sounds to me a very
> attractive feature and worthwhile the extra miles. :)
>
> Thanks,
> Michael
>
> >
> > What do you think?
> >
> > Be well,
> > --Robbie
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

[-- Attachment #2: Type: text/html, Size: 2225 bytes --]