bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

Hi!

Following the Shepherd upgrade in commit
400c9ed3d779308e56038305d40cd93acb496180, attempts to open non-root LUKS
devices from a Shepherd service fail with this cryptsetup message:

  Nothing to read on input.

This is because standard input is now /dev/null so it cannot read the
passphrase.

This is because “shepherd.conf” now does:

    (start-in-the-background …)

    (redirect-port (open-input-file "/dev/null")
                   (current-input-port))

… meaning that ‘redirect-port’ is now called before services are
started.

‘fsck’ invocations for non-root file systems (started from Shepherd
services) would have the same problem.

I’ll try and come up with a solution…

Ludo’.

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

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

Hi,

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

> Following the Shepherd upgrade in commit
> 400c9ed3d779308e56038305d40cd93acb496180, attempts to open non-root LUKS
> devices from a Shepherd service fail with this cryptsetup message:
>
>   Nothing to read on input.
>
> This is because standard input is now /dev/null so it cannot read the
> passphrase.

In Cryptsetup, the ‘tools_get_key’ function reads this:

--8<---------------cut here---------------start------------->8---
if (tools_is_stdin(key_file)) {
        if (isatty(STDIN_FILENO)) {
                if (keyfile_offset) {
                        log_err(_("Cannot use offset with terminal input."));
                } else {
                        if (!prompt && !crypt_get_device_name(cd))
                                snprintf(tmp, sizeof(tmp), _("Enter passphrase: "));
                        else if (!prompt) {
                                backing_file = crypt_loop_backing_file(crypt_get_device_name(cd));
                                snprintf(tmp, sizeof(tmp), _("Enter passphrase for %s: "), backing_file ?: crypt_get_device_name(cd));
                                free(backing_file);
                        }
                        r = crypt_get_key_tty(prompt ?: tmp, key, key_size, timeout, verify, cd);
                }
        } else {
                log_dbg("STDIN descriptor passphrase entry requested.");
                /* No keyfile means STDIN with EOL handling (\n will end input)). */
                r = crypt_keyfile_device_read(cd, NULL, key, key_size,
                                keyfile_offset, keyfile_size_max,
                                key_file ? 0 : CRYPT_KEYFILE_STOP_EOL);
        }
}
--8<---------------cut here---------------end--------------->8---

isatty(3) would return 0 when stdin is /dev/null; simply binding stdin
to /dev/console:

  (with-input-from-file "/dev/console"
    (lambda ()
      (system* "cryptsetup" …)))

wouldn’t help, for reasons that are less clear to me¹.

The attached patch solves the ‘cryptsetup open’ problem for the case
when ‘cryptsetup’ is invoked from shepherd—e.g., for an encrypted /home.
I’m now running the “encrypted-root-os” test.

I’m not sure how to test fsck interactivity though; ideas welcome.  If
you’re reading this and would like to test it on the bare metal (worst
case is it fails to boot and you have to reboot into the older
generation), that’s also much appreciated.

Feedback welcome!

Thanks,
Ludo’.

¹ This returns true:
  sudo strace -f -o ,,s guile -c '(with-input-from-file "/dev/console" (lambda () (system* "guile" "-c" "(pk (isatty? (current-input-port)))")))'


[-- Attachment #2: Type: text/x-patch, Size: 13483 bytes --]

diff --git a/gnu/build/file-systems.scm b/gnu/build/file-systems.scm
index d95340df83..b06a4cc25c 100644
--- a/gnu/build/file-systems.scm
+++ b/gnu/build/file-systems.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014-2018, 2020-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016, 2017 David Craven <david@craven.ch>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019 Guillaume Le Vaillant <glv@posteo.net>
@@ -54,6 +54,7 @@ (define-module (gnu build file-systems)
 
             bind-mount
 
+            system*/tty
             mount-flags->bit-mask
             check-file-system
             mount-file-system
@@ -67,6 +68,33 @@ (define-module (gnu build file-systems)
 ;;;
 ;;; Code:
 
+(define (system*/console program . args)
+  "Run PROGRAM with ARGS in a tty on top of /dev/console.  The return value is
+as for 'system*'."
+  (match (primitive-fork)
+    (0
+     (dynamic-wind
+       (const #t)
+       (lambda ()
+         (login-tty (open-fdes "/dev/console" O_RDWR))
+         (apply execlp program program args))
+       (lambda ()
+         (primitive-_exit 127))))
+    (pid
+     (cdr (waitpid pid)))))
+
+(define (system*/tty program . args)
+  "Run PROGRAM with ARGS, creating a tty if its standard input isn't one.
+The return value is as for 'system*'.
+
+This is necessary for commands such as 'cryptsetup open' or 'fsck' that may
+need to interact with the user but might be invoked from shepherd, where
+standard input is /dev/null."
+  (apply (if (isatty? (current-input-port))
+             system*
+             system*/console)
+         program args))
+
 (define (bind-mount source target)
   "Bind-mount SOURCE at TARGET."
   (mount source target "" MS_BIND))
@@ -180,13 +208,13 @@ (define (check-ext2-file-system device force? repair)
 do not write to the file system to fix errors.  If it's #t, fix all
 errors.  Otherwise, fix only those considered safe to repair automatically."
   (match (status:exit-val
-          (apply system* `("e2fsck" "-v" "-C" "0"
-                           ,@(if force? '("-f") '())
-                           ,@(match repair
-                               (#f '("-n"))
-                               (#t '("-y"))
-                               (_  '("-p")))
-                           ,device)))
+          (apply system*/tty "e2fsck" "-v" "-C" "0"
+                 `(,@(if force? '("-f") '())
+                   ,@(match repair
+                       (#f '("-n"))
+                       (#t '("-y"))
+                       (_  '("-p")))
+                   ,device)))
     (0 'pass)
     (1 'errors-corrected)
     (2 'reboot-required)
@@ -312,14 +340,14 @@ (define (check-bcachefs-file-system device force? repair)
         (status
          ;; A number, or #f on abnormal termination (e.g., assertion failure).
          (status:exit-val
-          (apply system* `("bcachefs" "fsck" "-v"
-                           ,@(if force? '("-f") '())
-                           ,@(match repair
-                               (#f '("-n"))
-                               (#t '("-y"))
-                               (_  '("-p")))
-                           ;; Make each multi-device member a separate argument.
-                           ,@(string-split device #\:))))))
+          (apply system*/tty "bcachefs" "fsck" "-v"
+                 `(,@(if force? '("-f") '())
+                   ,@(match repair
+                       (#f '("-n"))
+                       (#t '("-y"))
+                       (_  '("-p")))
+                   ;; Make each multi-device member a separate argument.
+                   ,@(string-split device #\:))))))
     (match (and=> status (cut logand <> (lognot ignored-bits)))
       (0 'pass)
       (1 'errors-corrected)
@@ -364,17 +392,17 @@ (define (check-btrfs-file-system device force? repair)
 fix only those considered safe to repair automatically."
   (if force?
       (match (status:exit-val
-              (apply system* `("btrfs" "check" "--progress"
-                               ;; Btrfs's ‘--force’ is not relevant to us here.
-                               ,@(match repair
-                                   ;; Upstream considers ALL repairs dangerous
-                                   ;; and will warn the user at run time.
-                                   (#t '("--repair"))
-                                   (_  '("--readonly" ; a no-op for clarity
-                                         ;; A 466G file system with 180G used is
-                                         ;; enough to kill btrfs with 6G of RAM.
-                                         "--mode" "lowmem")))
-                               ,device)))
+              (apply system*/tty "btrfs" "check" "--progress"
+                     ;; Btrfs's ‘--force’ is not relevant to us here.
+                     `(,@(match repair
+                           ;; Upstream considers ALL repairs dangerous
+                           ;; and will warn the user at run time.
+                           (#t '("--repair"))
+                           (_  '("--readonly"     ; a no-op for clarity
+                                 ;; A 466G file system with 180G used is
+                                 ;; enough to kill btrfs with 6G of RAM.
+                                 "--mode" "lowmem")))
+                       ,device)))
         (0 'pass)
         (_ 'fatal-error))
       'pass))
@@ -412,11 +440,11 @@ (define (check-fat-file-system device force? repair)
 not write to the file system to fix errors. Otherwise, automatically fix them
 using the least destructive approach."
   (match (status:exit-val
-          (apply system* `("fsck.vfat" "-v"
-                           ,@(match repair
-                               (#f '("-n"))
-                               (_  '("-a"))) ; no 'safe/#t distinction
-                           ,device)))
+          (system*/tty "fsck.vfat" "-v"
+                       (match repair
+                         (#f "-n")
+                         (_  "-a"))               ;no 'safe/#t distinction
+                       device))
     (0 'pass)
     (1 'errors-corrected)
     (_ 'fatal-error)))
@@ -545,7 +573,7 @@ (define (check-jfs-file-system device force? repair)
 only if FORCE?  is true. Otherwise, replay the transaction log before checking
 and automatically fix found errors."
   (match (status:exit-val
-          (apply system*
+          (apply system*/tty
                  `("jfs_fsck" "-v"
                    ;; The ‘LEVEL’ logic is convoluted.  To quote fsck/xchkdsk.c
                    ;; (‘-p’, ‘-a’, and ‘-r’ are aliases in every way):
@@ -621,10 +649,10 @@ (define (check-f2fs-file-system device force? repair)
             "warning: forced check of F2FS ~a implies repairing any errors~%"
             device))
   (match (status:exit-val
-          (apply system* `("fsck.f2fs"
-                           ,@(if force? '("-f") '())
-                           ,@(if repair '("-p") '("--dry-run"))
-                           ,device)))
+          (apply system*/tty "fsck.f2fs"
+                 `(,@(if force? '("-f") '())
+                   ,@(if repair '("-p") '("--dry-run"))
+                   ,device)))
     ;; 0 and -1 are the only two possibilities according to the man page.
     (0 'pass)
     (_ 'fatal-error)))
@@ -709,9 +737,9 @@ (define (check-ntfs-file-system device force? repair)
 true and the volume has been repaired by an external tool, clear the volume
 dirty flag to indicate that it's now safe to mount."
   (match (status:exit-val
-          (apply system* `("ntfsfix"
-                           ,@(if repair '("--clear-dirty") '("--no-action"))
-                           ,device)))
+          (system*/tty "ntfsfix"
+                       (if repair "--clear-dirty" "--no-action")
+                       device))
     (0 'pass)
     (_ 'fatal-error)))
 
@@ -754,11 +782,11 @@ (define (check-xfs-file-system device force? repair)
 Otherwise, only replay the log, and check without attempting further repairs."
   (define (xfs_repair)
     (status:exit-val
-     (apply system* `("xfs_repair" "-Pv"
-                      ,@(match repair
-                          (#t '("-e"))
-                          (_  '("-n"))) ; will miss some errors
-                      ,device))))
+     (system*/tty "xfs_repair" "-Pv"
+                  (match repair
+                    (#t "-e")
+                    (_  "-n"))                    ;will miss some errors
+                  device)))
   (if force?
       ;; xfs_repair fails with exit status 2 if the log is dirty, which is
       ;; likely in situations where you're running xfs_repair.  Only the kernel
diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm
index 96a381d5fe..e6b8970c12 100644
--- a/gnu/system/mapped-devices.scm
+++ b/gnu/system/mapped-devices.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2017, 2018 Mark H Weaver <mhw@netris.org>
 ;;;
@@ -202,7 +202,8 @@ (define (open-luks-device source targets)
            ;; XXX: 'use-modules' should be at the top level.
            (use-modules (rnrs bytevectors) ;bytevector?
                         ((gnu build file-systems)
-                         #:select (find-partition-by-luks-uuid))
+                         #:select (find-partition-by-luks-uuid
+                                   system*/tty))
                         ((guix build utils) #:select (mkdir-p)))
 
            ;; Create '/run/cryptsetup/' if it does not exist, as device locking
@@ -211,28 +212,32 @@ (define (open-luks-device source targets)
 
            ;; Use 'cryptsetup-static', not 'cryptsetup', to avoid pulling the
            ;; whole world inside the initrd (for when we're in an initrd).
-           (zero? (system* #$(file-append cryptsetup-static "/sbin/cryptsetup")
-                           "open" "--type" "luks"
+           ;; 'cryptsetup open' requires standard input to be a tty to allow
+           ;; for interaction but shepherd sets standard input to /dev/null;
+           ;; thus, explicitly request a tty.
+           (zero? (system*/tty
+                   #$(file-append cryptsetup-static "/sbin/cryptsetup")
+                   "open" "--type" "luks"
 
-                           ;; Note: We cannot use the "UUID=source" syntax here
-                           ;; because 'cryptsetup' implements it by searching the
-                           ;; udev-populated /dev/disk/by-id directory but udev may
-                           ;; be unavailable at the time we run this.
-                           (if (bytevector? source)
-                               (or (let loop ((tries-left 10))
-                                     (and (positive? tries-left)
-                                          (or (find-partition-by-luks-uuid source)
-                                              ;; If the underlying partition is
-                                              ;; not found, try again after
-                                              ;; waiting a second, up to ten
-                                              ;; times.  FIXME: This should be
-                                              ;; dealt with in a more robust way.
-                                              (begin (sleep 1)
-                                                     (loop (- tries-left 1))))))
-                                   (error "LUKS partition not found" source))
-                               source)
+                   ;; Note: We cannot use the "UUID=source" syntax here
+                   ;; because 'cryptsetup' implements it by searching the
+                   ;; udev-populated /dev/disk/by-id directory but udev may
+                   ;; be unavailable at the time we run this.
+                   (if (bytevector? source)
+                       (or (let loop ((tries-left 10))
+                             (and (positive? tries-left)
+                                  (or (find-partition-by-luks-uuid source)
+                                      ;; If the underlying partition is
+                                      ;; not found, try again after
+                                      ;; waiting a second, up to ten
+                                      ;; times.  FIXME: This should be
+                                      ;; dealt with in a more robust way.
+                                      (begin (sleep 1)
+                                             (loop (- tries-left 1))))))
+                           (error "LUKS partition not found" source))
+                       source)
 
-                           #$target)))))))
+                   #$target)))))))
 
 (define (close-luks-device source targets)
   "Return a gexp that closes TARGET, a LUKS device."

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

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

A note on my debugging tricks, for posterity…

Initially, I tried to reproduce the issue (in a VM) with a Guile or Bash
process that would be invoked from shepherd before ‘user-processes’ and
that would try to read from stdin:

--8<---------------cut here---------------start------------->8---
(simple-service 'input
                shepherd-root-service-type
                (list (shepherd-service
                       (provision '(input))
                       (start #~(lambda ()
                                  (pk 'tty-before?
                                      (current-input-port)
                                      (isatty?
                                       (current-input-port)))

                                  (with-output-to-file
                                      "/dev/tty1"
                                    (lambda ()
                                     (system*
                                      #$(file-append
                                         coreutils "/bin/ls") "-l" "/proc/self/fd")
                                     (with-input-from-file
                                         "/dev/tty1"
                                       (lambda ()
                                         (pk 'tty?
                                             (isatty?
                                              (current-input-port)))
                                         (system*
                                          #$(file-append
                                             coreutils "/bin/ls") "-l"
                                          "/proc/self/fd")
                                         (system* "/bin/sh"
                                                  "-c"
                                                  "echo read; read x; echo got $x; read y"))))))))))
(simple-service 'wait-for-input user-processes-service-type
                '(input))
--8<---------------cut here---------------end--------------->8---

For some reason, that did not reproduce the issue; ‘isatty?’ would
return true.

So I though I’d arrange to run ‘cryptsetup open --type luks’.  To do
that, I copied the header of a real LUKS partition:

  sudo dd if=/dev/sda2 of=/tmp/luks.img bs=1024 count=1025

and then came up with an OS config that would try to open than fake LUKS
device:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-scheme, Size: 2422 bytes --]

(use-modules (gnu))
(use-service-modules networking ssh shepherd)
(use-package-modules base linux screen ssh)

(operating-system
  (host-name "komputilo")
  (timezone "Europe/Berlin")
  (locale "en_US.utf8")

  ;; Boot in "legacy" BIOS mode, assuming /dev/sdX is the
  ;; target hard disk, and "my-root" is the label of the target
  ;; root file system.
  (bootloader (bootloader-configuration
                (bootloader grub-bootloader)
                (targets '("/dev/sdX"))))
  (mapped-devices (list (mapped-device
                         (source "/dev/loop0")
                         (target "root")
                         (type luks-device-mapping))))
  (file-systems (cons (file-system
                        (device (file-system-label "my-root"))
                        (mount-point "/")
                        (type "ext4"))
                      %base-file-systems))

  ;; This is where user accounts are specified.  The "root"
  ;; account is implicit, and is initially created with the
  ;; empty password.
  (users (cons (user-account
                (name "alice")
                (comment "Bob's sister")
                (group "users")

                ;; Adding the account to the "wheel" group
                ;; makes it a sudoer.  Adding it to "audio"
                ;; and "video" allows the user to play sound
                ;; and access the webcam.
                (supplementary-groups '("wheel"
                                        "audio" "video")))
               %base-user-accounts))

  ;; Globally-installed packages.
  (packages (cons screen %base-packages))

  ;; Add services to the baseline: a DHCP client and
  ;; an SSH server.
  (services (append (list (service dhcp-client-service-type)
                          (simple-service 'losetup activation-service-type
                                          #~(system*
                                             #$(file-append util-linux
                                                            "/sbin/losetup")
                                             "/dev/loop0"
                                             #$(local-file "/tmp/luks.img")))
                          (service openssh-service-type
                                   (openssh-configuration
                                    (openssh openssh-sans-x)
                                    (port-number 2222))))
                    %base-services)))

[-- Attachment #3: Type: text/plain, Size: 308 bytes --]


That’s enough to see whether ‘cryptsetup open’ manages to read the
passphrase and all.

Eventually I confirmed by testing it on the bare metal, on a victim’s
laptop.

Currently we don’t have an installation test with cleartext root +
encrypted home; we should prolly do that.

Ludo’.

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

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

> Following the Shepherd upgrade in commit
> 400c9ed3d779308e56038305d40cd93acb496180, attempts to open non-root LUKS
> devices from a Shepherd service fail with this cryptsetup message:
>
>   Nothing to read on input.
>
> This is because standard input is now /dev/null so it cannot read the
> passphrase.

Fixed with these commits:

  f239b9d788 tests: Add "encrypted-home-os" installation test.
  dcb0e54a4f file-systems: Invoke fsck tools with 'system*/tty'.
  931f13840b mapped-devices: Ensure 'cryptsetup open' gets a tty.

Ludo’.

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Christoph Weiss]

I ran into the same issue (no prompt but instead about 32 times the 
message "Nothing to read on input.")

I bisected this issue myself (starting before the fix was pushed) and 
arrived at 400c9ed3d779308e56038305d40cd93acb496180 as the culprit.

However, even with the current master 
(aa5716703fd4603dd89ce9b55f555b7aa85f4c9b), the issue has not been fixed 
for me.

Please let me know if I should provide any specific info.

Thank you.

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

Hi Christoph,

Christoph Weiss <weiss@wsoptics.de> skribis:

> I ran into the same issue (no prompt but instead about 32 times the
> message "Nothing to read on input.")
>
> I bisected this issue myself (starting before the fix was pushed) and
> arrived at 400c9ed3d779308e56038305d40cd93acb496180 as the culprit.
>
> However, even with the current master
> (aa5716703fd4603dd89ce9b55f555b7aa85f4c9b), the issue has not been
> fixed for me.

It’s definitely fixed for me and the new ‘separate-home-os’ system test
agrees¹.

Did you reconfigure after pulling?  What does ‘guix system describe’
report?

Thanks,
Ludo’.

¹ https://ci.guix.gnu.org/search?query=spec%3Atests+separate-home

bug#54770: (no subject)

[Christoph Weiss]

Hi Ludo,

Thanks for your reply.  Just to double check, I ran another `sudo guix 
pull` and `sudo guix system reconfigure /etc/config.scm`.  I still 
cannot boot that installation.

This is `guix system describe`'s output for my working revision:

Generation 40   Apr 08 2022 19:52:13
   file name: /var/guix/profiles/system-40-link
   canonical file name: /gnu/store/sp41bjidi9qrl0k1gqpxivrdign7f7d6-system
   label: GNU with Linux-Libre 5.16.14
   bootloader: grub
   root device: UUID: b63f8b65-48f2-41a7-9652-9a0cb1759048
   kernel: 
/gnu/store/1shy73077i3im91i4lv9bamifm4w29q1-linux-libre-5.16.14/bzImage
   channels:
     guix:
       repository URL: https://git.savannah.gnu.org/git/guix.git
       commit: f077fa0fd0519f0e04f458192a87854094aa36eb
   configuration file: 
/gnu/store/18hd645wcz8l5bvhp4lsy49cadq0d655-configuration.scm

I'm not quite sure how to print the describe for the failing revision...

Christoph

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Ludovic Courtès]

Hi,

Christoph Weiss <weiss@wsoptics.de> skribis:

> Thanks for your reply.  Just to double check, I ran another `sudo guix
> pull` and `sudo guix system reconfigure /etc/config.scm`.  I still 
> cannot boot that installation.
>
> This is `guix system describe`'s output for my working revision:
>
> Generation 40   Apr 08 2022 19:52:13
>   file name: /var/guix/profiles/system-40-link
>   canonical file name: /gnu/store/sp41bjidi9qrl0k1gqpxivrdign7f7d6-system
>   label: GNU with Linux-Libre 5.16.14
>   bootloader: grub
>   root device: UUID: b63f8b65-48f2-41a7-9652-9a0cb1759048
>   kernel:
>   /gnu/store/1shy73077i3im91i4lv9bamifm4w29q1-linux-libre-5.16.14/bzImage
>   channels:
>     guix:
>       repository URL: https://git.savannah.gnu.org/git/guix.git
>       commit: f077fa0fd0519f0e04f458192a87854094aa36eb
>   configuration file:
>   /gnu/store/18hd645wcz8l5bvhp4lsy49cadq0d655-configuration.scm

This commit is older than the fix:

  https://git.savannah.gnu.org/cgit/guix.git/commit/?id=931f13840bad7bd3cb98b7806f09ca2b3cf7cd10

Probably this comes from a confusion: ‘sudo guix pull’ updates
~root/.config/guix/bin, but ‘sudo guix system reconfigure’ runs your
user’s ~/.config/guix/bin.

If you use sudo, what you need to do is:

  guix pull  # without sudo!
  sudo guix system reconfigure …

Let me know if that helps!

Ludo’.

bug#54770: Non-root LUKS devices unusable after Shepherd upgrade

[Christoph Weiss]

Hi Ludo,

Oh, I had no idea that's how it worked... sorry about that.  Indeed this 
has fixed the issue.  Thank you for your time!

Christoph