libxl: add support for running bootloader in restricted mode

Much like the device model depriv mode, add the same kind of support for the
bootloader.  Such feature allows passing a UID as a parameter for the
bootloader to run as, together with the bootloader itself taking the necessary
actions to isolate.

Note that the user to run the bootloader as must have the right permissions to
access the guest disk image (in read mode only), and that the bootloader will
be run in non-interactive mode when restricted.

If enabled bootloader restrict mode will attempt to re-use the user(s) from the
QEMU depriv implementation if no user is provided on the configuration file or
the environment.  See docs/features/qemu-deprivilege.pandoc for more
information about how to setup those users.

Bootloader restrict mode is not enabled by default as it requires certain
setup to be done first (setup of the user(s) to use in restrict mode).

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index 9ba22a8fa2229f71621a22673f8a6661cc9a836f..73e2b3b6114c99d633f0f27de0ef2391517e8a7c 100644 (file)
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -1963,6 +1963,30 @@ ignored:
 
 =back
 
+=head1 ENVIRONMENT VARIABLES
+
+The following environment variables shall affect the execution of xl:
+
+=over 4
+
+=item LIBXL_BOOTLOADER_RESTRICT
+
+Equivalent to L<xl.cfg(5)> B<bootloader_restrict> option.  Provided for
+compatibility reasons.  Having this variable set is equivalent to enabling
+the option, even if the value is 0.
+
+If set takes precedence over L<xl.cfg(5)> and L<xl.conf(5)>
+B<bootloader_restrict> options.
+
+=item LIBXL_BOOTLOADER_USER
+
+Equivalent to L<xl.cfg(5)> B<bootloader_user> option.  Provided for
+compatibility reasons.
+
+If set takes precedence over L<xl.cfg(5)> B<bootloader_user> option.
+
+=back
+
 =head1 SEE ALSO
 
 The following man pages:

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index ec4864958e0eb8081b2843ea904937b53c7a3f22..2e234b450efbcfd2408441fbd20159cbbf722189 100644 (file)
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1694,6 +1694,28 @@ Append B<ARG>s to the arguments to the B<bootloader>
 program. Alternatively if the argument is a simple string then it will
 be split into words at whitespace B<(this second option is deprecated)>.
 
+=item B<bootloader_restrict=BOOLEAN>
+
+Attempt to restrict the bootloader after startup, to limit the
+consequences of security vulnerabilities due to parsing guest
+owned image files.
+
+See docs/features/qemu-deprivilege.pandoc for more information
+on how to setup the unprivileged users.
+
+Note that running the bootloader in restricted mode also implies using
+non-interactive mode, and the disk image must be readable by the
+restricted user.
+
+=item B<bootloader_user=USERNAME>
+
+When using bootloader_restrict, run the bootloader as this user.  If not
+set the default QEMU restrict users will be used.
+
+NOTE: Each domain MUST have a SEPARATE username.
+
+See docs/features/qemu-deprivilege.pandoc for more information.
+
 =item B<e820_host=BOOLEAN>
 
 Selects whether to expose the host e820 (memory map) to the guest via
@@ -2736,6 +2758,27 @@ Append B<ARG>s to the arguments to the B<bootloader>
 program. Alternatively if the argument is a simple string then it will
 be split into words at whitespace B<(this second option is deprecated)>.
 
+=item B<bootloader_restrict=BOOLEAN>
+
+Attempt to restrict the bootloader after startup, to limit the
+consequences of security vulnerabilities due to parsing guest
+owned image files.
+
+See docs/features/qemu-deprivilege.pandoc for more information
+on how to setup the unprivileged users.
+
+Note that running the bootloader in restricted mode also implies using
+non-interactive mode, and the disk image must be readable by the
+restricted user.
+
+=item B<bootloader_user=USERNAME>
+
+When using bootloader_restrict, run the bootloader as this user.
+
+NOTE: Each domain MUST have a SEPARATE username.
+
+See docs/features/qemu-deprivilege.pandoc for more information.
+
 =item B<timer_mode="MODE">
 
 Specifies the mode for Virtual Timers. The valid values are as follows:

diff --git a/docs/man/xl.conf.5.pod.in b/docs/man/xl.conf.5.pod.in
index df20c08137bfaef7425a3518ac583e327dda4315..44738b80bf155e0eba3dc94c33e0d1636eb2f846 100644 (file)
--- a/docs/man/xl.conf.5.pod.in
+++ b/docs/man/xl.conf.5.pod.in
@@ -220,6 +220,12 @@ Due to bug(s), these options may not interact well with other options
 concerning CPU affinity. One example is CPU pools. Users should always double
 check that the required affinity has taken effect.
 
+=item B<bootloader_restrict=BOOLEAN>
+
+System wide default for whether the bootloader should be run in a restricted
+environment.  See L<xl.cfg(5)> B<bootloader_restrict> for more information on
+how to setup and use the option.
+
 =back
 
 =head1 SEE ALSO

diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index abc5fd52da97e90de402aa1c7796496f16dc609f..907aa0a3303aeb71898ca2a65e7bc674d766f94b 100644 (file)
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -600,6 +600,14 @@
  * first ABI incompatible change in a development branch.
  */
 
+#define LIBXL_HAVE_BOOTLOADER_RESTRICT 1
+/*
+ * LIBXL_HAVE_BOOTLOADER_RESTRICT indicates the presence of the
+ * bootloader_restrict and bootloader_user fields in libxl_domain_build_info.
+ * Such fields signal the need to pass a --runas parameter to the bootloader
+ * executable in order to not run it as the same user as libxl.
+ */
+
 /*
  * libxl memory management
  *

diff --git a/tools/libs/light/libxl_bootloader.c b/tools/libs/light/libxl_bootloader.c
index 108329b4a5bb219c1ab1dc19a8f56984110035bf..d732367fc0535a0a48efba7461f42f3a27ac7b33 100644 (file)
--- a/tools/libs/light/libxl_bootloader.c
+++ b/tools/libs/light/libxl_bootloader.c
@@ -14,6 +14,7 @@
 
 #include "libxl_osdeps.h" /* must come before any other headers */
 
+#include <pwd.h>
 #include <termios.h>
 #ifdef HAVE_UTMP_H
 #include <utmp.h>
@@ -42,8 +43,71 @@ static void bootloader_arg(libxl__bootloader_state *bl, const char *arg)
     bl->args[bl->nargs++] = arg;
 }
 
-static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
-                                 const char *bootloader_path)
+static int bootloader_uid(libxl__gc *gc, domid_t guest_domid,
+                          const char *user, uid_t *intended_uid)
+{
+    struct passwd *user_base, user_pwbuf;
+    int rc;
+
+    if (user) {
+        rc = userlookup_helper_getpwnam(gc, user, &user_pwbuf, &user_base);
+        if (rc) return rc;
+
+        if (!user_base) {
+            LOGD(ERROR, guest_domid, "Couldn't find user %s", user);
+            return ERROR_INVAL;
+        }
+
+        *intended_uid = user_base->pw_uid;
+        return 0;
+    }
+
+    /* Re-use QEMU user range for the bootloader. */
+    rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_RANGE_BASE,
+                                    &user_pwbuf, &user_base);
+    if (rc) return rc;
+
+    if (user_base) {
+        struct passwd *user_clash, user_clash_pwbuf;
+        uid_t temp_uid = user_base->pw_uid + guest_domid;
+
+        rc = userlookup_helper_getpwuid(gc, temp_uid, &user_clash_pwbuf,
+                                        &user_clash);
+        if (rc) return rc;
+
+        if (user_clash) {
+            LOGD(ERROR, guest_domid,
+                 "wanted to use uid %ld (%s + %d) but that is user %s !",
+                 (long)temp_uid, LIBXL_QEMU_USER_RANGE_BASE,
+                 guest_domid, user_clash->pw_name);
+            return ERROR_INVAL;
+        }
+
+        *intended_uid = temp_uid;
+        return 0;
+    }
+
+    rc = userlookup_helper_getpwnam(gc, LIBXL_QEMU_USER_SHARED, &user_pwbuf,
+                                    &user_base);
+    if (rc) return rc;
+
+    if (user_base) {
+        LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s",
+             LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED);
+        *intended_uid = user_base->pw_uid;
+
+        return 0;
+    }
+
+    LOGD(ERROR, guest_domid,
+    "Could not find user %s or range base pseudo-user %s, cannot restrict",
+         LIBXL_QEMU_USER_SHARED, LIBXL_QEMU_USER_RANGE_BASE);
+
+    return ERROR_INVAL;
+}
+
+static int make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
+                                const char *bootloader_path)
 {
     const libxl_domain_build_info *info = bl->info;
 
@@ -61,6 +125,22 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
         ARG(GCSPRINTF("--ramdisk=%s", info->ramdisk));
     if (info->cmdline && *info->cmdline != '\0')
         ARG(GCSPRINTF("--args=%s", info->cmdline));
+    if (libxl_defbool_val(info->bootloader_restrict)) {
+        uid_t uid = -1;
+        int rc = bootloader_uid(gc, bl->domid, info->bootloader_user,
+                                &uid);
+
+        if (rc) return rc;
+
+        assert(uid != -1);
+        if (!uid) {
+            LOGD(ERROR, bl->domid, "bootloader restrict UID is 0 (root)!");
+            return ERROR_INVAL;
+        }
+        LOGD(DEBUG, bl->domid, "using uid %ld", (long)uid);
+        ARG(GCSPRINTF("--runas=%ld", (long)uid));
+        ARG("--quiet");
+    }
 
     ARG(GCSPRINTF("--output=%s", bl->outputpath));
     ARG("--output-format=simple0");
@@ -79,6 +159,7 @@ static void make_bootloader_args(libxl__gc *gc, libxl__bootloader_state *bl,
     /* Sentinel for execv */
     ARG(NULL);
 
+    return 0;
 #undef ARG
 }
 
@@ -443,7 +524,8 @@ static void bootloader_disk_attached_cb(libxl__egc *egc,
             bootloader = bltmp;
     }
 
-    make_bootloader_args(gc, bl, bootloader);
+    rc = make_bootloader_args(gc, bl, bootloader);
+    if (rc) goto out;
 
     bl->openpty.ao = ao;
     bl->openpty.callback = bootloader_gotptys;

diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index c91059d713091a8282c3f0c439f3bd2c1e69d28b..ce1d4311033671bcd1dd096e2f8e635176da1fd3 100644 (file)
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -482,6 +482,17 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
         return -ERROR_INVAL;
     }
 
+    /* Assume that providing a bootloader user implies enabling restrict. */
+    libxl_defbool_setdefault(&b_info->bootloader_restrict,
+                             !!b_info->bootloader_user);
+    /* ENV takes precedence over provided domain_build_info. */
+    if (getenv("LIBXL_BOOTLOADER_RESTRICT") ||
+        getenv("LIBXL_BOOTLOADER_USER"))
+        libxl_defbool_set(&b_info->bootloader_restrict, true);
+    if(getenv("LIBXL_BOOTLOADER_USER"))
+        b_info->bootloader_user =
+            libxl__strdup(gc, getenv("LIBXL_BOOTLOADER_USER"));
+
     return 0;
 }
 

diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
index fc264a3a13a6a5a956ea25b7d8d1c4adb507cc71..14b593110f7c509d7189d56c01210930ba721d21 100644 (file)
--- a/tools/libs/light/libxl_dm.c
+++ b/tools/libs/light/libxl_dm.c
@@ -80,10 +80,10 @@ static int libxl__create_qemu_logfile(libxl__gc *gc, char *name)
  *  On error, return a libxl-style error code.
  */
 #define DEFINE_USERLOOKUP_HELPER(NAME,SPEC_TYPE,STRUCTNAME,SYSCONF)     \
-    static int userlookup_helper_##NAME(libxl__gc *gc,                  \
-                                        SPEC_TYPE spec,                 \
-                                        struct STRUCTNAME *resultbuf,   \
-                                        struct STRUCTNAME **out)        \
+    int userlookup_helper_##NAME(libxl__gc *gc,                         \
+                                 SPEC_TYPE spec,                        \
+                                 struct STRUCTNAME *resultbuf,          \
+                                 struct STRUCTNAME **out)               \
     {                                                                   \
         struct STRUCTNAME *resultp = NULL;                              \
         char *buf = NULL;                                               \

diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index b1a7cd9f615b25452e324f40abdd7b7ff0f1cf9d..1219ff8dbd899dc4d333e136d781ab5645b479fd 100644 (file)
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -4874,6 +4874,14 @@ struct libxl__cpu_policy {
     struct xc_msr *msr;
 };
 
+struct passwd;
+_hidden int userlookup_helper_getpwnam(libxl__gc*, const char *user,
+                                       struct passwd *res,
+                                       struct passwd **out);
+_hidden int userlookup_helper_getpwuid(libxl__gc*, uid_t uid,
+                                       struct passwd *res,
+                                       struct passwd **out);
+
 #endif
 
 /*

diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 3bd66291afd4cf488366331cf04de5f0148b5bcb..7d8bd5d21667f5dec3fad30f5600ba66891c000a 100644 (file)
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -624,6 +624,8 @@ libxl_domain_build_info = Struct("domain_build_info",[
     ("acpi",             libxl_defbool),
     ("bootloader",       string),
     ("bootloader_args",  libxl_string_list),
+    ("bootloader_restrict", libxl_defbool),
+    ("bootloader_user",  string),
     ("timer_mode",       libxl_timer_mode),
     ("nested_hvm",       libxl_defbool),
     ("apic",             libxl_defbool),

diff --git a/tools/xl/xl.c b/tools/xl/xl.c
index 2d1ec18ea30f8103af64e64a108af4b7656f0e20..ec72ca60c32ab6566e76d64d38963d3321e4b581 100644 (file)
--- a/tools/xl/xl.c
+++ b/tools/xl/xl.c
@@ -57,6 +57,7 @@ int max_grant_frames = -1;
 int max_maptrack_frames = -1;
 int max_grant_version = LIBXL_MAX_GRANT_DEFAULT;
 libxl_domid domid_policy = INVALID_DOMID;
+libxl_defbool bootloader_restrict;
 
 xentoollog_level minmsglevel = minmsglevel_default;
 
@@ -253,6 +254,9 @@ static void parse_global_config(const char *configfile,
             fprintf(stderr, "invalid domid_policy option");
     }
 
+    xlu_cfg_get_defbool(config, "bootloader_restrict",
+                        &bootloader_restrict, 0);
+
     xlu_cfg_destroy(config);
 }
 

diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 3045b5a8e3f07f966cd23e7e4d1cb0ac4904bd7b..9c86bb1d98247c02cdbee4f4bd3ada1855b6acd4 100644 (file)
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -288,6 +288,7 @@ extern libxl_bitmap global_vm_affinity_mask;
 extern libxl_bitmap global_hvm_affinity_mask;
 extern libxl_bitmap global_pv_affinity_mask;
 extern libxl_domid domid_policy;
+extern libxl_defbool bootloader_restrict;
 
 enum output_format {
     OUTPUT_FORMAT_JSON,

diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 0e8c604bbf06171a0afed9178a1556727aa52c88..ed983200c3f812c0960c6b474c2715702e43df3b 100644 (file)
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1700,6 +1700,13 @@ void parse_config_data(const char *config_source,
         exit(-ERROR_FAIL);
     }
 #endif
+    xlu_cfg_get_defbool(config, "bootloader_restrict",
+                        &b_info->bootloader_restrict, 0);
+    if (!libxl_defbool_is_default(bootloader_restrict))
+        libxl_defbool_setdefault(&b_info->bootloader_restrict,
+                                 libxl_defbool_val(bootloader_restrict));
+    xlu_cfg_replace_string(config, "bootloader_user",
+                           &b_info->bootloader_user, 0);
 
     switch (xlu_cfg_get_list_as_string_list(config, "bootloader_args",
                                             &b_info->bootloader_args, 1)) {