[Libguestfs] [v2v PATCH] nbdkit, qemuNBD: run_unix: formally require externally provided socket

At this point, virt-v2v never relies on the Unix domain sockets created inside the "run_unix" implementations. Simplify the code by removing this option. Consequently, the internally created temporary directory only holds the NBD server's PID file, and never its UNIX domain socket. Therefore: (1) we no longer need the libguestfs socket dir to be our temp dir, (2) we need not change the file mode bits on the temp dir, (3) we can rename "tmpdir" to the more specific "piddir". Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2066773 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: This patch depends on [PATCH v2v v3] lib: Improve security of in/out sockets when running virt-v2v as root https://listman.redhat.com/archives/libguestfs/2022-March/028460.html with or without the "chmod socket 0o700" suggested in <https://listman.redhat.com/archives/libguestfs/2022-March/028463.html>;. lib/nbdkit.mli | 6 +---- lib/qemuNBD.mli | 6 +---- input/input_disk.ml | 4 ++-- input/input_libvirt.ml | 8 +++---- input/input_ova.ml | 2 +- input/input_vddk.ml | 2 +- input/input_vmx.ml | 4 ++-- input/input_xen_ssh.ml | 2 +- input/vCenter.ml | 2 +- lib/nbdkit.ml | 24 ++++--------------- lib/qemuNBD.ml | 25 ++++---------------- output/output.ml | 4 ++-- output/output_null.ml | 2 +- output/output_rhv_upload.ml | 2 +- 14 files changed, 28 insertions(+), 65 deletions(-) diff --git a/lib/nbdkit.mli b/lib/nbdkit.mli index dc2fd04b2984..5ba83ab04540 100644 --- a/lib/nbdkit.mli +++ b/lib/nbdkit.mli @@ -92,14 +92,10 @@ val add_args : cmd -> (string * string) list -> unit val add_env : cmd -> string -> string -> unit (** Add name=value environment variable. *) -val run_unix : ?socket:string -> cmd -> string * int +val run_unix : string -> cmd -> string * int (** Start nbdkit command listening on a Unix domain socket, waiting for the process to start up. - If optional [?socket] parameter is omitted, then a temporary - Unix domain socket name is created. If [?socket] is present - then this overrides the temporary name. - Returns the Unix domain socket name and the nbdkit process ID. The --exit-with-parent, --foreground, --pidfile, --newstyle and diff --git a/lib/qemuNBD.mli b/lib/qemuNBD.mli index 83871c5bfb9e..e10d3106c7e7 100644 --- a/lib/qemuNBD.mli +++ b/lib/qemuNBD.mli @@ -43,12 +43,8 @@ val set_snapshot : cmd -> bool -> unit val set_format : cmd -> string option -> unit (** Set the format [--format] parameter. *) -val run_unix : ?socket:string -> cmd -> string * int +val run_unix : string -> cmd -> string * int (** Start qemu-nbd command listening on a Unix domain socket, waiting for the process to start up. - If optional [?socket] parameter is omitted, then a temporary - Unix domain socket name is created. If [?socket] is present - then this overrides the temporary name. - Returns the Unix domain socket name and the qemu-nbd process ID. *) diff --git a/input/input_disk.ml b/input/input_disk.ml index bdb8a5103b93..508adf9dd634 100644 --- a/input/input_disk.ml +++ b/input/input_disk.ml @@ -110,7 +110,7 @@ module Disk = struct Nbdkit.add_arg cmd "file" disk; if Nbdkit.version nbdkit_config >= (1, 22, 0) then Nbdkit.add_arg cmd "cache" "none"; - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. @@ -121,7 +121,7 @@ module Disk = struct let cmd = QemuNBD.create disk in QemuNBD.set_snapshot cmd options.read_only; QemuNBD.set_format cmd (Some format); - let _, pid = QemuNBD.run_unix ~socket cmd in + let _, pid = QemuNBD.run_unix socket cmd in On_exit.kill pid ) args; diff --git a/input/input_libvirt.ml b/input/input_libvirt.ml index 2458d1b0eee7..9311e89a2cba 100644 --- a/input/input_libvirt.ml +++ b/input/input_libvirt.ml @@ -88,7 +88,7 @@ and setup_servers options dir disks = Nbdkit.add_arg cmd "hostname" hostname; Nbdkit.add_arg cmd "port" (string_of_int port); Nbdkit.add_arg cmd "shared" "true"; - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. @@ -102,7 +102,7 @@ and setup_servers options dir disks = let cor = dir // "convert" in let cmd = Nbdkit_curl.create_curl ~cor url in - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. @@ -118,7 +118,7 @@ and setup_servers options dir disks = Nbdkit.add_arg cmd "file" filename; if Nbdkit.version nbdkit_config >= (1, 22, 0) then Nbdkit.add_arg cmd "cache" "none"; - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. @@ -130,7 +130,7 @@ and setup_servers options dir disks = let cmd = QemuNBD.create filename in QemuNBD.set_snapshot cmd options.read_only; QemuNBD.set_format cmd format; - let _, pid = QemuNBD.run_unix ~socket cmd in + let _, pid = QemuNBD.run_unix socket cmd in On_exit.kill pid ) disks diff --git a/input/input_ova.ml b/input/input_ova.ml index 1b0e136723c3..8ec1f8026b19 100644 --- a/input/input_ova.ml +++ b/input/input_ova.ml @@ -192,7 +192,7 @@ module OVA = struct let cmd = QemuNBD.create qemu_uri in QemuNBD.set_snapshot cmd options.read_only; (* protective overlay *) QemuNBD.set_format cmd None; (* auto-detect format *) - let _, pid = QemuNBD.run_unix ~socket cmd in + let _, pid = QemuNBD.run_unix socket cmd in On_exit.kill pid ) qemu_uris; diff --git a/input/input_vddk.ml b/input/input_vddk.ml index 23cfd529a2be..e48495d36d34 100644 --- a/input/input_vddk.ml +++ b/input/input_vddk.ml @@ -199,7 +199,7 @@ information on these settings. ?nfchostport ?password_file:options.input_password ?port ~server ?snapshot ~thumbprint ?transports ?user path in - let _, pid = Nbdkit.run_unix ~socket nbdkit in + let _, pid = Nbdkit.run_unix socket nbdkit in On_exit.kill pid ) disks; diff --git a/input/input_vmx.ml b/input/input_vmx.ml index 8ceb0bb0d175..9921419b5a85 100644 --- a/input/input_vmx.ml +++ b/input/input_vmx.ml @@ -69,7 +69,7 @@ module VMX = struct (absolute_path_from_other_file vmx_filename filename) in QemuNBD.set_snapshot cmd true; (* protective overlay *) QemuNBD.set_format cmd (Some "vmdk"); - let _, pid = QemuNBD.run_unix ~socket cmd in + let _, pid = QemuNBD.run_unix socket cmd in On_exit.kill pid ) filenames @@ -111,7 +111,7 @@ module VMX = struct let bandwidth = options.bandwidth in let nbdkit = Nbdkit_ssh.create_ssh ?bandwidth ~cor ~password ~server ?port ?user abs_path in - let _, pid = Nbdkit.run_unix ~socket nbdkit in + let _, pid = Nbdkit.run_unix socket nbdkit in On_exit.kill pid ) filenames ); diff --git a/input/input_xen_ssh.ml b/input/input_xen_ssh.ml index b154a1a5b6bd..0aad36a8befc 100644 --- a/input/input_xen_ssh.ml +++ b/input/input_xen_ssh.ml @@ -121,7 +121,7 @@ module XenSSH = struct let bandwidth = options.bandwidth in let nbdkit = Nbdkit_ssh.create_ssh ?bandwidth ~cor ~password ?port ~server ?user path in - let _, pid = Nbdkit.run_unix ~socket nbdkit in + let _, pid = Nbdkit.run_unix socket nbdkit in On_exit.kill pid ) disks; diff --git a/input/vCenter.ml b/input/vCenter.ml index 40d594f0d6ea..8a1a5655ec8a 100644 --- a/input/vCenter.ml +++ b/input/vCenter.ml @@ -117,7 +117,7 @@ let rec start_nbdkit_for_path ?bandwidth ?cor ?password_file Nbdkit_curl.create_curl ?bandwidth ?cor ~cookie_script ~cookie_script_renew ~sslverify https_url in - let _, pid = Nbdkit.run_unix ~socket nbdkit in + let _, pid = Nbdkit.run_unix socket nbdkit in pid and get_https_url dcPath uri server path = diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml index 9ee6f39ce335..07896684e861 100644 --- a/lib/nbdkit.ml +++ b/lib/nbdkit.ml @@ -102,27 +102,13 @@ let add_env cmd name value = cmd.env <- (name, value) :: cmd.env let add_filter_if_available cmd filter = if probe_filter filter then add_filter cmd filter -let run_unix ?socket cmd = - (* Create a temporary directory where we place the socket and PID file. - * Use the libguestfs socket directory, so it is more likely the full path - * of the UNIX sockets will fit in the (limited) socket pathname. - *) - let tmpdir = - let base_dir = (open_guestfs ())#get_sockdir () in - let t = Mkdtemp.temp_dir ~base_dir "v2vnbdkit." in - (* tmpdir must be readable (but not writable) by "other" so that - * qemu can open the sockets. - *) - chmod t 0o755; - On_exit.rmdir t; - t in +let run_unix socket cmd = + (* Create a temporary directory where we place the PID file. *) + let piddir = Mkdtemp.temp_dir "v2vnbdkit." in + On_exit.rmdir piddir; let id = unique () in - let pidfile = tmpdir // sprintf "nbdkit%d.pid" id in - let socket = - match socket with - | None -> tmpdir // sprintf "nbdkit%d.sock" id - | Some socket -> socket in + let pidfile = piddir // sprintf "nbdkit%d.pid" id in (* Construct the final command line. *) let add_arg, add_args_reversed, get_args = diff --git a/lib/qemuNBD.ml b/lib/qemuNBD.ml index 2c999b9f8f8b..ae21b17c7de2 100644 --- a/lib/qemuNBD.ml +++ b/lib/qemuNBD.ml @@ -62,30 +62,15 @@ let create disk = { disk; snapshot = false; format = None } let set_snapshot cmd snap = cmd.snapshot <- snap let set_format cmd format = cmd.format <- format -let run_unix ?socket { disk; snapshot; format } = +let run_unix socket { disk; snapshot; format } = assert (disk <> ""); - (* Create a temporary directory where we place the socket and PID file. - * Use the libguestfs socket directory, so it is more likely the full path - * of the UNIX sockets will fit in the (limited) socket pathname. - *) - let tmpdir = - let base_dir = (open_guestfs ())#get_sockdir () in - let t = Mkdtemp.temp_dir ~base_dir "v2vqemunbd." in - (* tmpdir must be readable (but not writable) by "other" so that - * qemu can open the sockets. - *) - chmod t 0o755; - On_exit.rmdir t; - t in + (* Create a temporary directory where we place the PID file. *) + let piddir = Mkdtemp.temp_dir "v2vqemunbd." in + On_exit.rmdir piddir; let id = unique () in - let pidfile = tmpdir // sprintf "qemunbd%d.pid" id in - - let socket = - match socket with - | Some socket -> socket - | None -> tmpdir // sprintf "qemunbd%d.sock" id in + let pidfile = piddir // sprintf "qemunbd%d.pid" id in (* Construct the qemu-nbd command line. *) let args = ref [] in diff --git a/output/output.ml b/output/output.ml index 7256b547e080..10e685c46926 100644 --- a/output/output.ml +++ b/output/output.ml @@ -90,7 +90,7 @@ let output_to_local_file ?(changeuid = fun f -> f ()) let cmd = Nbdkit.add_arg cmd "cache" "none" in cmd ); - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. @@ -101,7 +101,7 @@ let output_to_local_file ?(changeuid = fun f -> f ()) let cmd = QemuNBD.create filename in QemuNBD.set_snapshot cmd false; QemuNBD.set_format cmd (Some "qcow2"); - let _, pid = QemuNBD.run_unix ~socket cmd in + let _, pid = QemuNBD.run_unix socket cmd in On_exit.kill pid | _ -> diff --git a/output/output_null.ml b/output/output_null.ml index 86d81eaa7b64..c8e27c0b0aaf 100644 --- a/output/output_null.ml +++ b/output/output_null.ml @@ -70,7 +70,7 @@ module Null = struct let () = let cmd = Nbdkit.create ~quiet:true "null" in Nbdkit.add_arg cmd "size" "7E"; - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in (* --exit-with-parent should ensure nbdkit is cleaned * up when we exit, but it's not supported everywhere. diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml index 72463e572f6d..828996b36261 100644 --- a/output/output_rhv_upload.ml +++ b/output/output_rhv_upload.ml @@ -398,7 +398,7 @@ e command line has to match the number of guest disk images (for this guest: %d) Nbdkit.add_arg cmd "insecure" "true"; if is_ovirt_host then Nbdkit.add_arg cmd "is_ovirt_host" "true"; - let _, pid = Nbdkit.run_unix ~socket cmd in + let _, pid = Nbdkit.run_unix socket cmd in List.push_front pid nbdkit_pids ) (List.combine disks disk_uuids); -- 2.19.1.3.g30247aa5d201