unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#37851: Grub installation only checks for encrypted /boot folder
@ 2019-10-21 11:07 Miguel Arruga Vivas
  2019-10-21 12:47 ` Miguel Arruga Vivas
  2019-10-21 14:55 ` bug#37851: Miguel Arruga Vivas
  0 siblings, 2 replies; 10+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-21 11:07 UTC (permalink / raw)
  To: 37851

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 ====================

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#37851: Grub installation only checks for encrypted /boot folder
  2019-10-21 11:07 bug#37851: Grub installation only checks for encrypted /boot folder Miguel Arruga Vivas
@ 2019-10-21 12:47 ` Miguel Arruga Vivas
  2019-10-22 14:12   ` Ludovic Courtès
  2019-10-21 14:55 ` bug#37851: Miguel Arruga Vivas
  1 sibling, 1 reply; 10+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-21 12:47 UTC (permalink / raw)
  To: 37851

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

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

[-- Attachment #2: 0001-system-Mount-luks-devices-on-boot.patch --]
[-- Type: text/x-patch, Size: 847 bytes --]

From 9b50e2d8eb8b744595a54a9543993eb4e3813742 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@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
 ")
           #$sugar
           #$keyboard-layout-config
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#37851:
  2019-10-21 11:07 bug#37851: Grub installation only checks for encrypted /boot folder Miguel Arruga Vivas
  2019-10-21 12:47 ` Miguel Arruga Vivas
@ 2019-10-21 14:55 ` Miguel Arruga Vivas
  1 sibling, 0 replies; 10+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-21 14:55 UTC (permalink / raw)
  To: 37851

merge 25305 37851
quit

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#37851: Grub installation only checks for encrypted /boot folder
  2019-10-21 12:47 ` Miguel Arruga Vivas
@ 2019-10-22 14:12   ` Ludovic Courtès
  2019-10-27  1:00     ` Miguel Arruga Vivas
  2020-10-26 22:15     ` bug#25305: " Miguel Ángel Arruga Vivas
  0 siblings, 2 replies; 10+ messages in thread
From: Ludovic Courtès @ 2019-10-22 14:12 UTC (permalink / raw)
  To: Miguel Arruga Vivas; +Cc: 37851

Hola Miguel,

Miguel Arruga Vivas <rosen644835@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@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’.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#37851: Grub installation only checks for encrypted /boot folder
  2019-10-22 14:12   ` Ludovic Courtès
@ 2019-10-27  1:00     ` Miguel Arruga Vivas
  2020-10-26 22:15     ` bug#25305: " Miguel Ángel Arruga Vivas
  1 sibling, 0 replies; 10+ messages in thread
From: Miguel Arruga Vivas @ 2019-10-27  1:00 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 37851

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

Hi Ludo’,

El Tue, 22 Oct 2019 16:12:49 +0200
Ludovic Courtès <ludo@gnu.org> escribió:
> Hola Miguel,
> 
> Miguel Arruga Vivas <rosen644835@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-system-Use-of-mapped-devices-for-boot-process.patch --]
[-- Type: text/x-patch, Size: 12753 bytes --]

From f6438d1175a1d60d842ab502255a7685b05f4e7d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@gmail.com>
Date: Sun, 27 Oct 2019 01:35:59 +0200
Subject: [PATCH] system: Use of mapped-devices for boot process.

* gnu/bootloader/depthcharge.scm (depthcharge-configuration-file): New
parameter crypto-devices, not used.
* gnu/bootloader/extlinux.scm (extlinux-configuration-file): Likewise.
* gnu/bootloader/grub.scm (grub-configuration-file)[declaration]: New
parameter crypto-devices, used to ensure unlock every encrypted
partition needed by the bootloader.
[device-uuid->gexp]: New function, emits cryptomount calls.
[body]: Map crypto-devices with device-uuid->gexp.
* gnu/machine/ssh.scm (roll-back-managed-host): Use the crypto-devices
stored from the selected generation in the call to the bootloader
configuration generator.
* gnu/scripts/system.scm (reinstall-bootloader): Likewise.
* gnu/system.scm (define-module)[export]: Export new accessor
boot-parameters-crypto-devices.
(boot-parameters)[crypto-devices]: New field.
(read-boot-parameters)[uuid-sexp->uuid]: New function.
(read-boot-parameters)[body]: Read new field crypto-devices.
(operating-system-boot-parameters-file): Add the new field.
(operating-system-boot-crypto-devices): New function.  Warn about
devices without an UUID.  They are ignored as they would be dependant
on the hardware configuration.
(operating-system-bootcfg): Use operating-system-boot-crypto-devices in
the call to the bootloader configuration generator.
(operating-system-boot-parameters): Use
operating-system-boot-crypto-devices to store the needed devices.
---
 gnu/bootloader/depthcharge.scm |  1 +
 gnu/bootloader/extlinux.scm    |  1 +
 gnu/bootloader/grub.scm        | 14 ++++++++++++
 gnu/machine/ssh.scm            |  3 +++
 gnu/system.scm                 | 40 ++++++++++++++++++++++++++++++++++
 guix/scripts/system.scm        |  2 ++
 6 files changed, 61 insertions(+)

diff --git a/gnu/bootloader/depthcharge.scm b/gnu/bootloader/depthcharge.scm
index 58cc3f3932..fe4302e93c 100644
--- a/gnu/bootloader/depthcharge.scm
+++ b/gnu/bootloader/depthcharge.scm
@@ -82,6 +82,7 @@
 (define* (depthcharge-configuration-file config entries
                                          #:key
                                          (system (%current-system))
+                                         (crypto-devices '())
                                          (old-entries '()))
   (match entries
     ((entry)
diff --git a/gnu/bootloader/extlinux.scm b/gnu/bootloader/extlinux.scm
index 40108584a8..3defeab3dd 100644
--- a/gnu/bootloader/extlinux.scm
+++ b/gnu/bootloader/extlinux.scm
@@ -28,6 +28,7 @@
 (define* (extlinux-configuration-file config entries
                                       #:key
                                       (system (%current-system))
+                                      (crypto-devices '())
                                       (old-entries '()))
   "Return the U-Boot configuration file corresponding to CONFIG, a
 <u-boot-configuration> object, and where the store is available at STORE-FS, a
diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index d984d5f5e3..8b5cf848af 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -316,6 +317,7 @@ code."
 (define* (grub-configuration-file config entries
                                   #:key
                                   (system (%current-system))
+                                  (crypto-devices '())
                                   (old-entries '()))
   "Return the GRUB configuration file corresponding to CONFIG, a
 <bootloader-configuration> object, and where the store is available at
@@ -345,6 +347,17 @@ entries corresponding to old generations of the system."
                   #$(grub-root-search device kernel)
                   #$kernel (string-join (list #$@arguments))
                   #$initrd))))
+  (define (device-uuid->gexp device-uuid)
+    (let* ((uuid-string (uuid->string device-uuid))
+           ;; XXX: My tests only worked with UUID values without
+           ;; any hyphen character.
+           (filtered-uuid (string-filter (lambda (c)
+                                           (not (eqv? c #\-)))
+                                         uuid-string)))
+      #~(format port "# Unlock encrypted device ~a
+cryptomount -u ~a~%"
+                #$uuid-string
+                #$filtered-uuid)))
   (define sugar
     (eye-candy config
                (menu-entry-device (first all-entries))
@@ -370,6 +383,7 @@ keymap ~a~%" keymap)))))
                   "# This file was generated from your Guix configuration.  Any changes
 # will be lost upon reconfiguration.
 ")
+          #$@(map device-uuid->gexp crypto-devices)
           #$sugar
           #$keyboard-layout-config
           (format port "
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index 6e3ed0e092..e8750bbe81 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -435,11 +435,14 @@ an environment type of 'managed-host."
                                             (drop boot-parameters 2)))
                        (bootloader -> (operating-system-bootloader
                                        (machine-operating-system machine)))
+                       (crypto-devices -> (boot-parameters-crypto-devices
+                                           (second boot-parameters)))
                        (bootcfg (lower-object
                                  ((bootloader-configuration-file-generator
                                    (bootloader-configuration-bootloader
                                     bootloader))
                                   bootloader entries
+                                  #:crypto-devices crypto-devices
                                   #:old-entries old-entries)))
                        (remote-result (machine-remote-eval machine remote-exp)))
     (when (eqv? 'error remote-result)
diff --git a/gnu/system.scm b/gnu/system.scm
index a353b1a5c8..9835fddf89 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Meiyo Peng <meiyo.peng@gmail.com>
+;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -119,6 +120,7 @@
             boot-parameters-bootloader-menu-entries
             boot-parameters-store-device
             boot-parameters-store-mount-point
+            boot-parameters-crypto-devices
             boot-parameters-kernel
             boot-parameters-kernel-arguments
             boot-parameters-initrd
@@ -256,6 +258,7 @@ directly by the user."
    boot-parameters-bootloader-menu-entries)
   (store-device     boot-parameters-store-device)
   (store-mount-point boot-parameters-store-mount-point)
+  (crypto-devices   boot-parameters-crypto-devices)
   (kernel           boot-parameters-kernel)
   (kernel-arguments boot-parameters-kernel-arguments)
   (initrd           boot-parameters-initrd))
@@ -286,6 +289,14 @@ file system labels."
            device
            (file-system-label device)))))
 
+  (define uuid-sexp->uuid
+    (match-lambda
+      (('uuid (? symbol? type) (? bytevector? bv))
+       (bytevector->uuid bv type))
+      (x
+       (warning (G_ "unrecognized uuid ~a at '~a'~%") x (port-filename port))
+       #f)))
+
   (match (read port)
     (('boot-parameters ('version 0)
                        ('label label) ('root-device root)
@@ -324,6 +335,11 @@ file system labels."
          (('initrd (? string? file))
           file)))
 
+      (crypto-devices
+       (match (assq 'crypto-devices rest)
+         ((_ device-list) (map uuid-sexp->uuid device-list))
+         (#f              '())))
+
       (store-device
        ;; Linux device names like "/dev/sda1" are not suitable GRUB device
        ;; identifiers, so we just filter them out.
@@ -438,6 +454,23 @@ from the initrd."
                (any file-system-needed-for-boot? users)))
            devices)))
 
+(define (operating-system-boot-crypto-devices os)
+  (define (crypto-device? device)
+    (let ((type (mapped-device-type device)))
+      (eq? type luks-device-mapping)))
+  (define (with-uuid? device)
+    (if (uuid? (mapped-device-source device))
+        #t
+        (begin
+          (warning (G_ "the source from mapped-device at ~a is not an UUID.
+It will be ignored for the bootloader configuration.~%")
+                   (mapped-device-location device))
+          #f)))
+  (let* ((mapped-devices (operating-system-boot-mapped-devices os))
+         (crypto-devices (filter crypto-device? mapped-devices))
+         (valid-devices (filter with-uuid? crypto-devices)))
+    (map mapped-device-source valid-devices)))
+
 (define (device-mapping-services os)
   "Return the list of device-mapping services for OS as a list."
   (map device-mapping-service
@@ -989,6 +1022,7 @@ entry."
 a list of <menu-entry>, to populate the \"old entries\" menu."
   (let* ((root-fs         (operating-system-root-file-system os))
          (root-device     (file-system-device root-fs))
+         (crypto-devices  (operating-system-boot-crypto-devices os))
          (params          (operating-system-boot-parameters
                            os root-device
                            #:system-kernel-arguments? #t))
@@ -999,6 +1033,7 @@ a list of <menu-entry>, to populate the \"old entries\" menu."
        (bootloader-configuration-bootloader bootloader-conf)))
 
     (generate-config-file bootloader-conf (list entry)
+                          #:crypto-devices crypto-devices
                           #:old-entries old-entries)))
 
 (define* (operating-system-boot-parameters os root-device
@@ -1011,6 +1046,7 @@ such as '--root' and '--load' to <boot-parameters>."
          (bootloader      (bootloader-configuration-bootloader
                            (operating-system-bootloader os)))
          (bootloader-name (bootloader-name bootloader))
+         (crypto-devices  (operating-system-boot-crypto-devices os))
          (label           (operating-system-label os)))
     (boot-parameters
      (label label)
@@ -1024,6 +1060,7 @@ such as '--root' and '--load' to <boot-parameters>."
      (bootloader-name bootloader-name)
      (bootloader-menu-entries
       (bootloader-configuration-menu-entries (operating-system-bootloader os)))
+     (crypto-devices crypto-devices)
      (store-device (ensure-not-/dev (file-system-device store)))
      (store-mount-point (file-system-mount-point store)))))
 
@@ -1070,6 +1107,9 @@ being stored into the \"parameters\" file)."
                             (or (and=> (operating-system-bootloader os)
                                        bootloader-configuration-menu-entries)
                                 '())))
+                    (crypto-devices
+                     #$(map device->sexp
+                            (boot-parameters-crypto-devices params)))
                     (store
                      (device
                       #$(device->sexp (boot-parameters-store-device params)))
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index 27b014db68..95cffec52d 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -392,12 +392,14 @@ STORE is an open connection to the store."
                        %system-profile old-generations))
          (entries (cons (boot-parameters->menu-entry params)
                         (boot-parameters-bootloader-menu-entries params)))
+         (crypto-devices (boot-parameters-crypto-devices params))
          (old-entries (map boot-parameters->menu-entry old-params)))
     (run-with-store store
       (mlet* %store-monad
           ((bootcfg (lower-object
                      ((bootloader-configuration-file-generator bootloader)
                       bootloader-config entries
+                      #:crypto-devices crypto-devices
                       #:old-entries old-entries)))
            (drvs -> (list bootcfg)))
         (mbegin %store-monad
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#25305: bug#37851: Grub installation only checks for encrypted /boot folder
  2019-10-22 14:12   ` Ludovic Courtès
  2019-10-27  1:00     ` Miguel Arruga Vivas
@ 2020-10-26 22:15     ` Miguel Ángel Arruga Vivas
  2020-10-28 21:42       ` Miguel Ángel Arruga Vivas
  1 sibling, 1 reply; 10+ messages in thread
From: Miguel Ángel Arruga Vivas @ 2020-10-26 22:15 UTC (permalink / raw)
  To: Ludovic Courtès, Mathieu Othacehe; +Cc: 25305, 37851


[-- Attachment #1.1: Type: text/plain, Size: 942 bytes --]

Hello!

Ludovic Courtès <ludo@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.


[-- Attachment #1.2: 0001-system-Allow-separated-boot-and-encrypted-root.patch --]
[-- Type: text/x-patch, Size: 22074 bytes --]

From d40f0a26afef194e7e68906ba793ca0ffac6da5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@gmail.com>
Date: Sun, 25 Oct 2020 16:31:17 +0100
Subject: [PATCH v3 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   |  19 ++++++-
 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, 208 insertions(+), 5 deletions(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index 8636e9c690..c6e7d3fd6d 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019, 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
-;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
+;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2020 Stefan <stefan-guix@vodafonemail.de>
 ;;;
@@ -361,6 +361,7 @@ 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
@@ -413,6 +414,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))))
+
   (define (sugar)
     (let* ((entry (first all-entries))
            (device (menu-entry-device entry))
@@ -469,6 +485,7 @@ keymap ~a~%" #$keymap))))
                   "# This file was generated from your Guix configuration.  Any changes
 # will be lost upon reconfiguration.
 ")
+          #$@(crypto-devices)
           #$(sugar)
           #$locale-config
           #$keyboard-layout-config
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index a3a12fb54b..822f401c1a 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -482,6 +482,8 @@ an environment type of 'managed-host."
                                         (list (second boot-parameters))))
                        (locale -> (boot-parameters-locale
                                    (second boot-parameters)))
+                       (crypto-dev -> (boot-parameters-store-crypto-devices
+                                      (second boot-parameters)))
                        (store-dir -> (boot-parameters-store-directory-prefix
                                       (second boot-parameters)))
                        (old-entries -> (map boot-parameters->menu-entry
@@ -494,6 +496,7 @@ an environment type of 'managed-host."
                                     bootloader))
                                   bootloader entries
                                   #:locale locale
+                                  #:store-crypto-devices crypto-dev
                                   #:store-directory-prefix store-dir
                                   #:old-entries old-entries)))
                        (remote-result (machine-remote-eval machine remote-exp)))
diff --git a/gnu/system.scm b/gnu/system.scm
index 30a5c418d0..3a718642cf 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -5,7 +5,7 @@
 ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Meiyo Peng <meiyo.peng@gmail.com>
-;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
+;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
 ;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian@pelzflorian.de>
@@ -112,6 +112,7 @@
             operating-system-store-file-system
             operating-system-user-mapped-devices
             operating-system-boot-mapped-devices
+            operating-system-bootloader-crypto-devices
             operating-system-activation-script
             operating-system-user-accounts
             operating-system-shepherd-service-names
@@ -147,6 +148,7 @@
             boot-parameters-root-device
             boot-parameters-bootloader-name
             boot-parameters-bootloader-menu-entries
+            boot-parameters-store-crypto-devices
             boot-parameters-store-device
             boot-parameters-store-directory-prefix
             boot-parameters-store-mount-point
@@ -301,6 +303,8 @@ directly by the user."
   (store-device     boot-parameters-store-device)
   (store-mount-point boot-parameters-store-mount-point)
   (store-directory-prefix boot-parameters-store-directory-prefix)
+  (store-crypto-devices boot-parameters-store-crypto-devices
+                        (default '()))
   (locale           boot-parameters-locale)
   (kernel           boot-parameters-kernel)
   (kernel-arguments boot-parameters-kernel-arguments)
@@ -334,6 +338,13 @@ file system labels."
            (if (string-prefix? "/" device)
                device
                (file-system-label device))))))
+  (define uuid-sexp->uuid
+    (match-lambda
+      (('uuid (? symbol? type) (? bytevector? bv))
+       (bytevector->uuid bv type))
+      (x
+       (warning (G_ "unrecognized uuid ~a at '~a'~%") x (port-filename port))
+       #f)))
 
   (match (read port)
     (('boot-parameters ('version 0)
@@ -407,6 +418,24 @@ file system labels."
           ;; No store found, old format.
           #f)))
 
+      (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))
+                   '())))
+            (_
+             ;; No crypto-devices found
+             '())))
+         (_
+          ;; No store found, old format.
+          '())))
+
       (store-mount-point
        (match (assq 'store rest)
          (('store ('device _) ('mount-point mount-point) _ ...)
@@ -520,6 +549,23 @@ from the initrd."
                (any file-system-needed-for-boot? users)))
            devices)))
 
+(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))))
+
 (define (device-mapping-services os)
   "Return the list of device-mapping services for OS as a list."
   (map device-mapping-service
@@ -1256,6 +1302,7 @@ a list of <menu-entry>, to populate the \"old entries\" menu."
          (root-fs         (operating-system-root-file-system os))
          (root-device     (file-system-device root-fs))
          (locale          (operating-system-locale os))
+         (crypto-devices  (operating-system-bootloader-crypto-devices os))
          (params          (operating-system-boot-parameters
                            os root-device
                            #:system-kernel-arguments? #t))
@@ -1269,6 +1316,7 @@ a list of <menu-entry>, to populate the \"old entries\" menu."
     (generate-config-file bootloader-conf (list entry)
                           #:old-entries old-entries
                           #:locale locale
+                          #:store-crypto-devices crypto-devices
                           #:store-directory-prefix
 			  (btrfs-store-subvolume-file-name file-systems))))
 
@@ -1308,6 +1356,7 @@ such as '--root' and '--load' to <boot-parameters>."
                                (operating-system-initrd-file os)))
          (store           (operating-system-store-file-system os))
          (file-systems    (operating-system-file-systems os))
+         (crypto-devices  (operating-system-bootloader-crypto-devices os))
          (locale          (operating-system-locale os))
          (bootloader      (bootloader-configuration-bootloader
                            (operating-system-bootloader os)))
@@ -1330,6 +1379,7 @@ such as '--root' and '--load' to <boot-parameters>."
      (locale locale)
      (store-device (ensure-not-/dev (file-system-device store)))
      (store-directory-prefix (btrfs-store-subvolume-file-name file-systems))
+     (store-crypto-devices crypto-devices)
      (store-mount-point (file-system-mount-point store)))))
 
 (define (device->sexp device)
@@ -1388,7 +1438,10 @@ being stored into the \"parameters\" file)."
                       (mount-point #$(boot-parameters-store-mount-point
                                       params))
                       (directory-prefix
-                       #$(boot-parameters-store-directory-prefix params))))
+                       #$(boot-parameters-store-directory-prefix params))
+                      (crypto-devices
+                       #$(map device->sexp
+                              (boot-parameters-store-crypto-devices params)))))
                   #:set-load-path? #f)))
 
 (define-gexp-compiler (operating-system-compiler (os <operating-system>)
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 86bd93966b..8f1668bab2 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -63,6 +63,8 @@
             %test-separate-home-os
             %test-raid-root-os
             %test-encrypted-root-os
+            %test-encrypted-root-not-boot-os
+            %test-encrypted-root-and-boot-os
             %test-btrfs-root-os
             %test-btrfs-root-on-subvolume-os
             %test-jfs-root-os
@@ -796,6 +798,107 @@ build (current-guix) and then store a couple of full system images.")
       (run-basic-test %encrypted-root-os command "encrypted-root-os"
                       #:initialization enter-luks-passphrase)))))
 
+\f
+;;;
+;;; LUKS-encrypted root file system and /boot in a non-encrypted partition.
+;;;
+
+(define-os-with-source (%encrypted-root-not-boot-os
+                        %encrypted-root-not-boot-os-source)
+  ;; The OS we want to install.
+  (use-modules (gnu) (gnu tests) (srfi srfi-1))
+
+  (operating-system
+    (host-name "bootroot")
+    (timezone "Europe/Madrid")
+    (locale "en_US.UTF-8")
+
+    (bootloader (bootloader-configuration
+                 (bootloader grub-bootloader)
+                 (target "/dev/vdb")))
+
+    (mapped-devices (list (mapped-device
+                           (source
+                            (uuid "12345678-1234-1234-1234-123456789abc"))
+                           (target "root")
+                           (type luks-device-mapping))))
+    (file-systems (cons* (file-system
+                           (device (file-system-label "my-boot"))
+                           (mount-point "/boot")
+                           (type "ext4"))
+                         (file-system
+                           (device "/dev/mapper/root")
+                           (mount-point "/")
+                           (type "ext4"))
+                         %base-file-systems))
+    (users (cons (user-account
+                  (name "alice")
+                  (group "users")
+                  (supplementary-groups '("wheel" "audio" "video")))
+                 %base-user-accounts))
+    (services (cons (service marionette-service-type
+                             (marionette-configuration
+                              (imported-modules '((gnu services herd)
+                                                  (guix combinators)))))
+                    %base-services))))
+
+(define %encrypted-root-not-boot-installation-script
+  ;; Shell script for an installation with boot not encrypted but root
+  ;; encrypted.
+  (format #f "\
+. /etc/profile
+set -e -x
+guix --version
+
+export GUIX_BUILD_OPTIONS=--no-grafts
+ls -l /run/current-system/gc-roots
+parted --script /dev/vdb mklabel gpt \\
+  mkpart primary ext2 1M 3M \\
+  mkpart primary ext2 3M 50M \\
+  mkpart primary ext2 50M 1.6G \\
+  set 1 boot on \\
+  set 1 bios_grub on
+echo -n \"~a\" | cryptsetup luksFormat --uuid=\"~a\" -q /dev/vdb3 -
+echo -n \"~a\" | cryptsetup open --type luks --key-file - /dev/vdb3 root
+mkfs.ext4 -L my-root /dev/mapper/root
+mkfs.ext4 -L my-boot /dev/vdb2
+mount LABEL=my-root /mnt
+mkdir /mnt/boot
+mount LABEL=my-boot /mnt/boot
+echo \"Checking mounts\"
+mount
+herd start cow-store /mnt
+mkdir /mnt/etc
+cp /etc/target-config.scm /mnt/etc/config.scm
+guix system build /mnt/etc/config.scm
+guix system init /mnt/etc/config.scm /mnt --no-substitutes
+sync
+echo \"Debugging info\"
+blkid
+cat /mnt/boot/grub/grub.cfg
+reboot\n"
+          %luks-passphrase "12345678-1234-1234-1234-123456789abc"
+          %luks-passphrase))
+
+(define %test-encrypted-root-not-boot-os
+  (system-test
+   (name "encrypted-root-not-boot-os")
+   (description
+    "Test the manual installation on an OS with / in an encrypted partition
+but /boot on a different, non-encrypted partition.  This test is expensive in
+terms of CPU and storage usage since we need to build (current-guix) and then
+store a couple of full system images.")
+   (value
+    (mlet* %store-monad
+        ((image (run-install %encrypted-root-not-boot-os
+                             %encrypted-root-not-boot-os-source
+                             #:script
+                             %encrypted-root-not-boot-installation-script))
+         (command (qemu-command/writable-image image)))
+      (run-basic-test %encrypted-root-not-boot-os command
+                      "encrypted-root-not-boot-os"
+                      #:initialization enter-luks-passphrase)))))
+
 \f
 ;;;
 ;;; Btrfs root file system.
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index ad998156c2..02cf2a12a2 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -385,6 +385,7 @@ STORE is an open connection to the store."
          (params (first (profile-boot-parameters %system-profile
                                                  (list number))))
          (locale (boot-parameters-locale params))
+         (store-crypto-devices (boot-parameters-store-crypto-devices params))
          (store-directory-prefix
           (boot-parameters-store-directory-prefix params))
          (old-generations
@@ -400,6 +401,7 @@ STORE is an open connection to the store."
                      ((bootloader-configuration-file-generator bootloader)
                       bootloader-config entries
                       #:locale locale
+                      #:store-crypto-devices store-crypto-devices
                       #:store-directory-prefix store-directory-prefix
                       #:old-entries old-entries)))
            (drvs -> (list bootcfg)))
diff --git a/tests/boot-parameters.scm b/tests/boot-parameters.scm
index a00b227551..c26ac83b7b 100644
--- a/tests/boot-parameters.scm
+++ b/tests/boot-parameters.scm
@@ -50,6 +50,8 @@
 (define %default-store-directory-prefix
   (string-append "/" %default-btrfs-subvolume))
 (define %default-store-mount-point (%store-prefix))
+(define %default-store-crypto-devices
+  (list (uuid "00000000-1111-2222-3333-444444444444")))
 (define %default-multiboot-modules '())
 (define %default-locale "es_ES.utf8")
 (define %root-path "/")
@@ -67,6 +69,7 @@
    (locale %default-locale)
    (store-device %default-store-device)
    (store-directory-prefix %default-store-directory-prefix)
+   (store-crypto-devices %default-store-crypto-devices)
    (store-mount-point %default-store-mount-point)))
 
 (define %default-operating-system
@@ -110,6 +113,8 @@
           (with-store #t)
           (store-device
            (quote-uuid %default-store-device))
+          (store-crypto-devices
+           (map quote-uuid %default-store-crypto-devices))
           (store-directory-prefix %default-store-directory-prefix)
           (store-mount-point %default-store-mount-point))
   (define (generate-boot-parameters)
@@ -125,12 +130,14 @@
             (sexp-or-nothing " (kernel-arguments ~S)" kernel-arguments)
             (sexp-or-nothing " (initrd ~S)" initrd)
             (if with-store
-                (format #false " (store~a~a~a)"
+                (format #false " (store~a~a~a~a)"
                         (sexp-or-nothing " (device ~S)" store-device)
                         (sexp-or-nothing " (mount-point ~S)"
                                          store-mount-point)
                         (sexp-or-nothing " (directory-prefix ~S)"
-                                         store-directory-prefix))
+                                         store-directory-prefix)
+                        (sexp-or-nothing " (crypto-devices ~S)"
+                                         store-crypto-devices))
                 "")
             (sexp-or-nothing " (locale ~S)" locale)
             (sexp-or-nothing " (bootloader-name ~a)" bootloader-name)
@@ -159,6 +166,7 @@
        (test-read-boot-parameters #:store-device #false)
        (test-read-boot-parameters #:store-device 'false)
        (test-read-boot-parameters #:store-mount-point #false)
+       (test-read-boot-parameters #:store-crypto-devices #false)
        (test-read-boot-parameters #:store-directory-prefix #false)
        (test-read-boot-parameters #:multiboot-modules #false)
        (test-read-boot-parameters #:locale #false)
@@ -254,6 +262,23 @@
   (boot-parameters-store-mount-point
    (test-read-boot-parameters #:with-store #false)))
 
+(test-equal "read, store-crypto-devices, default"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices #false)))
+
+;; XXX: <warning: unrecognized crypto-devices #f at '#f'>
+(test-equal "read, store-crypto-devices, false"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices 'false)))
+
+;; XXX: <warning: unrecognized crypto-device "bad" at '#f'>
+(test-equal "read, store-crypto-devices, string"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices "bad")))
+
 ;; For whitebox testing
 (define operating-system-boot-parameters
   (@@ (gnu system) operating-system-boot-parameters))
-- 
2.28.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#25305: bug#37851: Grub installation only checks for encrypted /boot folder
  2020-10-26 22:15     ` bug#25305: " Miguel Ángel Arruga Vivas
@ 2020-10-28 21:42       ` Miguel Ángel Arruga Vivas
  2020-12-14 13:11         ` bug#25305: bug#37851: " Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: Miguel Ángel Arruga Vivas @ 2020-10-28 21:42 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25305, Mathieu Othacehe, 37851

[-- Attachment #1: Type: text/plain, Size: 214 bytes --]

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

[-- Attachment #2: 0001-system-Allow-separated-/boot-and-encrypted-root.patch --]
[-- Type: text/x-patch, Size: 22498 bytes --]

From 52993db19da43699ea96ea16ebb051b9652934f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
 <rosen644835@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(-)

diff --git a/gnu/bootloader/grub.scm b/gnu/bootloader/grub.scm
index e5fc3470a9..40ea4fbaf7 100644
--- a/gnu/bootloader/grub.scm
+++ b/gnu/bootloader/grub.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019, 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
-;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
+;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2020 Stefan <stefan-guix@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))))
+
   (define (sugar)
     (let* ((entry (first all-entries))
            (device (menu-entry-device entry))
@@ -468,6 +486,7 @@ keymap ~a~%" #$keymap))))
                   "# This file was generated from your Guix configuration.  Any changes
 # will be lost upon reconfiguration.
 ")
+          #$@(crypto-devices)
           #$(sugar)
           #$locale-config
           #$keyboard-layout-config
diff --git a/gnu/machine/ssh.scm b/gnu/machine/ssh.scm
index a3a12fb54b..822f401c1a 100644
--- a/gnu/machine/ssh.scm
+++ b/gnu/machine/ssh.scm
@@ -482,6 +482,8 @@ an environment type of 'managed-host."
                                         (list (second boot-parameters))))
                        (locale -> (boot-parameters-locale
                                    (second boot-parameters)))
+                       (crypto-dev -> (boot-parameters-store-crypto-devices
+                                      (second boot-parameters)))
                        (store-dir -> (boot-parameters-store-directory-prefix
                                       (second boot-parameters)))
                        (old-entries -> (map boot-parameters->menu-entry
@@ -494,6 +496,7 @@ an environment type of 'managed-host."
                                     bootloader))
                                   bootloader entries
                                   #:locale locale
+                                  #:store-crypto-devices crypto-dev
                                   #:store-directory-prefix store-dir
                                   #:old-entries old-entries)))
                        (remote-result (machine-remote-eval machine remote-exp)))
diff --git a/gnu/system.scm b/gnu/system.scm
index 30a5c418d0..3a718642cf 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -5,7 +5,7 @@
 ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Meiyo Peng <meiyo.peng@gmail.com>
-;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
+;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
 ;;; Copyright © 2020 Danny Milosavljevic <dannym@scratchpost.org>
 ;;; Copyright © 2020 Brice Waegeneire <brice@waegenei.re>
 ;;; Copyright © 2020 Florian Pelz <pelzflorian@pelzflorian.de>
@@ -112,6 +112,7 @@
             operating-system-store-file-system
             operating-system-user-mapped-devices
             operating-system-boot-mapped-devices
+            operating-system-bootloader-crypto-devices
             operating-system-activation-script
             operating-system-user-accounts
             operating-system-shepherd-service-names
@@ -147,6 +148,7 @@
             boot-parameters-root-device
             boot-parameters-bootloader-name
             boot-parameters-bootloader-menu-entries
+            boot-parameters-store-crypto-devices
             boot-parameters-store-device
             boot-parameters-store-directory-prefix
             boot-parameters-store-mount-point
@@ -301,6 +303,8 @@ directly by the user."
   (store-device     boot-parameters-store-device)
   (store-mount-point boot-parameters-store-mount-point)
   (store-directory-prefix boot-parameters-store-directory-prefix)
+  (store-crypto-devices boot-parameters-store-crypto-devices
+                        (default '()))
   (locale           boot-parameters-locale)
   (kernel           boot-parameters-kernel)
   (kernel-arguments boot-parameters-kernel-arguments)
@@ -334,6 +338,13 @@ file system labels."
            (if (string-prefix? "/" device)
                device
                (file-system-label device))))))
+  (define uuid-sexp->uuid
+    (match-lambda
+      (('uuid (? symbol? type) (? bytevector? bv))
+       (bytevector->uuid bv type))
+      (x
+       (warning (G_ "unrecognized uuid ~a at '~a'~%") x (port-filename port))
+       #f)))
 
   (match (read port)
     (('boot-parameters ('version 0)
@@ -407,6 +418,24 @@ file system labels."
           ;; No store found, old format.
           #f)))
 
+      (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))
+                   '())))
+            (_
+             ;; No crypto-devices found
+             '())))
+         (_
+          ;; No store found, old format.
+          '())))
+
       (store-mount-point
        (match (assq 'store rest)
          (('store ('device _) ('mount-point mount-point) _ ...)
@@ -520,6 +549,23 @@ from the initrd."
                (any file-system-needed-for-boot? users)))
            devices)))
 
+(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))))
+
 (define (device-mapping-services os)
   "Return the list of device-mapping services for OS as a list."
   (map device-mapping-service
@@ -1256,6 +1302,7 @@ a list of <menu-entry>, to populate the \"old entries\" menu."
          (root-fs         (operating-system-root-file-system os))
          (root-device     (file-system-device root-fs))
          (locale          (operating-system-locale os))
+         (crypto-devices  (operating-system-bootloader-crypto-devices os))
          (params          (operating-system-boot-parameters
                            os root-device
                            #:system-kernel-arguments? #t))
@@ -1269,6 +1316,7 @@ a list of <menu-entry>, to populate the \"old entries\" menu."
     (generate-config-file bootloader-conf (list entry)
                           #:old-entries old-entries
                           #:locale locale
+                          #:store-crypto-devices crypto-devices
                           #:store-directory-prefix
 			  (btrfs-store-subvolume-file-name file-systems))))
 
@@ -1308,6 +1356,7 @@ such as '--root' and '--load' to <boot-parameters>."
                                (operating-system-initrd-file os)))
          (store           (operating-system-store-file-system os))
          (file-systems    (operating-system-file-systems os))
+         (crypto-devices  (operating-system-bootloader-crypto-devices os))
          (locale          (operating-system-locale os))
          (bootloader      (bootloader-configuration-bootloader
                            (operating-system-bootloader os)))
@@ -1330,6 +1379,7 @@ such as '--root' and '--load' to <boot-parameters>."
      (locale locale)
      (store-device (ensure-not-/dev (file-system-device store)))
      (store-directory-prefix (btrfs-store-subvolume-file-name file-systems))
+     (store-crypto-devices crypto-devices)
      (store-mount-point (file-system-mount-point store)))))
 
 (define (device->sexp device)
@@ -1388,7 +1438,10 @@ being stored into the \"parameters\" file)."
                       (mount-point #$(boot-parameters-store-mount-point
                                       params))
                       (directory-prefix
-                       #$(boot-parameters-store-directory-prefix params))))
+                       #$(boot-parameters-store-directory-prefix params))
+                      (crypto-devices
+                       #$(map device->sexp
+                              (boot-parameters-store-crypto-devices params)))))
                   #:set-load-path? #f)))
 
 (define-gexp-compiler (operating-system-compiler (os <operating-system>)
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 86bd93966b..8f1668bab2 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -63,6 +63,8 @@
             %test-separate-home-os
             %test-raid-root-os
             %test-encrypted-root-os
+            %test-encrypted-root-not-boot-os
+            %test-encrypted-root-and-boot-os
             %test-btrfs-root-os
             %test-btrfs-root-on-subvolume-os
             %test-jfs-root-os
@@ -796,6 +798,107 @@ build (current-guix) and then store a couple of full system images.")
       (run-basic-test %encrypted-root-os command "encrypted-root-os"
                       #:initialization enter-luks-passphrase)))))
 
+\f
+;;;
+;;; LUKS-encrypted root file system and /boot in a non-encrypted partition.
+;;;
+
+(define-os-with-source (%encrypted-root-not-boot-os
+                        %encrypted-root-not-boot-os-source)
+  ;; The OS we want to install.
+  (use-modules (gnu) (gnu tests) (srfi srfi-1))
+
+  (operating-system
+    (host-name "bootroot")
+    (timezone "Europe/Madrid")
+    (locale "en_US.UTF-8")
+
+    (bootloader (bootloader-configuration
+                 (bootloader grub-bootloader)
+                 (target "/dev/vdb")))
+
+    (mapped-devices (list (mapped-device
+                           (source
+                            (uuid "12345678-1234-1234-1234-123456789abc"))
+                           (target "root")
+                           (type luks-device-mapping))))
+    (file-systems (cons* (file-system
+                           (device (file-system-label "my-boot"))
+                           (mount-point "/boot")
+                           (type "ext4"))
+                         (file-system
+                           (device "/dev/mapper/root")
+                           (mount-point "/")
+                           (type "ext4"))
+                         %base-file-systems))
+    (users (cons (user-account
+                  (name "alice")
+                  (group "users")
+                  (supplementary-groups '("wheel" "audio" "video")))
+                 %base-user-accounts))
+    (services (cons (service marionette-service-type
+                             (marionette-configuration
+                              (imported-modules '((gnu services herd)
+                                                  (guix combinators)))))
+                    %base-services))))
+
+(define %encrypted-root-not-boot-installation-script
+  ;; Shell script for an installation with boot not encrypted but root
+  ;; encrypted.
+  (format #f "\
+. /etc/profile
+set -e -x
+guix --version
+
+export GUIX_BUILD_OPTIONS=--no-grafts
+ls -l /run/current-system/gc-roots
+parted --script /dev/vdb mklabel gpt \\
+  mkpart primary ext2 1M 3M \\
+  mkpart primary ext2 3M 50M \\
+  mkpart primary ext2 50M 1.6G \\
+  set 1 boot on \\
+  set 1 bios_grub on
+echo -n \"~a\" | cryptsetup luksFormat --uuid=\"~a\" -q /dev/vdb3 -
+echo -n \"~a\" | cryptsetup open --type luks --key-file - /dev/vdb3 root
+mkfs.ext4 -L my-root /dev/mapper/root
+mkfs.ext4 -L my-boot /dev/vdb2
+mount LABEL=my-root /mnt
+mkdir /mnt/boot
+mount LABEL=my-boot /mnt/boot
+echo \"Checking mounts\"
+mount
+herd start cow-store /mnt
+mkdir /mnt/etc
+cp /etc/target-config.scm /mnt/etc/config.scm
+guix system build /mnt/etc/config.scm
+guix system init /mnt/etc/config.scm /mnt --no-substitutes
+sync
+echo \"Debugging info\"
+blkid
+cat /mnt/boot/grub/grub.cfg
+reboot\n"
+          %luks-passphrase "12345678-1234-1234-1234-123456789abc"
+          %luks-passphrase))
+
+(define %test-encrypted-root-not-boot-os
+  (system-test
+   (name "encrypted-root-not-boot-os")
+   (description
+    "Test the manual installation on an OS with / in an encrypted partition
+but /boot on a different, non-encrypted partition.  This test is expensive in
+terms of CPU and storage usage since we need to build (current-guix) and then
+store a couple of full system images.")
+   (value
+    (mlet* %store-monad
+        ((image (run-install %encrypted-root-not-boot-os
+                             %encrypted-root-not-boot-os-source
+                             #:script
+                             %encrypted-root-not-boot-installation-script))
+         (command (qemu-command/writable-image image)))
+      (run-basic-test %encrypted-root-not-boot-os command
+                      "encrypted-root-not-boot-os"
+                      #:initialization enter-luks-passphrase)))))
+
 \f
 ;;;
 ;;; Btrfs root file system.
diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index ad998156c2..02cf2a12a2 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -385,6 +385,7 @@ STORE is an open connection to the store."
          (params (first (profile-boot-parameters %system-profile
                                                  (list number))))
          (locale (boot-parameters-locale params))
+         (store-crypto-devices (boot-parameters-store-crypto-devices params))
          (store-directory-prefix
           (boot-parameters-store-directory-prefix params))
          (old-generations
@@ -400,6 +401,7 @@ STORE is an open connection to the store."
                      ((bootloader-configuration-file-generator bootloader)
                       bootloader-config entries
                       #:locale locale
+                      #:store-crypto-devices store-crypto-devices
                       #:store-directory-prefix store-directory-prefix
                       #:old-entries old-entries)))
            (drvs -> (list bootcfg)))
diff --git a/tests/boot-parameters.scm b/tests/boot-parameters.scm
index a00b227551..c26ac83b7b 100644
--- a/tests/boot-parameters.scm
+++ b/tests/boot-parameters.scm
@@ -50,6 +50,8 @@
 (define %default-store-directory-prefix
   (string-append "/" %default-btrfs-subvolume))
 (define %default-store-mount-point (%store-prefix))
+(define %default-store-crypto-devices
+  (list (uuid "00000000-1111-2222-3333-444444444444")))
 (define %default-multiboot-modules '())
 (define %default-locale "es_ES.utf8")
 (define %root-path "/")
@@ -67,6 +69,7 @@
    (locale %default-locale)
    (store-device %default-store-device)
    (store-directory-prefix %default-store-directory-prefix)
+   (store-crypto-devices %default-store-crypto-devices)
    (store-mount-point %default-store-mount-point)))
 
 (define %default-operating-system
@@ -110,6 +113,8 @@
           (with-store #t)
           (store-device
            (quote-uuid %default-store-device))
+          (store-crypto-devices
+           (map quote-uuid %default-store-crypto-devices))
           (store-directory-prefix %default-store-directory-prefix)
           (store-mount-point %default-store-mount-point))
   (define (generate-boot-parameters)
@@ -125,12 +130,14 @@
             (sexp-or-nothing " (kernel-arguments ~S)" kernel-arguments)
             (sexp-or-nothing " (initrd ~S)" initrd)
             (if with-store
-                (format #false " (store~a~a~a)"
+                (format #false " (store~a~a~a~a)"
                         (sexp-or-nothing " (device ~S)" store-device)
                         (sexp-or-nothing " (mount-point ~S)"
                                          store-mount-point)
                         (sexp-or-nothing " (directory-prefix ~S)"
-                                         store-directory-prefix))
+                                         store-directory-prefix)
+                        (sexp-or-nothing " (crypto-devices ~S)"
+                                         store-crypto-devices))
                 "")
             (sexp-or-nothing " (locale ~S)" locale)
             (sexp-or-nothing " (bootloader-name ~a)" bootloader-name)
@@ -159,6 +166,7 @@
        (test-read-boot-parameters #:store-device #false)
        (test-read-boot-parameters #:store-device 'false)
        (test-read-boot-parameters #:store-mount-point #false)
+       (test-read-boot-parameters #:store-crypto-devices #false)
        (test-read-boot-parameters #:store-directory-prefix #false)
        (test-read-boot-parameters #:multiboot-modules #false)
        (test-read-boot-parameters #:locale #false)
@@ -254,6 +262,23 @@
   (boot-parameters-store-mount-point
    (test-read-boot-parameters #:with-store #false)))
 
+(test-equal "read, store-crypto-devices, default"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices #false)))
+
+;; XXX: <warning: unrecognized crypto-devices #f at '#f'>
+(test-equal "read, store-crypto-devices, false"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices 'false)))
+
+;; XXX: <warning: unrecognized crypto-device "bad" at '#f'>
+(test-equal "read, store-crypto-devices, string"
+  '()
+  (boot-parameters-store-crypto-devices
+   (test-read-boot-parameters #:store-crypto-devices "bad")))
+
 ;; For whitebox testing
 (define operating-system-boot-parameters
   (@@ (gnu system) operating-system-boot-parameters))
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#25305: bug#37851: bug#25305: bug#37851: Grub installation only checks for encrypted /boot folder
  2020-10-28 21:42       ` Miguel Ángel Arruga Vivas
@ 2020-12-14 13:11         ` Ludovic Courtès
  2020-12-21 20:23           ` Miguel Ángel Arruga Vivas
  0 siblings, 1 reply; 10+ messages in thread
From: Ludovic Courtès @ 2020-12-14 13:11 UTC (permalink / raw)
  To: Miguel Ángel Arruga Vivas; +Cc: 25305, Mathieu Othacehe, 37851

Hi Miguel,

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

>>From 52993db19da43699ea96ea16ebb051b9652934f9 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Miguel=20=C3=81ngel=20Arruga=20Vivas?=
>  <rosen644835@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@famulari.name>
>  ;;; Copyright © 2017, 2020 Mathieu Othacehe <m.othacehe@gmail.com>
>  ;;; Copyright © 2019, 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
> -;;; Copyright © 2019 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
> +;;; Copyright © 2019, 2020 Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
>  ;;; Copyright © 2020 Maxim Cournoyer <maxim.cournoyer@gmail.com>
>  ;;; Copyright © 2020 Stefan <stefan-guix@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’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#25305: bug#37851: bug#25305: bug#37851: Grub installation only checks for encrypted /boot folder
  2020-12-14 13:11         ` bug#25305: bug#37851: " Ludovic Courtès
@ 2020-12-21 20:23           ` Miguel Ángel Arruga Vivas
  2020-12-22 13:41             ` Ludovic Courtès
  0 siblings, 1 reply; 10+ messages in thread
From: Miguel Ángel Arruga Vivas @ 2020-12-21 20:23 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 25305-done, Mathieu Othacehe, 37851-done

Hi Ludo,

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

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

> Hi Miguel,
>
> Miguel Ángel Arruga Vivas <rosen644835@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




^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#25305: bug#37851: bug#25305: bug#37851: Grub installation only checks for encrypted /boot folder
  2020-12-21 20:23           ` Miguel Ángel Arruga Vivas
@ 2020-12-22 13:41             ` Ludovic Courtès
  0 siblings, 0 replies; 10+ messages in thread
From: Ludovic Courtès @ 2020-12-22 13:41 UTC (permalink / raw)
  To: Miguel Ángel Arruga Vivas; +Cc: 25305-done, Mathieu Othacehe, 37851-done

Hi,

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

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

Woohoo, thank you!

Ludo’.




^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-12-22 13:42 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 11:07 bug#37851: Grub installation only checks for encrypted /boot folder Miguel Arruga Vivas
2019-10-21 12:47 ` Miguel Arruga Vivas
2019-10-22 14:12   ` Ludovic Courtès
2019-10-27  1:00     ` Miguel Arruga Vivas
2020-10-26 22:15     ` bug#25305: " Miguel Ángel Arruga Vivas
2020-10-28 21:42       ` Miguel Ángel Arruga Vivas
2020-12-14 13:11         ` bug#25305: bug#37851: " Ludovic Courtès
2020-12-21 20:23           ` Miguel Ángel Arruga Vivas
2020-12-22 13:41             ` Ludovic Courtès
2019-10-21 14:55 ` bug#37851: Miguel Arruga Vivas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).