Re: [PATCH RFC] qga: Add selinux-helper for guest-exec subcommand (bsc#1237450)

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Mar 27, 2026 at 11:25:19AM +0100, Cathy Hu wrote:
> From: Cathy Hu <cahu@suse.de>
> 
> Problem:
> 
> ATM the QEMU Guest Agent and SELinux are not working together properly.
> The fedora (and therefor also the openSUSE) policy confine the qemu-guest-agent
> service in the domain `qemu_ga_t`. That means, qemu-guest-agent
> is only allowed to do what the policy says.
> 
> However, the `guest-exec` command allows arbitrary execution
> of code from a privileged service, which conflicts with the
> notion of SELinux confinement.
> 
> ATM, the policy allows only some accesses that are used
> by other qemu-guest-agent commands.
> That means, the qemu-guest-agent fails sporadically, depending
> on what is allowed for other commands.
> However, `guest-exec` would need to allow everything.
> 
> see https://bugzilla.suse.com/show_bug.cgi?id=1237450
> 
> Solution:
> 
> This is not an great solution, but it works like this:
> We add a "wrapper" which is executed instead of the program
> that is called via `guest-exec`. The "wrapper" just
> re-executes the command given by `guest-exec`.
> This way, on the SELinux policy side we can give that
> wrapper executable a label on the file system.
> With that label, we can transition into a more broader
> unconfined domain _and_ toggle that transition with a
> SELinux boolean. That would make `guest-exec`
> consistently allowed to execute or not by policy.
> 
> This needs a change on the SELinux policy side to
> accompany this with:
> https://github.com/fedora-selinux/selinux-policy/pull/3122
> 
> What other options have been tried unsuccessfully:
> 
> - Fixing via SELinux policy: It is not possible for
>   one domain to have different permissions depending on
>   code path. It is also not possible to toggle the permissive
>   state via a SELinux boolean, so users would need
>   to add it via semanage.
> - Setting the domain of the executed commands directly
>   to a broader domain with setcon/setexeccon.
>   The SELinux kernel does not allow to spawn a process
>   directly with those that has broader privileges than the parent.
> 
> What other options are there to solve this issue:
> 
> - Making the qemu-guest-agent unconfined by default
> - Document the workaround to use semanage to make the domain permissive
>   if `exec-guest` is needed as works as intended and ignore the problem

IMHO, the 'exec' command should never have been added to the
QEMU guest agent, for precisely this reason that it makes it
impractical to put any meaningful security controls around it.
Likewise the commands which allow arbitrary file read/write.

QEMU guest agent should focus on specific targetted tasks
with dedicated commands.

NB: in RHEL we disable all those unconfinable commands.


If users want to support an ability to have arbitrary command
execution, then that should be done with SSH over VSock, where
the guest owner can choose whether to require authentication
first or not, and use SSH authorized_keys if desired to limit
what commands can be run for a given recorded key.

These days systemd installs magic to allow SSH'ing directly
to a guest using VSOCK addresses, and libvirt further
enhances that to allow SSH'ing to a named VM.


We're repeatedly getting patches to add more functionality to
the "exec" command and they all reflect the fact that what
users need is a general purpose shell with vastly more
functionality than the 'exec' command - authentication,
and authorization being top of the list. IMHO this is not
something QEMU should be trying to address.

> 
> Signed-off-by: Cathy Hu <cahu@suse.de>
> ---
>  qga/commands.c               | 13 +++++++++++++
>  qga/meson.build              |  7 +++++++
>  qga/qemu-ga-selinux-helper.c | 17 +++++++++++++++++
>  3 files changed, 37 insertions(+)
>  create mode 100644 qga/qemu-ga-selinux-helper.c
> 
> diff --git a/qga/commands.c b/qga/commands.c
> index 5f20af25d3..29c092630b 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -30,6 +30,10 @@
>   */
>  #define GUEST_FILE_READ_COUNT_MAX (48 * MiB)
>  
> +#ifdef CONFIG_SELINUX
> +#define GUEST_EXEC_SELINUX_HELPER CONFIG_QEMU_HELPERDIR "/qemu-ga-selinux-helper"
> +#endif
> +
>  /* Note: in some situations, like with the fsfreeze, logging may be
>   * temporarily disabled. if it is necessary that a command be able
>   * to log for accounting purposes, check ga_logging_enabled() beforehand.
> @@ -418,6 +422,9 @@ GuestExec *qmp_guest_exec(const char *path,
>      GuestExecInfo *gei;
>      char **argv, **envp;
>      strList arglist;
> +#ifdef CONFIG_SELINUX
> +    strList helper_arg;
> +#endif
>      gboolean ret;
>      GError *gerr = NULL;
>      gint in_fd, out_fd, err_fd;
> @@ -439,7 +446,13 @@ GuestExec *qmp_guest_exec(const char *path,
>          }
>      }
>  
> +#ifdef CONFIG_SELINUX
> +    helper_arg.value = get_relocated_path(GUEST_EXEC_SELINUX_HELPER);
> +    helper_arg.next = &arglist;
> +    argv = guest_exec_get_args(&helper_arg, true);
> +#else
>      argv = guest_exec_get_args(&arglist, true);
> +#endif
>      envp = has_env ? guest_exec_get_args(env, false) : NULL;
>  
>      flags = G_SPAWN_SEARCH_PATH | G_SPAWN_DO_NOT_REAP_CHILD |
> diff --git a/qga/meson.build b/qga/meson.build
> index 89a4a8f713..61f60fba26 100644
> --- a/qga/meson.build
> +++ b/qga/meson.build
> @@ -125,6 +125,13 @@ qga = executable('qemu-ga', qga_ss.sources() + qga_objs,
>                   install: true)
>  all_qga += qga
>  
> +if selinux.found()
> +  qga_selinux_helper = executable('qemu-ga-selinux-helper', files('qemu-ga-selinux-helper.c'),
> +             install: true,
> +             install_dir: get_option('libexecdir'))
> +  all_qga += qga_selinux_helper
> +endif
> +
>  if host_os == 'windows'
>    qemu_ga_msi_arch = {
>      'x86': ['-D', 'Arch=32'],
> diff --git a/qga/qemu-ga-selinux-helper.c b/qga/qemu-ga-selinux-helper.c
> new file mode 100644
> index 0000000000..a184e74ede
> --- /dev/null
> +++ b/qga/qemu-ga-selinux-helper.c
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#include <stdio.h>
> +#include <glib.h>
> +
> +int main(int argc, char **argv)
> +{
> +    if (argc < 2) {
> +        return EXIT_FAILURE;
> +    }
> +
> +    execvp(argv[1], argv + 1);
> +
> +    int err = errno;
> +    fprintf(stderr, "%s: %s\n", argv[1], strerror(err));
> +
> +    exit(EXIT_FAILURE);
> +}
> -- 
> 2.53.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|