Re: UEFI Secureboot not succeeding with Grub 2.06 and later version

[Javier Martinez Canillas]

On 7/15/21 4:43 PM, Sayanta Pattanayak wrote:
> Hi Javier,
> 
> I tried with your suggested change, but observing Exception as following - 
>

Thanks for testing.

[snip]
 
> 
> Synchronous Exception at 0x00000000F92699DC
> Synchronous Exception at 0x00000000F92699DC

Hmm, I found another bug in the patch since the error was returned inside
the for loop and not after that. So may lead to a NULL pointer dereference
error if not using the --disable-shim-lock option but booting without shim.

[snip]

> 
> Another doubt, should the Image be detected as "UEFI stub kernel", as 
> happened with experimental suggestion by Daniel?
>

I don't think is needed but I'll leave that to Daniel.
 
> One minor addition in your patch, added below.
>

Thanks for that. That happen when I write a patch without even build
testing it....

Can you give it a try to this one now? I built tested this time but
still couldn't test it. I should be able to do that but no earlier
than next week.

>From a7c205faef72df4dd6decb114b35b53941c17014 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@redhat.com>
Date: Thu, 15 Jul 2021 13:08:11 +0200
Subject: [RFC PATCH v2] kern/efi/sb: Allow validation to be done by the UEFI 
firmware

The shim_lock protocol is used to delegate that PE32+ binaries have been
signed with a trusted key. This is done because GRUB currently lacks the
ability to do the validation itself.

But in certain configurations a user may not want to use shim for this,
and either delegate on a different verifier (i.e: pgp) or just leave it
to the UEFI firmware. The latter can be done if both GRUB and the Linux
kernel have been signed by a key trusted by the UEFI firmware.

There's an grub-mkimage --disable-shim-lock option that could be used to
avoid using he shim_lock protocol and rely on another verifier, but that
will not work for the latter case. Since the lockdown verifier defers it
to another verifier but no verifier validates the Linux kernel images.

To workaround that, let's make the shim_lock verifier always validate a
kernel file type if the --disable-shim-lock option has been enabled.

Reported-by: Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
 grub-core/kern/efi/sb.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
index c52ec6226a6..479f4adcba4 100644
--- a/grub-core/kern/efi/sb.c
+++ b/grub-core/kern/efi/sb.c
@@ -141,10 +141,19 @@ 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)
 {
+  struct grub_module_header *header;
   grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol 
(&shim_lock_guid, 0);
 
   if (!sl)
-    return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not 
found"));
+    {
+      /* shim_lock is missing, check if GRUB image is built with 
--disable-shim-lock. */
+      FOR_MODULES (header)
+        {
+          if (header->type == OBJ_TYPE_DISABLE_SHIM_LOCK)
+            return GRUB_ERR_NONE;
+        }
+      return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not 
found"));
+    }
 
   if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
     return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
@@ -162,20 +171,9 @@ struct grub_file_verifier shim_lock_verifier =
 void
 grub_shim_lock_verifier_setup (void)
 {
-  struct grub_module_header *header;
   grub_efi_shim_lock_protocol_t *sl =
     grub_efi_locate_protocol (&shim_lock_guid, 0);
 
-  /* shim_lock is missing, check if GRUB image is built with 
--disable-shim-lock. */
-  if (!sl)
-    {
-      FOR_MODULES (header)
-       {
-         if (header->type == OBJ_TYPE_DISABLE_SHIM_LOCK)
-           return;
-       }
-    }
-
   /* Secure Boot is off. Do not load shim_lock. */
   if (grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED)
     return;
-- 
2.31.1