rpms/guestfs-tools
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix virt-sysprep and LUKS-on-LVM guests

Browse Source 
resolves: rhbz#2106286
This commit is contained in:
Richard W.M. Jones 2022-07-15 15:55:07 +01:00
parent 1df9b8cb6b
commit b6a23f03ef
4 changed files with 277 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
0011-sysprep-make-an-effort-to-cope-with-LUKS-on-LVM.patch Normal file
View File

@ -0,0 +1,87 @@
From d15d829d20c1a0d21da584257c4634517d4271d1 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 14 Jul 2022 12:40:04 +0200
Subject: [PATCH] sysprep: make an effort to cope with LUKS-on-LVM
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@redhat.com>
Message-Id: <20220714104005.8334-2-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(cherry picked from commit 361a447bcb7aef399abad8075ee41197c4071f71)
---
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 c67b21487..5fc623039 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.31.1

39
0012-sysprep-advise-against-cloning-VMs-with-internal-ful.patch Normal file
View File

@ -0,0 +1,39 @@
From 0b92347337e9201140ed2daf77a934c731de6630 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Thu, 14 Jul 2022 12:40:05 +0200
Subject: [PATCH] sysprep: advise against cloning VMs with internal full disk
encryption
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@redhat.com>
Message-Id: <20220714104005.8334-3-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(cherry picked from commit b49ee909f5d1a0d7b5c668335b9098ca8ff85bfd)
---
sysprep/virt-sysprep.pod | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/sysprep/virt-sysprep.pod b/sysprep/virt-sysprep.pod
index deeb5341e..232b9f24b 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.31.1

144
0013-builder-dib-Replace-On_exit.rmdir-with-On_exit.rm_rf.patch Normal file
View File

@ -0,0 +1,144 @@
From 3576da023fb42ceaea80b81aebad345de606a332 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Fri, 15 Jul 2022 08:55:53 +0100
Subject: [PATCH] builder, dib: Replace On_exit.rmdir with On_exit.rm_rf
Update common submodule.
(cherry picked from commit f5baf83e464c276d3dae6f8e878b8f47fe0d43d9)
---
builder/builder.ml | 2 +-
builder/index_parser_tests.ml | 2 +-
builder/repository_main.ml | 2 +-
common | 2 +-
dib/dib.ml | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builder/builder.ml b/builder/builder.ml
index 2c9c83085..e34aae6c7 100644
--- a/builder/builder.ml
+++ b/builder/builder.ml
@@ -182,7 +182,7 @@ let main () =
* create.
*)
let tmpdir = Mkdtemp.temp_dir "virt-builder." in
- On_exit.rmdir tmpdir;
+ On_exit.rm_rf tmpdir;
(* Download the sources. *)
let downloader = Downloader.create ~curl:cmdline.curl ~cache ~tmpdir in
diff --git a/builder/index_parser_tests.ml b/builder/index_parser_tests.ml
index 39983faba..5262a1607 100644
--- a/builder/index_parser_tests.ml
+++ b/builder/index_parser_tests.ml
@@ -28,7 +28,7 @@ open Tools_utils
let tmpdir =
let tmpdir = Mkdtemp.temp_dir "guestfs-tests." in
- On_exit.rmdir tmpdir;
+ On_exit.rm_rf tmpdir;
tmpdir
let dummy_sigchecker = Sigchecker.create ~gpg:"gpg"
diff --git a/builder/repository_main.ml b/builder/repository_main.ml
index c5b656310..c24729c4c 100644
--- a/builder/repository_main.ml
+++ b/builder/repository_main.ml
@@ -420,7 +420,7 @@ let main () =
(* Create a temporary folder to work in *)
let tmpdir = Mkdtemp.temp_dir ~base_dir:cmdline.repo
"virt-builder-repository." in
- On_exit.rmdir tmpdir;
+ On_exit.rm_rf tmpdir;
let tmprepo = tmpdir // "repo" in
mkdir_p tmprepo 0o700;
Submodule common af6cb55bc..fd964c1ba:
diff --git a/common/mlcustomize/guest_packages.ml b/common/mlcustomize/guest_packages.ml
index 4c3c34e..7c29a2a 100644
--- a/common/mlcustomize/guest_packages.ml
+++ b/common/mlcustomize/guest_packages.ml
@@ -73,9 +73,9 @@ let install_command packages package_management =
| "zypper" -> sprintf "zypper -n in -l %s" quoted_args
| "unknown" ->
- error_unknown_package_manager (s_"--install")
+ error_unknown_package_manager "--install"
| pm ->
- error_unimplemented_package_manager (s_"--install") pm
+ error_unimplemented_package_manager "--install" pm
let update_command package_management =
match package_management with
@@ -103,9 +103,9 @@ let update_command package_management =
| "zypper" -> "zypper -n update -l"
| "unknown" ->
- error_unknown_package_manager (s_"--update")
+ error_unknown_package_manager "--update"
| pm ->
- error_unimplemented_package_manager (s_"--update") pm
+ error_unimplemented_package_manager "--update" pm
let uninstall_command packages package_management =
let quoted_args = String.concat " " (List.map quote packages) in
@@ -127,6 +127,6 @@ let uninstall_command packages package_management =
| "zypper" -> sprintf "zypper -n rm %s" quoted_args
| "unknown" ->
- error_unknown_package_manager (s_"--uninstall")
+ error_unknown_package_manager "--uninstall"
| pm ->
- error_unimplemented_package_manager (s_"--uninstall") pm
+ error_unimplemented_package_manager "--uninstall" pm
diff --git a/common/mltools/on_exit.ml b/common/mltools/on_exit.ml
index 53ccb68..cae12e7 100644
--- a/common/mltools/on_exit.ml
+++ b/common/mltools/on_exit.ml
@@ -52,7 +52,7 @@ let do_actions () =
List.iter (do_action (fun file -> Unix.unlink file)) !files;
List.iter (do_action (
fun dir ->
- let cmd = sprintf "rm -rf %s" (Filename.quote dir) in
+ let cmd = sprintf "rm -rf -- %s" (Filename.quote dir) in
ignore (Tools_utils.shell_command cmd)
)
) !rmdirs;
@@ -102,7 +102,7 @@ let unlink filename =
register ();
List.push_front filename files
-let rmdir dir =
+let rm_rf dir =
register ();
List.push_front dir rmdirs
diff --git a/common/mltools/on_exit.mli b/common/mltools/on_exit.mli
index a02e3db..9bcf104 100644
--- a/common/mltools/on_exit.mli
+++ b/common/mltools/on_exit.mli
@@ -47,7 +47,7 @@ val f : (unit -> unit) -> unit
val unlink : string -> unit
(** Unlink a single temporary file on exit. *)
-val rmdir : string -> unit
+val rm_rf : string -> unit
(** Recursively remove a temporary directory on exit (using [rm -rf]). *)
val kill : ?signal:int -> int -> unit
diff --git a/dib/dib.ml b/dib/dib.ml
index f5ce604c8..a4ba36040 100644
--- a/dib/dib.ml
+++ b/dib/dib.ml
@@ -550,7 +550,7 @@ let main () =
let image_basename_d = image_basename ^ ".d" in
let tmpdir = Mkdtemp.temp_dir "dib." in
- On_exit.rmdir tmpdir;
+ On_exit.rm_rf tmpdir;
let auxtmpdir = tmpdir // "in_target.aux" in
do_mkdir auxtmpdir;
let hookstmpdir = auxtmpdir // "hooks" in
--
2.31.1

9
guestfs-tools.spec
View File

@ -26,7 +26,7 @@
Summary: Tools to access and modify virtual machine disk images
Name: guestfs-tools
Version: 1.48.2
Release: 4%{?dist}
Release: 5%{?dist}
License: GPLv2+
# Build only for architectures that have a kernel
@ -65,6 +65,9 @@ Patch0007: 0007-cat-log-ls-tail-diff-edit-insp.-set-networking-for-k.patch
Patch0008: 0008-get-kernel-sparsify-set-networking-for-key-ID-clevis.patch
Patch0009: 0009-customize-add-reminder-about-key-ID-clevis.patch
Patch0010: 0010-sysprep-set-networking-for-key-ID-clevis.patch
Patch0011: 0011-sysprep-make-an-effort-to-cope-with-LUKS-on-LVM.patch
Patch0012: 0012-sysprep-advise-against-cloning-VMs-with-internal-ful.patch
Patch0013: 0013-builder-dib-Replace-On_exit.rmdir-with-On_exit.rm_rf.patch
%if 0%{patches_touch_autotools}
BuildRequires: autoconf, automake, libtool, gettext-devel
@ -435,7 +438,7 @@ end
%changelog
* Sat Jul 02 2022 Richard W.M. Jones <rjones@redhat.com> - 1.48.2-4
* Fri Jul 15 2022 Richard W.M. Jones <rjones@redhat.com> - 1.48.2-5
- Rebase to guestfs-tools 1.48.2
resolves: rhbz#2059286
- Default to --selinux-relabel in various tools
@ -447,6 +450,8 @@ end
resolves: rhbz#1809453
- Fix CVE-2022-2211 Denial of Service in --key parameter
resolves: rhbz#2102721
- Fix virt-sysprep and LUKS-on-LVM guests
resolves: rhbz#2106286
* Sat Dec 04 2021 Richard W.M. Jones <rjones@redhat.com> - 1.46.1-6
- Clean up NetworkManager connection files