Re: [PATCH v1 11/15] grub-install: disable support for EFI platforms

[Neal Gompa]

On Thu, Oct 31, 2024 at 3:43 PM Leo Sandoval <lsandova@redhat.com> wrote:
>
> From: Jan Hlavac <jhlavc@redhat.com>
>
> For each platform, GRUB is shipped as a kernel image and a set of
> modules. These files are then used by the grub-install utility to
> install GRUB on a specific device. However, in order to support UEFI
> Secure Boot, the resulting EFI binary must be signed by a recognized
> private key. For this reason, for EFI platforms, most distributions also
> ship prebuilt EFI binaries signed by a distribution-specific private
> key. In this case, however, the grub-install utility should not be used
> because it would overwrite the signed EFI binary.
>
> The current fix is suboptimal because it preserves all EFI-related code.
> A better solution could be to modularize the code and provide a
> build-time option.
>
> Resolves: rhbz#1737444
>
> Signed-off-by: Jan Hlavac <jhlavac@redhat.com>
> [rharwood: drop man page]
> ---
>  docs/grub.texi      |  7 +++++++
>  util/grub-install.c | 41 +++++++++++++++++++----------------------
>  2 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..bc9791794 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -7080,6 +7080,13 @@ grub-install @var{install_device}
>  The device name @var{install_device} is an OS device name or a GRUB
>  device name.
>
> +In order to support UEFI Secure Boot, the resulting GRUB EFI binary must
> +be signed by a recognized private key. For this reason, for EFI
> +platforms, most distributions also ship prebuilt GRUB EFI binaries
> +signed by a distribution-specific private key. In this case, however,
> +@command{grub2-install} should not be used because it would overwrite
> +the signed EFI binary.
> +
>  @command{grub-install} accepts the following options:
>
>  @table @option
> diff --git a/util/grub-install.c b/util/grub-install.c
> index c245d9359..ee61b042b 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -910,6 +910,25 @@ main (int argc, char *argv[])
>
>    platform = grub_install_get_target (grub_install_source_directory);
>
> +  switch (platform)
> +    {
> +    case GRUB_INSTALL_PLATFORM_ARM_EFI:
> +    case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> +    case GRUB_INSTALL_PLATFORM_I386_EFI:
> +    case GRUB_INSTALL_PLATFORM_IA64_EFI:
> +    case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> +    case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> +    case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> +    case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> +      is_efi = 1;
> +      grub_util_error (_("this utility cannot be used for EFI platforms"
> +                         " because it does not support UEFI Secure Boot"));
> +      break;
> +    default:
> +      is_efi = 0;
> +      break;
> +    }
> +
>    {
>      char *platname = grub_install_get_platform_name (platform);
>      fprintf (stderr, _("Installing for %s platform.\n"), platname);
> @@ -1024,27 +1043,6 @@ main (int argc, char *argv[])
>    grub_hostfs_init ();
>    grub_host_init ();
>
> -  switch (platform)
> -    {
> -    case GRUB_INSTALL_PLATFORM_I386_EFI:
> -    case GRUB_INSTALL_PLATFORM_X86_64_EFI:
> -    case GRUB_INSTALL_PLATFORM_ARM_EFI:
> -    case GRUB_INSTALL_PLATFORM_ARM64_EFI:
> -    case GRUB_INSTALL_PLATFORM_LOONGARCH64_EFI:
> -    case GRUB_INSTALL_PLATFORM_RISCV32_EFI:
> -    case GRUB_INSTALL_PLATFORM_RISCV64_EFI:
> -    case GRUB_INSTALL_PLATFORM_IA64_EFI:
> -      is_efi = 1;
> -      break;
> -    default:
> -      is_efi = 0;
> -      break;
> -
> -      /* pacify warning.  */
> -    case GRUB_INSTALL_PLATFORM_MAX:
> -      break;
> -    }
> -
>    switch (platform)
>      {
>      case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
> @@ -1060,7 +1058,6 @@ main (int argc, char *argv[])
>      }
>
>    /* Find the EFI System Partition.  */
> -
>    if (is_efi)
>      {
>        grub_fs_t fs;
> --
> 2.46.2

This patch should not be upstreamed as-is, since it completely breaks
the ability to generate grub-efi binaries on the system with the
assumption that pre-existing ones have been shipped by a distributor.

The rework mentioned in the commit message body needs to happen here,
because it's unacceptable to just completely break stuff.



-- 
真実はいつも一つ！/ Always, there's only one truth!