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"
+ };
+
+ for (i = 0; i < G_N_ELEMENTS(knownModels); i++) {
+ if (STREQ_NULLABLE(secmodel, knownModels[i])) {
+ VIR_INFO("Ignoring seclabel with model %s and relabel=no", secmodel);
+ return 0;
+ }
+ }
+ }
+
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Security driver model '%1$s' is not available"),
secmodel);
@@ -758,8 +772,11 @@ virSecurityManagerCheckDomainLabel(virSecurityManager *mgr,
size_t i;
for (i = 0; i < def->nseclabels; i++) {
- if (virSecurityManagerCheckModel(mgr, def->seclabels[i]->model) < 0)
+ if (virSecurityManagerCheckModel(mgr,
+ def->seclabels[i]->model,
+ def->seclabels[i]->relabel) < 0) {
return -1;
+ }
}
return 0;
@@ -773,8 +790,11 @@ virSecurityManagerCheckDiskLabel(virSecurityManager *mgr,
size_t i;
for (i = 0; i < disk->src->nseclabels; i++) {
- if (virSecurityManagerCheckModel(mgr, disk->src->seclabels[i]->model) < 0)
+ if (virSecurityManagerCheckModel(mgr,
+ disk->src->seclabels[i]->model,
+ disk->src->seclabels[i]->relabel) < 0) {
return -1;
+ }
}
return 0;
@@ -788,8 +808,11 @@ virSecurityManagerCheckChardevLabel(virSecurityManager *mgr,
size_t i;
for (i = 0; i < dev->source->nseclabels; i++) {
- if (virSecurityManagerCheckModel(mgr, dev->source->seclabels[i]->model) < 0)
+ if (virSecurityManagerCheckModel(mgr,
+ dev->source->seclabels[i]->model,
+ dev->source->seclabels[i]->relabel) < 0) {
return -1;
+ }
}
return 0;
--
2.52.0On 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
> +
> + 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.
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
I tested the series and it resolves the issue, thus:
Tested-by: Richard W.M. Jones <rjones@redhat.com>
- - -
I wanted to clarify how we are using <seclabel/> since the original
description on RHEL-156689 was wrong (I've updated it there).
We put this in the main body of the XML:
<seclabel type="none" model="selinux"/>
We put this into some <disk/> sections:
<disk device="disk" type="file">
<source file="scratch1.img">
<seclabel model="selinux" relabel="no"/>
</source>
I was concerned that in your comment you said this second usage of
<seclabel/> is wrong. However the documentation seems to contradict
this (although the documentation is not very clear). Is the second
usage above correct or not?
There's no simple way to check the XML we generate against the RNG, so
if libvirt accepts it then we won't find out if it's technically
wrong.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
© 2016 - 2026 Red Hat, Inc.