Re: [PATCH v1 14/15] grub-install: install on EFI if forced

[Didier Spaier]

On 11/4/24 02:08, Neal Gompa wrote:
> On Thu, Oct 31, 2024 at 3:43 PM Leo Sandoval <lsandova@redhat.com> wrote:
>>
>> From: Marta Lewandowska <mlewando@redhat.com>
>>
>> UEFI Secure Boot requires signed grub binaries to work, so grub-
>> install should not be used. However, users who have Secure Boot
>> disabled and wish to use the command should not be prevented from
>> doing so if they invoke --force.
>>
>> fixes bz#1917213 / bz#2240994
>>
>> Signed-off-by: Marta Lewandowska <mlewando@redhat.com>
>> ---
>>  util/grub-install.c | 38 +++++++++++++++++++-------------------
>>  1 file changed, 19 insertions(+), 19 deletions(-)
>>
>> diff --git a/util/grub-install.c b/util/grub-install.c
>> index ee61b042b..b924c2d34 100644
>> --- a/util/grub-install.c
>> +++ b/util/grub-install.c
>> @@ -910,25 +910,6 @@ 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);
>> @@ -1045,6 +1026,22 @@ main (int argc, char *argv[])
>>
>>    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;
>> +      if (!force)
>> +        grub_util_error (_("This utility should not be used for EFI 
>> platforms"
>> +                          " because it does not support UEFI Secure Boot."
>> +                          " If you really wish to proceed, invoke the 
>> --force"
>> +                          " option.\nMake sure Secure Boot is disabled 
>> before"
>> +                          " proceeding"));
>> +      break;
>>      case GRUB_INSTALL_PLATFORM_I386_IEEE1275:
>>      case GRUB_INSTALL_PLATFORM_POWERPC_IEEE1275:
>>  #ifdef __linux__
>> @@ -1053,6 +1050,9 @@ main (int argc, char *argv[])
>>          try_open ("/dev/nvram");
>>  #endif
>>        break;
>> +      /* pacify warning.  */
>> +    case GRUB_INSTALL_PLATFORM_MAX:
>> +      break;
>>      default:
>>        break;
>>      }
>> --
>> 2.46.2
>>
> 
> This should be merged with the patch that breaks grub-install for EFI
> and re-sent as one *new* patch. It's not okay to break it and then fix
> it in the same patch series, since we don't want broken functionality
> in a commit applied to git.

In addition, making mandatory the option --force when Secure Boot is not enabled
will need many changes of grub installation handling by many distributions,
admins and end users, and lead to a lot of systems unable to boot until done.

Why not instead, in case of Linux systems, check the Secure Boot status of the
firmware, for instance querying the NVRAM variable exposed as:
/sys/efi/efivars/SecureBoot-8be4df61-93ca-11d2-aa0d-00e098032b8c
and only if enabled, when running grub-install, display an error or warning?

Didier