Subject: [bug#65002] [PATCH v2 2/2] gnu: bootloader: grub: Add support for loading an additional initrd

From: Ludovic Courtès <[email-masked]>
Date: Wed, 10 Jan 2024 00:28:18 +0100

Tomas Volf <[email-masked]> skribis:

> In order to be able to provide decryption keys for the LUKS device, they need
> to be available in the initial ram disk.  However they cannot be stored inside
> the usual initrd, since it is stored in the store and being a
> world-readable (as files in the store are) is not a desired property for a
> initrd containing decryption keys.

This explanation should go in the manual IMO (it’s already partly there).

> This commit adds an option to load additional initrd during the boot,
> one that is not stored inside the store and therefore can contain
> secrets.
>
> Since only grub supports encrypted /boot, only grub is modified to use the
> extra-initrd.  There is no use case for the other bootloaders.
>
> * doc/guix.texi (Bootloader Configuration): Describe the new extra-initrd
> field.
> * gnu/bootloader.scm: Add extra-initrd field to bootloader-configuration
> * gnu/bootloader/grub.scm: Use the new extra-initrd field

It’d be great if you could specify the entities changes in each file
(which variable/procedure is changed, what is added/removed).  A
committer can do it on your behalf later if you’re unsure.

> +@item @code{extra-initrd} (default: @code{#f})
> +Path to an additional initrd to load.  Should not point to a file in the

s/Path/File name/ (by convention)

Please make full sentences.  “Should not” is probably too strong;
perhaps: “It may or may not point to a file in the store, but the main
use case is for out-of-store files containing secrets.”

> +store.  Typical use case is making keys to unlock LUKS device available

Add a line break after “store.” to distinguish the reference from the
discussion of one possible use case.

> +during the boot process.  For any use case not involving secrets, you
> +should use regular initrd (@pxref{operating-system Reference,
> +@code{initrd}}) instead.
> +
> +Suitable image can be created for example like this:
> +
> +@example
> +echo /key-file.bin | cpio -oH newc >/key-file.cpio
> +chmod 0000 /key-file.cpio
> +@end example
> +
> +Be careful when using this option, since pointing to a file that is not
> +readable by the grub while booting will cause the boot to fail and
> +require a manual edit of the initrd line in the grub menu.
> +
> +Currently only supported by grub.

s/grub/GRUB/

Would be great if you could include also a short config example here, or
add a cross-reference to the example for
‘luks-device-mapping-with-options’ if that covers both.

> +  (extra-initrd          bootloader-configuration-extra-initrd
> +                         (default #f))    ;string | #f
> +  )

No lonely paren please.  :-)

Otherwise LGTM.

Could you send updated patches with these minor changes?

Thanks!

Ludo’.