| Message ID | 058b41c5060e1811048fe44c20278c64fdfc3ece.1690981365.git.wolf@wolfsden.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#65002,v2,1/2] mapped-devices: Allow unlocking by a key file | expand |
Hello! I know, I know, it’s taken way too long… My apologies! Tomas Volf <wolf@wolfsden.cz> skribis: > Requiring the user to input their password in order to unlock a device is not > always reasonable, so having an option to unlock the device using a key file > is a nice quality of life change. Agreed; there’s interest for this feature, I’ve heard it quite a few times. > * gnu/system/mapped-devices.scm (luks-device-mapping): New keyword argument > * gnu/system/mapped-devices.scm (luks-device-mapping-with-options): New > procedure No need to repeat the file name here. Please also mention the doc/guix.texi changes. > +@deffn {Procedure} luks-device-mapping-with-options [#:key-file] > +Return a @code{luks-device-mapping} object, which defines LUKS block > +device encryption using the @command{cryptsetup} command from the > +package with the same name. It relies on the @code{dm-crypt} Linux > +kernel module. > + > +If @code{key-file} is provided, unlocking is first attempted using that > +key file. If it fails, password unlock is attempted as well. Key file > +is not stored in the store and needs to be available at the specified > +path at the time of the unlock attempt. s/specified path/given location/ Perhaps add a sentence or two saying that the advantage is that it allows you to avoid typing the passphrase, for instance by passing the key file on a USB key (would that work?), but that this may not be suitable for all use cases. I’d also add a short commented config example. I wonder if we could have a system test; it doesn’t sound very easy so maybe we’ll skip, but you can check that the “encrypted-root-os” test, which exercises ‘luks-device-mapping’, still passes (it takes time and disk space). The rest LGTM! Ludo’.
On 2024-01-10 00:21:19 +0100, Ludovic Courtès wrote: > Hello! > > I know, I know, it’s taken way too long… My apologies! No worries, thank you for getting to it. :) > > > * gnu/system/mapped-devices.scm (luks-device-mapping): New keyword argument > > * gnu/system/mapped-devices.scm (luks-device-mapping-with-options): New > > procedure > > No need to repeat the file name here. Please also mention the > doc/guix.texi changes. Adjusted. I also fixed the name of the first procedure (should have been open-luks-device). > > > +@deffn {Procedure} luks-device-mapping-with-options [#:key-file] > > +Return a @code{luks-device-mapping} object, which defines LUKS block > > +device encryption using the @command{cryptsetup} command from the > > +package with the same name. It relies on the @code{dm-crypt} Linux > > +kernel module. > > + > > +If @code{key-file} is provided, unlocking is first attempted using that > > +key file. If it fails, password unlock is attempted as well. Key file > > +is not stored in the store and needs to be available at the specified > > +path at the time of the unlock attempt. > > s/specified path/given location/ > > Perhaps add a sentence or two saying that the advantage is that it > allows you to avoid typing the passphrase, for instance by passing the > key file on a USB key (would that work?), but that this may not be > suitable for all use cases. Added a sentence. As for the USB key, that would not currently work. The file needs to be accessible to the init script, so the USB would need to be mounted first. I believe extending the code to support it would not be hard (adding e.g. #:device to luks-device-mapping-with-options), but I have not use for it, so I did not intend to do it in this series. Maybe later. > > I’d also add a short commented config example. Done. > > I wonder if we could have a system test; it doesn’t sound very easy so > maybe we’ll skip, but you can check that the “encrypted-root-os” test, > which exercises ‘luks-device-mapping’, still passes (it takes time and > disk space). It does not pass, but it fails even on master ¯\_ (ツ)_/¯: guix system: warning: at least 1526.8 MB needed but only 1408.3 MB available in /mnt It seems somewhat hard to do it based on encrypted-root-os, but should be much easier basing it on encrypted-home-os. I might give it a try. > > The rest LGTM! > > Ludo’.
On 2024-01-11 13:39:36 +0100, Tomas Volf wrote: > > > > > I wonder if we could have a system test; it doesn’t sound very easy so > > maybe we’ll skip, but you can check that the “encrypted-root-os” test, > > which exercises ‘luks-device-mapping’, still passes (it takes time and > > disk space). > > It does not pass, but it fails even on master ¯\_ (ツ)_/¯: > > guix system: warning: at least 1526.8 MB needed but only 1408.3 MB available in /mnt > > It seems somewhat hard to do it based on encrypted-root-os, but should be much > easier basing it on encrypted-home-os. I might give it a try. I managed to figure out the system test for this, however it required unrelated changes, since encrypted-root-os and encrypted-home-os were broken even on master. I included my new test (together with the fixes) in v3. Also, I messed up, and sent this *without* the v3 by accident. When I realized, I sent it once more, this time properly as v3. Sorry for the noise. Tomas
diff --git a/doc/guix.texi b/doc/guix.texi index 58cc3d7aad..a857654191 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -17622,6 +17622,18 @@ Mapped Devices @code{dm-crypt} Linux kernel module. @end defvar +@deffn {Procedure} luks-device-mapping-with-options [#:key-file] +Return a @code{luks-device-mapping} object, which defines LUKS block +device encryption using the @command{cryptsetup} command from the +package with the same name. It relies on the @code{dm-crypt} Linux +kernel module. + +If @code{key-file} is provided, unlocking is first attempted using that +key file. If it fails, password unlock is attempted as well. Key file +is not stored in the store and needs to be available at the specified +path at the time of the unlock attempt. +@end deffn + @defvar raid-device-mapping This defines a RAID device, which is assembled using the @code{mdadm} command from the package with the same name. It requires a Linux kernel diff --git a/gnu/system/mapped-devices.scm b/gnu/system/mapped-devices.scm index e6b8970c12..0755036763 100644 --- a/gnu/system/mapped-devices.scm +++ b/gnu/system/mapped-devices.scm @@ -2,6 +2,7 @@ ;;; 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> +;;; Copyright © 2023 Tomas Volf <wolf@wolfsden.cz> ;;; ;;; This file is part of GNU Guix. ;;; @@ -64,6 +65,7 @@ (define-module (gnu system mapped-devices) check-device-initrd-modules ;XXX: needs a better place luks-device-mapping + luks-device-mapping-with-options raid-device-mapping lvm-device-mapping)) @@ -188,7 +190,7 @@ (define (check-device-initrd-modules device linux-modules location) ;;; Common device mappings. ;;; -(define (open-luks-device source targets) +(define* (open-luks-device source targets #:key key-file) "Return a gexp that maps SOURCE to TARGET as a LUKS device, using 'cryptsetup'." (with-imported-modules (source-module-closure @@ -198,7 +200,8 @@ (define (open-luks-device source targets) ((target) #~(let ((source #$(if (uuid? source) (uuid-bytevector source) - source))) + source)) + (keyfile #$key-file)) ;; XXX: 'use-modules' should be at the top level. (use-modules (rnrs bytevectors) ;bytevector? ((gnu build file-systems) @@ -215,29 +218,35 @@ (define (open-luks-device source targets) ;; '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) - - #$target))))))) + (let ((partition + ;; 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))) + ;; We want to fallback to the password unlock if the keyfile fails. + (or (and keyfile + (zero? (system*/tty + #$(file-append cryptsetup-static "/sbin/cryptsetup") + "open" "--type" "luks" + "--key-file" keyfile + partition #$target))) + (zero? (system*/tty + #$(file-append cryptsetup-static "/sbin/cryptsetup") + "open" "--type" "luks" + partition #$target))))))))) (define (close-luks-device source targets) "Return a gexp that closes TARGET, a LUKS device." @@ -276,6 +285,14 @@ (define luks-device-mapping (close close-luks-device) (check check-luks-device))) +(define* (luks-device-mapping-with-options #:key key-file) + "Return a luks-device-mapping object with open modified to pass the arguments +into the open-luks-device procedure." + (mapped-device-kind + (inherit luks-device-mapping) + (open (λ (source targets) (open-luks-device source targets + #:key-file key-file))))) + (define (open-raid-device sources targets) "Return a gexp that assembles SOURCES (a list of devices) to the RAID device TARGET (e.g., \"/dev/md0\"), using 'mdadm'."