[libvirt PATCH 0/4] qemu/security: start passt process with correct SELinux label
All the necessary explanation is in Path 3/4 We may want to turn on this same behavior for some other external processes, but right now the one we need it for is passt. Resolves: https://bugzilla.redhat.com/2172267 Laine Stump (4): util: add an API to retrieve the resolved path to a virCommand's binary security: make args to virSecuritySELinuxContextAddRange() const security: make it possible to set SELinux label of child process from its binary qemu: set SELinux label of passt process to its own binary's label src/libvirt_private.syms | 1 + src/qemu/qemu_dbus.c | 2 +- src/qemu/qemu_passt.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_security.c | 5 ++- src/qemu/qemu_security.h | 1 + src/qemu/qemu_slirp.c | 2 +- src/qemu/qemu_tpm.c | 3 +- src/qemu/qemu_vhost_user_gpu.c | 2 +- src/security/security_apparmor.c | 1 + src/security/security_dac.c | 1 + src/security/security_driver.h | 1 + src/security/security_manager.c | 8 +++- src/security/security_manager.h | 1 + src/security/security_nop.c | 1 + src/security/security_selinux.c | 77 ++++++++++++++++++++++++++++++-- src/security/security_stack.c | 5 ++- src/util/vircommand.c | 51 ++++++++++++++++----- src/util/vircommand.h | 1 + 19 files changed, 143 insertions(+), 24 deletions(-) -- 2.39.2
The binary to be exec'ed by virExec() is stored in
virCommand::args[0], and is resolved to a full absolute path (stored
in a local of virExec() just prior to execve().
Since we will have another use for the full absolute path, lets make
an API to resolve/retrieve the absolute path, and cache it in
virCommand::binaryPath so we only have to do the resolution once.
Signed-off-by: Laine Stump
Neither of these are modified anywhere in the function, and the
function will soon be called with an arg that actually is a const.
Signed-off-by: Laine Stump
Normally when a child process is started by libvirt, the SELinux label
of that process is set to virtd_t (plus an MCS range). In at least one
case (passt) we need for the SELinux label of a child process label to
match the label that the binary would have transitioned to
automatically if it had been run standalone (in the case of passt,
that label is passt_t).
This patch modifies virSecuritySELinuxSetChildProcessLabel() (and all
the functions above it in the call chain) so that the toplevel
function can set a new argument "useBinarySpecificLabel" to true. If
it is true, then virSecuritySELinuxSetChildProcessLabel() will call
the new function virSecuritySELinuxContextSetFromFile(), which uses
the selinux library function security_compute_create() to determine
what would be the label of the new process if it had been run
standalone (rather than being run by libvirt) - the MCS range from the
normally-used label is added to this newly derived label, and that is
what is used for the new process rather than whatever is in the
domain's security label (which will usually be virtd_t).
In order to easily verify that nothing was broken by these changes to
the call chain, all callers currently set useBinarySpecificPath =
false, so all behavior should be completely unchanged. (The next
patch will set it to true only for the case of running passt.)
https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump
set useBinarySpecificLabel = true when calling qemuSecurityCommandRun
for the passt process, so that the new process context will include
the binary-specific label that should be used for passt (passt_t)
rather than svirt_t (as would happen if useBinarySpecificLabel was
false). (The MCS part of the label, which is common to all child
processes related to a particular qemu domain instance, is also set).
Resolves: https://bugzilla.redhat.com/2172267
Signed-off-by: Laine Stump
On 3/8/23 11:49 PM, Laine Stump wrote:
All the necessary explanation is in Path 3/4
We may want to turn on this same behavior for some other external processes, but right now the one we need it for is passt.
Resolves: https://bugzilla.redhat.com/2172267
I forgot to mention that proper operation requires the latest updates to passt, as well as a patch to selinux-policy that still needs to be posted/merged.
Laine Stump (4): util: add an API to retrieve the resolved path to a virCommand's binary security: make args to virSecuritySELinuxContextAddRange() const security: make it possible to set SELinux label of child process from its binary qemu: set SELinux label of passt process to its own binary's label
src/libvirt_private.syms | 1 + src/qemu/qemu_dbus.c | 2 +- src/qemu/qemu_passt.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_security.c | 5 ++- src/qemu/qemu_security.h | 1 + src/qemu/qemu_slirp.c | 2 +- src/qemu/qemu_tpm.c | 3 +- src/qemu/qemu_vhost_user_gpu.c | 2 +- src/security/security_apparmor.c | 1 + src/security/security_dac.c | 1 + src/security/security_driver.h | 1 + src/security/security_manager.c | 8 +++- src/security/security_manager.h | 1 + src/security/security_nop.c | 1 + src/security/security_selinux.c | 77 ++++++++++++++++++++++++++++++-- src/security/security_stack.c | 5 ++- src/util/vircommand.c | 51 ++++++++++++++++----- src/util/vircommand.h | 1 + 19 files changed, 143 insertions(+), 24 deletions(-)
On 3/9/23 05:49, Laine Stump wrote:
All the necessary explanation is in Path 3/4
We may want to turn on this same behavior for some other external processes, but right now the one we need it for is passt.
Resolves: https://bugzilla.redhat.com/2172267
Laine Stump (4): util: add an API to retrieve the resolved path to a virCommand's binary security: make args to virSecuritySELinuxContextAddRange() const security: make it possible to set SELinux label of child process from its binary qemu: set SELinux label of passt process to its own binary's label
src/libvirt_private.syms | 1 + src/qemu/qemu_dbus.c | 2 +- src/qemu/qemu_passt.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_security.c | 5 ++- src/qemu/qemu_security.h | 1 + src/qemu/qemu_slirp.c | 2 +- src/qemu/qemu_tpm.c | 3 +- src/qemu/qemu_vhost_user_gpu.c | 2 +- src/security/security_apparmor.c | 1 + src/security/security_dac.c | 1 + src/security/security_driver.h | 1 + src/security/security_manager.c | 8 +++- src/security/security_manager.h | 1 + src/security/security_nop.c | 1 + src/security/security_selinux.c | 77 ++++++++++++++++++++++++++++++-- src/security/security_stack.c | 5 ++- src/util/vircommand.c | 51 ++++++++++++++++----- src/util/vircommand.h | 1 + 19 files changed, 143 insertions(+), 24 deletions(-)
Reviewed-by: Michal Privoznik
On Fri, Mar 10, 2023 at 12:58:46PM +0100, Michal Prívozník wrote:
On 3/9/23 05:49, Laine Stump wrote:
Laine Stump (4): util: add an API to retrieve the resolved path to a virCommand's binary security: make args to virSecuritySELinuxContextAddRange() const security: make it possible to set SELinux label of child process from its binary qemu: set SELinux label of passt process to its own binary's label
src/libvirt_private.syms | 1 + src/qemu/qemu_dbus.c | 2 +- src/qemu/qemu_passt.c | 2 +- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_security.c | 5 ++- src/qemu/qemu_security.h | 1 + src/qemu/qemu_slirp.c | 2 +- src/qemu/qemu_tpm.c | 3 +- src/qemu/qemu_vhost_user_gpu.c | 2 +- src/security/security_apparmor.c | 1 + src/security/security_dac.c | 1 + src/security/security_driver.h | 1 + src/security/security_manager.c | 8 +++- src/security/security_manager.h | 1 + src/security/security_nop.c | 1 + src/security/security_selinux.c | 77 ++++++++++++++++++++++++++++++-- src/security/security_stack.c | 5 ++- src/util/vircommand.c | 51 ++++++++++++++++----- src/util/vircommand.h | 1 + 19 files changed, 143 insertions(+), 24 deletions(-)
Reviewed-by: Andrea Bolognani
Does this mean, we should lift the temporary limitation documented in NEWS.rst?
Yes, we should definitely include that information in the release notes. And since I've just pushed the patch that addresses the same limitation for AppArmor, we can mention both in the same entry. -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
Laine Stump
-
Michal Prívozník