From 3893613fbe781ad5b03c79cf3f18fc6d0cd24611 Mon Sep 17 00:00:00 2001 From: Alejandro Vallejo Date: Thu, 28 Sep 2023 14:42:05 +0000 Subject: [PATCH 1/2] CA-383491: Run pygrub in deprivileged mode when invoked from XAPI pygrub doesn't need all the authority it runs with. Run it in depriv mode instead so breaking into pygrub doesn't cause privilege escalation. Signed-off-by: Alejandro Vallejo --- Makefile | 1 + ocaml/xenopsd/lib/bootloader.ml | 7 ++-- ocaml/xenopsd/lib/bootloader.mli | 1 + ocaml/xenopsd/lib/resources.ml | 2 +- .../xenopsd/scripts/make-custom-xenopsd.conf | 1 + ocaml/xenopsd/scripts/pygrub-wrapper | 33 +++++++++++++++++++ ocaml/xenopsd/xc/xenops_server_xen.ml | 6 ++-- 7 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 ocaml/xenopsd/scripts/pygrub-wrapper diff --git a/Makefile b/Makefile index b36defa6c2a..1d2200a9d5a 100644 --- a/Makefile +++ b/Makefile @@ -184,6 +184,7 @@ install: build doc sdk doc-json install -D ./ocaml/xenopsd/scripts/igmp_query_injector.py $(DESTDIR)/$(XENOPSD_LIBEXECDIR)/igmp_query_injector.py install -D ./ocaml/xenopsd/scripts/qemu-wrapper $(DESTDIR)/$(QEMU_WRAPPER_DIR)/qemu-wrapper install -D ./ocaml/xenopsd/scripts/swtpm-wrapper $(DESTDIR)/$(QEMU_WRAPPER_DIR)/swtpm-wrapper + install -D ./ocaml/xenopsd/scripts/pygrub-wrapper $(DESTDIR)/$(QEMU_WRAPPER_DIR)/pygrub-wrapper DESTDIR=$(DESTDIR) SBINDIR=$(SBINDIR) QEMU_WRAPPER_DIR=$(QEMU_WRAPPER_DIR) XENOPSD_LIBEXECDIR=$(XENOPSD_LIBEXECDIR) ETCDIR=$(ETCDIR) ./ocaml/xenopsd/scripts/make-custom-xenopsd.conf # squeezed install -D _build/install/default/bin/squeezed $(DESTDIR)/$(SBINDIR)/squeezed diff --git a/ocaml/xenopsd/lib/bootloader.ml b/ocaml/xenopsd/lib/bootloader.ml index e66eb9392c4..db87d6cea9e 100644 --- a/ocaml/xenopsd/lib/bootloader.ml +++ b/ocaml/xenopsd/lib/bootloader.ml @@ -59,7 +59,7 @@ exception Error_from_bootloader of string type t = {kernel_path: string; initrd_path: string option; kernel_args: string} (** Helper function to generate a bootloader commandline *) -let command bootloader q pv_bootloader_args image vm_uuid = +let command bootloader q pv_bootloader_args image vm_uuid domid = (* Let's not do anything fancy while parsing the pv_bootloader_args string: no escaping of spaces or quotes for now *) let pv_bootloader_args = @@ -77,6 +77,7 @@ let command bootloader q pv_bootloader_args image vm_uuid = [ ["--output-format=simple"] ; q + ; [Printf.sprintf "--domid=%d" domid] ; (* --vm is unnecessary for pygrub and not supported upstream *) pv_bootloader_args ; image @@ -221,11 +222,11 @@ let sanity_check_path p = (** Extract the default kernel using the -q option *) let extract (task : Xenops_task.task_handle) ~bootloader ~disk ?(legacy_args = "") ?(extra_args = "") ?(pv_bootloader_args = "") - ~vm:vm_uuid () = + ~vm:vm_uuid ~domid:domid () = (* Without this path, pygrub will fail: *) Unixext.mkdir_rec "/var/run/xend/boot" 0o0755 ; let bootloader_path, cmdline = - command bootloader true pv_bootloader_args disk vm_uuid + command bootloader true pv_bootloader_args disk vm_uuid domid in debug "Bootloader commandline: %s %s\n" bootloader_path (String.concat " " cmdline) ; diff --git a/ocaml/xenopsd/lib/bootloader.mli b/ocaml/xenopsd/lib/bootloader.mli index 4a3927e9111..b3ba437e72c 100644 --- a/ocaml/xenopsd/lib/bootloader.mli +++ b/ocaml/xenopsd/lib/bootloader.mli @@ -38,6 +38,7 @@ val extract : -> ?extra_args:string -> ?pv_bootloader_args:string -> vm:string + -> domid:int -> unit -> t (** Extract the default kernel from the disk *) diff --git a/ocaml/xenopsd/lib/resources.ml b/ocaml/xenopsd/lib/resources.ml index 3bab66702db..729242abe6f 100644 --- a/ocaml/xenopsd/lib/resources.ml +++ b/ocaml/xenopsd/lib/resources.ml @@ -28,7 +28,7 @@ let rmmod = ref "/usr/sbin/rmmod" let hvmloader = ref "hvmloader" -let pygrub = ref "pygrub" +let pygrub = ref "pygrub-wrapper" let eliloader = ref "eliloader" diff --git a/ocaml/xenopsd/scripts/make-custom-xenopsd.conf b/ocaml/xenopsd/scripts/make-custom-xenopsd.conf index d873c6439ce..09fe4559268 100755 --- a/ocaml/xenopsd/scripts/make-custom-xenopsd.conf +++ b/ocaml/xenopsd/scripts/make-custom-xenopsd.conf @@ -46,6 +46,7 @@ setup-vif-rules=${LIBEXECDIR}/setup-vif-rules sockets-group=$group qemu-wrapper=${QEMU_WRAPPER_DIR}/qemu-wrapper swtpm-wrapper=${QEMU_WRAPPER_DIR}/qemu-wrapper +pygrub-wrapper=${QEMU_WRAPPER_DIR}/pygrub-wrapper disable-logging-for=http # Workaround xenopsd bug #45 diff --git a/ocaml/xenopsd/scripts/pygrub-wrapper b/ocaml/xenopsd/scripts/pygrub-wrapper new file mode 100644 index 00000000000..03189c4ca9a --- /dev/null +++ b/ocaml/xenopsd/scripts/pygrub-wrapper @@ -0,0 +1,33 @@ +#! /usr/bin/python +# +# Copyright (C) 2023 Cloud Software Group +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published +# by the Free Software Foundation; version 2.1 only. with the special +# exception on linking described in file LICENSE. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. + +from __future__ import print_function +import pwd, subprocess, sys + +cmd = ["pygrub"] + +# Get the usage string. We can't use check_output() because the exit status isn't 0 +pygrub_usage = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE).communicate()[1] + +for arg in sys.argv[1:]: + # Catch the synthetic --domid argument and turn it into --runas + argname_domid = "--domid=" + if arg.startswith(argname_domid): + domid = int(arg[len(argname_domid):]) + uid = pwd.getpwnam('qemu_base').pw_uid + domid + cmd += ["--runas=" + str(uid)] + else: + cmd += [arg] + +sys.exit(subprocess.call(cmd)) diff --git a/ocaml/xenopsd/xc/xenops_server_xen.ml b/ocaml/xenopsd/xc/xenops_server_xen.ml index f76acd083da..72cf7647e77 100644 --- a/ocaml/xenopsd/xc/xenops_server_xen.ml +++ b/ocaml/xenopsd/xc/xenops_server_xen.ml @@ -2099,7 +2099,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id () + ~vm:vm.Vm.id ~domid:domid () in kernel_to_cleanup := Some b ; let builder_spec_info = @@ -2144,7 +2144,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id () + ~vm:vm.Vm.id ~domid:domid () in kernel_to_cleanup := Some b ; let builder_spec_info = @@ -2199,7 +2199,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id () + ~vm:vm.Vm.id ~domid:domid () in kernel_to_cleanup := Some b ; let builder_spec_info = From 7128f603985a6ec0d24934a9e1885f89d80d420e Mon Sep 17 00:00:00 2001 From: Pau Ruiz Safont Date: Wed, 11 Oct 2023 12:33:05 +0100 Subject: [PATCH 2/2] xenopsd: apply ocamlformat Signed-off-by: Pau Ruiz Safont --- ocaml/xenopsd/lib/bootloader.ml | 2 +- ocaml/xenopsd/xc/xenops_server_xen.ml | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ocaml/xenopsd/lib/bootloader.ml b/ocaml/xenopsd/lib/bootloader.ml index db87d6cea9e..8e4012a90d1 100644 --- a/ocaml/xenopsd/lib/bootloader.ml +++ b/ocaml/xenopsd/lib/bootloader.ml @@ -222,7 +222,7 @@ let sanity_check_path p = (** Extract the default kernel using the -q option *) let extract (task : Xenops_task.task_handle) ~bootloader ~disk ?(legacy_args = "") ?(extra_args = "") ?(pv_bootloader_args = "") - ~vm:vm_uuid ~domid:domid () = + ~vm:vm_uuid ~domid () = (* Without this path, pygrub will fail: *) Unixext.mkdir_rec "/var/run/xend/boot" 0o0755 ; let bootloader_path, cmdline = diff --git a/ocaml/xenopsd/xc/xenops_server_xen.ml b/ocaml/xenopsd/xc/xenops_server_xen.ml index 72cf7647e77..4a83e9b18eb 100644 --- a/ocaml/xenopsd/xc/xenops_server_xen.ml +++ b/ocaml/xenopsd/xc/xenops_server_xen.ml @@ -2099,7 +2099,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id ~domid:domid () + ~vm:vm.Vm.id ~domid () in kernel_to_cleanup := Some b ; let builder_spec_info = @@ -2144,7 +2144,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id ~domid:domid () + ~vm:vm.Vm.id ~domid () in kernel_to_cleanup := Some b ; let builder_spec_info = @@ -2199,7 +2199,7 @@ module VM = struct Bootloader.extract task ~bootloader:i.bootloader ~legacy_args:i.legacy_args ~extra_args:i.extra_args ~pv_bootloader_args:i.bootloader_args ~disk:dev - ~vm:vm.Vm.id ~domid:domid () + ~vm:vm.Vm.id ~domid () in kernel_to_cleanup := Some b ; let builder_spec_info =