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’.

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.