Re: [PATCH] efi: Initialize canary to non-zero value

[Dimitri John Ledkov]

On Tue, 14 Nov 2023, 03:19 Glenn Washburn, 
wrote:

> On Mon, 13 Nov 2023 17:18:50 +0100
> Daniel Kiper  wrote:
>
> > On Sun, Nov 12, 2023 at 08:22:42AM +0100, Heinrich Schuchardt wrote:
> > > On 11/12/23 04:23, Glenn Washburn wrote:
> > > > The canary, __stack_chk_guard, is in the BSS and so will get
> initialized to
> > > > zero if it is not explicitly initialized. If the UEFI firmware does
> not
> > > > support the RNG protocol, then the canary will not be randomized and
> will
> > > > be used as zero. This seems like a possibly easier value to write by
> an
> > > > attacker. Initialize canary to static random bytes, so that it is
> still
> > > > random when there is not RNG protocol.
>
> Ugh. s/not/no/
>
> > > >
> > > > Signed-off-by: Glenn Washburn 
> > >
> > > Reviewed-by: Heinrich Schuchardt 
> >
> > Reviewed-by: Daniel Kiper 
> >
> > > > ---
> > > >   grub-core/kern/efi/init.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/grub-core/kern/efi/init.c b/grub-core/kern/efi/init.c
> > > > index 0e28bea17a76..b85d98ca47fd 100644
> > > > --- a/grub-core/kern/efi/init.c
> > > > +++ b/grub-core/kern/efi/init.c
> > > > @@ -41,7 +41,7 @@ static grub_guid_t rng_protocol_guid =
> GRUB_EFI_RNG_PROTOCOL_GUID;
> > > >
> > > >   static grub_efi_uint8_t stack_chk_guard_buf[32];
> > > >
> > > > -grub_addr_t __stack_chk_guard;
> > > > +grub_addr_t __stack_chk_guard = (grub_addr_t) 0x92f2b7e2f193b25c;
>
> Just now reflecting on this I had the idea that perhaps even better is
> to have this randomly generated at build time and inserted as an
> autoconf variable. Then every build would have a different canary.
> I think I'll send a patch for this soon.
>


But please insure it is reproducible (as in reproducible builds initiative).

But also, one already has time and date macros that are unique and
reproducible in most distros. Can you use TIME macro as either the random
value or the seed to make a suitable random number?

https://reproducible-builds.org/docs/source-date-epoch/ and all the ways it
is already available at cpp time.



> >
> > I would add last sentence from the commit message before this line.
> > I can do it for you before push...
>
> Sure. And please remove the 't' mentioned above in the commit message.
>
> Glenn
>
> >
> > > >
> > > >   void __attribute__ ((noreturn))
> > > >   __stack_chk_fail (void)
> >
> > Daniel
>
> ___
> 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

Re: x86: Apply microcode updates in GRUB?

[Dimitri John Ledkov]

On Thu, 24 Aug 2023 at 15:54, Paul Menzel  wrote:
>
> Dear Dimitri,
>
>
> Thank you for your answer.
>
> Am 08.08.23 um 17:25 schrieb Dimitri John Ledkov:
> > On Sat, 29 Jul 2023 at 06:54, Paul Menzel wrote:
>
> >> On x86 microcode updates often are recommended to be applied to fix
> >> bugs. Just recently new microcode updates where published for AMD Zen 2
> >> processors to fix “Zenbleed” [1].
> >>
> >> Currently, these updates are shipped and applied by the firmware, and –
> >> mainly due to the proprietary and closed source x86 firmware ecosystem
> >> is slow to ship these updates and firmware updates are cumbersome to
> >> apply in this ecosystem – the operating systems like the Linux kernel
> >> [2] (but I believe also Microsoft Windows) also support to apply these
> >> updates.
> >>
> >> Should boot loaders be able to apply these updates, so these can be
> >> applied on systems for operating systems without such a feature?
> >
> > Most distributions already do this via early-initrd. For example, on
> > all ubuntu systems if you unpack initramfs with `unmkinitramfs`
> > command you will notice early1 and early2 uncompressed initrd portions
> > that contain Intel and AMD microcode. these are loaded and applied by
> > kernel early on, before unpacking the rest of the initrd or
> > initialising the system.
> >
> > Specifically for Zenbleed, Ubuntu Security team has shipped
> > amd64-microcode package at CRD time, which is automatically installed
> > by all systems and thus a reboot has already applied those.
>
> Does CRD mean Cargo Ready Date?


Coordinated Release Date of security vulnerability details,
reproducers, fixes, patches, research paper and binary builds for
stable distributions to release things
https://www.kernel.org/doc/html/v4.18/admin-guide/security-bugs.html#coordination

>
>
> Thank you for the explanation. I was well aware of that. It is very
> GNU/Linux focused though, and GRUB also loads other programs. If the
> boot loader would do that, not all operating systems would need to
> implement that. Anyway, it’s present in the Linux kernel, so – as a
> Linux user – I can still use that.
>
> > This is a standard mechanism that is already implemented by all
> > distributions (i.e. Ubuntu, Ubuntu Core, Fedora, etc). If your
> > distribution/device doesn't install and doesn't generate early initrd,
> > please implement that. Reference implementations are available in
> > initramfs-tools (debian/ubuntu), core-initrd (ubuntu core), dracut,
> > and likely many others.
>
> Back then, reading up on the choices, I do not like this, as you need
> the wrappers `lsinitramfs` and `unmkinitramfs` to easily deal with these
> files. For our distribution MarIuX we went the separate files for the
> microcode update archives, and let GRUB load them and pass to the Linux
> kernel [3].


You don't need to patch grub. And you can build things stand alone already.

For example, on Ubuntu instead of having a fat initrd that contains
early-initrd components and main ones, you can do them as separate
files already. as supported by stock grub

```
linux /boot/vmlinuz
initrd /boot/microcode-intel.cpio /boot/microcode-amd.cpio /boot/initrd.img
```

Is already supported by grub and will cause to load all of these
separate cpio archives separately, which the linux kernel correctly
apply microcode from.

For a working implementation you can see src:microcode-initrd package
in Ubuntu and Debian that does that.

> > It is a nice property to bundle this in the initrd, as sometimes there
> > are microcode regressions, thus booting old kernel abi, with an old
> > initrd, with an old microcode is desirable.
>
> It’s a two-edged sword, as people do not know what microcode updates are
> going to be applied using older boot entries.
>


As a distribution integrator, one can make appropraite choices and
documentation as needed. In ubuntu we provide alternative boot entries
that include booting without microcode update being applied as a
debugging and recovery fallback boot entry. Because sometimes
microcode updates crash the OS and we provide that failsafe to the end
user to discover in the grub menu.


>
>
> Kind regards,
>
> Paul
>
>
> >> [1]: https://lock.cmpxchg8b.com/zenbleed.html
> >> [2]: https://docs.kernel.org/arch/x86/microcode.html[3]: 
> >> https://github.molgen.mpg.de/mariux64/mxtools/pull/342
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] loader/efi/linux: Implement x86 mixed mode using legacy boot

[Dimitri John Ledkov]

#include ascii art of can of worms here

On Tue, 8 Aug 2023 at 17:27, Ard Biesheuvel  wrote:
>
> On Tue, 8 Aug 2023 at 17:34, Dimitri John Ledkov
>  wrote:
> >
> > On Mon, 7 Aug 2023, 13:23 Ard Biesheuvel,  wrote:
> > >
> > > Recent mixed-mode Linux kernels (i.e., v4.0 or newer) can access EFI
> > > runtime services at OS runtime even when the OS was not entered via the
> > > EFI stub. This is because, instead of reverting back to the firmware's
> > > segment selectors, GDTs and IDTs, the 64-bit kernel simply calls 32-bit
> > > runtime services using compatilibity mode (i.e., the same mode used for
> > > 32-bit user space) without taking down all interrupt handling, exception
> > > handling etc.
> > >
> > > This means that GRUB's legacy x86 boot mode is sufficient to make use of
> > > this: 32-bit i686 builds of GRUB can already boot 64-bit kernels in EFI
> > > enlightened mode (but without going via the EFI stub), and provide all
> > > the metadata that the OS needs to map the EFI runtime regions and call
> > > EFI runtime services successfully.
> >
> > I'm not sure I understand the target combination here, and I'm not
> > sure we want to support it.
> >
>
> The point is not what we /want/ to support - it is what we already
> supported in the past, and the recent EFI changes result in
> regressions for those scenarios.
>
> > For now, we keep shipping both 32bit and 64bit x86 revocations
> > together. However, the time will come that we will want or need to
> > drop the 32bit revocations. Ubuntu has never signed a 32bit build of
> > grub for secureboot, nor have we ever signed 32bit kernels to boot
> > either. Most distributions have also stopped 32bit x86 builds, and/or
> > don't support them to the same feature level (there are some that
> > enable lots of combinations, i.e. debian, but even there i am not sure
> > such hardware exists).
> >
> > I see this as a bug that 32-bit i686 build of Grub can launch 64-bit
> > kernels - potentially without using shim / shim-lock / sbat
> > verification.
> >
>
> A 32-bit *EFI* build of GRUB, which you will never be able to boot on
> anything other than 32-bit EFI, i.e., systems that nobody cares about
> wrt secure boot.
>
> > I would feel much better if under shim-lock/secureboot only matching
> > bitness was allowed to be booted and supported. Meaning no mixing &
> > matching i686 grub builds launching x86_64 kernels, or vice versa, and
> > ditoo on any other arches too (i.e. no mixing between
> > riscv32/riscv64/riscv128, nor armhf/arm64).
> >
>
> AFAICT, the non-EFIstub x86 boot path in GRUB has always supported
> booting kernels of any bitness (provided, of course, that the CPU is
> long mode capable)
>
> With adding the EFI support, we broke this which may potentially harm users.
>
> > I am impressed it is technologically possible, but do we really want
> > to implement/design/support/encourage that?
> >
>
> Distros already do, that is the whole point. It is called mixed mode,
> and it is something I am trying to get rid of.
>
> The current mixed-mode implementation has elaborate plumbing so that
> the 64-bit EFI stub can make use of 32-bit boot services. This was
> presented to me in the past as a prerequisite for being able to
> support mixed mode (and therefore a justification for upstreaming the
> EFI handover protocol).

I believe the only platform that shipped that out of the box, most
recently was https://www.minnowboard.org/
It had an alternative 64-bit boot service firmware download file,
which most people did flash and use, but out of the factory it was
shipped in 32-bit boot services mode, and yet expected to
boot/load/run 64-bit kernels.

Conveniently it wasn't too popular, was taken off the market
https://www.silicom-usa.com/wp-content/uploads/2020/07/EOL_MinnowBoard-Turbot.pdf
on 27th of July 2020, with all support & warranty for it seizing two
weeks ago. Thus I can see why there was a strong push in the past to
keep this commercial failure still working, as it was still shipping.

> However, I realized that the current mixed
> mode support in Linux does not actually rely on this: the only thing
> you need is a bootloader that implements the x86 Linux boot protocol
> directly but also passes EFI memory map, system table, etc. GRUB
> already did this. IOW, mainline i386-efi GRUB already supported mixed
> mode, and the recent EFI changes broke that.
>
> > I'm not even sure if 32-bit x86 EFI grub platform even makes sense
> > anymore, and maybe it should be ripped out altogether? and leave i686
> 

Re: [PATCH] loader/efi/linux: Implement x86 mixed mode using legacy boot

[Dimitri John Ledkov]

On Mon, 7 Aug 2023, 13:23 Ard Biesheuvel,  wrote:
>
> Recent mixed-mode Linux kernels (i.e., v4.0 or newer) can access EFI
> runtime services at OS runtime even when the OS was not entered via the
> EFI stub. This is because, instead of reverting back to the firmware's
> segment selectors, GDTs and IDTs, the 64-bit kernel simply calls 32-bit
> runtime services using compatilibity mode (i.e., the same mode used for
> 32-bit user space) without taking down all interrupt handling, exception
> handling etc.
>
> This means that GRUB's legacy x86 boot mode is sufficient to make use of
> this: 32-bit i686 builds of GRUB can already boot 64-bit kernels in EFI
> enlightened mode (but without going via the EFI stub), and provide all
> the metadata that the OS needs to map the EFI runtime regions and call
> EFI runtime services successfully.

I'm not sure I understand the target combination here, and I'm not
sure we want to support it.

For now, we keep shipping both 32bit and 64bit x86 revocations
together. However, the time will come that we will want or need to
drop the 32bit revocations. Ubuntu has never signed a 32bit build of
grub for secureboot, nor have we ever signed 32bit kernels to boot
either. Most distributions have also stopped 32bit x86 builds, and/or
don't support them to the same feature level (there are some that
enable lots of combinations, i.e. debian, but even there i am not sure
such hardware exists).

I see this as a bug that 32-bit i686 build of Grub can launch 64-bit
kernels - potentially without using shim / shim-lock / sbat
verification.

I would feel much better if under shim-lock/secureboot only matching
bitness was allowed to be booted and supported. Meaning no mixing &
matching i686 grub builds launching x86_64 kernels, or vice versa, and
ditoo on any other arches too (i.e. no mixing between
riscv32/riscv64/riscv128, nor armhf/arm64).

I am impressed it is technologically possible, but do we really want
to implement/design/support/encourage that?

I'm not even sure if 32-bit x86 EFI grub platform even makes sense
anymore, and maybe it should be ripped out altogether? and leave i686
boot to pc-bios only. i686 EFI was a short lived experiment that died
without ever taking off.

>
> It does mean that GRUB should not attempt to invoke the firmware's
> LoadImage/StartImage methods on kernel builds that it knows cannot be
> started natively. So add a check for this in the native EFI boot path,
> and fall back to legacy x86 mode in such cases.
>
> Note that in the general case, booting non-native images of the same
> native word size (e.g., X64 EFI apps on arm64 firmware) might be
> supported by means of emulation, so let's only disallow images that use
> a non-native word size. This will also permit booting i686 kernels on
> x86_64 builds, although without access to runtime services, as this is
> not supported by Linux.
>
> This change on top of 2.12-rc1 is sufficient to boot ordinary Linux
> mixed mode builds and get full access to the EFI runtime services.
>
> Cc: Daniel Kiper 
> Cc: Steve McIntyre 
> Cc: Julian Andres Klode 
> Signed-off-by: Ard Biesheuvel 
> ---
>  grub-core/loader/efi/linux.c | 5 +
>  include/grub/efi/pe32.h  | 6 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c
> index ed325f2b0aae2d6f..1d0734e295043df7 100644
> --- a/grub-core/loader/efi/linux.c
> +++ b/grub-core/loader/efi/linux.c
> @@ -120,6 +120,11 @@ grub_arch_efi_linux_load_image_header (grub_file_t file,
>  return grub_error (GRUB_ERR_FILE_READ_ERROR, "failed to read COFF 
> image header");
>  }
>
> +  if (lh->pe_image_header.optional_header.magic != GRUB_PE32_NATIVE_MAGIC)
> +{
> +  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "non-native image not 
> supported");
> +}
> +
>/*
> * Linux kernels built for any architecture are guaranteed to support the
> * LoadFile2 based initrd loading protocol if the image version is >= 1.
> diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h
> index 101859af1ea64237..4e6e9d254bd35c9b 100644
> --- a/include/grub/efi/pe32.h
> +++ b/include/grub/efi/pe32.h
> @@ -267,6 +267,12 @@ struct grub_pe32_section_table
>
>  #define GRUB_PE32_SIGNATURE_SIZE 4
>
> +#if GRUB_TARGET_SIZEOF_VOID_P == 8
> +#define GRUB_PE32_NATIVE_MAGIC GRUB_PE32_PE64_MAGIC
> +#else
> +#define GRUB_PE32_NATIVE_MAGIC GRUB_PE32_PE32_MAGIC
> +#endif
> +
>  struct grub_pe_image_header
>  {
>/* This is always PE\0\0.  */
> --
> 2.39.2
>
>
> ___
> 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

Re: x86: Apply microcode updates in GRUB?

[Dimitri John Ledkov]

On Sat, 29 Jul 2023 at 06:54, Paul Menzel  wrote:
>
> Dear GRUB folks,
>
>
> On x86 microcode updates often are recommended to be applied to fix
> bugs. Just recently new microcode updates where published for AMD Zen 2
> processors to fix “Zenbleed” [1].
>
> Currently, these updates are shipped and applied by the firmware, and –
> mainly due to the proprietary and closed source x86 firmware ecosystem
> is slow to ship these updates and firmware updates are cumbersome to
> apply in this ecosystem – the operating systems like the Linux kernel
> [2] (but I believe also Microsoft Windows) also support to apply these
> updates.
>
> Should boot loaders be able to apply these updates, so these can be
> applied on systems for operating systems without such a feature?
>

Most distributions already do this via early-initrd. For example, on
all ubuntu systems if you unpack initramfs with `unmkinitramfs`
command you will notice early1 and early2 uncompressed initrd portions
that contain Intel and AMD microcode. these are loaded and applied by
kernel early on, before unpacking the rest of the initrd or
initialising the system.

Specifically for Zenbleed, Ubuntu Security team has shipped
amd64-microcode package at CRD time, which is automatically installed
by all systems and thus a reboot has already applied those.

This is a standard mechanism that is already implemented by all
distributions (i.e. Ubuntu, Ubuntu Core, Fedora, etc). If your
distribution/device doesn't install and doesn't generate early initrd,
please implement that. Reference implementations are available in
initramfs-tools (debian/ubuntu), core-initrd (ubuntu core), dracut,
and likely many others.

It is a nice property to bundle this in the initrd, as sometimes there
are microcode regressions, thus booting old kernel abi, with an old
initrd, with an old microcode is desirable.

>
> Kind regards,
>
> Paul
>
>
> [1]: https://lock.cmpxchg8b.com/zenbleed.html
> [2]: https://docs.kernel.org/arch/x86/microcode.html
>

-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/4] EFI envblk

[Dimitri John Ledkov]

On Wed, 26 Jul 2023, 23:03 Vladimir 'phcoder' Serbinenko, 
wrote:

> Have you considered that some firmwares brick if EFI storage is over 50%
> full? Why not having a file on ESP instead?
>

Yes bricking did happen before. It was triggered by excessive runtime
updates of bootloader settings. It did surface bugs of reclaiming variable
storage which got fixed and released by most manufacturers. Due to recent
revocations EFI storage is now routinely excercises meaning it is now
reliable to use. EFI storage is used by kernel pstore too. Unlike read-only
ESP EFI storage can be ephemeral r/w through the lifecycle of VM across
reboots without a shutdown. Which is better than ESP.



> Le jeu. 27 juil. 2023, 00:09, Glenn Washburn 
> a écrit :
>
>> This patch series (for me) was motivated by the "gdb: Add more support for
>> debugging on EFI platforms" patch series, which allowed printing in early
>> EFI init of the GDB command for properly loading symbols. That approach of
>> having the functionality be included at compile time was ultimately
>> rejected. During the discussion of that series, Robbie suggested[1] using
>> patches by Peter and in the Redhat downstream repo which uses an EFI
>> variable to store a GRUB env block. Using this, a user could store a
>> variable in the env block stored in the EFI variable and then have GRUB
>> load that env block in early init as a way to enable the printing of the
>> GDB command.
>>
>> I've taken the original patches by Peter, made more suitable for including
>> in GRUB, fixed some bugs, and added a minor feature. The first patch, adds
>> env block loading in early EFI init from the GRUB_ENV EFI variable. The
>> second patch is included to provide tools for GRUB to set this EFI env
>> block itself, as opposed to needing to use the method suggested by
>> Robbie[1], which requires GNU/Linux and the user-space grub2-editenv
>> utility. Of course, if GRUB is crashing before one can run the
>> efi-export-env command, then other ways of creating and setting the EFI
>> envblk variable are necessary. The third patch adds a test which checks
>> the
>> usability of the commands and the early init loading. And the last patch,
>> whichvmotivated this series, prints the GDB command string if the GRUB
>> variable named "enable_earlyinit_gdbinfo" is present and is set to "1",
>> after
>> the env block is loaded from the GRUB_ENV EFI variable. We might want a
>> different name for the GRUB variable, I don't really have a preference.
>>
>> Glenn
>>
>> [1] https://mail.gnu.org/archive/html/grub-devel/2023-03/msg00072.html
>>
>> Glenn Washburn (2):
>>   tests: Add efienv_test for testing efi-export-env and efi-load-env
>>   efi: Print GDB command for loading symbols if variable exists
>>
>> Peter Jones (2):
>>   efi: Load env block from GRUB_ENV EFI variable in early init
>>   efi: Add efi-export-env and efi-load-env commands
>>
>>  Makefile.util.def|   6 ++
>>  docs/grub.texi   |  24 +
>>  grub-core/Makefile.core.def  |   7 ++
>>  grub-core/commands/efi/env.c | 182 +++
>>  grub-core/kern/efi/efi.c |   3 +
>>  grub-core/kern/efi/init.c|  37 +++
>>  grub-core/lib/envblk.c   |  43 +
>>  include/grub/efi/efi.h   |   5 +
>>  include/grub/lib/envblk.h|   3 +
>>  tests/efienv_test.in |  57 +++
>>  10 files changed, 367 insertions(+)
>>  create mode 100644 grub-core/commands/efi/env.c
>>  create mode 100644 tests/efienv_test.in
>>
>> --
>> 2.34.1
>>
>>
>> ___
>> 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
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Dimitri John Ledkov]

On Fri, 23 Jun 2023 at 15:12, Ard Biesheuvel  wrote:
>
> On Fri, 23 Jun 2023 at 15:46, Tobias Powalowski
>  wrote:
> >
> >
> >
> > Am Fr., 23. Juni 2023 um 15:41 Uhr schrieb Ard Biesheuvel :
> >>
> >> On Thu, 22 Jun 2023 at 11:41, Tobias Powalowski
> >>  wrote:
> >> >
> >> > Hi tackled it down to this commit:
> >> > https://git.savannah.gnu.org/cgit/grub.git/commit/?id=6a080b9cde0be5d08b71daf17a806067e32fc13f
> >> > starting with this commit shim verification fails for kernel hash with 
> >> > bad shim verification and makes Secure Boot impossible.
> >>
> >> Could you elaborate on your setup? How are you signing and
> >> authenticating the kernel image?
> >>
> >> GRUB calls LoadImage/StartImage, and these calls will be intercepted
> >> by shim to implement its own authentication. The expectation here is
> >> that the kernel's PE image is signed with a MOK key.
> >
> > Hi,
> > here is  how it worked before:
> > Add the kernel and grub hashes through MOK manager. After adding those grub 
> > was able to boot the hashed kernel.
> > On the installation medium I cannot expect someone already has a MOK 
> > certificate, though hashes needs to be used in first place.
>
> Apparently, SHIM hooks LoadImage and StartImage, but does not actually
> perform any verification against the MOK db in this case.

To be fair it should fail, as kernels typically do not have an SBAT section.

>
> This probably implies that upstream GRUB in its current state (without
> the EFI stub boot patches that the distros are carrying) should
> fallback to 'legacy' x86 EFI boot when secure boot is enabled and the
> shim lock verifier is active.

Which likely will become unusable going forward on x86 as well, when
users and kernels rely on functionality only available via
loadimage/startimage.

-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: Bad shim signature on kernel loading with patchset from 25.05.2023 and up

[Dimitri John Ledkov]

Hi,

On Fri, 23 Jun 2023 at 14:46, Tobias Powalowski via Grub-devel
 wrote:
>
>
>
> Am Fr., 23. Juni 2023 um 15:41 Uhr schrieb Ard Biesheuvel :
>>
>> On Thu, 22 Jun 2023 at 11:41, Tobias Powalowski
>>  wrote:
>> >
>> > Hi tackled it down to this commit:
>> > https://git.savannah.gnu.org/cgit/grub.git/commit/?id=6a080b9cde0be5d08b71daf17a806067e32fc13f
>> > starting with this commit shim verification fails for kernel hash with bad 
>> > shim verification and makes Secure Boot impossible.
>>
>> Could you elaborate on your setup? How are you signing and
>> authenticating the kernel image?
>>
>> GRUB calls LoadImage/StartImage, and these calls will be intercepted
>> by shim to implement its own authentication. The expectation here is
>> that the kernel's PE image is signed with a MOK key.
>
> Hi,
> here is  how it worked before:
> Add the kernel and grub hashes through MOK manager. After adding those grub 
> was able to boot the hashed kernel.
> On the installation medium I cannot expect someone already has a MOK 
> certificate, though hashes needs to be used in first place.
> greetings
> tpowa

Is the kernel image padded? is it unsigned? is it signed?

If it is not signed, are you able to add any secureboot signature to
it (i.e. sbsign it with a snakeoil cert), then compute the
authenticode hash of it & enroll that hash into MOK and then does it
become bootable?

Note that authenticode of unpadded/unsigned EFI applications is
unfortunately under defined and often buggy.

-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [RFC PATCH 4/4] kern/efi/sb: Use shim to verify font files

[Dimitri John Ledkov]

Yes yes yes yes. Signed dtb in grub at last.

On Wed, 7 Dec 2022, 03:16 Michael Chang via Grub-devel, 
wrote:

> On Tue, Dec 06, 2022 at 11:09:57AM -0500, Robbie Harwood wrote:
> > Zhang Boyang  writes:
> >
> > > Since font files can be wrapped as PE images by grub-wrap, use shim to
> > > verify font files if Secure Boot is enabled. To prevent other PE files
> > > (e.g. kernel images) used as wrappers, it only allows files marked as
> > > Windows GUI used as wrappers.
> >
> > Thanks for writing this; it's helpful to have something concrete to look
> > at.
> >
> > This approach is very font-focused, and while I understand that given
> > the discussion, I do still wonder if it wouldn't be better to make fonts
> > an instance of modules.  If fonts become instances of modules, and
> > modules are wrapped into PE files, that not only seems cleaner but also
> > gives us signed module support without baking those into the image.
>
> Why not just making the PE wrap applicable to all file types, be it font
> files, grub modules or even (static) initrd. Providing a solution to
> sign arbitrary data or binary with this PE envelope sounds to me a very
> attractive feature and worthwhile the extra miles. :)
>
> Thanks,
> Michael
>
> >
> > What do you think?
> >
> > Be well,
> > --Robbie
>
>
>
> ___
> 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

[PATCH] util/grub.d/30_uefi-firmware.in: Re-arrange if conditions

[Dimitri John Ledkov]

Only perform call to fwsetup if one is on EFI platform. On all other
platorms fwsetup command does not exists, and thus returns 0 and a
useless uefi-firmware menuentry gets generated.

Signed-off-by: Dimitri John Ledkov 
---
 util/grub.d/30_uefi-firmware.in | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/30_uefi-firmware.in
index c1731e5bbb..1c2365ddbd 100644
--- a/util/grub.d/30_uefi-firmware.in
+++ b/util/grub.d/30_uefi-firmware.in
@@ -31,10 +31,12 @@ LABEL="UEFI Firmware Settings"
 gettext_printf "Adding boot menu entry for UEFI Firmware Settings ...\n" >&2
 
 cat << EOF
-fwsetup --is-supported
-if [ "\$grub_platform" = "efi" -a "\$?" = 0 ]; then
-   menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
-   fwsetup
-   }
+if [ "\$grub_platform" = "efi" ]; then
+   fwsetup --is-supported
+   if [ "\$?" = 0 ]; then
+   menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
+   fwsetup
+   }
+   fi
 fi
 EOF
-- 
2.34.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Regression] efi: Don't display a uefi-firmware entry if it's not supported

[Dimitri John Ledkov]

On Tue, 30 Aug 2022 at 21:22, Robbie Harwood  wrote:
>
> Philip Müller  writes:
>
> >> Hello Robbie, hello Daniel,
> >>
> >> with the commit 26031d3b101648352e4e427f04bf69d320088e77
> >> 30_uefi-firmware will always call `fwsetup --is-supported' to check
> >> if the system supports EFI or not. However most installed grub
> >> versions on MBR don't support the '--is-supported' flag and crash the
> >> menu creation routine.
> >>
> >> Arch Linux is tracking the issue here: 
> >> https://bugs.archlinux.org/task/75701
> >>
> >> What would be the best approach with older installations of grub via
> >> grub-install not having to reinstall grub to MBR as some users don't
> >> even know on how to install grub properly as that job a graphical
> >> installer does.
> >
> > Hi all,
>
> Adding grub-devel to CC.
>
> > I looked into the '30_uefi-firmware' changes a little more and also
> > commented my findings at the Arch-Bugtracker:
> > https://bugs.archlinux.org/task/75701#comment210684
> >
> > Before Menu changes we simply had this:
> >
> > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> >fwsetup
> > }
> > ### END /etc/grub.d/30_uefi-firmware ###
> >
> > After Menu changes we went to that:
> >
> > ### BEGIN /etc/grub.d/30_uefi-firmware ###
> > fwsetup --is-supported
> > if [ "$grub_platform" = "efi" -a "$?" = 0 ]; then
> >menuentry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
> >  fwsetup
> >}
> > fi

above code looks buggy to me. As far as I understand fwsetup only
works on EFI grub_platform, and thus originally the menu entry and
fwsetup call was scoped under efi platform. thus there should be two
if conditions, not one:

if [ "$grub_platform" = "efi" ]; then
fwsetup --is-supported
if [ "$?" = 0 ]; then
menuetry 'UEFI Firmware Settings' $menuentry_id_option 'uefi-firmware' {
fwsetup
}
fi
fi

This way the code continues to work correctly on bios platforms
(skipped completely), and on EFI like platforms fwsetup menu entry is
only shown if fwsetup --is-supported executes successfully.

the optimisation of having two conditionals evaluated together, whilst
efficient, is wrong given cross grub platform compatibilities desires.

This fwsetup has never before been executed on bios platform, and we
should not be introducing this.

> > ### END /etc/grub.d/30_uefi-firmware ###
> >
> > So 0eb684e8bfb0a9d2d42017a354740be25947babe I get and
> > 26031d3b101648352e4e427f04bf69d320088e77 tries to improve things,
> > however doesn't count in what will happen if 'fwsetup' is not supporting
> > the flag. It may either crash due to
> > 1e79bbfbda24a08cb856ff30f8b1bec460779b91 or start UEFI firmware instead.
>
> Why doesn't grub on the MBR get updated when you install a new grub
> package?  That seems like the real issue here - what am I missing?
>
> Regardless, if we can't count on fwsetup being updated, then I think we
> need to go back to the original version of my patch which doesn't have
> --is-supported.
>
> > Simply don't break user-space when older grub got installed to MBR.
> > Somehow this idea needs some more thinking and a solution for that
> > case too.
>
> Sure, what do you suggest?
>
> Be well,
> --Robbie
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 1/2] efi/chainloader: Do not require a valid $root when chainloading

[Dimitri John Ledkov]

Hi,

This is interesting. I had to work around this same issue in loopback
to allow chainloading from loopback devices see
https://github.com/rhboot/grub2/commit/0e5cb733f3cb227293ea58397ea10891519095f0



On Fri, 26 Aug 2022 at 05:34, Glenn Washburn
 wrote:
>
> The EFI chainloader checks that a device path can be created for the $root
> device before allowing chainloading to a given file. This is probably to
> ensure that the given file can be accessed and loaded by the firmware.
> However, since GRUB is loading the image itself, the firmware need not
> be able to access the file location of the image. So remove this check.
>
> Also, this fixes an issue where chainloading an image file on a location
> that is accessible by the firmware, eg. (hd0,1)/efi/boot.efi, would
> fail when root is a location inaccessible by the firmware, eg. memdisk.
>
> Use GRUB_EFI_BYTES_TO_PAGES() instead of donig the calculation explicitly.
>
> Add comment noting the section where the load options for the chainloaded
> EFI application is constructed.
>
> Signed-off-by: Glenn Washburn 
> ---
>  grub-core/loader/efi/chainloader.c | 31 +++---
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/grub-core/loader/efi/chainloader.c 
> b/grub-core/loader/efi/chainloader.c
> index 7557eb269b..5aac3c59fd 100644
> --- a/grub-core/loader/efi/chainloader.c
> +++ b/grub-core/loader/efi/chainloader.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -239,11 +240,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>if (! file)
>  goto fail;
>
> -  /* Get the root device's device path.  */
> -  dev = grub_device_open (0);
> -  if (! dev)
> -goto fail;
> -
> +  dev = file->device;
>if (dev->disk)
>  dev_handle = grub_efidisk_get_device_handle (dev->disk);
>else if (dev->net && dev->net->server)
> @@ -267,18 +264,15 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>if (dev_handle)
>  dp = grub_efi_get_device_path (dev_handle);
>
> -  if (! dp)
> +  if (dp != NULL)
>  {
> -  grub_error (GRUB_ERR_BAD_DEVICE, "not a valid root device");
> -  goto fail;
> -}
> -
> -  file_path = make_file_path (dp, filename);
> -  if (! file_path)
> -goto fail;
> +  file_path = make_file_path (dp, filename);
> +  if (file_path == NULL)
> +   goto fail;
>
> -  grub_printf ("file path: ");
> -  grub_efi_print_device_path (file_path);
> +  grub_printf ("file path: ");
> +  grub_efi_print_device_path (file_path);
> +}
>
>size = grub_file_size (file);
>if (!size)
> @@ -287,7 +281,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>   filename);
>goto fail;
>  }
> -  pages = (((grub_efi_uintn_t) size + ((1 << 12) - 1)) >> 12);
> +  pages = (grub_efi_uintn_t) GRUB_EFI_BYTES_TO_PAGES (size);
>
>status = efi_call_4 (b->allocate_pages, GRUB_EFI_ALLOCATE_ANY_PAGES,
>   GRUB_EFI_LOADER_CODE,
> @@ -370,6 +364,7 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>  }
>loaded_image->device_handle = dev_handle;
>
> +  /* Build load options with arguments from chainloader command line. */
>if (argc > 1)
>  {
>int i, len;
> @@ -400,7 +395,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>  }
>
>grub_file_close (file);
> -  grub_device_close (dev);
>
>/* We're finished with the source image buffer and file path now. */
>efi_call_2 (b->free_pages, address, pages);
> @@ -411,9 +405,6 @@ grub_cmd_chainloader (grub_command_t cmd __attribute__ 
> ((unused)),
>
>   fail:
>
> -  if (dev)
> -grub_device_close (dev);
> -
>if (file)
>  grub_file_close (file);
>
> --
> 2.34.1
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
okurrr,

Dimitri

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Remove HFS support

[Dimitri John Ledkov]

There is no need for that code on any signed grubs or upstream. Ports that
want to support this patch can have it conditionally compiled / enabled
only on that arch, but not other.

For example, in Ubuntu we already use separate builds for signed & unsigned
bootloaders. Or one may keep grub-2.06 as separate source package. It's not
like those old platforms need any new features in the bootloader ever again.

The issue of insecure code is for signed bootloaders. Because there is a
separate level of protection that prevents replacing arbitrary bootloaders
(whilst potentially allow downgrade/upgrade attacks). Thus a responsible
upstream should drop this code.

On Fri, 19 Aug 2022, 20:39 John Paul Adrian Glaubitz, <
glaub...@physik.fu-berlin.de> wrote:

> On 8/19/22 20:09, Steve McIntyre wrote:
> > On Fri, Aug 19, 2022 at 04:03:38PM +0200, John Paul Adrian Glaubitz
> wrote:
> >>> On Aug 19, 2022, at 3:59 PM, Daniel Kiper  wrote:
> >>>
> >>> If I do not hear any major objections in the following weeks I will
> >>> merge this patch or a variant of it in the second half of September.
> >>
> >> We’re still formatting our /boot partitions for Debian PowerPC for
> >> PowerMacs using HFS, so this change would be a breaking change for
> >> us.
> >>
> >> So, that would be a no from Debian’s side.
> >
> > Not so fast please, Adrian. At the risk of sounding harsh, non-release
> > old ports like powerpc *really* don't get to dictate things in Debian
> > terms.
>
> Add "Ports" to this.
>
> > As Daniel Axtens has been finding out, the HFS code is terrible in
> > terms of security. If you still need it for old/semi-dead machines,
> > maybe you should fork an older grub release and stay with that?
>
> I don't know what should be the deal with the security of a boot loader
> to be honest. If someone has access to your hardware so they can control
> your bootloader, you have much worse problems anyway.
>
> Forking is also a terrible idea as every forked package means having to
> track it manually.
>
> Adrian
>
> --
>   .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer
> `. `'   Physicist
>`-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
>
> ___
> 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

Re: Can't find a solution to a failed secure boot kernel loading

[Dimitri John Ledkov]

On Wed, 11 May 2022 at 11:14, Łukasz Piątkowski  wrote:
>
> That was it, I created a new without that EKU and everything works! Thank you 
> very much, this was not easy to find, unfortunately :( Esp. when some 
> official pages like here 
> https://ubuntu.com/blog/how-to-sign-things-for-secure-boot still list it as a 
> needed EKU.
>

Indeed in that blog post's section "What about kernels and
bootloaders?" is out of date, not only one needs DER cert, one needs a
cert without modules EKU set =/ I opened
https://github.com/canonical-web-and-design/ubuntu.com/issues/11595 to
see if it can be corrected.

https://wiki.ubuntu.com/UEFI/SecureBoot/KeyManagement/ImageSigning

Is the authoritative documentation (and sibling pages) that we do
maintain and keep accurate. They look correct to me.

> On Tue, May 10, 2022 at 4:59 PM Łukasz Piątkowski  wrote:
>>
>> Huh, I've never seen that before... thanks, I'm gonna give it a try and 
>> report back!
>>
>> On Tue, May 10, 2022 at 4:44 PM Dimitri John Ledkov 
>>  wrote:
>>>
>>> On Tue, 10 May 2022 at 15:07, Łukasz Piątkowski  wrote:
>>> >
>>> > What I'm trying to do is to sign a mainline kernel built by ubuntu 
>>> > (https://kernel.ubuntu.com/~kernel-ppa/mainline/) with my private key, 
>>> > that is already enrolled to MOK, and boot it with Secure Boot.
>>> >
>>> > > the MOK key as generated by Ubuntu/Debian tooling, creates a signing 
>>> > > certificate that self-limits itself to only support Kernel Module 
>>> > > signing.
>>> >
>>> > OK, that explains why the key in `/var/lib/shim-signed/mok` doesn't work. 
>>> > Still, I have created my own key as well (listed below for inspection, it 
>>> > has code signing extension), enrolled that key in MOK and signed the 
>>> > ubuntu mainline kernel (the kernel I'm trying to boot) with it. The 
>>> > result is exactly the same. I was using exactly the same procedure a few 
>>> > ubuntu editions back and it was definitely working. From what I learned 
>>> > so far, this might be related to the BootHole bug 
>>> > (https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-10713) that was 
>>> > fixed some time ago.
>>> >
>>> > My generated key is:
>>> >
>>> > root@T495:~/mok# openssl x509 -in MOK.pem -text -noout
>>> > Certificate:
>>> > Data:
>>> > Version: 3 (0x2)
>>> > Serial Number:
>>> > 42:61:86:b2:29:3d:ca:eb:98:87:ae:3d:74:95:c7:f2:63:8f:8a:3b
>>> > Signature Algorithm: sha256WithRSAEncryption
>>> > Issuer: C = PL, ST = Poznan, L = Poznan, O = none, CN = Secure 
>>> > Boot Signing, emailAddress = exam...@example.com
>>> > Validity
>>> > Not Before: Feb 18 19:28:16 2020 GMT
>>> > Not After : Jan 25 19:28:16 2120 GMT
>>> > Subject: C = PL, ST = Poznan, L = Poznan, O = none, CN = Secure 
>>> > Boot Signing, emailAddress = exam...@example.com
>>> > Subject Public Key Info:
>>> > Public Key Algorithm: rsaEncryption
>>> > Public-Key: (2048 bit)
>>> > Modulus: [cut]
>>> > Exponent: 65537 (0x10001)
>>> > X509v3 extensions:
>>> > X509v3 Subject Key Identifier:
>>> > 
>>> > EC:57:4E:BD:DC:1A:CF:B4:55:16:4A:CE:CB:E4:9E:44:5C:C4:63:F6
>>> > X509v3 Authority Key Identifier:
>>> > 
>>> > EC:57:4E:BD:DC:1A:CF:B4:55:16:4A:CE:CB:E4:9E:44:5C:C4:63:F6
>>> > X509v3 Basic Constraints: critical
>>> > CA:FALSE
>>> > X509v3 Extended Key Usage:
>>> > Code Signing, 1.3.6.1.4.1.311.10.3.6, 
>>> > 1.3.6.1.4.1.2312.16.1.2
>>>
>>> This is bad... certs that have 1.3.6.1.4.1.2312.16.1.2 cannot be used
>>> to sign kernels.
>>>
>>> Your cert must _not_ have 1.3.6.1.4.1.2312.16.1.2 EKU set on it.
>>>
>>> You cannot use the same certificate to sign both kernel and modules.
>>>
>>> > Netscape Comment:
>>> > OpenSSL Generated Certificate
>>> > Signature Algorithm: sha256WithRSAEncryption
>>> > Signature Value: [cut]
>>> >
>>> > On Tue, May 10, 2022 at 3:26 PM Dimitri J

Re: Can't find a solution to a failed secure boot kernel loading

[Dimitri John Ledkov]

On Tue, 10 May 2022 at 15:07, Łukasz Piątkowski  wrote:
>
> What I'm trying to do is to sign a mainline kernel built by ubuntu 
> (https://kernel.ubuntu.com/~kernel-ppa/mainline/) with my private key, that 
> is already enrolled to MOK, and boot it with Secure Boot.
>
> > the MOK key as generated by Ubuntu/Debian tooling, creates a signing 
> > certificate that self-limits itself to only support Kernel Module signing.
>
> OK, that explains why the key in `/var/lib/shim-signed/mok` doesn't work. 
> Still, I have created my own key as well (listed below for inspection, it has 
> code signing extension), enrolled that key in MOK and signed the ubuntu 
> mainline kernel (the kernel I'm trying to boot) with it. The result is 
> exactly the same. I was using exactly the same procedure a few ubuntu 
> editions back and it was definitely working. From what I learned so far, this 
> might be related to the BootHole bug 
> (https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-10713) that was 
> fixed some time ago.
>
> My generated key is:
>
> root@T495:~/mok# openssl x509 -in MOK.pem -text -noout
> Certificate:
> Data:
> Version: 3 (0x2)
> Serial Number:
> 42:61:86:b2:29:3d:ca:eb:98:87:ae:3d:74:95:c7:f2:63:8f:8a:3b
> Signature Algorithm: sha256WithRSAEncryption
> Issuer: C = PL, ST = Poznan, L = Poznan, O = none, CN = Secure Boot 
> Signing, emailAddress = exam...@example.com
> Validity
> Not Before: Feb 18 19:28:16 2020 GMT
> Not After : Jan 25 19:28:16 2120 GMT
> Subject: C = PL, ST = Poznan, L = Poznan, O = none, CN = Secure Boot 
> Signing, emailAddress = exam...@example.com
> Subject Public Key Info:
> Public Key Algorithm: rsaEncryption
> Public-Key: (2048 bit)
> Modulus: [cut]
> Exponent: 65537 (0x10001)
> X509v3 extensions:
> X509v3 Subject Key Identifier:
> EC:57:4E:BD:DC:1A:CF:B4:55:16:4A:CE:CB:E4:9E:44:5C:C4:63:F6
> X509v3 Authority Key Identifier:
> EC:57:4E:BD:DC:1A:CF:B4:55:16:4A:CE:CB:E4:9E:44:5C:C4:63:F6
> X509v3 Basic Constraints: critical
> CA:FALSE
> X509v3 Extended Key Usage:
> Code Signing, 1.3.6.1.4.1.311.10.3.6, 1.3.6.1.4.1.2312.16.1.2

This is bad... certs that have 1.3.6.1.4.1.2312.16.1.2 cannot be used
to sign kernels.

Your cert must _not_ have 1.3.6.1.4.1.2312.16.1.2 EKU set on it.

You cannot use the same certificate to sign both kernel and modules.

> Netscape Comment:
>     OpenSSL Generated Certificate
> Signature Algorithm: sha256WithRSAEncryption
> Signature Value: [cut]
>
> On Tue, May 10, 2022 at 3:26 PM Dimitri John Ledkov 
>  wrote:
>>
>> the MOK key as generated by Ubuntu/Debian tooling, creates a signing
>> certificate that self-limits itself to only support Kernel Module
>> signing.
>> Signatures made by such certificate, are not trusted by shim for the
>> purpose of code signing of bootloaders (i.e. grub) or kernels (i.e.
>> linux).
>> I also responded this on stackoverflow.
>>
>> The automatically generated MOK key is only usable to sign kernel
>> modules, i.e. self-built DKMS modules.
>>
>> --
>> okurrr,
>>
>> Dimitri
>>
>> On Tue, 10 May 2022 at 11:33, Łukasz Piątkowski  wrote:
>> >
>> > Hi everyone - I'm new here!
>> >
>> > Sorry for going with my problem directly to the grub-devel maling list, 
>> > but I'm pretty sure my problem is GRUB related. Still, I've spent some 
>> > hours trying to find a solution on the Internet and I failed :( So, here 
>> > it comes - if anyone has time to explain my problem to a layman, it would 
>> > be awesome. Even better, if you can maybe answer here on stackoverflow, 
>> > where it can be easier to find, I believe 
>> > (https://unix.stackexchange.com/questions/701612/cant-load-self-signed-kernel-with-secure-boot-on-bad-shim-signature).
>> >
>> > I'm running ubuntu with Secure Boot on. Everything works fine when I use a 
>> > kernel that comes packaged from cannonical. Still, I have issues running a 
>> > self-signed kernel (this is actually an externally built kernel, that I 
>> > have verified and want to use for my own machine). I'm pretty sure my 
>> > signature with MOK key is OK (verification below), but still when I try to 
>> > boot the kernel from grub, after selecting the correct entry, I get an 
>> > error that reads "Loading .

Re: Can't find a solution to a failed secure boot kernel loading

[Dimitri John Ledkov]

the MOK key as generated by Ubuntu/Debian tooling, creates a signing
certificate that self-limits itself to only support Kernel Module
signing.
Signatures made by such certificate, are not trusted by shim for the
purpose of code signing of bootloaders (i.e. grub) or kernels (i.e.
linux).
I also responded this on stackoverflow.

The automatically generated MOK key is only usable to sign kernel
modules, i.e. self-built DKMS modules.

-- 
okurrr,

Dimitri

On Tue, 10 May 2022 at 11:33, Łukasz Piątkowski  wrote:
>
> Hi everyone - I'm new here!
>
> Sorry for going with my problem directly to the grub-devel maling list, but 
> I'm pretty sure my problem is GRUB related. Still, I've spent some hours 
> trying to find a solution on the Internet and I failed :( So, here it comes - 
> if anyone has time to explain my problem to a layman, it would be awesome. 
> Even better, if you can maybe answer here on stackoverflow, where it can be 
> easier to find, I believe 
> (https://unix.stackexchange.com/questions/701612/cant-load-self-signed-kernel-with-secure-boot-on-bad-shim-signature).
>
> I'm running ubuntu with Secure Boot on. Everything works fine when I use a 
> kernel that comes packaged from cannonical. Still, I have issues running a 
> self-signed kernel (this is actually an externally built kernel, that I have 
> verified and want to use for my own machine). I'm pretty sure my signature 
> with MOK key is OK (verification below), but still when I try to boot the 
> kernel from grub, after selecting the correct entry, I get an error that 
> reads "Loading ... error: bad shim signature." I'm wrapping my head around it 
> and can't find a solution. Why, even though both kernels are signed with MOK 
> keys, one of them works and the other doesn't?
>
> Here's info about kernel signatures:
>
> root@T495:~# sbsign --key /var/lib/shim-signed/mok/MOK.priv --cert 
> /var/lib/shim-signed/mok/MOK.pem /boot/vmlinuz
> Image was already signed; adding additional signature
>
> root@T495:~# sbverify --list /boot/vmlinuz
> signature 1
> image signature issuers:
>  - /C=PL/ST=Poznan/L=Poznan/O=none/CN=Secure Boot 
> Signing/emailAddress=exam...@example.com
> image signature certificates:
>  - subject: /C=PL/ST=yes/L=yes/O=none/CN=Secure Boot 
> Signing/emailAddress=exam...@example.com
>issuer:  /C=PL/ST=yes/L=yes/O=none/CN=Secure Boot 
> Signing/emailAddress=exam...@example.com
> signature 2
> image signature issuers:
>  - /CN=ubuntu Secure Boot Module Signature key
> image signature certificates:
>  - subject: /CN=ubuntu Secure Boot Module Signature key
>issuer:  /CN=ubuntu Secure Boot Module Signature key
>
>
> And here about MOK keys:
>
> root@T495:~# openssl x509 -in /var/lib/shim-signed/mok/MOK.pem -fingerprint 
> -noout
> SHA1 Fingerprint=81:A2:93:CB:06:6F:52:BA:D9:E2:39:68:9D:FA:E2:2B:0C:95:3C:F7
> root@T495:~# mokutil --list-enrolled | grep "81:a2:93"
> SHA1 Fingerprint: 81:a2:93:cb:06:6f:52:ba:d9:e2:39:68:9d:fa:e2:2b:0c:95:3c:f7
>
> If there are any docs that help understand that, I'm happy to be redirected 
> there :)
>
> piontec
> ___
> 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

Re: submenu fails to see variables

[Dimitri John Ledkov]

On Tue, Sep 7, 2021 at 10:30 AM Olaf Hering  wrote:
>
> On Mon, Sep 06, Vladimir 'phcoder' Serbinenko wrote:
>
> > Le lun. 6 sept. 2021 à 12:49, Olaf Hering  a écrit :
> > For some reason global variables are not seen in a submenu {} section.
> > Does anyone happen to know why this behavior is useful?
> > You need to export variable to make it visible in submenu
>
> Thanks. This was less than obvious. I did not expect a command named 'export' 
> in the context of grub.
>
> The documentation needs to be updated to state what the difference between 
> 'set key=val', 'export key=val' and plain 'key=val' actually is.

I have seen somewhere that some distros applied a patch to simply
export all variables always; or like not to create new contexts for
submenus, to keep the variable space the same.

Imho keeping variable space global, and exported by default seems to
lead to least surprises. But it is a behaviour change/break.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 2/2] autogen.sh: Detect python

[Dimitri John Ledkov]

On Wed, Aug 18, 2021 at 6:51 PM Petr Vorel  wrote:
>
> Hi,
>
> > Personally I would just change it to "set PYTHON to python3 if not set" and
> > that's it.
> Why bothering user to set environment variable when autodetection is possible?
>
> > Python2 is irrelevant.
> Fair enough for me to drop python 2 from autodetection. There probably aren't
> many people who would like to use new grub in old distros. But again, when the
> support is there, why to drop it?

10 year old distributions have python3.



--
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 2/2] autogen.sh: Detect python

[Dimitri John Ledkov]

Personally I would just change it to "set PYTHON to python3 if not set" and
that's it.

Python2 is irrelevant.

On Fri, 6 Aug 2021, 07:45 Petr Vorel,  wrote:

> It help to avoid error on distros which has only python3 binary:
> ./autogen.sh: line 20: python: command not found
>
> Using bash builtin 'command -v' to avoid requiring which as extra
> dependency (usable on containers).
>
> Keep the possibility to define PYTHON.
>
> Signed-off-by: Petr Vorel 
> ---
>  autogen.sh | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/autogen.sh b/autogen.sh
> index 31b0ced7e..46f9e1a6d 100755
> --- a/autogen.sh
> +++ b/autogen.sh
> @@ -7,8 +7,21 @@ if [ ! -e grub-core/lib/gnulib/stdlib.in.h ]; then
>exit 1
>  fi
>
> -# Set ${PYTHON} to plain 'python' if not set already
> -: ${PYTHON:=python}
> +# Detect python
> +if [ -z "$PYTHON" ]; then
> +   for i in python python3 python2; do
> +   if command -v "$i" > /dev/null 2>&1; then
> +   PYTHON="$i"
> +   echo "Using $PYTHON" >&2
> +   break
> +   fi
> +   done
> +
> +   if [ -z "$PYTHON" ]; then
> +   echo "python not found" >&2
> +   exit 1
> +   fi
> +fi
>
>  export LC_COLLATE=C
>  unset LC_ALL
> --
> 2.32.0
>
>
> ___
> 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

Re: [PATCH] Adjust deprecated QEMU device name.

[Dimitri John Ledkov]

On Wed, Jul 7, 2021 at 2:52 PM Daniel Kiper  wrote:
>
> On Sun, Jun 13, 2021 at 03:11:51PM +0200, Marius Bakke wrote:
> > The 'ide-drive' device was removed in QEMU 6.0.
>
> Could you add your Signed-off-by?
>
> > * tests/ahci_test.in (outfile): s/ide-drive/ide-hd/
>
> Please drop this.
>
> > ---
> >  tests/ahci_test.in | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/ahci_test.in b/tests/ahci_test.in
> > index 7df560462..d844fe680 100644
> > --- a/tests/ahci_test.in
> > +++ b/tests/ahci_test.in
> > @@ -41,7 +41,7 @@ echo "hello" > "$outfile"
> >
> >  tar cf "$imgfile" "$outfile"
> >
> > -if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}" 
> > --qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci 
> > -device ide-drive,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; 
> > then
> > +if [ "$(echo "nativedisk; source '(ahci0)/$outfile';" | "${grubshell}" 
> > --qemu-opts="-drive id=disk,file=$imgfile,if=none -device ahci,id=ahci 
> > -device ide-hd,drive=disk,bus=ahci.0 " | tail -n 1)" != "Hello World" ]; 
> > then
>
> Is it possible to check QEMU version here and use correct variant then?
>

ide-hd has been available for more than 10 years now in qemu, thus
there shouldn't be any need for backwards compatible names.

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

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

[Dimitri John Ledkov]

On Mon, 12 Jul 2021, 17:16 Sayanta Pattanayak, 
wrote:

> Hi Dimitri,
>
>
>
> Thanks for detailed response.  Sorry for bit late response.
>
>
>
> We are generating own keys and signing with same.
>
> You can kindly have a look at the steps, which are followed for Generating
> Secure Keys and Secure Busybox boot
> https://gitlab.arm.com/arm-reference-solutions/arm-reference-solutions-docs/-/blob/master/docs/infra/common/secure-boot.rst
>
>
>
> In addition, during UEFI Secureboot image building, Grub image is signed
> with following command-
>
> sbsign --key DB.key --cert DB.crt --output bootaa64_signed.efi bootaa64.efi
>
>
>
> And also Kernel Image is signed -
>
> sbsign --key DB.key --cert DB.crt --output Image_signed Image
>
>
>
> We don't intend to Shim, but if we use "--disable-shim-lock" then the
> image is locked down. Does that mean we need to implement a new verifier as
> you suggested in one of your point?
>
>
>
Above all looks sensible to me.

Own db keys, grub-mkimage with disable-shim-lock, signing grub, signing
kernel = should be all working with secureboot on with grub&kernel going
into lockdown without any extra steps or code.

So we probably have a bug somewhere, as others have started to triage
already.

> Thanks,
>
> Sayanta
>
> *From:* Dimitri John Ledkov 
> *Sent:* Thursday, July 8, 2021 4:21 PM
> *To:* Sayanta Pattanayak 
> *Cc:* The development of GNU GRUB ; nd 
> *Subject:* Re: UEFI Secureboot not succeeding with Grub 2.06 and later
> version
>
>
>
> Hi,
>
>
>
> The below mentioned commands are useful. Hence we need to debug this
> further and establish further details about your setup.
>
>
>
> 1) which keys are in DB? ( mokutil --db --list-enrolled )
>
>
>
> 2) which keys are used to sign grub image? ( sbverify --list grub*.efi )
>
>
>
> 3) which keys are used to sign grub image? ( sbverify --list Image )
>
>
>
> 4) since shim verifier was not disabled during grub mkimage build, which
> Shim did you compile, with what toolchain, and which keys was it signed
> with?
>
>
>
> 5) if you don't want to use Shim (and loose ability for users to enroll
> their own machine owner key, and revoke grub via sbat revocation - if
> underlying firmware can do those things i.e. secure edk2 builds), you must
> create grub image with disable shim lock verifier option.
>
>
>
> 6) if you do not want to sbsign kernel image using secureboot keys, you
> can alternative provide detached gpg signature and create grub image with a
> gpg public key built-in.
>
>
>
> 8) maybe there is some other way to verify kernel, i.e. you could
> implement a new verifier module that that use calls to a prior stage
> bootloader or firmware to verify kernel authenticity.
>
>
>
> 9) if you do not want to sign kernel at all in any way, you must disable
> secureboot at either firmware level (SecureBoot variable) or
> shim/grub/linux-only level (MokSBState see mokutil --disable-validation).
> Because if firmware SecureBoot is on, and mokutil validation is on, loading
> unverifiable kernels is not supported in grub 2.06 thanks to implementing
> lockdown.
>
>
>
> If the kernel is expected to be verifiable and yet fails to verify please
> provide further details. We have experienced buggy compilers, binutils,
> sbsign tooling which would produce invalid / unverifiable signatures in the
> past. Also we have seen buggy firmware that fail to verify correctly signed
> binaries.
>
>
>
> On Thu, 8 Jul 2021, 08:05 Sayanta Pattanayak, 
> wrote:
>
> Hi Daniel,
>
> Thanks for your reply and hope you had a great vacation.
> We use Upstream 2.06 tagged version. Mentioning below the Build Commands
> and Console Output.
>
> >-Original Message-
> >From: Daniel Kiper 
> >Sent: Wednesday, July 7, 2021 6:45 PM
> >To: Sayanta Pattanayak 
> >Cc: grub-devel@gnu.org; nd ; javi...@redhat.com;
> >x...@ubuntu.com; pjo...@redhat.com; l...@nuviainc.com
> >Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later
> version
> >
> >Hi Sayanta,
> >
> >Sorry for late reply but I am just recovering after vacation...
> >
> >CC-ing Javier, Dimitri, Peter and Leif.
> >
> >On Thu, Jul 01, 2021 at 03:23:03PM +, Sayanta Pattanayak wrote:
> >> Hi All,
> >> I am new to grub and UEFI secure boot and so a beginners question.
> >> UEFI secureboot on a Arm64 platform works fine with Grub 2.04 version.
> >> The linux kernel image is authenticated and loaded. But the same with
> >> Grub 2.06 version does not progress - following error messages

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

[Dimitri John Ledkov]

On Thu, 8 Jul 2021, 13:05 Michael Chang via Grub-devel, 
wrote:

> Hi Dimitri,
>
> On Thu, Jul 08, 2021 at 11:51:25AM +0100, Dimitri John Ledkov wrote:
> > Hi,
> >
> > The below mentioned commands are useful. Hence we need to debug this
> > further and establish further details about your setup.
>
> I think the problem here is that arm64 already uses LoadImage to verify
> the kernel image so the shim lock is not really required. IMHO the
> lockdown verifier should be relaxed for the arm platform as always will
> be a verifier (LoadImage) used to booting the kernel.
>

But UX is not nice. Many production arm64 servers and cloud instances ship
with UEFI 2011 db keys, and some shims ship revocations. If grub calls into
shim to verify a kernel and gets a reject, grub can stay up and still let
someone choose another boot option. Can grub still do that when calling
LoadImage?

Also, many shims at the moment still ship with EBS Protection turned on on
ARM64 which prevents booting with just LoadImage without first using shim
protocol to verify. Ubuntu's shim has that disabled, but not others and
upstream still do.

Indeed it would be ideal if all grub EFI platforms used LoadImage2 API
without explicit calls to shim protocol, and it would be upto shim to
install LoadImage2 API. Such that from grub's point of view it wouldn't
care if Shim or Firmware verified things. I guess it is 2.08 material.



Thanks,
> Michael
>
> >
> > 1) which keys are in DB? ( mokutil --db --list-enrolled )
> >
> > 2) which keys are used to sign grub image? ( sbverify --list grub*.efi )
> >
> > 3) which keys are used to sign grub image? ( sbverify --list Image )
> >
> > 4) since shim verifier was not disabled during grub mkimage build, which
> > Shim did you compile, with what toolchain, and which keys was it signed
> > with?
> >
> > 5) if you don't want to use Shim (and loose ability for users to enroll
> > their own machine owner key, and revoke grub via sbat revocation - if
> > underlying firmware can do those things i.e. secure edk2 builds), you
> must
> > create grub image with disable shim lock verifier option.
> >
> > 6) if you do not want to sbsign kernel image using secureboot keys, you
> can
> > alternative provide detached gpg signature and create grub image with a
> gpg
> > public key built-in.
> >
> > 8) maybe there is some other way to verify kernel, i.e. you could
> implement
> > a new verifier module that that use calls to a prior stage bootloader or
> > firmware to verify kernel authenticity.
> >
> > 9) if you do not want to sign kernel at all in any way, you must disable
> > secureboot at either firmware level (SecureBoot variable) or
> > shim/grub/linux-only level (MokSBState see mokutil --disable-validation).
> > Because if firmware SecureBoot is on, and mokutil validation is on,
> loading
> > unverifiable kernels is not supported in grub 2.06 thanks to implementing
> > lockdown.
> >
> > If the kernel is expected to be verifiable and yet fails to verify please
> > provide further details. We have experienced buggy compilers, binutils,
> > sbsign tooling which would produce invalid / unverifiable signatures in
> the
> > past. Also we have seen buggy firmware that fail to verify correctly
> signed
> > binaries.
> >
> > On Thu, 8 Jul 2021, 08:05 Sayanta Pattanayak, <
> sayanta.pattana...@arm.com>
> > wrote:
> >
> > > Hi Daniel,
> > >
> > > Thanks for your reply and hope you had a great vacation.
> > > We use Upstream 2.06 tagged version. Mentioning below the Build
> Commands
> > > and Console Output.
> > >
> > > >-Original Message-
> > > >From: Daniel Kiper 
> > > >Sent: Wednesday, July 7, 2021 6:45 PM
> > > >To: Sayanta Pattanayak 
> > > >Cc: grub-devel@gnu.org; nd ; javi...@redhat.com;
> > > >x...@ubuntu.com; pjo...@redhat.com; l...@nuviainc.com
> > > >Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later
> > > version
> > > >
> > > >Hi Sayanta,
> > > >
> > > >Sorry for late reply but I am just recovering after vacation...
> > > >
> > > >CC-ing Javier, Dimitri, Peter and Leif.
> > > >
> > > >On Thu, Jul 01, 2021 at 03:23:03PM +, Sayanta Pattanayak wrote:
> > > >> Hi All,
> > > >> I am new to grub and UEFI secure boot and so a beginners question.
> > > >> UEFI secureboot on a Arm64 platform works fine with Grub 2.04
> version.
> > > >> The linux kernel

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

[Dimitri John Ledkov]

Hi,

The below mentioned commands are useful. Hence we need to debug this
further and establish further details about your setup.

1) which keys are in DB? ( mokutil --db --list-enrolled )

2) which keys are used to sign grub image? ( sbverify --list grub*.efi )

3) which keys are used to sign grub image? ( sbverify --list Image )

4) since shim verifier was not disabled during grub mkimage build, which
Shim did you compile, with what toolchain, and which keys was it signed
with?

5) if you don't want to use Shim (and loose ability for users to enroll
their own machine owner key, and revoke grub via sbat revocation - if
underlying firmware can do those things i.e. secure edk2 builds), you must
create grub image with disable shim lock verifier option.

6) if you do not want to sbsign kernel image using secureboot keys, you can
alternative provide detached gpg signature and create grub image with a gpg
public key built-in.

8) maybe there is some other way to verify kernel, i.e. you could implement
a new verifier module that that use calls to a prior stage bootloader or
firmware to verify kernel authenticity.

9) if you do not want to sign kernel at all in any way, you must disable
secureboot at either firmware level (SecureBoot variable) or
shim/grub/linux-only level (MokSBState see mokutil --disable-validation).
Because if firmware SecureBoot is on, and mokutil validation is on, loading
unverifiable kernels is not supported in grub 2.06 thanks to implementing
lockdown.

If the kernel is expected to be verifiable and yet fails to verify please
provide further details. We have experienced buggy compilers, binutils,
sbsign tooling which would produce invalid / unverifiable signatures in the
past. Also we have seen buggy firmware that fail to verify correctly signed
binaries.

On Thu, 8 Jul 2021, 08:05 Sayanta Pattanayak, 
wrote:

> Hi Daniel,
>
> Thanks for your reply and hope you had a great vacation.
> We use Upstream 2.06 tagged version. Mentioning below the Build Commands
> and Console Output.
>
> >-Original Message-
> >From: Daniel Kiper 
> >Sent: Wednesday, July 7, 2021 6:45 PM
> >To: Sayanta Pattanayak 
> >Cc: grub-devel@gnu.org; nd ; javi...@redhat.com;
> >x...@ubuntu.com; pjo...@redhat.com; l...@nuviainc.com
> >Subject: Re: UEFI Secureboot not succeeding with Grub 2.06 and later
> version
> >
> >Hi Sayanta,
> >
> >Sorry for late reply but I am just recovering after vacation...
> >
> >CC-ing Javier, Dimitri, Peter and Leif.
> >
> >On Thu, Jul 01, 2021 at 03:23:03PM +, Sayanta Pattanayak wrote:
> >> Hi All,
> >> I am new to grub and UEFI secure boot and so a beginners question.
> >> UEFI secureboot on a Arm64 platform works fine with Grub 2.04 version.
> >> The linux kernel image is authenticated and loaded. But the same with
> >> Grub 2.06 version does not progress - following error messages are
> >> displayed.
> >>
> >> error: shim_lock protocol not found.
> >> error: you need to load the kernel first.
> >>
> >> With reference of
> >> "https://www.mail-archive.com/help-grub@gnu.org/msg05375.html";,
> >> created Grub image with "--disable-shim-lock" option. This change
> >> solved the "shim_lock" error but then the following error message
> >> started appearing-
> >>
> >> error: verification requested but nobody cares: /Image.
> >> error: you need to load the kernel first.
> >> Press any key to continue...
> >>
> >> A large set of patches addressing bootHole vulnerability
> >> (https://lists.gnu.org/archive/html/grub-devel/2021-03/msg7.html)
> >> have been merged in the Grub 2.06 version. Does this change the way
> >> images are signed or is there any other change introduced that
> >> required UEFI secure boot to be handled differently on the platform.
> >>
> >> Request any suggestion that would help validate UEFI secure boot with
> >> Grub 2.06 and later version.
> >
> >Do you use GRUB 2.06 upstream or a Linux distribution variant? If upstream
> >could you provide us commands used to build the GRUB and console output
> >when debug is enabled, i.e. "set debug=all"?
> >
>
>
> Commands used -
>
> ./autogen.sh
> ./configure
> STRIP=gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu/bin/aarch64-none-linux-gnu-strip
> --target=aarch64-none-linux-gnu --with-platform=efi --prefix=grub/output/
> --disable-werror
> Make
> make -j $PARALLELISM install
> output/bin/grub-mkimage -v -c ${GRUB_PLAT_CONFIG_FILE} -o
> output/grubaa64.efi -O arm64-efi -p "" part_gpt part_msdos ntfs ntfscomp
> hfsplus fat ext2 normal chain boot configfile linux help part_msdos
> terminal terminfo configfile lsefi search normal gettext loadenv read
> search_fs_file search_fs_uuid search_label
>
>
>
> Following is the console output when "--disable-shim-lock" Not used -
>
> [2J[04D[=3h[2J[09DPress ESCAPE for boot options ...[Bds]Booting
> UEFI Non-Block Boot Device [Bds]Booting UEFI Misc Device [Bds]Booting UEFI
> Misc Device 2 Installed Fat filesystem on FE6E9318 Loading driver at
> 0x000F9264000 EntryPoint=0x000

Re: [PATCH] templates: allow loading a Device Tree file from user conf

[Dimitri John Ledkov]

Hi,

I have proposed a more generic patch to handle devicetree stanaza
before see https://lists.gnu.org/archive/html/grub-devel/2019-05/msg00121.html

Fedora separately ships a similarish patch as well

I have no idea why many distributions ship integration to generate
devicetree commands in grub.cfg and yet such patch is not yet in
upstream grub.

I recommend you to look at Ubuntu or Fedora patches to handle
devicetree command generation.

On Mon, Dec 7, 2020 at 11:37 PM Matteo Croce  wrote:
>
> From: Matteo Croce 
>
> Some machines rely on Device Tree for hardware discovery, so a
> .dtb file must be passed to the kernel.
>
> GRUB can do this via the `devicetree` command, but it's not possible
> to do this from the user configuration.
>
> Add a GRUB_DEVICETREE_FILE variable which holds the path of the .dtb
> file relative to the boot partition and, if present, adds the
> devicetree command to the menuentry.
>
> Signed-off-by: Matteo Croce 
> ---
>  util/grub-mkconfig.in   | 1 +
>  util/grub.d/10_linux.in | 5 +
>  2 files changed, 6 insertions(+)
>
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index d3e879b8e..3e15e6f20 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -228,6 +228,7 @@ export GRUB_DEFAULT \
>GRUB_CMDLINE_NETBSD \
>GRUB_CMDLINE_NETBSD_DEFAULT \
>GRUB_CMDLINE_GNUMACH \
> +  GRUB_DEVICETREE_FILE \
>GRUB_EARLY_INITRD_LINUX_CUSTOM \
>GRUB_EARLY_INITRD_LINUX_STOCK \
>GRUB_TERMINAL_INPUT \
> diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> index e8b01c0d0..fb1bd30f5 100644
> --- a/util/grub.d/10_linux.in
> +++ b/util/grub.d/10_linux.in
> @@ -143,6 +143,11 @@ linux_entry ()
> echo'$(echo "$message" | grub_quote)'
> linux   ${rel_dirname}/${basename} 
> root=${linux_root_device_thisversion} ro ${args}
>  EOF
> +  if [ -n "${GRUB_DEVICETREE_FILE}" ] && test -e 
> "${dirname}/${GRUB_DEVICETREE_FILE}"; then
> +sed "s/^/$submenu_indentation/" << EOF
> +   devicetree ${rel_dirname}/${GRUB_DEVICETREE_FILE}
> +EOF
> +  fi
>if test -n "${initrd}" ; then
>  # TRANSLATORS: ramdisk isn't identifier. Should be translated.
>  message="$(gettext_printf "Loading initial ramdisk ...")"
> --
> 2.28.0
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v6] grub-install: Add backup and restore

[Dimitri John Ledkov]

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register atexit handle to restore the backup if errors occur before
point of no return, or remove the backup if everything was
successful. If atexit is not available, the backup remains on disk for
manual recovery.

Some platforms defined a point of no return, i.e. after modules & core
images were updated. Failures from any commands after that stage are
ignored, and backup is cleanedup. For example, on EFI platforms update
is not reverted when efibootmgr fails.

Extra care is taken to ensure atexit handler is only invoked by the
parent process and not any children forks. Some older grub codebases
can invoke parent atexit hooks from forks, which can mess up the
backup.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR.

This patch only handles backup and restore of files copied to
/boot/grub. This patch does not perform backup (or restoration) of MBR
itself or blocklists. Thus when installing i386-pc platform,
corruption may still occur with MBR and blocklists which will not be
attempted to be automatically recovered.

Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
to ensure it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---
 Changes since v5:
 - use xstrdup
 - change ponr flag from int, to a setter function
 - ensure that all ponr things are guarded with HAVE_ATEXIT check

 configure.ac|   2 +-
 include/grub/util/install.h |  14 
 util/grub-install-common.c  | 159 
 util/grub-install.c |  40 +++--
 util/grub-mknetdir.c|   3 +
 util/grub-mkrescue.c|   3 +
 util/grub-mkstandalone.c|   2 +
 7 files changed, 199 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 74719416c4..a5e94f360e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index b93c73caac..faba902779 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -275,4 +275,18 @@ extern char *grub_install_copy_buffer;
 int
 grub_install_is_short_mbrgap_supported (void);
 
+/*
+ * grub-install-common tries to make backups of modules & auxilary
+ * files, and restore the backup upon failure to install
+ * core.img. There are platforms with additional actions after modules
+ * & core got installed in place. It is a point of no return, as
+ * core.img cannot be reverted from this point onwards, and new
+ * modules should be kept installed. Before performing these
+ * additional actions call grub_set_install_backup_ponr to set the
+ * grub_install_backup_ponr flag, this way failure to perform
+ * additional actions will not result in reverting new modules to the
+ * old ones. (i.e. in case efivars updates fails)
+ */
+void grub_set_install_backup_ponr(void);
+
 #endif
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index b4f28840f1..92970d3630 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,167 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN_NEW,
+  CLEAN_BACKUP,
+  CREATE_BACKUP,
+  RESTORE_BACKUP
+};
+
+#ifdef HAVE_ATEXIT
+static size_t backup_dirs_size = 0;
+static char **backup_dirs = NULL;
+static pid_t backup_process = 0;
+static int grub_install_backup_ponr = 0;
+
+void grub_set_install_backup_ponr(void)
+{
+  grub_install_backup_ponr = 1;
+}
+#else
+void grub_set_install_backup_ponr(void)
+{
+}
+#endif
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  const char *suffix = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+suffix = "~";
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("can

[PATCHv5] grub-install: Add backup and restore

[Dimitri John Ledkov]

From: Dimitri John Ledkov 

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register atexit handle to restore the backup if errors occur before
point of no return, or remove the backup if everything was
successful. If atexit is not available, the backup remains on disk for
manual recovery.

Some platforms defined a point of no return, i.e. after modules & core
images were updated. Failures from any commands after that stage are
ignored, and backup is cleanedup. For example, on EFI platforms update
is not reverted when efibootmgr fails.

Extra care is taken to ensure atexit handler is only invoked by the
parent process and not any children forks. Some older grub codebases
can invoke parent atexit hooks from forks, which can mess up the
backup.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR.

This patch only handles backup and restore of files copied to
/boot/grub. This patch does not perform backup (or restoration) of MBR
itself or blocklists. Thus when installing i386-pc platform,
corruption may still occur with MBR and blocklists which will not be
attempted to be automatically recovered.

Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
to ensure it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---

changes since v4:
 + change ponr from size_t to int (there is no bool in grub yet)
 + declare loop variable at the start of the function
 + format one more comment correctly
 + add comment about significance to set ponr

 configure.ac|   2 +-
 include/grub/util/install.h |  13 
 util/grub-install-common.c  | 143 
 util/grub-install.c |  40 --
 util/grub-mknetdir.c|   3 +
 util/grub-mkrescue.c|   3 +
 util/grub-mkstandalone.c|   2 +
 7 files changed, 182 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 74719416c4..a5e94f360e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index b93c73caac..b2a0175374 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -275,4 +275,17 @@ extern char *grub_install_copy_buffer;
 int
 grub_install_is_short_mbrgap_supported (void);
 
+/*
+ * grub-install-common tries to make backups of modules & auxilary
+ * files, and restore the backup upon failure to install core.img. There
+ * are platforms with additional actions after modules & core got
+ * installed in place. It is a point of no return, as core.img cannot be
+ * reverted from this point onwards, and new modules should be kept
+ * installed. Before performing these additional actions raise
+ * grub_install_backup_ponr flag, this way failure to perform additional
+ * actions will not result in reverting new modules to the old
+ * ones. (i.e. in case efivars updates fails)
+ */
+extern int grub_install_backup_ponr;
+
 #endif
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index b4f28840f1..d493e4a119 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,151 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN_NEW,
+  CLEAN_BACKUP,
+  CREATE_BACKUP,
+  RESTORE_BACKUP
+};
+
+static size_t backup_dirs_size = 0;
+static char **backup_dirs = NULL;
+static pid_t backup_process = 0;
+int grub_install_backup_ponr = 0;
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  const char *suffix = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+suffix = "~";
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("cannot open directory `%s': %s"),
+  di, grub_util_fd_strerror ());
+}
 
   while ((de = grub_util_fd

[PATCH v4] grub-install: Add backup and restore

[Dimitri John Ledkov]

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register atexit handle to restore the backup if errors occur before
point of no return, or remove the backup if everything was
successful. If atexit is not available, the backup remains on disk for
manual recovery.

Some platforms defined a point of no return, i.e. after modules & core
images were updated. Failures from any commands after that stage are
ignored, and backup is cleanedup. For example, on EFI platforms update
is not reverted when efibootmgr fails.

Extra care is taken to ensure atexit handler is only invoked by the
parent process and not any children forks. Some older grub codebases
can invoke parent atexit hooks from forks, which can mess up the
backup.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR.

This patch only handles backup and restore of files copied to
/boot/grub. This patch does not perform backup (or restoration) of MBR
itself or blocklists. Thus when installing i386-pc platform,
corruption may still occur with MBR and blocklists which will not be
attempted to be automatically recovered.

Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
to ensure it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---
 configure.ac|   2 +-
 include/grub/util/install.h |  13 
 util/grub-install-common.c  | 140 
 util/grub-install.c |  33 ++---
 util/grub-mknetdir.c|   3 +
 util/grub-mkrescue.c|   3 +
 util/grub-mkstandalone.c|   2 +
 7 files changed, 172 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 74719416c4..a5e94f360e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index b93c73caac..d6e9ce4460 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -275,4 +275,17 @@ extern char *grub_install_copy_buffer;
 int
 grub_install_is_short_mbrgap_supported (void);
 
+/*
+ * grub-install-common tries to make backups of modules & auxilary
+ * files, and restore the backup upon failure to install core.img. There
+ * are platforms with additional actions after modules & core got
+ * installed in place. It is a point of no return, as core.img cannot be
+ * reverted from this point onwards, and new modules should be kept
+ * installed. Before performing these additional actions raise
+ * grub_install_backup_ponr flag, this way failure to perform additional
+ * actions will not result in reverting new modules to the old
+ * ones. (i.e. in case efivars updates fails)
+ */
+extern size_t grub_install_backup_ponr;
+
 #endif
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index b4f28840f1..84c1db8674 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,148 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN_NEW,
+  CLEAN_BACKUP,
+  CREATE_BACKUP,
+  RESTORE_BACKUP
+};
+
+static size_t backup_dirs_size = 0;
+static char **backup_dirs = NULL;
+static pid_t backup_process = 0;
+size_t grub_install_backup_ponr = 0;
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  const char *suffix = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+suffix = "~";
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("cannot open directory `%s': %s"),
+  di, grub_util_fd_strerror ());
+}
 
   while ((de = grub_util_fd_readdir (d)))
 {
   const char *ext = strrchr (de->d_name, '.');
-  if ((ext && (strcmp (ext, ".mod") == 0
-  || strcmp (ext, ".lst") == 0
-  || strcmp (ext, &quo

Re: [PATCH] grub-install: Add backup and restore

[Dimitri John Ledkov]

On Tue, 4 May 2021 at 18:41, Daniel Kiper  wrote:
>
> Hey,
>
> In general much better but...
>
> On Thu, Apr 29, 2021 at 12:36:37PM +0100, Dimitri John Ledkov wrote:
> > Refactor clean_grub_dir to create a backup of all the files, instead
> > of just irrevocably removing them as the first action. If available,
> > register atexit handle to restore the backup if errors occur before
> > point of no return, or remove the backup if everything was
> > successful. If atexit is not available, the backup remains on disk for
> > manual recovery.
> >
> > Some platforms defined a point of no return, i.e. after modules & core
> > images were updated. Failures from any commands after that stage are
> > ignored, and backup is cleanedup. For example, on EFI platforms update
> > is not reverted when efibootmgr fails.
> >
> > Extra care is taken to ensure atexit handler is only invoked by the
> > parent process and not any children forks. Some older grub codebases
> > can invoke parent atexit hooks from forks, which can mess up the
> > backup.
> >
> > This allows safer upgrades of MBR & modules, such that
> > modules/images/fonts/translations are consistent with MBR in case of
> > errors. For example accidental grub-install /dev/non-existent-disk
> > currently clobbers and upgrades modules in /boot/grub, despite not
> > actually updating any MBR. This increases peak disk-usage slightly, by
> > requiring temporarily twice the disk space to complete grub-install.
>
> I think this last sentence does not parse with the rest of paragraph.
>

simply dropped.

> Additionally, may I ask you to add a blurb here about the blocklist?
> I think about most important things from the email exchange between
> Micheal and you.
>

Added a summary.

> > Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
> > to ensure it is also cleaned / backed up / restored.
> >
> > Signed-off-by: Dimitri John Ledkov 
> > ---
> >
> >  Changes since v2:
> >  - switch from on_exit, to atexit
> >  - introduce point of no return flag, as atexit doesn't know about
> >exit status and at times it is desired to declare point of no
> >return earlier and ignore some error.
> >  - address review feedback wrt styling
> >  - improve reliablity, verify that only parent process calls atexit
> >hook
> >
> >  configure.ac|   2 +-
> >  include/grub/util/install.h |  11 +++
> >  util/grub-install-common.c  | 142 
> >  util/grub-install.c |  33 +++--
> >  util/grub-mknetdir.c|   3 +
> >  util/grub-mkrescue.c|   3 +
> >  util/grub-mkstandalone.c|   2 +
> >  7 files changed, 172 insertions(+), 24 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 74719416c4..a5e94f360e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -419,7 +419,7 @@ else
> >  fi
> >
> >  # Check for functions and headers.
> > -AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
> > +AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
> >  AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
> >
> >  # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
> > deprecation
> > diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> > index b93c73caac..d729060ce7 100644
> > --- a/include/grub/util/install.h
> > +++ b/include/grub/util/install.h
> > @@ -275,4 +275,15 @@ extern char *grub_install_copy_buffer;
> >  int
> >  grub_install_is_short_mbrgap_supported (void);
> >
> > +/* grub-install-common tries to make backups of modules & auxilary
> > +files, and restore the backup upon failure to install core.img. There
> > +are platforms with additional actions after modules & core got
> > +installed in place. It is a point of no return, as core.img cannot be
> > +reverted from this point onwards, and new modules should be kept
> > +installed. Before performing these additional actions raise
> > +grub_install_backup_ponr flag, this way failure to perform additional
> > +actions will not result in reverting new modules to the old
> > +ones. (i.e. in case efivars updates fails) */
>
> Could you format comments like it is documented here:
>   https://www.gnu.org/software/grub/manual/grub-dev/grub-dev.html#Comments
>

done.

> > +extern size_t grub_install_backup_ponr;
> > +
> >  #endif
> > diff --git a/util/grub-install-common.c b/util/grub-install-common.c
> > index

Re: [PATCH] grub-install: Add backup and restore

[Dimitri John Ledkov]

On Mon, May 3, 2021 at 6:09 AM Michael Chang via Grub-devel
 wrote:
>
> On Thu, Apr 29, 2021 at 12:36:37PM +0100, Dimitri John Ledkov wrote:
> > Refactor clean_grub_dir to create a backup of all the files, instead
> > of just irrevocably removing them as the first action. If available,
> > register atexit handle to restore the backup if errors occur before
> > point of no return, or remove the backup if everything was
> > successful. If atexit is not available, the backup remains on disk for
> > manual recovery.
> >
> > Some platforms defined a point of no return, i.e. after modules & core
> > images were updated. Failures from any commands after that stage are
> > ignored, and backup is cleanedup. For example, on EFI platforms update
> > is not reverted when efibootmgr fails.
>
> Thank you for implementing this. I think it has addressed my question in
> other discussion that backup/restore may be inadvertently triggered in
> some cases which is not desirable, for eg when efibootmgr errors out as
> you also depicted.
>
> >
> > Extra care is taken to ensure atexit handler is only invoked by the
> > parent process and not any children forks. Some older grub codebases
> > can invoke parent atexit hooks from forks, which can mess up the
> > backup.
> >
> > This allows safer upgrades of MBR & modules, such that
> > modules/images/fonts/translations are consistent with MBR in case of
> > errors. For example accidental grub-install /dev/non-existent-disk
> > currently clobbers and upgrades modules in /boot/grub, despite not
> > actually updating any MBR. This increases peak disk-usage slightly, by
> > requiring temporarily twice the disk space to complete grub-install.
> >
> > Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
> > to ensure it is also cleaned / backed up / restored.
>
> There's a corner case that the installation can be done via blocklist,
> which means restore files doesn't help at all to recover error, given
> the blocklist recorded in the images no longer apply to the lately
> restored core.img.
>
> I know the blocklist is neither recommended nor enabled by default so it
> is good for me to neglect that. Just to complete the message here in
> case anyone else would hold an opinion otherwise.
>

That is correct. At the moment point of no return for bios updates is
set after trying to install core.img into
mbr/blocklist/grub-pc-gpt-partition. There is no backup of
mbr/blocklists/grub-pc-gpt-partitions performed, nor is restoration
attempted.

At the very least this patch prevents machines to have i386-pc
core.img & modules missmatched when the wrong drive is specified, the
gap is too small, drive does not exist, drive is read-only etc. I.e.
when no update of mbr/blocklist/grub-pc-gpt-partition has actually
occurred and it bailed at the pre-update checks stage.

It would be neat to attempt backup of those things, and restore them
upon partial update. However, I fear if after all the pre-update
validations one fails to update mbr/blocklist/grub-pc-gpt-partition it
probably is hardware / drive failure and the backup we read or the
backup we try to restore will also fail.


> Thanks,
> Michael
>
> >
> > Signed-off-by: Dimitri John Ledkov 
> > ---
> >
> >  Changes since v2:
> >  - switch from on_exit, to atexit
> >  - introduce point of no return flag, as atexit doesn't know about
> >exit status and at times it is desired to declare point of no
> >return earlier and ignore some error.
> >  - address review feedback wrt styling
> >  - improve reliablity, verify that only parent process calls atexit
> >hook
> >
> >  configure.ac|   2 +-
> >  include/grub/util/install.h |  11 +++
> >  util/grub-install-common.c  | 142 
> >  util/grub-install.c |  33 +++--
> >  util/grub-mknetdir.c|   3 +
> >  util/grub-mkrescue.c|   3 +
> >  util/grub-mkstandalone.c|   2 +
> >  7 files changed, 172 insertions(+), 24 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 74719416c4..a5e94f360e 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -419,7 +419,7 @@ else
> >  fi
> >
> >  # Check for functions and headers.
> > -AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
> > +AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
> >  AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
> >
> >  # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
> > deprecation
> > diff --git a/inclu

[PATCH] grub-install: Add backup and restore

[Dimitri John Ledkov]

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register atexit handle to restore the backup if errors occur before
point of no return, or remove the backup if everything was
successful. If atexit is not available, the backup remains on disk for
manual recovery.

Some platforms defined a point of no return, i.e. after modules & core
images were updated. Failures from any commands after that stage are
ignored, and backup is cleanedup. For example, on EFI platforms update
is not reverted when efibootmgr fails.

Extra care is taken to ensure atexit handler is only invoked by the
parent process and not any children forks. Some older grub codebases
can invoke parent atexit hooks from forks, which can mess up the
backup.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR. This increases peak disk-usage slightly, by
requiring temporarily twice the disk space to complete grub-install.

Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
to ensure it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---

 Changes since v2:
 - switch from on_exit, to atexit
 - introduce point of no return flag, as atexit doesn't know about
   exit status and at times it is desired to declare point of no
   return earlier and ignore some error.
 - address review feedback wrt styling
 - improve reliablity, verify that only parent process calls atexit
   hook

 configure.ac|   2 +-
 include/grub/util/install.h |  11 +++
 util/grub-install-common.c  | 142 
 util/grub-install.c |  33 +++--
 util/grub-mknetdir.c|   3 +
 util/grub-mkrescue.c|   3 +
 util/grub-mkstandalone.c|   2 +
 7 files changed, 172 insertions(+), 24 deletions(-)

diff --git a/configure.ac b/configure.ac
index 74719416c4..a5e94f360e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent atexit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/include/grub/util/install.h b/include/grub/util/install.h
index b93c73caac..d729060ce7 100644
--- a/include/grub/util/install.h
+++ b/include/grub/util/install.h
@@ -275,4 +275,15 @@ extern char *grub_install_copy_buffer;
 int
 grub_install_is_short_mbrgap_supported (void);
 
+/* grub-install-common tries to make backups of modules & auxilary
+files, and restore the backup upon failure to install core.img. There
+are platforms with additional actions after modules & core got
+installed in place. It is a point of no return, as core.img cannot be
+reverted from this point onwards, and new modules should be kept
+installed. Before performing these additional actions raise
+grub_install_backup_ponr flag, this way failure to perform additional
+actions will not result in reverting new modules to the old
+ones. (i.e. in case efivars updates fails) */
+extern size_t grub_install_backup_ponr;
+
 #endif
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index b4f28840f1..db7feae7d3 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,150 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN,
+  CLEAN_BACKUP,
+  CREATE_BACKUP,
+  RESTORE_BACKUP,
+};
+
+static size_t backup_dirs_len = 0;
+static char **backup_dirs;
+static pid_t backup_process = 0;
+size_t grub_install_backup_ponr = 0;
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  char suffix[2] = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+strcpy (suffix, "~");
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("cannot open directory `%s': %s"),
+  di, grub_util_fd_strerror ());
+}
 
   while ((de = grub_util_fd_readdir (d)))
 {
   const char *ext = strrchr (de->d_name, '.');
-  if ((ext && (strcmp (ext, &q

[PATCH] unix exec: avoid atexit handlers when child execvp fails

[Dimitri John Ledkov]

Functions `grub_util_exec_pipe()` and `grub_util_exec_pipe_stderr()`
currently call `execvp()`. If the call fails for any reason, the child
currently calls `exit(127)`. This in turn executes the parents
`atexit()` handlers from the forked child, and then the same handlers
are called again from parent. This is usually not desired, and can
lead to deadlocks, and undesired behaviour.

This patch fixes up "unix exec: avoid atexit handlers when child
exits" further.

Fixes e75cf4a58b5eaf482804e5e1b2cc7d4399df350e

Signed-off-by: Dimitri John Ledkov 
---
 grub-core/osdep/unix/exec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grub-core/osdep/unix/exec.c b/grub-core/osdep/unix/exec.c
index db3259f650..e8db9202fb 100644
--- a/grub-core/osdep/unix/exec.c
+++ b/grub-core/osdep/unix/exec.c
@@ -188,7 +188,7 @@ grub_util_exec_pipe (const char *const *argv, int *fd)
   close (pipe_fd[1]);
 
   execvp ((char *) argv[0], (char **) argv);
-  exit (127);
+  _exit (127);
 }
   else
 {
@@ -234,7 +234,7 @@ grub_util_exec_pipe_stderr (const char *const *argv, int 
*fd)
   close (pipe_fd[1]);
 
   execvp ((char *) argv[0], (char **) argv);
-  exit (127);
+  _exit (127);
 }
   else
 {
-- 
2.27.0


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v2 1/8] linux/arm: fix ARM Linux header layout

[Dimitri John Ledkov]

On Thu, Mar 11, 2021 at 11:44 AM Dimitri John Ledkov
 wrote:
>
> The patch from 
> https://lists.gnu.org/archive/html/grub-devel/2020-10/msg00122.html
>
> Is still not applied, and yet it is required to boot armhf linux
> kernel in qemu OVMF with grub.
>
> It is applied in rhboot/grub2
> https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/0143-Make-linux_arm_kernel_header.hdr_offset-be-at-the-ri.patch
>
> And it is applied in OpenSUSE too
> https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/0005-Make-linux_arm_kernel_header.hdr_offset-be-at-the-ri.patch?expand=1
>
> I want to apply it in Ubuntu too.
>
> It is still not present in grub2 mainline, and applies cleanly.
>
> Can it please be applied?

And to respond to
https://lists.gnu.org/archive/html/grub-devel/2020-11/msg00014.html

All of the above mentioned distros carry further patches to use
LoadImage api to boot on armhf just like arm64 loader does, and then
hdr_header/offset are being accessed and things blow up. So the header
is wrong, doesn't blow up in upstream configuration, but will once
LoadImage is used on armhf upstream too.
Please fix the header. Even when not used, it should not be a lie.

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v2 1/8] linux/arm: fix ARM Linux header layout

[Dimitri John Ledkov]

The patch from 
https://lists.gnu.org/archive/html/grub-devel/2020-10/msg00122.html

Is still not applied, and yet it is required to boot armhf linux
kernel in qemu OVMF with grub.

It is applied in rhboot/grub2
https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/0143-Make-linux_arm_kernel_header.hdr_offset-be-at-the-ri.patch

And it is applied in OpenSUSE too
https://build.opensuse.org/package/view_file/openSUSE:Factory/grub2/0005-Make-linux_arm_kernel_header.hdr_offset-be-at-the-ri.patch?expand=1

I want to apply it in Ubuntu too.

It is still not present in grub2 mainline, and applies cleanly.

Can it please be applied?

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add chainloaded image as shim's verifiable object

[Dimitri John Ledkov]

On Fri, Mar 5, 2021 at 1:34 PM Michael Chang  wrote:
>
> On Fri, Mar 05, 2021 at 12:21:49PM +, Dimitri John Ledkov wrote:
> > This is not an oversight but intentional.
> >
> > Currently there is no chainloader support with SBAT as further
> > development is required to ensure policy is applied correctly. Once
> > SBAT support for chainloading is defined, it will be introduced.
>
> Hm. What I heard is that shim wouldn't enfore SBAT validation for the
> "indirect" image loaded by grub for now. So we should still able to
> boot, eg, kernel and other efi image loaded by grub which is still in
> lacking of SBAT support.
>

There is a difference between chainloading a new bootloader, or
booting a linux kernel.
When chainloading a different grub that is verified by the vendor-db
in shim, sbat should be enforced, if the current grub has sbat.
When chainloading windows bootmgr, which is signed by things in db,
sbat need not be enforced.

It is a delicate balance to strike, as otherwise one can do sbat
bypass via downgrading to older grub via chainload.

I would recommend to add windowsbootmgr efi entry, and then perform
`exit 1` such that this current shim+grub exit, and firmware goes to
load the next entry for windows. For `exit 1` support you need
https://github.com/rhboot/grub2/commit/ccce3d69ae3eacc7bdc70217304586bd7e74fe1e
too.

I understand that this is very suboptimal. I kind of wish grub2 could
generate and display EFI boot entries, and then let people select them
which would set it as BootNext and reset. Cause then one would truly
be able to jump to a specific boot menu entry from grub menu.

I also need chainloader support elsewhere, thus at the moment we are
not shipping 2.06rc1 and instead are shipping 2.04 with backports of
patches.


> > And yes it is intended to continue to allow "boot windows" as the menu
> > entry in grub.
>
> So what is recommended solution in the interim ? We just can't afford
> to release new grub version that cannot do the chainload ...
>
> Thanks,
> Michael
>
> >
> > On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel
> >  wrote:
> > >
> > > While attempting to dual boot Microsoft Windows with efi chainloader, it
> > > failed with below error when secure boot was enabled.
> > >
> > > error ../../grub-core/kern/verifiers.c:119:verification requested but
> > > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi.
> > >
> > > It is a regression, as previously it worked without problem.
> > >
> > > It turns out chainloading image has been locked down introduced by
> > >
> > > 578c95298 kern: Add lockdown support
> > >
> > > However we should consider it as verifiable object to shim to allow
> > > booting in secure boot enabled mode. The chainloaded image could also
> > > have trusted signature signed by vendor with their pubkey cert in db.
> > > For that matters it's usage should not be locked down in secure boot,
> > > and instead use shim to validate it's signature before running it.
> > >
> > > Signed-off-by: Michael Chang 
> > > ---
> > >  grub-core/kern/efi/sb.c   | 1 +
> > >  grub-core/kern/lockdown.c | 1 -
> > >  2 files changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
> > > index 41dadcd14..96d237722 100644
> > > --- a/grub-core/kern/efi/sb.c
> > > +++ b/grub-core/kern/efi/sb.c
> > > @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ 
> > > ((unused)),
> > >  case GRUB_FILE_TYPE_BSD_KERNEL:
> > >  case GRUB_FILE_TYPE_XNU_KERNEL:
> > >  case GRUB_FILE_TYPE_PLAN9_KERNEL:
> > > +case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE:
> > >*flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
> > >
> > >/* Fall through. */
> > > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c
> > > index 0bc70fd42..e1fd1c1e2 100644
> > > --- a/grub-core/kern/lockdown.c
> > > +++ b/grub-core/kern/lockdown.c
> > > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ 
> > > ((unused)),
> > >  case GRUB_FILE_TYPE_PXECHAINLOADER:
> > >  case GRUB_FILE_TYPE_PCCHAINLOADER:
> > >  case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER:
> > > -case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE:
> > >  case GRUB_FILE_TYPE_ACPI_TABLE:
> > >  case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE:
> > >*flags = GRUB_VERIFY_FLAGS_DEFER_AUTH;
> > > --
> > > 2.26.2
> > >
> > >
> > > ___
> > > Grub-devel mailing list
> > > Grub-devel@gnu.org
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> >
> > --
> > Regards,
> >
> > Dimitri.
> >
>


-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] Add chainloaded image as shim's verifiable object

[Dimitri John Ledkov]

This is not an oversight but intentional.

Currently there is no chainloader support with SBAT as further
development is required to ensure policy is applied correctly. Once
SBAT support for chainloading is defined, it will be introduced.

And yes it is intended to continue to allow "boot windows" as the menu
entry in grub.

On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel
 wrote:
>
> While attempting to dual boot Microsoft Windows with efi chainloader, it
> failed with below error when secure boot was enabled.
>
> error ../../grub-core/kern/verifiers.c:119:verification requested but
> nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi.
>
> It is a regression, as previously it worked without problem.
>
> It turns out chainloading image has been locked down introduced by
>
> 578c95298 kern: Add lockdown support
>
> However we should consider it as verifiable object to shim to allow
> booting in secure boot enabled mode. The chainloaded image could also
> have trusted signature signed by vendor with their pubkey cert in db.
> For that matters it's usage should not be locked down in secure boot,
> and instead use shim to validate it's signature before running it.
>
> Signed-off-by: Michael Chang 
> ---
>  grub-core/kern/efi/sb.c   | 1 +
>  grub-core/kern/lockdown.c | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c
> index 41dadcd14..96d237722 100644
> --- a/grub-core/kern/efi/sb.c
> +++ b/grub-core/kern/efi/sb.c
> @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ 
> ((unused)),
>  case GRUB_FILE_TYPE_BSD_KERNEL:
>  case GRUB_FILE_TYPE_XNU_KERNEL:
>  case GRUB_FILE_TYPE_PLAN9_KERNEL:
> +case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE:
>*flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK;
>
>/* Fall through. */
> diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c
> index 0bc70fd42..e1fd1c1e2 100644
> --- a/grub-core/kern/lockdown.c
> +++ b/grub-core/kern/lockdown.c
> @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ 
> ((unused)),
>  case GRUB_FILE_TYPE_PXECHAINLOADER:
>  case GRUB_FILE_TYPE_PCCHAINLOADER:
>  case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER:
> -case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE:
>  case GRUB_FILE_TYPE_ACPI_TABLE:
>  case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE:
>*flags = GRUB_VERIFY_FLAGS_DEFER_AUTH;
> --
> 2.26.2
>
>
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCHv2] grub-install: Add backup and restore

[Dimitri John Ledkov]

Hi,

On Wed, 9 Dec 2020 at 05:15, Michael Chang  wrote:
>
> On Tue, Dec 08, 2020 at 05:58:40AM +, Dimitri John Ledkov wrote:
> > On Tue, 8 Dec 2020, 03:17 Michael Chang,  wrote:
> >
> > > On Mon, Dec 07, 2020 at 12:37:28PM +, Dimitri John Ledkov wrote:
> > > > Refactor clean_grub_dir to create a backup of all the files, instead
> > > > of just irrevocably removing them as the first action. If available,
> > > > register on_exit handle to restore the backup if any errors occur, or
> > > > remove the backup if everything was successful. If on_exit is not
> > > > available, the backup remains on disk for manual recovery.
> > > >
> > > > This allows safer upgrades of MBR & modules, such that
> > > > modules/images/fonts/translations are consistent with MBR in case of
> > > > errors. For example accidental grub-install /dev/non-existent-disk
> > > > currently clobbers and upgrades modules in /boot/grub, despite not
> > > > actually updating any MBR. This increases peak disk-usage slightly, by
> > > > requiring temporarily twice the disk space to complete grub-install.
> > >
> > > I'd love to have the described problem fixed too, as it helps a lot in
> > > the update path to survive grub-install error which can be contributed
> > > by many different reasons, even though grub has to stay with old version
> > > but still much better than unbootable system.
> > >
> > > But here I have a concern, as to what will happen if the error comes
> > > after image installation ? For example, the efibootmgr may fail to
> > > register boot entry after copying the images to efi partition. It looks
> > > to me that the restoring backup would be triggerd incorrectly for the
> > > new image to load restored old backups.
> > >
> > > Would you please help to clarify that ?
> > >
> >
> >
> > On Ubuntu we install secureboot signed prebuilt EFI images only which
> > prohibit module loading. Hence usually it is irrelevant if the EFI grub
> > modules are old or new.
> >
> > And we do not call efibootmgr at all, as it does excessive writes to
> > efivars. Instead we don't even try to update efivars if there is no need.
> > It was submitted to this mailing list but I am not sure on the acceptance
> > status https://lists.gnu.org/archive/html/grub-devel/2019-03/msg00123.html
> >
> > Above two things make EFI updates less fatal, as it is harder for a
> > prebuilt signed grub, which does not use modules, to fail booting. Unlike
> > i386-pc case.
>
> My concern is exactly that these are not upstreamed yet so accepting the
> patch would be premature. Either it has to be adapted to satisfy the
> expected behavior of existing codebase or wait until those dependent
> patches gets merged.
>

this patch does not depend on efivars patch behaviour, and is useful
with either efibootmgr or efivars codepath. If either way to update
EFI vars fails, rollback is performed.

> Besides, other architecture like openfirmware would update nvram in a
> similar fashion to uefi but cannot be covered by the aforementioned
> efivars enhacement patch.
>

If nvram update of openfirmware fails, modules which are copied over
first must too be rolled backed. In case of failures there is no way
to know for sure which state to take, but it is safer to revert.

> >
> > And to answer your immediate question - the new EFI image will be used for
> > boot without rolling back.
> >
> > Also, I am not sure how everyone installs signed grubs. Thus adding support
> > for ESP backup and restore might be futile upstream. As far as I can tell
> > it is safe to call grub-install on Ubuntu, whereas it might not be on other
> > distros. As Ubuntu preserves / reinstalls signed grub EFI images, instead
> > of generating a new random one and putting it on ESP thus causing failure
> > to boot with secureboot due to effectively replacing signed grub with an
> > unsigned one. And for resilient boot we have support for maintaining and
> > synchronising multiple ESP for Linux raid.
>
> Installing signed grub can happen in parallel with any unsigned one
> installed by grub-install as long as they didn't overlap in the position
> on EFI System partition. For that matters the efibootmgr is used to
> manage them.
>
> The signed grub is so far not controlled by grub-install, so should be
> disregared at this moment. The entire backup/recovery should be done
> only for those made aware by grub-install.
>

This patch so far only manages things that are done grub-install,

Re: [PATCHv2] grub-install: Add backup and restore

[Dimitri John Ledkov]

On Tue, 8 Dec 2020, 03:17 Michael Chang,  wrote:

> On Mon, Dec 07, 2020 at 12:37:28PM +0000, Dimitri John Ledkov wrote:
> > Refactor clean_grub_dir to create a backup of all the files, instead
> > of just irrevocably removing them as the first action. If available,
> > register on_exit handle to restore the backup if any errors occur, or
> > remove the backup if everything was successful. If on_exit is not
> > available, the backup remains on disk for manual recovery.
> >
> > This allows safer upgrades of MBR & modules, such that
> > modules/images/fonts/translations are consistent with MBR in case of
> > errors. For example accidental grub-install /dev/non-existent-disk
> > currently clobbers and upgrades modules in /boot/grub, despite not
> > actually updating any MBR. This increases peak disk-usage slightly, by
> > requiring temporarily twice the disk space to complete grub-install.
>
> I'd love to have the described problem fixed too, as it helps a lot in
> the update path to survive grub-install error which can be contributed
> by many different reasons, even though grub has to stay with old version
> but still much better than unbootable system.
>
> But here I have a concern, as to what will happen if the error comes
> after image installation ? For example, the efibootmgr may fail to
> register boot entry after copying the images to efi partition. It looks
> to me that the restoring backup would be triggerd incorrectly for the
> new image to load restored old backups.
>
> Would you please help to clarify that ?
>


On Ubuntu we install secureboot signed prebuilt EFI images only which
prohibit module loading. Hence usually it is irrelevant if the EFI grub
modules are old or new.

And we do not call efibootmgr at all, as it does excessive writes to
efivars. Instead we don't even try to update efivars if there is no need.
It was submitted to this mailing list but I am not sure on the acceptance
status https://lists.gnu.org/archive/html/grub-devel/2019-03/msg00123.html

Above two things make EFI updates less fatal, as it is harder for a
prebuilt signed grub, which does not use modules, to fail booting. Unlike
i386-pc case.

And to answer your immediate question - the new EFI image will be used for
boot without rolling back.

Also, I am not sure how everyone installs signed grubs. Thus adding support
for ESP backup and restore might be futile upstream. As far as I can tell
it is safe to call grub-install on Ubuntu, whereas it might not be on other
distros. As Ubuntu preserves / reinstalls signed grub EFI images, instead
of generating a new random one and putting it on ESP thus causing failure
to boot with secureboot due to effectively replacing signed grub with an
unsigned one. And for resilient boot we have support for maintaining and
synchronising multiple ESP for Linux raid.

I kind of wish we'd prebuilt more core images at package build time and
simply copy them around. Not just the ones that have signing. Instead of
invoking mkimage on every installed system.

Nonetheless, it should be possible to hook up more files and directories to
perform backup, cleanup and restore on. Is there is a desire. The function
calls simply need to be added in the appropriate places.


> >
> > Also add modinfo.sh to the cleanup/backup/restore codepath, to ensure
> > it is also cleaned / backed up / restored.
> >
> > Signed-off-by: Dimitri John Ledkov 
> > ---
> >
> >  Changes since v1: as reported on linux-ext4 mailing list and debugged
> >  by Colin Watson, the grub_util_path_concat_ext call was incorrect in
> >  the restore backup case as there was no extention needed. Instead the
> >  call is corrected to just grub_util_path_concat.
> >
> >  This patch is now shipped in Ubuntu & Debian in multiple series. It
> >  would be nice to have this merged upstream, as it greatly improves
> >  grub upgrades and prevents missmatches of MBR & modules.
> >
> >  configure.ac   |   2 +-
> >  util/grub-install-common.c | 105 +++--
> >  2 files changed, 91 insertions(+), 16 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 7c10a4db7..71cd414c3 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -419,7 +419,7 @@ else
> >  fi
> >
> >  # Check for functions and headers.
> > -AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
> > +AC_CHECK_FUNCS(posix_memalign memalign getextmntent on_exit)
> >  AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
> >
> >  # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits
> deprecation
> > diff --git a/util/grub-install-common.c b/util/grub-install-co

[PATCHv2] grub-install: Add backup and restore

[Dimitri John Ledkov]

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register on_exit handle to restore the backup if any errors occur, or
remove the backup if everything was successful. If on_exit is not
available, the backup remains on disk for manual recovery.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR. This increases peak disk-usage slightly, by
requiring temporarily twice the disk space to complete grub-install.

Also add modinfo.sh to the cleanup/backup/restore codepath, to ensure
it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---

 Changes since v1: as reported on linux-ext4 mailing list and debugged
 by Colin Watson, the grub_util_path_concat_ext call was incorrect in
 the restore backup case as there was no extention needed. Instead the
 call is corrected to just grub_util_path_concat.

 This patch is now shipped in Ubuntu & Debian in multiple series. It
 would be nice to have this merged upstream, as it greatly improves
 grub upgrades and prevents missmatches of MBR & modules.

 configure.ac   |   2 +-
 util/grub-install-common.c | 105 +++--
 2 files changed, 91 insertions(+), 16 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c10a4db7..71cd414c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent on_exit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 277eaf4e2..74584b92b 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,113 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN = 0,
+  CLEAN_BACKUP = 1,
+  CREATE_BACKUP = 2,
+  RESTORE_BACKUP = 3,
+};
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  char suffix[2] = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+{
+  strcpy (suffix, "-");
+}
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("cannot open directory `%s': %s"),
+  di, grub_util_fd_strerror ());
+}
 
   while ((de = grub_util_fd_readdir (d)))
 {
   const char *ext = strrchr (de->d_name, '.');
-  if ((ext && (strcmp (ext, ".mod") == 0
-  || strcmp (ext, ".lst") == 0
-  || strcmp (ext, ".img") == 0
-  || strcmp (ext, ".mo") == 0)
-  && strcmp (de->d_name, "menu.lst") != 0)
- || strcmp (de->d_name, "efiemu32.o") == 0
- || strcmp (de->d_name, "efiemu64.o") == 0)
+  if ((ext && (strcmp_ext (ext, ".mod", suffix) == 0
+  || strcmp_ext (ext, ".lst", suffix) == 0
+  || strcmp_ext (ext, ".img", suffix) == 0
+  || strcmp_ext (ext, ".mo", suffix) == 0)
+  && strcmp_ext (de->d_name, "menu.lst", suffix) != 0)
+ || strcmp_ext (de->d_name, "modinfo.sh", suffix) == 0
+ || strcmp_ext (de->d_name, "efiemu32.o", suffix) == 0
+ || strcmp_ext (de->d_name, "efiemu64.o", suffix) == 0)
{
- char *x = grub_util_path_concat (2, di, de->d_name);
- if (grub_util_unlink (x) < 0)
-   grub_util_error (_("cannot delete `%s': %s"), x,
-grub_util_fd_strerror ());
- free (x);
+ char *srcf = grub_util_path_concat (2, di, de->d_name);
+
+ if (mode == CREATE_BACKUP)
+   {
+ char *dstf = grub_util_path_concat_ext (2, di, de->d_name, "-");
+ if (grub_util_rename (srcf, dstf) < 0)
+   grub_util_error (_("cannot backup `%s': %s&quo

[PATCH] grub-install: Add backup and restore

[Dimitri John Ledkov]

Refactor clean_grub_dir to create a backup of all the files, instead
of just irrevocably removing them as the first action. If available,
register on_exit handle to restore the backup if any errors occur, or
remove the backup if everything was successful. If on_exit is not
available, the backup remains on disk for manual recovery.

This allows safer upgrades of MBR & modules, such that
modules/images/fonts/translations are consistent with MBR in case of
errors. For example accidental grub-install /dev/non-existent-disk
currently clobbers and upgrades modules in /boot/grub, despite not
actually updating any MBR. This increases peak disk-usage slightly, by
requiring temporarily twice the disk space to complete grub-install.

Also add modinfo.sh to the cleanup/backup/restore codepath, to ensure
it is also cleaned / backed up / restored.

Signed-off-by: Dimitri John Ledkov 
---
 configure.ac   |   2 +-
 util/grub-install-common.c | 105 +++--
 2 files changed, 91 insertions(+), 16 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c10a4db7..71cd414c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -419,7 +419,7 @@ else
 fi
 
 # Check for functions and headers.
-AC_CHECK_FUNCS(posix_memalign memalign getextmntent)
+AC_CHECK_FUNCS(posix_memalign memalign getextmntent on_exit)
 AC_CHECK_HEADERS(sys/param.h sys/mount.h sys/mnttab.h limits.h)
 
 # glibc 2.25 still includes sys/sysmacros.h in sys/types.h but emits 
deprecation
diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 0295d40f5..e5f069a13 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -185,38 +185,113 @@ grub_install_mkdir_p (const char *dst)
   free (t);
 }
 
+static int
+strcmp_ext (const char *a, const char *b, const char *ext)
+{
+  char *bsuffix = grub_util_path_concat_ext (1, b, ext);
+  int r = strcmp (a, bsuffix);
+  free (bsuffix);
+  return r;
+}
+
+enum clean_grub_dir_mode
+{
+  CLEAN = 0,
+  CLEAN_BACKUP = 1,
+  CREATE_BACKUP = 2,
+  RESTORE_BACKUP = 3,
+};
+
 static void
-clean_grub_dir (const char *di)
+clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode)
 {
   grub_util_fd_dir_t d;
   grub_util_fd_dirent_t de;
+  char suffix[2] = "";
+
+  if ((mode == CLEAN_BACKUP) || (mode == RESTORE_BACKUP))
+{
+  strcpy (suffix, "-");
+}
 
   d = grub_util_fd_opendir (di);
   if (!d)
-grub_util_error (_("cannot open directory `%s': %s"),
-di, grub_util_fd_strerror ());
+{
+  if (mode == CLEAN_BACKUP)
+   return;
+  grub_util_error (_("cannot open directory `%s': %s"),
+  di, grub_util_fd_strerror ());
+}
 
   while ((de = grub_util_fd_readdir (d)))
 {
   const char *ext = strrchr (de->d_name, '.');
-  if ((ext && (strcmp (ext, ".mod") == 0
-  || strcmp (ext, ".lst") == 0
-  || strcmp (ext, ".img") == 0
-  || strcmp (ext, ".mo") == 0)
-  && strcmp (de->d_name, "menu.lst") != 0)
- || strcmp (de->d_name, "efiemu32.o") == 0
- || strcmp (de->d_name, "efiemu64.o") == 0)
+  if ((ext && (strcmp_ext (ext, ".mod", suffix) == 0
+  || strcmp_ext (ext, ".lst", suffix) == 0
+  || strcmp_ext (ext, ".img", suffix) == 0
+  || strcmp_ext (ext, ".mo", suffix) == 0)
+  && strcmp_ext (de->d_name, "menu.lst", suffix) != 0)
+ || strcmp_ext (de->d_name, "modinfo.sh", suffix) == 0
+ || strcmp_ext (de->d_name, "efiemu32.o", suffix) == 0
+ || strcmp_ext (de->d_name, "efiemu64.o", suffix) == 0)
{
- char *x = grub_util_path_concat (2, di, de->d_name);
- if (grub_util_unlink (x) < 0)
-   grub_util_error (_("cannot delete `%s': %s"), x,
-grub_util_fd_strerror ());
- free (x);
+ char *srcf = grub_util_path_concat (2, di, de->d_name);
+
+ if (mode == CREATE_BACKUP)
+   {
+ char *dstf = grub_util_path_concat_ext (2, di, de->d_name, "-");
+ if (grub_util_rename (srcf, dstf) < 0)
+   grub_util_error (_("cannot backup `%s': %s"), srcf,
+grub_util_fd_strerror ());
+ free (dstf);
+   }
+ else if (mode == RESTORE_BACKUP)
+   {
+ char *dstf = grub_util_path_concat_ext (2, di, de->d_name);
+ dstf[strlen (dstf) - 1] = 0;
+ if (grub_util_rename (srcf, dstf) < 0)
+   grub_util_error (_("cannot restore `%s': %s"), dstf,
+grub_ut

Re: [SECURITY PATCH 00/28] Multiple GRUB2 vulnerabilities - BootHole

[Dimitri John Ledkov]

On Wed, 29 Jul 2020 at 21:20, John Paul Adrian Glaubitz
 wrote:
>
> On 7/29/20 10:12 PM, Christian Hesse wrote:
> > This does not apply on top of grub 2.04. Will downstream maintainers have to
> > do their cherry-picking on its own or will a maintenance branch on top of
> > grub-2.04 (or what ever) be available?
> > I would like to push updates to the Arch Linux repositories.
>
> You may want to look at the Debian package which already has the patches
> applied in Debian unstable [1].
>
> I'm surprised that Arch did not receive a disclosure of the vulnerabilities
> under NDA since Debian and the various enterprise distributions have it
> already.

Disclosures were done to a subset of binary distributions that have a
trust path to shims signed with Microsoft UEFI CA 2011 db key. Arch
Linux does not provide shim-signed with keys controlled by Arch Linux
and it doesn't provide pre-signed secureboot kernels.

Reading Arch Linux documentation it seems that Fedora's shim is used
together with self-signed Mok Keys.

Mitigation strategy for Arch Linux will then be quite different to
everyone else:

1) Update to new shim from fedora when available, as previous ones are
going to be revoked by the dbxupdate from uefi.org
2) Patch Archlinux grub
3) Patch Archilinux kernel for lockdown bypass
4) Generate new MOK key, enroll it into MOK
5) Sign patched grub/kernel with the new MOK key
6) Provide instructions for users to revoke their old key via MOKX,
i.e. use mokutil --mokx --import existing cert; or for example delete
the old key from MOK with --delete old-cert.der

This is just a rough guideline, please analyze how signing keys are
controlled and used on typical Arch Linux deployment and adjust things
to taste.

The key point is to rotate the signing key used for
shim/grub/kernel/fwupd, only use the new key to sign fixed things, and
ensure that old key is no longer trusted (removed from MOK, or added
to MOKX).

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] disk/loopback: Don't verify loopback images

[Dimitri John Ledkov]

On Tue, 2 Jun 2020, 13:53 Vladimir 'phcoder' Serbinenko, 
wrote:

>
>
> On Tue, Jun 2, 2020, 13:21 Dimitri John Ledkov  wrote:
>
>> On Tue, 2 Jun 2020 at 12:12, Vladimir 'phcoder' Serbinenko
>>  wrote:
>> >
>> >
>> >
>> > On Mon, Jun 1, 2020, 15:21 Chris Coulson 
>> wrote:
>> >>
>> >> When a file is verified, the entire contents of the verified file are
>> >> loaded in to memory and retained until the file handle is closed. A
>> >> consequence of this is that opening a loopback image can incur a
>> >> significant memory cost.
>> >>
>> >> As loopback devices are just another disk implementation, don't treat
>> >> loopback images any differently to physical disk images, and skip
>> >> verification of them. Files opened from the filesystem within a
>> loopback
>> >> image will still be passed to verifier modules where required.
>> >>
>> >> Signed-off-by: Chris Coulson 
>> >> ---
>> >>  grub-core/disk/loopback.c | 3 ++-
>> >>  1 file changed, 2 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/grub-core/disk/loopback.c b/grub-core/disk/loopback.c
>> >> index cdf9123fa..01267e577 100644
>> >> --- a/grub-core/disk/loopback.c
>> >> +++ b/grub-core/disk/loopback.c
>> >> @@ -93,7 +93,8 @@ grub_cmd_loopback (grub_extcmd_context_t ctxt, int
>> argc, char **args)
>> >>  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename
>> expected"));
>> >>
>> >>file = grub_file_open (args[1], GRUB_FILE_TYPE_LOOPBACK
>> >> -| GRUB_FILE_TYPE_NO_DECOMPRESS);
>> >> +| GRUB_FILE_TYPE_NO_DECOMPRESS |
>> >> +GRUB_FILE_TYPE_SKIP_SIGNATURE);
>> >
>> > Maybe the verifier should rather decide to skip verification if fuller
>> type is loopback?
>>
>> How would it be used then? For example, I imagine one can gpg sign the
>> .iso or a .squashfs and then assume everything inside it is trusted
>> (even if things inside are not signed).
>> However, I don't believe any verifier works this way today, nor not
>> sure it is desired versus signing individual things inside the
>> loopback device.
>>
>> At the moment, without this patch, a lot of things break. It is common
>> to loopback mount .iso which currently eats all the RAM and machines
>> crash with out of memory. (I.e. loopback mounting 2.5GB .iso).
>> Similarly it is common to use things like WUBI, where .raw image file
>> is loopback mounted from NTFS. If one is doing secureboot and tpm is
>> present that again results in out of memory.
>>
>> Taking the measurement / checksum / verifying the loopback device is
>> not a problem, but keeping it all in RAM is. And imho trusting
>> unsigned things inside verified loopback device is dubious too.
>>
>> So yeah, probably it's something that verifiers should be able to
>> decide upon and perform intelligently. But at the moment, the only
>> practical answer for all of them is to skip.
>>
> GPG one can read the file in chunks and then keep in memory only hash of
> every chunk to prevent TOCTOU.
> But then the question is when is our makes sense versus signing individual
> files. I can see uses like signing squashfs.
> We probably need a "secure device" flag somewhere long term
>

Preventing TOCTOU is nice.

And "secure device" is a nice concept.

For example a policy can consider contents of LUKS2 encrypted or dmverity
sealed /boot to be trusted. Because encryption prevents tampering with that
filesystem. Especially if it is not decrypted or mounted at runtime.

Do we need to add comments here to explain the design thinking for future
improvements? Or should we change verifiers with a link to a TODO comment
explaining what could be done to make this better?

Or should we hide this flag behind an argument
--skip-sig of the loopback command to give scripting flexibility?



>> --
>> Regards,
>>
>> Dimitri.
>>
>> ___
>> 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
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] disk/loopback: Don't verify loopback images

[Dimitri John Ledkov]

On Tue, 2 Jun 2020 at 12:12, Vladimir 'phcoder' Serbinenko
 wrote:
>
>
>
> On Mon, Jun 1, 2020, 15:21 Chris Coulson  wrote:
>>
>> When a file is verified, the entire contents of the verified file are
>> loaded in to memory and retained until the file handle is closed. A
>> consequence of this is that opening a loopback image can incur a
>> significant memory cost.
>>
>> As loopback devices are just another disk implementation, don't treat
>> loopback images any differently to physical disk images, and skip
>> verification of them. Files opened from the filesystem within a loopback
>> image will still be passed to verifier modules where required.
>>
>> Signed-off-by: Chris Coulson 
>> ---
>>  grub-core/disk/loopback.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/disk/loopback.c b/grub-core/disk/loopback.c
>> index cdf9123fa..01267e577 100644
>> --- a/grub-core/disk/loopback.c
>> +++ b/grub-core/disk/loopback.c
>> @@ -93,7 +93,8 @@ grub_cmd_loopback (grub_extcmd_context_t ctxt, int argc, 
>> char **args)
>>  return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
>>
>>file = grub_file_open (args[1], GRUB_FILE_TYPE_LOOPBACK
>> -| GRUB_FILE_TYPE_NO_DECOMPRESS);
>> +| GRUB_FILE_TYPE_NO_DECOMPRESS |
>> +GRUB_FILE_TYPE_SKIP_SIGNATURE);
>
> Maybe the verifier should rather decide to skip verification if fuller type 
> is loopback?

How would it be used then? For example, I imagine one can gpg sign the
.iso or a .squashfs and then assume everything inside it is trusted
(even if things inside are not signed).
However, I don't believe any verifier works this way today, nor not
sure it is desired versus signing individual things inside the
loopback device.

At the moment, without this patch, a lot of things break. It is common
to loopback mount .iso which currently eats all the RAM and machines
crash with out of memory. (I.e. loopback mounting 2.5GB .iso).
Similarly it is common to use things like WUBI, where .raw image file
is loopback mounted from NTFS. If one is doing secureboot and tpm is
present that again results in out of memory.

Taking the measurement / checksum / verifying the loopback device is
not a problem, but keeping it all in RAM is. And imho trusting
unsigned things inside verified loopback device is dubious too.

So yeah, probably it's something that verifiers should be able to
decide upon and perform intelligently. But at the moment, the only
practical answer for all of them is to skip.

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 08/10] templates: Output a menu entry for firmware setup on UEFI FastBoot systems

[Dimitri John Ledkov]

There should be a slightly more recent version of the patch which changes
the label to "UEFI Firmware Settings" which more consistent with vendor
Bios documentation, GUI tools, systemctl, and Windows 10. We are currently
trying to have that label consistently everywhere in Ubuntu.

I was thinking to upstream this patch too. So +1 from me.

On Fri, 13 Mar 2020, 19:16 Javier Martinez Canillas, 
wrote:

> From: Steve Langasek 
>
> If fastboot is enabled in the BIOS then often it is not possible to
> enter the firmware setup menu, add a menu entry for this.
>
> hdegoede: Cherry picked the Ubuntu patch from:
>
> https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/tree/debian/patches/uefi_firmware_setup.patch
> Into the Fedora / RH grub version
>
> According to:
>
> https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/tree/debian/copyright
> The patch is licensed under GPL-3+
>
> Signed-off-by: Steve Langasek 
> [hdegoede: fix use with /sys/firmware/efi/efivars]
> Signed-off-by: Hans de Goede 
> Signed-off-by: Javier Martinez Canillas 
>
> ---
>
>  Makefile.util.def   |  6 +
>  util/grub.d/30_uefi-firmware.in | 46 +
>  2 files changed, 52 insertions(+)
>  create mode 100644 util/grub.d/30_uefi-firmware.in
>
> diff --git a/Makefile.util.def b/Makefile.util.def
> index 1e0799a68f9..d9e2bd84de8 100644
> --- a/Makefile.util.def
> +++ b/Makefile.util.def
> @@ -515,6 +515,12 @@ script = {
>installdir = grubconf;
>  };
>
> +script = {
> +  name = '30_uefi-firmware';
> +  common = util/grub.d/30_uefi-firmware.in;
> +  installdir = grubconf;
> +};
> +
>  script = {
>name = '40_custom';
>common = util/grub.d/40_custom.in;
> diff --git a/util/grub.d/30_uefi-firmware.in b/util/grub.d/
> 30_uefi-firmware.in
> new file mode 100644
> index 000..93ececffea7
> --- /dev/null
> +++ b/util/grub.d/30_uefi-firmware.in
> @@ -0,0 +1,46 @@
> +#! /bin/sh
> +set -e
> +
> +# grub-mkconfig helper script.
> +# Copyright (C) 2012  Free Software Foundation, Inc.
> +#
> +# GRUB is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# GRUB is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with GRUB.  If not, see .
> +
> +prefix="@prefix@"
> +exec_prefix="@exec_prefix@"
> +datarootdir="@datarootdir@"
> +
> +export TEXTDOMAIN=@PACKAGE@
> +export TEXTDOMAINDIR="@localedir@"
> +
> +. "@datadir@/@PACKAGE@/grub-mkconfig_lib"
> +
> +efi_vars_dir=/sys/firmware/efi/efivars
> +EFI_GLOBAL_VARIABLE=8be4df61-93ca-11d2-aa0d-00e098032b8c
> +OsIndications="$efi_vars_dir/OsIndicationsSupported-$EFI_GLOBAL_VARIABLE"
> +
> +if [ -e "$OsIndications" ] && \
> +   [ "$(( $(printf 0x%x \'"$(cat $OsIndications | cut -b5)") & 1 ))" = 1
> ]; then
> +  LABEL="System setup"
> +
> +  gettext_printf "Adding boot menu entry for EFI firmware
> configuration\n" >&2
> +
> +  onstr="$(gettext_printf "(on %s)" "${DEVICE}")"
> +
> +  cat << EOF
> +menuentry '$LABEL' \$menuentry_id_option 'uefi-firmware' {
> +   fwsetup
> +}
> +EOF
> +fi
> --
> 2.24.1
>
>
> ___
> 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

Re: are posix-shell-compliant continuation lines valid/supported, or not, in /etc/default/grub ?

[Dimitri John Ledkov]

I am sorry which email are the "quoted" phrases from?

Because that's nothing that I have said. I hope you do understand that it
is intended for all of the Ubuntu installer & upgrade integrations to keep
valid configs compatible. And in general, distros do not intentially break
their users for no reason. I simply stated what are the most typical and
the most tested customisation and integration point for grub configs on
Ubuntu, without making any statements about supportability of the configs
that you have observed getting malformed.

On Fri, 20 Mar 2020, 01:08 PGNet Dev,  wrote:

> hi
>
> On 3/19/20 5:57 PM, Dimitri John Ledkov wrote:
>
>
> > What is the bug number there?
>
>
>
>
> https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1868138
>
>
>
> >  In general, we advise to customize via grub.d drop-in files instead of
> modifying etc/default/grub file itself.
>
>
>
> Sure, that's fine as advice.  Fwiw, it's different for different distros.
>
>
>
> Declarations that using posix-compliant continuations is "highly unusual
> abuse of POSIX shell semantics" is a different matter.
>
>
>
> Changing/breaking behavior in managing valid/working grub configs, and
> rationalizing it by "I do not see a need to support it"
>
>  is ... unhelpful.
>
>
>
> Particularly if Grub project/devs say that its usage *is* OK & compliant.
> Which is why I asked _here_; and, iiuc, seems that it *is*.
>
>
>
> Grub's stability, reproducibility and portability are critical.  It all
> works great, particularly when the excellent docs are followed.
>
> Happy to take ongoing conversation to the bug.
>
>
>
>
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: are posix-shell-compliant continuation lines valid/supported, or not, in /etc/default/grub ?

[Dimitri John Ledkov]

Have you opened a launchpad bug report against the grub2 package with both
configs before and after? What is the bug number there?

In general, we do parse and rewrite configs using debconf which is Perl / C
/ Shell processing using tools external to grub. In general, we advise to
customize via grub.d drop-in files instead of modifying etc/default/grub
file itself.

This is a bit out of scope for the upstream mailing list, as debconf
integration is downstream specific on Ubuntu/Debian systems.

On Thu, 19 Mar 2020, 21:20 PGNet Dev,  wrote:

> a recent grub package update, in ubuntu 18LTS, is breaking
> /etc/default/grub by mangling/overwriting users' entries, in the specific
> case of using continuation lines in the file/config. and, subsequently, the
> upgrade process on these servers.
>
> as stated clearly at
>
>
> https://www.gnu.org/software/grub/manual/grub/html_node/Simple-configuration.html#Simple-configuration-handling
>
> "...
> The file /etc/default/grub controls the operation of
> grub-mkconfig. It is sourced by a shell script, and so must be valid POSIX
> shell input; normally, it will just be a sequence of ‘KEY=value’ lines, but
> if the value contains spaces or other special characters then it must be
> quoted
> ..."
>
> per POSIX
>
>
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_02_01
>
> "...
> Escape Character (Backslash)
>
> A  that is not quoted shall preserve the literal value
> of the following character, with the exception of a . If a
>  follows the , the shell shall interpret this as line
> continuation. The  and  shall be removed before
> splitting the input into tokens. Since the escaped  is removed
> entirely from the input and is not replaced by any white space, it cannot
> serve as a token separator.
>
> ..."
>
> all that's to say that continuation lines appear to be supported in
> posix-shell-compliant /etc/default/grub
>
> they've been in use here, on 100s of servers, in grub configs for ages.
>
> with grub 2.04 on other, non Ubu OS, it continues to work fine.
>
> re: the use of continuation lines, ubu dev states that
>
> "Regardless of what the POSIX spec says, this is highly unusual
> abuse of POSIX shell semantics, and I do not see a need to support it"
>
> it'd be useful/helpful to get clear on what the 'official' grub project
> support for use of posix-shell-compliant continuation lines is ...
>
> are they 'supported' as valid use in /etc/default/grub, or not?
>
> also useful to know/understand whether any grub update can/should mangle a
> user's /etc/default/grub.  allowed? expected?
>
> thx!
>
>
>
> ___
> 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

Re: [PATCH v7 2/2] Add a module for retrieving SMBIOS information

[Dimitri John Ledkov]

On Fri, 5 Jul 2019 at 13:48, David Michael  wrote:
>
> The following are two use cases from Rajat Jain :
>
> 1) We have a board that boots Linux and this board itself can be plugged into 
> one of different chassis types. We need to pass different parameters to the 
> kernel based on the "CHASSIS_TYPE" information that is passed by the bios in 
> the DMI / SMBIOS tables.
>
> 2) We may have a USB stick that can go into multiple boards, and the exact 
> kernel to be loaded depends on the machine information (PRODUCT_NAME etc) 
> passed via the DMI.
>
> Signed-off-by: David Michael 
>

Another use case is aarch64 laptops effort. We have the same kernel,
and the same image bootable and usable across 4 different laptop
models, with the only difference between them being the DTB to load.
I was looking at how I can distinguish between the four models and
automatically pick the right dtb. I did see these patches and was very
sad when I found out that they didn't get merged.

Thank you for rebasing these on top of v2.04. I will try to cherrypick
them on top of Debian's experimental 2.04 grub and try to build an
installer "that just boots" without requiring users to pick a
particular dtb menuentry by hand. There are only dtbs in these images,
and only one right one for each model.

(Post install, we have flash-kernel mechanisms to continuously flash
and use the right dtb, this is needed for the installer's grub to be
able to detect models and thus boot the right dtb)

Whilst these laptops might become dtb-free in the future, I expect new
hardware to come out which would need dtb picker based on SMBIOS info
again.

-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH] 10_linux: Add devicetree command, if a dtb is present.

[Dimitri John Ledkov]

On Tue, 28 May 2019 at 12:01, Leif Lindholm  wrote:
>
> On Tue, May 28, 2019 at 11:27:08AM +0200, Daniel Kiper wrote:
> > On Thu, May 23, 2019 at 10:31:13PM +0100, Dimitri John Ledkov wrote:
> > > Specifically support dtb paths as created by flash-kernel package on
> > > Debian/Ubuntu systems, and a generic one. Possibly this needs to be
> > > extended to support other devicetree blob paths as installed on other
> > > distributions (similar to how multiple initrd paths are
> > > supported).
> > >
> > > This is particularly useful during new platforms bring up, which may
> > > not yet be fully supported by Linux kernel. Currently I have tested
> > > this patch, on top of Debian grub2 packaging/distro-patches to
> > > successfully boot Asus Novago TP3700QL. Similarly other laptops would
> > > find this useful, like Lenovo Mixx 630, Lenovo Yoga C630, and HP Envy
> > > X2.
> > >
> > > Signed-off-by: Dimitri John Ledkov 
> >
> > LGTM but I would like to hear Leif and/or Alex (CC-ed) opinion here.
>
> This is not my preferred solution to this problem (Dimitri, I'll ping
> you offline regarding this).
>
> Also it is fundamentally incompatible with UEFI Secure Boot (see
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=927888).
>
> We should probably look into disabling the devicetree command by
> default for the UEFI port. I'll revisit that point post release.
>

Yes, in an ideal world all devices would boot correctly with
SecureBoot enforced, using plain ACPI and no dtbs would be needed and
no devicetree command would be needed in grub at all. And when bugs in
ACPI are discovered vendors would issue firmware updates promptly that
would get applied by users in a timely manner using fwupd mechanism /
uefi capsules / windows updater.

In practice, I have had to patch dtbs on Intel NUCs, arm64 servers and
laptops. I suspect we will experience broken ACPI on other devices and
architectures in the future too (RISC-V comes to mind). This is an
unsecureboot crutch, that works in concert with other crutches already
available in the distros (i.e. at package upgrade time copying things
to /boot & generating matching sets of kernel/initrd/dtb). And makes
crutches look like a useful matching set, and works today. And yes,
this is not the preferred way of doing things. But I hope it will be
useful, however temporary, to whomever is working on new devices
brings-ups / iterating kernels&dtbs. In those cases automatic
integration of devicetree command in grub.cfg generator would be a
nice touch.

The alternative proposal would probably still end up reading the dtbs
from these very same paths, if we expect to ship dtbs in distro's and
make them upgradable using package managers. Unless they are
statically embeded / signed in the alternative utility to poke them
into UEFI environment.

Originally I submitted this as a distro patch to Debian, with
maintainer requesting it to be forwarded upstream - as this is a
universally general feature, rather than something distro specific:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=929427


Regards,

Dimitri.


> Regards,
>
> Leif
>
> > And this is not a release material.
> >
> > Daniel
> >
> > > ---
> > >  util/grub.d/10_linux.in | 15 +++
> > >  1 file changed, 15 insertions(+)
> > >
> > > diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
> > > index 4532266be..bb6c8912f 100644
> > > --- a/util/grub.d/10_linux.in
> > > +++ b/util/grub.d/10_linux.in
> > > @@ -153,6 +153,13 @@ EOF
> > >  sed "s/^/$submenu_indentation/" << EOF
> > > echo'$(echo "$message" | grub_quote)'
> > > initrd  $(echo $initrd_path)
> > > +EOF
> > > +  fi
> > > +  if test -n "${dtb}" ; then
> > > +message="$(gettext_printf "Loading device tree blob...")"
> > > +sed "s/^/$submenu_indentation/" << EOF
> > > +   echo'$(echo "$message" | grub_quote)'
> > > +   devicetree  ${rel_dirname}/${dtb}
> > >  EOF
> > >fi
> > >sed "s/^/$submenu_indentation/" << EOF
> > > @@ -236,6 +243,14 @@ while [ "x$list" != "x" ] ; do
> > >  gettext_printf "Found initrd image: %s\n" "$(echo $initrd_display)" 
> > > >&2
> > >fi
> > >
> > > +  dtb=
> > > +  for i in "dtb-${version}" "dtb-${alt_version}" "dtb"; do
> > > +if test -e "${dirname}/${i}" ; then
> > > +  dtb="$i"
> > > +  break
> > > +fi
> > > +  done
> > > +
> > >config=
> > >for i in "${dirname}/config-${version}" 
> > > "${dirname}/config-${alt_version}" 
> > > "/etc/kernels/kernel-config-${version}" ; do
> > >  if test -e "${i}" ; then
> > > --
> > > 2.20.1



-- 
Regards,

Dimitri.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH] 10_linux: Add devicetree command, if a dtb is present.

[Dimitri John Ledkov]

Specifically support dtb paths as created by flash-kernel package on
Debian/Ubuntu systems, and a generic one. Possibly this needs to be
extended to support other devicetree blob paths as installed on other
distributions (similar to how multiple initrd paths are
supported).

This is particularly useful during new platforms bring up, which may
not yet be fully supported by Linux kernel. Currently I have tested
this patch, on top of Debian grub2 packaging/distro-patches to
successfully boot Asus Novago TP3700QL. Similarly other laptops would
find this useful, like Lenovo Mixx 630, Lenovo Yoga C630, and HP Envy
X2.

Signed-off-by: Dimitri John Ledkov 
---
 util/grub.d/10_linux.in | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/util/grub.d/10_linux.in b/util/grub.d/10_linux.in
index 4532266be..bb6c8912f 100644
--- a/util/grub.d/10_linux.in
+++ b/util/grub.d/10_linux.in
@@ -153,6 +153,13 @@ EOF
 sed "s/^/$submenu_indentation/" << EOF
echo'$(echo "$message" | grub_quote)'
initrd  $(echo $initrd_path)
+EOF
+  fi
+  if test -n "${dtb}" ; then
+message="$(gettext_printf "Loading device tree blob...")"
+sed "s/^/$submenu_indentation/" << EOF
+   echo'$(echo "$message" | grub_quote)'
+   devicetree  ${rel_dirname}/${dtb}
 EOF
   fi
   sed "s/^/$submenu_indentation/" << EOF
@@ -236,6 +243,14 @@ while [ "x$list" != "x" ] ; do
 gettext_printf "Found initrd image: %s\n" "$(echo $initrd_display)" >&2
   fi
 
+  dtb=
+  for i in "dtb-${version}" "dtb-${alt_version}" "dtb"; do
+if test -e "${dirname}/${i}" ; then
+  dtb="$i"
+  break
+fi
+  done
+
   config=
   for i in "${dirname}/config-${version}" "${dirname}/config-${alt_version}" 
"/etc/kernels/kernel-config-${version}" ; do
 if test -e "${i}" ; then
-- 
2.20.1


___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel