Merge ~xnox/grub:do-not-validate-twice into ~ubuntu-core-dev/grub/+git/ubuntu:ubuntu

[Dimitri John Ledkov]

Do not validate kernels twice

[Julian Andres Klode (juliank)]

Rebased and merged

diff --git a/debian/patches/linuxefi-do-not-validate-kernels-twice.patch b/debian/patches/linuxefi-do-not-validate-kernels-twice.patch
new file mode 100644
index 0000000..e6c059f
--- /dev/null
+++ b/debian/patches/linuxefi-do-not-validate-kernels-twice.patch
@@ -0,0 +1,227 @@
+From: Dimitri John Ledkov <[e-mail]>
+Date: Thu, 3 Mar 2022 13:10:56 +0100
+Subject: linuxefi: do not validate kernels twice
+
+On codebases that have shim-lock-verifier built into the grub core
+(like 2.06 upstream), shim-lock-verifier is in enforcing mode when
+booted with secureboot. It means that grub_cmd_linux() command
+attempts to perform shim validate upon opening linux kernel image,
+including kernel measurement. And the verifier correctly returns file
+open error when shim validate protocol is not present or shim fails to
+validate the kernel.
+
+This makes the call to grub_linuxefi_secure_validate() redundant, but
+also harmful. As validating the kernel image twice, extends the PCRs
+with the same measurement twice. Which breaks existing sealing
+policies when upgrading from grub2.04+rhboot+sb+linuxefi to
+grub2.06+rhboot+sb+linuxefi builds. It is also incorrect to measure
+the kernel twice.
+
+This patch must not be ported to older editions of grub code bases
+that do not have verifiers framework, or it is not builtin, or
+shim-lock-verifier is an optional module.
+
+This patch is tested to ensure that unsigned kernels are not possible
+to boot in secureboot mode when shim rejects kernel, or shim protocol
+is missing, and that the measurements become stable once again. The
+above also ensures that CVE-2020-15705 is not reintroduced.
+
+This is a backport of https://github.com/rhboot/grub2/pull/97 onto
+Ubuntu packaging.
+
+Signed-off-by: Dimitri John Ledkov <[e-mail]>
+---
+ grub-core/kern/arm/coreboot/coreboot.S |  2 --
+ grub-core/loader/efi/chainloader.c     |  8 ++-----
+ grub-core/loader/efi/linux.c           | 12 ----------
+ grub-core/loader/efi/linux_sb.c        | 41 ----------------------------------
+ grub-core/loader/i386/efi/linux.c      | 13 -----------
+ include/grub/efi/linux.h               |  2 --
+ 6 files changed, 2 insertions(+), 76 deletions(-)
+
+diff --git a/grub-core/kern/arm/coreboot/coreboot.S b/grub-core/kern/arm/coreboot/coreboot.S
+index 70998c0..13e73ac 100644
+--- a/grub-core/kern/arm/coreboot/coreboot.S
++++ b/grub-core/kern/arm/coreboot/coreboot.S
+@@ -42,8 +42,6 @@ FUNCTION(grub_armv7_get_timer_frequency)
+ 	mrc p15, 0, r0, c14, c0, 0
+ 	bx	lr
+ 
+-int
+-EXPORT_FUNC(grub_linuxefi_secure_validate) (void *data, grub_uint32_t size);
+ grub_err_t
+ EXPORT_FUNC(grub_efi_linux_boot) (void *kernel_address, grub_off_t offset,
+ 				  void *kernel_param);
+diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
+index 4a85f41..c6ae2fc 100644
+--- a/grub-core/loader/efi/chainloader.c
++++ b/grub-core/loader/efi/chainloader.c
+@@ -910,7 +910,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
+   grub_efi_device_path_t *dp = 0;
+   char *filename;
+   void *boot_image = 0;
+-  int rc;
+ 
+   if (argc == 0)
+     return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
+@@ -1080,9 +1079,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
+     }
+ #endif
+ 
+-  rc = grub_linuxefi_secure_validate((void *)((grub_addr_t) address), fsize);
+-  grub_dprintf ("chain", "linuxefi_secure_validate: %d\n", rc);
+-  if (rc > 0)
++  if (grub_efi_get_secureboot () == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
+     {
+       grub_file_close (file);
+       if (orig_dev)
+@@ -1092,7 +1089,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
+ 		       grub_secureboot_chainloader_unload, 0);
+       return 0;
+     }
+-  else if (rc == 0)
++  else
+     {
+       grub_load_and_start_image(boot_image);
+       grub_file_close (file);
+@@ -1103,7 +1100,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ ((unused)),
+ 
+       return 0;
+     }
+-  // -1 fall-through to fail
+ 
+ fail:
+   if (orig_dev)
+diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
+index 576d8cb..24abc0c 100644
+--- a/grub-core/loader/efi/linux.c
++++ b/grub-core/loader/efi/linux.c
+@@ -433,7 +433,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
+   struct linux_arch_kernel_header lh;
+   struct grub_arm64_linux_pe_header *pe;
+   grub_err_t err;
+-  int rc;
+ 
+   grub_dl_ref (my_mod);
+ 
+@@ -478,17 +477,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
+ 
+   grub_dprintf ("linux", "kernel @ %p\n", kernel_addr);
+ 
+-  if (grub_efi_get_secureboot() == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
+-    {
+-      rc = grub_linuxefi_secure_validate (kernel_addr, kernel_size);
+-      if (rc <= 0)
+-        {
+-          grub_error (GRUB_ERR_INVALID_COMMAND,
+-		      N_("%s has invalid signature"), argv[0]);
+-          goto fail;
+-        }
+-    }
+-
+   pe = (void *)((unsigned long)kernel_addr + lh.hdr_offset);
+   handover_offset = pe->opt.entry_addr;
+ 
+diff --git a/grub-core/loader/efi/linux_sb.c b/grub-core/loader/efi/linux_sb.c
+index a09479c..13bcb02 100644
+--- a/grub-core/loader/efi/linux_sb.c
++++ b/grub-core/loader/efi/linux_sb.c
+@@ -25,47 +25,6 @@
+ #include <grub/efi/linux.h>
+ #include <grub/efi/sb.h>
+ 
+-#define SHIM_LOCK_GUID \
+- { 0x605dab50, 0xe046, 0x4300, {0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23} }
+-
+-struct grub_efi_shim_lock
+-{
+-  grub_efi_status_t (*verify) (void *buffer, grub_uint32_t size);
+-};
+-typedef struct grub_efi_shim_lock grub_efi_shim_lock_t;
+-
+-// Returns 1 on success, -1 on error, 0 when not available
+-int
+-grub_linuxefi_secure_validate (void *data, grub_uint32_t size)
+-{
+-  grub_efi_guid_t guid = SHIM_LOCK_GUID;
+-  grub_efi_shim_lock_t *shim_lock;
+-  int status;
+-
+-  grub_dprintf ("linuxefi", "Locating shim protocol\n");
+-  shim_lock = grub_efi_locate_protocol(&guid, NULL);
+-  grub_dprintf ("secureboot", "shim_lock: %p\n", shim_lock);
+-  if (!shim_lock)
+-    {
+-      grub_dprintf ("secureboot", "shim not available\n");
+-      return 0;
+-    }
+-
+-  grub_dprintf ("secureboot", "Asking shim to verify kernel signature\n");
+-  status = shim_lock->verify (data, size);
+-  grub_dprintf ("secureboot", "shim_lock->verify(): %d\n", status);
+-  if (status == GRUB_EFI_SUCCESS)
+-    {
+-      grub_dprintf ("secureboot", "Kernel signature verification passed\n");
+-      return 1;
+-    }
+-
+-  grub_dprintf ("secureboot", "Kernel signature verification failed (0x%lx)\n",
+-		(unsigned long) status);
+-
+-  return -1;
+-}
+-
+ typedef void (*handover_func) (void *, grub_efi_system_table_t *, void *);
+ 
+ grub_err_t
+diff --git a/grub-core/loader/i386/efi/linux.c b/grub-core/loader/i386/efi/linux.c
+index 6a90743..cbd313a 100644
+--- a/grub-core/loader/i386/efi/linux.c
++++ b/grub-core/loader/i386/efi/linux.c
+@@ -27,7 +27,6 @@
+ #include <grub/lib/cmdline.h>
+ #include <grub/efi/efi.h>
+ #include <grub/efi/linux.h>
+-#include <grub/efi/sb.h>
+ #include <grub/safemath.h>
+ 
+ GRUB_MOD_LICENSE ("GPLv3+");
+@@ -170,7 +169,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
+   grub_ssize_t start, filelen;
+   void *kernel = NULL;
+   int setup_header_end_offset;
+-  int rc;
+ 
+   grub_dl_ref (my_mod);
+ 
+@@ -201,17 +199,6 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
+       goto fail;
+     }
+ 
+-  if (grub_efi_get_secureboot() == GRUB_EFI_SECUREBOOT_MODE_ENABLED)
+-    {
+-      rc = grub_linuxefi_secure_validate (kernel, filelen);
+-      if (rc <= 0)
+-	{
+-	  grub_error (GRUB_ERR_ACCESS_DENIED, N_("%s has invalid signature"),
+-		      argv[0]);
+-	  goto fail;
+-	}
+-    }
+-
+   params = grub_efi_allocate_pages_max (0x3fffffff,
+ 					BYTES_TO_PAGES(sizeof(*params)));
+   if (! params)
+diff --git a/include/grub/efi/linux.h b/include/grub/efi/linux.h
+index 39f5a9a..cedf20c 100644
+--- a/include/grub/efi/linux.h
++++ b/include/grub/efi/linux.h
+@@ -22,8 +22,6 @@
+ #include <grub/err.h>
+ #include <grub/symbol.h>
+ 
+-int
+-EXPORT_FUNC(grub_linuxefi_secure_validate) (void *data, grub_uint32_t size);
+ grub_err_t
+ EXPORT_FUNC(grub_efi_linux_boot) (void *kernel_address, grub_off_t offset,
+ 				  void *kernel_param);
diff --git a/debian/patches/series b/debian/patches/series
index 7942ada..3b296a2 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -121,3 +121,4 @@ ubuntu-os-prober-auto.patch
 efi-correct-struct-grub_efi_boot_services.patch
 efi-implement-grub_efi_run_image.patch
 fat-fix-listing-the-root-directory.patch
+linuxefi-do-not-validate-kernels-twice.patch