152 lines
5.4 KiB
Diff
152 lines
5.4 KiB
Diff
From 6ca02e37d72a81e7e32d4d3eef24d8a0abe3deb2 Mon Sep 17 00:00:00 2001
|
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
|
Date: Tue, 22 Mar 2022 13:53:41 +0000
|
|
Subject: [PATCH] lib: Improve security of in/out sockets when running virt-v2v
|
|
as root
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
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).
|
|
|
|
Additionally we were setting the sockets themselves to 0777 mode.
|
|
|
|
Instead of using directory permissions, change the owner of the
|
|
directory and sockets to precisely give access to the qemu user and no
|
|
one else.
|
|
|
|
Reported-by: Xiaodai Wang
|
|
Thanks: Dr David Gilbert, Daniel Berrangé, Laszlo Ersek
|
|
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2066773
|
|
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
|
|
(cherry picked from commit 4e7f206843735ba24e2034f694a214ef057ee139)
|
|
---
|
|
lib/nbdkit.ml | 3 ++-
|
|
lib/qemuNBD.ml | 3 ++-
|
|
lib/utils.ml | 47 +++++++++++++++++++++++++++++++++++++++++++++--
|
|
lib/utils.mli | 11 +++++++++++
|
|
4 files changed, 60 insertions(+), 4 deletions(-)
|
|
|
|
diff --git a/lib/nbdkit.ml b/lib/nbdkit.ml
|
|
index 85621775..9ee6f39c 100644
|
|
--- a/lib/nbdkit.ml
|
|
+++ b/lib/nbdkit.ml
|
|
@@ -205,6 +205,7 @@ If the messages above are not sufficient to diagnose the problem then add the
|
|
(* Set the regular Unix permissions, in case nbdkit is
|
|
* running as another user.
|
|
*)
|
|
- chmod socket 0o777;
|
|
+ chown_for_libvirt_rhbz_1045069 socket;
|
|
+ chmod socket 0o700;
|
|
|
|
socket, pid
|
|
diff --git a/lib/qemuNBD.ml b/lib/qemuNBD.ml
|
|
index 54139ce0..2c999b9f 100644
|
|
--- a/lib/qemuNBD.ml
|
|
+++ b/lib/qemuNBD.ml
|
|
@@ -150,7 +150,8 @@ If the messages above are not sufficient to diagnose the problem then add the
|
|
(* Set the regular Unix permissions, in case qemu is
|
|
* running as another user.
|
|
*)
|
|
- chmod socket 0o777;
|
|
+ chown_for_libvirt_rhbz_1045069 socket;
|
|
+ chmod socket 0o700;
|
|
|
|
(* We don't need the PID file any longer. *)
|
|
unlink pidfile;
|
|
diff --git a/lib/utils.ml b/lib/utils.ml
|
|
index 876a44c6..7116a4f9 100644
|
|
--- a/lib/utils.ml
|
|
+++ b/lib/utils.ml
|
|
@@ -147,6 +147,50 @@ let backend_is_libvirt () =
|
|
let backend = fst (String.split ":" backend) in
|
|
backend = "libvirt"
|
|
|
|
+let rec chown_for_libvirt_rhbz_1045069 file =
|
|
+ let running_as_root = Unix.geteuid () = 0 in
|
|
+ if running_as_root && backend_is_libvirt () then (
|
|
+ try
|
|
+ let user = Option.default "qemu" (libvirt_qemu_user ()) in
|
|
+ let uid =
|
|
+ if String.is_prefix user "+" then
|
|
+ int_of_string (String.sub user 1 (String.length user - 1))
|
|
+ else
|
|
+ (Unix.getpwnam user).pw_uid in
|
|
+ debug "setting owner of %s to %d:root" file uid;
|
|
+ Unix.chown file uid 0
|
|
+ with
|
|
+ | exn -> (* Print exception, but continue. *)
|
|
+ debug "could not set owner of %s: %s"
|
|
+ file (Printexc.to_string exn)
|
|
+ )
|
|
+
|
|
+(* Get the local user that libvirt uses to run qemu when we are
|
|
+ * running as root. This is returned as an optional string
|
|
+ * containing the username. The username might be "+NNN"
|
|
+ * meaning a numeric UID.
|
|
+ * https://listman.redhat.com/archives/libguestfs/2022-March/028450.html
|
|
+ *)
|
|
+and libvirt_qemu_user =
|
|
+ let user =
|
|
+ lazy (
|
|
+ let conn = Libvirt.Connect.connect_readonly () in
|
|
+ let xml = Libvirt.Connect.get_capabilities conn in
|
|
+ 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
|
|
+ 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 the
|
|
+ * UID part.
|
|
+ *)
|
|
+ Some (fst (String.split ":" uid_gid))
|
|
+ ) in
|
|
+ fun () -> Lazy.force user
|
|
+
|
|
(* 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.
|
|
@@ -159,8 +203,7 @@ 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
|
|
- let running_as_root = Unix.geteuid () = 0 in
|
|
- if running_as_root then Unix.chmod d 0o711;
|
|
+ chown_for_libvirt_rhbz_1045069 d;
|
|
On_exit.rmdir d;
|
|
d
|
|
|
|
diff --git a/lib/utils.mli b/lib/utils.mli
|
|
index c571cca5..d431e21f 100644
|
|
--- a/lib/utils.mli
|
|
+++ b/lib/utils.mli
|
|
@@ -61,6 +61,17 @@ val qemu_img_supports_offset_and_size : unit -> bool
|
|
val backend_is_libvirt : unit -> bool
|
|
(** Return true iff the current backend is libvirt. *)
|
|
|
|
+val chown_for_libvirt_rhbz_1045069 : string -> unit
|
|
+(** If running and root, and if the backend is libvirt, libvirt
|
|
+ will run qemu as a non-root user. This prevents access
|
|
+ to root-owned files and directories. To fix this, provide
|
|
+ a function to chown things we might need to qemu:root so
|
|
+ qemu can access them. Note that root normally ignores
|
|
+ permissions so can still access the resource.
|
|
+
|
|
+ This is best-effort. If something fails then we carry
|
|
+ on and hope for the best. *)
|
|
+
|
|
val error_if_no_ssh_agent : unit -> unit
|
|
|
|
val create_v2v_directory : unit -> string
|
|
--
|
|
2.31.1
|
|
|