On 3/31/26 14:52, Peter Krempa wrote:
On Thu, Mar 26, 2026 at 12:51:25 +0100, Michal Privoznik via Devel wrote:
From: Michal Privoznik <mprivozn@redhat.com>
Ever since of commit v1.2.13-rc1~66 the model attribute of a <seclabel/> is validated against secdriver names enabled. In nearly all cases this is something users want so that domain XML does not claim to set seclabels of a model that's not enabled. However, consider the following seclabel:
<seclabel type='none' model='selinux'/>
It tells us to not bother setting selinux labels on given domain. A mgmt app might format this into domain XML if it sees selinux is disabled on the host. But if that's the case, selinux driver is not loaded and this virSecurityManagerCheckModel() doesn't find it and reports an error.
Well, the error doesn't need to be reported as we will just ignore selinux as each driver callback checks if relabel is false (which it is for type='none'). This is true for other secdrivers too.
Resolves: https://redhat.atlassian.net/browse/RHEL-156689 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_manager.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f2f3bb4f19..7023ac2db8 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -727,7 +727,8 @@ virSecurityManagerReleaseLabel(virSecurityManager *mgr,
static int virSecurityManagerCheckModel(virSecurityManager *mgr, - char *secmodel) + char *secmodel, + bool relabel) { g_autofree virSecurityManager **sec_managers = NULL; size_t i; @@ -744,6 +745,19 @@ static int virSecurityManagerCheckModel(virSecurityManager *mgr, } }
+ if (relabel == false) { + const char * const knownModels[] = { + "none", "apparmor", "dac", "selinux" + };
I don't like the hardcoded list of security drivers
Yeah, me neither. But there's hardly going to be a new one.
+ + for (i = 0; i < G_N_ELEMENTS(knownModels); i++) {
If you NULL-terminate the list you can use g_strv_contains instead of the loop.
+ if (STREQ_NULLABLE(secmodel, knownModels[i])) { + VIR_INFO("Ignoring seclabel with model %s and relabel=no", secmodel); + return 0; + }
I was considering if this should behave differently if the secdriver is explicitly disabled rather than disabled either by autoprobe or not even compiled in.
Since for the existing secdrivers we disable them by disabling "relabel" (e.g. there are no necessary steps to avoid confinment) I guess this is okay.
IMO to avoid the hardcoded list you IMO should ignore any label requesting disabling of given security driver if the driver is not enabled. That includes unknown ones. I don't think there's a need to report error for those.
Fair enough. I've got stuck on a corner case where I set the following seclabel: <seclabel type='none' model='MyOwnModel'/> and it was formatted back into the live XML. But maybe there's nothing wrong with that. Alright, for completeness, let me post v2 which drops knownModels[]. Michal