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

[Sayanta Pattanayak]

Thanks for your quick response.
I did try with the latest change, but still observing  "shim_lock protocol not 
found" error.  For " grub-mkimage", the option " --disable-shim-lock" is used.

disk/efi/efidisk.c:531: opening hd0 succeeded
partmap/gpt.c:93: Read a valid GPT header
partmap/gpt.c:115: GPT entry 0: start=2048, length=40959
partmap/gpt.c:115: GPT entry 1: start=43008, length=409599
kern/fs.c:56: Detecting ext2...
kern/verifiers.c:88: file: /Image type: 3
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc40 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0xcc80 from hd0

...

disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dcc0 from hd0
disk/efi/efidisk.c:593: reading 0x40 sectors at the sector 0x1dd00 from hd0
kern/disk.c:295: Closing `hd0'.
disk/efi/efidisk.c:540: closing hd0
error: shim_lock protocol not found.
script/script.c:65: free 0x81fff4a6e0

>-----Original Message-----
>From: Javier Martinez Canillas <javierm@redhat.com>
>Sent: Thursday, July 15, 2021 8:43 PM
>To: Sayanta Pattanayak <Sayanta.Pattanayak@arm.com>; Daniel Kiper
><dkiper@net-space.pl>
>Cc: grub-devel@gnu.org; nd <nd@arm.com>; xnox@ubuntu.com;
>pjones@redhat.com; leif@nuviainc.com
>Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later version
>
>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