[Libguestfs] [libguestfs-common PATCH 2/2] options: decrypt LUKS-on-LV devices

[Richard W.M. Jones]

On Wed, Feb 23, 2022 at 05:19:15PM +0100, Laszlo Ersek wrote:
> Using the previously extracted function decrypt_mountables(), look for
> LUKS devices on logical volumes (LVs) too.
> 
> In the LVM-on-LUKS scheme, the names of the plaintext (decrypted) block
> devices don't matter, as these devices host Physical Volumes, and LVM
> enumerates PVs automatically -- there are no references to these decrypted
> block devices in "/etc/fstab", for example. For naming such decrypted
> devices, continue calling make_mapname().
> 
> In the LUKS-on-LVM scheme however, the decrypted devices are supposed to
> hold filesystems, and "/etc/fstab" may refer to them. Such decrypted
> devices are commonly called /dev/mapper/luks-<UUID>, where <UUID> is the
> UUID inside the LUKS header. Employ this naming when decrypting Logical
> Volumes. Reuse make_mapname() as a fallback in this case.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1658126
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1658126
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  options/decrypt.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 9141cf5193ad..f7c1d876b3ed 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -38,8 +38,11 @@
>  #include "options.h"
>  
>  /**
> - * Make a LUKS map name from the partition name,
> - * eg. C<"/dev/vda2" =E<gt> "cryptvda2">
> + * Make a LUKS map name from the partition or logical volume name, eg.
> + * C<"/dev/vda2" =E<gt> "cryptvda2">, or C<"/dev/vg-ssd/lv-root7" =E<gt>
> + * "cryptvgssdlvroot7">.  Note that, in logical volume device names,
> + * c_isalnum() eliminates the "/" separator from between the VG and the LV, so
> + * this mapping is not unique; but for our purposes, it will do.
>   */
>  static void
>  make_mapname (const char *device, char *mapname, size_t len)
> @@ -67,7 +70,7 @@ make_mapname (const char *device, char *mapname, size_t len)
>  
>  static bool
>  decrypt_mountables (guestfs_h *g, const char * const *mountables,
> -                    struct key_store *ks)
> +                    struct key_store *ks, bool name_decrypted_by_uuid)
>  {
>    bool decrypted_some = false;
>    const char * const *mnt_scan = mountables;
> @@ -77,7 +80,7 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
>      CLEANUP_FREE char *type = NULL;
>      CLEANUP_FREE char *uuid = NULL;
>      CLEANUP_FREE_STRING_LIST char **keys = NULL;
> -    char mapname[32];
> +    char mapname[512];
>      const char * const *key_scan;
>      const char *key;
>  
> @@ -102,7 +105,9 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
>      assert (keys[0] != NULL);
>  
>      /* Generate a node name for the plaintext (decrypted) device node. */
> -    make_mapname (mountable, mapname, sizeof mapname);
> +    if (!name_decrypted_by_uuid || uuid == NULL ||
> +        snprintf (mapname, sizeof mapname, "luks-%s", uuid) < 0)
> +      make_mapname (mountable, mapname, sizeof mapname);

This should all really be using asprintf, and not stack-allocating
large arrays.  Although it's kind of not related to this change, this
change does make the stack frame larger.

Saying that, when removing gnulib (libguestfs commit 0f54df53d26), I
dropped the warning about large stack frames:

  -dnl Warn about large stack frames, including estimates for alloca
  -dnl and variable length arrays.
  -gl_WARN_ADD([-Wstack-usage=10000])

I really need to add that back ...

>      /* Try each key in turn. */
>      key_scan = (const char * const *)keys;
> @@ -145,15 +150,22 @@ void
>  inspect_do_decrypt (guestfs_h *g, struct key_store *ks)
>  {
>    CLEANUP_FREE_STRING_LIST char **partitions = guestfs_list_partitions (g);
> +  CLEANUP_FREE_STRING_LIST char **lvs = NULL;
>    bool need_rescan;
>  
>    if (partitions == NULL)
>      exit (EXIT_FAILURE);
>  
> -  need_rescan = decrypt_mountables (g, (const char * const *)partitions, ks);
> +  need_rescan = decrypt_mountables (g, (const char * const *)partitions, ks,
> +                                    false);
>  
>    if (need_rescan) {
>      if (guestfs_lvm_scan (g, 1) == -1)
>        exit (EXIT_FAILURE);
>    }
> +
> +  lvs = guestfs_lvs (g);
> +  if (lvs == NULL)
> +    exit (EXIT_FAILURE);
> +  decrypt_mountables (g, (const char * const *)lvs, ks, true);

ACK

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html