GNU bug report logs - #37851
Grub installation only checks for encrypted /boot folder

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37851 in the body.
You can then email your comments to 37851 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Miguel Arruga Vivas <rosen644835 <at> gmail.com>
To: bug-guix <at> gnu.org
Subject: Grub installation only checks for encrypted /boot folder
Date: Mon, 21 Oct 2019 13:07:09 +0200

Hi,

The following configuration results in an unbootable system.  The
root partition must be manually mounted with cryptomount in order to
boot the system.

The core issue is that grub unencrypts automatically, as
GRUB_ENABLE_CRYPTODISK=y was provided during installation, the /boot
partition, but not the partition which contains /gnu/store.

Happy hacking!
Miguel

==================== config.scm ====================
;; ....
(operating-system
  ;; ...
  (bootloader
    (bootloader-configuration
      (bootloader grub-bootloader)
      (target "/dev/sda")))
  (mapped-devices
    (list (mapped-device
            (source (uuid "uuid root device"))
            (target "root")
            (type luks-device-mapping))
          (mapped-device
            (source (uuid "uuid boot device"))
            (target "boot")
            (type luks-device-mapping))))
  (file-systems
    (cons* (file-system
             (mount-point "/")
             (device "/dev/mapper/root")
             (type "btrfs")
             (dependencies mapped-devices))
           (file-system
             (mount-point "/boot")
             (device "/dev/mapper/boot")
             (type "ext4")
             (dependencies mapped-devices))
           %base-file-systems)))
==================== config.scm ====================

Message #8 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Miguel Arruga Vivas <rosen644835 <at> gmail.com>
To: 37851 <at> debbugs.gnu.org
Subject: Re: Grub installation only checks for encrypted /boot folder
Date: Mon, 21 Oct 2019 14:47:58 +0200

[Message part 1 (text/plain, inline)]

Hi again,

Attached can be found a workaround to mount all encrypted partitions.
There is no way to tell the devices to mount without changing
boot-parameters, where I'd add another field with the needed mapped
devices (a traversal onto the mapped-device dependency tree
of /gnu/store).  Do you think this is a good idea?  At least I think
it's the best way to encode the dependencies into the grub.cfg file,
even though the typical graph will contain 0 or 1 nodes.

Ideas?

Best regards,
Miguel

[0001-system-Mount-luks-devices-on-boot.patch (text/x-patch, attachment)]

Message #11 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Miguel Arruga Vivas <rosen644835 <at> gmail.com>
To: 37851 <at> debbugs.gnu.org
Subject: bug#37851
Date: Mon, 21 Oct 2019 16:55:36 +0200

merge 25305 37851
quit

Message #16 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Miguel Arruga Vivas <rosen644835 <at> gmail.com>
Cc: 37851 <at> debbugs.gnu.org
Subject: Re: bug#37851: Grub installation only checks for encrypted /boot
 folder
Date: Tue, 22 Oct 2019 16:12:49 +0200

Hola Miguel,

Miguel Arruga Vivas <rosen644835 <at> gmail.com> skribis:

> Attached can be found a workaround to mount all encrypted partitions.
> There is no way to tell the devices to mount without changing
> boot-parameters, where I'd add another field with the needed mapped
> devices (a traversal onto the mapped-device dependency tree
> of /gnu/store).  Do you think this is a good idea?  At least I think
> it's the best way to encode the dependencies into the grub.cfg file,
> even though the typical graph will contain 0 or 1 nodes.

> From 9b50e2d8eb8b744595a54a9543993eb4e3813742 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
>  <rosen644835 <at> gmail.com>
> Date: Mon, 21 Oct 2019 14:35:02 +0200
> Subject: [PATCH] system: Mount luks devices on boot.
>
> * gnu/bootloader/grub.scm (grub-configuration-file)[builder]: Mount all
> encrypted partitions.
> ---
>  gnu/bootloader/grub.scm | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
> index d984d5f5e3..b29477ec71 100644
> --- a/gnu/bootloader/grub.scm
> +++ b/gnu/bootloader/grub.scm
> @@ -369,6 +369,7 @@ keymap ~a~%" keymap)))))
>            (format port
>                    "# This file was generated from your Guix configuration.  Any changes
>  # will be lost upon reconfiguration.
> +cryptomount -a

Does that cause GRUB to mount all the LUKS partitions it was aware of at
installation time, or does it cause it to scan all the partitions in
search of a LUKS signature?

In the latter case that wouldn’t be great, but in the former case it
sounds like we could go ahead (well, with a comment above explaining
what this does.  :-)).

Thanks for working on it!

Ludo’.

Message #19 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Miguel Arruga Vivas <rosen644835 <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 37851 <at> debbugs.gnu.org
Subject: Re: bug#37851: Grub installation only checks for encrypted /boot
 folder
Date: Sun, 27 Oct 2019 02:00:31 +0100

[Message part 1 (text/plain, inline)]

Hi Ludo’,

El Tue, 22 Oct 2019 16:12:49 +0200
Ludovic Courtès <ludo <at> gnu.org> escribió:
> Hola Miguel,
> 
> Miguel Arruga Vivas <rosen644835 <at> gmail.com> skribis:
> > (...)
> > +cryptomount -a  
> 
> Does that cause GRUB to mount all the LUKS partitions it was aware of
> at installation time, or does it cause it to scan all the partitions
> in search of a LUKS signature?

That patch is the first one, it mounts everything it can find, unlike
this one.

The only option I've seen was to modify boot-parameters (as in #35394,
wink wink nudge nudge) in order to store the needed partitions.  I've
reduced it this time to one patch, is it somehow easier to read this
way?  I could split it in two stages (one add the boot-parameters
field, the other one to make use of it) or squash the three for the
other feature into one if that easier for the review.  The main issues
I've found is that the source of the device-mappings needed for boot up
has to be declared by the UUID to ensure they are not system
dependent.  Also, the warning is shown several times and the message
isn't quite good, any idea how to fix/improve this?

Happy hacking!
Miguel

[0001-system-Use-of-mapped-devices-for-boot-process.patch (text/x-patch, attachment)]

Message #24 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Mathieu Othacehe
 <othacehe <at> gnu.org>
Cc: 25305 <at> debbugs.gnu.org, 37851 <at> debbugs.gnu.org
Subject: Re: bug#37851: Grub installation only checks for encrypted /boot
 folder
Date: Mon, 26 Oct 2020 23:15:03 +0100

[Message part 1 (text/plain, inline)]

Hello!

Ludovic Courtès <ludo <at> gnu.org> writes:
> Does that cause GRUB to mount all the LUKS partitions it was aware of at
> installation time, or does it cause it to scan all the partitions in
> search of a LUKS signature?
>
> In the latter case that wouldn’t be great, but in the former case it
> sounds like we could go ahead (well, with a comment above explaining
> what this does.  :-)).

Sorry for this huuuuuuuuuge delay, but I have this patch for this.  It
includes a test case, even though I have been suffering a lot until I
noticed that OCR was returning garbage and I was trying to be too
specific, so I've left it as basic as I could.

I add Mathieu to the loop to bring more eyes over it, I'm open to any
suggestion. :-)

Happy hacking!
Miguel

PS: It should apply on top of master too, but I tested it on top of some
other grub.cfg fixes, I'll send a new version if there is any problem
with this.

[v3-0005-system-Allow-separated-boot-and-encrypted-root.patch (text/x-patch, attachment)]

[signature.asc (application/pgp-signature, inline)]

Message #27 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 25305 <at> debbugs.gnu.org, Mathieu Othacehe <othacehe <at> gnu.org>,
 37851 <at> debbugs.gnu.org
Subject: Re: bug#25305: bug#37851: Grub installation only checks for
 encrypted /boot folder
Date: Wed, 28 Oct 2020 22:42:19 +0100

[Message part 1 (text/plain, inline)]

In this v2 I've fixed two flaws I saw in the previous patch: the
parameter store-crypto-devices now has the empty list as default value,
and now the function documents for the parameter too.

Happy hacking!
Miguel

[v4-0005-system-Allow-separated-boot-and-encrypted-root.patch (text/x-patch, attachment)]

Message #30 received at 37851 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
Cc: 25305 <at> debbugs.gnu.org, Mathieu Othacehe <othacehe <at> gnu.org>,
 37851 <at> debbugs.gnu.org
Subject: Re: bug#37851: bug#25305: bug#37851: Grub installation only checks
 for encrypted /boot folder
Date: Mon, 14 Dec 2020 14:11:37 +0100

Hi Miguel,

Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com> skribis:

>>From 52993db19da43699ea96ea16ebb051b9652934f9 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
>  <rosen644835 <at> gmail.com>
> Date: Sun, 25 Oct 2020 16:31:17 +0100
> Subject: [PATCH v4 5/5] system: Allow separated /boot and encrypted root.
>
> * gnu/bootloader/grub.scm (grub-configuration-file): New parameter
> store-crypto-devices.
> [crypto-devices]: New helper function.
> [builder]: Use crypto-devices.
> * gnu/machine/ssh.scm (roll-back-managed-host): Use
> boot-parameters-store-crypto-devices to provide its contents to the
> bootloader configuration generation process.
> * gnu/tests/install.scm (%encrypted-root-not-boot-os,
> %encrypted-root-not-boot-os): New os declaration.
> (%encrypted-root-not-boot-installation-script): New script, whose contents
> were initially taken from %encrypted-root-installation-script.
> (%test-encrypted-root-not-boot-os): New test.
> * gnu/system.scm (define-module): Export
> operating-system-bootoader-crypto-devices and
> boot-parameters-store-crypto-devices.
> (<boot-parameters>): Add field store-crypto-devices.
> (read-boot-parameters): Parse store-crypto-devices field.
> [uuid-sexp->uuid]: New helper function extracted from
> device-sexp->device.
> (operating-system-bootloader-crypto-devices): New function.
> (operating-system-bootcfg): Use
> operating-system-bootloader-crypto-devices to provide its contents to
> the bootloader configuration generation process.
> (operating-system-boot-parameters): Add store-crypto-devices to the
> generated boot-parameters.
> (operating-system-boot-parameters-file): Likewise to the file with
> the serialized structure.
> * guix/scripts/system.scm (reinstall-bootloader): Use
> boot-parameters-store-crypto-devices to provide its contents to the
> bootloader configuration generation process.
> * tests/boot-parameters.scm (%default-store-crypto-devices): New
> variable.
> (%grub-boot-parameters, test-read-boot-parameters): Use
> %default-store-crypto-devices.
> (tests store-crypto-devices): New tests.
> ---
>  gnu/bootloader/grub.scm   |  21 +++++++-
>  gnu/machine/ssh.scm       |   3 ++
>  gnu/system.scm            |  57 ++++++++++++++++++++-
>  gnu/tests/install.scm     | 103 ++++++++++++++++++++++++++++++++++++++
>  guix/scripts/system.scm   |   2 +
>  tests/boot-parameters.scm |  29 ++++++++++-
>  6 files changed, 210 insertions(+), 5 deletions(-)

Woohoo!

> --- a/gnu/bootloader/grub.scm
> +++ b/gnu/bootloader/grub.scm
> @@ -4,7 +4,7 @@
>  ;;; Copyright © 2017 Leo Famulari <leo <at> famulari.name>
>  ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe <at> gmail.com>
>  ;;; Copyright © 2019, 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
> -;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
> +;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
>  ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
>  ;;; Copyright © 2020 Stefan <stefan-guix <at> vodafonemail.de>
>  ;;;
> @@ -360,11 +360,14 @@ code."
>                                    (locale #f)
>                                    (system (%current-system))
>                                    (old-entries '())
> +                                  (store-crypto-devices '())
>                                    store-directory-prefix)
>    "Return the GRUB configuration file corresponding to CONFIG, a
>  <bootloader-configuration> object, and where the store is available at
>  STORE-FS, a <file-system> object.  OLD-ENTRIES is taken to be a list of menu
>  entries corresponding to old generations of the system.
> +STORE-CRYPTO-DEVICES contain the UUIDs of the encrypted units that must
> +be unlocked to access the store contents.
>  STORE-DIRECTORY-PREFIX may be used to specify a store prefix, as is required
>  when booting a root file system on a Btrfs subvolume."
>    (define all-entries
> @@ -412,6 +415,21 @@ menuentry ~s {
>                    (string-join (map string-join '#$modules)
>                                 "\n  module " 'prefix))))))
>  
> +  (define (crypto-devices)
> +    (define (crypto-device->cryptomount dev)
> +      (if (uuid? dev)
> +          #~(format port "cryptomount -u ~a~%"
> +                    ;; cryptomount only accepts UUID without the hypen.
> +                    #$(string-delete #\- (uuid->string dev)))
> +          ;; Other type of devices aren't implemented.
> +          #~()))
> +    (let ((devices (map crypto-device->cryptomount store-crypto-devices))
> +          ;; XXX: Add luks2 when grub 2.06 is packaged.
> +          (modules #~(format port "insmod luks~%")))
> +      (if (null? devices)
> +          devices
> +          (cons modules devices))))

What I don’t get is why we’re able to use an encrypted root right now
without emitting “cryptomount” GRUB commands?

> +      (store-crypto-devices
> +       (match (assq 'store rest)
> +         (('store . store-data)
> +          (match (assq 'crypto-devices store-data)
> +            (('crypto-devices devices)
> +             (if (list? devices)
> +                 (map uuid-sexp->uuid devices)
> +                 (begin
> +                   (warning (G_ "unrecognized crypto-device ~S at '~a'~%")
> +                            devices (port-filename port))
> +                   '())))

You could avoid ‘if’ by having clauses like:

  (('crypto-devices (devices ...))
   ;; …
   )
  (('crypto-devices _)
   (warning …)
   '())
  (_
   '())

> +            (_
> +             ;; No crypto-devices found
> +             '())))
> +         (_
> +          ;; No store found, old format.
> +          '())))

s/No store found/No crypto devices found/ ?

> +(define (operating-system-bootloader-crypto-devices os)
> +  "Return the subset of mapped devices that the bootloader must open.
> +Only devices specified by uuid are supported."
> +  (map mapped-device-source
> +       (filter (match-lambda
> +                 ((and (= mapped-device-type type)
> +                       (= mapped-device-source source))
> +                  (and (eq? luks-device-mapping type)
> +                       (or (uuid? source)
> +                           (begin
> +                             (warning (G_ "\
> +mapped-device '~a' won't be mounted by the bootloader.~%")
> +                                      source)
> +                             #f)))))
> +               ;; XXX: Ordering is important, we trust the returned one.
> +               (operating-system-boot-mapped-devices os))))

You can use ‘filter-map’ here.

The rest LGTM!  Make sure the “installed-os” and “encrypted-root-os”
system tests are still fine, and if they are, I guess you can go ahead.

Thanks!

Ludo’.

Message #33 received at 37851-done <at> debbugs.gnu.org (full text, mbox):

From: Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 25305-done <at> debbugs.gnu.org, Mathieu Othacehe <othacehe <at> gnu.org>,
 37851-done <at> debbugs.gnu.org
Subject: Re: bug#37851: bug#25305: bug#37851: Grub installation only checks
 for encrypted /boot folder
Date: Mon, 21 Dec 2020 21:23:36 +0100

Hi Ludo,

First of all, thanks for your review. :-)

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Miguel,
>
> Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com> skribis:
>> +  (define (crypto-devices)
>> +    (define (crypto-device->cryptomount dev)
>> +      (if (uuid? dev)
>> +          #~(format port "cryptomount -u ~a~%"
>> +                    ;; cryptomount only accepts UUID without the hypen.
>> +                    #$(string-delete #\- (uuid->string dev)))
>> +          ;; Other type of devices aren't implemented.
>> +          #~()))
>> +    (let ((devices (map crypto-device->cryptomount store-crypto-devices))
>> +          ;; XXX: Add luks2 when grub 2.06 is packaged.
>> +          (modules #~(format port "insmod luks~%")))
>> +      (if (null? devices)
>> +          devices
>> +          (cons modules devices))))
>
> What I don’t get is why we’re able to use an encrypted root right now
> without emitting “cryptomount” GRUB commands?

The grub boot process goes more or less like this:

1. Firmware loads the initial image.
1.1. If that image is not the final one, it contains a "pointer" to the
     final one, which is loaded by it; this chain can be viewed as part
     of the firmware loading for this purpose.
2. The image code reads an initial configuration file, which is usually
   generated by grub-install/grub-mkstandalone.  Here Grub is placing
   the needed the cryptomount lines for the devices needed to mount
   target in order to read grub.cfg and other modules.
3. grub.cfg is read (a.k.a. normal mode) and the usual boot process
   follows.

The first configuration file is generated automatically by grub-install,
which physically scans the target location (still /boot in our case) and
inserts the needed insmod and cryptomount calls.  When the target and
the store don't share the device, the calls leading to the store must be
inserted manually into grub.cfg.

It could be easier to remove completely /boot and use a directory from
the store, but that leads to more writes of the image, as each
reconfiguration involving a change on the devices used for the store
must end up returning a different store file name too.  Nonetheless,
that would leave /boot untouched if anybody wants to install their
version of grub there for other purposes...

>> +            (_
>> +             ;; No crypto-devices found
>> +             '())))
>> +         (_
>> +          ;; No store found, old format.
>> +          '())))
>
> s/No store found/No crypto devices found/ ?

The first comment is reached when crypto-devices isn't found in a
(boot-parameters ... (store ...) ...) form.  The second one is reached
when (boot-parameters ...) form doesn't even contain a tag store in it.
It follows the same pattern as store-device, as the old format didn't
have a store element.

On the other hand, I added a period to the first sentence as it was
missing. 0:)

>> +(define (operating-system-bootloader-crypto-devices os)
>> +  "Return the subset of mapped devices that the bootloader must open.
>> +Only devices specified by uuid are supported."
>> +  (map mapped-device-source
>> +       (filter (match-lambda
>> +                 ((and (= mapped-device-type type)
>> +                       (= mapped-device-source source))
>> +                  (and (eq? luks-device-mapping type)
>> +                       (or (uuid? source)
>> +                           (begin
>> +                             (warning (G_ "\
>> +mapped-device '~a' won't be mounted by the bootloader.~%")
>> +                                      source)
>> +                             #f)))))
>> +               ;; XXX: Ordering is important, we trust the returned one.
>> +               (operating-system-boot-mapped-devices os))))
>
> You can use ‘filter-map’ here.

Thanks for the pointer!  I've modified a bit tests/boot-parameters.scm
to be extra-sure that I was doing that change OK, as I moved the or to a
internal function for readability too.

> The rest LGTM!  Make sure the “installed-os” and “encrypted-root-os”
> system tests are still fine, and if they are, I guess you can go ahead.

Pushed to master as f00e68ace0 with these changes, after running the
tests and booting up my system.

Happy hacking!
Miguel

Message #36 received at 37851-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com>
Cc: 25305-done <at> debbugs.gnu.org, Mathieu Othacehe <othacehe <at> gnu.org>,
 37851-done <at> debbugs.gnu.org
Subject: Re: bug#37851: bug#25305: bug#37851: Grub installation only checks
 for encrypted /boot folder
Date: Tue, 22 Dec 2020 14:41:07 +0100

Hi,

Miguel Ángel Arruga Vivas <rosen644835 <at> gmail.com> skribis:

> Pushed to master as f00e68ace0 with these changes, after running the
> tests and booting up my system.

Woohoo, thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.