Re: [PATCH 2/2] security: Avoid calling virSecurityManagerCheckModel with NULL model
On 12/3/20 3:57 AM, Jim Fehlig wrote:
Attempting to create a domain with <seclabel
type='none'/> results in
virsh --connect lxc:/// create distro_nosec.xml
error: Failed to create domain from distro_nosec.xml
error: unsupported configuration: Security driver model '(null)' is not
available
With <seclabel type='none'/>, the model field of virSecurityLabelDef will
be NULL, causing virSecurityManagerCheckModel() to fail with the above
error. Avoid calling virSecurityManagerCheckModel() when they seclabel
type is VIR_DOMAIN_SECLABEL_NONE.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
This could also be fixed by checking for a NULL secmodel in
virSecurityManagerCheckModel, but it seems more appropriate to check for
a valid seclabel type before checking the model.
src/security/security_manager.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index be81ee5e44..789e24d273 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -781,6 +781,9 @@ virSecurityManagerCheckDomainLabel(virSecurityManagerPtr mgr,
size_t i;
for (i = 0; i < def->nseclabels; i++) {
+ if (def->seclabels[i]->type == VIR_DOMAIN_SECLABEL_NONE)
+ continue;
+
if (virSecurityManagerCheckModel(mgr, def->seclabels[i]->model) < 0)
return -1;
}
This doesn't look right. If type='none' then ->model should contain
"none" string which in turn means virSecurityManagerCheckModel() is a NOP.
Looking into the code I've found the following in src/conf/domain_conf.c
line 9239:
/* Copy model from host. */
VIR_DEBUG("Found seclabel without a model, using '%s'",
xmlopt->config.defSecModel);
def->seclabels[0]->model = g_strdup(xmlopt->config.defSecModel);
and looking around and on git blame I found the following commit:
638ffa222847acc74dd2d84d2088590ecbf8eb70 (let's not get into who
reviewed it O:-)) which made ->model initialization happen only if
drvier initialized xmlopt .defSecModel which is happening only in qemu
driver and not LXC. Therefore I think we need something like this:
diff --git i/src/lxc/lxc_conf.c w/src/lxc/lxc_conf.c
index 13da6c4586..fc26a0a38b 100644
--- i/src/lxc/lxc_conf.c
+++ w/src/lxc/lxc_conf.c
@@ -212,6 +212,7 @@ virDomainXMLOptionPtr
lxcDomainXMLConfInit(virLXCDriverPtr driver)
{
virLXCDriverDomainDefParserConfig.priv = driver;
+ virLXCDriverDomainDefParserConfig.defSecModel = "none";
return virDomainXMLOptionNew(&virLXCDriverDomainDefParserConfig,
&virLXCDriverPrivateDataCallbacks,
&virLXCDriverDomainXMLNamespace,
But that makes lxcxml2xmltest fail, because the output XMLs don't
contain the model. I'm wondering if that's because we did not/do not
pass caps with host->nsecModels > 0 and thus this autofill wasn't run
but with the change I'm suggesting it is now.
Michal