Yes, sorry...
On Wed, Apr 2, 2025 at 9:39 AM Michael Chang <mch...@suse.com> wrote:
>
> On Tue, Apr 01, 2025 at 11:26:42AM +0100, Mate Kukri wrote:
> > Use loader protocol for image verification where available, otherwise
> > fall back to the old shim lock protocol.
> >
> > Signed-off-by: Mate Kukri <mate.ku...@canonical.com>
> > ---
> > grub-core/kern/efi/sb.c | 58 ++++++++++++++++++++----------------
> > grub-core/loader/efi/linux.c | 6 ++--
> > include/grub/efi/api.h | 5 ++++
> > include/grub/efi/sb.h | 2 +-
> > 4 files changed, 41 insertions(+), 30 deletions(-)
> >
> > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
> > index 8d3e41360..34ba323ee 100644
> > --- a/grub-core/kern/efi/sb.c
> > +++ b/grub-core/kern/efi/sb.c
> > @@ -31,8 +31,10 @@
> > #include <grub/verify.h>
> >
> > static grub_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
> > +static grub_guid_t shim_loader_guid = GRUB_EFI_SHIM_IMAGE_LOADER_GUID;
> >
> > -static bool shim_lock_enabled = false;
> > +static grub_efi_loader_t *shim_loader = NULL;
> > +static grub_efi_shim_lock_protocol_t *shim_lock = NULL;
> >
> > /*
> > * Determine whether we're in secure boot mode.
> > @@ -95,14 +97,6 @@ grub_efi_get_secureboot (void)
> > if (!(attr & GRUB_EFI_VARIABLE_RUNTIME_ACCESS) && *moksbstate == 1)
> > {
> > secureboot = GRUB_EFI_SECUREBOOT_MODE_DISABLED;
> > - /*
> > - * TODO: Replace this all with shim's LoadImage protocol, delegating
> > policy to it.
> > - *
> > - * We need to set shim_lock_enabled here because we disabled secure
> > boot
> > - * validation *inside* shim but not in the firmware, so we set this
> > variable
> > - * here to trigger that code path, whereas the actual verifier is
> > not enabled.
> > - */
> > - shim_lock_enabled = true;
> > goto out;
> > }
> >
> > @@ -183,15 +177,24 @@ 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)
> > {
> > - grub_efi_shim_lock_protocol_t *sl = grub_efi_locate_protocol
> > (&shim_lock_guid, 0);
> > + grub_efi_handle_t image_handle;
> >
> > - if (!sl)
> > - return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim_lock protocol not
> > found"));
> > + if (shim_loader)
> > + {
> > +
> > + if (shim_loader->load_image (false, grub_efi_image_handle, NULL,
> > buf, size, &image_handle) != GRUB_EFI_SUCCESS)
> > + return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim
> > signature"));
> >
> > - if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
> > - return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim signature"));
> > + return GRUB_ERR_NONE;
> > + }
> > + if (shim_lock)
> > + {
> > + if (sl->verify (buf, size) != GRUB_EFI_SUCCESS)
>
> Should it be s/sl/shim_lock/ ?
>
> Thanks,
> Michael
>
> > + return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("bad shim
> > signature"));
> > + return GRUB_ERR_NONE;
> > + }
> >
> > - return GRUB_ERR_NONE;
> > + return grub_error (GRUB_ERR_ACCESS_DENIED, N_("shim protocols not
> > found"));
> > }
> >
> > struct grub_file_verifier shim_lock_verifier =
> > @@ -205,11 +208,17 @@ 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)
> > + /* Secure Boot is off. Ignore shim. */
> > + if (grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED)
> > + return;
> > +
> > + /* Find both shim protocols */
> > + shim_loader = grub_efi_locate_protocol (&shim_loader_guid, 0);
> > + shim_lock = grub_efi_locate_protocol (&shim_lock_guid, 0);
> > +
> > + /* shim is missing, check if GRUB image is built with
> > --disable-shim-lock. */
> > + if (!shim_loader && !shim_lock)
> > {
> > FOR_MODULES (header)
> > {
> > @@ -218,21 +227,18 @@ grub_shim_lock_verifier_setup (void)
> > }
> > }
> >
> > - /* Secure Boot is off. Do not load shim_lock. */
> > - if (grub_efi_get_secureboot () != GRUB_EFI_SECUREBOOT_MODE_ENABLED)
> > - return;
> > -
> > /* Enforce shim_lock_verifier. */
> > grub_verifier_register (&shim_lock_verifier);
> >
> > - shim_lock_enabled = true;
> > + /* Register shim loader if supported. */
> > + grub_efi_register_loader (shim_loader);
> >
> > grub_env_set ("shim_lock", "y");
> > grub_env_export ("shim_lock");
> > }
> >
> > bool
> > -grub_is_shim_lock_enabled (void)
> > +grub_is_using_legacy_shim_lock_protocol (void)
> > {
> > - return shim_lock_enabled;
> > + return !shim_loader && shim_lock;
> > }
> > diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> > index 7342f3ee7..9cd84ab12 100644
> > --- a/grub-core/loader/efi/linux.c
> > +++ b/grub-core/loader/efi/linux.c
> > @@ -461,10 +461,10 @@ grub_cmd_linux (grub_command_t cmd __attribute__
> > ((unused)),
> >
> > grub_dl_ref (my_mod);
> >
> > - if (grub_is_shim_lock_enabled () == true)
> > + if (grub_is_using_legacy_shim_lock_protocol () == true)
> > {
> > #if defined(__i386__) || defined(__x86_64__)
> > - grub_dprintf ("linux", "shim_lock enabled, falling back to legacy
> > Linux kernel loader\n");
> > + grub_dprintf ("linux", "using legacy shim_lock protocol, falling
> > back to legacy Linux kernel loader\n");
> >
> > err = grub_cmd_linux_x86_legacy (cmd, argc, argv);
> >
> > @@ -473,7 +473,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__
> > ((unused)),
> > else
> > goto fail;
> > #else
> > - grub_dprintf ("linux", "shim_lock enabled, trying Linux kernel EFI
> > stub loader\n");
> > + grub_dprintf ("linux", "using legacy shim_lock protocol on non-x86,
> > only db verifiable kernels will work\n");
> > #endif
> > }
> >
> > diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
> > index b686e8afe..9ae908729 100644
> > --- a/include/grub/efi/api.h
> > +++ b/include/grub/efi/api.h
> > @@ -364,6 +364,11 @@
> > { 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
> > }
> >
> > +#define GRUB_EFI_SHIM_IMAGE_LOADER_GUID \
> > + { 0x1f492041, 0xfadb, 0x4e59, \
> > + {0x9e, 0x57, 0x7c, 0xaf, 0xe7, 0x3a, 0x55, 0xab } \
> > + }
> > +
> > #define GRUB_EFI_RNG_PROTOCOL_GUID \
> > { 0x3152bca5, 0xeade, 0x433d, \
> > { 0x86, 0x2e, 0xc0, 0x1c, 0xdc, 0x29, 0x1f, 0x44 } \
> > diff --git a/include/grub/efi/sb.h b/include/grub/efi/sb.h
> > index 49a9ad01c..4cae88376 100644
> > --- a/include/grub/efi/sb.h
> > +++ b/include/grub/efi/sb.h
> > @@ -32,7 +32,7 @@ extern grub_uint8_t
> > EXPORT_FUNC (grub_efi_get_secureboot) (void);
> >
> > extern bool
> > -EXPORT_FUNC (grub_is_shim_lock_enabled) (void);
> > +EXPORT_FUNC (grub_is_using_legacy_shim_lock_protocol) (void);
> >
> > extern void
> > grub_shim_lock_verifier_setup (void);
> > --
> > 2.39.5
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
