On 03/22/22 16:48, Richard W.M. Jones wrote:
When using the libvirt backend and running as root, libvirt will run
qemu as a non-root user (eg. qemu:qemu). The v2v directory stores NBD
endpoints that qemu must be able to open and so we set the directory
to mode 0711. Unfortunately this permits any non-root user to open
the sockets (since, by design, they have predictable names within the
directory).
So instead of using directory permissions, use an ACL which allows us
to precisely give access to the qemu user and no one else.
Reported-by: Xiaodai Wang
Thanks: Dr David Gilbert
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=2066773
---
lib/utils.ml | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/lib/utils.ml b/lib/utils.ml
index 757bc73c8e..48e6b6c82d 100644
--- a/lib/utils.ml
+++ b/lib/utils.ml
@@ -146,6 +146,26 @@ let backend_is_libvirt () =
let backend = fst (String.split ":" backend) in
backend = "libvirt"
+(* Get the local user that libvirt uses to run qemu when we are
+ * running as root. This is returned as an optional string
+ * containing either the UID or username.
+ *
https://listman.redhat.com/archives/libguestfs/2022-March/028450.html
+ *)
+let libvirt_qemu_uid () =
+ let conn = Libvirt.Connect.connect_readonly () in
+ let xml = Libvirt.Connect.get_capabilities conn in
Huh, I didn't know this existed already :)
+ let doc = Xml.parse_memory xml in
+ let xpathctx = Xml.xpath_new_context doc in
+ let expr =
"//secmodel[./model=\"dac\"]/baselabel[@type=\"kvm\"]/text()"
in
So if you run "virsh capabilities", you get:
<secmodel>
<model>dac</model>
<doi>0</doi>
<baselabel type='kvm'>+107:+107</baselabel>
<baselabel type='qemu'>+107:+107</baselabel>
</secmodel>
In case libvirt starts the domain with TCG, then I think the
@type='qemu' filter should apply.
(Or am I confusing myself here? I know that the libguestfs appliance may
use TCG, but maybe that is irrelevant here? Sorry!)
+ let uid_gid = Xpath_helpers.xpath_string xpathctx expr in
+ match uid_gid with
+ | None -> None
+ | Some uid_gid ->
+ (* The string will be something like "+107:+107", return
+ * just the UID part.
+ *)
+ Some (fst (String.split ":" uid_gid))
+
(* When using the SSH driver in qemu (currently) this requires
* ssh-agent authentication. Give a clear error if this hasn't been
* set up (RHBZ#1139973). This might improve if we switch to libssh1.
@@ -158,8 +178,20 @@ let error_if_no_ssh_agent () =
(* Create the directory containing inX and outX sockets. *)
let create_v2v_directory () =
let d = Mkdtemp.temp_dir "v2v." in
+ (* If running as root, and if the backend is libvirt, libvirt
+ * will run qemu as a non-root user. Allow qemu to open the directory.
+ *)
let running_as_root = Unix.geteuid () = 0 in
- if running_as_root then Unix.chmod d 0o711;
+ if running_as_root && backend_is_libvirt () then (
+ try
+ let uid = Option.default "qemu" (libvirt_qemu_uid ()) in
+ let cmd = sprintf "setfacl -m user:%s:rwx %s" (quote uid) (quote d) in
Is it OK if we pass "+107" to "setfacl" here?
Thanks,
Laszlo
+ debug "%s" cmd;
+ ignore (Sys.command cmd)
+ with
+ | exn -> (* Print exception, but continue. *)
+ debug "could not set ACL on v2v directory: %s" (Printexc.to_string
exn)
+ );
On_exit.rmdir d;
d