[Libguestfs] [libguestfs-common PATCH] options: attempt naming all decrypted LUKS devices by UUID

[Laszlo Ersek]

On 04/11/22 14:37, Richard W.M. Jones wrote:
> On Mon, Apr 11, 2022 at 02:09:52PM +0200, Laszlo Ersek wrote:
>> In commit 2d8c0f8d40b5 ("options: decrypt LUKS-on-LV devices",
>> 2022-02-28), in order to keep that change as contained as possible, we
>> didn't modify the naming scheme of those decrypted LUKS devices that
>> originated directly from partitions -- we passed "name_decrypted_by_uuid =
>> false" for partitions fetched with guestfs_list_partitions().
>>
>> Turns out that this is exactly what prevents us from decrypting the
>> following block device structure (seen in RHEL6 guests; for example one
>> installed from "RHEL-6.10-20180525.0-Server-x86_64-dvd1.iso"):
>>
>>> NAME                                                 MAJ:MIN RM  SIZE RO TYPE  MOUNTPOINT
>>> vda                                                  252:0    0    9G  0 disk
>>> ├─vda1                                               252:1    0    1G  0 part  /boot
>>> ├─vda2                                               252:2    0    7G  0 part
>>> │ └─luks-37f5c9df-acda-4955-8cfd-872f0da5e35b (dm-0) 253:0    0    7G  0 crypt /
>>> └─vda3                                               252:3    0 1023M  0 part  [SWAP]
>>> sr0                                                   11:0    1 1024M  0 rom
>>
>> The problem is that we prefer (a) make_mapname() due to the LUKS header
>> residing directly on a partition, so we call the plaintext device
>> "/dev/mapper/cryptsda2"; however (b) "/etc/fstab" in the guest refers to
>> the same plaintext device by the standard, UUID-based
>> "/dev/mapper/luks-37f5c9df-acda-4955-8cfd-872f0da5e35b" pathname.
>> Therefore "inspect_get_mountpoints" in "libguestfs/daemon/inspect.ml"
>> returns the latter pathname -- which we can't mount.
>>
>> Hardwire "name_decrypted_by_uuid = true" in "options/decrypt.c" -- by
>> which effort we can as well remove the "name_decrypted_by_uuid" parameter.
>>
>> Testing: the libguestfs, guestfs-tools, and virt-v2v test suites (make
>> check) pass with this update. Furthermore, "guestfish -i", virt-inspector,
>> and virt-v2v now recognize the above blockdev / fs structure (and the
>> converted guest boots).
>>
>> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1658128
>> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
>> ---
>>  options/decrypt.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/options/decrypt.c b/options/decrypt.c
>> index b899a0028620..1cd7b627e264 100644
>> --- a/options/decrypt.c
>> +++ b/options/decrypt.c
>> @@ -111,15 +111,15 @@ make_mapname (const char *device)
>>    }
>>  
>>    return mapname;
>>  }
>>  
>>  static bool
>>  decrypt_mountables (guestfs_h *g, const char * const *mountables,
>> -                    struct key_store *ks, bool name_decrypted_by_uuid)
>> +                    struct key_store *ks)
>>  {
>>    bool decrypted_some = false;
>>    const char * const *mnt_scan = mountables;
>>    const char *mountable;
>>  
>>    while ((mountable = *mnt_scan++) != NULL) {
>>      CLEANUP_FREE char *type = NULL;
>> @@ -144,16 +144,15 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
>>      /* Grab the keys that we should try with this device, based on device name,
>>       * or UUID (if any).
>>       */
>>      keys = get_keys (ks, mountable, uuid);
>>      assert (keys[0] != NULL);
>>  
>>      /* Generate a node name for the plaintext (decrypted) device node. */
>> -    if (!name_decrypted_by_uuid || uuid == NULL ||
>> -        asprintf (&mapname, "luks-%s", uuid) == -1)
>> +    if (uuid == NULL || asprintf (&mapname, "luks-%s", uuid) == -1)
>>        mapname = make_mapname (mountable);
>>  
>>      /* Try each key in turn. */
>>      key_scan = (const char * const *)keys;
>>      while ((key = *key_scan++) != NULL) {
>>        int r;
>>  
>> @@ -188,20 +187,19 @@ 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,
>> -                                    false);
>> +  need_rescan = decrypt_mountables (g, (const char * const *)partitions, ks);
>>  
>>    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);
>> +  decrypt_mountables (g, (const char * const *)lvs, ks);
>>  }
>>
>> base-commit: ab708d11d832457d2a0c74e7a6d8c219a4fdd90f
> 
> ACK

libguestfs-common commit e96698865bf5 ("options: attempt naming all decrypted LUKS devices by UUID", 2022-04-12)
libguestfs commit 05419dbcec71 ("Update common submodule", 2022-04-12)
guestfs-tools commit 8418b44d32c3 ("Update common submodule", 2022-04-12)
virt-v2v commit 68211371411d ("Update common submodule", 2022-04-12)

Thanks
Laszlo