Libguestfs - Jul 2022 - [guestfs-tools PATCH 2/2] sysprep: advise against cloning VMs with internal full disk encryption

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2106286

The first patch extends our support for LUKS-on-LVM a bit.

The second patch explains why the functionality from the first patch
should not be relied upon.

Laszlo

Laszlo Ersek (2):
  sysprep: make an effort to cope with LUKS-on-LVM
  sysprep: advise against cloning VMs with internal full disk encryption

 sysprep/sysprep_operation_lvm_uuids.ml | 42 +++++++++++++++++++-
 sysprep/virt-sysprep.pod               |  7 ++++
 2 files changed, 48 insertions(+), 1 deletion(-)

-- 
2.19.1.3.g30247aa5d201

If the guest disk uses the LUKS-on-LVM scheme, then sysprep has a problem:

- the "fs-uuids" blockdev operation depends on the decrypted LUKS
devices
  being open,

- the "lvm-uuids" blockdev operation depends on the same devices being
  closed.

Attempt to deal with this in "lvm-uuids".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2106286
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 sysprep/sysprep_operation_lvm_uuids.ml | 42 +++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/sysprep/sysprep_operation_lvm_uuids.ml
b/sysprep/sysprep_operation_lvm_uuids.ml
index c67b2148765b..5fc623039479 100644
--- a/sysprep/sysprep_operation_lvm_uuids.ml
+++ b/sysprep/sysprep_operation_lvm_uuids.ml
@@ -30,7 +30,46 @@ let rec lvm_uuids_perform g root side_effects        try
g#available [|"lvm2"|]; true with G.Error _ -> false in
     if has_lvm2_feature then (
       let has_pvs, has_vgs = g#pvs () <> [||], g#vgs () <> [||] in
-      if has_pvs || has_vgs then g#vg_activate_all false;
+      if has_pvs || has_vgs then (
+        try g#vg_activate_all false
+        with G.Error _ as exn ->
+          (* If the "luks" feature is not available, re-raise the
exception. *)
+          (try g#available [|"luks"|] with G.Error _ -> raise
exn);
+
+          (* Assume VG deactivation failed due to the guest using the
+           * FS-on-LUKS-on-LVM scheme.
+           *
+           * By now, we have unmounted filesystems, but the decrypted LUKS
+           * devices still keep the LVs open. Therefore, attempt closing all
+           * decrypted LUKS devices that were opened by inspection (i.e.,
device
+           * nodes with pathnames like
"/dev/mapper/luks-<uuid>"). Closing the
+           * decrypted LUKS devices should remove the references from their
+           * underlying LVs, and then VG deactivation should succeed too.
+           *
+           * Note that closing the decrypted LUKS devices prevents the
+           * blockdev-level manipulation of those filesystems that reside on
+           * said decrypted LUKS devices, such as the "fs-uuids"
operation. But
+           * that should be OK, as we order the present operation after all
+           * other block device ops.
+           *
+           * In case the guest uses the FS-on-LVM-on-LUKS scheme, then the
+           * original VG deactivation must have failed for a different reason.
+           * (As we have unmounted filesystems earlier, and LUKS is below, not
+           * on top of, LVM.) The LUKS-closing attempts below will fail then,
+           * due to LVM keeping the decrypted LUKS devices open. This failure
is
+           * harmless and can be considered a no-op. The final, retried VG
+           * deactivation should reproduce the original failure.
+           *)
+          let luks_re = PCRE.compile ("^/dev/mapper/luks" ^
+                                      "-[[:xdigit:]]{8}" ^
+                                      "(?:-[[:xdigit:]]{4}){3}" ^
+                                      "-[[:xdigit:]]{12}$")
+          and dmdevs = Array.to_list (g#list_dm_devices ()) in
+          let plaintext_devs = List.filter (PCRE.matches luks_re) dmdevs in
+          List.iter (fun dev -> try g#cryptsetup_close dev with _ -> ())
+            plaintext_devs;
+          g#vg_activate_all false
+      );
       if has_pvs then g#pvchange_uuid_all ();
       if has_vgs then g#vgchange_uuid_all ();
       if has_pvs || has_vgs then g#vg_activate_all true
@@ -39,6 +78,7 @@ let rec lvm_uuids_perform g root side_effects  
 let op = {
   defaults with
+    order = 99; (* Run it after other block device ops. *)
     name = "lvm-uuids";
     enabled_by_default = true;
     heading = s_"Change LVM2 PV and VG UUIDs";
-- 
2.19.1.3.g30247aa5d201

This is relevant for sysprep because we recommend sysprep for facilitating
cloning.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2106286
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
 sysprep/virt-sysprep.pod | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod
index deeb5341e57c..232b9f24ba27 100644
--- a/sysprep/virt-sysprep.pod
+++ b/sysprep/virt-sysprep.pod
@@ -519,6 +519,13 @@ Either or both options can be used multiple times on the
command line.
 
 =head1 SECURITY
 
+Virtual machines that employ full disk encryption I<internally to the
+guest> should not be considered for cloning and distribution, as it
+provides multiple parties with the same internal volume key, enabling
+any one such party to decrypt all the other clones.  Refer to the L<LUKS
+FAQ|https://gitlab.com/cryptsetup/cryptsetup/-/blob/main/FAQ.md> for
+details.
+
 Although virt-sysprep removes some sensitive information from the
 guest, it does not pretend to remove all of it.  You should examine
 the L</OPERATIONS> above and the guest afterwards.
-- 
2.19.1.3.g30247aa5d201